diff --git a/internal/overlay/declaration_test.go b/internal/overlay/declaration_test.go index 64fabd9..af54dcd 100644 --- a/internal/overlay/declaration_test.go +++ b/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) + } } } diff --git a/internal/overlay/graph_test.go b/internal/overlay/graph_test.go index 5cfe6ea..eb8c4e1 100644 --- a/internal/overlay/graph_test.go +++ b/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) + } + } +}