diff --git a/cmd/mesh-controller/network_test.go b/cmd/mesh-controller/network_test.go index ec42473..0da4c37 100644 --- a/cmd/mesh-controller/network_test.go +++ b/cmd/mesh-controller/network_test.go @@ -2,9 +2,11 @@ package main import ( "context" + "os" "strings" "testing" + "github.com/novox/mesh-controller/internal/catalogue" "github.com/novox/mesh-controller/internal/inventory" "github.com/novox/mesh-controller/internal/overlay" ) @@ -108,3 +110,69 @@ func TestOnlyAMachineOnThePrivateNetworkIsNamed(t *testing.T) { t.Fatalf("a machine that left the network is still named, or the one that stayed is not: %v", names) } } + +// theResolver is the catalogue's dnsmasq module as it is, or the test is skipped where the +// catalogue is not beside this checkout. +func theResolver(t *testing.T) catalogue.Manifest { + t.Helper() + raw, err := os.ReadFile("../../../mesh-catalog/modules/dnsmasq/module.json") + if err != nil { + t.Skipf("the catalogue is not beside this checkout: %v", err) + } + m, err := catalogue.ParseManifest(raw) + if err != nil { + t.Fatalf("dnsmasq does not parse:\n%v", err) + } + return m +} + +// The resolver is handed every machine on the private network as a wildcard, the same set and the +// same source as the hosts file, and is handed it again when a machine leaves — through the +// module's own manifest asking for the fact, with no module of the mesh's own in between (hal +// dnsmasq-app conversion, novox/hq 08-connectivity). The runtime on that machine is pointed at the +// machine's own address, where the resolver answers for its containers. +func TestTheResolverIsToldEveryMachineOnTheNetworkAndToldAgainWhenOneLeaves(t *testing.T) { + open := aMesh(t) + ctx := t.Context() + register(t, open, theResolver(t)) + if _, err := assign(ctx, open, "anchor", "dnsmasq"); err != nil { + t.Fatal(err) + } + zones := func() string { + t.Helper() + for _, r := range composed(t, open, "anchor").Resources { + if r["id"] == "dnsmasq.fact-node-zones" { + if r["path"] != "/etc/mesh-resolver/nodes.conf" { + t.Fatalf("the machines were written somewhere the resolver does not read: %v", r["path"]) + } + return r["content"].(string) + } + } + t.Fatal("the resolver was not handed the machines") + return "" + } + first := zones() + for _, want := range []string{ + "local=/internal/", "address=/anchor.internal/10.77.0.1\n", "address=/laptop.internal/10.77.0.2\n", + } { + if !strings.Contains(first, want) { + t.Errorf("the resolver's machines lack %q:\n%s", want, first) + } + } + for _, r := range composed(t, open, "anchor").Resources { + if r["id"] == "dnsmasq.runtime-dns" { + if !strings.Contains(r["content"].(string), `"10.77.0.1"`) || r["into"] != "json" { + t.Errorf("the runtime is not pointed at this machine's own address, written into its file: %v", r) + } + } + } + + // The laptop keeps its place and its address, and stops running the network. + if err := open.inventory.Unassign(ctx, "laptop", overlay.Name); err != nil { + t.Fatal(err) + } + after := zones() + if strings.Contains(after, "laptop") || !strings.Contains(after, "address=/anchor.internal/10.77.0.1\n") { + t.Fatalf("a machine that left the network is still a wildcard, or the one that stayed is not:\n%s", after) + } +} diff --git a/internal/catalogue/declaration.go b/internal/catalogue/declaration.go index a9b5549..a45fd02 100644 --- a/internal/catalogue/declaration.go +++ b/internal/catalogue/declaration.go @@ -495,7 +495,7 @@ func (r Resolution) compose(with Rendering, owner map[string]string) ([]map[stri // And what its bindings say, for the half of a connection that is not secret. known := knownFor(m, r.Needs, r.Node) // And the machine underneath, which no binding of its own can tell it. - thisMachine := machineFacts(r) + thisMachine := machineFacts(r, with.Names) // Which of this module's files carry a secret, for the rule that a container may not read // one of them as its environment without saying so (ADR 0086, issue 041). diff --git a/internal/catalogue/facts.go b/internal/catalogue/facts.go index baf919f..c9c38df 100644 --- a/internal/catalogue/facts.go +++ b/internal/catalogue/facts.go @@ -122,10 +122,18 @@ func nodeNames(r Resolution, addresses map[string]string, suffix string) string // // `*.homer.internal` is homer, which is the whole rule: if homer is at an address, so is anything // homer serves. A module wanting this runs the resolver; the mesh only says what is true. +// +// **And the suffix itself, as a local domain.** A resolver that forwards what it cannot answer +// would otherwise send a mesh name it does not know — a machine that left, a typo — to a public +// resolver, which is a leak of the mesh's names for no answer. `local=` keeps everything under the +// suffix here: answered from the lines below or refused. Written in this file rather than in the +// resolver's own configuration because the suffix is the mesh's choice (the operator may have +// picked another) and this file is the one place the mesh writes what it chose. func nodeZones(_ Resolution, addresses map[string]string, suffix string) string { var b strings.Builder b.WriteString("# Generated by the mesh. Do not edit — this file is replaced whenever a machine\n") b.WriteString("# joins or leaves, and an edit would survive until then and vanish.\n\n") + fmt.Fprintf(&b, "local=/%s/\n", strings.TrimPrefix(suffixOr(suffix), ".")) for _, name := range sortedNames(addresses) { internal, _ := meshName(name, suffix) fmt.Fprintf(&b, "address=/%s/%s\n", internal, addresses[name]) @@ -139,16 +147,22 @@ func nodeZones(_ Resolution, addresses map[string]string, suffix string) string // suffix is the one the control plane composed those names with, handed down rather than written // here a second time — the alternative was `homer.internal.internal` on every machine. func meshName(name, suffix string) (internal, bare string) { - if suffix == "" { - suffix = "internal" - } - dotted := "." + strings.TrimPrefix(suffix, ".") + dotted := "." + strings.TrimPrefix(suffixOr(suffix), ".") if strings.HasSuffix(name, dotted) { return name, strings.TrimSuffix(name, dotted) } return name + dotted, name } +// suffixOr is the suffix given, or the one the mesh composes names with when none was handed down. +// The one place the default is written in this file, so a fact and a name cannot disagree about it. +func suffixOr(suffix string) string { + if suffix == "" { + return "internal" + } + return suffix +} + func sortedNames(addresses map[string]string) []string { out := make([]string, 0, len(addresses)) for name, at := range addresses { diff --git a/internal/catalogue/facts_test.go b/internal/catalogue/facts_test.go index 2540ab4..32e578d 100644 --- a/internal/catalogue/facts_test.go +++ b/internal/catalogue/facts_test.go @@ -8,10 +8,14 @@ import ( // Keyed by the internal name, as the control plane hands them (issue 079). var threeMachines = map[string]string{"homer.internal": "10.42.0.1", "marge.internal": "10.42.0.2", "bart.internal": ""} -// **`*.homer.internal` is homer. That is the whole rule.** +// **`*.homer.internal` is homer. That is the whole rule.** And the suffix itself is local: a +// resolver that forwards what it cannot answer must not send a mesh name it does not know — a +// machine that left, a typo — to a public resolver (hal dnsmasq-app conversion, novox/hq +// 08-connectivity). func TestEveryMachineIsAWildcardUnderItsOwnName(t *testing.T) { out := nodeZones(Resolution{Node: "homer"}, threeMachines, "") for _, want := range []string{ + "local=/internal/", "address=/homer.internal/10.42.0.1", "address=/marge.internal/10.42.0.2", } { @@ -128,6 +132,9 @@ func TestTheFactsWriteTheSuffixTheNamesWereComposedWith(t *testing.T) { if !strings.Contains(zones, "address=/homer.lan/10.42.0.1") || strings.Contains(zones, "internal") { t.Fatalf("the zones do not carry the operator's suffix as given:\n%s", zones) } + if !strings.Contains(zones, "local=/lan/") { + t.Fatalf("the local domain is not the operator's suffix, so its names would leak upstream:\n%s", zones) + } hosts := nodeNames(Resolution{Node: "homer"}, names, "lan") if !strings.Contains(hosts, "10.42.0.1\thomer.lan\thomer\t# this machine") { t.Fatalf("the hosts line does not carry the operator's suffix as given:\n%s", hosts) diff --git a/internal/catalogue/machine_into_files.go b/internal/catalogue/machine_into_files.go index 046fd3b..cb642bc 100644 --- a/internal/catalogue/machine_into_files.go +++ b/internal/catalogue/machine_into_files.go @@ -22,8 +22,11 @@ import ( // provides, it does not require. Written as a literal it would be a manifest carrying one // deployment's machine name, which is the shape [ADR 0066] exists to remove. // -// Two facts, both the mesh's own vocabulary — the same `node` and `at` a contribution already -// carries. Nothing about what a machine is *for*: that would be the mesh learning what a module +// Three facts, all the mesh's own vocabulary — the same `node` and `at` a contribution already +// carries, and the address behind `at`, for software that takes an address and not a name. The +// case that found the third is a resolver pointing the container runtime at itself: the runtime's +// list of resolvers is addresses, because a name there would have to be resolved by the resolver +// it names. Nothing about what a machine is *for*: that would be the mesh learning what a module // means, which it does not do. // ofMachine is where a module says a fact about the machine underneath it belongs: @@ -49,10 +52,18 @@ func machineUsed(content string) []string { // to be reached at an address that does not exist is a misconfiguration, and it is said here — // where the module and the machine are both named — rather than discovered later as a certificate // nobody can verify. -func machineFacts(r Resolution) map[string]string { +// +// `address` is what `at` resolves to, read from the names the control plane composed — the same map +// the hosts file and the resolver's wildcards are written from, so a file naming the machine's +// address and the file every other machine reaches it by cannot disagree. Absent, like `at`, when +// the machine is off the network or the mesh has not placed it. +func machineFacts(r Resolution, names map[string]string) map[string]string { out := map[string]string{"name": r.Node} if r.At != "" { out["at"] = r.At + if address := names[r.At]; address != "" { + out["address"] = address + } } return out } diff --git a/internal/catalogue/machine_into_files_test.go b/internal/catalogue/machine_into_files_test.go index 9fd1ae7..e59f3a2 100644 --- a/internal/catalogue/machine_into_files_test.go +++ b/internal/catalogue/machine_into_files_test.go @@ -70,3 +70,36 @@ func TestAnAddressAMachineDoesNotHaveIsRefused(t *testing.T) { t.Errorf("the refusal does not name what was asked for: %v", err) } } + +// A module that must give software the machine's ADDRESS rather than its name — a resolver pointing +// the container runtime at itself, whose list of resolvers cannot be a name — says +// ${machine:address}, and gets what the machine's name resolves to on the private network: the same +// address every other machine's hosts file carries for it. +func TestAModuleNamesTheAddressBehindItsMachinesName(t *testing.T) { + pointing := Manifest{Module: "pointing", Version: "1", Resources: []map[string]any{{ + "id": "runtime", "type": "file", "path": "/etc/runtime.json", + "content": `{"dns":["${machine:address}"]}`, + }}} + got, err := Resolve(shelf(pointing), []string{"pointing"}, anchored(), World{}) + if err != nil { + t.Fatal(err) + } + out, err := got.Declaration(Rendering{Names: map[string]string{ + "workstation.internal": "10.42.0.7", "anchor.internal": "10.42.0.1"}}) + if err != nil { + t.Fatal(err) + } + if content := plainly(out[0]["content"]); content != `{"dns":["10.42.0.7"]}` { + t.Fatalf("the machine's address was not the one its name resolves to: %q", content) + } + + // Off the network there is no such address, and the refusal says what the machine does have — + // rather than a placeholder written into the runtime's file and read as an address. + got, err = Resolve(shelf(pointing), []string{"pointing"}, workstation(), World{}) + if err != nil { + t.Fatal(err) + } + if _, err := got.Declaration(Rendering{}); err == nil || !strings.Contains(err.Error(), "${machine:address}") { + t.Fatalf("a machine off the network was given an address, or refused for another reason: %v", err) + } +} diff --git a/internal/catalogue/resolver_manifests_test.go b/internal/catalogue/resolver_manifests_test.go new file mode 100644 index 0000000..9debbc1 --- /dev/null +++ b/internal/catalogue/resolver_manifests_test.go @@ -0,0 +1,188 @@ +package catalogue + +import ( + "encoding/json" + "strings" + "testing" +) + +// The catalogue's resolver modules as they are, parsed by the real parser and composed as a +// machine would receive them (hal dnsmasq-app conversion, novox/hq 08-connectivity). +// +// The predecessor's resolver answered every name on a machine: the mesh's own itself, the rest +// forwarded to two fixed upstreams, with the machine's resolv.conf naming it alone and the +// container runtime pointed at its private-network address. These hold the mesh's modules to the +// same arrangement, and to the two things a resolver here must never do — read resolv.conf for +// its upstreams, or take an address systemd-resolved holds. + +// resolverShelf is the three resolver modules beside something that answers `mesh-addressing`. +// The networking module that really does is composed in the controller and cannot be imported +// here, so a stand-in offers the same word; what is under test is the manifests, not the network. +func resolverShelf(t *testing.T) map[string]Manifest { + t.Helper() + shelf := map[string]Manifest{ + "net": {Module: "net", Version: "1", Provides: []Offer{{Name: "mesh-addressing"}}}, + } + for _, name := range []string{"dnsmasq", "resolv-conf", "resolved-split-dns"} { + shelf[name] = catalogueManifest(t, name) + } + return shelf +} + +// twoMachines is what the control plane hands a rendering: internal names and their addresses. +var twoMachines = map[string]string{"anchor.internal": "10.42.0.1", "laptop.internal": "10.42.0.2"} + +// Its configuration forwards to the upstreams the predecessor's module shipped, and gets them from +// nowhere else: `no-resolv` is what makes the documented loop — the resolver finding its own +// address in resolv.conf and becoming its own upstream — impossible. +func TestTheResolverForwardsToFixedUpstreamsAndNeverReadsResolvConf(t *testing.T) { + m := catalogueManifest(t, "dnsmasq") + var config string + for _, r := range m.Resources { + if r["id"] == "config" { + config, _ = r["content"].(string) + } + } + if config == "" { + t.Fatal("the resolver has no configuration file") + } + for _, want := range []string{ + "\nno-resolv\n", "\nserver=1.1.1.1\n", "\nserver=8.8.8.8\n", + "\nlisten-address=127.0.0.1\n", "\ninterface=mesh0\n", "\nbind-dynamic\n", + "\ndomain-needed\n", "\nbogus-priv\n", + "\nconf-file=" + m.Facts[FactNodeZones] + "\n", + } { + if !strings.Contains(config, want) { + t.Errorf("the resolver's configuration lacks %q:\n%s", strings.TrimSpace(want), config) + } + } + // Not .53 or .54, which systemd-resolved holds; and not .55 any more, which was a convention + // beside the one every machine already followed — the predecessor's resolv.conf says .1. + for _, taken := range []string{"127.0.0.53", "127.0.0.54", "127.0.0.55"} { + if strings.Contains(config, "listen-address="+taken) { + t.Errorf("the resolver listens on %s", taken) + } + } + // And the file that decides what the machine asks names it there, alone. + var resolv string + for _, r := range catalogueManifest(t, "resolv-conf").Resources { + if r["path"] == "/etc/resolv.conf" { + resolv, _ = r["content"].(string) + } + } + var nameservers []string + for _, line := range strings.Split(resolv, "\n") { + if strings.HasPrefix(line, "nameserver ") { + nameservers = append(nameservers, strings.TrimPrefix(line, "nameserver ")) + } + } + if len(nameservers) != 1 || nameservers[0] != "127.0.0.1" { + t.Errorf("resolv.conf names %v; the predecessor's names the mesh's resolver alone at 127.0.0.1", nameservers) + } + // The split-DNS alternative points at the same address, or a machine that keeps + // systemd-resolved in charge would route the mesh's suffix to nothing. + for _, r := range catalogueManifest(t, "resolved-split-dns").Resources { + if content, _ := r["content"].(string); content != "" && !strings.Contains(content, "DNS=127.0.0.1\n") { + t.Errorf("resolved-split-dns does not point at the resolver's address:\n%s", content) + } + } +} + +// The resolver and what points the machine at it compose on one machine, and what arrives is the +// mesh's account of every machine as a wildcard, the suffix kept local, the daemon restarting on +// that file, and the runtime pointed at this machine's own address. +func TestTheResolverAndWhatAsksItComposeOnOneMachine(t *testing.T) { + got, err := Resolve(resolverShelf(t), []string{"dnsmasq", "resolv-conf"}, + Node{Name: "anchor", At: "anchor.internal"}, World{}) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(strings.Join(named(got), " "), "net") { + t.Fatalf("the resolver's data is the mesh's addresses, and nothing answering them was taken: %v", named(got)) + } + out, err := got.Declaration(Rendering{ + Names: twoMachines, Suffix: "internal", + Needed: map[string]map[string]string{"dnsmasq": {"broker": "sealed"}}, + }) + if err != nil { + t.Fatal(err) + } + + ids := byID(out) + zones := ids["dnsmasq.fact-node-zones"] + if zones == nil || zones["path"] != "/etc/mesh-resolver/nodes.conf" { + t.Fatalf("the resolver was not given the machines where its configuration reads them: %v", zones) + } + content, _ := zones["content"].(string) + for _, want := range []string{ + "local=/internal/", "address=/anchor.internal/10.42.0.1", "address=/laptop.internal/10.42.0.2", + } { + if !strings.Contains(content, want) { + t.Errorf("the machines file lacks %q:\n%s", want, content) + } + } + + service := ids["dnsmasq.service"] + if service == nil { + t.Fatal("no resolver service composed") + } + reflects := map[string]bool{} + for _, id := range service["restart-on"].([]any) { + reflects[id.(string)] = true + } + if !reflects["dnsmasq.config"] || !reflects["dnsmasq.fact-node-zones"] { + t.Errorf("the daemon does not restart on its configuration and the machines file both: %v", service["restart-on"]) + } + + // The runtime's own file, written into (novox/hq ADR 0102) with the one key this module states. + runtime := ids["dnsmasq.runtime-dns"] + if runtime == nil || runtime["path"] != "/etc/docker/daemon.json" || runtime["into"] != "json" { + t.Fatalf("the runtime's dns is not written into its file: %v", runtime) + } + var keys map[string][]string + if err := json.Unmarshal([]byte(runtime["content"].(string)), &keys); err != nil { + t.Fatalf("the runtime's keys are not JSON: %v", err) + } + if len(keys) != 1 || len(keys["dns"]) != 1 || keys["dns"][0] != "10.42.0.1" { + t.Errorf("the runtime is pointed at %v; containers resolve at this machine's own private-network address, and nothing else is written", keys) + } + for _, r := range out { + if r["type"] == "service" && r["unit"] == "docker.service" && r["id"] != "" && + strings.HasPrefix(r["id"].(string), "dnsmasq.") { + t.Errorf("the resolver orders the runtime restarted or reloaded, which stops every container (ADR 0102) or does nothing for dns: %v", r) + } + } + + resolv := ids["resolv-conf.resolv"] + if resolv == nil || !strings.Contains(resolv["content"].(string), "\nnameserver 127.0.0.1\n") { + t.Fatalf("the machine is not pointed at the resolver: %v", resolv) + } +} + +// Two modules deciding what a machine asks are refused on one machine, as before — the claim +// exists so they never take turns overwriting each other. +func TestTwoThingsDecidingWhatAMachineAsksAreRefused(t *testing.T) { + _, err := Resolve(resolverShelf(t), []string{"dnsmasq", "resolv-conf", "resolved-split-dns"}, + Node{Name: "anchor", At: "anchor.internal"}, World{}) + if err == nil { + t.Fatal("resolv-conf and resolved-split-dns were both assigned to one machine") + } + if !strings.Contains(err.Error(), "the-resolver-configuration") { + t.Fatalf("the refusal does not say what was claimed: %v", err) + } +} + +// A machine that is not on the private network has no address for the runtime to be pointed at. +// Refused where the module and the machine are both named, rather than a placeholder written into +// the runtime's file and read as an address. +func TestTheResolverOnAMachineOffTheNetworkIsRefused(t *testing.T) { + got, err := Resolve(resolverShelf(t), []string{"dnsmasq"}, Node{Name: "anchor"}, World{}) + if err != nil { + t.Fatal(err) + } + _, err = got.Declaration(Rendering{Names: twoMachines, Suffix: "internal", + Needed: map[string]map[string]string{"dnsmasq": {"broker": "sealed"}}}) + if err == nil || !strings.Contains(err.Error(), "${machine:address}") { + t.Fatalf("a machine off the network was composed a resolver, or refused for another reason: %v", err) + } +} diff --git a/internal/overlay/generator.go b/internal/overlay/generator.go index c30eb67..eb0b9c9 100644 --- a/internal/overlay/generator.go +++ b/internal/overlay/generator.go @@ -54,17 +54,13 @@ const Addressing = "mesh-addressing" // something that needed the mesh's own addresses. Which is exactly what happened, once. const TheNetwork = "the-private-network" -// Resolver is the module that answers every name under a machine, and the claim it holds. -// -// A claim because a machine has one resolver: two daemons answering the same names on one machine -// is a coin toss about which one a query reaches, and the answer differing between them is the -// kind of fault nobody finds by looking at either. -const ( - Resolver = "mesh-resolver" - // ResolverData is what a module running a resolver requires: the mesh's own account of which - // machines exist and where, in a file. - ResolverData = "resolver-data" -) +// **A resolver's data went the same way as the names.** Two constants used to sit here — a +// `mesh-resolver` module that would write the machines as wildcards, and a `resolver-data` +// requirement a daemon would ask for. Nothing ever provided or consumed either: a module that +// answers names asks for the `node-zones` fact in its own manifest (catalogue.FactsInto) and +// requires Addressing, since the file is made of the mesh's addresses and means nothing off the +// network. A requirement nothing provides is refused at resolution, so leaving the names here +// would only have documented a mechanism that does not exist. // Domain is the module for people who want a network and do not want to choose one. //