o/snapstate: prevent components in the seed from being removed

Author: natibek

overlord/devicestate/devicestate.go

@@ -298,6 +298,7 @@ func delayedCrossMgrInit() {
 	snapstate.SeedRefreshTasks = SeedRefreshTasks
 	snapstate.UpdateSeedRefreshChange = UpdateSeedRefreshChange
 	snapstate.CheckSeedRefreshRemove = CheckSeedRefreshRemove
+	snapstate.CheckComponentSeedRefreshRemove = CheckComponentSeedRefreshRemove
 }
 
 // proxyStore returns the store assertion for the proxy store if one is set.
@@ -1881,6 +1882,24 @@ func CheckSeedRefreshRemove(st *state.State, si *snap.Info, dctx snapstate.Devic
 	return nil
 }
 
+// CheckComponentSeedRefreshRemove is set by devicestate to prevent removal of
+// components that must remain present for seed-refresh.
+var CheckComponentSeedRefreshRemove = func(st *state.State, si *snap.Info, componentName string, dctx snapstate.DeviceContext) error {
+	filter := seedRefreshFilter(st, dctx)
+	_, seedRefreshTriggered, err := filter(snapstate.SeedRefreshCandidate{
+		InstanceName:          si.SnapName(),
+		ComponentSetupTaskIDs: map[string]string{componentName: ""},
+	})
+	if err != nil {
+		return err
+	}
+
+	if seedRefreshTriggered {
+		return errors.New("cannot remove component present in the current seed while seed-refresh is enabled")
+	}
+	return nil
+}
+
 // seedRefreshFilter returns a closure that filters the given
 // SeedRefreshCandidate so that it only contains the tasks
 // of the snaps and components that can trigger a seed refresh

overlord/snapstate/component.go

@@ -796,27 +796,49 @@ func RemoveComponents(st *state.State, snapName string, compName []string, opts
 	return tss, nil
 }
 
-func removeComponentTasks(st *state.State, snapst *SnapState, compst *sequence.ComponentState, info *snap.Info, setupSecurity *state.Task, copts ConflictOptions) (*state.TaskSet, error) {
+func canRemoveComponent(st *state.State, compst *sequence.ComponentState, info *snap.Info, copts ConflictOptions) error {
+	deviceCtx, err := DeviceCtxFromState(st, nil)
+	if err != nil {
+		return err
+	}
 	instName := info.InstanceName()
-
 	// For the moment we consider the same conflicts as if the component
 	// was actually the snap.
 	if err := checkChangeConflictIgnoringOneChange(st, instName, nil, copts); err != nil {
-		return nil, err
+		return err
 	}
 
 	// check if this component is required by any validation set in enforcing mode
 	enforcedSets, err := EnforcedValidationSets(st)
 	if err != nil {
-		return nil, err
+		return err
 	}
 	pres, err := enforcedSets.Presence(info)
 	if err != nil {
-		return nil, err
+		return err
 	}
 	compPres := pres.Component(compst.SideInfo.Component.ComponentName)
 	if compPres.Presence == asserts.PresenceRequired {
-		return nil, fmt.Errorf("cannot remove component %q as it is required by an enforcing validation set", compst.SideInfo.Component)
+		return fmt.Errorf("cannot remove component %q as it is required by an enforcing validation set", compst.SideInfo.Component)
+	}
+
+	seedRefresh, err := seedRefreshEnabled(st)
+	if err != nil {
+		return err
+	}
+	if seedRefresh {
+		if err := CheckComponentSeedRefreshRemove(st, info, compst.SideInfo.Component.ComponentName, deviceCtx); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+func removeComponentTasks(st *state.State, snapst *SnapState, compst *sequence.ComponentState, info *snap.Info, setupSecurity *state.Task, copts ConflictOptions) (*state.TaskSet, error) {
+	instName := info.InstanceName()
+	if err := canRemoveComponent(st, compst, info, copts); err != nil {
+		return nil, err
 	}
 
 	snapSup := &SnapSetup{

overlord/snapstate/component_remove_test.go

@@ -21,7 +21,9 @@ package snapstate_test
 
 import (
 	"errors"
+	"fmt"
 
+	"github.com/snapcore/snapd/overlord/configstate/config"
 	"github.com/snapcore/snapd/overlord/snapstate"
 	"github.com/snapcore/snapd/overlord/snapstate/sequence"
 	"github.com/snapcore/snapd/overlord/snapstate/snapstatetest"
@@ -460,3 +462,35 @@ func (s *snapmgrTestSuite) TestRemoveComponentUpdateNoConflict(c *C) {
 	c.Assert(len(tss), Equals, 1)
 	verifyComponentRemoveTasks(c, compCurrentIsDiscarded, tss[0])
 }
+
+func (s *snapmgrTestSuite) TestRemoveComponentInSeedRefresh(c *C) {
+	const snapName = "some-snap"
+	const compName = "mycomp"
+	snapRev := snap.R(1)
+	info := createTestSnapInfoForComponent(c, snapName, snapRev, compName)
+	ci, _ := createTestComponent(c, snapName, compName, info)
+	s.AddCleanup(snapstate.MockReadComponentInfo(func(compMntDir string,
+		snapInfo *snap.Info, csi *snap.ComponentSideInfo) (*snap.ComponentInfo, error) {
+		return ci, nil
+	}))
+
+	s.state.Lock()
+	defer s.state.Unlock()
+	tr := config.NewTransaction(s.state)
+	c.Assert(tr.Set("core", "experimental.seed-refresh", true), IsNil)
+	tr.Commit()
+
+	s.AddCleanup(snapstate.MockCheckComponentSeedRefreshRemove(func(st *state.State,
+		si *snap.Info, componentName string, dctx snapstate.DeviceContext) error {
+		return fmt.Errorf("blocked by seed refresh")
+	}))
+
+	csi1 := snap.NewComponentSideInfo(naming.NewComponentRef(snapName, compName), snap.R(1))
+	cs1 := sequence.NewComponentState(csi1, snap.StandardComponent)
+	setStateWithComponents(s.state, snapName, snapRev, []*sequence.ComponentState{cs1})
+
+	_, err := snapstate.RemoveComponents(s.state, snapName, []string{compName},
+		snapstate.RemoveComponentsOpts{})
+
+	c.Assert(err, ErrorMatches, "blocked by seed refresh")
+}

overlord/snapstate/export_test.go

@@ -80,6 +80,14 @@ func MockReadComponentInfo(mock func(compMntDir string, snapInfo *snap.Info, csi
 	return func() { readComponentInfoAt = old }
 }
 
+func MockCheckComponentSeedRefreshRemove(mock func(st *state.State, si *snap.Info, componentName string, dctx DeviceContext) error) (restore func()) {
+	old := CheckComponentSeedRefreshRemove
+	CheckComponentSeedRefreshRemove = mock
+	return func() {
+		CheckComponentSeedRefreshRemove = old
+	}
+}
+
 func MockMountPollInterval(intv time.Duration) (restore func()) {
 	old := mountPollInterval
 	mountPollInterval = intv

overlord/snapstate/seed.go

@@ -80,6 +80,12 @@ var CheckSeedRefreshRemove = func(st *state.State, si *snap.Info, dctx DeviceCon
 	panic("internal error: snapstate.CheckSeedRefreshRemove is unset")
 }
 
+// CheckComponentSeedRefreshRemove is set by devicestate to prevent removal of
+// components that must remain present for seed-refresh.
+var CheckComponentSeedRefreshRemove = func(st *state.State, si *snap.Info, componentName string, dctx DeviceContext) error {
+	panic("internal error: snapstate.CheckComponentSeedRefreshRemove is unset")
+}
+
 func seedRefreshCandidateForTaskSet(ts *state.TaskSet) (SeedRefreshCandidate, error) {
 	t, err := ts.Edge(SnapSetupEdge)
 	if err != nil {