diff --git a/internal/catalogue/co_located_test.go b/internal/catalogue/co_located_test.go new file mode 100644 index 0000000..1417645 --- /dev/null +++ b/internal/catalogue/co_located_test.go @@ -0,0 +1,272 @@ +package catalogue + +import ( + "encoding/json" + "strings" + "testing" +) + +// A provider and its consumer on ONE machine, which is what ADR 0056's anchor is. +// +// **Every fault in this file is the same shape: the same-node path diverging from the cross-node +// one.** A provider on another machine is walked by the control plane — its served facts are +// re-derived from its manifest with that machine's port assignments and settled with that node's +// settings layers — and only then handed to the consumer. A provider on the consumer's OWN machine +// never passes through that walk, so every step of it had to be repeated here, and each step that +// was not is a promise the co-located arrangement quietly breaks. +// +// 04-ISSUES/038 was the first of them (the port). These are the rest. + +// A root certificate, in the shape a certificate authority serves one. +const servedRoot = `-----BEGIN CERTIFICATE----- +MIIBeDCCAR2gAwIBAgIQfake0000000000000000000000 +-----END CERTIFICATE----- +` + +// stepCA is an internal ACME authority: it answers `acme-ca` from anywhere in the mesh, and what a +// consumer must know is the directory URL and the root to trust. **The root is not knowable when +// the module is written** — it exists only once the CA has been initialised — so the manifest +// declares the key and the operator supplies the value as a setting, which is exactly the shape the +// cross-node path settles and the same-node path did not. +func stepCA() Manifest { + return Manifest{ + Module: "step-ca", Version: "1", + Provides: FromAnywhere("acme-ca"), + Listens: []Listening{{Port: 9000, Protocol: "tcp", From: FromMesh}}, + Serves: map[string]map[string]any{"acme-ca": { + "directory": "https://anchor.internal/acme/acme/directory", + // Declared empty: a manifest is the same on every mesh, and this mesh's root is not. + "root": "", + }}, + Resources: []map[string]any{{ + "id": "server", "type": "container", "name": "step-ca", "ports": []any{"9000"}, + }}, + } +} + +// routeProxy issues from that authority, so it must be told the root to verify it with. +func routeProxy() Manifest { + return Manifest{ + Module: "route-proxy", Version: "1", + Requires: []string{"acme-ca"}, + Provides: FromAnywhere("route"), + Binds: map[string]string{"acme-ca": "/var/lib/route-proxy/acme-ca.json"}, + Receives: map[string]string{"route": "/var/lib/route-proxy/routes.json"}, + Resources: []map[string]any{{ + "id": "bundle", "type": "file", "path": "/var/lib/route-proxy/ca.pem", "mode": "0644", + "content": "${bound:acme-ca:root}", + }}, + } +} + +// A co-located provider's served VALUES reach its consumer, not just its served keys. +// +// The mesh walks a provider on ANOTHER machine and settles what it serves with that node's settings +// layers before offering it (cmd/mesh-control plan.go, theRestOfTheMesh). A provider on the +// consumer's own machine was never settled at all: resolve.go's servedHere and declaration.go's +// here() both read the manifest and stop there. So a served value the operator supplied — the one +// kind of value a manifest cannot carry, because it is different on every mesh — arrived as the +// manifest's empty default. +// +// The consequence is silent and total: route-proxy wrote an empty CA bundle, fell back to the +// system trust store, could not verify the internal authority, and no certificate was ever issued. +func TestACoLocatedProvidersServedValuesReachItsConsumer(t *testing.T) { + r, err := Resolve(shelf(stepCA(), routeProxy()), + []string{"step-ca", "route-proxy"}, reachable(), World{}) + if err != nil { + t.Fatal(err) + } + + // What the operator set on the provider, on this node — the CA's root, which only exists once + // the CA has been initialised. + out, err := r.Declaration(Rendering{Settings: SettingsBy{ + "step-ca": {{From: "the operator", Values: map[string]any{"root": servedRoot}}}, + }}) + if err != nil { + t.Fatal(err) + } + + bundle := fileNamed(out, "route-proxy.bundle") + if bundle == nil { + t.Fatalf("the consumer was given no bundle at all: %v", out) + } + if got := bundle["content"]; got != servedRoot { + t.Errorf("the co-located consumer was not told the root it must trust:\n got %q\nwant %q", + got, servedRoot) + } +} + +// And the binding file says the same thing, for a consumer that reads the binding rather than a +// substituted placeholder. The two are one fact and must not be able to disagree. +func TestACoLocatedConsumersBindingCarriesTheSettledValues(t *testing.T) { + r, err := Resolve(shelf(stepCA(), routeProxy()), + []string{"step-ca", "route-proxy"}, reachable(), World{}) + if err != nil { + t.Fatal(err) + } + out, err := r.Declaration(Rendering{ + Settings: SettingsBy{ + "step-ca": {{From: "the operator", Values: map[string]any{"root": servedRoot}}}, + }, + // And the machine moved the provider's port while it was at it, so both halves of the + // cross-node derivation are exercised at once. + Ports: map[string]map[int]int{"step-ca": {9000: 19000}}, + }) + if err != nil { + t.Fatal(err) + } + + binding := fileNamed(out, "route-proxy.bound-acme-ca") + if binding == nil { + t.Fatalf("the consumer was given no binding at all: %v", out) + } + var said struct { + Serves map[string]any `json:"serves"` + } + if err := json.Unmarshal([]byte(binding["content"].(string)), &said); err != nil { + t.Fatal(err) + } + if said.Serves["root"] != servedRoot { + t.Errorf("the binding does not carry the settled root: %q", said.Serves["root"]) + } + if port, _ := asPort(said.Serves["port"]); port != 19000 { + t.Errorf("the binding does not carry the port the machine publishes: %v", said.Serves["port"]) + } +} + +// A provider PULLED IN rather than assigned is settled the same way. +// +// The resolver's same-node need is built during the walk, from whichever modules had been chosen by +// the time the requirement came up — so which path a co-located binding took depended on the order +// a person happened to assign things in. Settling after the closure is known removes that: both +// orders now produce the same file. +func TestACoLocatedProviderIsSettledWhicheverWayItWasPulledIn(t *testing.T) { + for _, assigned := range [][]string{ + {"step-ca", "route-proxy"}, + {"route-proxy", "step-ca"}, + } { + r, err := Resolve(shelf(stepCA(), routeProxy()), assigned, reachable(), World{}) + if err != nil { + t.Fatal(err) + } + out, err := r.Declaration(Rendering{Settings: SettingsBy{ + "step-ca": {{From: "the operator", Values: map[string]any{"root": servedRoot}}}, + }}) + if err != nil { + t.Fatal(err) + } + bundle := fileNamed(out, "route-proxy.bundle") + if bundle == nil || bundle["content"] != servedRoot { + t.Errorf("assigned %v: the consumer was not told the root", assigned) + } + } +} + +// A same-node contribution names the port the MACHINE publishes, not the one the module declared. +// +// 04-ISSUES/038 fixed this for what a consumer is TOLD about a provider. This is the other +// direction: what a workload TELLS the provider about itself. gitea declares a bare container port +// 3000, the mesh publishes it as 20000:3000 — and gitea's route contribution still said 3000, so +// the proxy beside it dialled a port nothing listens on and answered 502 for every request. +// +// The redirect uses the CONTRIBUTING module's assignment: the port belongs to the workload, and +// using the provider's map would move it to wherever the proxy happens to be published. +func TestASameNodeContributionNamesThePortTheMachinePublishes(t *testing.T) { + gitea := Manifest{ + Module: "gitea", Version: "1", + Listens: []Listening{{Port: 3000, Protocol: "tcp", From: FromMesh}}, + Contributes: map[string]map[string]any{"route": {"label": "git", "port": 3000}}, + Resources: []map[string]any{{ + "id": "server", "type": "container", "name": "gitea", "ports": []any{"3000"}, + }}, + } + r, err := Resolve(shelf(gitea, routeProxy(), stepCA()), + []string{"gitea", "route-proxy", "step-ca"}, reachable(), World{}) + if err != nil { + t.Fatal(err) + } + out, err := r.Declaration(Rendering{ + // The machine put gitea's 3000 on 20000, and published it that way. + Ports: map[string]map[int]int{"gitea": {3000: 20000}}, + }) + if err != nil { + t.Fatal(err) + } + + // Where the workload is actually published. + server := fileNamed(out, "gitea.server") + if published := strings.Join(asStrings(server["ports"]), ","); published != "20000:3000" { + t.Fatalf("the workload was not published on the assigned port: %v", server["ports"]) + } + + // What the proxy beside it was told to dial: the SAME port. + routes := fileNamed(out, "route-proxy.received-route") + if routes == nil { + t.Fatalf("the proxy was given no routes file at all: %v", out) + } + var given struct { + Given []Contribution `json:"given"` + } + if err := json.Unmarshal([]byte(routes["content"].(string)), &given); err != nil { + t.Fatal(err) + } + if len(given.Given) != 1 { + t.Fatalf("expected one route, got %v", given.Given) + } + port, ok := asPort(given.Given[0].Values["port"]) + if !ok || port != 20000 { + t.Errorf("the proxy was told to dial a port nothing listens on: %v", + given.Given[0].Values["port"]) + } +} + +// A contribution from ANOTHER machine is left exactly as it was. +// +// Its port belongs to that machine's assignment, which this node's map knows nothing about — +// applying this node's map to it would move a remote workload's port to wherever a local module of +// the same name happens to be published, which is worse than the fault being fixed. +func TestAContributionFromAnotherMachineKeepsItsOwnPort(t *testing.T) { + r, err := Resolve(shelf(routeProxy(), stepCA()), + []string{"route-proxy", "step-ca"}, reachable(), World{}) + if err != nil { + t.Fatal(err) + } + out, err := r.Declaration(Rendering{ + // A same-named module on this machine, moved. The grant below is a laptop's, and must not + // be dragged along with it. + Ports: map[string]map[int]int{"gitea": {3000: 20000}}, + Grants: []Grant{{ + Provision: "route", Consumer: "laptop", From: "gitea", At: "laptop.internal", + Values: map[string]any{"name": "git.example", "port": 3000}, + }}, + }) + if err != nil { + t.Fatal(err) + } + routes := fileNamed(out, "route-proxy.received-route") + var given struct { + Given []Contribution `json:"given"` + } + if err := json.Unmarshal([]byte(routes["content"].(string)), &given); err != nil { + t.Fatal(err) + } + if len(given.Given) != 1 { + t.Fatalf("expected one route, got %v", given.Given) + } + if port, _ := asPort(given.Given[0].Values["port"]); port != 3000 { + t.Errorf("another machine's contribution was rewritten with this machine's ports: %v", + given.Given[0].Values["port"]) + } +} + +func asStrings(v any) []string { + listed, ok := v.([]any) + if !ok { + return nil + } + out := make([]string, 0, len(listed)) + for _, one := range listed { + out = append(out, strings.TrimSpace(plainly(one))) + } + return out +} diff --git a/internal/catalogue/declaration.go b/internal/catalogue/declaration.go index ab5ab85..07f4a13 100644 --- a/internal/catalogue/declaration.go +++ b/internal/catalogue/declaration.go @@ -141,18 +141,52 @@ func (r Resolution) Declaration(with Rendering) ([]map[string]any, error) { return nil, err } - // A provider answered on this same machine is announced at the port it *declared*, because the - // need was settled while resolving — before this machine assigned the port a bare `ports` - // mapping is published on (novox/hq 04-ISSUES/038). Redirect those same-node needs to where the - // machine actually publishes the provider, exactly as the cross-node path already does with the - // provider's own assignment, so a co-located consumer is told the port that is really listening. - servedBy := servedByModule(r.Modules) + // A workload on THIS machine contributes the port it declared, and the machine may have + // published it somewhere else (novox/hq ADR 0056). The mirror of the redirect below: 038 fixed + // what a consumer is TOLD about a provider, and this is what a workload TELLS the provider about + // itself — gitea declaring 3000, published as 20000:3000, and the co-located proxy dialling 3000, + // where nothing listens, for every request. + // + // **The contributing module's assignment, never the provider's.** The port belongs to the + // workload; using the map of whatever answers the requirement would move a route to wherever the + // proxy happens to be published. A contribution carried here from another machine is left exactly + // as it is — its port is that machine's to assign, and this map knows nothing about it. + for provision := range given { + for i := range given[provision] { + said := &given[provision][i] + if said.Node != "" && said.Node != r.Node { + continue + } + said.Values = atMachinePort(said.Values, said.From, with.Ports) + } + } + + // A provider answered on this same machine never passed through the walk that works out what a + // provider on ANOTHER machine serves (novox/hq 04-ISSUES/038, ADR 0056). That walk does two + // things — it derives the served facts with the provider node's port assignments, and it settles + // them with that node's settings layers — and the same-node paths (resolve.go's servedHere, and + // here() below) did neither, because both run while resolving, before either is known. + // + // The port was the first half to be noticed (038). The second half is worse and quieter: a served + // value the OPERATOR supplied — a certificate authority's root, which is the one kind of value a + // manifest cannot carry, because it differs on every mesh — arrived as the manifest's empty + // default. A proxy then wrote an empty CA bundle, fell back to the system trust store, could not + // verify the internal authority, and stopped issuing with nothing saying why. + // + // So it is re-derived here, in full, exactly as the cross-node path derives it. **After the + // closure is known**, which also removes an order dependence: the resolver built these needs + // mid-walk from whichever modules had been chosen by then, so what a co-located binding carried + // depended on the order somebody happened to assign things in. for i := range r.Needs { - if r.Needs[i].From != r.Node { + if r.Needs[i].ByRecord || r.Needs[i].From != r.Node { continue } - if module, ok := servedBy[r.Needs[i].Name]; ok { - r.Needs[i].Serves = atMachinePort(r.Needs[i].Serves, module, with.Ports) + serves, answered, err := r.servedOnThisMachine(r.Needs[i].Name, with) + if err != nil { + return nil, err + } + if answered { + r.Needs[i].Serves = serves } } @@ -319,18 +353,16 @@ func (r Resolution) Declaration(with Rendering) ([]map[string]any, error) { // must know, which is the port; a consumer cannot invent that, and got an absent // file with no explanation. A build machine sharing a node with the registry it // pushes to sat in a loop saying it could not read its own binding. - here := here(r, to) + here, err := here(r, to, with) + if err != nil { + return nil, err + } if here == nil { // Nothing in this node's set offers it either, so there is genuinely nothing // to say. Resolution has already refused anything unanswerable, so this is a // requirement met by the module itself. continue } - // Answered here, so its served port needs the same redirect the resolver's - // same-node needs got above — this fallback builds one afresh, outside r.Needs. - if provider, ok := servedBy[to]; ok { - here.Serves = atMachinePort(here.Serves, provider, with.Ports) - } found = here } file, err := boundFile(*found, m.Binds[to], ConsumerIdentity(r.Node, IdentitySource(m.Slug, m.Module))) @@ -733,20 +765,51 @@ func (r Resolution) ContributionsFrom(requirement, module string, settings Setti // The address is this machine's own name on the private network when it has one, and loopback // when it does not — a machine not on the network still reaches itself, and naming it by a name // nothing resolves would be worse than naming it by an address that always works. -func here(r Resolution, requirement string) *Needed { +func here(r Resolution, requirement string, with Rendering) (*Needed, error) { + serves, answered, err := r.servedOnThisMachine(requirement, with) + if err != nil || !answered { + return nil, err + } + at := r.At + if at == "" { + at = "127.0.0.1" + } + return &Needed{Name: requirement, From: r.Node, At: at, Serves: serves}, nil +} + +// servedOnThisMachine is what a provider in this node's own set says a consumer must know — +// derived exactly as the mesh derives it for a provider on any OTHER machine. +// +// **The one derivation, so the two arrangements cannot disagree.** For a provider elsewhere the +// control plane walks that node, reads its manifest with that machine's port assignments, and +// settles the result with that node's settings layers before offering it (cmd/mesh-control plan.go, +// theRestOfTheMesh). A provider on the consumer's own machine never passes through that walk, so +// every step of it has to be repeated here — and each step that was not repeated was a promise the +// co-located arrangement quietly broke: first the port (novox/hq 04-ISSUES/038), then the settled +// values (ADR 0056, an internal CA's root arriving empty). +// +// Here rather than in the resolver, because here is the first moment both halves exist: resolving +// runs before a machine's ports are assigned and knows nothing of settings. +// +// The first module in the resolved order that says it serves the provision answers, which is the +// same choice here() has always made and is stable — providersFirst orders the slice, and nothing +// downstream depends on a map's iteration. +func (r Resolution) servedOnThisMachine(provision string, with Rendering) (map[string]any, bool, error) { for _, m := range r.Modules { - _, said := m.Serves[requirement] - serves := ServedOn(m, requirement, nil) - if !said || len(serves) == 0 { + if _, said := m.Serves[provision]; !said { continue } - at := r.At - if at == "" { - at = "127.0.0.1" + serves := ServedOn(m, provision, with.Ports[m.Module]) + if len(serves) == 0 { + return nil, false, nil } - return &Needed{Name: requirement, From: r.Node, At: at, Serves: serves} + settled, err := Settle(serves, with.Settings[m.Module]) + if err != nil { + return nil, false, fmt.Errorf("%s serving %s: %w", m.Module, provision, err) + } + return settled, true, nil } - return nil + return nil, false, nil } // withMeshNames gives every container in a set the mesh's names. @@ -899,35 +962,22 @@ func asPort(v any) (int, bool) { return 0, false } -// servedByModule maps each provision offered on this node to the module that offers it. +// atMachinePort redirects a `port` written by a module on this machine to where this machine +// actually published it (novox/hq 04-ISSUES/038, ADR 0056). // -// The same node may both provide and consume a provision, and a consumer's binding names the -// provision, not the module behind it — so redirecting a served port to where the machine put it -// needs this reverse lookup, from the provision back to the module whose assignment moved it. -func servedByModule(mods []Manifest) map[string]string { - out := map[string]string{} - for _, m := range mods { - for provision := range m.Serves { - out[provision] = m.Module - } - } - return out -} - -// atMachinePort redirects a served `port` fact to where this machine actually published the -// provider (novox/hq 04-ISSUES/038). +// **A module writes the port its software uses; only the mesh knows where the machine put it.** A +// bare `ports` mapping is assigned a host port when the declaration is composed, which is after +// everything a module wrote has been read — so any number carried out of a manifest is the declared +// one until it passes through here. Both directions need it: what a co-located consumer is TOLD +// about a provider, and what a co-located workload TELLS a provider about itself. // -// **Same-node served facts are settled before the machine's port is assigned.** The resolver names -// what a consumer must know (resolve.go's servedHere, declaration.go's here()) while resolving, -// which happens before a bare `ports` declaration is assigned a host port — so it can only carry -// the port the provider *declared*. The cross-node path re-derives the fact after assignment, with -// the provider node's port map (cmd plan.go), and so already tells the truth. This closes the gap -// for the co-located case: without it a consumer is announced `.internal:` while -// the provider is published on `.internal:`, and dials a port nothing listens on. +// Named by the module the port belongs to, which is not always the one being talked about: a route +// contribution's port is the contributing workload's, never the proxy's. // -// A fresh map when it changes anything, so the manifest-derived value the resolver handed back is -// never mutated under a caller that still holds it. Keyed by the *declared* port, so applying it a -// second time is a no-op: once redirected the value is the machine port, which is not itself a key. +// A fresh map when it changes anything, so the manifest-derived value handed back by the resolver — +// or by a grant gathered elsewhere — is never mutated under a caller that still holds it. Keyed by +// the *declared* port, so applying it a second time is a no-op: once redirected the value is the +// machine port, which is not itself a key. func atMachinePort(serves map[string]any, module string, ports map[string]map[int]int) map[string]any { number, ok := asPort(serves["port"]) if !ok { diff --git a/internal/catalogue/resolve.go b/internal/catalogue/resolve.go index f2be1ce..fd6ae42 100644 --- a/internal/catalogue/resolve.go +++ b/internal/catalogue/resolve.go @@ -531,18 +531,26 @@ func Resolve(catalogue map[string]Manifest, assigned []string, node Node, world return resolution, nil } -// servedHere is what a provider in this node's own set says a consumer needs to know. +// servedHere is *whether* a provider in this node's own set says a consumer needs to know +// anything, and provisionally what. // -// The same facts a provider elsewhere would have contributed through the world, taken from the -// module directly because a provider on this machine never passes through it. +// **Provisional, and deliberately so.** Resolving runs before a machine's ports are assigned and +// before any settings are in view, so nothing said here can be the final answer: it is the module's +// manifest and nothing else. What reaches the consumer is re-derived from the resolved closure by +// Declaration (servedOnThisMachine), with this machine's port assignments and the provider's +// settings layers — the same two steps the mesh applies to a provider on any other machine. +// +// It still matters that this is right about *emptiness*: a keyless same-node provider is only +// delivered as a need at all when it serves something (novox/hq 04-ISSUES/038's sibling), and a +// need that is never created is a binding the consumer never gets. It is right about that from the +// manifest alone, which is why walking the catalogue mid-resolution is enough here and is not +// enough for the values. func servedHere(catalogue map[string]Manifest, chosen map[string]bool, want string) map[string]any { for name, m := range catalogue { if !chosen[name] { continue } if _, ok := m.Serves[want]; ok { - // Without assignments: this is resolution, which runs before a machine's ports are - // known. The declaration fills the machine's own in afterwards, where it has them. return ServedOn(m, want, nil) } }