Merge pull request #3972 from stolowski/early-sanitize

interfaces: sanitize plugs and slots early in ReadInfo

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)

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.

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) {

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) {

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) }

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"\)`)
+}

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
 }
 

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)

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()

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

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) {

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",

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
 	}

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
 	}

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,

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("")

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("")
 }
 

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()

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) {

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
 }