diff --git a/cmd/mesh-controller/plan.go b/cmd/mesh-controller/plan.go index d066c27..23a97fa 100644 --- a/cmd/mesh-controller/plan.go +++ b/cmd/mesh-controller/plan.go @@ -509,6 +509,12 @@ func renderingFor(ctx context.Context, open *stores, node string, // `.internal` names above, so a container — or an internal ACME validator — resolves a // routed name to the proxy that serves it, mesh-wide. The mesh publishes the names it was told // to serve and knows nothing about what they mean. + // Kept apart from the machines, because a fact about the machines must not be handed the names + // the mesh merely serves (novox/hq 04-ISSUES/111). + machines := make(map[string]string, len(names)) + for name, at := range names { + machines[name] = at + } routes, err := routeNamesInTheMesh(ctx, open) if err != nil { return catalogue.Rendering{}, inventory.Node{}, err @@ -574,7 +580,8 @@ func renderingFor(ctx context.Context, open *stores, node string, return catalogue.Rendering{ Settings: settings, Generators: gens, Grants: grants, Needed: needed, Ports: ports, Certificate: certificate, Authority: authority, Mesh: private, Names: names, - Suffix: overlay.Suffix(), Foundation: foundation, Kept: kept, Adopted: record.Adopted, + Machines: machines, + Suffix: overlay.Suffix(), Foundation: foundation, Kept: kept, Adopted: record.Adopted, Given: given, Taken: taken, Seats: seats, ArtifactStore: artifactStore, Built: built, }, record, nil } diff --git a/internal/catalogue/declaration.go b/internal/catalogue/declaration.go index 07ac8b6..fefd2ff 100644 --- a/internal/catalogue/declaration.go +++ b/internal/catalogue/declaration.go @@ -114,6 +114,14 @@ type Rendering struct { // because which machines exist is a fact about the mesh. Names map[string]string + // Machines is only the machines, by the same internal name — the subset of Names that is a + // node of this mesh rather than a name it was told to serve. Both matter and they are not the + // same set: a container's hosts wants every name, so a routed name resolves to the proxy that + // serves it, while a resolver told the mesh's suffix is authoritative for it answers from what + // it is given and forwards nothing — so a routed name written there is a name nobody asks for, + // standing beside the machines and looking as real as they do. + Machines map[string]string + Settings SettingsBy Generators map[string]Generator // Grants are the credentials this node must create, for the provisions it offers. Passed in @@ -604,7 +612,7 @@ func (r Resolution) compose(with Rendering, owner map[string]string) ([]map[stri // plane's; making a name resolve is the module's software. Emitted as ordinary files under // this module's name, so they are applied, reported and removed exactly as anything else // it declares. - given, err := FactsInto(m, r, with.Names, with.Suffix) + given, err := FactsInto(m, r, with.Names, with.Machines, with.Suffix) if err != nil { return nil, err } diff --git a/internal/catalogue/facts.go b/internal/catalogue/facts.go index c9c38df..2fdd510 100644 --- a/internal/catalogue/facts.go +++ b/internal/catalogue/facts.go @@ -36,16 +36,23 @@ const ( // **A closed list.** A module asking for a fact the mesh does not have is asking for a file nobody // will write, and finding that out on a machine — as a daemon that starts, reads nothing, and // answers no queries — is worse than being told where the manifest is. -var facts = map[string]func(Resolution, map[string]string, string) string{ - FactNodeNames: nodeNames, - FactNodeZones: nodeZones, +// A fact is written from the names it is about. `every` is every name the mesh serves — machines +// and the names it was told to route; `machines` is only the machines. A fact takes the set it is +// true of, and the two must not be confused (novox/hq 04-ISSUES/111). +var facts = map[string]func(r Resolution, every, machines map[string]string, suffix string) string{ + FactNodeNames: func(r Resolution, every, _ map[string]string, suffix string) string { + return nodeNames(r, every, suffix) + }, + FactNodeZones: func(r Resolution, _, machines map[string]string, suffix string) string { + return nodeZones(r, machines, suffix) + }, } // FactsInto renders the facts a module asked for, as files it will be given. // // The module owns everything after the file exists: loading it, restarting on it, what a resolver // does with it. This only puts it there. -func FactsInto(m Manifest, r Resolution, addresses map[string]string, suffix string) ([]map[string]any, error) { +func FactsInto(m Manifest, r Resolution, addresses, machines map[string]string, suffix string) ([]map[string]any, error) { if len(m.Facts) == 0 { return nil, nil } @@ -70,7 +77,7 @@ func FactsInto(m Manifest, r Resolution, addresses map[string]string, suffix str } out = append(out, map[string]any{ "id": "fact-" + name, "type": "file", "path": path, "mode": "0644", - "content": write(r, addresses, suffix), + "content": write(r, addresses, machines, suffix), }) } return out, nil diff --git a/internal/catalogue/facts_test.go b/internal/catalogue/facts_test.go index 32e578d..1f30b69 100644 --- a/internal/catalogue/facts_test.go +++ b/internal/catalogue/facts_test.go @@ -63,7 +63,7 @@ func TestAMachinesOwnNameIsItsMeshAddress(t *testing.T) { // A module says where it wants a fact, and is given a file. func TestAModuleIsGivenTheFactsItAskedFor(t *testing.T) { m := Manifest{Module: "dnsmasq", Facts: map[string]string{FactNodeZones: "/etc/mesh/zones.conf"}} - given, err := FactsInto(m, Resolution{Node: "homer"}, threeMachines, "") + given, err := FactsInto(m, Resolution{Node: "homer"}, threeMachines, threeMachines, "") if err != nil { t.Fatal(err) } @@ -82,7 +82,7 @@ func TestAModuleIsGivenTheFactsItAskedFor(t *testing.T) { // starts, reads a file nobody wrote, and answers no queries is a much worse way to find out. func TestAskingForAFactTheMeshDoesNotHaveIsRefused(t *testing.T) { m := Manifest{Module: "dnsmasq", Facts: map[string]string{"the-weather": "/etc/weather"}} - _, err := FactsInto(m, Resolution{}, nil, "") + _, err := FactsInto(m, Resolution{}, nil, nil, "") if err == nil { t.Fatal("a module asked for something nobody computes and was given nothing, silently") } @@ -96,7 +96,7 @@ func TestAskingForAFactTheMeshDoesNotHaveIsRefused(t *testing.T) { // And a relative path is refused, or a module decides where the mesh writes on a machine. func TestAFactMustBeAskedForAtAnAbsolutePath(t *testing.T) { m := Manifest{Module: "dnsmasq", Facts: map[string]string{FactNodeNames: "etc/hosts"}} - if _, err := FactsInto(m, Resolution{}, nil, ""); err == nil { + if _, err := FactsInto(m, Resolution{}, nil, nil, ""); err == nil { t.Fatal("a relative path was accepted") } } @@ -140,3 +140,49 @@ func TestTheFactsWriteTheSuffixTheNamesWereComposedWith(t *testing.T) { t.Fatalf("the hosts line does not carry the operator's suffix as given:\n%s", hosts) } } + +// novox/hq 04-ISSUES/111: the map the control plane hands a resolution holds every name the mesh +// serves — the machines, and the names it was told to route to whichever machine serves them. A +// container's hosts wants all of it. A resolver's zones want only the machines: told the mesh's +// suffix is its own, it answers authoritatively for everything under it and forwards nothing, so a +// routed name written there with the suffix appended is a name nobody will ever ask for, standing +// beside the machines and looking as real. +func TestTheResolverIsToldTheMachinesAndNotTheNamesTheMeshMerelyServes(t *testing.T) { + machines := map[string]string{"homer.internal": "10.42.0.1", "marge.internal": "10.42.0.2"} + every := map[string]string{ + "homer.internal": "10.42.0.1", "marge.internal": "10.42.0.2", + "drive.example.test": "10.42.0.1", "git.example.test": "10.42.0.2", + } + m := Manifest{Module: "resolver", Facts: map[string]string{ + FactNodeZones: "/etc/zones.conf", FactNodeNames: "/etc/hosts", + }} + given, err := FactsInto(m, Resolution{Node: "homer"}, every, machines, "") + if err != nil { + t.Fatal(err) + } + by := map[string]string{} + for _, f := range given { + by[f["path"].(string)] = f["content"].(string) + } + + zones := by["/etc/zones.conf"] + for _, machine := range []string{"address=/homer.internal/10.42.0.1", "address=/marge.internal/10.42.0.2"} { + if !strings.Contains(zones, machine) { + t.Fatalf("the resolver was not told %q:\n%s", machine, zones) + } + } + for _, served := range []string{"drive.example.test", "git.example.test"} { + if strings.Contains(zones, served) { + t.Fatalf("the resolver was told %q, a name the mesh serves rather than a machine:\n%s", served, zones) + } + } + + // And the hosts file is the other way about: every name, so a container reaching a routed name + // finds the machine serving it. + hosts := by["/etc/hosts"] + for _, name := range []string{"homer.internal", "drive.example.test", "git.example.test"} { + if !strings.Contains(hosts, name) { + t.Fatalf("a container would not resolve %q from its hosts:\n%s", name, hosts) + } + } +}