From fd83e44deb5f5f5f3522d1fdafd69cfeba4a24b4 Mon Sep 17 00:00:00 2001 From: Miguel Pires Date: Thu, 16 Jul 2026 12:00:15 +0100 Subject: [PATCH 1/3] o/assertstate: move fakeStore into assertstatetest Signed-off-by: Miguel Pires --- overlord/assertstate/assertstate_test.go | 301 ++++-------------- .../assertstate/assertstatetest/fakestore.go | 235 ++++++++++++++ 2 files changed, 291 insertions(+), 245 deletions(-) create mode 100644 overlord/assertstate/assertstatetest/fakestore.go diff --git a/overlord/assertstate/assertstate_test.go b/overlord/assertstate/assertstate_test.go index 56c5cfcfee7..41b13b73091 100644 --- a/overlord/assertstate/assertstate_test.go +++ b/overlord/assertstate/assertstate_test.go @@ -21,14 +21,12 @@ package assertstate_test import ( "bytes" - "context" "crypto" "errors" "fmt" "os" "path/filepath" "regexp" - "sort" "strconv" "strings" "testing" @@ -47,6 +45,7 @@ import ( "github.com/snapcore/snapd/logger" "github.com/snapcore/snapd/overlord" "github.com/snapcore/snapd/overlord/assertstate" + "github.com/snapcore/snapd/overlord/assertstate/assertstatetest" "github.com/snapcore/snapd/overlord/auth" "github.com/snapcore/snapd/overlord/snapstate" "github.com/snapcore/snapd/overlord/snapstate/snapstatetest" @@ -82,194 +81,6 @@ type assertMgrSuite struct { var _ = Suite(&assertMgrSuite{}) -type fakeStore struct { - storetest.Store - state *state.State - db asserts.RODatabase - maxDeclSupportedFormat int - maxValidationSetSupportedFormat int - - requestedTypes [][]string - opts *store.RefreshOptions - - snapActionErr error - downloadAssertionsErr error - assertionErr error -} - -func (sto *fakeStore) pokeStateLock() { - // the store should be called without the state lock held. Try - // to acquire it. - sto.state.Lock() - sto.state.Unlock() -} - -func (sto *fakeStore) Assertion(assertType *asserts.AssertionType, key []string, _ *auth.UserState) (asserts.Assertion, error) { - if sto.assertionErr != nil { - return nil, sto.assertionErr - } - sto.pokeStateLock() - - restore := asserts.MockMaxSupportedFormat(asserts.SnapDeclarationType, sto.maxDeclSupportedFormat) - defer restore() - - ref := &asserts.Ref{Type: assertType, PrimaryKey: key} - return ref.Resolve(sto.db.Find) -} - -func (sto *fakeStore) SeqFormingAssertion(assertType *asserts.AssertionType, sequenceKey []string, sequence int, user *auth.UserState) (asserts.Assertion, error) { - sto.pokeStateLock() - - restore := asserts.MockMaxSupportedFormat(asserts.SnapDeclarationType, sto.maxDeclSupportedFormat) - defer restore() - - ref := &asserts.AtSequence{ - Type: assertType, - SequenceKey: sequenceKey, - Sequence: sequence, - Pinned: sequence > 0, - } - - if ref.Sequence <= 0 { - hdrs, err := asserts.HeadersFromSequenceKey(ref.Type, ref.SequenceKey) - if err != nil { - return nil, err - } - return sto.db.FindSequence(ref.Type, hdrs, -1, -1) - } - - return ref.Resolve(sto.db.Find) -} - -func (sto *fakeStore) SnapAction(_ context.Context, currentSnaps []*store.CurrentSnap, actions []*store.SnapAction, assertQuery store.AssertionQuery, user *auth.UserState, opts *store.RefreshOptions) ([]store.SnapActionResult, []store.AssertionResult, error) { - sto.pokeStateLock() - - if len(currentSnaps) != 0 || len(actions) != 0 { - panic("only assertion query supported") - } - - toResolve, toResolveSeq, err := assertQuery.ToResolve() - if err != nil { - return nil, nil, err - } - - if sto.snapActionErr != nil { - return nil, nil, sto.snapActionErr - } - - sto.opts = opts - - restore := asserts.MockMaxSupportedFormat(asserts.SnapDeclarationType, sto.maxDeclSupportedFormat) - defer restore() - - restoreSeq := asserts.MockMaxSupportedFormat(asserts.ValidationSetType, sto.maxValidationSetSupportedFormat) - defer restoreSeq() - - reqTypes := make(map[string]bool) - ares := make([]store.AssertionResult, 0, len(toResolve)+len(toResolveSeq)) - for g, ats := range toResolve { - urls := make([]string, 0, len(ats)) - for _, at := range ats { - reqTypes[at.Ref.Type.Name] = true - a, err := at.Ref.Resolve(sto.db.Find) - if err != nil { - assertQuery.AddError(err, &at.Ref) - continue - } - if a.Revision() > at.Revision { - urls = append(urls, fmt.Sprintf("/assertions/%s", at.Unique())) - } - } - ares = append(ares, store.AssertionResult{ - Grouping: asserts.Grouping(g), - StreamURLs: urls, - }) - } - - for g, ats := range toResolveSeq { - urls := make([]string, 0, len(ats)) - for _, at := range ats { - reqTypes[at.Type.Name] = true - var a asserts.Assertion - headers, err := asserts.HeadersFromSequenceKey(at.Type, at.SequenceKey) - if err != nil { - return nil, nil, err - } - if !at.Pinned { - a, err = sto.db.FindSequence(at.Type, headers, -1, asserts.ValidationSetType.MaxSupportedFormat()) - } else { - a, err = at.Resolve(sto.db.Find) - } - if err != nil { - assertQuery.AddSequenceError(err, at) - continue - } - storeVs := a.(*asserts.ValidationSet) - if storeVs.Sequence() > at.Sequence || (storeVs.Sequence() == at.Sequence && storeVs.Revision() >= at.Revision) { - urls = append(urls, fmt.Sprintf("/assertions/%s/%s", a.Type().Name, strings.Join(a.At().PrimaryKey, "/"))) - } - } - ares = append(ares, store.AssertionResult{ - Grouping: asserts.Grouping(g), - StreamURLs: urls, - }) - } - - // behave like the actual SnapAction if there are no results - if len(ares) == 0 { - return nil, ares, &store.SnapActionError{ - NoResults: true, - } - } - - typeNames := make([]string, 0, len(reqTypes)) - for k := range reqTypes { - typeNames = append(typeNames, k) - } - sort.Strings(typeNames) - sto.requestedTypes = append(sto.requestedTypes, typeNames) - - return nil, ares, nil -} - -func (sto *fakeStore) DownloadAssertions(urls []string, b *asserts.Batch, user *auth.UserState) error { - sto.pokeStateLock() - - if sto.downloadAssertionsErr != nil { - return sto.downloadAssertionsErr - } - - resolve := func(ref *asserts.Ref) (asserts.Assertion, error) { - restore := asserts.MockMaxSupportedFormat(asserts.SnapDeclarationType, sto.maxDeclSupportedFormat) - defer restore() - - restoreSeq := asserts.MockMaxSupportedFormat(asserts.ValidationSetType, sto.maxValidationSetSupportedFormat) - defer restoreSeq() - return ref.Resolve(sto.db.Find) - } - - for _, u := range urls { - comps := strings.Split(u, "/") - - if len(comps) < 4 { - return fmt.Errorf("cannot use URL: %s", u) - } - - assertType := asserts.Type(comps[2]) - key := comps[3:] - ref := &asserts.Ref{Type: assertType, PrimaryKey: key} - a, err := resolve(ref) - if err != nil { - return err - } - if err := b.Add(a); err != nil { - return err - } - } - - return nil -} - var ( dev1PrivKey, _ = assertstest.GenerateKey(752) ) @@ -301,12 +112,12 @@ func (s *assertMgrSuite) SetUpTest(c *C) { s.o.AddManager(s.o.TaskRunner()) - s.fakeStore = &fakeStore{ - state: s.state, - db: s.storeSigning, + s.fakeStore = &assertstatetest.FakeStore{ + State: s.state, + DB: s.storeSigning, // leave this comment to keep old gofmt happy - maxDeclSupportedFormat: asserts.SnapDeclarationType.MaxSupportedFormat(), - maxValidationSetSupportedFormat: asserts.ValidationSetType.MaxSupportedFormat(), + MaxDeclSupportedFormat: asserts.SnapDeclarationType.MaxSupportedFormat(), + MaxValidationSetSupportedFormat: asserts.ValidationSetType.MaxSupportedFormat(), } s.trivialDeviceCtx = &snapstatetest.TrivialDeviceContext{ CtxStore: s.fakeStore, @@ -783,7 +594,7 @@ func (s *assertMgrSuite) TestFetchUnsupportedUpdateIgnored(c *C) { return f.Fetch(ref) } - s.fakeStore.(*fakeStore).maxDeclSupportedFormat = 999 + s.fakeStore.(*assertstatetest.FakeStore).MaxDeclSupportedFormat = 999 err = assertstate.DoFetch(s.state, 0, s.trivialDeviceCtx, nil, fetching) // no error and the old one was kept c.Assert(err, IsNil) @@ -824,7 +635,7 @@ func (s *assertMgrSuite) TestFetchUnsupportedError(c *C) { return f.Fetch(ref) } - s.fakeStore.(*fakeStore).maxDeclSupportedFormat = 999 + s.fakeStore.(*assertstatetest.FakeStore).MaxDeclSupportedFormat = 999 err := assertstate.DoFetch(s.state, 0, s.trivialDeviceCtx, nil, fetching) c.Check(err, ErrorMatches, `(?s).*proposed "snap-declaration" assertion has format 999 but 111 is latest supported.*`) } @@ -1372,7 +1183,7 @@ func (s *assertMgrSuite) TestRefreshSnapAssertions(c *C) { c.Assert(err, IsNil) c.Check(a.Revision(), Equals, 2) c.Assert(err, IsNil) - c.Check(s.fakeStore.(*fakeStore).opts.Scheduled, Equals, false) + c.Check(s.fakeStore.(*assertstatetest.FakeStore).Opts.Scheduled, Equals, false) // changed validation set assertion again vsetAs3 := s.validationSetAssert(c, "bar", "4", "5", "required", "1") @@ -1425,7 +1236,7 @@ func (s *assertMgrSuite) TestRefreshSnapDeclarationsNop(c *C) { err := assertstate.RefreshSnapDeclarations(s.state, 0, &assertstate.RefreshAssertionsOptions{IsAutoRefresh: true}) c.Assert(err, IsNil) - c.Check(s.fakeStore.(*fakeStore).opts.Scheduled, Equals, true) + c.Check(s.fakeStore.(*assertstatetest.FakeStore).Opts.Scheduled, Equals, true) } func (s *assertMgrSuite) TestRefreshSnapDeclarationsNoStore(c *C) { @@ -1501,7 +1312,7 @@ func (s *assertMgrSuite) TestRefreshSnapDeclarationsNoStore(c *C) { c.Check(a.(*asserts.Account).DisplayName(), Equals, "Dev 1 edited display-name") // change snap decl to something that has a too new format - s.fakeStore.(*fakeStore).maxDeclSupportedFormat = 999 + s.fakeStore.(*assertstatetest.FakeStore).MaxDeclSupportedFormat = 999 (func() { restore := asserts.MockMaxSupportedFormat(asserts.SnapDeclarationType, 999) defer restore() @@ -1728,7 +1539,7 @@ func (s *assertMgrSuite) TestRefreshSnapDeclarationsDownloadError(c *C) { err = s.storeSigning.Add(snapDeclFoo1) c.Assert(err, IsNil) - s.fakeStore.(*fakeStore).downloadAssertionsErr = errors.New("download error") + s.fakeStore.(*assertstatetest.FakeStore).DownloadAssertionsErr = errors.New("download error") err = assertstate.RefreshSnapDeclarations(s.state, 0, nil) c.Assert(err, ErrorMatches, `cannot refresh snap-declarations for snaps: @@ -1768,7 +1579,7 @@ func (s *assertMgrSuite) TestRefreshSnapDeclarationsPersistentNetworkError(c *C) c.Assert(err, IsNil) pne := new(httputil.PersistentNetworkError) - s.fakeStore.(*fakeStore).snapActionErr = pne + s.fakeStore.(*assertstatetest.FakeStore).SnapActionErr = pne err = assertstate.RefreshSnapDeclarations(s.state, 0, nil) c.Assert(err, Equals, pne) @@ -1777,7 +1588,7 @@ func (s *assertMgrSuite) TestRefreshSnapDeclarationsPersistentNetworkError(c *C) func (s *assertMgrSuite) TestRefreshSnapDeclarationsNoStoreFallback(c *C) { // test that if we get a 4xx or 500 error from the store trying bulk // assertion refresh we fall back to the old logic - s.fakeStore.(*fakeStore).snapActionErr = &store.UnexpectedHTTPStatusError{StatusCode: 400} + s.fakeStore.(*assertstatetest.FakeStore).SnapActionErr = &store.UnexpectedHTTPStatusError{StatusCode: 400} logbuf, restore := logger.MockLogger() defer restore() @@ -1791,7 +1602,7 @@ func (s *assertMgrSuite) TestRefreshSnapDeclarationsNoStoreFallbackUnexpectedSna // test that if we get an unexpected SnapAction error from the // store trying bulk assertion refresh we fall back to the old // logic - s.fakeStore.(*fakeStore).snapActionErr = &store.SnapActionError{ + s.fakeStore.(*assertstatetest.FakeStore).SnapActionErr = &store.SnapActionError{ NoResults: true, Other: []error{errors.New("unexpected error")}, } @@ -1807,7 +1618,7 @@ func (s *assertMgrSuite) TestRefreshSnapDeclarationsNoStoreFallbackUnexpectedSna func (s *assertMgrSuite) TestRefreshSnapDeclarationsWithStoreFallback(c *C) { // test that if we get a 4xx or 500 error from the store trying bulk // assertion refresh we fall back to the old logic - s.fakeStore.(*fakeStore).snapActionErr = &store.UnexpectedHTTPStatusError{StatusCode: 500} + s.fakeStore.(*assertstatetest.FakeStore).SnapActionErr = &store.UnexpectedHTTPStatusError{StatusCode: 500} logbuf, restore := logger.MockLogger() defer restore() @@ -1884,7 +1695,7 @@ func (s *assertMgrSuite) TestRefreshSnapDeclarationsMany14NoStore(c *C) { err := s.testRefreshSnapDeclarationsMany(c, 14) c.Assert(err, IsNil) - c.Check(s.fakeStore.(*fakeStore).requestedTypes, DeepEquals, [][]string{ + c.Check(s.fakeStore.(*assertstatetest.FakeStore).RequestedTypes, DeepEquals, [][]string{ {"account", "account-key", "snap-declaration"}, }) } @@ -1897,7 +1708,7 @@ func (s *assertMgrSuite) TestRefreshSnapDeclarationsMany16NoStore(c *C) { err := s.testRefreshSnapDeclarationsMany(c, 16) c.Assert(err, IsNil) - c.Check(s.fakeStore.(*fakeStore).requestedTypes, DeepEquals, [][]string{ + c.Check(s.fakeStore.(*assertstatetest.FakeStore).RequestedTypes, DeepEquals, [][]string{ {"account", "account-key", "snap-declaration"}, }) } @@ -1913,7 +1724,7 @@ func (s *assertMgrSuite) TestRefreshSnapDeclarationsMany16WithStore(c *C) { err = s.testRefreshSnapDeclarationsMany(c, 16) c.Assert(err, IsNil) - c.Check(s.fakeStore.(*fakeStore).requestedTypes, DeepEquals, [][]string{ + c.Check(s.fakeStore.(*assertstatetest.FakeStore).RequestedTypes, DeepEquals, [][]string{ // first 16 groups request {"account", "account-key", "snap-declaration"}, // final separate request covering store only @@ -1935,7 +1746,7 @@ func (s *assertMgrSuite) TestRefreshSnapDeclarationsMany17NoStore(c *C) { err := s.testRefreshSnapDeclarationsMany(c, 17) c.Assert(err, IsNil) - c.Check(s.fakeStore.(*fakeStore).requestedTypes, DeepEquals, [][]string{ + c.Check(s.fakeStore.(*assertstatetest.FakeStore).RequestedTypes, DeepEquals, [][]string{ // first 16 groups request {"account", "account-key", "snap-declaration"}, // final separate request for the rest @@ -1948,7 +1759,7 @@ func (s *assertMgrSuite) TestRefreshSnapDeclarationsMany17NoStoreMergeErrors(c * defer s.state.Unlock() s.setModel(sysdb.GenericClassicModel()) - s.fakeStore.(*fakeStore).downloadAssertionsErr = errors.New("download error") + s.fakeStore.(*assertstatetest.FakeStore).DownloadAssertionsErr = errors.New("download error") err := s.testRefreshSnapDeclarationsMany(c, 17) c.Check(err, ErrorMatches, `(?s)cannot refresh snap-declarations for snaps: @@ -1956,7 +1767,7 @@ func (s *assertMgrSuite) TestRefreshSnapDeclarationsMany17NoStoreMergeErrors(c * // all foo* snaps accounted for c.Check(strings.Count(err.Error(), "foo"), Equals, 17) - c.Check(s.fakeStore.(*fakeStore).requestedTypes, DeepEquals, [][]string{ + c.Check(s.fakeStore.(*assertstatetest.FakeStore).RequestedTypes, DeepEquals, [][]string{ // first 16 groups request {"account", "account-key", "snap-declaration"}, // final separate request for the rest @@ -1975,7 +1786,7 @@ func (s *assertMgrSuite) TestRefreshSnapDeclarationsMany31WithStore(c *C) { err = s.testRefreshSnapDeclarationsMany(c, 31) c.Assert(err, IsNil) - c.Check(s.fakeStore.(*fakeStore).requestedTypes, DeepEquals, [][]string{ + c.Check(s.fakeStore.(*assertstatetest.FakeStore).RequestedTypes, DeepEquals, [][]string{ // first 16 groups request {"account", "account-key", "snap-declaration"}, // final separate request for the rest and store @@ -2000,7 +1811,7 @@ func (s *assertMgrSuite) TestRefreshSnapDeclarationsMany32WithStore(c *C) { err = s.testRefreshSnapDeclarationsMany(c, 32) c.Assert(err, IsNil) - c.Check(s.fakeStore.(*fakeStore).requestedTypes, DeepEquals, [][]string{ + c.Check(s.fakeStore.(*assertstatetest.FakeStore).RequestedTypes, DeepEquals, [][]string{ // first 16 groups request {"account", "account-key", "snap-declaration"}, // 2nd round request @@ -2687,7 +2498,7 @@ func (s *assertMgrSuite) TestValidationSetAssertionsAutoRefresh(c *C) { assertstate.UpdateValidationSet(s.state, &tr) c.Assert(assertstate.AutoRefreshAssertions(s.state, 0), IsNil) - c.Check(s.fakeStore.(*fakeStore).opts.Scheduled, Equals, true) + c.Check(s.fakeStore.(*assertstatetest.FakeStore).Opts.Scheduled, Equals, true) a, err := assertstate.DB(s.state).Find(asserts.ValidationSetType, map[string]string{ "series": "16", @@ -2719,7 +2530,7 @@ func (s *assertMgrSuite) TestValidationSetAssertionsAutoRefreshError(c *C) { } func (s *assertMgrSuite) TestRefreshValidationSetAssertionsStoreError(c *C) { - s.fakeStore.(*fakeStore).snapActionErr = &store.UnexpectedHTTPStatusError{StatusCode: 400} + s.fakeStore.(*assertstatetest.FakeStore).SnapActionErr = &store.UnexpectedHTTPStatusError{StatusCode: 400} s.state.Lock() defer s.state.Unlock() @@ -2787,10 +2598,10 @@ func (s *assertMgrSuite) TestRefreshValidationSetAssertions(c *C) { c.Check(a.(*asserts.ValidationSet).Name(), Equals, "bar") c.Check(a.Revision(), Equals, 2) - c.Check(s.fakeStore.(*fakeStore).requestedTypes, DeepEquals, [][]string{ + c.Check(s.fakeStore.(*assertstatetest.FakeStore).RequestedTypes, DeepEquals, [][]string{ {"account", "account-key", "validation-set"}, }) - c.Check(s.fakeStore.(*fakeStore).opts.Scheduled, Equals, true) + c.Check(s.fakeStore.(*assertstatetest.FakeStore).Opts.Scheduled, Equals, true) // sequence changed in the store to 4 vsetAs3 := s.validationSetAssert(c, "bar", "4", "3", "required", "1") @@ -2806,11 +2617,11 @@ func (s *assertMgrSuite) TestRefreshValidationSetAssertions(c *C) { }) c.Assert(errors.Is(err, &asserts.NotFoundError{}), Equals, true) - s.fakeStore.(*fakeStore).requestedTypes = nil + s.fakeStore.(*assertstatetest.FakeStore).RequestedTypes = nil err = assertstate.RefreshValidationSetAssertions(s.state, 0, nil) c.Assert(err, IsNil) - c.Check(s.fakeStore.(*fakeStore).requestedTypes, DeepEquals, [][]string{ + c.Check(s.fakeStore.(*assertstatetest.FakeStore).RequestedTypes, DeepEquals, [][]string{ {"account", "account-key", "validation-set"}, }) @@ -2920,7 +2731,7 @@ func (s *assertMgrSuite) TestRefreshValidationSetAssertionsPinned(c *C) { c.Check(a.(*asserts.ValidationSet).Sequence(), Equals, 2) c.Check(a.Revision(), Equals, 5) - c.Check(s.fakeStore.(*fakeStore).requestedTypes, DeepEquals, [][]string{ + c.Check(s.fakeStore.(*assertstatetest.FakeStore).RequestedTypes, DeepEquals, [][]string{ {"account", "account-key", "validation-set"}, }) @@ -2929,11 +2740,11 @@ func (s *assertMgrSuite) TestRefreshValidationSetAssertionsPinned(c *C) { err = s.storeSigning.Add(vsetAs3) c.Assert(err, IsNil) - s.fakeStore.(*fakeStore).requestedTypes = nil + s.fakeStore.(*assertstatetest.FakeStore).RequestedTypes = nil err = assertstate.RefreshValidationSetAssertions(s.state, 0, nil) c.Assert(err, IsNil) - c.Check(s.fakeStore.(*fakeStore).requestedTypes, DeepEquals, [][]string{ + c.Check(s.fakeStore.(*assertstatetest.FakeStore).RequestedTypes, DeepEquals, [][]string{ {"account", "account-key", "validation-set"}, }) @@ -3100,7 +2911,7 @@ version: 1`), &snap.SideInfo{ c.Check(a.(*asserts.ValidationSet).Sequence(), Equals, 2) c.Check(a.Revision(), Equals, 3) - c.Check(s.fakeStore.(*fakeStore).requestedTypes, DeepEquals, [][]string{ + c.Check(s.fakeStore.(*assertstatetest.FakeStore).RequestedTypes, DeepEquals, [][]string{ {"account", "account-key", "validation-set"}, }) @@ -3162,7 +2973,7 @@ version: 1`), &snap.SideInfo{Revision: snap.R("1")}) c.Check(a.(*asserts.ValidationSet).Sequence(), Equals, 1) c.Check(a.Revision(), Equals, 2) - c.Check(s.fakeStore.(*fakeStore).requestedTypes, DeepEquals, [][]string{ + c.Check(s.fakeStore.(*assertstatetest.FakeStore).RequestedTypes, DeepEquals, [][]string{ {"account", "account-key", "validation-set"}, }) @@ -3238,7 +3049,7 @@ func (s *assertMgrSuite) TestFetchAllIgnoresValidationSetIfConflict(c *C) { "sequence": "2", }) c.Assert(errors.Is(err, &asserts.NotFoundError{}), Equals, true) - c.Check(s.fakeStore.(*fakeStore).requestedTypes, DeepEquals, [][]string{ + c.Check(s.fakeStore.(*assertstatetest.FakeStore).RequestedTypes, DeepEquals, [][]string{ {"account", "account-key", "validation-set"}, }) } @@ -3274,7 +3085,7 @@ func (s *assertMgrSuite) TestRefreshValidationSetAssertionsEnforcingModeConflict }) c.Assert(errors.Is(err, &asserts.NotFoundError{}), Equals, true) - c.Check(s.fakeStore.(*fakeStore).requestedTypes, DeepEquals, [][]string{ + c.Check(s.fakeStore.(*assertstatetest.FakeStore).RequestedTypes, DeepEquals, [][]string{ {"account", "account-key", "validation-set"}, }) @@ -3339,7 +3150,7 @@ func (s *assertMgrSuite) TestRefreshValidationSetAssertionsEnforcingModeMissingS }) c.Assert(errors.Is(err, &asserts.NotFoundError{}), Equals, true) - c.Check(s.fakeStore.(*fakeStore).requestedTypes, DeepEquals, [][]string{ + c.Check(s.fakeStore.(*assertstatetest.FakeStore).RequestedTypes, DeepEquals, [][]string{ {"account", "account-key", "validation-set"}, }) @@ -3397,7 +3208,7 @@ func (s *assertMgrSuite) TestRefreshValidationSetAssertionsEnforcingModeWrongSna }) c.Assert(err, IsNil) - c.Check(s.fakeStore.(*fakeStore).requestedTypes, DeepEquals, [][]string{ + c.Check(s.fakeStore.(*assertstatetest.FakeStore).RequestedTypes, DeepEquals, [][]string{ {"account", "account-key", "validation-set"}, }) @@ -3575,7 +3386,7 @@ func (s *assertMgrSuite) TestValidationSetAssertionForEnforcePinnedHappy(c *C) { "sequence": "2", }) c.Assert(err, IsNil) - c.Check(s.fakeStore.(*fakeStore).opts.Scheduled, Equals, false) + c.Check(s.fakeStore.(*assertstatetest.FakeStore).Opts.Scheduled, Equals, false) } func (s *assertMgrSuite) TestValidationSetAssertionForEnforceNotPinnedUnhappyMissingSnap(c *C) { @@ -3869,7 +3680,7 @@ func (s *assertMgrSuite) TestEnforceValidationSetAssertion(c *C) { "sequence": "2", }) c.Assert(err, IsNil) - c.Check(s.fakeStore.(*fakeStore).opts.Scheduled, Equals, false) + c.Check(s.fakeStore.(*assertstatetest.FakeStore).Opts.Scheduled, Equals, false) var tr assertstate.ValidationSetTracking c.Assert(assertstate.GetValidationSet(s.state, s.dev1Acct.AccountID(), "bar", &tr), IsNil) @@ -3930,7 +3741,7 @@ func (s *assertMgrSuite) TestEnforceValidationSetAssertionUpdate(c *C) { "sequence": "2", }) c.Assert(err, IsNil) - c.Check(s.fakeStore.(*fakeStore).opts.Scheduled, Equals, false) + c.Check(s.fakeStore.(*assertstatetest.FakeStore).Opts.Scheduled, Equals, false) var tr assertstate.ValidationSetTracking c.Assert(assertstate.GetValidationSet(s.state, s.dev1Acct.AccountID(), "bar", &tr), IsNil) @@ -4007,7 +3818,7 @@ func (s *assertMgrSuite) TestEnforceValidationSetAssertionPinToOlderSequence(c * "sequence": "2", }) c.Assert(err, IsNil) - c.Check(s.fakeStore.(*fakeStore).opts.Scheduled, Equals, false) + c.Check(s.fakeStore.(*assertstatetest.FakeStore).Opts.Scheduled, Equals, false) var tr assertstate.ValidationSetTracking c.Assert(assertstate.GetValidationSet(s.state, s.dev1Acct.AccountID(), "bar", &tr), IsNil) @@ -4082,7 +3893,7 @@ func (s *assertMgrSuite) TestEnforceValidationSetAssertionAfterMonitor(c *C) { "sequence": "2", }) c.Assert(err, IsNil) - c.Check(s.fakeStore.(*fakeStore).opts.Scheduled, Equals, false) + c.Check(s.fakeStore.(*assertstatetest.FakeStore).Opts.Scheduled, Equals, false) var tr assertstate.ValidationSetTracking c.Assert(assertstate.GetValidationSet(s.state, s.dev1Acct.AccountID(), "bar", &tr), IsNil) @@ -4137,7 +3948,7 @@ func (s *assertMgrSuite) TestEnforceValidationSetAssertionIgnoreValidation(c *C) "sequence": "2", }) c.Assert(err, IsNil) - c.Check(s.fakeStore.(*fakeStore).opts.Scheduled, Equals, false) + c.Check(s.fakeStore.(*assertstatetest.FakeStore).Opts.Scheduled, Equals, false) var tr assertstate.ValidationSetTracking c.Assert(assertstate.GetValidationSet(s.state, s.dev1Acct.AccountID(), "bar", &tr), IsNil) @@ -4240,7 +4051,7 @@ func (s *assertMgrSuite) TestTryEnforceValidationSetsAssertionsValidationError(c "sequence": "1", }) c.Assert(errors.Is(err, &asserts.NotFoundError{}), Equals, true) - c.Check(s.fakeStore.(*fakeStore).opts.Scheduled, Equals, false) + c.Check(s.fakeStore.(*assertstatetest.FakeStore).Opts.Scheduled, Equals, false) } func (s *assertMgrSuite) TestTryEnforceValidationSetsAssertionsValidationErrorCommitsOnlyPrerequisites(c *C) { @@ -4377,7 +4188,7 @@ func (s *assertMgrSuite) TestTryEnforceValidationSetsAssertionsOK(c *C) { "sequence": "1", }) c.Assert(err, IsNil) - c.Check(s.fakeStore.(*fakeStore).opts.Scheduled, Equals, false) + c.Check(s.fakeStore.(*assertstatetest.FakeStore).Opts.Scheduled, Equals, false) // tracking was updated var tr assertstate.ValidationSetTracking @@ -4478,7 +4289,7 @@ func (s *assertMgrSuite) TestTryEnforceValidationSetsAssertionsAlreadyTrackedUpd "sequence": "2", }) c.Assert(err, IsNil) - c.Check(s.fakeStore.(*fakeStore).opts.Scheduled, Equals, false) + c.Check(s.fakeStore.(*assertstatetest.FakeStore).Opts.Scheduled, Equals, false) // tracking was updated var tr assertstate.ValidationSetTracking @@ -4561,7 +4372,7 @@ func (s *assertMgrSuite) TestTryEnforceValidationSetsAssertionsConflictError(c * "sequence": "2", }) c.Assert(errors.Is(err, &asserts.NotFoundError{}), Equals, true) - c.Check(s.fakeStore.(*fakeStore).opts.Scheduled, Equals, false) + c.Check(s.fakeStore.(*assertstatetest.FakeStore).Opts.Scheduled, Equals, false) } func (s *assertMgrSuite) TestMonitorValidationSet(c *C) { @@ -4600,7 +4411,7 @@ func (s *assertMgrSuite) TestMonitorValidationSet(c *C) { "sequence": "2", }) c.Assert(err, IsNil) - c.Check(s.fakeStore.(*fakeStore).opts.Scheduled, Equals, false) + c.Check(s.fakeStore.(*assertstatetest.FakeStore).Opts.Scheduled, Equals, false) var tr assertstate.ValidationSetTracking c.Assert(assertstate.GetValidationSet(s.state, s.dev1Acct.AccountID(), "bar", &tr), IsNil) @@ -4886,7 +4697,7 @@ func (s *assertMgrSuite) TestEnforceValidationSetsWithLocalPrerequisitesDoesNotN snapasserts.NewInstalledSnap("some-snap", "qOqKhntON3vR7kwEbVPsILm7bUViPDzz", snap.Revision{N: 1}, nil), } - s.fakeStore.(*fakeStore).assertionErr = store.ErrStoreOffline + s.fakeStore.(*assertstatetest.FakeStore).AssertionErr = store.ErrStoreOffline err := assertstate.ApplyEnforcedValidationSets(st, valSets, nil, installedSnaps, nil, 0) c.Assert(err, IsNil) @@ -5946,14 +5757,14 @@ func (s *assertMgrSuite) TestFetchConfdbAssertion(c *C) { func (s *assertMgrSuite) TestConfdbAssertionsAutoRefreshBulkFetch(c *C) { s.testConfdbAssertionsAutoRefresh(c) - c.Check(s.fakeStore.(*fakeStore).opts.Scheduled, Equals, true) + c.Check(s.fakeStore.(*assertstatetest.FakeStore).Opts.Scheduled, Equals, true) } func (s *assertMgrSuite) TestConfdbAssertionsAutoRefreshSingleFetch(c *C) { logbuf, restore := logger.MockLogger() defer restore() - s.fakeStore.(*fakeStore).snapActionErr = &store.UnexpectedHTTPStatusError{StatusCode: 500} + s.fakeStore.(*assertstatetest.FakeStore).SnapActionErr = &store.UnexpectedHTTPStatusError{StatusCode: 500} s.testConfdbAssertionsAutoRefresh(c) // get the last line (we call AutoRefresh more than once) @@ -6009,7 +5820,7 @@ func (s *assertMgrSuite) TestBulkRefreshLocalConfdbSchemaNotFound(c *C) { } func (s *assertMgrSuite) TestSingleRefreshLocalConfdbSchemaNotFound(c *C) { - s.fakeStore.(*fakeStore).snapActionErr = &store.UnexpectedHTTPStatusError{StatusCode: 500} + s.fakeStore.(*assertstatetest.FakeStore).SnapActionErr = &store.UnexpectedHTTPStatusError{StatusCode: 500} log := s.testRefreshLocalConfdbSchemaNotFound(c) @@ -6192,8 +6003,8 @@ func (s *assertMgrSuite) TestOfflineErrorSurfaced(c *C) { s.state.Lock() defer s.state.Unlock() - sto := s.fakeStore.(*fakeStore) - sto.assertionErr = store.ErrStoreOffline + sto := s.fakeStore.(*assertstatetest.FakeStore) + sto.AssertionErr = store.ErrStoreOffline s.setupModelAndStore(c) diff --git a/overlord/assertstate/assertstatetest/fakestore.go b/overlord/assertstate/assertstatetest/fakestore.go new file mode 100644 index 00000000000..aa3403129df --- /dev/null +++ b/overlord/assertstate/assertstatetest/fakestore.go @@ -0,0 +1,235 @@ +// -*- Mode: Go; indent-tabs-mode: t -*- + +/* + * Copyright (C) 2026 Canonical Ltd + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 3 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + */ + +package assertstatetest + +import ( + "context" + "fmt" + "sort" + "strings" + + "github.com/snapcore/snapd/asserts" + "github.com/snapcore/snapd/overlord/auth" + "github.com/snapcore/snapd/overlord/state" + "github.com/snapcore/snapd/store" + "github.com/snapcore/snapd/store/storetest" +) + +// FakeStore is a minimal in-memory implementation of snapstate.StoreService +// used by assertstate and related tests to exercise the assertion fetch code +// paths without a real store. Assertions are resolved from DB. +type FakeStore struct { + storetest.Store + + State *state.State + DB asserts.RODatabase + + // MaxDeclSupportedFormat, if non-zero, sets the max supported format for + // snap-declaration assertions during store operations. + MaxDeclSupportedFormat int + // MaxValidationSetSupportedFormat, if non-zero, sets the max supported + // format for validation-set assertions during store operations. + MaxValidationSetSupportedFormat int + + // RequestedTypes captures the assertion types requested via SnapAction. + RequestedTypes [][]string + // Opts captures the last RefreshOptions passed to SnapAction. + Opts *store.RefreshOptions + + // SnapActionErr, if non-nil, is returned from SnapAction. + SnapActionErr error + // DownloadAssertionsErr, if non-nil, is returned from DownloadAssertions. + DownloadAssertionsErr error + // AssertionErr, if non-nil, is returned from Assertion. + AssertionErr error +} + +func (sto *FakeStore) pokeStateLock() { + // the store should be called without the state lock held. Try + // to acquire it. + sto.State.Lock() + sto.State.Unlock() +} + +func (sto *FakeStore) Assertion(assertType *asserts.AssertionType, key []string, _ *auth.UserState) (asserts.Assertion, error) { + if sto.AssertionErr != nil { + return nil, sto.AssertionErr + } + sto.pokeStateLock() + + restore := asserts.MockMaxSupportedFormat(asserts.SnapDeclarationType, sto.MaxDeclSupportedFormat) + defer restore() + + ref := &asserts.Ref{Type: assertType, PrimaryKey: key} + return ref.Resolve(sto.DB.Find) +} + +func (sto *FakeStore) SeqFormingAssertion(assertType *asserts.AssertionType, sequenceKey []string, sequence int, _ *auth.UserState) (asserts.Assertion, error) { + sto.pokeStateLock() + + restore := asserts.MockMaxSupportedFormat(asserts.SnapDeclarationType, sto.MaxDeclSupportedFormat) + defer restore() + + ref := &asserts.AtSequence{ + Type: assertType, + SequenceKey: sequenceKey, + Sequence: sequence, + Pinned: sequence > 0, + } + + if ref.Sequence <= 0 { + hdrs, err := asserts.HeadersFromSequenceKey(ref.Type, ref.SequenceKey) + if err != nil { + return nil, err + } + return sto.DB.FindSequence(ref.Type, hdrs, -1, -1) + } + + return ref.Resolve(sto.DB.Find) +} + +func (sto *FakeStore) SnapAction(_ context.Context, currentSnaps []*store.CurrentSnap, actions []*store.SnapAction, assertQuery store.AssertionQuery, _ *auth.UserState, opts *store.RefreshOptions) ([]store.SnapActionResult, []store.AssertionResult, error) { + sto.pokeStateLock() + + if len(currentSnaps) != 0 || len(actions) != 0 { + panic("only assertion query supported") + } + + toResolve, toResolveSeq, err := assertQuery.ToResolve() + if err != nil { + return nil, nil, err + } + + if sto.SnapActionErr != nil { + return nil, nil, sto.SnapActionErr + } + + sto.Opts = opts + + restore := asserts.MockMaxSupportedFormat(asserts.SnapDeclarationType, sto.MaxDeclSupportedFormat) + defer restore() + + restoreSeq := asserts.MockMaxSupportedFormat(asserts.ValidationSetType, sto.MaxValidationSetSupportedFormat) + defer restoreSeq() + + reqTypes := make(map[string]bool) + ares := make([]store.AssertionResult, 0, len(toResolve)+len(toResolveSeq)) + for g, ats := range toResolve { + urls := make([]string, 0, len(ats)) + for _, at := range ats { + reqTypes[at.Ref.Type.Name] = true + a, err := at.Ref.Resolve(sto.DB.Find) + if err != nil { + assertQuery.AddError(err, &at.Ref) + continue + } + if a.Revision() > at.Revision { + urls = append(urls, fmt.Sprintf("/assertions/%s", at.Unique())) + } + } + ares = append(ares, store.AssertionResult{ + Grouping: asserts.Grouping(g), + StreamURLs: urls, + }) + } + + for g, ats := range toResolveSeq { + urls := make([]string, 0, len(ats)) + for _, at := range ats { + reqTypes[at.Type.Name] = true + var a asserts.Assertion + headers, err := asserts.HeadersFromSequenceKey(at.Type, at.SequenceKey) + if err != nil { + return nil, nil, err + } + if !at.Pinned { + a, err = sto.DB.FindSequence(at.Type, headers, -1, asserts.ValidationSetType.MaxSupportedFormat()) + } else { + a, err = at.Resolve(sto.DB.Find) + } + if err != nil { + assertQuery.AddSequenceError(err, at) + continue + } + storeVs := a.(*asserts.ValidationSet) + if storeVs.Sequence() > at.Sequence || (storeVs.Sequence() == at.Sequence && storeVs.Revision() >= at.Revision) { + urls = append(urls, fmt.Sprintf("/assertions/%s/%s", a.Type().Name, strings.Join(a.At().PrimaryKey, "/"))) + } + } + ares = append(ares, store.AssertionResult{ + Grouping: asserts.Grouping(g), + StreamURLs: urls, + }) + } + + // behave like the actual SnapAction if there are no results + if len(ares) == 0 { + return nil, ares, &store.SnapActionError{ + NoResults: true, + } + } + + typeNames := make([]string, 0, len(reqTypes)) + for k := range reqTypes { + typeNames = append(typeNames, k) + } + sort.Strings(typeNames) + sto.RequestedTypes = append(sto.RequestedTypes, typeNames) + + return nil, ares, nil +} + +func (sto *FakeStore) DownloadAssertions(urls []string, b *asserts.Batch, _ *auth.UserState) error { + sto.pokeStateLock() + + if sto.DownloadAssertionsErr != nil { + return sto.DownloadAssertionsErr + } + + resolve := func(ref *asserts.Ref) (asserts.Assertion, error) { + restore := asserts.MockMaxSupportedFormat(asserts.SnapDeclarationType, sto.MaxDeclSupportedFormat) + defer restore() + + restoreSeq := asserts.MockMaxSupportedFormat(asserts.ValidationSetType, sto.MaxValidationSetSupportedFormat) + defer restoreSeq() + return ref.Resolve(sto.DB.Find) + } + + for _, u := range urls { + comps := strings.Split(u, "/") + + if len(comps) < 4 { + return fmt.Errorf("cannot use URL: %s", u) + } + + assertType := asserts.Type(comps[2]) + key := comps[3:] + ref := &asserts.Ref{Type: assertType, PrimaryKey: key} + a, err := resolve(ref) + if err != nil { + return err + } + if err := b.Add(a); err != nil { + return err + } + } + + return nil +} From d16542d8d1ad945d1c0ecc281fd2156f85e2fe56 Mon Sep 17 00:00:00 2001 From: Miguel Pires Date: Thu, 16 Jul 2026 17:13:33 +0100 Subject: [PATCH 2/3] o/assertstate: Commit fetches asserts and checks enforcement Use (TryEnforced|Monitor)ValidationSets to fetch validation sets and perform checks (enforcement or against model) before changing state. Since changes made through confdb can affect many validation sets, it's important to note that changes are not atomic. We'll first check if all the enforce changes can be applied and abort, if not. These is the most frequent failure scenario so it's the first one to be checked. From then on, if a monitor or forget change fails, the previous changes are still persisted. This could be due to model checks failing or a failure to fetch a validation set. We can make these changes atomic, but it would require quite a bit of special-purpose code, so I didn't think it was was worth it, since the original API doesn't support it anyway. Signed-off-by: Miguel Pires --- .../assertstate/confdb/validation_sets.go | 172 +++++++---- .../confdb/validation_sets_test.go | 268 ++++++++++++++++-- 2 files changed, 355 insertions(+), 85 deletions(-) diff --git a/overlord/assertstate/confdb/validation_sets.go b/overlord/assertstate/confdb/validation_sets.go index 0aa0dae776c..89f07f52da1 100644 --- a/overlord/assertstate/confdb/validation_sets.go +++ b/overlord/assertstate/confdb/validation_sets.go @@ -338,15 +338,16 @@ func buildSnapsEntry(snaps []*asserts.ValidationSetSnap) []map[string]any { } // Commit translates the changes in the Transaction into validation-set state. -// State must be locked by caller. +// State must be locked by caller. Changes are not persisted atomically across +// all kinds and validation sets (see apply for more details). func (c *ValsetsConfdbHandler) Commit(st *state.State, tx *confdbstate.Transaction) ([]*state.TaskSet, error) { view, err := confdbstate.GetView(st, "system", "validation-sets", "admin") if err != nil { return nil, fmt.Errorf("internal error: unexpected confdb-schema in validation-sets handler: %v", err) } - type vsKey struct{ account, name string } - valsets := make(map[vsKey][][]confdb.Accessor) + seen := make(map[valsetRef]bool) + changes := &validationSetChanges{} for _, path := range tx.AlteredPaths() { if len(path) < 3 { // shouldn't be possible as confdb-schema doesn't allow it @@ -359,81 +360,144 @@ func (c *ValsetsConfdbHandler) Commit(st *state.State, tx *confdbstate.Transacti return nil, fmt.Errorf("internal error: cannot write to system/validation-sets: unsupported storage version %q", path[0].Name()) } - k := vsKey{account: path[1].Name(), name: path[2].Name()} - valsets[k] = append(valsets[k], path) - } + // deduplicate the ids since AlteredPaths() may return several specific paths + // under the same validation set (for mode and pinned-sequence, for example) + ref := valsetRef{accountID: path[1].Name(), name: path[2].Name()} + if seen[ref] { + continue + } + seen[ref] = true - for k := range valsets { - request := k.account + "." + k.name - result, err := view.Get(tx, request, nil, confdb.AdminAccess) + // separate changes into forgets, monitors and enforcement so we can check + // do enforcement checks before the other two + change, err := extractChangeData(view, tx, ref) if err != nil { - if errors.Is(err, &confdb.NoDataError{}) { - if err := assertstate.ForgetValidationSet(st, k.account, k.name, assertstate.ForgetValidationSetOpts{}); err != nil { - return nil, fmt.Errorf("cannot forget validation set %s/%s: %v", k.account, k.name, err) - } - continue - } - return nil, fmt.Errorf("cannot read validation set %s/%s from confdb: %v", k.account, k.name, err) + return nil, err } + changes.add(change) + } - val, ok := result.(map[string]any) - if !ok { - return nil, fmt.Errorf("internal error: unexpected result type %T for validation set %s/%s", result, k.account, k.name) - } + return nil, changes.apply(st) +} - tr := &assertstate.ValidationSetTracking{} - err = assertstate.GetValidationSet(st, k.account, k.name, tr) - if err != nil && !errors.Is(err, state.ErrNoState) { - return nil, fmt.Errorf("cannot read validation-set %s/%s for commit: %v", k.account, k.name, err) - } - tr.AccountID = k.account - tr.Name = k.name +type valsetRef struct { + accountID string + name string - err = applyChanges(k.account, k.name, tr, val) - if err != nil { - return nil, err - } + pinnedSeq int +} - assertstate.UpdateValidationSet(st, tr) +func (r valsetRef) String() string { + var seqStr string + if r.pinnedSeq != 0 { + seqStr = "=" + strconv.Itoa(r.pinnedSeq) } + return r.accountID + "/" + r.name + seqStr +} - return nil, nil +// valsetChange represents a change to a validation-set's tracking state. +type valsetChange struct { + // kind can be "monitor", "enforce" or "forget" + kind string + id valsetRef } -// applyChanges applies values set through confdb to the ValidationSetTracking. -func applyChanges(accountID, name string, tr *assertstate.ValidationSetTracking, val any) error { - valset, ok := val.(map[string]any) +// extractChangeData extracts the validation set reference and change type +// (enforce, monitor or forget) for the data modified through confdb. +func extractChangeData(view *confdb.View, tx *confdbstate.Transaction, ref valsetRef) (valsetChange, error) { + request := ref.accountID + "." + ref.name + result, err := view.Get(tx, request, nil, confdb.AdminAccess) + if err != nil { + if errors.Is(err, &confdb.NoDataError{}) { + return valsetChange{kind: "forget", id: ref}, nil + } + return valsetChange{}, fmt.Errorf("cannot read validation set %s/%s from confdb: %v", ref.accountID, ref.name, err) + } + + val, ok := result.(map[string]any) if !ok { - return fmt.Errorf("internal error: unexpected type %T for validation set %s/%s", val, accountID, name) + return valsetChange{}, fmt.Errorf("internal error: unexpected result type %T for validation set %s/%s", result, ref.accountID, ref.name) } - if rawMode, ok := valset["mode"]; ok { - mode, ok := rawMode.(string) - if !ok { - // writes are validated against the storage schema so shouldn't be possible - return fmt.Errorf(`internal error: "mode" should be a string, got %[1]T: %[1]v`, rawMode) + // extract mode + var mode string + if rawMode, ok := val["mode"]; ok { + if modeStr, ok := rawMode.(string); ok { + mode = modeStr } + } - // per the storage schema these are the only choices and it can't be unset - switch mode { - case "monitor": - tr.Mode = assertstate.Monitor - case "enforce": - tr.Mode = assertstate.Enforce - } + if mode != "monitor" && mode != "enforce" { + // the schema already validates the mode so this should be impossible + return valsetChange{}, fmt.Errorf(`internal error: mode must be present as either "monitor" or "enforce": got %v`, val["mode"]) } - if rawSeq, ok := valset["pinned-sequence"]; ok { + // extract pinned-sequence, if any is set + if rawSeq, ok := val["pinned-sequence"]; ok { v, ok := rawSeq.(float64) if !ok { // writes are validated against the storage schema so shouldn't be possible - return fmt.Errorf(`internal error: "pinned-sequence" should be an int, got %[1]T: %[1]v`, rawSeq) + return valsetChange{}, fmt.Errorf(`internal error: "pinned-sequence" should be an int, got %v`, rawSeq) } + ref.pinnedSeq = int(v) + } - tr.PinnedAt = int(v) - } else { - tr.PinnedAt = 0 + return valsetChange{kind: mode, id: ref}, nil +} + +// validationSetChanges collects the changes to be applied to validation-set +// tracking state as part of a confdb commit. +type validationSetChanges struct { + forgets []valsetRef + monitors []valsetRef + enforces []valsetRef +} + +// add buckets the given change into the appropriate slice. +func (c *validationSetChanges) add(change valsetChange) { + switch change.kind { + case "forget": + c.forgets = append(c.forgets, change.id) + case "enforce": + c.enforces = append(c.enforces, change.id) + case "monitor": + c.monitors = append(c.monitors, change.id) } +} + +// apply persists the collected changes starting with enforcement, then monitoring +// and finally forgetting (sorted by likelihood of failing). Enforcement changes +// are atomic but other kinds are not. Failures to enforce do not change the +// state or asserts DB, but failures to monitor do not rollback previous monitor +// or enforcement changes. +func (c *validationSetChanges) apply(st *state.State) error { + if len(c.enforces) > 0 { + snaps, ignoreValidation, err := snapstate.InstalledSnaps(st) + if err != nil { + return err + } + enforceStrings := make([]string, 0, len(c.enforces)) + for _, ref := range c.enforces { + enforceStrings = append(enforceStrings, ref.String()) + } + + if err := assertstate.TryEnforcedValidationSets(st, enforceStrings, 0, snaps, ignoreValidation); err != nil { + return fmt.Errorf("cannot enforce validation sets: %v", err) + } + } + + for _, ref := range c.monitors { + if _, err := assertstate.MonitorValidationSet(st, ref.accountID, ref.name, ref.pinnedSeq, 0); err != nil { + return fmt.Errorf("cannot monitor validation set %s/%s: %v", ref.accountID, ref.name, err) + } + } + + for _, ref := range c.forgets { + opts := assertstate.ForgetValidationSetOpts{} + if err := assertstate.ForgetValidationSet(st, ref.accountID, ref.name, opts); err != nil { + return fmt.Errorf("cannot forget validation set %s/%s: %v", ref.accountID, ref.name, err) + } + } return nil } diff --git a/overlord/assertstate/confdb/validation_sets_test.go b/overlord/assertstate/confdb/validation_sets_test.go index 3ab477f8f8a..cedb3e45e58 100644 --- a/overlord/assertstate/confdb/validation_sets_test.go +++ b/overlord/assertstate/confdb/validation_sets_test.go @@ -28,9 +28,11 @@ import ( "github.com/snapcore/snapd/asserts" "github.com/snapcore/snapd/asserts/assertstest" + "github.com/snapcore/snapd/asserts/sysdb" "github.com/snapcore/snapd/confdb" "github.com/snapcore/snapd/dirs" "github.com/snapcore/snapd/overlord/assertstate" + "github.com/snapcore/snapd/overlord/assertstate/assertstatetest" assertstateconfdb "github.com/snapcore/snapd/overlord/assertstate/confdb" "github.com/snapcore/snapd/overlord/confdbstate" "github.com/snapcore/snapd/overlord/snapstate" @@ -88,19 +90,10 @@ func (s *confdbHandlerSuite) SetUpTest(c *C) { assertstate.ReplaceDB(s.st, db) - // mock a model so ForgetValidationSet doesn't panic - a := assertstest.FakeAssertion(map[string]any{ - "type": "model", - "authority-id": "canonical", - "series": "16", - "brand-id": "canonical", - "model": "pc", - "architecture": "amd64", - "gadget": "pc", - "kernel": "pc-kernel", - }) + // mock a model and store so we can use the real (Monitor|TryEnforced)ValidationSet helpers deviceCtx := &snapstatetest.TrivialDeviceContext{ - DeviceModel: a.(*asserts.Model), + DeviceModel: sysdb.GenericClassicModel(), + CtxStore: &assertstatetest.FakeStore{State: s.st, DB: s.storeSigning}, } s.AddCleanup(snapstatetest.MockDeviceContext(deviceCtx)) s.st.Set("seeded", true) @@ -123,20 +116,36 @@ func (s *confdbHandlerSuite) SetUpTest(c *C) { }, }) + // valid-set's constraints are met and the set can be enforced + s.addValidationSetAssert(c, "my-account", "valid-set", 1, []any{ + map[string]any{ + "id": "qOqKhntON3vR7kwEbVPsILm7bUViPDzz", + "name": "enforced-snap", + "presence": "required", + "revision": "7", + }, + }) + s.view, err = confdbstate.GetView(s.st, "system", "validation-sets", "admin") c.Assert(err, IsNil) } -// addValidationSetAssert signs and adds a validation-set assertion to the DB. -// Developer accounts and signing keys are created on first use per account. +// addValidationSetAssert signs a validation-set assertion and adds it to the +// fake store as well as the local assertion DB. Adding to the store allows +// MonitorValidationSet/TryEnforcedValidationSets to fetch it; adding to the +// local DB allows tests that only inspect tracking (e.g. via Databag) to work +// without going through a Commit. Developer accounts and signing keys are +// created on first use per account. func (s *confdbHandlerSuite) addValidationSetAssert(c *C, accountID, name string, sequence int, snaps []any) { if _, ok := s.devSignings[accountID]; !ok { privKey, _ := assertstest.GenerateKey(752) acct := assertstest.NewAccount(s.storeSigning, accountID, map[string]any{ "account-id": accountID, }, "") + c.Assert(s.storeSigning.Add(acct), IsNil) c.Assert(assertstate.Add(s.st, acct), IsNil) acctKey := assertstest.NewAccountKey(s.storeSigning, acct, nil, privKey.PublicKey(), "") + c.Assert(s.storeSigning.Add(acctKey), IsNil) c.Assert(assertstate.Add(s.st, acctKey), IsNil) s.devSignings[accountID] = assertstest.NewSigningDB(accountID, privKey) } @@ -158,6 +167,7 @@ func (s *confdbHandlerSuite) addValidationSetAssert(c *C, accountID, name string } a, err := s.devSignings[accountID].Sign(asserts.ValidationSetType, headers, nil, "") c.Assert(err, IsNil) + c.Assert(s.storeSigning.Add(a), IsNil) c.Assert(assertstate.Add(s.st, a), IsNil) } @@ -281,6 +291,17 @@ func (s *confdbHandlerSuite) TestUpdateEntireValidationSet(c *C) { s.st.Lock() defer s.st.Unlock() + // add an assertion at sequence 5 with an installed snap so the enforce + // constraint check succeeds + s.addValidationSetAssert(c, "my-account", "my-set", 5, []any{ + map[string]any{ + "id": "qOqKhntON3vR7kwEbVPsILm7bUViPDzz", + "name": "enforced-snap", + "presence": "required", + "revision": "7", + }, + }) + assertstate.UpdateValidationSet(s.st, &assertstate.ValidationSetTracking{ AccountID: "my-account", Name: "my-set", @@ -312,9 +333,9 @@ func (s *confdbHandlerSuite) TestCommitUpdatesOnlyMode(c *C) { assertstate.UpdateValidationSet(s.st, &assertstate.ValidationSetTracking{ AccountID: "my-account", - Name: "my-set", + Name: "valid-set", Mode: assertstate.Enforce, - PinnedAt: 3, + PinnedAt: 1, Current: 1, }) @@ -322,7 +343,7 @@ func (s *confdbHandlerSuite) TestCommitUpdatesOnlyMode(c *C) { c.Assert(err, IsNil) // set specific path and check other data isn't affected - err = s.view.Set(tx, "my-account.my-set.mode", "monitor") + err = s.view.Set(tx, "my-account.valid-set.mode", "monitor") c.Assert(err, IsNil) handler := &assertstateconfdb.ValsetsConfdbHandler{} @@ -330,22 +351,22 @@ func (s *confdbHandlerSuite) TestCommitUpdatesOnlyMode(c *C) { c.Assert(err, IsNil) var tr assertstate.ValidationSetTracking - err = assertstate.GetValidationSet(s.st, "my-account", "my-set", &tr) + err = assertstate.GetValidationSet(s.st, "my-account", "valid-set", &tr) c.Assert(err, IsNil) c.Check(tr.Mode, Equals, assertstate.Monitor) - c.Check(tr.PinnedAt, Equals, 3) + c.Check(tr.PinnedAt, Equals, 1) // if we set the entire validation set map without pinned-sequence, it's removed tx, err = confdbstate.NewTransaction(s.st, "system", "validation-sets") c.Assert(err, IsNil) - err = s.view.Set(tx, "my-account.my-set", map[string]any{"mode": "enforce"}) + err = s.view.Set(tx, "my-account.valid-set", map[string]any{"mode": "enforce"}) c.Assert(err, IsNil) _, err = handler.Commit(s.st, tx) c.Assert(err, IsNil) - err = assertstate.GetValidationSet(s.st, "my-account", "my-set", &tr) + err = assertstate.GetValidationSet(s.st, "my-account", "valid-set", &tr) c.Assert(err, IsNil) c.Check(tr.Mode, Equals, assertstate.Enforce) c.Check(tr.PinnedAt, Equals, 0) @@ -357,9 +378,9 @@ func (s *confdbHandlerSuite) TestCommitUnsetsPinnedSequence(c *C) { assertstate.UpdateValidationSet(s.st, &assertstate.ValidationSetTracking{ AccountID: "my-account", - Name: "my-set", + Name: "valid-set", Mode: assertstate.Enforce, - PinnedAt: 7, + PinnedAt: 1, Current: 1, }) @@ -367,7 +388,7 @@ func (s *confdbHandlerSuite) TestCommitUnsetsPinnedSequence(c *C) { c.Assert(err, IsNil) // unset part of the data - err = s.view.Unset(tx, "my-account.my-set.pinned-sequence") + err = s.view.Unset(tx, "my-account.valid-set.pinned-sequence") c.Assert(err, IsNil) handler := &assertstateconfdb.ValsetsConfdbHandler{} @@ -375,7 +396,7 @@ func (s *confdbHandlerSuite) TestCommitUnsetsPinnedSequence(c *C) { c.Assert(err, IsNil) var tr assertstate.ValidationSetTracking - err = assertstate.GetValidationSet(s.st, "my-account", "my-set", &tr) + err = assertstate.GetValidationSet(s.st, "my-account", "valid-set", &tr) c.Assert(err, IsNil) c.Check(tr.Mode, Equals, assertstate.Enforce) c.Check(tr.PinnedAt, Equals, 0) @@ -436,6 +457,20 @@ func (s *confdbHandlerSuite) TestCommitForgetsDeletedValidationSet(c *C) { var tr assertstate.ValidationSetTracking err = assertstate.GetValidationSet(s.st, "my-account", "my-set", &tr) c.Assert(err, testutil.ErrorIs, state.ErrNoState) + + // try to unset a val set that's not currently tracked + tx, err = confdbstate.NewTransaction(s.st, "system", "validation-sets") + c.Assert(err, IsNil) + + err = s.view.Unset(tx, "my-account.my-set") + c.Assert(err, IsNil) + + // unsetting an untracked validation set should be a no-op + _, err = handler.Commit(s.st, tx) + c.Assert(err, IsNil) + + err = assertstate.GetValidationSet(s.st, "my-account", "my-set", &assertstate.ValidationSetTracking{}) + c.Assert(err, testutil.ErrorIs, state.ErrNoState) } func (s *confdbHandlerSuite) TestCommitRejectsUnsupportedStorageVersion(c *C) { @@ -475,12 +510,13 @@ func (s *confdbHandlerSuite) TestCommitMultipleSetsAcrossAccounts(c *C) { Current: 1, }) s.addValidationSetAssert(c, "acct2", "set-c", 1, nil) + s.addValidationSetAssert(c, "acct2", "set-c", 2, nil) assertstate.UpdateValidationSet(s.st, &assertstate.ValidationSetTracking{ AccountID: "acct2", Name: "set-c", Mode: assertstate.Enforce, PinnedAt: 2, - Current: 1, + Current: 2, }) tx, err := confdbstate.NewTransaction(s.st, "system", "validation-sets") @@ -488,7 +524,7 @@ func (s *confdbHandlerSuite) TestCommitMultipleSetsAcrossAccounts(c *C) { err = s.view.Set(tx, "acct1.set-a", map[string]any{ "mode": "enforce", - "pinned-sequence": 10, + "pinned-sequence": 1, }) c.Assert(err, IsNil) err = s.view.Set(tx, "acct1.set-b", map[string]any{ @@ -511,16 +547,186 @@ func (s *confdbHandlerSuite) TestCommitMultipleSetsAcrossAccounts(c *C) { name string mode assertstate.ValidationSetMode pin int + current int }{ - {account: "acct1", name: "set-a", mode: assertstate.Enforce, pin: 10}, - {account: "acct1", name: "set-b", mode: assertstate.Monitor, pin: 1}, - {account: "acct2", name: "set-c", mode: assertstate.Monitor, pin: 2}, + {account: "acct1", name: "set-a", mode: assertstate.Enforce, pin: 1, current: 1}, + {account: "acct1", name: "set-b", mode: assertstate.Monitor, pin: 1, current: 1}, + {account: "acct2", name: "set-c", mode: assertstate.Monitor, pin: 2, current: 2}, } { var tr assertstate.ValidationSetTracking err = assertstate.GetValidationSet(s.st, tc.account, tc.name, &tr) c.Assert(err, IsNil) c.Check(tr.Mode, Equals, tc.mode) - c.Check(tr.Current, Equals, 1) + c.Check(tr.Current, Equals, tc.current) c.Check(tr.PinnedAt, Equals, tc.pin) } } + +func (s *confdbHandlerSuite) TestCommitFailsIfEnforcingUnknownSequence(c *C) { + s.st.Lock() + defer s.st.Unlock() + + assertstate.UpdateValidationSet(s.st, &assertstate.ValidationSetTracking{ + AccountID: "my-account", + Name: "my-set", + Mode: assertstate.Monitor, + Current: 1, + }) + + tx, err := confdbstate.NewTransaction(s.st, "system", "validation-sets") + c.Assert(err, IsNil) + + // pin to a sequence that has no local assertion + err = s.view.Set(tx, "my-account.my-set", map[string]any{"mode": "enforce", "pinned-sequence": 99}) + c.Assert(err, IsNil) + + handler := &assertstateconfdb.ValsetsConfdbHandler{} + _, err = handler.Commit(s.st, tx) + c.Assert(err, ErrorMatches, `cannot enforce validation sets: .*validation-set \(99; series:16 account-id:my-account name:my-set\) not found`) +} + +func (s *confdbHandlerSuite) TestCommitEnforceFailureLeavesStateUnchanged(c *C) { + s.st.Lock() + defer s.st.Unlock() + + s.addValidationSetAssert(c, "my-account", "another-valid-set", 1, []any{ + map[string]any{ + "id": "qOqKhntON3vR7kwEbVPsILm7bUViPDzz", + "name": "enforced-snap", + "presence": "required", + "revision": "7", + }, + }) + + initialSets := []*assertstate.ValidationSetTracking{ + {AccountID: "my-account", Name: "valid-set", Mode: assertstate.Enforce, PinnedAt: 1, Current: 1}, + {AccountID: "my-account", Name: "another-valid-set", Mode: assertstate.Enforce, PinnedAt: 1, Current: 1}, + } + for _, tr := range initialSets { + assertstate.UpdateValidationSet(s.st, tr) + } + + initialHistory, err := assertstate.ValidationSetsHistory(s.st) + c.Assert(err, IsNil) + + tx, err := confdbstate.NewTransaction(s.st, "system", "validation-sets") + c.Assert(err, IsNil) + + // enforcing my-set will fail due to a missing snap, so state shouldn't change + c.Assert(s.view.Set(tx, "my-account.my-set.mode", "enforce"), IsNil) + c.Assert(s.view.Set(tx, "my-account.valid-set.mode", "monitor"), IsNil) + c.Assert(s.view.Unset(tx, "my-account.another-valid-set"), IsNil) + + handler := &assertstateconfdb.ValsetsConfdbHandler{} + _, err = handler.Commit(s.st, tx) + c.Assert(err, ErrorMatches, `(?s)cannot enforce validation sets: .*missing required snaps.*`) + + // check state looks the same + sets, err := assertstate.ValidationSets(s.st) + c.Assert(err, IsNil) + c.Assert(sets, HasLen, len(initialSets)) + + for _, tr := range initialSets { + got, ok := sets[assertstate.ValidationSetKey(tr.AccountID, tr.Name)] + c.Assert(ok, Equals, true, Commentf("expected %s/%s to still be tracked", tr.AccountID, tr.Name)) + c.Check(got.Mode, Equals, tr.Mode) + c.Check(got.PinnedAt, Equals, tr.PinnedAt) + c.Check(got.Current, Equals, tr.Current) + } + + // history is also unchanged + newHistory, err := assertstate.ValidationSetsHistory(s.st) + c.Assert(err, IsNil) + c.Check(newHistory, DeepEquals, initialHistory) +} + +func (s *confdbHandlerSuite) TestCommitMonitorFailureDoesNotRollbackEnforce(c *C) { + s.st.Lock() + defer s.st.Unlock() + + tx, err := confdbstate.NewTransaction(s.st, "system", "validation-sets") + c.Assert(err, IsNil) + + // the monitor change will fail but the enforcement isn't rolled back + c.Assert(s.view.Set(tx, "my-account.valid-set.mode", "enforce"), IsNil) + c.Assert(s.view.Set(tx, "my-account.nonexistent-set.mode", "monitor"), IsNil) + + handler := &assertstateconfdb.ValsetsConfdbHandler{} + _, err = handler.Commit(s.st, tx) + c.Assert(err, ErrorMatches, `(?s)cannot monitor validation set my-account/nonexistent-set: .*`) + + sets, err := assertstate.ValidationSets(s.st) + c.Assert(err, IsNil) + c.Assert(sets, HasLen, 1) + + got, ok := sets[assertstate.ValidationSetKey("my-account", "valid-set")] + c.Assert(ok, Equals, true) + c.Check(got.Mode, Equals, assertstate.Enforce) + c.Check(got.Current, Equals, 1) +} + +func (s *confdbHandlerSuite) TestCommitForgetFailureDoesNotRollbackEnforceOrMonitor(c *C) { + s.st.Lock() + defer s.st.Unlock() + + // mock a model that enforces a validation set we'll try (and fail) to forget + headers := sysdb.GenericClassicModel().Headers() + headers["validation-sets"] = []any{ + map[string]any{ + "account-id": "my-account", + "name": "valid-set", + "mode": "enforce", + "sequence": "1", + }, + } + + s.AddCleanup(snapstatetest.MockDeviceContext(&snapstatetest.TrivialDeviceContext{ + DeviceModel: assertstest.FakeAssertion(headers).(*asserts.Model), + CtxStore: &assertstatetest.FakeStore{State: s.st, DB: s.storeSigning}, + })) + + s.addValidationSetAssert(c, "my-account", "another-valid-set", 1, []any{ + map[string]any{ + "id": "qOqKhntON3vR7kwEbVPsILm7bUViPDzz", + "name": "enforced-snap", + "presence": "required", + "revision": "7", + }, + }) + assertstate.UpdateValidationSet(s.st, &assertstate.ValidationSetTracking{ + AccountID: "my-account", Name: "another-valid-set", Mode: assertstate.Monitor, PinnedAt: 1, Current: 1, + }) + assertstate.UpdateValidationSet(s.st, &assertstate.ValidationSetTracking{ + AccountID: "my-account", Name: "valid-set", Mode: assertstate.Enforce, PinnedAt: 1, Current: 1, + }) + + tx, err := confdbstate.NewTransaction(s.st, "system", "validation-sets") + c.Assert(err, IsNil) + + // these will succeed + c.Assert(s.view.Set(tx, "my-account.another-valid-set.mode", "enforce"), IsNil) + c.Assert(s.view.Set(tx, "my-account.my-set.mode", "monitor"), IsNil) + // but this will fail (enforced by the model) + c.Assert(s.view.Unset(tx, "my-account.valid-set"), IsNil) + + handler := &assertstateconfdb.ValsetsConfdbHandler{} + _, err = handler.Commit(s.st, tx) + c.Assert(err, ErrorMatches, `cannot forget validation set my-account/valid-set: validation-set is enforced by the model`) + + sets, err := assertstate.ValidationSets(s.st) + c.Assert(err, IsNil) + c.Assert(sets, HasLen, 3) + + // valid-set wasn't forgotten but the other two changes were persisted + got, ok := sets[assertstate.ValidationSetKey("my-account", "valid-set")] + c.Assert(ok, Equals, true) + c.Check(got.Mode, Equals, assertstate.Enforce) + + got, ok = sets[assertstate.ValidationSetKey("my-account", "another-valid-set")] + c.Assert(ok, Equals, true) + c.Check(got.Mode, Equals, assertstate.Enforce) + + got, ok = sets[assertstate.ValidationSetKey("my-account", "my-set")] + c.Assert(ok, Equals, true) + c.Check(got.Mode, Equals, assertstate.Monitor) +} From 93df1a9d76f6416e65d4aaf974d2a8636bbc6b73 Mon Sep 17 00:00:00 2001 From: Miguel Pires Date: Wed, 22 Jul 2026 10:57:25 +0100 Subject: [PATCH 3/3] o/assertstate: rename valset id type Signed-off-by: Miguel Pires --- .../assertstate/confdb/validation_sets.go | 38 ++++++++++--------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/overlord/assertstate/confdb/validation_sets.go b/overlord/assertstate/confdb/validation_sets.go index 89f07f52da1..c0518f3975d 100644 --- a/overlord/assertstate/confdb/validation_sets.go +++ b/overlord/assertstate/confdb/validation_sets.go @@ -346,7 +346,7 @@ func (c *ValsetsConfdbHandler) Commit(st *state.State, tx *confdbstate.Transacti return nil, fmt.Errorf("internal error: unexpected confdb-schema in validation-sets handler: %v", err) } - seen := make(map[valsetRef]bool) + seen := make(map[atSequence]bool) changes := &validationSetChanges{} for _, path := range tx.AlteredPaths() { if len(path) < 3 { @@ -362,15 +362,15 @@ func (c *ValsetsConfdbHandler) Commit(st *state.State, tx *confdbstate.Transacti // deduplicate the ids since AlteredPaths() may return several specific paths // under the same validation set (for mode and pinned-sequence, for example) - ref := valsetRef{accountID: path[1].Name(), name: path[2].Name()} - if seen[ref] { + seq := atSequence{accountID: path[1].Name(), name: path[2].Name()} + if seen[seq] { continue } - seen[ref] = true + seen[seq] = true // separate changes into forgets, monitors and enforcement so we can check // do enforcement checks before the other two - change, err := extractChangeData(view, tx, ref) + change, err := extractChangeData(view, tx, seq) if err != nil { return nil, err } @@ -380,14 +380,15 @@ func (c *ValsetsConfdbHandler) Commit(st *state.State, tx *confdbstate.Transacti return nil, changes.apply(st) } -type valsetRef struct { +// atSequence identifies a validation-set, optionally specifying a sequence. +type atSequence struct { accountID string name string pinnedSeq int } -func (r valsetRef) String() string { +func (r atSequence) String() string { var seqStr string if r.pinnedSeq != 0 { seqStr = "=" + strconv.Itoa(r.pinnedSeq) @@ -397,19 +398,20 @@ func (r valsetRef) String() string { // valsetChange represents a change to a validation-set's tracking state. type valsetChange struct { - // kind can be "monitor", "enforce" or "forget" + // kind denotes the type of change which can be "monitor", "enforce" or "forget". kind string - id valsetRef + // valsetID identifies the validation-set, possibly pinning to a sequence. + valsetID atSequence } // extractChangeData extracts the validation set reference and change type // (enforce, monitor or forget) for the data modified through confdb. -func extractChangeData(view *confdb.View, tx *confdbstate.Transaction, ref valsetRef) (valsetChange, error) { +func extractChangeData(view *confdb.View, tx *confdbstate.Transaction, ref atSequence) (valsetChange, error) { request := ref.accountID + "." + ref.name result, err := view.Get(tx, request, nil, confdb.AdminAccess) if err != nil { if errors.Is(err, &confdb.NoDataError{}) { - return valsetChange{kind: "forget", id: ref}, nil + return valsetChange{kind: "forget", valsetID: ref}, nil } return valsetChange{}, fmt.Errorf("cannot read validation set %s/%s from confdb: %v", ref.accountID, ref.name, err) } @@ -442,26 +444,26 @@ func extractChangeData(view *confdb.View, tx *confdbstate.Transaction, ref valse ref.pinnedSeq = int(v) } - return valsetChange{kind: mode, id: ref}, nil + return valsetChange{kind: mode, valsetID: ref}, nil } // validationSetChanges collects the changes to be applied to validation-set // tracking state as part of a confdb commit. type validationSetChanges struct { - forgets []valsetRef - monitors []valsetRef - enforces []valsetRef + forgets []atSequence + monitors []atSequence + enforces []atSequence } // add buckets the given change into the appropriate slice. func (c *validationSetChanges) add(change valsetChange) { switch change.kind { case "forget": - c.forgets = append(c.forgets, change.id) + c.forgets = append(c.forgets, change.valsetID) case "enforce": - c.enforces = append(c.enforces, change.id) + c.enforces = append(c.enforces, change.valsetID) case "monitor": - c.monitors = append(c.monitors, change.id) + c.monitors = append(c.monitors, change.valsetID) } }