Let a requirement be optional, binding only a provider that already runs (hq issue 166, ADR 0330)

Making every soft dependency a provision made it hard: a mesh without influxdb could not assign
grafana, and one with influxdb in its catalogue would install it for grafana. A requirement may now
be written {"name": ..., "optional": true}. It resolves as a hard one when a provider is assigned or
reachable, and is otherwise absent: never refused, nothing installed for it. Its binding, credential
and only-when-bound files are left out, its ${bound:...|default:...} placeholders take their
defaults, the module check refuses one that could render empty, and plan and node list it unbound.

internal/catalogue/optional_requirements_test.go

@@ -0,0 +1,249 @@
+package catalogue
+
+import (
+	"encoding/json"
+	"strings"
+	"testing"
+)
+
+// A requirement a module can run without (novox/hq ADR 0330, issue 166). The shape of the issue:
+// grafana runs without a data source and requires `influxdb-api` softly; a mesh without influxdb
+// could not assign grafana at all, and one with influxdb in its catalogue would have had it installed
+// for grafana.
+
+func optionalShelf() map[string]Manifest {
+	grafana := Manifest{Module: "grafana", Version: "1", Requires: []string{"influxdb-api", "shell"},
+		OptionalRequires: map[string]bool{"influxdb-api": true, "shell": true}}
+	return shelf(
+		Manifest{Module: "influxdb", Version: "1", Provides: FromAnywhere("influxdb-api")},
+		Manifest{Module: "zsh", Version: "1", Provides: Offers("shell")},
+		grafana,
+		// What wants the same names hard, for the cases where one consumer's optional is another's hard.
+		Manifest{Module: "telegraf", Version: "1", Requires: []string{"influxdb-api"}},
+		Manifest{Module: "editor", Version: "1", Requires: []string{"shell"}},
+		Manifest{Module: "suite", Version: "1", Requires: []string{"editor"}},
+	)
+}
+
+func TestARequirementIsHardUnlessMarkedOptional(t *testing.T) {
+	m := mustParse(t, `{"module":"grafana","version":"1",
+		"requires":["route",{"name":"influxdb-api","optional":true},{"name":"oidc-client"}]}`)
+	if strings.Join(m.Requires, " ") != "route influxdb-api oidc-client" {
+		t.Fatalf("the requirements read as %v", m.Requires)
+	}
+	if m.IsOptional("route") || m.IsOptional("oidc-client") {
+		t.Error("a requirement without `optional` read as optional; hard is the default (ADR 0330 §1)")
+	}
+	if !m.IsOptional("influxdb-api") {
+		t.Error("a requirement marked optional read as hard")
+	}
+	// And written back as it was read, so a manifest through the store keeps its form.
+	raw, err := json.Marshal(m)
+	if err != nil {
+		t.Fatal(err)
+	}
+	var again Manifest
+	if err := json.Unmarshal(raw, &again); err != nil {
+		t.Fatal(err)
+	}
+	if !again.IsOptional("influxdb-api") || again.IsOptional("route") || len(again.Requires) != 3 {
+		t.Fatalf("written and read back, it says %v optional %v (%s)", again.Requires, again.OptionalRequires, raw)
+	}
+	if strings.Contains(string(raw), `"name":"route"`) {
+		t.Errorf("a hard requirement was written in the long form: %s", raw)
+	}
+}
+
+func TestARequirementsLongFormRefusesWhatItDoesNotKnow(t *testing.T) {
+	for _, raw := range []string{
+		`{"module":"x","version":"1","requires":[{"name":"influxdb-api","optinal":true}]}`,
+		`{"module":"x","version":"1","requires":[{"optional":true}]}`,
+	} {
+		if _, err := ParseManifest([]byte(raw)); err == nil {
+			t.Errorf("accepted %s", raw)
+		}
+	}
+}
+
+func TestAnOptionalRequirementWithNoProviderIsAbsentAndNothingIsInstalled(t *testing.T) {
+	// Exactly one catalogue module could answer each — the case where a hard requirement installs it.
+	// Brokered (influxdb-api, from anywhere) and on this machine (shell); both passes.
+	for _, world := range []World{{}, {Unchecked: true}} {
+		got, err := Resolve(optionalShelf(), []string{"grafana"}, reachable(), world)
+		if err != nil {
+			t.Fatalf("an optional requirement with no provider refused the assignment: %v", err)
+		}
+		if have := strings.Join(names(got), " "); have != "grafana" {
+			t.Fatalf("assigning grafana put %s on the machine; nothing is installed for an optional requirement (ADR 0330 §3)", have)
+		}
+		if world.Unchecked {
+			continue
+		}
+		want := []Absent{{Consumer: "grafana", Requirement: "influxdb-api"}, {Consumer: "grafana", Requirement: "shell"}}
+		if len(got.Absent) != 2 || got.Absent[0] != want[0] || got.Absent[1] != want[1] {
+			t.Fatalf("absent: %+v, want %+v", got.Absent, want)
+		}
+		if len(got.Needs) != 0 {
+			t.Fatalf("an absent requirement was given a binding: %+v", got.Needs)
+		}
+	}
+}
+
+func TestAnOptionalRequirementWithAProviderBindsAsAHardOne(t *testing.T) {
+	world := World{Offered: map[string][]Provider{"influxdb-api": {
+		{Node: "anchor", At: "anchor.internal", Module: "influxdb", Serves: map[string]any{"port": 8086}}}}}
+	got, err := Resolve(optionalShelf(), []string{"grafana", "zsh"}, reachable(), world)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if n := boundFrom(t, got, "grafana", "influxdb-api"); n.From != "anchor" || n.Module != "influxdb" {
+		t.Fatalf("bound to %s/%s", n.From, n.Module)
+	}
+	if len(got.Absent) != 0 {
+		t.Fatalf("a requirement with a provider was left absent: %+v", got.Absent)
+	}
+	// And the same assignment without the provider is absent: what binds it later is only the
+	// provider being assigned, with no act on grafana (ADR 0330 §7).
+	before, err := Resolve(optionalShelf(), []string{"grafana", "zsh"}, reachable(), World{})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(before.Absent) != 1 || before.Absent[0].Requirement != "influxdb-api" {
+		t.Fatalf("absent before the provider was assigned: %+v", before.Absent)
+	}
+}
+
+func TestOneConsumersOptionalRequirementIsAnothersHardOne(t *testing.T) {
+	// telegraf wants influxdb-api hard: with nothing offering it, the assignment is refused as
+	// before, naming what to assign — grafana's optional does not soften telegraf's.
+	_, err := Resolve(optionalShelf(), []string{"grafana", "telegraf"}, reachable(), World{})
+	if err == nil || !strings.Contains(err.Error(), "assign influxdb") {
+		t.Fatalf("a hard requirement with no provider was not refused naming the candidate: %v", err)
+	}
+	// And a module pulled in later (suite → editor) that wants the shell hard installs it, even
+	// though grafana, met first, wanted it only optionally — and grafana is then bound to it.
+	got, err := Resolve(optionalShelf(), []string{"grafana", "suite"}, workstation(), World{})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if have := strings.Join(names(got), " "); !strings.Contains(have, "zsh") {
+		t.Fatalf("a hard requirement met after an optional one was not installed: %s", have)
+	}
+	for _, a := range got.Absent {
+		if a.Requirement == "shell" {
+			t.Fatalf("the shell is installed and still listed absent: %+v", got.Absent)
+		}
+	}
+}
+
+func TestAHardRequirementWithNoProviderIsStillRefused(t *testing.T) {
+	_, err := Resolve(optionalShelf(), []string{"telegraf"}, reachable(), World{})
+	if err == nil || !strings.Contains(err.Error(), "influxdb") {
+		t.Fatalf("got %v", err)
+	}
+}
+
+func optionalModule(resources string) string {
+	return `{"module":"grafana","version":"1",
+		"requires":["route",{"name":"influxdb-api","optional":true}],
+		"secrets":{"influxdb-api":"/var/lib/grafana/influxdb-api"},
+		"resources":[` + resources + `]}`
+}
+
+func TestTheModuleCheckRefusesAnOptionalPlaceholderThatCouldRenderEmpty(t *testing.T) {
+	refused := map[string]string{
+		"a placeholder without a default": `{"id":"ds","type":"file","path":"/a","mode":"0644",
+			"content":"url: ${bound:influxdb-api:at}"}`,
+		"a secret outside an only-when-bound file": `{"id":"s","type":"file","path":"/a","mode":"0400",
+			"content":"${secret:influxdb-api}"}`,
+		"only-when-bound on a hard requirement": `{"id":"r","type":"file","path":"/a","mode":"0644",
+			"content":"x","only-when-bound":"route"}`,
+		"only-when-bound on something not required": `{"id":"r","type":"file","path":"/a","mode":"0644",
+			"content":"x","only-when-bound":"mqtt-topic"}`,
+		"only-when-bound on a directory": `{"id":"d","type":"directory","mode":"0700",
+			"only-when-bound":"influxdb-api"}`,
+		"a default on a hard requirement": `{"id":"r","type":"file","path":"/a","mode":"0644",
+			"content":"${bound:route:name|default:x}"}`,
+	}
+	for what, resource := range refused {
+		if _, err := ParseManifest([]byte(optionalModule(resource))); err == nil {
+			t.Errorf("%s passed the module check", what)
+		} else if what == "a placeholder without a default" && !strings.Contains(err.Error(), "|default:") {
+			t.Errorf("the refusal does not say how to write it: %v", err)
+		}
+	}
+	accepted := map[string]string{
+		"a placeholder with a default": `{"id":"ds","type":"file","path":"/a","mode":"0644",
+			"content":"url: ${bound:influxdb-api:at|default:none}"}`,
+		"a placeholder and a secret inside an only-when-bound file": `{"id":"ds","type":"file","path":"/a",
+			"mode":"0400","content":"url: ${bound:influxdb-api:at}\npassword: ${secret:influxdb-api}",
+			"only-when-bound":"influxdb-api"}`,
+	}
+	for what, resource := range accepted {
+		if _, err := ParseManifest([]byte(optionalModule(resource))); err != nil {
+			t.Errorf("%s was refused: %v", what, err)
+		}
+	}
+}
+
+func absentGrafana(t *testing.T, absent bool) Resolution {
+	t.Helper()
+	m := mustParse(t, optionalModule(`
+		{"id":"env","type":"file","path":"/var/lib/grafana/env","mode":"0644",
+		 "content":"INFLUX=${bound:influxdb-api:at|default:none}:${bound:influxdb-api:port|default:0}\n"},
+		{"id":"ds","type":"file","path":"/var/lib/grafana/ds.yaml","mode":"0400",
+		 "content":"url: ${bound:influxdb-api:at}\npassword: ${secret:influxdb-api}\n","only-when-bound":"influxdb-api"}`))
+	m.Requires = []string{"influxdb-api"}
+	m.Binds = map[string]string{"influxdb-api": "/var/lib/grafana/influxdb.json"}
+	r := Resolution{Node: "workstation", Modules: []Manifest{m}}
+	if absent {
+		r.Absent = []Absent{{Consumer: "grafana", Requirement: "influxdb-api"}}
+	} else {
+		r.Needs = []Needed{{Name: "influxdb-api", From: "anchor", At: "anchor.internal", For: "grafana",
+			Module: "influxdb", Sealed: "sealed-influx", Serves: map[string]any{"port": float64(8086)}}}
+	}
+	return r
+}
+
+func TestAnAbsentRequirementIsComposedWithItsDefaultsAndWithoutItsFiles(t *testing.T) {
+	out, err := absentGrafana(t, true).Declaration(Rendering{})
+	if err != nil {
+		t.Fatal(err)
+	}
+	env := fileNamed(out, "grafana.env")
+	if env == nil || env["content"] != "INFLUX=none:0\n" {
+		t.Fatalf("the defaults were not rendered: %v", env)
+	}
+	for _, id := range []string{"grafana.ds", "grafana." + SecretID("influxdb-api"), "grafana." + BoundID("influxdb-api")} {
+		if fileNamed(out, id) != nil {
+			t.Errorf("%s was delivered while influxdb-api is absent (ADR 0330 §5, §6)", id)
+		}
+	}
+	for _, r := range out {
+		if _, has := r[OnlyWhenBound]; has {
+			t.Errorf("%v carries %s to the node-engine, which knows no such field", r["id"], OnlyWhenBound)
+		}
+	}
+}
+
+func TestABoundOptionalRequirementIsComposedAsAHardOne(t *testing.T) {
+	out, err := absentGrafana(t, false).Declaration(Rendering{})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if env := fileNamed(out, "grafana.env"); env == nil || env["content"] != "INFLUX=anchor.internal:8086\n" {
+		t.Fatalf("the bound values were not rendered: %v", env)
+	}
+	ds := fileNamed(out, "grafana.ds")
+	if ds == nil {
+		t.Fatalf("the only-when-bound file was left out while bound: %v", out)
+	}
+	if _, has := ds[OnlyWhenBound]; has {
+		t.Errorf("the file carries %s to the node-engine", OnlyWhenBound)
+	}
+	for _, id := range []string{"grafana." + SecretID("influxdb-api"), "grafana." + BoundID("influxdb-api")} {
+		if fileNamed(out, id) == nil {
+			t.Errorf("%s was not delivered while bound", id)
+		}
+	}
+}