internal/overlay/declaration_test.go

@@ -287,25 +287,19 @@ func TestANodeWithNoTunnelAndNoEndpointStillListensOnNothing(t *testing.T) {
 	}
 }
 
+// A path tuned to a smaller MTU (1380 here) survives the takeover: the mesh interface comes up with the same MTU, or
+// transfers hang silently (novox/hq: a taken tunnel carries its MTU). A tunnel that set none gets no MTU line.
 func TestATakenTunnelCarriesItsMTU(t *testing.T) {
-	// A path tuned to a smaller MTU (1380 here) must survive the takeover — the mesh interface
-	// comes up with the same MTU, or transfers hang silently (novox/hq: a taken tunnel carries
-	// its MTU).
-	config, _ := declarationFor(t, Node{
-		Name: "shanks", Key: "SPOKE", Address: "10.10.0.3",
-		TakesOver: &TakeOver{Interface: "wg0", Port: 51820, MTU: 1380},
-	}, nil)
-	if !strings.Contains(config, "MTU = 1380") {
-		t.Fatalf("the tuned MTU was dropped:\n%s", config)
-	}
-}
-
-func TestNoMTULineWhenTheTunnelSetNone(t *testing.T) {
-	config, _ := declarationFor(t, Node{
-		Name: "shanks", Key: "SPOKE", Address: "10.10.0.3",
-		TakesOver: &TakeOver{Interface: "wg0", Port: 51820},
-	}, nil)
-	if strings.Contains(config, "MTU") {
-		t.Fatalf("no MTU was found, so none should be written:\n%s", config)
+	for _, c := range []struct {
+		mtu  int
+		want string
+	}{{1380, "MTU = 1380"}, {0, ""}} {
+		config, _ := declarationFor(t, Node{
+			Name: "shanks", Key: "SPOKE", Address: "10.10.0.3",
+			TakesOver: &TakeOver{Interface: "wg0", Port: 51820, MTU: c.mtu},
+		}, nil)
+		if c.want != "" && !strings.Contains(config, c.want) || c.want == "" && strings.Contains(config, "MTU") {
+			t.Errorf("a tunnel of MTU %d gives:\n%s", c.mtu, config)
+		}
 	}
 }

internal/overlay/graph_test.go

@@ -158,35 +158,6 @@ func TestTheHubKnowsEveryoneItRoutesFor(t *testing.T) {
 	}
 }
 
-func TestAMeshWithNoHubIsRefused(t *testing.T) {
-	// Not an empty graph. A mesh with no hub has no path between sites, and answering "no peers"
-	// would look like a working network in which nothing can reach anything — which is how the
-	// old arrangement failed, silently, when nobody knew the address convention.
-	_, err := Compute([]Node{
-		at("a", "", "10.42.0.1", "", false),
-		at("b", "", "10.42.0.2", "", false),
-	}, cidr)
-	if !errors.Is(err, ErrNoHub) {
-		t.Fatalf("a mesh with no hub gave %v", err)
-	}
-}
-
-func TestAnUnreachableHubIsRefused(t *testing.T) {
-	// The hub is the one node that must be dialable from wherever the others are. Left
-	// unchecked, every node would be given a peer it can never reach and the mesh would look
-	// configured and be silent.
-	_, err := Compute([]Node{
-		at("anchor", "datacentre", "10.42.0.1", "", true),
-		at("laptop", "", "10.42.0.2", "", false),
-	}, cidr)
-	if err == nil {
-		t.Fatal("a hub with no endpoint was accepted")
-	}
-	if !strings.Contains(err.Error(), "must be reachable") {
-		t.Errorf("the refusal does not say why: %v", err)
-	}
-}
-
 func TestANodeWithNoPlaceYetIsSkippedRatherThanFatal(t *testing.T) {
 	// A node that has enrolled and not yet been given an address is an ordinary in-between state.
 	// Failing the whole graph over it would mean no node gets a network because one is half done.
@@ -206,23 +177,6 @@ func TestANodeWithNoPlaceYetIsSkippedRatherThanFatal(t *testing.T) {
 	}
 }
 
-func TestNobodyPeersWithThemselves(t *testing.T) {
-	g, err := Compute([]Node{
-		at("anchor", "house", "10.42.0.1", "198.51.100.1:51820", true),
-		at("desk", "house", "10.42.0.2", "192.0.2.2:51820", false),
-	}, cidr)
-	if err != nil {
-		t.Fatal(err)
-	}
-	for node, peers := range g {
-		for _, p := range peers {
-			if p.Name == node {
-				t.Errorf("%s peers with itself", node)
-			}
-		}
-	}
-}
-
 func TestAHubAtYourOwnSiteAppearsOnceNotTwice(t *testing.T) {
 	// WireGuard takes one entry per public key. A hub that shares a site with a spoke was being
 	// emitted twice — once as a direct peer and once as the route of last resort — producing a
@@ -257,6 +211,23 @@ func TestAHubAtYourOwnSiteAppearsOnceNotTwice(t *testing.T) {
 		t.Errorf("the single entry allows %s; it is both the direct path and the route of last "+
 			"resort, so it carries the whole overlay", got)
 	}
+	// And nobody peers with themselves, here or in a site of two dialable nodes.
+	other, err := Compute([]Node{
+		at("anchor", "house", "10.42.0.1", "198.51.100.1:51820", true),
+		at("desk", "house", "10.42.0.2", "192.0.2.2:51820", false),
+	}, cidr)
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, graph := range []Graph{g, other} {
+		for node, peers := range graph {
+			for _, p := range peers {
+				if p.Name == node {
+					t.Errorf("%s peers with itself", node)
+				}
+			}
+		}
+	}
 }
 
 func TestTwoUnreachableNodesAtOneSiteDoNotPeerDirectly(t *testing.T) {
@@ -340,3 +311,23 @@ func TestTheHubCarriesTheTunnelsPeersUntilTheyEnrol(t *testing.T) {
 		t.Error("a carried peer appeared in a spoke's peer list")
 	}
 }
+
+// A mesh whose hub cannot carry the paths between sites is refused, never answered as an empty graph: with no hub there
+// is no path between sites, and a hub with no endpoint is a peer every node is given and none can reach. Either would
+// look configured and be silent, which is how the old arrangement failed when nobody knew the address convention.
+func TestAHubThatCannotCarryThePathsIsRefused(t *testing.T) {
+	for _, c := range []struct {
+		name  string
+		nodes []Node
+		says  func(error) bool
+	}{
+		{"no hub", []Node{at("a", "", "10.42.0.1", "", false), at("b", "", "10.42.0.2", "", false)},
+			func(err error) bool { return errors.Is(err, ErrNoHub) }},
+		{"a hub with no endpoint", []Node{at("anchor", "datacentre", "10.42.0.1", "", true), at("laptop", "", "10.42.0.2", "", false)},
+			func(err error) bool { return err != nil && strings.Contains(err.Error(), "must be reachable") }},
+	} {
+		if _, err := Compute(c.nodes, cidr); !c.says(err) {
+			t.Errorf("%s: %v", c.name, err)
+		}
+	}
+}