diff --git a/cmd/snap-exec/main.go b/cmd/snap-exec/main.go index cd7ff344ae..07e745cf6c 100644 --- a/cmd/snap-exec/main.go +++ b/cmd/snap-exec/main.go @@ -43,6 +43,11 @@ var opts struct { Hook string `long:"hook" description:"hook to run" hidden:"yes"` } +func init() { + // plug/slot sanitization not used nor possible from snap-exec, make it no-op + snap.SanitizePlugsSlots = func(snapInfo *snap.Info) {} +} + func main() { if err := run(); err != nil { fmt.Fprintf(os.Stderr, "cannot snap-exec: %s\n", err) diff --git a/cmd/snap/main.go b/cmd/snap/main.go index 1d58ce3b32..8d74786521 100644 --- a/cmd/snap/main.go +++ b/cmd/snap/main.go @@ -38,11 +38,14 @@ import ( "github.com/snapcore/snapd/i18n" "github.com/snapcore/snapd/logger" "github.com/snapcore/snapd/osutil" + "github.com/snapcore/snapd/snap" ) func init() { // set User-Agent for when 'snap' talks to the store directly (snap download etc...) httputil.SetUserAgentFromVersion(cmd.Version, "snap") + // plug/slot sanitization not used nor possible from snap command, make it no-op + snap.SanitizePlugsSlots = func(snapInfo *snap.Info) {} } // Standard streams, redirected for testing. diff --git a/cmd/snap/main_test.go b/cmd/snap/main_test.go index 100a78ec46..3fddd9e752 100644 --- a/cmd/snap/main_test.go +++ b/cmd/snap/main_test.go @@ -37,6 +37,7 @@ import ( "github.com/snapcore/snapd/cmd" "github.com/snapcore/snapd/dirs" "github.com/snapcore/snapd/osutil" + snapdsnap "github.com/snapcore/snapd/snap" "github.com/snapcore/snapd/testutil" snap "github.com/snapcore/snapd/cmd/snap" @@ -74,6 +75,8 @@ func (s *BaseSnapSuite) SetUpTest(c *C) { snap.ReadPassword = s.readPassword s.AuthFile = filepath.Join(c.MkDir(), "json") os.Setenv(TestAuthFileEnvKey, s.AuthFile) + + snapdsnap.MockSanitizePlugsSlots(func(snapInfo *snapdsnap.Info) {}) } func (s *BaseSnapSuite) TearDownTest(c *C) { diff --git a/corecfg/services_test.go b/corecfg/services_test.go index 9543e4bd32..45678d249a 100644 --- a/corecfg/services_test.go +++ b/corecfg/services_test.go @@ -29,23 +29,28 @@ import ( "github.com/snapcore/snapd/corecfg" "github.com/snapcore/snapd/dirs" "github.com/snapcore/snapd/release" + "github.com/snapcore/snapd/snap" "github.com/snapcore/snapd/testutil" ) type servicesSuite struct { coreCfgSuite + testutil.BaseTest } var _ = Suite(&servicesSuite{}) func (s *servicesSuite) SetUpTest(c *C) { + s.BaseTest.SetUpTest(c) dirs.SetRootDir(c.MkDir()) c.Assert(os.MkdirAll(filepath.Join(dirs.GlobalRootDir, "etc"), 0755), IsNil) s.systemctlArgs = nil + s.BaseTest.AddCleanup(snap.MockSanitizePlugsSlots(func(snapInfo *snap.Info) {})) } func (s *servicesSuite) TearDownTest(c *C) { dirs.SetRootDir("/") + s.BaseTest.TearDownTest(c) } func (s *servicesSuite) TestConfigureServiceInvalidValue(c *C) { diff --git a/interfaces/builtin/all.go b/interfaces/builtin/all.go index 6582df7203..3356742071 100644 --- a/interfaces/builtin/all.go +++ b/interfaces/builtin/all.go @@ -24,8 +24,13 @@ import ( "sort" "github.com/snapcore/snapd/interfaces" + "github.com/snapcore/snapd/snap" ) +func init() { + snap.SanitizePlugsSlots = SanitizePlugsSlots +} + var ( allInterfaces map[string]interfaces.Interface ) @@ -51,6 +56,44 @@ func registerIface(iface interfaces.Interface) { allInterfaces[iface.Name()] = iface } +func SanitizePlugsSlots(snapInfo *snap.Info) { + for plugName, plugInfo := range snapInfo.Plugs { + iface, ok := allInterfaces[plugInfo.Interface] + if !ok { + snapInfo.BadInterfaces[plugName] = fmt.Sprintf("unknown interface %q", plugInfo.Interface) + continue + } + // Reject plug with invalid name + if err := interfaces.ValidateName(plugName); err != nil { + snapInfo.BadInterfaces[plugName] = err.Error() + continue + } + plug := &interfaces.Plug{PlugInfo: plugInfo} + if err := plug.Sanitize(iface); err != nil { + snapInfo.BadInterfaces[plugName] = err.Error() + continue + } + } + + for slotName, slotInfo := range snapInfo.Slots { + iface, ok := allInterfaces[slotInfo.Interface] + if !ok { + snapInfo.BadInterfaces[slotName] = fmt.Sprintf("unknown interface %q", slotInfo.Interface) + continue + } + // Reject slot with invalid name + if err := interfaces.ValidateName(slotName); err != nil { + snapInfo.BadInterfaces[slotName] = err.Error() + continue + } + slot := &interfaces.Slot{SlotInfo: slotInfo} + if err := slot.Sanitize(iface); err != nil { + snapInfo.BadInterfaces[slotName] = err.Error() + continue + } + } +} + type byIfaceName []interfaces.Interface func (c byIfaceName) Len() int { return len(c) } diff --git a/interfaces/builtin/all_test.go b/interfaces/builtin/all_test.go index 36443f092a..23a5072edb 100644 --- a/interfaces/builtin/all_test.go +++ b/interfaces/builtin/all_test.go @@ -32,6 +32,8 @@ import ( "github.com/snapcore/snapd/interfaces/seccomp" "github.com/snapcore/snapd/interfaces/systemd" "github.com/snapcore/snapd/interfaces/udev" + "github.com/snapcore/snapd/snap" + "github.com/snapcore/snapd/snap/snaptest" . "gopkg.in/check.v1" ) @@ -307,3 +309,47 @@ func (s *AllSuite) TestRegisterIface(c *C) { // Duplicates are detected. c.Assert(func() { builtin.RegisterIface(iface) }, PanicMatches, `cannot register duplicate interface "foo"`) } + +const testConsumerInvalidSlotNameYaml = ` +name: consumer +slots: + ttyS5: + interface: iface +apps: + app: + slots: [iface] +` + +const testConsumerInvalidPlugNameYaml = ` +name: consumer +plugs: + ttyS3: + interface: iface +apps: + app: + plugs: [iface] +` + +func (s *AllSuite) TestSanitizeErrorsOnInvalidSlotNames(c *C) { + restore := builtin.MockInterfaces(map[string]interfaces.Interface{ + "iface": &ifacetest.TestInterface{InterfaceName: "iface"}, + }) + defer restore() + + snapInfo := snaptest.MockInfo(c, testConsumerInvalidSlotNameYaml, nil) + snap.SanitizePlugsSlots(snapInfo) + c.Assert(snapInfo.BadInterfaces, HasLen, 1) + c.Check(snap.BadInterfacesSummary(snapInfo), Matches, `snap "consumer" has bad plugs or slots: ttyS5 \(invalid interface name: "ttyS5"\)`) +} + +func (s *AllSuite) TestSanitizeErrorsOnInvalidPlugNames(c *C) { + restore := builtin.MockInterfaces(map[string]interfaces.Interface{ + "iface": &ifacetest.TestInterface{InterfaceName: "iface"}, + }) + defer restore() + + snapInfo := snaptest.MockInfo(c, testConsumerInvalidPlugNameYaml, nil) + snap.SanitizePlugsSlots(snapInfo) + c.Assert(snapInfo.BadInterfaces, HasLen, 1) + c.Check(snap.BadInterfacesSummary(snapInfo), Matches, `snap "consumer" has bad plugs or slots: ttyS3 \(invalid interface name: "ttyS3"\)`) +} diff --git a/interfaces/repo.go b/interfaces/repo.go index da05ff603f..024a1bb8af 100644 --- a/interfaces/repo.go +++ b/interfaces/repo.go @@ -20,7 +20,6 @@ package interfaces import ( - "bytes" "fmt" "sort" "strings" @@ -46,7 +45,7 @@ type Repository struct { // NewRepository creates an empty plug repository. func NewRepository() *Repository { - return &Repository{ + repo := &Repository{ ifaces: make(map[string]Interface), plugs: make(map[string]map[string]*Plug), slots: make(map[string]map[string]*Slot), @@ -54,6 +53,8 @@ func NewRepository() *Repository { plugSlots: make(map[*Plug]map[*Slot]*Connection), backends: make(map[SecuritySystem]SecurityBackend), } + + return repo } // Interface returns an interface with a given name. @@ -815,39 +816,6 @@ func (r *Repository) SnapSpecification(securitySystem SecuritySystem, snapName s return spec, nil } -// BadInterfacesError is returned when some snap interfaces could not be registered. -// Those interfaces not mentioned in the error were successfully registered. -type BadInterfacesError struct { - snap string - issues map[string]string // slot or plug name => message -} - -func (e *BadInterfacesError) Error() string { - inverted := make(map[string][]string) - for name, reason := range e.issues { - inverted[reason] = append(inverted[reason], name) - } - var buf bytes.Buffer - fmt.Fprintf(&buf, "snap %q has bad plugs or slots: ", e.snap) - reasons := make([]string, 0, len(inverted)) - for reason := range inverted { - reasons = append(reasons, reason) - } - sort.Strings(reasons) - for _, reason := range reasons { - names := inverted[reason] - sort.Strings(names) - for i, name := range names { - if i > 0 { - buf.WriteString(", ") - } - buf.WriteString(name) - } - fmt.Fprintf(&buf, " (%s); ", reason) - } - return strings.TrimSuffix(buf.String(), "; ") -} - // AddSnap adds plugs and slots declared by the given snap to the repository. // // This function can be used to implement snap install or, when used along with @@ -876,58 +844,21 @@ func (r *Repository) AddSnap(snapInfo *snap.Info) error { return fmt.Errorf("cannot register interfaces for snap %q more than once", snapName) } - bad := BadInterfacesError{ - snap: snapName, - issues: make(map[string]string), - } - for plugName, plugInfo := range snapInfo.Plugs { - iface, ok := r.ifaces[plugInfo.Interface] - if !ok { - bad.issues[plugName] = "unknown interface" - continue - } - // Reject plug with invalid name - if err := ValidateName(plugName); err != nil { - bad.issues[plugName] = err.Error() - continue - } - plug := &Plug{PlugInfo: plugInfo} - if err := plug.Sanitize(iface); err != nil { - bad.issues[plugName] = err.Error() - continue - } if r.plugs[snapName] == nil { r.plugs[snapName] = make(map[string]*Plug) } + plug := &Plug{PlugInfo: plugInfo} r.plugs[snapName][plugName] = plug } for slotName, slotInfo := range snapInfo.Slots { - iface, ok := r.ifaces[slotInfo.Interface] - if !ok { - bad.issues[slotName] = "unknown interface" - continue - } - // Reject slot with invalid name - if err := ValidateName(slotName); err != nil { - bad.issues[slotName] = err.Error() - continue - } - slot := &Slot{SlotInfo: slotInfo} - if err := slot.Sanitize(iface); err != nil { - bad.issues[slotName] = err.Error() - continue - } if r.slots[snapName] == nil { r.slots[snapName] = make(map[string]*Slot) } + slot := &Slot{SlotInfo: slotInfo} r.slots[snapName][slotName] = slot } - - if len(bad.issues) > 0 { - return &bad - } return nil } diff --git a/interfaces/repo_test.go b/interfaces/repo_test.go index 1901155d12..55317b36e2 100644 --- a/interfaces/repo_test.go +++ b/interfaces/repo_test.go @@ -1539,38 +1539,6 @@ func (s *AddRemoveSuite) SetUpTest(c *C) { c.Assert(err, IsNil) } -func (s *AddRemoveSuite) TestAddSnapComplexErrorHandling(c *C) { - err := s.repo.AddInterface(&ifacetest.TestInterface{ - InterfaceName: "invalid-plug-iface", - SanitizePlugCallback: func(plug *Plug) error { return fmt.Errorf("plug is invalid") }, - SanitizeSlotCallback: func(slot *Slot) error { return fmt.Errorf("slot is invalid") }, - }) - c.Assert(err, IsNil) - err = s.repo.AddInterface(&ifacetest.TestInterface{ - InterfaceName: "invalid-slot-iface", - SanitizePlugCallback: func(plug *Plug) error { return fmt.Errorf("plug is invalid") }, - SanitizeSlotCallback: func(slot *Slot) error { return fmt.Errorf("slot is invalid") }, - }) - c.Assert(err, IsNil) - snapInfo := snaptest.MockInfo(c, ` -name: complex -plugs: - invalid-plug-iface: - unknown-plug-iface: -slots: - invalid-slot-iface: - unknown-slot-iface: -`, nil) - err = s.repo.AddSnap(snapInfo) - c.Check(err, ErrorMatches, - `snap "complex" has bad plugs or slots: invalid-plug-iface \(plug is invalid\); invalid-slot-iface \(slot is invalid\); unknown-plug-iface, unknown-slot-iface \(unknown interface\)`) - // Nothing was added - c.Check(s.repo.Plug("complex", "invalid-plug-iface"), IsNil) - c.Check(s.repo.Plug("complex", "unknown-plug-iface"), IsNil) - c.Check(s.repo.Slot("complex", "invalid-slot-iface"), IsNil) - c.Check(s.repo.Slot("complex", "unknown-slot-iface"), IsNil) -} - const testConsumerYaml = ` name: consumer apps: @@ -1616,18 +1584,6 @@ func (s *AddRemoveSuite) TestAddSnapAddsPlugs(c *C) { c.Assert(s.repo.Plug("consumer", "iface"), Not(IsNil)) } -func (s *AddRemoveSuite) TestAddSnapErrorsOnInvalidSlotNames(c *C) { - _, err := s.addSnap(c, testConsumerInvalidSlotNameYaml) - c.Assert(err, NotNil) - c.Check(err, ErrorMatches, `snap "consumer" has bad plugs or slots: ttyS5 \(invalid interface name: "ttyS5"\)`) -} - -func (s *AddRemoveSuite) TestAddSnapErrorsOnInvalidPlugNames(c *C) { - _, err := s.addSnap(c, testConsumerInvalidPlugNameYaml) - c.Assert(err, NotNil) - c.Check(err, ErrorMatches, `snap "consumer" has bad plugs or slots: ttyS3 \(invalid interface name: "ttyS3"\)`) -} - func (s *AddRemoveSuite) TestAddSnapErrorsOnExistingSnapPlugs(c *C) { _, err := s.addSnap(c, testConsumerYaml) c.Assert(err, IsNil) diff --git a/overlord/configstate/handler_test.go b/overlord/configstate/handler_test.go index de624870b9..20f69e466d 100644 --- a/overlord/configstate/handler_test.go +++ b/overlord/configstate/handler_test.go @@ -43,6 +43,7 @@ type configureHandlerSuite struct { state *state.State context *hookstate.Context handler hookstate.Handler + restore func() } var _ = Suite(&configureHandlerSuite{}) @@ -52,6 +53,8 @@ func (s *configureHandlerSuite) SetUpTest(c *C) { s.state.Lock() defer s.state.Unlock() + s.restore = snap.MockSanitizePlugsSlots(func(snapInfo *snap.Info) {}) + task := s.state.NewTask("test-task", "my test task") setup := &hookstate.HookSetup{Snap: "test-snap", Revision: snap.R(1), Hook: "test-hook"} @@ -62,6 +65,10 @@ func (s *configureHandlerSuite) SetUpTest(c *C) { s.handler = configstate.NewConfigureHandler(s.context) } +func (s *configureHandlerSuite) TearDownTest(c *C) { + s.restore() +} + func (s *configureHandlerSuite) TestBeforeInitializesTransaction(c *C) { // Initialize context s.context.Lock() diff --git a/overlord/devicestate/devicestate_test.go b/overlord/devicestate/devicestate_test.go index 6aa86585f8..d8d670aa45 100644 --- a/overlord/devicestate/devicestate_test.go +++ b/overlord/devicestate/devicestate_test.go @@ -77,6 +77,7 @@ type deviceMgrSuite struct { restoreOnClassic func() restoreGenericClassicMod func() + restoreSanitize func() } var _ = Suite(&deviceMgrSuite{}) @@ -106,6 +107,8 @@ func (s *deviceMgrSuite) SetUpTest(c *C) { dirs.SetRootDir(c.MkDir()) os.MkdirAll(dirs.SnapRunDir, 0755) + s.restoreSanitize = snap.MockSanitizePlugsSlots(func(snapInfo *snap.Info) {}) + s.bootloader = boottest.NewMockBootloader("mock", c.MkDir()) partition.ForceBootloader(s.bootloader) @@ -161,6 +164,7 @@ func (s *deviceMgrSuite) TearDownTest(c *C) { dirs.SetRootDir("") s.restoreGenericClassicMod() s.restoreOnClassic() + s.restoreSanitize() } var settleTimeout = 15 * time.Second diff --git a/overlord/hookstate/ctlcmd/services_test.go b/overlord/hookstate/ctlcmd/services_test.go index 6a4bd08d32..12a1b787d5 100644 --- a/overlord/hookstate/ctlcmd/services_test.go +++ b/overlord/hookstate/ctlcmd/services_test.go @@ -32,13 +32,13 @@ import ( "github.com/snapcore/snapd/overlord/state" "github.com/snapcore/snapd/snap" "github.com/snapcore/snapd/snap/snaptest" + "github.com/snapcore/snapd/testutil" ) type servicectlSuite struct { + testutil.BaseTest mockContext *hookstate.Context mockHandler *hooktest.MockHandler - - restore func() } var _ = Suite(&servicectlSuite{}) @@ -77,11 +77,14 @@ func mockServiceChangeFunc(testServiceControlInputs func(appInfos []*snap.AppInf } func (s *servicectlSuite) SetUpTest(c *C) { + s.BaseTest.SetUpTest(c) oldRoot := dirs.GlobalRootDir dirs.SetRootDir(c.MkDir()) - s.restore = func() { + + s.BaseTest.AddCleanup(func() { dirs.SetRootDir(oldRoot) - } + }) + s.BaseTest.AddCleanup(snap.MockSanitizePlugsSlots(func(snapInfo *snap.Info) {})) s.mockHandler = hooktest.NewMockHandler() @@ -128,7 +131,7 @@ func (s *servicectlSuite) SetUpTest(c *C) { } func (s *servicectlSuite) TearDownTest(c *C) { - s.restore() + s.BaseTest.TearDownTest(c) } func (s *servicectlSuite) TestStopCommand(c *C) { diff --git a/overlord/hookstate/hookstate_test.go b/overlord/hookstate/hookstate_test.go index e38f4372c7..b4be232961 100644 --- a/overlord/hookstate/hookstate_test.go +++ b/overlord/hookstate/hookstate_test.go @@ -94,6 +94,8 @@ func (s *hookManagerSuite) SetUpTest(c *C) { s.manager = manager s.o.AddManager(s.manager) + s.AddCleanup(snap.MockSanitizePlugsSlots(func(snapInfo *snap.Info) {})) + hooksup := &hookstate.HookSetup{ Snap: "test-snap", Hook: "configure", diff --git a/overlord/ifacestate/handlers.go b/overlord/ifacestate/handlers.go index f9f7671d09..eecf03119c 100644 --- a/overlord/ifacestate/handlers.go +++ b/overlord/ifacestate/handlers.go @@ -148,12 +148,12 @@ func (m *InterfaceManager) setupProfilesForSnap(task *state.Task, _ *tomb.Tomb, return err } if err := m.repo.AddSnap(snapInfo); err != nil { - if _, ok := err.(*interfaces.BadInterfacesError); ok { - task.Logf("%s", err) - } else { - return err - } + return err } + if len(snapInfo.BadInterfaces) > 0 { + task.Logf("%s", snap.BadInterfacesSummary(snapInfo)) + } + if err := m.reloadConnections(snapName); err != nil { return err } diff --git a/overlord/ifacestate/ifacemgr.go b/overlord/ifacestate/ifacemgr.go index 90f4cf2939..0f344d9fc4 100644 --- a/overlord/ifacestate/ifacemgr.go +++ b/overlord/ifacestate/ifacemgr.go @@ -51,6 +51,7 @@ func Manager(s *state.State, hookManager *hookstate.HookManager, extraInterfaces runner: runner, repo: interfaces.NewRepository(), } + if err := m.initialize(extraInterfaces, extraBackends); err != nil { return nil, err } diff --git a/overlord/ifacestate/ifacestate_test.go b/overlord/ifacestate/ifacestate_test.go index 5068dbc159..99fa37eca6 100644 --- a/overlord/ifacestate/ifacestate_test.go +++ b/overlord/ifacestate/ifacestate_test.go @@ -46,22 +46,23 @@ import ( func TestInterfaceManager(t *testing.T) { TestingT(t) } type interfaceManagerSuite struct { - o *overlord.Overlord - state *state.State - db *asserts.Database - privateMgr *ifacestate.InterfaceManager - privateHookMgr *hookstate.HookManager - extraIfaces []interfaces.Interface - extraBackends []interfaces.SecurityBackend - secBackend *ifacetest.TestSecurityBackend - restoreBackends func() - mockSnapCmd *testutil.MockCmd - storeSigning *assertstest.StoreStack + testutil.BaseTest + o *overlord.Overlord + state *state.State + db *asserts.Database + privateMgr *ifacestate.InterfaceManager + privateHookMgr *hookstate.HookManager + extraIfaces []interfaces.Interface + extraBackends []interfaces.SecurityBackend + secBackend *ifacetest.TestSecurityBackend + mockSnapCmd *testutil.MockCmd + storeSigning *assertstest.StoreStack } var _ = Suite(&interfaceManagerSuite{}) func (s *interfaceManagerSuite) SetUpTest(c *C) { + s.BaseTest.SetUpTest(c) s.storeSigning = assertstest.NewStoreStack("canonical", nil) s.mockSnapCmd = testutil.MockCommand(c, "snap", "") @@ -78,6 +79,8 @@ func (s *interfaceManagerSuite) SetUpTest(c *C) { err = db.Add(s.storeSigning.StoreAccountKey("")) c.Assert(err, IsNil) + s.BaseTest.AddCleanup(snap.MockSanitizePlugsSlots(func(snapInfo *snap.Info) {})) + s.state.Lock() assertstate.ReplaceDB(s.state, s.db) s.state.Unlock() @@ -90,17 +93,18 @@ func (s *interfaceManagerSuite) SetUpTest(c *C) { // TODO: transition this so that we don't load real backends and instead // just load the test backend here and this is nicely integrated with // extraBackends above. - s.restoreBackends = ifacestate.MockSecurityBackends([]interfaces.SecurityBackend{s.secBackend}) + s.BaseTest.AddCleanup(ifacestate.MockSecurityBackends([]interfaces.SecurityBackend{s.secBackend})) } func (s *interfaceManagerSuite) TearDownTest(c *C) { + s.BaseTest.TearDownTest(c) + s.mockSnapCmd.Restore() if s.privateMgr != nil { s.privateMgr.Stop() } dirs.SetRootDir("") - s.restoreBackends() } func (s *interfaceManagerSuite) manager(c *C) *ifacestate.InterfaceManager { @@ -136,7 +140,7 @@ func (s *interfaceManagerSuite) TestSmoke(c *C) { } func (s *interfaceManagerSuite) TestConnectTask(c *C) { - s.mockIface(c, &ifacetest.TestInterface{InterfaceName: "test"}) + s.mockIfaces(c, &ifacetest.TestInterface{InterfaceName: "test"}, &ifacetest.TestInterface{InterfaceName: "test2"}) s.mockSnap(c, consumerYaml) s.mockSnap(c, producerYaml) _ = s.manager(c) @@ -262,7 +266,7 @@ func (s *interfaceManagerSuite) TestConnectDoesntConflict(c *C) { } func (s *interfaceManagerSuite) TestEnsureProcessesConnectTask(c *C) { - s.mockIface(c, &ifacetest.TestInterface{InterfaceName: "test"}) + s.mockIfaces(c, &ifacetest.TestInterface{InterfaceName: "test"}, &ifacetest.TestInterface{InterfaceName: "test2"}) s.mockSnap(c, consumerYaml) s.mockSnap(c, producerYaml) _ = s.manager(c) @@ -312,8 +316,7 @@ func (s *interfaceManagerSuite) TestEnsureProcessesConnectTask(c *C) { } func (s *interfaceManagerSuite) TestConnectTaskCheckInterfaceMismatch(c *C) { - s.mockIface(c, &ifacetest.TestInterface{InterfaceName: "test"}) - s.mockIface(c, &ifacetest.TestInterface{InterfaceName: "test2"}) + s.mockIfaces(c, &ifacetest.TestInterface{InterfaceName: "test"}, &ifacetest.TestInterface{InterfaceName: "test2"}) s.mockSnap(c, consumerYaml) s.mockSnap(c, producerYaml) _ = s.manager(c) @@ -346,7 +349,7 @@ func (s *interfaceManagerSuite) TestConnectTaskCheckInterfaceMismatch(c *C) { } func (s *interfaceManagerSuite) TestConnectTaskNoSuchSlot(c *C) { - s.mockIface(c, &ifacetest.TestInterface{InterfaceName: "test"}) + s.mockIfaces(c, &ifacetest.TestInterface{InterfaceName: "test"}, &ifacetest.TestInterface{InterfaceName: "test2"}) s.mockSnap(c, consumerYaml) s.mockSnap(c, producerYaml) _ = s.manager(c) @@ -358,7 +361,7 @@ func (s *interfaceManagerSuite) TestConnectTaskNoSuchSlot(c *C) { } func (s *interfaceManagerSuite) TestConnectTaskNoSuchPlug(c *C) { - s.mockIface(c, &ifacetest.TestInterface{InterfaceName: "test"}) + s.mockIfaces(c, &ifacetest.TestInterface{InterfaceName: "test"}, &ifacetest.TestInterface{InterfaceName: "test2"}) s.mockSnap(c, consumerYaml) s.mockSnap(c, producerYaml) _ = s.manager(c) @@ -435,7 +438,7 @@ slots: - $SLOT_PUBLISHER_ID `)) defer restore() - s.mockIface(c, &ifacetest.TestInterface{InterfaceName: "test"}) + s.mockIfaces(c, &ifacetest.TestInterface{InterfaceName: "test"}, &ifacetest.TestInterface{InterfaceName: "test2"}) setup() _ = s.manager(c) @@ -491,7 +494,7 @@ func (s *interfaceManagerSuite) TestDisconnectFull(c *C) { func (s *interfaceManagerSuite) testDisconnect(c *C, plugSnap, plugName, slotSnap, slotName string) { // Put two snaps in place They consumer has an plug that can be connected // to slot on the producer. - s.mockIface(c, &ifacetest.TestInterface{InterfaceName: "test"}) + s.mockIfaces(c, &ifacetest.TestInterface{InterfaceName: "test"}, &ifacetest.TestInterface{InterfaceName: "test2"}) s.mockSnap(c, consumerYaml) s.mockSnap(c, producerYaml) @@ -560,6 +563,10 @@ func (s *interfaceManagerSuite) mockIface(c *C, iface interfaces.Interface) { s.extraIfaces = append(s.extraIfaces, iface) } +func (s *interfaceManagerSuite) mockIfaces(c *C, ifaces ...interfaces.Interface) { + s.extraIfaces = append(s.extraIfaces, ifaces...) +} + func (s *interfaceManagerSuite) mockSnapDecl(c *C, name, publisher string, extraHeaders map[string]interface{}) { _, err := s.db.Find(asserts.AccountType, map[string]string{ "account-id": publisher, @@ -843,7 +850,7 @@ func (s *interfaceManagerSuite) TestDoSetupSnapSecurityAutoConnectsPlugs(c *C) { // The setup-profiles task will auto-connect slots with viable candidates. func (s *interfaceManagerSuite) TestDoSetupSnapSecurityAutoConnectsSlots(c *C) { // Mock the interface that will be used by the test - s.mockIface(c, &ifacetest.TestInterface{InterfaceName: "test"}) + s.mockIfaces(c, &ifacetest.TestInterface{InterfaceName: "test"}, &ifacetest.TestInterface{InterfaceName: "test2"}) // Add an OS snap. s.mockSnap(c, ubuntuCoreSnapYaml) // Add a consumer snap with unconnect plug (interface "test") @@ -892,7 +899,7 @@ func (s *interfaceManagerSuite) TestDoSetupSnapSecurityAutoConnectsSlots(c *C) { // The setup-profiles task will auto-connect slots with viable multiple candidates. func (s *interfaceManagerSuite) TestDoSetupSnapSecurityAutoConnectsSlotsMultiplePlugs(c *C) { // Mock the interface that will be used by the test - s.mockIface(c, &ifacetest.TestInterface{InterfaceName: "test"}) + s.mockIfaces(c, &ifacetest.TestInterface{InterfaceName: "test"}, &ifacetest.TestInterface{InterfaceName: "test2"}) // Add an OS snap. s.mockSnap(c, ubuntuCoreSnapYaml) // Add a consumer snap with unconnect plug (interface "test") @@ -1019,7 +1026,7 @@ slots: `)) defer restore() // Add the producer snap - s.mockIface(c, &ifacetest.TestInterface{InterfaceName: "test"}) + s.mockIfaces(c, &ifacetest.TestInterface{InterfaceName: "test"}, &ifacetest.TestInterface{InterfaceName: "test2"}) s.mockSnapDecl(c, "producer", "one-publisher", nil) s.mockSnap(c, producerYaml) @@ -1150,20 +1157,20 @@ func (s *interfaceManagerSuite) TestDoSetupProfilesAddsImplicitSlots(c *C) { } func (s *interfaceManagerSuite) TestDoSetupSnapSecuirtyReloadsConnectionsWhenInvokedOnPlugSide(c *C) { + s.mockIfaces(c, &ifacetest.TestInterface{InterfaceName: "test"}, &ifacetest.TestInterface{InterfaceName: "test2"}) snapInfo := s.mockSnap(c, consumerYaml) s.mockSnap(c, producerYaml) s.testDoSetupSnapSecuirtyReloadsConnectionsWhenInvokedOn(c, snapInfo.Name(), snapInfo.Revision) } func (s *interfaceManagerSuite) TestDoSetupSnapSecuirtyReloadsConnectionsWhenInvokedOnSlotSide(c *C) { + s.mockIfaces(c, &ifacetest.TestInterface{InterfaceName: "test"}, &ifacetest.TestInterface{InterfaceName: "test2"}) s.mockSnap(c, consumerYaml) snapInfo := s.mockSnap(c, producerYaml) s.testDoSetupSnapSecuirtyReloadsConnectionsWhenInvokedOn(c, snapInfo.Name(), snapInfo.Revision) } func (s *interfaceManagerSuite) testDoSetupSnapSecuirtyReloadsConnectionsWhenInvokedOn(c *C, snapName string, revision snap.Revision) { - s.mockIface(c, &ifacetest.TestInterface{InterfaceName: "test"}) - s.state.Lock() s.state.Set("conns", map[string]interface{}{ "consumer:plug producer:slot": map[string]interface{}{"interface": "test"}, @@ -1437,7 +1444,7 @@ func (s *interfaceManagerSuite) testUndoDicardConns(c *C, snapName string) { } func (s *interfaceManagerSuite) TestDoRemove(c *C) { - s.mockIface(c, &ifacetest.TestInterface{InterfaceName: "test"}) + s.mockIfaces(c, &ifacetest.TestInterface{InterfaceName: "test"}, &ifacetest.TestInterface{InterfaceName: "test2"}) s.mockSnap(c, consumerYaml) s.mockSnap(c, producerYaml) @@ -1482,7 +1489,7 @@ func (s *interfaceManagerSuite) TestDoRemove(c *C) { } func (s *interfaceManagerSuite) TestConnectTracksConnectionsInState(c *C) { - s.mockIface(c, &ifacetest.TestInterface{InterfaceName: "test"}) + s.mockIfaces(c, &ifacetest.TestInterface{InterfaceName: "test"}, &ifacetest.TestInterface{InterfaceName: "test2"}) s.mockSnap(c, consumerYaml) s.mockSnap(c, producerYaml) @@ -1521,10 +1528,10 @@ func (s *interfaceManagerSuite) TestConnectTracksConnectionsInState(c *C) { } func (s *interfaceManagerSuite) TestConnectSetsUpSecurity(c *C) { - s.mockIface(c, &ifacetest.TestInterface{InterfaceName: "test"}) + s.mockIfaces(c, &ifacetest.TestInterface{InterfaceName: "test"}, &ifacetest.TestInterface{InterfaceName: "test2"}) + s.mockSnap(c, consumerYaml) s.mockSnap(c, producerYaml) - _ = s.manager(c) s.state.Lock() @@ -1558,7 +1565,7 @@ func (s *interfaceManagerSuite) TestConnectSetsUpSecurity(c *C) { } func (s *interfaceManagerSuite) TestDisconnectSetsUpSecurity(c *C) { - s.mockIface(c, &ifacetest.TestInterface{InterfaceName: "test"}) + s.mockIfaces(c, &ifacetest.TestInterface{InterfaceName: "test"}, &ifacetest.TestInterface{InterfaceName: "test2"}) s.mockSnap(c, consumerYaml) s.mockSnap(c, producerYaml) @@ -1603,7 +1610,7 @@ func (s *interfaceManagerSuite) TestDisconnectSetsUpSecurity(c *C) { } func (s *interfaceManagerSuite) TestDisconnectTracksConnectionsInState(c *C) { - s.mockIface(c, &ifacetest.TestInterface{InterfaceName: "test"}) + s.mockIfaces(c, &ifacetest.TestInterface{InterfaceName: "test"}, &ifacetest.TestInterface{InterfaceName: "test2"}) s.mockSnap(c, consumerYaml) s.mockSnap(c, producerYaml) s.state.Lock() @@ -1643,7 +1650,7 @@ func (s *interfaceManagerSuite) TestDisconnectTracksConnectionsInState(c *C) { } func (s *interfaceManagerSuite) TestManagerReloadsConnections(c *C) { - s.mockIface(c, &ifacetest.TestInterface{InterfaceName: "test"}) + s.mockIfaces(c, &ifacetest.TestInterface{InterfaceName: "test"}, &ifacetest.TestInterface{InterfaceName: "test2"}) s.mockSnap(c, consumerYaml) s.mockSnap(c, producerYaml) @@ -1675,6 +1682,11 @@ func (s *interfaceManagerSuite) TestSetupProfilesDevModeMultiple(c *C) { InterfaceName: "test", }) c.Assert(err, IsNil) + err = repo.AddInterface(&ifacetest.TestInterface{ + InterfaceName: "test2", + }) + c.Assert(err, IsNil) + err = repo.AddSlot(&interfaces.Slot{ SlotInfo: &snap.SlotInfo{ Snap: siC, diff --git a/overlord/snapstate/booted_test.go b/overlord/snapstate/booted_test.go index 407883a62d..9f916d71f6 100644 --- a/overlord/snapstate/booted_test.go +++ b/overlord/snapstate/booted_test.go @@ -37,24 +37,31 @@ import ( "github.com/snapcore/snapd/release" "github.com/snapcore/snapd/snap" "github.com/snapcore/snapd/snap/snaptest" + "github.com/snapcore/snapd/testutil" ) type bootedSuite struct { + testutil.BaseTest bootloader *boottest.MockBootloader o *overlord.Overlord state *state.State snapmgr *snapstate.SnapManager fakeBackend *fakeSnappyBackend + restore func() } var _ = Suite(&bootedSuite{}) func (bs *bootedSuite) SetUpTest(c *C) { + bs.BaseTest.SetUpTest(c) + dirs.SetRootDir(c.MkDir()) err := os.MkdirAll(filepath.Dir(dirs.SnapStateFile), 0755) c.Assert(err, IsNil) + bs.BaseTest.AddCleanup(snap.MockSanitizePlugsSlots(func(snapInfo *snap.Info) {})) + // booted is not running on classic release.MockOnClassic(false) @@ -79,6 +86,7 @@ func (bs *bootedSuite) SetUpTest(c *C) { } func (bs *bootedSuite) TearDownTest(c *C) { + bs.BaseTest.TearDownTest(c) snapstate.AutoAliases = nil release.MockOnClassic(true) dirs.SetRootDir("") diff --git a/overlord/snapstate/check_snap_test.go b/overlord/snapstate/check_snap_test.go index c5e655c889..d4db15409e 100644 --- a/overlord/snapstate/check_snap_test.go +++ b/overlord/snapstate/check_snap_test.go @@ -32,22 +32,27 @@ import ( "github.com/snapcore/snapd/release" "github.com/snapcore/snapd/snap" "github.com/snapcore/snapd/snap/snaptest" + "github.com/snapcore/snapd/testutil" "github.com/snapcore/snapd/overlord/snapstate" ) type checkSnapSuite struct { + testutil.BaseTest st *state.State } var _ = Suite(&checkSnapSuite{}) func (s *checkSnapSuite) SetUpTest(c *C) { + s.BaseTest.SetUpTest(c) dirs.SetRootDir(c.MkDir()) s.st = state.New(nil) + s.BaseTest.AddCleanup(snap.MockSanitizePlugsSlots(func(snapInfo *snap.Info) {})) } func (s *checkSnapSuite) TearDownTest(c *C) { + s.BaseTest.TearDownTest(c) dirs.SetRootDir("") } diff --git a/overlord/snapstate/handlers.go b/overlord/snapstate/handlers.go index b71ecfdcaf..49ad878811 100644 --- a/overlord/snapstate/handlers.go +++ b/overlord/snapstate/handlers.go @@ -397,6 +397,7 @@ func (m *SnapManager) doMountSnap(t *state.Task, _ *tomb.Tomb) error { if err != nil { return err } + t.State().Lock() t.Set("snap-type", newInfo.Type) t.State().Unlock() diff --git a/overlord/snapstate/snapstate_test.go b/overlord/snapstate/snapstate_test.go index dfe6a4aec8..a980b6fe70 100644 --- a/overlord/snapstate/snapstate_test.go +++ b/overlord/snapstate/snapstate_test.go @@ -58,6 +58,7 @@ import ( func TestSnapManager(t *testing.T) { TestingT(t) } type snapmgrTestSuite struct { + testutil.BaseTest o *overlord.Overlord state *state.State snapmgr *snapstate.SnapManager @@ -66,8 +67,6 @@ type snapmgrTestSuite struct { fakeStore *fakeStore user *auth.UserState - - reset func() } func (s *snapmgrTestSuite) settle(c *C) { @@ -78,11 +77,14 @@ func (s *snapmgrTestSuite) settle(c *C) { var _ = Suite(&snapmgrTestSuite{}) func (s *snapmgrTestSuite) SetUpTest(c *C) { + s.BaseTest.SetUpTest(c) dirs.SetRootDir(c.MkDir()) s.o = overlord.Mock() s.state = s.o.State() + s.BaseTest.AddCleanup(snap.MockSanitizePlugsSlots(func(snapInfo *snap.Info) {})) + s.fakeBackend = &fakeSnappyBackend{} s.fakeStore = &fakeStore{ fakeCurrentProgress: 75, @@ -107,18 +109,16 @@ func (s *snapmgrTestSuite) SetUpTest(c *C) { s.o.AddManager(s.snapmgr) - restore1 := snapstate.MockReadInfo(s.fakeBackend.ReadInfo) - restore2 := snapstate.MockOpenSnapFile(s.fakeBackend.OpenSnapFile) + s.BaseTest.AddCleanup(snapstate.MockReadInfo(s.fakeBackend.ReadInfo)) + s.BaseTest.AddCleanup(snapstate.MockOpenSnapFile(s.fakeBackend.OpenSnapFile)) - s.reset = func() { + s.BaseTest.AddCleanup(func() { snapstate.SetupInstallHook = oldSetupInstallHook snapstate.SetupPostRefreshHook = oldSetupPostRefreshHook snapstate.SetupRemoveHook = oldSetupRemoveHook - restore2() - restore1() dirs.SetRootDir("/") - } + }) s.state.Lock() storestate.ReplaceStore(s.state, s.fakeStore) @@ -141,10 +141,10 @@ func (s *snapmgrTestSuite) SetUpTest(c *C) { } func (s *snapmgrTestSuite) TearDownTest(c *C) { + s.BaseTest.TearDownTest(c) snapstate.ValidateRefreshes = nil snapstate.AutoAliases = nil snapstate.CanAutoRefresh = nil - s.reset() } const ( @@ -4987,7 +4987,8 @@ func (s *snapmgrTestSuite) TestDefaultRefreshScheduleParsing(c *C) { } type snapmgrQuerySuite struct { - st *state.State + st *state.State + restore func() } var _ = Suite(&snapmgrQuerySuite{}) @@ -4997,6 +4998,11 @@ func (s *snapmgrQuerySuite) SetUpTest(c *C) { st.Lock() defer st.Unlock() + restoreSanitize := snap.MockSanitizePlugsSlots(func(snapInfo *snap.Info) {}) + s.restore = func() { + restoreSanitize() + } + s.st = st dirs.SetRootDir(c.MkDir()) @@ -5031,6 +5037,7 @@ description: | func (s *snapmgrQuerySuite) TearDownTest(c *C) { dirs.SetRootDir("") + s.restore() } func (s *snapmgrQuerySuite) TestInfo(c *C) { diff --git a/snap/info.go b/snap/info.go index 92a315bbeb..9deaa70f2e 100644 --- a/snap/info.go +++ b/snap/info.go @@ -20,6 +20,7 @@ package snap import ( + "bytes" "fmt" "io/ioutil" "os" @@ -162,6 +163,9 @@ type Info struct { Plugs map[string]*PlugInfo Slots map[string]*SlotInfo + // Plugs or slots with issues (they are not included in Plugs or Slots) + BadInterfaces map[string]string // slot or plug => message + // The information in all the remaining fields is not sourced from the snap blob itself. SideInfo @@ -323,6 +327,32 @@ func (s *Info) Services() []*AppInfo { return svcs } +func BadInterfacesSummary(snapInfo *Info) string { + inverted := make(map[string][]string) + for name, reason := range snapInfo.BadInterfaces { + inverted[reason] = append(inverted[reason], name) + } + var buf bytes.Buffer + fmt.Fprintf(&buf, "snap %q has bad plugs or slots: ", snapInfo.Name()) + reasons := make([]string, 0, len(inverted)) + for reason := range inverted { + reasons = append(reasons, reason) + } + sort.Strings(reasons) + for _, reason := range reasons { + names := inverted[reason] + sort.Strings(names) + for i, name := range names { + if i > 0 { + buf.WriteString(", ") + } + buf.WriteString(name) + } + fmt.Fprintf(&buf, " (%s); ", reason) + } + return strings.TrimSuffix(buf.String(), "; ") +} + // DownloadInfo contains the information to download a snap. // It can be marshalled. type DownloadInfo struct { @@ -568,6 +598,16 @@ func (e NotFoundError) Error() string { return fmt.Sprintf("cannot find installed snap %q at revision %s", e.Snap, e.Revision) } +func MockSanitizePlugsSlots(f func(snapInfo *Info)) (restore func()) { + old := SanitizePlugsSlots + SanitizePlugsSlots = f + return func() { SanitizePlugsSlots = old } +} + +var SanitizePlugsSlots = func(snapInfo *Info) { + panic("SanitizePlugsSlots function not set") +} + // ReadInfo reads the snap information for the installed snap with the given name and given side-info. func ReadInfo(name string, si *SideInfo) (*Info, error) { snapYamlFn := filepath.Join(MountDir(name, si.Revision), "meta", "snap.yaml") @@ -595,6 +635,8 @@ func ReadInfo(name string, si *SideInfo) (*Info, error) { return nil, err } + SanitizePlugsSlots(info) + return info, nil } diff --git a/snap/info_snap_yaml.go b/snap/info_snap_yaml.go index 0fd4731f71..770392b19c 100644 --- a/snap/info_snap_yaml.go +++ b/snap/info_snap_yaml.go @@ -164,6 +164,8 @@ func InfoFromSnapYaml(yamlData []byte) (*Info, error) { // Rename specific plugs on the core snap. snap.renameClashingCorePlugs() + snap.BadInterfaces = make(map[string]string) + // FIXME: validation of the fields return snap, nil } diff --git a/snap/info_test.go b/snap/info_test.go index 7c52382b53..9b1eeca511 100644 --- a/snap/info_test.go +++ b/snap/info_test.go @@ -33,23 +33,43 @@ import ( "github.com/snapcore/snapd/snap" "github.com/snapcore/snapd/snap/snaptest" "github.com/snapcore/snapd/snap/squashfs" + "github.com/snapcore/snapd/testutil" ) type infoSuite struct { - restore func() + testutil.BaseTest } +type infoSimpleSuite struct{} + var _ = Suite(&infoSuite{}) +var _ = Suite(&infoSimpleSuite{}) + +func (s *infoSimpleSuite) SetUpTest(c *C) { + dirs.SetRootDir(c.MkDir()) +} + +func (s *infoSimpleSuite) TearDownTest(c *C) { + dirs.SetRootDir("") +} + +func (s *infoSimpleSuite) TestReadInfoPanicsIfSanitizeUnset(c *C) { + si := &snap.SideInfo{Revision: snap.R(1)} + snaptest.MockSnap(c, sampleYaml, sampleContents, si) + c.Assert(func() { snap.ReadInfo("sample", si) }, Panics, `SanitizePlugsSlots function not set`) +} func (s *infoSuite) SetUpTest(c *C) { + s.BaseTest.SetUpTest(c) dirs.SetRootDir(c.MkDir()) hookType := snap.NewHookType(regexp.MustCompile(".*")) - s.restore = snap.MockSupportedHookTypes([]*snap.HookType{hookType}) + s.BaseTest.AddCleanup(snap.MockSanitizePlugsSlots(func(snapInfo *snap.Info) {})) + s.BaseTest.AddCleanup(snap.MockSupportedHookTypes([]*snap.HookType{hookType})) } func (s *infoSuite) TearDownTest(c *C) { + s.BaseTest.TearDownTest(c) dirs.SetRootDir("") - s.restore() } func (s *infoSuite) TestSideInfoOverrides(c *C) { @@ -531,7 +551,7 @@ version: 1.0` func (s *infoSuite) TestReadInfoInvalidImplicitHook(c *C) { hookType := snap.NewHookType(regexp.MustCompile("foo")) - s.restore = snap.MockSupportedHookTypes([]*snap.HookType{hookType}) + s.BaseTest.AddCleanup(snap.MockSupportedHookTypes([]*snap.HookType{hookType})) yaml := `name: foo version: 1.0`