Read settings for a config-form module that mesh-config holds, not only for one with a settings row (issue 1493)

cmd/mesh-controller/compose_input.go

@@ -375,7 +375,7 @@ func readLicences(ctx context.Context, open *stores) ([]compose.Licence, error)
 	return out, nil
 }
 
-// readSettings is each node's settings layers for every module that has any, read BEFORE any node is resolved: the first pass
+// readSettings is each node's settings layers for every module that has any (a row, or config in mesh-config), read BEFORE any node is resolved: the first pass
 // resolves a node whose set takes a provider's served facts from the node that runs it, with that node's settings on them
 // (servedOnNode), so a mesh whose provider says ${setting:domain} resolves only with the settings in the Input. A module's
 // settings that fail to read are returned in unread, by node and module, and are an error only for a module a node resolves to
@@ -385,13 +385,12 @@ func readSettings(ctx context.Context, open *stores, in *compose.Input) (unread
 	if err != nil {
 		return nil, err
 	}
-	names := make([]string, 0, len(have))
-	for module := range have {
-		if _, onShelf := in.Catalogue[module]; onShelf {
-			names = append(names, module)
-		}
+	names, err := modulesToRead(in.Catalogue, have, func(module string) (bool, error) {
+		return open.inventory.ReadsConfig(ctx, module)
+	})
+	if err != nil {
+		return nil, err
 	}
-	sort.Strings(names)
 	for i := range in.Nodes {
 		n := &in.Nodes[i]
 		for _, module := range names {
@@ -418,6 +417,32 @@ func readSettings(ctx context.Context, open *stores, in *compose.Input) (unread
 	return unread, nil
 }
 
+// modulesToRead is the modules on the shelf whose settings are read for a node: those with a settings row, and those in
+// the config form whose values mesh-config holds (readsConfig), which have no row when they were never in the settings
+// form (issue 1493: brother's scanner and printer, set only in mesh-config, composed empty). SettingsFor is where
+// mesh-config's values are laid in, so a gate on the settings table alone leaves them out.
+func modulesToRead(shelf map[string]catalogue.Manifest, have map[string]bool, readsConfig func(string) (bool, error)) ([]string, error) {
+	names := make([]string, 0, len(have))
+	for module, m := range shelf {
+		if have[module] {
+			names = append(names, module)
+			continue
+		}
+		if !m.DeclaresConfig() {
+			continue
+		}
+		reads, err := readsConfig(module)
+		if err != nil {
+			return nil, err
+		}
+		if reads {
+			names = append(names, module)
+		}
+	}
+	sort.Strings(names)
+	return names, nil
+}
+
 // unreadSettings is the error of a module's settings that did not read, for a module the node resolves to.
 func unreadSettings(unread map[string]map[string]error, plans map[string]compose.Resolved) error {
 	names := make([]string, 0, len(plans))

cmd/mesh-controller/compose_input_settings_test.go

@@ -0,0 +1,45 @@
+package main
+
+import (
+	"errors"
+	"reflect"
+	"testing"
+
+	"git.novox.be/novox/mesh-sdk/go/catalogue"
+)
+
+// configForm is a module in the config form, shaped as a real one (inventory's config_move_test).
+func configForm(t *testing.T, name string) catalogue.Manifest {
+	t.Helper()
+	m, err := catalogue.ParseManifest([]byte(`{"module": "` + name + `", "version": "1",
+		"config": {"level": {"type": "string", "description": "how much it says", "default": "info"}},
+		"resources": [{"id": "conf", "type": "file", "path": "/etc/` + name + `.json", "mode": "0640",
+			"content": "{\"level\": \"info\"}", "merge": "json", "takes-config": ["level"]}]}`))
+	if err != nil {
+		t.Fatal(err)
+	}
+	return m
+}
+
+// Issue 1493: a module in the config form whose values are set only in mesh-config has no settings row, and its
+// settings are still read, because SettingsFor is where mesh-config's values are laid in.
+func TestSettingsAreReadForAConfigModuleWithoutASettingsRow(t *testing.T) {
+	shelf := map[string]catalogue.Manifest{
+		"brother": configForm(t, "brother"),        // config only in mesh-config: no settings row
+		"older":   {Module: "older", Version: "1"}, // settings form, with a row
+		"idle":    {Module: "idle", Version: "1"},  // settings form, no row: nothing to read
+		"newer":   configForm(t, "newer"),          // config form, but the switch is off for it
+	}
+	have := map[string]bool{"older": true, "gone": true} // "gone" has a row and is not on the shelf
+	reads := func(module string) (bool, error) { return module == "brother", nil }
+	got, err := modulesToRead(shelf, have, reads)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if want := []string{"brother", "older"}; !reflect.DeepEqual(got, want) {
+		t.Errorf("settings are read for %v, want %v", got, want)
+	}
+	if _, err := modulesToRead(shelf, have, func(string) (bool, error) { return false, errors.New("store down") }); err == nil {
+		t.Error("a failing ReadsConfig is not returned")
+	}
+}