diff --git a/overlord/hookstate/ctlcmd/set.go b/overlord/hookstate/ctlcmd/set.go index d3649e64af2..3ca6815eda0 100644 --- 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 index b29c98ec094..821a3595b35 100644 --- a/overlord/hookstate/ctlcmd/unset.go +++ b/overlord/hookstate/ctlcmd/unset.go @@ -20,7 +20,9 @@ package ctlcmd import ( + "errors" "fmt" + "strings" "github.com/snapcore/snapd/i18n" "github.com/snapcore/snapd/overlord/configstate" @@ -28,6 +30,7 @@ import ( 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 \":\": %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 index 17045be5215..01e999bf2f7 100644 --- 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 ":": 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) + } +}