appc: add test to ensure that individual IPs are not removed during route updates

If control advised the connector to advertise a route that had already
been discovered by DNS it would be incorrectly removed. Now those routes
are preserved.

Updates tailscale/corp#16833

Signed-off-by: James Tucker <[email protected]>

appc/appconnector.go

@@ -155,7 +155,7 @@ nextRoute:
 	for _, r := range routes {
 		for _, addr := range e.domains {
 			for _, a := range addr {
-				if r.Contains(a) {
+				if r.Contains(a) && netip.PrefixFrom(a, a.BitLen()) != r {
 					pfx := netip.PrefixFrom(a, a.BitLen())
 					toRemove = append(toRemove, pfx)
 					continue nextRoute

appc/appconnector_test.go

@@ -45,13 +45,39 @@ func TestUpdateDomains(t *testing.T) {
 }
 
 func TestUpdateRoutes(t *testing.T) {
+	ctx := context.Background()
 	rc := &appctest.RouteCollector{}
 	a := NewAppConnector(t.Logf, rc)
-	routes := []netip.Prefix{netip.MustParsePrefix("192.0.2.0/24")}
+	a.updateDomains([]string{"*.example.com"})
+
+	// This route should be collapsed into the range
+	a.ObserveDNSResponse(dnsResponse("a.example.com.", "192.0.2.1"))
+	a.Wait(ctx)
+
+	if !slices.Equal(rc.Routes(), []netip.Prefix{netip.MustParsePrefix("192.0.2.1/32")}) {
+		t.Fatalf("got %v, want %v", rc.Routes(), []netip.Prefix{netip.MustParsePrefix("192.0.2.1/32")})
+	}
+
+	// This route should not be collapsed or removed
+	a.ObserveDNSResponse(dnsResponse("b.example.com.", "192.0.0.1"))
+	a.Wait(ctx)
+
+	routes := []netip.Prefix{netip.MustParsePrefix("192.0.2.0/24"), netip.MustParsePrefix("192.0.0.1/32")}
 	a.updateRoutes(routes)
 
+	slices.SortFunc(rc.Routes(), prefixCompare)
+	rc.SetRoutes(slices.Compact(rc.Routes()))
+	slices.SortFunc(routes, prefixCompare)
+
+	// Ensure that the non-matching /32 is preserved, even though it's in the domains table.
 	if !slices.EqualFunc(routes, rc.Routes(), prefixEqual) {
-		t.Fatalf("got %v, want %v", rc.Routes(), routes)
+		t.Errorf("added routes: got %v, want %v", rc.Routes(), routes)
+	}
+
+	// Ensure that the contained /32 is removed, replaced by the /24.
+	wantRemoved := []netip.Prefix{netip.MustParsePrefix("192.0.2.1/32")}
+	if !slices.EqualFunc(rc.RemovedRoutes(), wantRemoved, prefixEqual) {
+		t.Fatalf("unexpected removed routes: %v", rc.RemovedRoutes())
 	}
 }
 
@@ -195,3 +221,10 @@ func dnsResponse(domain, address string) []byte {
 func prefixEqual(a, b netip.Prefix) bool {
 	return a == b
 }
+
+func prefixCompare(a, b netip.Prefix) int {
+	if a.Addr().Compare(b.Addr()) == 0 {
+		return a.Bits() - b.Bits()
+	}
+	return a.Addr().Compare(b.Addr())
+}

appc/appctest/appctest.go

@@ -10,7 +10,8 @@ import (
 
 // RouteCollector is a test helper that collects the list of routes advertised
 type RouteCollector struct {
-	routes []netip.Prefix
+	routes        []netip.Prefix
+	removedRoutes []netip.Prefix
 }
 
 func (rc *RouteCollector) AdvertiseRoute(pfx ...netip.Prefix) error {
@@ -24,11 +25,18 @@ func (rc *RouteCollector) UnadvertiseRoute(toRemove ...netip.Prefix) error {
 	for _, r := range routes {
 		if !slices.Contains(toRemove, r) {
 			rc.routes = append(rc.routes, r)
+		} else {
+			rc.removedRoutes = append(rc.removedRoutes, r)
 		}
 	}
 	return nil
 }
 
+// RemovedRoutes returns the list of routes that were removed.
+func (rc *RouteCollector) RemovedRoutes() []netip.Prefix {
+	return rc.removedRoutes
+}
+
 // Routes returns the ordered list of routes that were added, including
 // possible duplicates.
 func (rc *RouteCollector) Routes() []netip.Prefix {