o/assertstate: add Commit to validation-sets confdb handler (#17344)

* o/assertstate: add Commit to validation-sets confdb handler

Adds a Commit() method to the validation-sets confdb handler. This
enables the handler to convert confdb changes into validation-set
metadata changes (mode, pinned-sequence).

Signed-off-by: Miguel Pires <miguel.pires@canonical.com>

Author: miguelpires

overlord/assertstate/confdb/validation_sets.go

@@ -27,7 +27,7 @@ import (
 
 	"github.com/snapcore/snapd/asserts"
 	"github.com/snapcore/snapd/asserts/snapasserts"
-	confdbpkg "github.com/snapcore/snapd/confdb"
+	"github.com/snapcore/snapd/confdb"
 	"github.com/snapcore/snapd/overlord/assertstate"
 	"github.com/snapcore/snapd/overlord/confdbstate"
 	"github.com/snapcore/snapd/overlord/snapstate"
@@ -226,14 +226,14 @@ func (c *ValsetsConfdbHandler) SchemaName() string {
 // Databag reads all validation set tracking from the state and returns a
 // confdb.JSONDatabag structured as described in the system/validation-sets
 // confdb-schema. State must be locked by caller.
-func (c *ValsetsConfdbHandler) Databag(st *state.State) (confdbpkg.JSONDatabag, error) {
+func (c *ValsetsConfdbHandler) Databag(st *state.State) (confdb.JSONDatabag, error) {
 	sets, err := assertstate.ValidationSets(st)
 	if err != nil {
 		return nil, err
 	}
 
 	if len(sets) == 0 {
-		return confdbpkg.NewJSONDatabag(), nil
+		return confdb.NewJSONDatabag(), nil
 	}
 
 	db := assertstate.DB(st)
@@ -337,6 +337,103 @@ func buildSnapsEntry(snaps []*asserts.ValidationSetSnap) []map[string]any {
 	return result
 }
 
-func (c *ValsetsConfdbHandler) Commit(*state.State, *confdbstate.Transaction) ([]*state.TaskSet, error) {
-	return nil, errors.New("not implemented yet")
+// Commit translates the changes in the Transaction into validation-set state.
+// State must be locked by caller.
+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)
+	for _, path := range tx.AlteredPaths() {
+		if len(path) < 3 {
+			// shouldn't be possible as confdb-schema doesn't allow it
+			return nil, fmt.Errorf("internal error: unexpected storage path: %v", confdb.JoinAccessors(path))
+		}
+
+		// Databag() will need changes if we add v2 paths in the confdb-schema, so
+		// fail here to flag the issue
+		if path[0].Name() != "v1" {
+			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)
+	}
+
+	for k := range valsets {
+		request := k.account + "." + k.name
+		result, err := view.Get(tx, request, nil, confdb.AdminAccess)
+		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)
+		}
+
+		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)
+		}
+
+		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
+
+		err = applyChanges(k.account, k.name, tr, val)
+		if err != nil {
+			return nil, err
+		}
+
+		assertstate.UpdateValidationSet(st, tr)
+	}
+
+	return nil, nil
+}
+
+// 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)
+	if !ok {
+		return fmt.Errorf("internal error: unexpected type %T for validation set %s/%s", val, accountID, 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)
+		}
+
+		// 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 rawSeq, ok := valset["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)
+		}
+
+		tr.PinnedAt = int(v)
+	} else {
+		tr.PinnedAt = 0
+	}
+
+	return nil
 }

overlord/assertstate/confdb/validation_sets_test.go

@@ -276,3 +276,251 @@ func (s *confdbHandlerSuite) TestDatabagMultipleSetsAndAccounts(c *C) {
 		},
 	})
 }
+
+func (s *confdbHandlerSuite) TestUpdateEntireValidationSet(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)
+
+	// set the entire validation set map
+	err = s.view.Set(tx, "my-account.my-set", map[string]any{"mode": "enforce", "pinned-sequence": 5})
+	c.Assert(err, IsNil)
+
+	handler := &assertstateconfdb.ValsetsConfdbHandler{}
+	_, err = handler.Commit(s.st, tx)
+	c.Assert(err, IsNil)
+
+	var tr assertstate.ValidationSetTracking
+	err = assertstate.GetValidationSet(s.st, "my-account", "my-set", &tr)
+	c.Assert(err, IsNil)
+	c.Check(tr.Mode, Equals, assertstate.Enforce)
+	c.Check(tr.PinnedAt, Equals, 5)
+}
+
+func (s *confdbHandlerSuite) TestCommitUpdatesOnlyMode(c *C) {
+	s.st.Lock()
+	defer s.st.Unlock()
+
+	assertstate.UpdateValidationSet(s.st, &assertstate.ValidationSetTracking{
+		AccountID: "my-account",
+		Name:      "my-set",
+		Mode:      assertstate.Enforce,
+		PinnedAt:  3,
+		Current:   1,
+	})
+
+	tx, err := confdbstate.NewTransaction(s.st, "system", "validation-sets")
+	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")
+	c.Assert(err, IsNil)
+
+	handler := &assertstateconfdb.ValsetsConfdbHandler{}
+	_, err = handler.Commit(s.st, tx)
+	c.Assert(err, IsNil)
+
+	var tr assertstate.ValidationSetTracking
+	err = assertstate.GetValidationSet(s.st, "my-account", "my-set", &tr)
+	c.Assert(err, IsNil)
+	c.Check(tr.Mode, Equals, assertstate.Monitor)
+	c.Check(tr.PinnedAt, Equals, 3)
+
+	// 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"})
+	c.Assert(err, IsNil)
+
+	_, err = handler.Commit(s.st, tx)
+	c.Assert(err, IsNil)
+
+	err = assertstate.GetValidationSet(s.st, "my-account", "my-set", &tr)
+	c.Assert(err, IsNil)
+	c.Check(tr.Mode, Equals, assertstate.Enforce)
+	c.Check(tr.PinnedAt, Equals, 0)
+}
+
+func (s *confdbHandlerSuite) TestCommitUnsetsPinnedSequence(c *C) {
+	s.st.Lock()
+	defer s.st.Unlock()
+
+	assertstate.UpdateValidationSet(s.st, &assertstate.ValidationSetTracking{
+		AccountID: "my-account",
+		Name:      "my-set",
+		Mode:      assertstate.Enforce,
+		PinnedAt:  7,
+		Current:   1,
+	})
+
+	tx, err := confdbstate.NewTransaction(s.st, "system", "validation-sets")
+	c.Assert(err, IsNil)
+
+	// unset part of the data
+	err = s.view.Unset(tx, "my-account.my-set.pinned-sequence")
+	c.Assert(err, IsNil)
+
+	handler := &assertstateconfdb.ValsetsConfdbHandler{}
+	_, err = handler.Commit(s.st, tx)
+	c.Assert(err, IsNil)
+
+	var tr assertstate.ValidationSetTracking
+	err = assertstate.GetValidationSet(s.st, "my-account", "my-set", &tr)
+	c.Assert(err, IsNil)
+	c.Check(tr.Mode, Equals, assertstate.Enforce)
+	c.Check(tr.PinnedAt, Equals, 0)
+}
+
+func (s *confdbHandlerSuite) TestCannotUnsetMode(c *C) {
+	s.st.Lock()
+	defer s.st.Unlock()
+
+	assertstate.UpdateValidationSet(s.st, &assertstate.ValidationSetTracking{
+		AccountID: "my-account",
+		Name:      "my-set",
+		Mode:      assertstate.Enforce,
+		PinnedAt:  7,
+		Current:   1,
+	})
+
+	tx, err := confdbstate.NewTransaction(s.st, "system", "validation-sets")
+	c.Assert(err, IsNil)
+
+	val, err := s.view.Get(tx, "my-account.my-set.mode", nil, confdb.AdminAccess)
+	c.Assert(err, IsNil)
+	c.Assert(val, Equals, "enforce")
+
+	err = s.view.Set(tx, "my-account.my-set", map[string]any{
+		"pinned-sequence": 5,
+	})
+	c.Assert(err, ErrorMatches, `.*cannot find required combinations of keys`)
+
+	// unsetting mode fails because the storage schema marks it as required
+	err = s.view.Unset(tx, "my-account.my-set.mode")
+	c.Assert(err, ErrorMatches, `.*cannot find required combinations of keys`)
+}
+
+func (s *confdbHandlerSuite) TestCommitForgetsDeletedValidationSet(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)
+
+	// unset through confdb
+	err = s.view.Unset(tx, "my-account.my-set")
+	c.Assert(err, IsNil)
+
+	handler := &assertstateconfdb.ValsetsConfdbHandler{}
+	_, err = handler.Commit(s.st, tx)
+	c.Assert(err, IsNil)
+
+	// check it was deleted from state
+	var tr assertstate.ValidationSetTracking
+	err = assertstate.GetValidationSet(s.st, "my-account", "my-set", &tr)
+	c.Assert(err, testutil.ErrorIs, state.ErrNoState)
+}
+
+func (s *confdbHandlerSuite) TestCommitRejectsUnsupportedStorageVersion(c *C) {
+	s.st.Lock()
+	defer s.st.Unlock()
+
+	tx, err := confdbstate.NewTransaction(s.st, "system", "validation-sets")
+	c.Assert(err, IsNil)
+
+	path, err := confdb.ParsePathIntoAccessors("v2.my-account.my-set.mode", confdb.ParseOptions{})
+	c.Assert(err, IsNil)
+	err = tx.Set(path, "enforce")
+	c.Assert(err, IsNil)
+
+	handler := &assertstateconfdb.ValsetsConfdbHandler{}
+	_, err = handler.Commit(s.st, tx)
+	c.Assert(err, ErrorMatches, `internal error: cannot write to system/validation-sets: unsupported storage version "v2"`)
+}
+
+func (s *confdbHandlerSuite) TestCommitMultipleSetsAcrossAccounts(c *C) {
+	s.st.Lock()
+	defer s.st.Unlock()
+
+	s.addValidationSetAssert(c, "acct1", "set-a", 1, nil)
+	assertstate.UpdateValidationSet(s.st, &assertstate.ValidationSetTracking{
+		AccountID: "acct1",
+		Name:      "set-a",
+		Mode:      assertstate.Monitor,
+		Current:   1,
+	})
+	s.addValidationSetAssert(c, "acct1", "set-b", 1, nil)
+	assertstate.UpdateValidationSet(s.st, &assertstate.ValidationSetTracking{
+		AccountID: "acct1",
+		Name:      "set-b",
+		Mode:      assertstate.Enforce,
+		PinnedAt:  1,
+		Current:   1,
+	})
+	s.addValidationSetAssert(c, "acct2", "set-c", 1, nil)
+	assertstate.UpdateValidationSet(s.st, &assertstate.ValidationSetTracking{
+		AccountID: "acct2",
+		Name:      "set-c",
+		Mode:      assertstate.Enforce,
+		PinnedAt:  2,
+		Current:   1,
+	})
+
+	tx, err := confdbstate.NewTransaction(s.st, "system", "validation-sets")
+	c.Assert(err, IsNil)
+
+	err = s.view.Set(tx, "acct1.set-a", map[string]any{
+		"mode":            "enforce",
+		"pinned-sequence": 10,
+	})
+	c.Assert(err, IsNil)
+	err = s.view.Set(tx, "acct1.set-b", map[string]any{
+		"mode":            "monitor",
+		"pinned-sequence": 1,
+	})
+	c.Assert(err, IsNil)
+	err = s.view.Set(tx, "acct2.set-c", map[string]any{
+		"mode":            "monitor",
+		"pinned-sequence": 2,
+	})
+	c.Assert(err, IsNil)
+
+	handler := &assertstateconfdb.ValsetsConfdbHandler{}
+	_, err = handler.Commit(s.st, tx)
+	c.Assert(err, IsNil)
+
+	for _, tc := range []struct {
+		account string
+		name    string
+		mode    assertstate.ValidationSetMode
+		pin     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},
+	} {
+		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.PinnedAt, Equals, tc.pin)
+	}
+}

overlord/confdbstate/confdbmgr.go

@@ -164,7 +164,7 @@ func (m *ConfdbManager) doCommitTransaction(t *state.Task, _ *tomb.Tomb) (err er
 		}
 
 		view := confdbAssert.Schema().View(viewName)
-		paths := tx.alteredPaths()
+		paths := tx.AlteredPaths()
 		mightAffectEph, err := view.WriteAffectsEphemeral(paths)
 		if err != nil {
 			return fmt.Errorf("cannot commit transaction: cannot check for ephemeral paths: %v", err)