Merge pull request 'A routed name resolves to the node whose proxy serves it, never to a provider merely told it (hq 178, forge 227)' (#181) from fix/227-a-name-resolves-to-the-node-that-serves-it into main

cmd/mesh-controller/plan.go

@@ -731,9 +731,13 @@ func routeNamesInTheMesh(ctx context.Context, open *stores) (map[string]string,
 		return nil, fmt.Errorf("which machines the mesh has cannot be read: %w", err)
 	}
 
-	out := map[string]string{}
+	// Every machine's resolution first, then the names across them at once: which node serves a
+	// name is a question about the graph — the consumer on one machine, the provider on another —
+	// and answered wrongly by looking at one contribution at a time (novox/hq issue 178).
+	plans := map[string]catalogue.Resolution{}
+	settings := map[string]catalogue.SettingsBy{}
 	for _, n := range nodes {
-		plan, settings, err := planFor(ctx, open, n.Name)
+		plan, layers, err := planFor(ctx, open, n.Name)
 		switch {
 		case unresolvable(err):
 			// Their set does not compose, so they serve no names. Passed over, so one machine's
@@ -745,38 +749,16 @@ func routeNamesInTheMesh(ctx context.Context, open *stores) (map[string]string,
 			// operator having withdrawn them (novox/hq 04-ISSUES/152).
 			return nil, fmt.Errorf("the names %s serves cannot be read: %w", n.Name, err)
 		}
-		for _, m := range plan.Modules {
+		plans[n.Name], settings[n.Name] = plan, layers
-			for to := range m.Contributes {
+	}
-				values, asks, err := plan.ContributionsFrom(to, m.Module, settings)
+	served, err := catalogue.NamesServed(plans, settings)
-				if err != nil {
+	if err != nil {
-					return nil, err
+		return nil, err
-				}
+	}
-				if !asks {
+	out := map[string]string{}
-					continue
+	for name, node := range served {
-				}
+		if at := address[node]; at != "" {
-				// A routed name, and only that: a contribution the mesh composed a name for from a
+			out[name] = at
-				// label it was given. A grant that happens to carry a `name` of its own — a database
-				// name — carries no label and is left alone.
-				if _, labelled := values["label"]; !labelled {
-					continue
-				}
-				name, _ := values["name"].(string)
-				if name == "" {
-					continue
-				}
-				// The node that serves it: whoever answers this consumer's route requirement, or
-				// this same node when the proxy is beside the consumer.
-				serving := n.Name
-				for _, need := range plan.Needs {
-					if need.Name == to && need.For == m.Module {
-						serving = need.From
-						break
-					}
-				}
-				if at := address[serving]; at != "" {
-					out[strings.ToLower(name)] = at
-				}
-			}
 		}
 	}
 	return out, nil

internal/catalogue/names_served.go

@@ -0,0 +1,124 @@
+package catalogue
+
+import (
+	"sort"
+	"strings"
+)
+
+// Which machine serves each routed name (novox/hq ADR 0066, issue 178).
+//
+// A routed name is a label the mesh composed for a consumer's endpoint, and it is *served* by the
+// provider that answers requests for it — the proxy the consumer's route reaches. The same name is
+// composed into every labelled contribution the consumer makes, because a provider that must know
+// the consumer's public name (an identity provider composing a redirect) is told it the same way
+// (04-ISSUES/122). Attributing the name to whichever of those providers a map happened to yield
+// last sent a public name to the identity provider's machine on one plan and to the proxy's on the
+// next (forge issue 227), and the whole names region flipped with it.
+//
+// **The terminus serves the name.** Among the providers a name reaches, the one that serves it is
+// the one that is not itself routed: a provider that contributes a labelled name of its own to some
+// requirement is published through another provider, and is a consumer of names, not their end.
+// Name-agnostic — nothing here knows what "route" means — and structural: it reads the graph the
+// modules declared. Deterministic: names, requirements and nodes are walked in order, so two
+// plans of one mesh yield one region.
+
+// NamesServed is every routed name across the mesh and the node that serves it, from every node's
+// resolution and settings. A name several termini claim goes to the first node in name order, so
+// the answer is stable; a name nothing terminal claims is left out.
+func NamesServed(plans map[string]Resolution, settings map[string]SettingsBy) (map[string]string, error) {
+	nodes := make([]string, 0, len(plans))
+	for n := range plans {
+		nodes = append(nodes, n)
+	}
+	sort.Strings(nodes)
+
+	out := map[string]string{}
+	for _, node := range nodes {
+		plan := plans[node]
+		all, err := plan.contributions(settings[node], nil, nil)
+		if err != nil {
+			return nil, err
+		}
+		requirements := make([]string, 0, len(all))
+		for to := range all {
+			requirements = append(requirements, to)
+		}
+		sort.Strings(requirements)
+		for _, to := range requirements {
+			for _, given := range all[to] {
+				if given.Node != "" {
+					// Said from another machine; that machine's own resolution carries it.
+					continue
+				}
+				// A routed name, and only that: a contribution the mesh composed a name for from a
+				// label it was given. A grant that happens to carry a `name` of its own — a database
+				// name — carries no label and is left alone.
+				if _, labelled := given.Values["label"]; !labelled {
+					continue
+				}
+				name, _ := given.Values["name"].(string)
+				if name == "" {
+					continue
+				}
+				serving := servingNodeOf(plan, to, given.From, node)
+				if !servesNames(plans[serving], to) {
+					continue
+				}
+				name = strings.ToLower(name)
+				if held, taken := out[name]; !taken || serving < held {
+					out[name] = serving
+				}
+			}
+		}
+	}
+	return out, nil
+}
+
+// servingNodeOf is the node answering one consumer's requirement: whoever the plan needs it from,
+// or this same node when the provider is beside the consumer.
+func servingNodeOf(plan Resolution, requirement, consumer, self string) string {
+	for _, need := range plan.Needs {
+		if need.Name == requirement && need.For == consumer && need.From != "" {
+			return need.From
+		}
+	}
+	return self
+}
+
+// servesNames says whether the module providing a requirement on a node is a terminus: it is not
+// itself published under a labelled name through some other provider. A node whose plan is not
+// known (it did not resolve) serves nothing.
+func servesNames(plan Resolution, requirement string) bool {
+	for _, m := range plan.Modules {
+		if !offers(m, requirement) {
+			continue
+		}
+		return !contributesALabel(m)
+	}
+	return false
+}
+
+func offers(m Manifest, requirement string) bool {
+	for _, o := range m.Offers() {
+		if o == requirement {
+			return true
+		}
+	}
+	return false
+}
+
+func contributesALabel(m Manifest) bool {
+	for _, values := range m.Contributes {
+		if _, labelled := values["label"]; labelled {
+			return true
+		}
+	}
+	for _, locals := range m.ContributesMany {
+		for _, values := range locals {
+			if _, labelled := values["label"]; labelled {
+				return true
+			}
+		}
+	}
+	return false
+}

internal/catalogue/names_served_test.go

@@ -0,0 +1,99 @@
+package catalogue
+
+import (
+	"testing"
+)
+
+// The mesh of forge issue 227 (novox/hq issue 178): a dashboard on the home server contributes its
+// label to the route its proxy serves AND to the identity provider on the control node, which must
+// know the dashboard's public name to compose a redirect. Both contributions carry the composed
+// name; only the proxy serves it.
+func twoNodesOneName(t *testing.T) (map[string]Resolution, map[string]SettingsBy) {
+	t.Helper()
+	catalogue := shelf(
+		Manifest{Module: "route-adapter", Version: "1", Provides: Offers("route"),
+			Serves: map[string]map[string]any{"route": {}}, Receives: map[string]string{"route": "/etc/adapter/mesh.json"}},
+		Manifest{Module: "route-proxy", Version: "1", Provides: Offers("route"),
+			Serves: map[string]map[string]any{"route": {}}, Receives: map[string]string{"route": "/etc/proxy/mesh.json"}},
+		Manifest{Module: "keycloak", Version: "1", Provides: FromAnywhere("oidc-client"),
+			Serves:   map[string]map[string]any{"oidc-client": {"token-path": "/token"}},
+			Receives: map[string]string{"oidc-client": "/etc/keycloak/clients.json"},
+			Listens:  []Listening{{Port: 8080, From: FromMesh, Why: "the login page"}},
+			// Published through the proxy itself: the identity provider is routed, not a router.
+			Contributes: map[string]map[string]any{"route": {"label": "login", "endpoint": "web", "port": 8080}}},
+		Manifest{Module: "grafana", Version: "1",
+			Listens: []Listening{{Port: 3000, From: FromMesh, Why: "dashboards"}},
+			Contributes: map[string]map[string]any{
+				"route":       {"label": "grafana", "endpoint": "web", "port": 3000},
+				"oidc-client": {"label": "grafana", "endpoint": "web", "port": 3000, "callback": "/login"},
+			}},
+	)
+	home := withDomain("home.example")
+	home.Name, home.At = "home-server", "home-server.internal"
+	control := withDomain("control.example")
+	control.Name, control.At = "anchor", "anchor.internal"
+
+	onHome, err := Resolve(catalogue, []string{"grafana", "route-adapter"}, home, World{
+		Offered: map[string][]Provider{"oidc-client": {{Node: "anchor", At: "anchor.internal", Module: "keycloak"}}},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	onControl, err := Resolve(catalogue, []string{"keycloak", "route-proxy"}, control, World{})
+	if err != nil {
+		t.Fatal(err)
+	}
+	return map[string]Resolution{"home-server": onHome, "anchor": onControl},
+		map[string]SettingsBy{"home-server": {}, "anchor": {}}
+}
+
+func TestANameResolvesToTheNodeWhoseProxyServesIt(t *testing.T) {
+	plans, settings := twoNodesOneName(t)
+	// Many times, because the fault was map order: one plan said one node, the next the other.
+	for i := 0; i < 25; i++ {
+		served, err := NamesServed(plans, settings)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if served["grafana.home.example"] != "home-server" {
+			t.Fatalf("run %d: the dashboard's name is served by %q, and its proxy is on the home server: %v",
+				i, served["grafana.home.example"], served)
+		}
+		if served["login.control.example"] != "anchor" {
+			t.Fatalf("run %d: the identity provider's own name is served by its proxy on the control node: %v", i, served)
+		}
+		if _, leaked := served["grafana.control.example"]; leaked {
+			t.Fatalf("a name composed for the identity provider's benefit is not one it serves: %v", served)
+		}
+	}
+}
+
+// A module that is routed several times names each route (ADR 0094's sibling for contributes);
+// every one of them is a name the mesh must resolve, and none reached the names region before.
+func TestEveryRouteOfAModuleWithSeveralIsANameServed(t *testing.T) {
+	catalogue := shelf(
+		Manifest{Module: "route-proxy", Version: "1", Provides: Offers("route"),
+			Serves: map[string]map[string]any{"route": {}}, Receives: map[string]string{"route": "/etc/proxy/mesh.json"}},
+		Manifest{Module: "photos", Version: "1",
+			Listens: []Listening{{Port: 8102, From: FromMesh, Why: "web"}, {Port: 9102, From: FromMesh, Why: "api"}},
+			ContributesMany: map[string]map[string]map[string]any{"route": {
+				"site": {"label": "photos", "endpoint": "web", "port": 8102},
+				"api":  {"label": "photos-api", "endpoint": "api", "port": 9102},
+			}}},
+	)
+	node := withDomain("control.example")
+	node.Name, node.At = "anchor", "anchor.internal"
+	plan, err := Resolve(catalogue, []string{"photos", "route-proxy"}, node, World{})
+	if err != nil {
+		t.Fatal(err)
+	}
+	served, err := NamesServed(map[string]Resolution{"anchor": plan}, map[string]SettingsBy{"anchor": {}})
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, name := range []string{"photos.control.example", "photos-api.control.example"} {
+		if served[name] != "anchor" {
+			t.Fatalf("%s is not served by its proxy: %v", name, served)
+		}
+	}
+}