diff --git a/appc/appconnector.go b/appc/appconnector.go index f2c096177..6f166b227 100644 --- a/appc/appconnector.go +++ b/appc/appconnector.go @@ -94,6 +94,18 @@ func (e *AppConnector) UpdateDomainsAndRoutes(domains []string, routes []netip.P }) } +func (e *AppConnector) RouteInfo() *routeinfo.RouteInfo { + if e.routeInfo == nil { + ret, err := e.routeAdvertiser.ReadRouteInfo() + if err != nil { + e.logf("Unsuccessful Read RouteInfo: ", err) + return routeinfo.NewRouteInfo() + } + return ret + } + return e.routeInfo +} + // UpdateDomains asynchronously replaces the current set of configured domains // with the supplied set of domains. Domains must not contain a trailing dot, // and should be lower case. If the domain contains a leading '*' label it diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 3b105493e..29c68797a 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -3144,6 +3144,21 @@ func (b *LocalBackend) checkFunnelEnabledLocked(p *ipn.Prefs) error { return nil } +func (b *LocalBackend) PatchPrefsHandler(mp *ipn.MaskedPrefs) (ipn.PrefsView, error) { + // we believe that for the purpose of figuring out advertisedRoutes setPrefsLockedOnEntry is _only_ called when + // up or set is used on the tailscale cli _not_ when we calculate the new advertisedRoutes field. + routeInfo := b.appConnector.RouteInfo() + curRoutes := routeInfo.Routes(false, true, true) + + if mp.AdvertiseRoutesSet { + routeInfo.Local = mp.AdvertiseRoutes + b.StoreRouteInfo(routeInfo) + curRoutes := append(curRoutes, mp.AdvertiseRoutes...) + mp.AdvertiseRoutes = curRoutes + } + return b.EditPrefs(mp) +} + func (b *LocalBackend) EditPrefs(mp *ipn.MaskedPrefs) (ipn.PrefsView, error) { b.mu.Lock() if mp.EggSet { @@ -6049,10 +6064,12 @@ func (b *LocalBackend) ReadRouteInfo() (*routeinfo.RouteInfo, error) { } key := namespaceKeyForCurrentProfile(b.pm, routeInfoStateStoreKey) bs, err := b.pm.Store().ReadState(key) - if err != nil { - return nil, err - } ri := &routeinfo.RouteInfo{} + if err != nil && err != ipn.ErrStateNotExist { + return nil, err + } else if err == ipn.ErrStateNotExist { + return ri, nil + } if err := json.Unmarshal(bs, ri); err != nil { return nil, err } diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go index c9001c41c..f45cac1ce 100644 --- a/ipn/ipnlocal/local_test.go +++ b/ipn/ipnlocal/local_test.go @@ -2555,3 +2555,54 @@ func TestReadWriteRouteInfo(t *testing.T) { t.Fatalf("wanted %v, got %v", routes[prefix3], dr.Routes[prefix3]) } } + +func TestPatchPrefsHandler(t *testing.T) { + // test can read what's written + prefix1 := netip.MustParsePrefix("1.2.3.4/32") + prefix2 := netip.MustParsePrefix("1.2.3.5/32") + prefix3 := netip.MustParsePrefix("1.2.3.6/32") + rc := &appctest.RouteCollector{} + testAppConnector := appc.NewAppConnector(t.Logf, rc) + mp := new(ipn.MaskedPrefs) + mp.AdvertiseRoutesSet = true + mp.AdvertiseRoutes = []netip.Prefix{prefix1} + now := time.Now() + ri := routeinfo.NewRouteInfo() + ri.Control = []netip.Prefix{prefix2} + discovered := make(map[string]*routeinfo.DatedRoutes) + routes := make(map[netip.Prefix]time.Time) + routes[prefix3] = now + discovered["example.com"] = &routeinfo.DatedRoutes{ + LastCleanup: now, + Routes: routes, + } + ri.Discovered = discovered + b := newTestBackend(t) + b.reconfigAppConnectorLocked(b.netMap, b.pm.prefs) + if b.appConnector != nil { + t.Fatal("unexpected app connector") + } + b.appConnector = testAppConnector + + prefView, err := b.PatchPrefsHandler(mp) + if err != nil { + t.Fatalf(err.Error()) + } + + if prefView.AdvertiseRoutes().Len() != 3 { + t.Fatalf("wanted %d, got %d", 3, prefView.AdvertiseRoutes().Len()) + } + + if !slices.Contains(prefView.AdvertiseRoutes().AsSlice(), prefix1) { + t.Fatalf("New prefix was not advertised") + } + + if !slices.Contains(prefView.AdvertiseRoutes().AsSlice(), prefix2) || !slices.Contains(prefView.AdvertiseRoutes().AsSlice(), prefix3) { + t.Fatalf("Old prefixes are no longer advertised.") + } + + //TODO: test if route if stored in Appc/Appc.routeAdvertiser + + //TODO: patch again with no route, see if prefix1 is removed/ prefix2, prefix3 presists. + +} diff --git a/ipn/localapi/localapi.go b/ipn/localapi/localapi.go index b6ed30d28..7515e70ff 100644 --- a/ipn/localapi/localapi.go +++ b/ipn/localapi/localapi.go @@ -1376,7 +1376,7 @@ func (h *Handler) servePrefs(w http.ResponseWriter, r *http.Request) { return } var err error - prefs, err = h.b.EditPrefs(mp) + prefs, err = h.b.PatchPrefsHandler(mp) if err != nil { w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusBadRequest)