diff --git a/cmd/mesh-controller/modules.go b/cmd/mesh-controller/modules.go index f2d60f4b..66804655 100644 --- a/cmd/mesh-controller/modules.go +++ b/cmd/mesh-controller/modules.go @@ -237,7 +237,15 @@ func moduleCommand(ctx context.Context, args []string) error { says = append(says, "provides "+describeOffers(m.Provides)) } if len(m.Requires) > 0 { - says = append(says, "requires "+strings.Join(m.Requires, ", ")) + // An optional requirement says so (novox/hq ADR 0330): it is not refused when absent. + required := make([]string, 0, len(m.Requires)) + for _, r := range m.Requires { + if m.IsOptional(r) { + r += " (optional)" + } + required = append(required, r) + } + says = append(says, "requires "+strings.Join(required, ", ")) } for _, c := range m.Claims { says = append(says, "claims "+c.At()+"/"+c.Name) diff --git a/cmd/mesh-controller/nodes.go b/cmd/mesh-controller/nodes.go index 7a269c7f..bc29e048 100644 --- a/cmd/mesh-controller/nodes.go +++ b/cmd/mesh-controller/nodes.go @@ -929,6 +929,13 @@ func showNode(ctx context.Context, stored *stores, name string) error { for _, line := range notAppliedLines(name, leftOutOf(plan, settings, node.Adopted)) { fmt.Println(line) } + // And which optional requirements of its modules nothing assigned answers (novox/hq ADR 0330 §7). + if lines := unboundLines(plan); len(lines) > 0 { + fmt.Println() + for _, line := range lines { + fmt.Println(line) + } + } } return nil } diff --git a/cmd/mesh-controller/plan.go b/cmd/mesh-controller/plan.go index 3a6a0a50..ce0147ca 100644 --- a/cmd/mesh-controller/plan.go +++ b/cmd/mesh-controller/plan.go @@ -1315,6 +1315,14 @@ func planCommand(ctx context.Context, args []string) error { fmt.Printf("%s is assigned nothing\n", args[0]) return nil } + // Which optional requirements are unbound (novox/hq ADR 0330 §7): beside the document on the + // error stream where the answer is the declaration itself, which carries no such field — the + // node-engine refuses an envelope key it does not know. + if *asDiff || *asJSON { + for _, line := range unboundLines(plan) { + fmt.Fprintf(os.Stderr, "%s:%s\n", args[0], line) + } + } if *asDiff { declared, err := declarationFor(ctx, open, args[0], plan, settings) if err != nil { @@ -1396,6 +1404,9 @@ func planCommand(ctx context.Context, args []string) error { for _, n := range plan.Needs { fmt.Printf(" needs %s from %s, for %s\n", n.Name, n.From, n.For) } + for _, line := range unboundLines(plan) { + fmt.Println(line) + } declared, err := declarationFor(ctx, open, args[0], plan, settings) if err != nil { return err diff --git a/cmd/mesh-controller/unbound.go b/cmd/mesh-controller/unbound.go new file mode 100644 index 00000000..670054bc --- /dev/null +++ b/cmd/mesh-controller/unbound.go @@ -0,0 +1,21 @@ +package main + +import ( + "fmt" + + "github.com/novox/mesh-controller/internal/catalogue" +) + +// unboundLines say which optional requirements of a machine's modules nothing assigned answers +// (novox/hq ADR 0330 §7): left unbound rather than refused, with nothing installed for them, so the +// module runs without — and said, because a data source that never appears is otherwise a silence. +// Each binds without anybody's act on the first composition after a provider is assigned. +func unboundLines(plan catalogue.Resolution) []string { + lines := make([]string, 0, len(plan.Absent)) + for _, a := range plan.Absent { + lines = append(lines, fmt.Sprintf( + " unbound %s, optional for %s — nothing assigned provides it; it binds once a provider is "+ + "assigned (novox/hq ADR 0330)", a.Requirement, a.Consumer)) + } + return lines +} diff --git a/cmd/mesh-controller/unbound_test.go b/cmd/mesh-controller/unbound_test.go new file mode 100644 index 00000000..8bbc28a7 --- /dev/null +++ b/cmd/mesh-controller/unbound_test.go @@ -0,0 +1,59 @@ +package main + +import ( + "strings" + "testing" + + "github.com/novox/mesh-controller/internal/catalogue" +) + +// A machine running grafana without influxdb says so (novox/hq ADR 0330 §7, issue 166): `plan` and +// `node` list the optional requirement nothing assigned answers, and the requirement is left +// absent rather than refused — influxdb is in the catalogue and is not installed for it. +func TestPlanAndNodeListAnOptionalRequirementLeftUnbound(t *testing.T) { + open := aMesh(t) + ctx := t.Context() + register(t, open, catalogue.Manifest{Module: "influxdb", Version: "1", + Provides: catalogue.FromAnywhere("influxdb-api")}) + register(t, open, catalogue.Manifest{Module: "grafana", Version: "1", + Requires: []string{"influxdb-api"}, OptionalRequires: map[string]bool{"influxdb-api": true}, + Resources: []map[string]any{{"id": "state", "type": "directory", "path": "/var/lib/grafana", "mode": "0700"}}}) + if _, err := assign(ctx, open, "laptop", "grafana"); err != nil { + t.Fatalf("grafana could not be assigned on a mesh without influxdb: %v", err) + } + + plan, _, err := planFor(ctx, open, "laptop") + if err != nil { + t.Fatal(err) + } + for _, m := range plan.Modules { + if m.Module == "influxdb" { + t.Fatal("influxdb was installed for grafana's optional requirement (ADR 0330 §3)") + } + } + const said = "unbound influxdb-api, optional for grafana" + if shown := printed(t, func() error { return planCommand(ctx, []string{"laptop"}) }); !strings.Contains(shown, said) { + t.Errorf("plan laptop does not say influxdb-api is unbound:\n%s", shown) + } + if shown := printed(t, func() error { return showNode(ctx, open, "laptop") }); !strings.Contains(shown, said) { + t.Errorf("node show laptop does not say influxdb-api is unbound:\n%s", shown) + } + // And once influxdb is assigned somewhere, grafana binds to it with no act on grafana. + if _, err := assign(ctx, open, "anchor", "influxdb"); err != nil { + t.Fatal(err) + } + plan, _, err = planFor(ctx, open, "laptop") + if err != nil { + t.Fatal(err) + } + if len(plan.Absent) != 0 { + t.Fatalf("still absent after influxdb was assigned: %+v", plan.Absent) + } + var bound bool + for _, n := range plan.Needs { + bound = bound || n.For == "grafana" && n.Name == "influxdb-api" && n.From == "anchor" + } + if !bound { + t.Fatalf("grafana was not bound to influxdb on anchor: %+v", plan.Needs) + } +} diff --git a/internal/catalogue/bound_into_files.go b/internal/catalogue/bound_into_files.go index 47b18682..e04caad4 100644 --- a/internal/catalogue/bound_into_files.go +++ b/internal/catalogue/bound_into_files.go @@ -26,21 +26,10 @@ import ( // serves — whose keys are agreed by the requirement's name, not by this file. // bound is where a module says a value from one of its bindings belongs: -// ${bound:.}. -var bound = regexp.MustCompile(`\$\{bound:([a-z0-9][a-z0-9.-]*[a-z0-9]):([a-z0-9][a-z0-9_-]*)\}`) - -// boundUsed are the (provision, key) pairs a file's content asks for, first appearance first. -func boundUsed(content string) [][2]string { - var used [][2]string - seen := map[string]bool{} - for _, m := range bound.FindAllStringSubmatch(content, -1) { - if key := m[1] + ":" + m[2]; !seen[key] { - seen[key] = true - used = append(used, [2]string{m[1], m[2]}) - } - } - return used -} +// ${bound::}, and for an optional requirement the value it takes while absent, +// ${bound::|default:} (novox/hq ADR 0330 §4). A default is never empty: an +// absent requirement is never rendered as an empty string. +var bound = regexp.MustCompile(`\$\{bound:([a-z0-9][a-z0-9.-]*[a-z0-9]):([a-z0-9][a-z0-9_-]*)(?:\|default:([^}\n]+))?\}`) // knownFor is everything a module may name from one of its bindings. // @@ -125,7 +114,11 @@ func plainly(value any) string { // serve, is refused. Left as it was, the literal `${bound:x:y}` would be written into a // configuration file and read as a value — a connection to a host called `${bound:x:y}`, failing // somewhere that names neither the module nor the mesh. -func boundInto(resource map[string]any, known map[string]map[string]string, module string) error { +// +// **An optional requirement left absent takes the default its placeholder names** (novox/hq ADR +// 0330 §4), and one that names none is refused rather than rendered empty. The module check refuses +// that already; this is the same rule where the file is made, for a manifest registered before it. +func boundInto(resource map[string]any, known map[string]map[string]string, module string, absent map[string]bool) error { if fmt.Sprint(resource["type"]) != "file" { return nil } @@ -133,24 +126,43 @@ func boundInto(resource map[string]any, known map[string]map[string]string, modu if !ok { return nil } - for _, pair := range boundUsed(content) { - provision, key := pair[0], pair[1] + var refused error + filled := bound.ReplaceAllStringFunc(content, func(token string) string { + if refused != nil { + return token + } + m := bound.FindStringSubmatch(token) + provision, key, fallback := m[1], m[2], m[3] values, has := known[provision] if !has { - return fmt.Errorf( + if absent[provision] { + if fallback != "" { + return fallback + } + refused = fmt.Errorf( + "%s has a file that says ${bound:%s:%s}, and %q is optional and absent here: it names no "+ + "value for then (${bound:%s:%s|default:}), so it would be written empty "+ + "(novox/hq ADR 0330 §4)", module, provision, key, provision, provision, key) + return token + } + refused = fmt.Errorf( "%s has a file that says ${bound:%s:%s}, and %s does not require %q. It may name %s", module, provision, key, module, provision, orNothing(namesOfBindings(known))) + return token } value, said := values[key] if !said { - return fmt.Errorf( + refused = fmt.Errorf( "%s asks its %s binding for %q, and what answers it says %s", module, provision, key, orNothing(namesOfKeys(values))) + return token } - resource["content"] = strings.ReplaceAll( - content, fmt.Sprintf("${bound:%s:%s}", provision, key), value) - content = resource["content"].(string) + return value + }) + if refused != nil { + return refused } + resource["content"] = filled return nil } diff --git a/internal/catalogue/declaration.go b/internal/catalogue/declaration.go index 4cbdde6b..f452d4a2 100644 --- a/internal/catalogue/declaration.go +++ b/internal/catalogue/declaration.go @@ -850,6 +850,11 @@ func (r Resolution) compose(with Rendering, owner map[string]string, // An account depends on nothing the mesh computes. accounts, rest := accountsFirst(resources) resources = append(append(accounts, first...), rest...) + // **A file delivered only while its optional requirement is bound is left out while it is + // absent** (novox/hq ADR 0330 §5), and the field is dropped from the rest: the node-engine + // parses strictly and knows no such field. + absent := r.AbsentFor(m.Module) + resources = withoutAbsent(resources, absent) // No container is given the mesh's names (novox/hq ADR 0148). It used to be: every // container got the whole roster as `--add-host` entries at creation, and a name that @@ -1013,7 +1018,7 @@ func (r Resolution) compose(with Rendering, owner map[string]string, // **Substituted here, not on the machine.** A bound value is not secret — the mesh // holds it in the clear — so there is nothing for the host to be the only witness of, // and sending it already filled in means the host learns no new field. - if err := boundInto(copied, known, m.Module); err != nil { + if err := boundInto(copied, known, m.Module, absent); err != nil { return nil, err } // And what the module could not have written: the machine it turned out to be diff --git a/internal/catalogue/manifest.go b/internal/catalogue/manifest.go index 6e8b398f..0d3f32d2 100644 --- a/internal/catalogue/manifest.go +++ b/internal/catalogue/manifest.go @@ -385,6 +385,10 @@ type Manifest struct { // Requires are names that must be provided by something assigned to the same node. Requires []string `json:"requires,omitempty"` + // OptionalRequires is which of Requires the module can run without (novox/hq ADR 0330): filled + // from the long form of a `requires` entry, {name, optional}, by UnmarshalJSON; never written by + // hand. + OptionalRequires map[string]bool `json:"-"` // Emits are the events this module publishes, named **locally**: `order.placed`, not a subject // and not a routing key. The mesh derives where it lands (design 29 §1), so reorganising the @@ -1354,6 +1358,19 @@ func (m *Manifest) UnmarshalJSON(raw []byte) error { } delete(keys, "contributes") } + var optionalRequires map[string]bool + if requires, ok := keys["requires"]; ok && string(requires) != "null" { + names, optional, err := readRequires(requires) + if err != nil { + return err + } + plainNames, err := json.Marshal(names) + if err != nil { + return err + } + keys["requires"] = plainNames + optionalRequires = optional + } rest, err := json.Marshal(keys) if err != nil { return err @@ -1385,6 +1402,7 @@ func (m *Manifest) UnmarshalJSON(raw []byte) error { } *m = Manifest(fields) m.unknown = unknown + m.OptionalRequires = optionalRequires if len(plain) > 0 { m.Secrets = plain } @@ -1407,13 +1425,20 @@ func (m Manifest) MarshalJSON() ([]byte, error) { if err != nil { return nil, err } - if len(m.SecretsMany) == 0 && len(m.ContributesMany) == 0 { + if len(m.SecretsMany) == 0 && len(m.ContributesMany) == 0 && len(m.OptionalRequires) == 0 { return raw, nil } var keys map[string]json.RawMessage if err := json.Unmarshal(raw, &keys); err != nil { return nil, err } + if len(m.OptionalRequires) > 0 { + requires, err := writeRequires(m) + if err != nil { + return nil, err + } + keys["requires"] = requires + } if len(m.ContributesMany) > 0 { mergedContributes := map[string]any{} for to, values := range m.Contributes { @@ -1693,6 +1718,8 @@ func ParseManifest(raw []byte) (Manifest, error) { problems = append(problems, LogProblems(m)...) // And the defaults it gives its settings (setting_defaults.go, novox/hq ADR 0262). problems = append(problems, SettingProblems(m)...) + // And what it requires optionally, and the files it delivers only while bound (novox/hq ADR 0330). + problems = append(problems, OptionalProblems(m)...) // And which setting keys each mergeable file takes (setting_takes.go, novox/hq ADR 0326). problems = append(problems, TakenSettingsProblems(m)...) wellFormed := true diff --git a/internal/catalogue/optional_requirements.go b/internal/catalogue/optional_requirements.go new file mode 100644 index 00000000..a6ccc2cd --- /dev/null +++ b/internal/catalogue/optional_requirements.go @@ -0,0 +1,257 @@ +package catalogue + +import ( + "bytes" + "encoding/json" + "fmt" + "sort" + "strings" +) + +// A requirement a module can run without (novox/hq ADR 0330, issue 166). +// +// **Hard stays the default.** A requirement written as a bare name is hard, exactly as before: it +// resolves, or the assignment is refused, and a single catalogue candidate is installed for it. One +// written `{"name": "influxdb-api", "optional": true}` resolves the same way when a provider is +// already assigned in the mesh or reachable from the consumer (§2), and is otherwise **absent**: not +// refused, and nothing is installed for it, on this machine or elsewhere (§3). +// +// **An absent requirement is never rendered as an empty value** (§4–§6). Its binding, its pair +// credential and its grant are not composed, so a file that names one of them is delivered only while +// the requirement is bound (`only-when-bound`), and a `${bound:…}` placeholder of it outside such a +// file names the value it takes while absent: `${bound::|default:}`. The +// module check refuses anything else, so the gap is found when the manifest is written, not on a +// machine where it reads as a blank. + +// OnlyWhenBound is a resource's field naming the optional requirement it is delivered only while +// bound (ADR 0330 §5). Said in the catalogue and dropped before the declaration is sent: the +// node-engine parses strictly and knows no such field. +const OnlyWhenBound = "only-when-bound" + +// requirementForm is one entry of `requires` in its long form. +type requirementForm struct { + Name string `json:"name"` + Optional bool `json:"optional,omitempty"` +} + +// readRequires reads `requires` in both of its forms — a bare name, or {name, optional} — into the +// names and which of them are optional. +func readRequires(raw json.RawMessage) ([]string, map[string]bool, error) { + var entries []json.RawMessage + if err := json.Unmarshal(raw, &entries); err != nil { + return nil, nil, fmt.Errorf("requires: a list of names, or of {name, optional}: %w", err) + } + var names []string + optional := map[string]bool{} + for i, e := range entries { + var plain string + if err := json.Unmarshal(e, &plain); err == nil { + names = append(names, plain) + continue + } + var full requirementForm + dec := json.NewDecoder(bytes.NewReader(e)) + dec.DisallowUnknownFields() + if err := dec.Decode(&full); err != nil { + return nil, nil, fmt.Errorf("requires[%d]: a name, or {name, optional} (novox/hq ADR 0330): %w", i, err) + } + if strings.TrimSpace(full.Name) == "" { + return nil, nil, fmt.Errorf("requires[%d] names no requirement: {name, optional}", i) + } + names = append(names, full.Name) + if full.Optional { + optional[full.Name] = true + } + } + if len(optional) == 0 { + optional = nil + } + return names, optional, nil +} + +// writeRequires is `requires` in the form it was read: a bare name for a hard requirement, the long +// form for an optional one, so a manifest that went through the mesh comes out as it went in. +func writeRequires(m Manifest) (json.RawMessage, error) { + out := make([]any, 0, len(m.Requires)) + for _, r := range m.Requires { + if m.OptionalRequires[r] { + out = append(out, requirementForm{Name: r, Optional: true}) + continue + } + out = append(out, r) + } + return json.Marshal(out) +} + +// IsOptional is whether this module requires the name optionally (ADR 0330 §1). A name it contributes +// to without requiring, or requires as a bare name, is hard. +func (m Manifest) IsOptional(requirement string) bool { return m.OptionalRequires[requirement] } + +// wantsHard is whether this module wants the name as a hard requirement: required without +// `optional`, or contributed to. +func (m Manifest) wantsHard(name string) bool { + for _, w := range m.Wants() { + if w == name { + return !m.IsOptional(name) + } + } + return false +} + +// Absent is one optional requirement of one consumer that nothing assigned answers (ADR 0330 §3): +// left unbound rather than refused, and listed by `plan` and `node` (§7). +type Absent struct { + // Consumer is the module that requires it optionally. + Consumer string `json:"consumer"` + // Requirement is what it requires. + Requirement string `json:"requirement"` +} + +// AbsentFor is the optional requirements of one module this resolution leaves unbound. +func (r Resolution) AbsentFor(module string) map[string]bool { + out := map[string]bool{} + for _, a := range r.Absent { + if a.Consumer == module { + out[a.Requirement] = true + } + } + return out +} + +// absentOf is, for each requirement left absent, every module of the set that requires it +// optionally, sorted by consumer then requirement. +func absentOf(names map[string]bool, order []string, catalogue map[string]Manifest) []Absent { + var out []Absent + for _, module := range order { + m := catalogue[module] + for _, r := range m.Requires { + if names[r] && m.IsOptional(r) { + out = append(out, Absent{Consumer: module, Requirement: r}) + } + } + } + sort.Slice(out, func(i, j int) bool { + if out[i].Consumer != out[j].Consumer { + return out[i].Consumer < out[j].Consumer + } + return out[i].Requirement < out[j].Requirement + }) + return out +} + +// withoutAbsent drops the resources delivered only while a requirement is bound whose requirement is +// absent here (ADR 0330 §5), and the field itself from every resource that keeps it. +func withoutAbsent(resources []map[string]any, absent map[string]bool) []map[string]any { + out := make([]map[string]any, 0, len(resources)) + for _, r := range resources { + when, has := r[OnlyWhenBound] + if !has { + out = append(out, r) + continue + } + if absent[fmt.Sprint(when)] { + continue + } + kept := make(map[string]any, len(r)) + for k, v := range r { + if k != OnlyWhenBound { + kept[k] = v + } + } + out = append(out, kept) + } + return out +} + +// secretNamesOf is every `${secret:…}` name that carries a credential of this requirement: its own +// name, and each local name it keeps several under (ADR 0094). +func secretNamesOf(m Manifest, requirement string) []string { + names := []string{requirement} + for local := range m.SecretsMany[requirement] { + names = append(names, local) + } + sort.Strings(names) + return names +} + +// OptionalProblems is what the module check refuses of a manifest's optional requirements and its +// only-when-bound resources (ADR 0330 §4–§6). +func OptionalProblems(m Manifest) []string { + var problems []string + required := map[string]bool{} + for _, r := range m.Requires { + if required[r] && m.IsOptional(r) { + problems = append(problems, fmt.Sprintf("%s requires %q twice; say it once, optional or not", m.Module, r)) + } + required[r] = true + } + // A contribution is to whoever answers the requirement, so it goes with it: contributing to one + // that is optional is allowed, and composed only while it is bound. + for _, r := range m.Resources { + id := fmt.Sprint(r["id"]) + when, marked := r[OnlyWhenBound] + on := "" + if marked { + name, ok := when.(string) + switch { + case !ok || name == "": + problems = append(problems, fmt.Sprintf( + "%s's resource %s says %s %v; it names one requirement the module requires optionally", + m.Module, id, OnlyWhenBound, when)) + continue + case fmt.Sprint(r["type"]) != "file": + problems = append(problems, fmt.Sprintf( + "%s's resource %s is a %v and says %s: only a file is left out while a requirement is "+ + "absent (novox/hq ADR 0330 §5)", m.Module, id, r["type"], OnlyWhenBound)) + continue + case !required[name]: + problems = append(problems, fmt.Sprintf( + "%s's resource %s is delivered only when %q is bound, and %s does not require it", + m.Module, id, name, m.Module)) + continue + case !m.IsOptional(name): + problems = append(problems, fmt.Sprintf( + "%s's resource %s is delivered only when %q is bound, which is a hard requirement and "+ + "always bound: mark the requirement optional, or drop %s", m.Module, id, name, OnlyWhenBound)) + continue + } + on = name + } + if fmt.Sprint(r["type"]) != "file" { + continue + } + content, _ := r["content"].(string) + for _, use := range bound.FindAllStringSubmatch(content, -1) { + provision, key, hasDefault := use[1], use[2], use[3] != "" + if !m.IsOptional(provision) { + if hasDefault { + problems = append(problems, fmt.Sprintf( + "%s's resource %s gives ${bound:%s:%s} a default, and %q is a hard requirement, always "+ + "bound: a default it never renders only misleads a reader", m.Module, id, provision, key, provision)) + } + continue + } + if !hasDefault && on != provision { + problems = append(problems, fmt.Sprintf( + "%s's resource %s says ${bound:%s:%s}, and %q is optional: while it is absent this would "+ + "be empty. Name its value then — ${bound:%s:%s|default:} — or deliver the file "+ + "only while it is bound, with \"%s\": %q (novox/hq ADR 0330 §4)", + m.Module, id, provision, key, provision, provision, key, OnlyWhenBound, provision)) + } + } + for _, provision := range m.Requires { + if !m.IsOptional(provision) || on == provision { + continue + } + for _, name := range secretNamesOf(m, provision) { + if strings.Contains(content, "${secret:"+name+"}") { + problems = append(problems, fmt.Sprintf( + "%s's resource %s says ${secret:%s}, the credential of %q, which is optional and not "+ + "delivered while absent: deliver the file only while it is bound, with \"%s\": %q "+ + "(novox/hq ADR 0330 §6)", m.Module, id, name, provision, OnlyWhenBound, provision)) + } + } + } + } + return problems +} diff --git a/internal/catalogue/optional_requirements_test.go b/internal/catalogue/optional_requirements_test.go new file mode 100644 index 00000000..64887ffe --- /dev/null +++ b/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) + } + } +} diff --git a/internal/catalogue/resolve.go b/internal/catalogue/resolve.go index cdd5e208..24d60f7a 100644 --- a/internal/catalogue/resolve.go +++ b/internal/catalogue/resolve.go @@ -183,6 +183,10 @@ type Resolution struct { // that keeps its data, and did not (novox/hq ADR 0232): it stays bound where its data is, and the // controller raises an urgent condition naming the move until a person pins one or the other. Kept []KeptBinding + // Absent is every optional requirement of this node's modules that nothing assigned answers + // (novox/hq ADR 0330 §3): left unbound rather than refused, with nothing installed for it, and + // listed by `plan` and `node` (§7). It binds on the first composition after a provider is assigned. + Absent []Absent } // Unhostable is one directly-assigned module the machine cannot run. @@ -374,12 +378,70 @@ func Resolve(catalogue map[string]Manifest, assigned []string, node Node, world // identical lines before realising there is none. reported := map[string]bool{} - for len(queue) > 0 { + // **An optional requirement is settled last** (novox/hq ADR 0330). Whether a name is optional is + // a question about every module that wants it, and a module pulled in later may want it hard; so + // a name only optional modules want so far is put aside until the walk has nothing else, and only + // then, if still nobody wants it hard, settled by the optional rule: bound to a provider that is + // already assigned or reachable, exactly as a hard one, and otherwise absent — never refused, + // and never answered by installing anything. + var deferred []string + settling := map[string]bool{} + absent := map[string]bool{} + wantedHard := func(want string) bool { + for _, name := range order { + if catalogue[name].wantsHard(want) { + return true + } + } + return false + } + wantedOptionally := func(want string) bool { + for _, name := range order { + if catalogue[name].IsOptional(want) { + return true + } + } + return false + } + leaveAbsent := func(want string) { + absent[want] = true + reported[want] = true + } + + for { + if len(queue) == 0 { + if len(deferred) == 0 { + break + } + // What a module chosen since wants hard goes back to the walk, which may install for + // it; only when none does is the rest settled, since settling installs nothing and so + // can change no other name's answer. + var hard, optional []string + for _, d := range deferred { + if wantedHard(d) { + hard = append(hard, d) + } else { + optional = append(optional, d) + } + } + if len(hard) > 0 { + queue, deferred = hard, optional + continue + } + for _, d := range optional { + settling[d] = true + } + queue, deferred = optional, nil + } want := queue[0] queue = queue[1:] if chosen[want] || reported[want] { continue } + if !settling[want] && because[want] != "assigned" && !wantedHard(want) && wantedOptionally(want) { + deferred = append(deferred, want) + continue + } // Already answered by something in the set. This is the case that makes assigning zsh do // what a person meant by it. // @@ -483,6 +545,12 @@ func Resolve(catalogue map[string]Manifest, assigned []string, node Node, world if brokered[want] { reported[want] = true where := world.Offered[want] + if settling[want] && len(where) == 0 { + // Nothing assigned anywhere answers it: absent, and nothing is put on a machine + // for it (ADR 0330 §3), however few modules in the catalogue could. + leaveAbsent(want) + continue + } names := providerNames(where) take := func(p Provider) { if node.At != "" && p.At == "" || node.At == "" && p.At != "" || node.At == "" && p.At == "" { @@ -579,6 +647,10 @@ func Resolve(catalogue map[string]Manifest, assigned []string, node Node, world // research 026 found. Another node's provider never counts — node scope is never brokered. if machineReach[want] && !isModule(catalogue, want) { reported[want] = true + if settling[want] { + leaveAbsent(want) + continue + } if world.Unchecked { // The first pass's refusals take a machine off the network; the second says it. continue @@ -591,6 +663,13 @@ func Resolve(catalogue map[string]Manifest, assigned []string, node Node, world } candidates := offers[want] + if settling[want] && !(len(candidates) == 0 && len(world.Licences[want]) > 0) { + // Not answered by anything in this node's set, and only installing could answer it + // here — which is never done for an optional requirement (ADR 0330 §3), not even + // for the only candidate. A licence record still answers it, in the post-pass. + leaveAbsent(want) + continue + } switch len(candidates) { case 0: if len(world.Licences[want]) > 0 { @@ -724,7 +803,8 @@ func Resolve(catalogue map[string]Manifest, assigned []string, node Node, world resolution := Resolution{Node: node.Name, At: node.At, PublicDomain: node.PublicDomain, Account: node.Account, AccountHome: node.AccountHome, AgentAccount: node.AgentAccount, AgentAccountHome: node.AgentAccountHome, Capabilities: node.Capabilities, - Because: because, Needs: needs, Unhostable: unhostable, Kept: kept} + Because: because, Needs: needs, Unhostable: unhostable, Kept: kept, + Absent: absentOf(absent, order, catalogue)} for _, n := range providersFirst(order, catalogue) { resolution.Modules = append(resolution.Modules, catalogue[n]) } diff --git a/testdata/store-tests b/testdata/store-tests index da888096..b20acfa1 100644 --- a/testdata/store-tests +++ b/testdata/store-tests @@ -38,3 +38,4 @@ ./cmd/mesh-controller TestReplay201ThePlansSend ./internal/inventory TestAWalksRestIsReadFromEachMachinesOwnSend ./internal/inventory TestAStoredBuildWithASilentMergeableFileIsStillReadAndComposed +./cmd/mesh-controller TestPlanAndNodeListAnOptionalRequirementLeftUnbound