o/confdbstate: handle read/writes to builtin confdbs

confdb/confdb.go

@@ -324,6 +324,11 @@ func (s *Schema) ID() SchemaID {
 	}
 }
 
+// IsSystem returns true if the schema represents a system builtin confdb-schema.
+func (s *Schema) IsSystem() bool {
+	return s.Account == "system"
+}
+
 // GetViewsAffectedByPath returns all the views in the confdb schema that have
 // visibility into a storage path.
 func (s *Schema) GetViewsAffectedByPath(path []Accessor) []*View {

confdb/confdb_test.go

@@ -197,6 +197,24 @@ func (*viewSuite) TestNewConfdb(c *C) {
 	}
 }
 
+func (*viewSuite) TestSchemaIsSystem(c *C) {
+	views := map[string]any{
+		"foo": map[string]any{
+			"rules": []any{
+				map[string]any{"storage": "foo"},
+			},
+		},
+	}
+
+	db, err := confdb.NewSchema("system", "foo", views, confdb.NewJSONSchema())
+	c.Assert(err, IsNil)
+	c.Check(db.IsSystem(), Equals, true)
+
+	db, err = confdb.NewSchema("other", "foo", views, confdb.NewJSONSchema())
+	c.Assert(err, IsNil)
+	c.Check(db.IsSystem(), Equals, false)
+}
+
 func (s *viewSuite) TestMissingRequestDefaultsToStorage(c *C) {
 	databag := confdb.NewJSONDatabag()
 	views := map[string]any{

interfaces/builtin/confdb.go

@@ -76,6 +76,10 @@ func (iface *confdbInterface) BeforePreparePlug(plug *snap.PlugInfo) error {
 		return fmt.Errorf(`optional confdb plug "role" attribute must be "custodian"`)
 	}
 
+	if role == "custodian" && account == "system" {
+		return fmt.Errorf(`snaps cannot be custodians of "system" confdb schemas`)
+	}
+
 	return nil
 }
 

interfaces/builtin/confdb_test.go

@@ -98,6 +98,16 @@ func (s *confdbSuite) TestConfdbSanitizePlug(c *C) {
 			account: "my-acc",
 			view:    "network/wifi",
 		},
+		{
+			account: "system",
+			view:    "network/wifi",
+		},
+		{
+			account: "system",
+			view:    "network/wifi",
+			role:    "custodian",
+			err:     `snaps cannot be custodians of "system" confdb schemas`,
+		},
 		{
 			err: `confdb plug must have an "account" attribute`,
 		},

overlord/confdbstate/confdbmgr.go

@@ -23,6 +23,7 @@ import (
 	"fmt"
 	"regexp"
 	"strings"
+	"time"
 
 	"github.com/snapcore/snapd/client"
 	"github.com/snapcore/snapd/confdb"
@@ -62,8 +63,11 @@ type SystemConfdbHandler interface {
 	Databag(st *state.State) (confdb.JSONDatabag, error)
 }
 
-// systemHandlers holds handlers for "system" confdb-schemas.
-var systemHandlers = map[string]SystemConfdbHandler{}
+var (
+	// systemHandlers holds handlers for "system" confdb-schemas.
+	systemHandlers = map[string]SystemConfdbHandler{}
+	commitTimeout  = 2 * time.Second
+)
 
 // RegisterConfdbHandler registers a handler for a "system" confdb-schema.
 func RegisterConfdbHandler(c SystemConfdbHandler) {
@@ -124,7 +128,7 @@ func (m *ConfdbManager) doCommitTransaction(t *state.Task, _ *tomb.Tomb) (err er
 	st.Lock()
 	defer st.Unlock()
 
-	tx, _, _, err := GetStoredTransaction(t)
+	tx, _, saveTxChanges, err := GetStoredTransaction(t)
 	if err != nil {
 		return err
 	}
@@ -153,6 +157,63 @@ func (m *ConfdbManager) doCommitTransaction(t *state.Task, _ *tomb.Tomb) (err er
 		}
 	}
 
+	// changes to "system" confdbs are persisted by subsystem handlers, not by
+	// the usual databag commit path
+	if tx.ConfdbAccount == "system" {
+		handler, ok := systemHandlers[tx.ConfdbName]
+		if !ok {
+			// shouldn't happen; we check for this early
+			return fmt.Errorf("internal error: no system handler registered for confdb-schema %q", tx.ConfdbName)
+		}
+
+		var committed bool
+		if err := t.Get("scheduled-tasks", &committed); err != nil && !errors.Is(err, state.ErrNoState) {
+			return err
+		}
+
+		if committed {
+			// a previous run of this task scheduled async tasks which have now
+			// finished (see the async path below) so we're done. Reset the tx which
+			// re-reads state, so hooks get fully updated state since the subsystem
+			// handlers may make state changes.
+			if err := tx.Reset(st); err != nil {
+				return err
+			}
+			saveTxChanges()
+			return nil
+		}
+
+		taskSets, err := handler.Commit(st, tx)
+		if err != nil {
+			return err
+		}
+
+		if len(taskSets) == 0 {
+			// synchronous commit, nothing to wait for. Reset the tx which re-reads
+			// state, so hooks get fully updated state since the subsystem handlers
+			// may make state changes.
+			if err := tx.Reset(st); err != nil {
+				return err
+			}
+			saveTxChanges()
+			return nil
+		}
+
+		// this confdb handler needs to run tasks asynchronously, so make this task
+		// wait for those and rerun when they're done
+		chg := t.Change()
+		for _, ts := range taskSets {
+			chg.AddAll(ts)
+			t.WaitAll(ts)
+		}
+		t.Set("scheduled-tasks", true)
+
+		// ensure the new tasks are picked up by the task runner
+		st.EnsureBefore(0)
+
+		return &state.Retry{After: commitTimeout}
+	}
+
 	// we error early if a write may affect ephemeral data but no save-view hook
 	// is present. However, a change-view hook may have written to an ephemeral
 	// path after that so we have to check again
@@ -587,7 +648,7 @@ func (h *saveViewHandler) Error(origErr error) (ignoreErr bool, err error) {
 	}
 
 	// clear the transaction changes
-	err = tx.Clear(st)
+	err = tx.Reset(st)
 	if err != nil {
 		return false, fmt.Errorf("cannot rollback failed save-view: cannot clear transaction changes: %v", err)
 	}

overlord/confdbstate/confdbmgr_test.go

@@ -465,7 +465,7 @@ func (s *confdbTestSuite) TestCommitTransaction(c *C) {
 
 	// clearing would remove non-committed changes, so if we read the set value
 	// it's because it has been successfully committed
-	err = tx.Clear(s.state)
+	err = tx.Reset(s.state)
 	c.Assert(err, IsNil)
 
 	val, err := tx.Get(parsePath(c, "wifi.ssid"), nil)

overlord/confdbstate/confdbstate.go

@@ -478,13 +478,22 @@ func WriteConfdbFromSnap(hookCtx *hookstate.Context, view *confdb.View, values m
 }
 
 func createChangeConfdbTasks(st *state.State, tx *Transaction, view *confdb.View, callingSnap string) (ts *state.TaskSet, commitTask, clearTxTask *state.Task, err error) {
-	custodians, custodianPlugs, err := getCustodianPlugsForView(st, view)
-	if err != nil {
-		return nil, nil, nil, err
-	}
+	var custodians []string
+	var custodianPlugs map[string]*snap.PlugInfo
+	if !view.Schema().IsSystem() {
+		custodians, custodianPlugs, err = getCustodianPlugsForView(st, view)
+		if err != nil {
+			return nil, nil, nil, err
+		}
 
-	if len(custodianPlugs) == 0 {
-		return nil, nil, nil, fmt.Errorf("cannot write confdb view %s: no custodian snap connected", view.ID())
+		if len(custodianPlugs) == 0 {
+			return nil, nil, nil, fmt.Errorf("cannot write confdb view %s: no custodian snap connected", view.ID())
+		}
+	} else {
+		// fail early if there is no appropriate subsystem handler
+		if _, ok := systemHandlers[tx.ConfdbName]; !ok {
+			return nil, nil, nil, fmt.Errorf("cannot write confdb system/%s: no internal handler", view.Schema().Name)
+		}
 	}
 
 	paths := tx.AlteredPaths()
@@ -524,7 +533,7 @@ func createChangeConfdbTasks(st *state.State, tx *Transaction, view *confdb.View
 			linkTask(chgViewTask)
 		}
 
-		if hookPrefix == "save-view-" && mightAffectEph && !saveViewHookPresent {
+		if hookPrefix == "save-view-" && mightAffectEph && !saveViewHookPresent && !view.Schema().IsSystem() {
 			return nil, nil, nil, fmt.Errorf("cannot write confdb view %s: write might change ephemeral data but no custodians has a save-view hook", view.ID())
 		}
 	}
@@ -949,13 +958,23 @@ func ReadConfdb(ctx context.Context, st *state.State, view *confdb.View, request
 // load-view or query-view hooks, nil is returned. If there are hooks to run,
 // a clear-confdb-tx task is also scheduled to remove the ongoing transaction at the end.
 func createLoadConfdbTasks(st *state.State, tx *Transaction, view *confdb.View, requests []string, constraints map[string]any) (*state.TaskSet, *state.Task, error) {
-	custodians, custodianPlugs, err := getCustodianPlugsForView(st, view)
-	if err != nil {
-		return nil, nil, err
-	}
+	var custodians []string
+	var custodianPlugs map[string]*snap.PlugInfo
+	if !view.Schema().IsSystem() {
+		var err error
+		custodians, custodianPlugs, err = getCustodianPlugsForView(st, view)
+		if err != nil {
+			return nil, nil, err
+		}
 
-	if len(custodians) == 0 {
-		return nil, nil, fmt.Errorf("cannot read confdb view %s: no custodian snap connected", view.ID())
+		if len(custodians) == 0 {
+			return nil, nil, fmt.Errorf("cannot read confdb view %s: no custodian snap connected", view.ID())
+		}
+	} else {
+		// fail early if there is no appropriate subsystem handler
+		if _, ok := systemHandlers[tx.ConfdbName]; !ok {
+			return nil, nil, fmt.Errorf("cannot read confdb system/%s: no internal handler", view.Schema().Name)
+		}
 	}
 
 	ts := state.NewTaskSet()
@@ -992,7 +1011,7 @@ func createLoadConfdbTasks(st *state.State, tx *Transaction, view *confdb.View,
 		}
 
 		// there must be least one load-view hook if we're accessing ephemeral data
-		if hookPrefix == "load-view-" && mightAffectEph && !loadViewHookPresent {
+		if hookPrefix == "load-view-" && mightAffectEph && !loadViewHookPresent && !view.Schema().IsSystem() {
 			return nil, nil, fmt.Errorf("cannot schedule tasks to read view %s: read might cover ephemeral data but no custodian has a load-view hook", view.ID())
 		}
 	}