overlord/ifacestate: setup security of snaps affected by auto-connection

overlord/ifacestate/handlers.go

@@ -102,7 +102,7 @@ func (m *InterfaceManager) setupProfilesForSnap(task *state.Task, _ *tomb.Tomb,
 	// - restore connections based on what is kept in the state
 	//   - if a connection cannot be restored then remove it from the state
 	// - setup the security of all the affected snaps
-	affectedSnaps, err := m.repo.DisconnectSnap(snapName)
+	disconnectAffected, err := m.repo.DisconnectSnap(snapName)
 	if err != nil {
 		return err
 	}
@@ -123,13 +123,27 @@ func (m *InterfaceManager) setupProfilesForSnap(task *state.Task, _ *tomb.Tomb,
 	}
 	// FIXME: here we should not reconnect auto-connect plug/slot
 	// pairs that were explicitly disconnected by the user
-	if err := m.autoConnect(task, snapName, nil); err != nil {
+	autoConnectAffected, err := m.autoConnect(task, snapName, nil)
+	if err != nil {
 		return err
 	}
 	if err := setupSnapSecurity(task, snapInfo, opts, m.repo); err != nil {
 		return err
 	}
-
+	affectedSet := make(map[string]bool)
+	for _, name := range disconnectAffected {
+		affectedSet[name] = true
+	}
+	for _, name := range autoConnectAffected {
+		affectedSet[name] = true
+	}
+	// The principal snap was already handled above.
+	delete(affectedSet, snapInfo.Name())
+	affectedSnaps := make([]string, 0, len(affectedSet))
+	for name := range affectedSet {
+		affectedSnaps = append(affectedSnaps, name)
+	}
+	sort.Strings(affectedSnaps)
 	return m.setupAffectedSnaps(task, snapName, affectedSnaps)
 }
 

overlord/ifacestate/helpers.go

@@ -200,19 +200,20 @@ func (c *autoConnectChecker) check(plug *interfaces.Plug, slot *interfaces.Slot)
 	return ic.CheckAutoConnect() == nil
 }
 
-func (m *InterfaceManager) autoConnect(task *state.Task, snapName string, blacklist map[string]bool) error {
+func (m *InterfaceManager) autoConnect(task *state.Task, snapName string, blacklist map[string]bool) ([]string, error) {
 	var conns map[string]connState
+	var affectedSnapNames []string
 	err := task.State().Get("conns", &conns)
 	if err != nil && err != state.ErrNoState {
-		return err
+		return nil, err
 	}
 	if conns == nil {
 		conns = make(map[string]connState)
 	}
 
 	autochecker, err := newAutoConnectChecker(task.State())
 	if err != nil {
-		return err
+		return nil, err
 	}
 
 	// XXX: quick hack, auto-connect everything
@@ -226,14 +227,20 @@ func (m *InterfaceManager) autoConnect(task *state.Task, snapName string, blackl
 		}
 		slot := candidates[0]
 		connRef := interfaces.ConnRef{PlugRef: plug.Ref(), SlotRef: slot.Ref()}
+		key := connRef.ID()
+		if _, ok := conns[key]; ok {
+			// Suggested connection already exist so don't clobber it.
+			continue
+		}
 		if err := m.repo.Connect(connRef); err != nil {
 			task.Logf("cannot auto connect %s to %s: %s", connRef.PlugRef, connRef.SlotRef, err)
 		}
-		key := connRef.ID()
+		affectedSnapNames = append(affectedSnapNames, connRef.PlugRef.Snap)
+		affectedSnapNames = append(affectedSnapNames, connRef.SlotRef.Snap)
 		conns[key] = connState{Interface: plug.Interface, Auto: true}
 	}
 	task.State().Set("conns", conns)
-	return nil
+	return affectedSnapNames, nil
 }
 
 func getPlugAndSlotRefs(task *state.Task) (interfaces.PlugRef, interfaces.SlotRef, error) {

overlord/ifacestate/ifacemgr_test.go

@@ -1078,6 +1078,46 @@ func (s *interfaceManagerSuite) TestSetupProfilesUsesFreshSnapInfo(c *C) {
 	c.Check(s.secBackend.SetupCalls[1].SnapInfo.Revision, Equals, coreSnapInfo.Revision)
 }
 
+// setup-profiles needs to setup security for connected slots after autoconnection
+func (s *interfaceManagerSuite) TestAutoConnectSetupSecurityForConnectedSlots(c *C) {
+	// Add an OS snap.
+	coreSnapInfo := s.mockSnap(c, osSnapYaml)
+
+	// Initialize the manager. This registers the OS snap.
+	mgr := s.manager(c)
+
+	// Add a sample snap with a "network" plug which should be auto-connected.
+	snapInfo := s.mockSnap(c, sampleSnapYaml)
+
+	// Run the setup-snap-security task and let it finish.
+	change := s.addSetupSnapSecurityChange(c, &snapstate.SnapSetup{
+		SideInfo: &snap.SideInfo{
+			RealName: snapInfo.Name(),
+			Revision: snapInfo.Revision,
+		},
+	})
+	mgr.Ensure()
+	mgr.Wait()
+	mgr.Stop()
+
+	s.state.Lock()
+	defer s.state.Unlock()
+
+	// Ensure that the task succeeded.
+	c.Assert(change.Err(), IsNil)
+	c.Assert(change.Status(), Equals, state.DoneStatus)
+
+	// Ensure that both snaps were setup correctly.
+	c.Assert(s.secBackend.SetupCalls, HasLen, 2)
+	c.Assert(s.secBackend.RemoveCalls, HasLen, 0)
+	// The sample snap was setup, with the correct new revision.
+	c.Check(s.secBackend.SetupCalls[0].SnapInfo.Name(), Equals, snapInfo.Name())
+	c.Check(s.secBackend.SetupCalls[0].SnapInfo.Revision, Equals, snapInfo.Revision)
+	// The OS snap was setup (because its connected to sample snap).
+	c.Check(s.secBackend.SetupCalls[1].SnapInfo.Name(), Equals, coreSnapInfo.Name())
+	c.Check(s.secBackend.SetupCalls[1].SnapInfo.Revision, Equals, coreSnapInfo.Revision)
+}
+
 func (s *interfaceManagerSuite) TestDoDiscardConnsPlug(c *C) {
 	s.testDoDicardConns(c, "consumer")
 }