daemon: fetch validation sets before checking refresh candidates (#14907

) * daemon: fetch validation sets before checking refresh candidates `snap refresh --list` didn't report snaps that would be refreshed with a `snap refresh` because it, unlike the latter, didn't refresh validation sets which could make new snap revisions available. This change fetches new validation sets (without tracking them) before checking for refreshable snaps. Signed-off-by: Miguel Pires <[email protected]> * o/assertstate: add test and comment Signed-off-by: Miguel Pires <[email protected]> --------- Signed-off-by: Miguel Pires <[email protected]>
canonical · Jan 9, 2025 · be6b631 · be6b631
1 parent e4d0a0c
commit be6b631
Show file tree

Hide file tree

Showing 6 changed files with 178 additions and 43 deletions.
diff --git a/daemon/api.go b/daemon/api.go
@@ -170,6 +170,7 @@ var (
 
 	assertstateRefreshSnapAssertions         = assertstate.RefreshSnapAssertions
 	assertstateRestoreValidationSetsTracking = assertstate.RestoreValidationSetsTracking
+	assertstateFetchAllValidationSets        = assertstate.FetchAllValidationSets
 
 	confdbstateGetView        = confdbstate.GetView
 	confdbstateGetTransaction = confdbstate.GetTransactionToModify

diff --git a/daemon/api_find.go b/daemon/api_find.go
@@ -201,7 +201,20 @@ func storeUpdates(c *Command, r *http.Request, user *auth.UserState) Response {
 		return InternalError("cannot find route for snaps")
 	}
 
+	var userID int
+	if user != nil {
+		userID = user.ID
+	}
+
 	state := c.d.overlord.State()
+	state.Lock()
+	// fetch new validation sets, since new versions might impact refresh candidates
+	err := assertstateFetchAllValidationSets(state, userID, nil)
+	state.Unlock()
+	if err != nil {
+		return InternalError("cannot fetch validation sets to check for snap updates: %v", err)
+	}
+
 	state.Lock()
 	updates, err := snapstateRefreshCandidates(state, user)
 	state.Unlock()

diff --git a/daemon/api_find_test.go b/daemon/api_find_test.go
@@ -29,9 +29,12 @@ import (
 	"gopkg.in/check.v1"
 
 	"github.com/snapcore/snapd/client"
+	"github.com/snapcore/snapd/daemon"
 	"github.com/snapcore/snapd/httputil"
+	"github.com/snapcore/snapd/overlord/assertstate"
 	"github.com/snapcore/snapd/overlord/auth"
 	"github.com/snapcore/snapd/overlord/snapstate"
+	"github.com/snapcore/snapd/overlord/state"
 	"github.com/snapcore/snapd/snap"
 	"github.com/snapcore/snapd/store"
 )
@@ -80,6 +83,13 @@ func (s *findSuite) TestFind(c *check.C) {
 func (s *findSuite) TestFindRefreshes(c *check.C) {
 	s.daemon(c)
 
+	var fetchedValidationSets bool
+	restore := daemon.MockAssertstateFetchAllValidationSets(func(*state.State, int, *assertstate.RefreshAssertionsOptions) error {
+		fetchedValidationSets = true
+		return nil
+	})
+	defer restore()
+
 	s.rsnaps = []*snap.Info{{
 		SideInfo: snap.SideInfo{
 			RealName: "store",
@@ -104,6 +114,7 @@ func (s *findSuite) TestFindRefreshes(c *check.C) {
 	c.Assert(snaps[0]["name"], check.Equals, "store")
 	c.Check(s.currentSnaps, check.HasLen, 1)
 	c.Check(s.actions, check.HasLen, 1)
+	c.Check(fetchedValidationSets, check.Equals, true)
 }
 
 func (s *findSuite) TestFindRefreshSideloaded(c *check.C) {

diff --git a/daemon/export_test.go b/daemon/export_test.go
@@ -428,3 +428,7 @@ func MockConfdbstateGetView(f func(_ *state.State, _, _, _ string) (*confdb.View
 func MockConfdbstateSetViaView(f func(confdb.DataBag, *confdb.View, map[string]interface{}) error) (restore func()) {
 	return testutil.Mock(&confdbstateSetViaView, f)
 }
+
+func MockAssertstateFetchAllValidationSets(f func(*state.State, int, *assertstate.RefreshAssertionsOptions) error) (restore func()) {
+	return testutil.Mock(&assertstateFetchAllValidationSets, f)
+}
diff --git a/overlord/assertstate/assertstate.go b/overlord/assertstate/assertstate.go
@@ -482,22 +482,17 @@ func RefreshSnapAssertions(s *state.State, userID int, opts *RefreshAssertionsOp
 	return RefreshValidationSetAssertions(s, userID, opts)
 }
 
-// RefreshValidationSetAssertions tries to refresh all validation set
-// assertions.
-func RefreshValidationSetAssertions(s *state.State, userID int, opts *RefreshAssertionsOptions) error {
+// FetchAllValidationSets updates the DB with new validation sets, if any exist.
+func FetchAllValidationSets(st *state.State, userID int, opts *RefreshAssertionsOptions) error {
 	if opts == nil {
 		opts = &RefreshAssertionsOptions{}
 	}
 
-	deviceCtx, err := snapstate.DevicePastSeeding(s, nil)
+	vsets, err := ValidationSets(st)
 	if err != nil {
 		return err
 	}
 
-	vsets, err := ValidationSets(s)
-	if err != nil {
-		return err
-	}
 	if len(vsets) == 0 {
 		return nil
 	}
@@ -512,33 +507,12 @@ func RefreshValidationSetAssertions(s *state.State, userID int, opts *RefreshAss
 		}
 	}
 
-	updateTracking := func(sets map[string]*ValidationSetTracking) error {
-		// update validation set tracking state
-		for _, vs := range sets {
-			if vs.PinnedAt == 0 {
-				headers := map[string]string{
-					"series":     release.Series,
-					"account-id": vs.AccountID,
-					"name":       vs.Name,
-				}
-				db := DB(s)
-				as, err := db.FindSequence(asserts.ValidationSetType, headers, -1, asserts.ValidationSetType.MaxSupportedFormat())
-				if err != nil {
-					return fmt.Errorf("internal error: cannot find assertion %v when refreshing validation-set assertions", headers)
-				}
-				if vs.Current != as.Sequence() {
-					vs.Current = as.Sequence()
-					UpdateValidationSet(s, vs)
-				}
-			}
-		}
-		return nil
-	}
-
-	if err := bulkRefreshValidationSetAsserts(s, monitorModeSets, nil, userID, deviceCtx, opts); err != nil {
+	deviceCtx, err := snapstate.DevicePastSeeding(st, nil)
+	if err != nil {
 		return err
 	}
-	if err := updateTracking(monitorModeSets); err != nil {
+
+	if err := bulkRefreshValidationSetAsserts(st, monitorModeSets, nil, userID, deviceCtx, opts); err != nil {
 		return err
 	}
 
@@ -575,7 +549,7 @@ func RefreshValidationSetAssertions(s *state.State, userID int, opts *RefreshAss
 			return err
 		}
 
-		snaps, ignoreValidation, err := snapstate.InstalledSnaps(s)
+		snaps, ignoreValidation, err := snapstate.InstalledSnaps(st)
 		if err != nil {
 			return err
 		}
@@ -590,7 +564,7 @@ func RefreshValidationSetAssertions(s *state.State, userID int, opts *RefreshAss
 		return err
 	}
 
-	if err := bulkRefreshValidationSetAsserts(s, enforceModeSets, checkConflictsAndPresence, userID, deviceCtx, opts); err != nil {
+	if err := bulkRefreshValidationSetAsserts(st, enforceModeSets, checkConflictsAndPresence, userID, deviceCtx, opts); err != nil {
 		if _, ok := err.(*snapasserts.ValidationSetsConflictError); ok {
 			logger.Noticef("cannot refresh to conflicting validation set assertions: %v", err)
 			return nil
@@ -601,10 +575,57 @@ func RefreshValidationSetAssertions(s *state.State, userID int, opts *RefreshAss
 		}
 		return err
 	}
-	if err := updateTracking(enforceModeSets); err != nil {
+
+	return nil
+}
+
+// RefreshValidationSetAssertions tries to refresh all validation set assertions,
+// updating the tracked validation sets accordingly.
+func RefreshValidationSetAssertions(s *state.State, userID int, opts *RefreshAssertionsOptions) error {
+	vsets, err := ValidationSets(s)
+	if err != nil {
+		return err
+	}
+
+	if len(vsets) == 0 {
+		return nil
+	}
+
+	err = FetchAllValidationSets(s, userID, opts)
+	if err != nil {
 		return err
 	}
 
+	monitorModeSets := make(map[string]*ValidationSetTracking)
+	enforceModeSets := make(map[string]*ValidationSetTracking)
+	for vk, vset := range vsets {
+		if vset.Mode == Monitor {
+			monitorModeSets[vk] = vset
+		} else {
+			enforceModeSets[vk] = vset
+		}
+	}
+
+	// update validation set tracking state
+	for _, vs := range vsets {
+		if vs.PinnedAt == 0 {
+			headers := map[string]string{
+				"series":     release.Series,
+				"account-id": vs.AccountID,
+				"name":       vs.Name,
+			}
+			db := DB(s)
+			as, err := db.FindSequence(asserts.ValidationSetType, headers, -1, asserts.ValidationSetType.MaxSupportedFormat())
+			if err != nil {
+				return fmt.Errorf("internal error: cannot find assertion %v when refreshing validation-set assertions", headers)
+			}
+			if vs.Current != as.Sequence() {
+				vs.Current = as.Sequence()
+				UpdateValidationSet(s, vs)
+			}
+		}
+	}
+
 	return nil
 }
 

diff --git a/overlord/assertstate/assertstate_test.go b/overlord/assertstate/assertstate_test.go
@@ -2768,6 +2768,51 @@ func (s *assertMgrSuite) TestRefreshValidationSetAssertions(c *C) {
 	c.Check(tr.Current, Equals, 4)
 }
 
+func (s *assertMgrSuite) TestFetchAllValidationSets(c *C) {
+	s.state.Lock()
+	defer s.state.Unlock()
+
+	// have a model and the store assertion available
+	storeAs := s.setupModelAndStore(c)
+	c.Assert(s.storeSigning.Add(storeAs), IsNil)
+
+	// store key already present
+	c.Assert(assertstate.Add(s.state, s.storeSigning.StoreAccountKey("")), IsNil)
+	c.Assert(assertstate.Add(s.state, s.dev1Acct), IsNil)
+	c.Assert(assertstate.Add(s.state, s.dev1AcctKey), IsNil)
+
+	vsetAs1 := s.validationSetAssert(c, "bar", "1", "1", "required", "1")
+	c.Assert(assertstate.Add(s.state, vsetAs1), IsNil)
+
+	vsetAs2 := s.validationSetAssert(c, "bar", "2", "1", "required", "1")
+	c.Assert(s.storeSigning.Add(vsetAs2), IsNil)
+
+	tr := assertstate.ValidationSetTracking{
+		AccountID: s.dev1Acct.AccountID(),
+		Name:      "bar",
+		Mode:      assertstate.Monitor,
+		Current:   1,
+	}
+	assertstate.UpdateValidationSet(s.state, &tr)
+
+	err := assertstate.FetchAllValidationSets(s.state, 0, nil)
+	c.Assert(err, IsNil)
+
+	// DB was updated with new validation set
+	a, err := assertstate.DB(s.state).Find(asserts.ValidationSetType, map[string]string{
+		"series":     "16",
+		"account-id": s.dev1Acct.AccountID(),
+		"name":       "bar",
+		"sequence":   "2",
+	})
+	c.Assert(err, IsNil)
+	c.Check(a.Revision(), Equals, 1)
+
+	// but the tracked validation set is still the old one
+	c.Assert(assertstate.GetValidationSet(s.state, s.dev1Acct.AccountID(), "bar", &tr), IsNil)
+	c.Check(tr.Current, Equals, 1)
+}
+
 func (s *assertMgrSuite) TestRefreshValidationSetAssertionsPinned(c *C) {
 	s.state.Lock()
 	defer s.state.Unlock()
@@ -3065,13 +3110,7 @@ version: 1`), &snap.SideInfo{Revision: snap.R("1")})
 	c.Check(tr.Current, Equals, 1)
 }
 
-func (s *assertMgrSuite) TestRefreshValidationSetAssertionsEnforcingModeConflict(c *C) {
-	s.state.Lock()
-	defer s.state.Unlock()
-
-	logbuf, restore := logger.MockLogger()
-	defer restore()
-
+func (s *assertMgrSuite) setupRefreshToConflict(c *C) {
 	// have a model and the store assertion available
 	storeAs := s.setupModelAndStore(c)
 	err := s.storeSigning.Add(storeAs)
@@ -3106,6 +3145,51 @@ func (s *assertMgrSuite) TestRefreshValidationSetAssertionsEnforcingModeConflict
 		Current:   1,
 	}
 	assertstate.UpdateValidationSet(s.state, &tr)
+}
+
+func (s *assertMgrSuite) TestFetchAllIgnoresValidationSetIfConflict(c *C) {
+	s.state.Lock()
+	defer s.state.Unlock()
+
+	logbuf, restore := logger.MockLogger()
+	defer restore()
+
+	s.setupRefreshToConflict(c)
+
+	c.Assert(assertstate.FetchAllValidationSets(s.state, 0, nil), IsNil)
+	c.Assert(logbuf.String(), Matches, `.*cannot refresh to conflicting validation set assertions: validation sets are in conflict:\n- cannot constrain snap "foo" as both invalid .* and required at revision 1.*\n`)
+
+	a, err := assertstate.DB(s.state).Find(asserts.ValidationSetType, map[string]string{
+		"series":     "16",
+		"account-id": s.dev1Acct.AccountID(),
+		"name":       "foo",
+		"sequence":   "1",
+	})
+	c.Assert(err, IsNil)
+	c.Check(a.(*asserts.ValidationSet).Name(), Equals, "foo")
+	c.Check(a.Revision(), Equals, 1)
+
+	// new assertion wasn't committed to the database.
+	_, err = assertstate.DB(s.state).Find(asserts.ValidationSetType, map[string]string{
+		"series":     "16",
+		"account-id": s.dev1Acct.AccountID(),
+		"name":       "foo",
+		"sequence":   "2",
+	})
+	c.Assert(errors.Is(err, &asserts.NotFoundError{}), Equals, true)
+	c.Check(s.fakeStore.(*fakeStore).requestedTypes, DeepEquals, [][]string{
+		{"account", "account-key", "validation-set"},
+	})
+}
+
+func (s *assertMgrSuite) TestRefreshValidationSetAssertionsEnforcingModeConflict(c *C) {
+	s.state.Lock()
+	defer s.state.Unlock()
+
+	logbuf, restore := logger.MockLogger()
+	defer restore()
+
+	s.setupRefreshToConflict(c)
 
 	c.Assert(assertstate.RefreshValidationSetAssertions(s.state, 0, nil), IsNil)
 	c.Assert(logbuf.String(), Matches, `.*cannot refresh to conflicting validation set assertions: validation sets are in conflict:\n- cannot constrain snap "foo" as both invalid .* and required at revision 1.*\n`)
@@ -3134,6 +3218,7 @@ func (s *assertMgrSuite) TestRefreshValidationSetAssertionsEnforcingModeConflict
 	})
 
 	// tracking current wasn't updated
+	var tr assertstate.ValidationSetTracking
 	c.Assert(assertstate.GetValidationSet(s.state, s.dev1Acct.AccountID(), "foo", &tr), IsNil)
 	c.Check(tr.Current, Equals, 1)
 }