The facts write the suffix the control plane composed the names with, handed down rather than written twice (review of issue 079); the fixture is keyed as production keys it

cmd/mesh-controller/plan.go

@@ -13,6 +13,7 @@ import (
 	"github.com/novox/mesh-controller/internal/catalogue"
 	"github.com/novox/mesh-controller/internal/inventory"
 	"github.com/novox/mesh-controller/internal/licences"
+	"github.com/novox/mesh-controller/internal/overlay"
 	"net"
 	"strconv"
 )
@@ -481,7 +482,7 @@ func declarationWith(ctx context.Context, open *stores, node string,
 	return plan.Declaration(catalogue.Rendering{
 		Settings: settings, Generators: gens, Grants: grants, Needed: needed, Ports: ports,
 		Certificate: certificate, Authority: authority, Mesh: private, Names: names,
-		Foundation: foundation, Kept: kept})
+		Suffix: overlay.Suffix(), Foundation: foundation, Kept: kept})
 }
 
 // routeNamesInTheMesh is every routed name and the address of the node that serves it (novox/hq

internal/catalogue/declaration.go

@@ -92,6 +92,10 @@ type Rendering struct {
 	// mesh" resolves to. Passed in for the same reason grants are: who else is on the network is
 	// a fact about the mesh, and resolution answers questions about one machine.
 	Mesh []string
+	// Suffix is what a machine's internal name ends in, as the control plane composed Names —
+	// `internal` unless the operator chose another — so a fact writing those names does not
+	// compose it a second time.
+	Suffix string
 
 	// Kept is every operator-sealed secret in the mesh, for a module that `keeps` them. Nil when
 	// nothing on this node keeps them, or the mesh has no operator key.
@@ -524,7 +528,7 @@ func (r Resolution) Declaration(with Rendering) ([]map[string]any, error) {
 		// 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)
+		given, err := FactsInto(m, r, with.Names, with.Suffix)
 		if err != nil {
 			return nil, err
 		}

internal/catalogue/facts.go

@@ -36,7 +36,7 @@ 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{
+var facts = map[string]func(Resolution, map[string]string, string) string{
 	FactNodeNames: nodeNames,
 	FactNodeZones: nodeZones,
 }
@@ -45,7 +45,7 @@ var facts = map[string]func(Resolution, map[string]string) string{
 //
 // 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) ([]map[string]any, error) {
+func FactsInto(m Manifest, r Resolution, addresses map[string]string, suffix string) ([]map[string]any, error) {
 	if len(m.Facts) == 0 {
 		return nil, nil
 	}
@@ -70,7 +70,7 @@ func FactsInto(m Manifest, r Resolution, addresses map[string]string) ([]map[str
 		}
 		out = append(out, map[string]any{
 			"id": "fact-" + name, "type": "file", "path": path, "mode": "0644",
-			"content": write(r, addresses),
+			"content": write(r, addresses, suffix),
 		})
 	}
 	return out, nil
@@ -92,7 +92,7 @@ func spokenFacts() string {
 // and does not yet know where it is, which is the ordinary state between adding a machine and it
 // joining. Writing the name anyway would give a name that resolves to nothing, and a connection to
 // that hangs; leaving it out fails at once and says the name is unknown.
-func nodeNames(r Resolution, addresses map[string]string) string {
+func nodeNames(r 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")
@@ -106,7 +106,7 @@ func nodeNames(r Resolution, addresses map[string]string) string {
 	b.WriteString("\n")
 	for _, name := range sortedNames(addresses) {
 		at := addresses[name]
-		internal, bare := meshName(name)
+		internal, bare := meshName(name, suffix)
 		// Its mesh name resolves to its address on the private network rather than to loopback,
 		// so a service binding the name it was given stays reachable from everywhere else.
 		fmt.Fprintf(&b, "%s\t%s\t%s", at, internal, bare)
@@ -122,12 +122,12 @@ func nodeNames(r Resolution, addresses map[string]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.
-func nodeZones(_ Resolution, addresses map[string]string) string {
+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")
 	for _, name := range sortedNames(addresses) {
-		internal, _ := meshName(name)
+		internal, _ := meshName(name, suffix)
 		fmt.Fprintf(&b, "address=/%s/%s\n", internal, addresses[name])
 	}
 	return b.String()
@@ -135,14 +135,18 @@ func nodeZones(_ Resolution, addresses map[string]string) string {
 
 // meshName is a machine's internal name and its bare one, from either. The control plane keys
 // the names it hands a resolution by the internal name (`homer.internal`), the same map a
-// container gets as its hosts; a caller that keys by the bare name gets the same answer. Written
+// container gets as its hosts; a caller that keys by the bare name gets the same answer. The
-// once, because the alternative was `homer.internal.internal` on every machine.
+// suffix is the one the control plane composed those names with, handed down rather than written
-func meshName(name string) (internal, bare string) {
+// here a second time — the alternative was `homer.internal.internal` on every machine.
-	const suffix = ".internal"
+func meshName(name, suffix string) (internal, bare string) {
-	if strings.HasSuffix(name, suffix) {
+	if suffix == "" {
-		return name, strings.TrimSuffix(name, suffix)
+		suffix = "internal"
 	}
-	return name + suffix, name
+	dotted := "." + strings.TrimPrefix(suffix, ".")
+	if strings.HasSuffix(name, dotted) {
+		return name, strings.TrimSuffix(name, dotted)
+	}
+	return name + dotted, name
 }
 
 func sortedNames(addresses map[string]string) []string {

internal/catalogue/facts_test.go

@@ -5,11 +5,12 @@ import (
 	"testing"
 )
 
-var threeMachines = map[string]string{"homer": "10.42.0.1", "marge": "10.42.0.2", "bart": ""}
+// 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.**
 func TestEveryMachineIsAWildcardUnderItsOwnName(t *testing.T) {
-	out := nodeZones(Resolution{Node: "homer"}, threeMachines)
+	out := nodeZones(Resolution{Node: "homer"}, threeMachines, "")
 	for _, want := range []string{
 		"address=/homer.internal/10.42.0.1",
 		"address=/marge.internal/10.42.0.2",
@@ -27,8 +28,8 @@ func TestEveryMachineIsAWildcardUnderItsOwnName(t *testing.T) {
 // unknown, which is a thing somebody can act on.
 func TestAMachineWithNoAddressIsNotNamed(t *testing.T) {
 	for _, out := range []string{
-		nodeNames(Resolution{Node: "homer"}, threeMachines),
+		nodeNames(Resolution{Node: "homer"}, threeMachines, ""),
-		nodeZones(Resolution{Node: "homer"}, threeMachines),
+		nodeZones(Resolution{Node: "homer"}, threeMachines, ""),
 	} {
 		if strings.Contains(out, "bart") {
 			t.Fatalf("a machine with no address was named, so its name resolves to nothing:\n%s", out)
@@ -39,7 +40,7 @@ func TestAMachineWithNoAddressIsNotNamed(t *testing.T) {
 // A machine's own mesh name points at its address on the private network, not at loopback — or a
 // service binding the name it was given is unreachable from everywhere else.
 func TestAMachinesOwnNameIsItsMeshAddress(t *testing.T) {
-	out := nodeNames(Resolution{Node: "homer"}, threeMachines)
+	out := nodeNames(Resolution{Node: "homer"}, threeMachines, "")
 	var line string
 	for _, l := range strings.Split(out, "\n") {
 		if strings.Contains(l, "homer.internal") {
@@ -58,7 +59,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, "")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -77,7 +78,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, "")
 	if err == nil {
 		t.Fatal("a module asked for something nobody computes and was given nothing, silently")
 	}
@@ -91,7 +92,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, ""); err == nil {
 		t.Fatal("a relative path was accepted")
 	}
 }
@@ -103,18 +104,32 @@ func TestAFactMustBeAskedForAtAnAbsolutePath(t *testing.T) {
 func TestNamesKeyedByInternalNameAreNotSuffixedTwice(t *testing.T) {
 	internal := map[string]string{"homer.internal": "10.42.0.1", "marge.internal": "10.42.0.2"}
 	bare := map[string]string{"homer": "10.42.0.1", "marge": "10.42.0.2"}
-	if a, b := nodeZones(Resolution{Node: "homer"}, internal), nodeZones(Resolution{Node: "homer"}, bare); a != b {
+	if a, b := nodeZones(Resolution{Node: "homer"}, internal, ""), nodeZones(Resolution{Node: "homer"}, bare, ""); a != b {
 		t.Fatalf("the zones differ by how the names were keyed:\n%s\n---\n%s", a, b)
 	}
-	if a, b := nodeNames(Resolution{Node: "homer"}, internal), nodeNames(Resolution{Node: "homer"}, bare); a != b {
+	if a, b := nodeNames(Resolution{Node: "homer"}, internal, ""), nodeNames(Resolution{Node: "homer"}, bare, ""); a != b {
 		t.Fatalf("the hosts differ by how the names were keyed:\n%s\n---\n%s", a, b)
 	}
-	zones := nodeZones(Resolution{Node: "homer"}, internal)
+	zones := nodeZones(Resolution{Node: "homer"}, internal, "")
 	if strings.Contains(zones, "internal.internal") || !strings.Contains(zones, "address=/homer.internal/10.42.0.1") {
 		t.Fatalf("the zones carry a doubled suffix or miss the name:\n%s", zones)
 	}
-	hosts := nodeNames(Resolution{Node: "homer"}, internal)
+	hosts := nodeNames(Resolution{Node: "homer"}, internal, "")
 	if !strings.Contains(hosts, "10.42.0.1\thomer.internal\thomer\t# this machine") {
 		t.Fatalf("the hosts line for the machine itself is not name, bare name and the mark:\n%s", hosts)
 	}
 }
+
+// The suffix the control plane composed the names with is the one the facts write — an operator
+// who chose another does not get `.internal` appended to it.
+func TestTheFactsWriteTheSuffixTheNamesWereComposedWith(t *testing.T) {
+	names := map[string]string{"homer.lan": "10.42.0.1"}
+	zones := nodeZones(Resolution{Node: "homer"}, names, "lan")
+	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)
+	}
+	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)
+	}
+}