From 886ffc309616b348c1bbee701407997b2799f1c7 Mon Sep 17 00:00:00 2001 From: Fran Bull Date: Mon, 1 Apr 2024 12:59:21 -0700 Subject: [PATCH] Add the presist storage for control routes --- appc/appconnector.go | 36 +++++++++++++++++++++++++++++++++-- appc/appconnector_test.go | 38 +++++++++++++++++++++++++++++++++++++ appc/appctest/appctest.go | 7 ++++++- appc/routeinfo/routeinfo.go | 37 ++++++++++++++++++++++++++++++++++++ 4 files changed, 115 insertions(+), 3 deletions(-) diff --git a/appc/appconnector.go b/appc/appconnector.go index 75db01217..d0c36a526 100644 --- a/appc/appconnector.go +++ b/appc/appconnector.go @@ -67,6 +67,9 @@ type AppConnector struct { // wildcards is the list of domain strings that match subdomains. wildcards []string + // the in memory copy of all the routes that's advertised + routeInfo *routeinfo.RouteInfo + // queue provides ordering for update operations queue execqueue.ExecQueue @@ -84,6 +87,7 @@ func NewAppConnector(logf logger.Logf, routeAdvertiser RouteAdvertiser, shouldSt logf: logger.WithPrefix(logf, "appc: "), routeAdvertiser: routeAdvertiser, ShouldStoreRoutes: shouldStoreRoutes, + routeInfo: routeinfo.NewRouteInfo(), } } @@ -158,12 +162,37 @@ func (e *AppConnector) updateRoutes(routes []netip.Prefix) { return } + var toRemove []netip.Prefix + var routeInfo *routeinfo.RouteInfo + var err error + if e.ShouldStoreRoutes { + routeInfo, err = e.routeAdvertiser.ReadRouteInfo() + if err != nil { + e.logf("failed to read routeInfo from store") + } + oldControl := routeInfo.Control + oldOtherRoutes := routeInfo.Routes(true, false, true) + for _, ipp := range oldControl { + if slices.Contains(routes, ipp) { + continue + } + // unadvertise the prefix if the prefix is not recorded from other source. + if !slices.Contains(oldOtherRoutes, ipp) { + toRemove = append(toRemove, ipp) + } + } + + if err := e.routeAdvertiser.UnadvertiseRoute(toRemove...); err != nil { + e.logf("failed to unadvertise old routes: %v: %v", routes, err) + } + routeInfo.Control = routes + } if err := e.routeAdvertiser.AdvertiseRoute(routes...); err != nil { e.logf("failed to advertise routes: %v: %v", routes, err) return } - var toRemove []netip.Prefix + toRemove = toRemove[:0] nextRoute: for _, r := range routes { @@ -181,8 +210,11 @@ nextRoute: if err := e.routeAdvertiser.UnadvertiseRoute(toRemove...); err != nil { e.logf("failed to unadvertise routes: %v: %v", toRemove, err) } - e.controlRoutes = routes + if e.ShouldStoreRoutes { + e.routeInfo = routeInfo + e.routeAdvertiser.StoreRouteInfo(e.routeInfo) + } } // Domains returns the currently configured domain list. diff --git a/appc/appconnector_test.go b/appc/appconnector_test.go index f1fec0743..86508e36b 100644 --- a/appc/appconnector_test.go +++ b/appc/appconnector_test.go @@ -13,6 +13,7 @@ import ( xmaps "golang.org/x/exp/maps" "golang.org/x/net/dns/dnsmessage" "tailscale.com/appc/appctest" + "tailscale.com/appc/routeinfo" "tailscale.com/util/mak" "tailscale.com/util/must" ) @@ -68,6 +69,14 @@ func TestUpdateRoutes(t *testing.T) { routes := []netip.Prefix{netip.MustParsePrefix("192.0.2.0/24"), netip.MustParsePrefix("192.0.0.1/32")} a.updateRoutes(routes) + wantRouteInfoControlRoutes := []netip.Prefix{} + if shouldStore { + wantRouteInfoControlRoutes = routes + } + if !slices.Equal(a.routeInfo.Control, wantRouteInfoControlRoutes) { + t.Fatalf("got %v, want %v (shouldStore=%t)", a.routeInfo.Control, wantRouteInfoControlRoutes, shouldStore) + } + slices.SortFunc(rc.Routes(), prefixCompare) rc.SetRoutes(slices.Compact(rc.Routes())) slices.SortFunc(routes, prefixCompare) @@ -85,6 +94,35 @@ func TestUpdateRoutes(t *testing.T) { } } +func TestUpdateRoutesNotUnadvertiseRoutesFromOtherSources(t *testing.T) { + for _, shouldStore := range []bool{true, false} { + rc := &appctest.RouteCollector{} + a := NewAppConnector(t.Logf, rc, shouldStore) + testRi := routeinfo.NewRouteInfo() + a.routeInfo.Local = []netip.Prefix{netip.MustParsePrefix("192.0.2.0/24")} + testRi.Local = append(testRi.Local, netip.MustParsePrefix("192.0.2.0/24")) + rc.StoreRouteInfo(testRi) + + routes := []netip.Prefix{netip.MustParsePrefix("192.0.2.0/24"), netip.MustParsePrefix("192.0.0.1/32")} + a.updateRoutes(routes) + wantRouteInfoControlRoutes := []netip.Prefix{} + if shouldStore { + wantRouteInfoControlRoutes = routes + } + if !slices.Equal(a.routeInfo.Control, wantRouteInfoControlRoutes) { + t.Fatalf("got %v, want %v (shouldStore=%t)", a.routeInfo.Control, wantRouteInfoControlRoutes, shouldStore) + } + + routes2 := []netip.Prefix{netip.MustParsePrefix("192.0.0.1/32")} + a.updateRoutes(routes2) + + wantRemoved := []netip.Prefix{} + if !slices.EqualFunc(rc.RemovedRoutes(), wantRemoved, prefixEqual) { + t.Fatalf("unexpected removed routes: %v", rc.RemovedRoutes()) + } + } +} + func TestUpdateRoutesUnadvertisesContainedRoutes(t *testing.T) { for _, shouldStore := range []bool{true, false} { rc := &appctest.RouteCollector{} diff --git a/appc/appctest/appctest.go b/appc/appctest/appctest.go index aaf2e4595..d513688f3 100644 --- a/appc/appctest/appctest.go +++ b/appc/appctest/appctest.go @@ -14,6 +14,7 @@ import ( type RouteCollector struct { routes []netip.Prefix removedRoutes []netip.Prefix + routeInfo *routeinfo.RouteInfo } func (rc *RouteCollector) AdvertiseRoute(pfx ...netip.Prefix) error { @@ -35,11 +36,15 @@ func (rc *RouteCollector) UnadvertiseRoute(toRemove ...netip.Prefix) error { } func (rc *RouteCollector) StoreRouteInfo(ri *routeinfo.RouteInfo) error { + rc.routeInfo = ri return nil } func (rc *RouteCollector) ReadRouteInfo() (*routeinfo.RouteInfo, error) { - return nil, nil + if rc.routeInfo == nil { + return routeinfo.NewRouteInfo(), nil + } + return rc.routeInfo, nil } // RemovedRoutes returns the list of routes that were removed. diff --git a/appc/routeinfo/routeinfo.go b/appc/routeinfo/routeinfo.go index 8858ca771..416bdc4e9 100644 --- a/appc/routeinfo/routeinfo.go +++ b/appc/routeinfo/routeinfo.go @@ -17,9 +17,46 @@ type RouteInfo struct { Discovered map[string]*DatedRoutes } +func NewRouteInfo() *RouteInfo { + discovered := make(map[string]*DatedRoutes) + return &RouteInfo{ + Local: []netip.Prefix{}, + Control: []netip.Prefix{}, + Discovered: discovered, + } +} + +// RouteInfo.Routes returns a slice containing all the routes stored from the wanted resources. +func (ri *RouteInfo) Routes(local, control, discovered bool) []netip.Prefix { + var ret []netip.Prefix + if local { + ret = ri.Local + } + if control && len(ret) == 0 { + ret = ri.Control + } else if control { + ret = append(ret, ri.Control...) + } + + if discovered { + for _, dr := range ri.Discovered { + ret = append(ret, dr.routesSlice()...) + } + } + return ret +} + type DatedRoutes struct { // routes discovered for a domain, and when they were last seen in a dns query Routes map[netip.Prefix]time.Time // the time at which we last expired old routes LastCleanup time.Time } + +func (dr *DatedRoutes) routesSlice() []netip.Prefix { + var routes []netip.Prefix + for k := range dr.Routes { + routes = append(routes, k) + } + return routes +}