cmd/mesh-controller/network.go

@@ -348,11 +348,28 @@ func whereEveryoneIs(ctx context.Context, inv *inventory.Inventory,
 		return nil, errors.New(
 			"asked where everyone is without the catalogue, which cannot be answered")
 	}
-	places, err := inv.Overlays(ctx)
+	places, err := onTheNetwork(ctx, inv, shelf)
 	if err != nil {
 		return nil, err
 	}
 	out := map[string]string{}
+	for _, p := range places {
+		out[p.Name] = overlay.InternalName(p.Name)
+	}
+	return out, nil
+}
+
+// onTheNetwork is every placed machine that resolves the private network — has an address AND
+// runs what puts it there. "Has an address" alone was true of every placed machine and told you
+// nothing about whether anything could reach it; a name written for such a machine resolves to
+// an address that does not answer, and a connection to it hangs (novox/hq issue 079).
+func onTheNetwork(ctx context.Context, inv *inventory.Inventory,
+	shelf map[string]catalogue.Manifest) ([]inventory.Overlay, error) {
+	places, err := inv.Overlays(ctx)
+	if err != nil {
+		return nil, err
+	}
+	var out []inventory.Overlay
 	for _, p := range places {
 		if p.Address == "" {
 			continue
@@ -371,7 +388,7 @@ func whereEveryoneIs(ctx context.Context, inv *inventory.Inventory,
 		for _, m := range got.Modules {
 			for _, offered := range m.Offers() {
 				if offered == overlay.Requirement {
-					out[p.Name] = overlay.InternalName(p.Name)
+					out = append(out, p)
 				}
 			}
 		}
@@ -379,14 +396,17 @@ func whereEveryoneIs(ctx context.Context, inv *inventory.Inventory,
 	return out, nil
 }
 
-// onThePrivateNetwork is every node's address on the overlay, sorted.
+// onThePrivateNetwork is every node's address on the private network, sorted — the same set
+// the names and the resolver mean by it (onTheNetwork), so a rule saying "from the mesh" admits
+// exactly the machines the mesh names.
 //
 // A node with no address is left out rather than rendered as an empty source: an empty entry in a
 // source set is a syntax error in the rule file, and a rule file that does not load leaves the
 // node filtering whatever it was filtering before -- the one outcome worse than a wrong rule,
 // because nothing reports it.
-func onThePrivateNetwork(ctx context.Context, inv *inventory.Inventory) ([]string, error) {
-	places, err := inv.Overlays(ctx)
+func onThePrivateNetwork(ctx context.Context, inv *inventory.Inventory,
+	shelf map[string]catalogue.Manifest) ([]string, error) {
+	places, err := onTheNetwork(ctx, inv, shelf)
 	if err != nil {
 		return nil, err
 	}
@@ -400,22 +420,21 @@ func onThePrivateNetwork(ctx context.Context, inv *inventory.Inventory) ([]strin
 	return out, nil
 }
 
-// namesInTheMesh is every machine's internal name and the address behind it.
+// namesInTheMesh is every machine's internal name and the address behind it — every machine
+// that is on the private network, the same set the resolver means by that.
 //
-// A machine with no address has no name: writing one that resolves to nothing is worse than not
+// A machine that is not has no name: writing one that resolves to nothing is worse than not
 // writing it, because a connection to an address that does not answer hangs where a name that
-// does not resolve fails at once and says so. That is the rule the hosts file already follows,
-// and this is the same set read the same way.
-func namesInTheMesh(ctx context.Context, inv *inventory.Inventory) (map[string]string, error) {
-	places, err := inv.Overlays(ctx)
+// does not resolve fails at once and says so. A machine placed on the overlay but not running
+// the module that puts it there is exactly that (novox/hq issue 079).
+func namesInTheMesh(ctx context.Context, inv *inventory.Inventory,
+	shelf map[string]catalogue.Manifest) (map[string]string, error) {
+	places, err := onTheNetwork(ctx, inv, shelf)
 	if err != nil {
 		return nil, err
 	}
 	out := map[string]string{}
 	for _, p := range places {
-		if strings.TrimSpace(p.Address) == "" {
-			continue
-		}
 		out[overlay.InternalName(p.Name)] = p.Address
 	}
 	return out, nil

cmd/mesh-controller/network_test.go

@@ -6,6 +6,7 @@ import (
 	"testing"
 
 	"github.com/novox/mesh-controller/internal/inventory"
+	"github.com/novox/mesh-controller/internal/overlay"
 )
 
 // placementOf is what the mesh holds about where one node is.
@@ -76,3 +77,34 @@ func TestAPlacementAndNothingTogetherIsRefused(t *testing.T) {
 		t.Fatal("a placement and --nothing together was accepted")
 	}
 }
+
+// A machine is named for the others only while it is on the private network — placed AND running
+// what puts it there — the same set the resolver means by "on the private network". A machine
+// that has an address and no networking is not named: a name resolving to an address that does
+// not answer hangs where an unknown name fails at once (novox/hq issue 079).
+func TestOnlyAMachineOnThePrivateNetworkIsNamed(t *testing.T) {
+	open := aMesh(t) // two placed machines, both assigned what puts them on the private network
+	ctx := t.Context()
+	shelf, err := open.inventory.Catalogue(ctx)
+	if err != nil {
+		t.Fatal(err)
+	}
+	names, err := namesInTheMesh(ctx, open.inventory, shelf)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if names["anchor.internal"] != "10.77.0.1" || names["laptop.internal"] != "10.77.0.2" {
+		t.Fatalf("two machines on the network are not both named: %v", names)
+	}
+	// 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)
+	}
+	names, err = namesInTheMesh(ctx, open.inventory, shelf)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, still := names["laptop.internal"]; still || names["anchor.internal"] != "10.77.0.1" {
+		t.Fatalf("a machine that left the network is still named, or the one that stayed is not: %v", names)
+	}
+}

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"
 )
@@ -425,7 +426,12 @@ func declarationWith(ctx context.Context, open *stores, node string,
 	// resolves to. Every node's address, including this one's: a machine reaching itself by its
 	// own overlay address rather than by loopback is ordinary, and leaving it out would filter
 	// the node's own traffic to itself with no rule naming why.
-	private, err := onThePrivateNetwork(ctx, inv)
+	// One reading of the catalogue for the three questions below that resolve the whole mesh.
+	shelf, err := inv.Catalogue(ctx)
+	if err != nil {
+		return nil, err
+	}
+	private, err := onThePrivateNetwork(ctx, inv, shelf)
 	if err != nil {
 		return nil, err
 	}
@@ -433,7 +439,7 @@ func declarationWith(ctx context.Context, open *stores, node string,
 	// And every machine's name, so a container can reach one. The same set that writes the
 	// machine's own hosts file — one reading, so a container and its machine cannot disagree
 	// about where another machine is.
-	names, err := namesInTheMesh(ctx, inv)
+	names, err := namesInTheMesh(ctx, inv, shelf)
 	if err != nil {
 		return nil, err
 	}
@@ -481,7 +487,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,10 +106,11 @@ func nodeNames(r Resolution, addresses map[string]string) string {
 	b.WriteString("\n")
 	for _, name := range sortedNames(addresses) {
 		at := addresses[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.internal\t%s", at, name, name)
-		if name == r.Node {
+		fmt.Fprintf(&b, "%s\t%s\t%s", at, internal, bare)
+		if bare == r.Node {
 			b.WriteString("\t# this machine")
 		}
 		b.WriteString("\n")
@@ -121,16 +122,33 @@ 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) {
-		fmt.Fprintf(&b, "address=/%s.internal/%s\n", name, addresses[name])
+		internal, _ := meshName(name, suffix)
+		fmt.Fprintf(&b, "address=/%s/%s\n", internal, addresses[name])
 	}
 	return b.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. The
+// 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, ".")
+	if strings.HasSuffix(name, dotted) {
+		return name, strings.TrimSuffix(name, dotted)
+	}
+	return name + dotted, name
+}
+
 func sortedNames(addresses map[string]string) []string {
 	out := make([]string, 0, len(addresses))
 	for name, at := range addresses {

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),
-		nodeZones(Resolution{Node: "homer"}, threeMachines),
+		nodeNames(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,44 @@ 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")
 	}
 }
+
+// **The names the control plane hands a resolution are already internal names** — `homer.internal`,
+// the same map every container gets as its hosts. Appending the suffix again wrote
+// `homer.internal.internal` into every hosts file and every resolver's zones, and the large mesh
+// bed's name test was the first to read it back. Either key gives the same files.
+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 {
+		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 {
+		t.Fatalf("the hosts differ by how the names were keyed:\n%s\n---\n%s", a, b)
+	}
+	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, "")
+	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)
+	}
+}