diff --git a/appc/appconnector.go b/appc/appconnector.go index 65cd7a643..34e9845ff 100644 --- a/appc/appconnector.go +++ b/appc/appconnector.go @@ -226,7 +226,13 @@ func (e *AppConnector) updateDomains(domains []string) { // everything left in oldDiscovered a) won't be in e.domains and b) can be unadvertised if it's not in local or control for domainName, domainsRoutes := range oldDiscovered { if domainsRoutes != nil { - toRemove := domainsRoutes.RoutesSlice() + toRemove := []netip.Prefix{} + for _, route := range domainsRoutes.RoutesSlice() { + if !slices.Contains(routeInfo.Local, route) { + toRemove = append(toRemove, route) + } + // TODO check control also + } e.logf("unadvertising %d routes for domain: %s", len(toRemove), domainName) e.scheduleUnadvertisement(domainName, toRemove...) } diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go index 7cc64d1c8..4f16bdc98 100644 --- a/ipn/ipnlocal/local_test.go +++ b/ipn/ipnlocal/local_test.go @@ -2775,14 +2775,15 @@ func TestPatchPrefsHandlerWithoutPresistStore(t *testing.T) { } func TestFran(t *testing.T) { + prefixSortFunc := func(i, j netip.Prefix) int { + return i.Addr().Compare(j.Addr()) + } epIP2 := netip.MustParsePrefix("192.1.0.9/32") explicitlyAdvertisedRoutes := []netip.Prefix{netip.MustParsePrefix("192.1.0.8/32"), epIP2} oneRoutes := []netip.Prefix{netip.MustParsePrefix("192.0.0.8/32"), netip.MustParsePrefix("192.0.0.16/32")} - //twoRoutes := []netip.Prefix{netip.MustParsePrefix("192.0.0.10/32"), netip.MustParsePrefix("192.0.0.18/32"), epIP2} - twoRoutes := []netip.Prefix{netip.MustParsePrefix("192.0.0.10/32"), netip.MustParsePrefix("192.0.0.18/32")} + twoRoutes := []netip.Prefix{netip.MustParsePrefix("192.0.0.10/32"), netip.MustParsePrefix("192.0.0.18/32"), epIP2} for _, shouldStore := range []bool{true, false} { - //for _, shouldStore := range []bool{false} { b := newTestBackend(t) b.ControlKnobs().AppCStoreRoutes.Store(shouldStore) // make b an app connector @@ -2826,9 +2827,13 @@ func TestFran(t *testing.T) { afterTwoRoutes := append([]netip.Prefix{}, explicitlyAdvertisedRoutes...) afterTwoRoutes = append(afterTwoRoutes, oneRoutes...) afterTwoRoutes = append(afterTwoRoutes, twoRoutes...) + slices.SortFunc(afterTwoRoutes, prefixSortFunc) + afterTwoRoutes = slices.Compact(afterTwoRoutes) afterOneRoutes := append([]netip.Prefix{}, explicitlyAdvertisedRoutes...) afterOneRoutes = append(afterOneRoutes, oneRoutes...) + slices.SortFunc(afterOneRoutes, prefixSortFunc) + afterOneRoutes = slices.Compact(afterOneRoutes) for _, tst := range []struct { domain string @@ -2840,11 +2845,12 @@ func TestFran(t *testing.T) { // doesn't care about example.com, so still just the route for one.com {domain: "example.com", route: []string{"192.0.0.9", "192.0.0.17"}, wantAfter: afterOneRoutes}, // learns the route for two.com as well - {domain: "two.com", route: []string{"192.0.0.10", "192.0.0.18"}, wantAfter: afterTwoRoutes}, + {domain: "two.com", route: []string{"192.0.0.10", "192.0.0.18", "192.1.0.9"}, wantAfter: afterTwoRoutes}, } { b.ObserveDNSResponse(dnsResponse(tst.domain+".", tst.route)) b.appConnector.Wait(context.Background()) routesNow := b.pm.prefs.AdvertiseRoutes().AsSlice() + slices.SortFunc(routesNow, prefixSortFunc) if !slices.Equal(routesNow, tst.wantAfter) { t.Fatalf("after dns response for %s got routes %v, want %v", tst.domain, routesNow, tst.wantAfter) } @@ -2857,6 +2863,7 @@ func TestFran(t *testing.T) { if !shouldStore { wantRoutes = afterTwoRoutes } + slices.SortFunc(routesNow, prefixSortFunc) if !slices.Equal(routesNow, wantRoutes) { t.Fatalf("after removing two.com (shouldStore=%t), got %v, want %v", shouldStore, routesNow, wantRoutes) } @@ -2879,6 +2886,7 @@ func TestFran(t *testing.T) { if !shouldStore { wantRoutes = afterTwoRoutes } + slices.SortFunc(routesNow, prefixSortFunc) if !slices.Equal(routesNow, wantRoutes) { t.Fatalf("after becoming not an app connector got routes %v, want %v", routesNow, wantRoutes) }