o/h/ctlcmd: support unsetting registry data in snapctl (#14218)

* o/h/ctlcmd: support unsetting registry data in snapctl Signed-off-by: Miguel Pires <[email protected]> * o/h/ctlcmd: minor refactor Signed-off-by: Miguel Pires <[email protected]> --------- Signed-off-by: Miguel Pires <[email protected]>
canonical · Jul 30, 2024 · 44970fd · 44970fd
1 parent 5a782b1
commit 44970fd
Show file tree

Hide file tree

Showing 3 changed files with 111 additions and 5 deletions.
diff --git a/overlord/hookstate/ctlcmd/set.go b/overlord/hookstate/ctlcmd/set.go
@@ -111,7 +111,7 @@ func (s *setCommand) Execute(args []string) error {
 			return fmt.Errorf(i18n.G("cannot set %s plug: %w"), s.Positional.PlugOrSlotSpec, err)
 		}
 
-		return s.setRegistryValues(context, name, requests)
+		return setRegistryValues(context, name, requests)
 	}
 
 	return s.setInterfaceSetting(context, name)
@@ -225,7 +225,7 @@ func (s *setCommand) setInterfaceSetting(context *hookstate.Context, plugOrSlot
 	return nil
 }
 
-func (s *setCommand) setRegistryValues(ctx *hookstate.Context, plugName string, requests map[string]interface{}) error {
+func setRegistryValues(ctx *hookstate.Context, plugName string, requests map[string]interface{}) error {
 	ctx.Lock()
 	defer ctx.Unlock()
 

diff --git a/overlord/hookstate/ctlcmd/unset.go b/overlord/hookstate/ctlcmd/unset.go
@@ -20,14 +20,17 @@
 package ctlcmd
 
 import (
+	"errors"
 	"fmt"
+	"strings"
 
 	"github.com/snapcore/snapd/i18n"
 	"github.com/snapcore/snapd/overlord/configstate"
 )
 
 type unsetCommand struct {
 	baseCommand
+	View bool `long:"view" description:"unset registry values in the view declared in the plug"`
 
 	Positional struct {
 		ConfKeys []string
@@ -66,9 +69,32 @@ func (s *unsetCommand) Execute(args []string) error {
 	tr := configstate.ContextTransaction(context)
 	context.Unlock()
 
-	for _, confKey := range s.Positional.ConfKeys {
-		tr.Set(context.InstanceName(), confKey, nil)
+	if !s.View {
+		// unsetting options
+		for _, confKey := range s.Positional.ConfKeys {
+			tr.Set(context.InstanceName(), confKey, nil)
+		}
+		return nil
 	}
 
-	return nil
+	// unsetting registry data
+	if !strings.HasPrefix(s.Positional.ConfKeys[0], ":") {
+		return fmt.Errorf(i18n.G("cannot unset registry: plug must conform to format \":<plug-name>\": %s"), s.Positional.ConfKeys[0])
+	}
+
+	plugName := strings.TrimPrefix(s.Positional.ConfKeys[0], ":")
+	if plugName == "" {
+		return errors.New(i18n.G("cannot unset registry: plug name was not provided"))
+	}
+
+	if len(s.Positional.ConfKeys) == 1 {
+		return errors.New(i18n.G("cannot unset registry: no paths provided to unset"))
+	}
+
+	confs := make(map[string]interface{}, len(s.Positional.ConfKeys)-1)
+	for _, key := range s.Positional.ConfKeys[1:] {
+		confs[key] = nil
+	}
+
+	return setRegistryValues(context, plugName, confs)
 }
diff --git a/overlord/hookstate/ctlcmd/unset_test.go b/overlord/hookstate/ctlcmd/unset_test.go
@@ -28,6 +28,7 @@ import (
 	"github.com/snapcore/snapd/overlord/hookstate"
 	"github.com/snapcore/snapd/overlord/hookstate/ctlcmd"
 	"github.com/snapcore/snapd/overlord/hookstate/hooktest"
+	"github.com/snapcore/snapd/overlord/registrystate"
 	"github.com/snapcore/snapd/overlord/state"
 	"github.com/snapcore/snapd/snap"
 )
@@ -160,3 +161,82 @@ func (s *unsetSuite) TestCommandWithoutContext(c *C) {
 	_, _, err := ctlcmd.Run(nil, []string{"unset", "foo"}, 0)
 	c.Check(err, ErrorMatches, `cannot invoke snapctl operation commands \(here "unset"\) from outside of a snap`)
 }
+
+func (s *registrySuite) TestRegistryUnsetManyViews(c *C) {
+	s.state.Lock()
+	err := registrystate.SetViaView(s.state, s.devAccID, "network", "write-wifi", map[string]interface{}{"ssid": "my-ssid", "password": "my-secret"})
+	s.state.Unlock()
+	c.Assert(err, IsNil)
+
+	stdout, stderr, err := ctlcmd.Run(s.mockContext, []string{"unset", "--view", ":write-wifi", "ssid", "password"}, 0)
+	c.Assert(err, IsNil)
+	c.Check(stdout, IsNil)
+	c.Check(stderr, IsNil)
+	s.mockContext.Lock()
+	c.Assert(s.mockContext.Done(), IsNil)
+	s.mockContext.Unlock()
+
+	s.state.Lock()
+	_, err = registrystate.GetViaView(s.state, s.devAccID, "network", "write-wifi", []string{"ssid", "password"})
+	s.state.Unlock()
+	c.Assert(err, ErrorMatches, `cannot get "ssid", "password" .*: matching rules don't map to any values`)
+}
+
+func (s *registrySuite) TestRegistryUnsetHappensTransactionally(c *C) {
+	s.state.Lock()
+	err := registrystate.SetViaView(s.state, s.devAccID, "network", "write-wifi", map[string]interface{}{"ssid": "my-ssid"})
+	s.state.Unlock()
+	c.Assert(err, IsNil)
+
+	stdout, stderr, err := ctlcmd.Run(s.mockContext, []string{"unset", "--view", ":write-wifi", "ssid"}, 0)
+	c.Assert(err, IsNil)
+	c.Check(stdout, IsNil)
+	c.Check(stderr, IsNil)
+
+	s.state.Lock()
+	val, err := registrystate.GetViaView(s.state, s.devAccID, "network", "read-wifi", []string{"ssid"})
+	s.state.Unlock()
+	c.Assert(err, IsNil)
+	c.Assert(val, DeepEquals, map[string]interface{}{
+		"ssid": "my-ssid",
+	})
+
+	// commit transaction
+	s.mockContext.Lock()
+	c.Assert(s.mockContext.Done(), IsNil)
+	s.mockContext.Unlock()
+
+	s.state.Lock()
+	_, err = registrystate.GetViaView(s.state, s.devAccID, "network", "read-wifi", []string{"ssid"})
+	s.state.Unlock()
+	c.Assert(err, ErrorMatches, `cannot get "ssid" .*: matching rules don't map to any values`)
+}
+
+func (s *registrySuite) TestRegistryUnsetInvalid(c *C) {
+	type testcase struct {
+		args []string
+		err  string
+	}
+
+	tcs := []testcase{
+		{
+			args: []string{"snap:plug"},
+			err:  `cannot unset registry: plug must conform to format ":<plug-name>": snap:plug`,
+		},
+		{
+			args: []string{":"},
+			err:  `cannot unset registry: plug name was not provided`,
+		},
+		{
+			args: []string{":plug"},
+			err:  `cannot unset registry: no paths provided to unset`,
+		},
+	}
+
+	for _, tc := range tcs {
+		stdout, stderr, err := ctlcmd.Run(s.mockContext, append([]string{"unset", "--view"}, tc.args...), 0)
+		c.Assert(err, ErrorMatches, tc.err)
+		c.Check(stdout, IsNil)
+		c.Check(stderr, IsNil)
+	}
+}