From c9eb5798c52529c7850b19a427434540593a2d61 Mon Sep 17 00:00:00 2001 From: Fran Bull Date: Fri, 29 Mar 2024 14:28:35 -0700 Subject: [PATCH] Add a control knob to toggle writing RouteInfo to StateStore To control the toggle in dev you can a) add a go workspace so that control is using the new tailcfg from this commit and b) add the feature flag to control. --- appc/appconnector.go | 14 +- appc/appconnector_test.go | 294 ++++++++++++------------ control/controlknobs/controlknobs.go | 7 + ipn/ipnlocal/local.go | 10 +- ipn/ipnlocal/local_test.go | 42 ++-- ipn/ipnlocal/peerapi_test.go | 320 ++++++++++++++------------- tailcfg/tailcfg.go | 5 + 7 files changed, 370 insertions(+), 322 deletions(-) diff --git a/appc/appconnector.go b/appc/appconnector.go index b96800f53..75db01217 100644 --- a/appc/appconnector.go +++ b/appc/appconnector.go @@ -69,13 +69,21 @@ type AppConnector struct { // queue provides ordering for update operations queue execqueue.ExecQueue + + // whether this tailscaled should persist routes. Storing RouteInfo enables the app connector + // to forget routes when appropriate and should make routes smaller. While we are verifying that + // writing the RouteInfo to StateStore is a good solution (and doesn't for example cause writes + // that are too frequent or too large) use a controlknob to manage this flag. + ShouldStoreRoutes bool } // NewAppConnector creates a new AppConnector. -func NewAppConnector(logf logger.Logf, routeAdvertiser RouteAdvertiser) *AppConnector { +func NewAppConnector(logf logger.Logf, routeAdvertiser RouteAdvertiser, shouldStoreRoutes bool) *AppConnector { + // TODO(fran) if !shouldStoreRoutes we probably want to try and clean up any stored routes return &AppConnector{ - logf: logger.WithPrefix(logf, "appc: "), - routeAdvertiser: routeAdvertiser, + logf: logger.WithPrefix(logf, "appc: "), + routeAdvertiser: routeAdvertiser, + ShouldStoreRoutes: shouldStoreRoutes, } } diff --git a/appc/appconnector_test.go b/appc/appconnector_test.go index 5b739b097..f1fec0743 100644 --- a/appc/appconnector_test.go +++ b/appc/appconnector_test.go @@ -18,193 +18,205 @@ import ( ) func TestUpdateDomains(t *testing.T) { - ctx := context.Background() - a := NewAppConnector(t.Logf, nil) - a.UpdateDomains([]string{"example.com"}) + for _, shouldStore := range []bool{true, false} { + ctx := context.Background() + a := NewAppConnector(t.Logf, nil, shouldStore) + a.UpdateDomains([]string{"example.com"}) - a.Wait(ctx) - if got, want := a.Domains().AsSlice(), []string{"example.com"}; !slices.Equal(got, want) { - t.Errorf("got %v; want %v", got, want) - } + a.Wait(ctx) + if got, want := a.Domains().AsSlice(), []string{"example.com"}; !slices.Equal(got, want) { + t.Errorf("got %v; want %v", got, want) + } - addr := netip.MustParseAddr("192.0.0.8") - a.domains["example.com"] = append(a.domains["example.com"], addr) - a.UpdateDomains([]string{"example.com"}) - a.Wait(ctx) + addr := netip.MustParseAddr("192.0.0.8") + a.domains["example.com"] = append(a.domains["example.com"], addr) + a.UpdateDomains([]string{"example.com"}) + a.Wait(ctx) - if got, want := a.domains["example.com"], []netip.Addr{addr}; !slices.Equal(got, want) { - t.Errorf("got %v; want %v", got, want) - } + if got, want := a.domains["example.com"], []netip.Addr{addr}; !slices.Equal(got, want) { + t.Errorf("got %v; want %v", got, want) + } - // domains are explicitly downcased on set. - a.UpdateDomains([]string{"UP.EXAMPLE.COM"}) - a.Wait(ctx) - if got, want := xmaps.Keys(a.domains), []string{"up.example.com"}; !slices.Equal(got, want) { - t.Errorf("got %v; want %v", got, want) + // domains are explicitly downcased on set. + a.UpdateDomains([]string{"UP.EXAMPLE.COM"}) + a.Wait(ctx) + if got, want := xmaps.Keys(a.domains), []string{"up.example.com"}; !slices.Equal(got, want) { + t.Errorf("got %v; want %v", got, want) + } } } func TestUpdateRoutes(t *testing.T) { - ctx := context.Background() - rc := &appctest.RouteCollector{} - a := NewAppConnector(t.Logf, rc) - a.updateDomains([]string{"*.example.com"}) + for _, shouldStore := range []bool{true, false} { + ctx := context.Background() + rc := &appctest.RouteCollector{} + a := NewAppConnector(t.Logf, rc, shouldStore) + 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) + // 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")}) - } + 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) + // 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) + 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) + 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.Errorf("added routes: got %v, want %v", rc.Routes(), routes) - } + // Ensure that the non-matching /32 is preserved, even though it's in the domains table. + if !slices.EqualFunc(routes, rc.Routes(), prefixEqual) { + 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()) + // 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()) + } } } func TestUpdateRoutesUnadvertisesContainedRoutes(t *testing.T) { - rc := &appctest.RouteCollector{} - a := NewAppConnector(t.Logf, rc) - mak.Set(&a.domains, "example.com", []netip.Addr{netip.MustParseAddr("192.0.2.1")}) - rc.SetRoutes([]netip.Prefix{netip.MustParsePrefix("192.0.2.1/32")}) - routes := []netip.Prefix{netip.MustParsePrefix("192.0.2.0/24")} - a.updateRoutes(routes) + for _, shouldStore := range []bool{true, false} { + rc := &appctest.RouteCollector{} + a := NewAppConnector(t.Logf, rc, shouldStore) + mak.Set(&a.domains, "example.com", []netip.Addr{netip.MustParseAddr("192.0.2.1")}) + rc.SetRoutes([]netip.Prefix{netip.MustParsePrefix("192.0.2.1/32")}) + routes := []netip.Prefix{netip.MustParsePrefix("192.0.2.0/24")} + a.updateRoutes(routes) - if !slices.EqualFunc(routes, rc.Routes(), prefixEqual) { - t.Fatalf("got %v, want %v", rc.Routes(), routes) + if !slices.EqualFunc(routes, rc.Routes(), prefixEqual) { + t.Fatalf("got %v, want %v", rc.Routes(), routes) + } } } func TestDomainRoutes(t *testing.T) { - rc := &appctest.RouteCollector{} - a := NewAppConnector(t.Logf, rc) - a.updateDomains([]string{"example.com"}) - a.ObserveDNSResponse(dnsResponse("example.com.", "192.0.0.8")) - a.Wait(context.Background()) + for _, shouldStore := range []bool{true, false} { + rc := &appctest.RouteCollector{} + a := NewAppConnector(t.Logf, rc, shouldStore) + a.updateDomains([]string{"example.com"}) + a.ObserveDNSResponse(dnsResponse("example.com.", "192.0.0.8")) + a.Wait(context.Background()) - want := map[string][]netip.Addr{ - "example.com": {netip.MustParseAddr("192.0.0.8")}, - } + want := map[string][]netip.Addr{ + "example.com": {netip.MustParseAddr("192.0.0.8")}, + } - if got := a.DomainRoutes(); !reflect.DeepEqual(got, want) { - t.Fatalf("DomainRoutes: got %v, want %v", got, want) + if got := a.DomainRoutes(); !reflect.DeepEqual(got, want) { + t.Fatalf("DomainRoutes: got %v, want %v", got, want) + } } } func TestObserveDNSResponse(t *testing.T) { - ctx := context.Background() - rc := &appctest.RouteCollector{} - a := NewAppConnector(t.Logf, rc) + for _, shouldStore := range []bool{true, false} { + ctx := context.Background() + rc := &appctest.RouteCollector{} + a := NewAppConnector(t.Logf, rc, shouldStore) - // a has no domains configured, so it should not advertise any routes - a.ObserveDNSResponse(dnsResponse("example.com.", "192.0.0.8")) - if got, want := rc.Routes(), ([]netip.Prefix)(nil); !slices.Equal(got, want) { - t.Errorf("got %v; want %v", got, want) - } + // a has no domains configured, so it should not advertise any routes + a.ObserveDNSResponse(dnsResponse("example.com.", "192.0.0.8")) + if got, want := rc.Routes(), ([]netip.Prefix)(nil); !slices.Equal(got, want) { + t.Errorf("got %v; want %v", got, want) + } - wantRoutes := []netip.Prefix{netip.MustParsePrefix("192.0.0.8/32")} + wantRoutes := []netip.Prefix{netip.MustParsePrefix("192.0.0.8/32")} - a.updateDomains([]string{"example.com"}) - a.ObserveDNSResponse(dnsResponse("example.com.", "192.0.0.8")) - a.Wait(ctx) - if got, want := rc.Routes(), wantRoutes; !slices.Equal(got, want) { - t.Errorf("got %v; want %v", got, want) - } + a.updateDomains([]string{"example.com"}) + a.ObserveDNSResponse(dnsResponse("example.com.", "192.0.0.8")) + a.Wait(ctx) + if got, want := rc.Routes(), wantRoutes; !slices.Equal(got, want) { + t.Errorf("got %v; want %v", got, want) + } - // a CNAME record chain should result in a route being added if the chain - // matches a routed domain. - a.updateDomains([]string{"www.example.com", "example.com"}) - a.ObserveDNSResponse(dnsCNAMEResponse("192.0.0.9", "www.example.com.", "chain.example.com.", "example.com.")) - a.Wait(ctx) - wantRoutes = append(wantRoutes, netip.MustParsePrefix("192.0.0.9/32")) - if got, want := rc.Routes(), wantRoutes; !slices.Equal(got, want) { - t.Errorf("got %v; want %v", got, want) - } + // a CNAME record chain should result in a route being added if the chain + // matches a routed domain. + a.updateDomains([]string{"www.example.com", "example.com"}) + a.ObserveDNSResponse(dnsCNAMEResponse("192.0.0.9", "www.example.com.", "chain.example.com.", "example.com.")) + a.Wait(ctx) + wantRoutes = append(wantRoutes, netip.MustParsePrefix("192.0.0.9/32")) + if got, want := rc.Routes(), wantRoutes; !slices.Equal(got, want) { + t.Errorf("got %v; want %v", got, want) + } - // a CNAME record chain should result in a route being added if the chain - // even if only found in the middle of the chain - a.ObserveDNSResponse(dnsCNAMEResponse("192.0.0.10", "outside.example.org.", "www.example.com.", "example.org.")) - a.Wait(ctx) - wantRoutes = append(wantRoutes, netip.MustParsePrefix("192.0.0.10/32")) - if got, want := rc.Routes(), wantRoutes; !slices.Equal(got, want) { - t.Errorf("got %v; want %v", got, want) - } + // a CNAME record chain should result in a route being added if the chain + // even if only found in the middle of the chain + a.ObserveDNSResponse(dnsCNAMEResponse("192.0.0.10", "outside.example.org.", "www.example.com.", "example.org.")) + a.Wait(ctx) + wantRoutes = append(wantRoutes, netip.MustParsePrefix("192.0.0.10/32")) + if got, want := rc.Routes(), wantRoutes; !slices.Equal(got, want) { + t.Errorf("got %v; want %v", got, want) + } - wantRoutes = append(wantRoutes, netip.MustParsePrefix("2001:db8::1/128")) + wantRoutes = append(wantRoutes, netip.MustParsePrefix("2001:db8::1/128")) - a.ObserveDNSResponse(dnsResponse("example.com.", "2001:db8::1")) - a.Wait(ctx) - if got, want := rc.Routes(), wantRoutes; !slices.Equal(got, want) { - t.Errorf("got %v; want %v", got, want) - } + a.ObserveDNSResponse(dnsResponse("example.com.", "2001:db8::1")) + a.Wait(ctx) + if got, want := rc.Routes(), wantRoutes; !slices.Equal(got, want) { + t.Errorf("got %v; want %v", got, want) + } - // don't re-advertise routes that have already been advertised - a.ObserveDNSResponse(dnsResponse("example.com.", "2001:db8::1")) - a.Wait(ctx) - if !slices.Equal(rc.Routes(), wantRoutes) { - t.Errorf("rc.Routes(): got %v; want %v", rc.Routes(), wantRoutes) - } + // don't re-advertise routes that have already been advertised + a.ObserveDNSResponse(dnsResponse("example.com.", "2001:db8::1")) + a.Wait(ctx) + if !slices.Equal(rc.Routes(), wantRoutes) { + t.Errorf("rc.Routes(): got %v; want %v", rc.Routes(), wantRoutes) + } - // don't advertise addresses that are already in a control provided route - pfx := netip.MustParsePrefix("192.0.2.0/24") - a.updateRoutes([]netip.Prefix{pfx}) - wantRoutes = append(wantRoutes, pfx) - a.ObserveDNSResponse(dnsResponse("example.com.", "192.0.2.1")) - a.Wait(ctx) - if !slices.Equal(rc.Routes(), wantRoutes) { - t.Errorf("rc.Routes(): got %v; want %v", rc.Routes(), wantRoutes) - } - if !slices.Contains(a.domains["example.com"], netip.MustParseAddr("192.0.2.1")) { - t.Errorf("missing %v from %v", "192.0.2.1", a.domains["exmaple.com"]) + // don't advertise addresses that are already in a control provided route + pfx := netip.MustParsePrefix("192.0.2.0/24") + a.updateRoutes([]netip.Prefix{pfx}) + wantRoutes = append(wantRoutes, pfx) + a.ObserveDNSResponse(dnsResponse("example.com.", "192.0.2.1")) + a.Wait(ctx) + if !slices.Equal(rc.Routes(), wantRoutes) { + t.Errorf("rc.Routes(): got %v; want %v", rc.Routes(), wantRoutes) + } + if !slices.Contains(a.domains["example.com"], netip.MustParseAddr("192.0.2.1")) { + t.Errorf("missing %v from %v", "192.0.2.1", a.domains["exmaple.com"]) + } } } func TestWildcardDomains(t *testing.T) { - ctx := context.Background() - rc := &appctest.RouteCollector{} - a := NewAppConnector(t.Logf, rc) + for _, shouldStore := range []bool{true, false} { + ctx := context.Background() + rc := &appctest.RouteCollector{} + a := NewAppConnector(t.Logf, rc, shouldStore) - a.updateDomains([]string{"*.example.com"}) - a.ObserveDNSResponse(dnsResponse("foo.example.com.", "192.0.0.8")) - a.Wait(ctx) - if got, want := rc.Routes(), []netip.Prefix{netip.MustParsePrefix("192.0.0.8/32")}; !slices.Equal(got, want) { - t.Errorf("routes: got %v; want %v", got, want) - } - if got, want := a.wildcards, []string{"example.com"}; !slices.Equal(got, want) { - t.Errorf("wildcards: got %v; want %v", got, want) - } + a.updateDomains([]string{"*.example.com"}) + a.ObserveDNSResponse(dnsResponse("foo.example.com.", "192.0.0.8")) + a.Wait(ctx) + if got, want := rc.Routes(), []netip.Prefix{netip.MustParsePrefix("192.0.0.8/32")}; !slices.Equal(got, want) { + t.Errorf("routes: got %v; want %v", got, want) + } + if got, want := a.wildcards, []string{"example.com"}; !slices.Equal(got, want) { + t.Errorf("wildcards: got %v; want %v", got, want) + } - a.updateDomains([]string{"*.example.com", "example.com"}) - if _, ok := a.domains["foo.example.com"]; !ok { - t.Errorf("expected foo.example.com to be preserved in domains due to wildcard") - } - if got, want := a.wildcards, []string{"example.com"}; !slices.Equal(got, want) { - t.Errorf("wildcards: got %v; want %v", got, want) - } + a.updateDomains([]string{"*.example.com", "example.com"}) + if _, ok := a.domains["foo.example.com"]; !ok { + t.Errorf("expected foo.example.com to be preserved in domains due to wildcard") + } + if got, want := a.wildcards, []string{"example.com"}; !slices.Equal(got, want) { + t.Errorf("wildcards: got %v; want %v", got, want) + } - // There was an early regression where the wildcard domain was added repeatedly, this guards against that. - a.updateDomains([]string{"*.example.com", "example.com"}) - if len(a.wildcards) != 1 { - t.Errorf("expected only one wildcard domain, got %v", a.wildcards) + // There was an early regression where the wildcard domain was added repeatedly, this guards against that. + a.updateDomains([]string{"*.example.com", "example.com"}) + if len(a.wildcards) != 1 { + t.Errorf("expected only one wildcard domain, got %v", a.wildcards) + } } } diff --git a/control/controlknobs/controlknobs.go b/control/controlknobs/controlknobs.go index 6e3a62967..2f80ba38c 100644 --- a/control/controlknobs/controlknobs.go +++ b/control/controlknobs/controlknobs.go @@ -72,6 +72,10 @@ type Knobs struct { // ProbeUDPLifetime is whether the node should probe UDP path lifetime on // the tail end of an active direct connection in magicsock. ProbeUDPLifetime atomic.Bool + + // AppCStoreRoutes is whether the node should store RouteInfo to StateStore + // if it's an app connector. + AppCStoreRoutes atomic.Bool } // UpdateFromNodeAttributes updates k (if non-nil) based on the provided self @@ -96,6 +100,7 @@ func (k *Knobs) UpdateFromNodeAttributes(capMap tailcfg.NodeCapMap) { forceNfTables = has(tailcfg.NodeAttrLinuxMustUseNfTables) seamlessKeyRenewal = has(tailcfg.NodeAttrSeamlessKeyRenewal) probeUDPLifetime = has(tailcfg.NodeAttrProbeUDPLifetime) + appCStoreRoutes = has(tailcfg.NodeAttrStoreAppCRoutes) ) if has(tailcfg.NodeAttrOneCGNATEnable) { @@ -118,6 +123,7 @@ func (k *Knobs) UpdateFromNodeAttributes(capMap tailcfg.NodeCapMap) { k.LinuxForceNfTables.Store(forceNfTables) k.SeamlessKeyRenewal.Store(seamlessKeyRenewal) k.ProbeUDPLifetime.Store(probeUDPLifetime) + k.AppCStoreRoutes.Store(appCStoreRoutes) } // AsDebugJSON returns k as something that can be marshalled with json.Marshal @@ -141,5 +147,6 @@ func (k *Knobs) AsDebugJSON() map[string]any { "LinuxForceNfTables": k.LinuxForceNfTables.Load(), "SeamlessKeyRenewal": k.SeamlessKeyRenewal.Load(), "ProbeUDPLifetime": k.ProbeUDPLifetime.Load(), + "AppCStoreRoutes": k.AppCStoreRoutes.Load(), } } diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 9cde9acab..d2f229167 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -3561,8 +3561,14 @@ func (b *LocalBackend) reconfigAppConnectorLocked(nm *netmap.NetworkMap, prefs i return } - if b.appConnector == nil { - b.appConnector = appc.NewAppConnector(b.logf, b) + shouldAppCStoreRoutesHasChanged := false + shouldAppCStoreRoutes := b.ControlKnobs().AppCStoreRoutes.Load() + if b.appConnector != nil { + shouldAppCStoreRoutesHasChanged = b.appConnector.ShouldStoreRoutes != shouldAppCStoreRoutes + } + + if b.appConnector == nil || shouldAppCStoreRoutesHasChanged { + b.appConnector = appc.NewAppConnector(b.logf, b, shouldAppCStoreRoutes) } if nm == nil { return diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go index b27267c62..57c336bad 100644 --- a/ipn/ipnlocal/local_test.go +++ b/ipn/ipnlocal/local_test.go @@ -1253,13 +1253,15 @@ func TestDNSConfigForNetmapForExitNodeConfigs(t *testing.T) { } func TestOfferingAppConnector(t *testing.T) { - b := newTestBackend(t) - if b.OfferingAppConnector() { - t.Fatal("unexpected offering app connector") - } - b.appConnector = appc.NewAppConnector(t.Logf, nil) - if !b.OfferingAppConnector() { - t.Fatal("unexpected not offering app connector") + for _, shouldStore := range []bool{true, false} { + b := newTestBackend(t) + if b.OfferingAppConnector() { + t.Fatal("unexpected offering app connector") + } + b.appConnector = appc.NewAppConnector(t.Logf, nil, shouldStore) + if !b.OfferingAppConnector() { + t.Fatal("unexpected not offering app connector") + } } } @@ -1304,21 +1306,23 @@ func TestRouterAdvertiserIgnoresContainedRoutes(t *testing.T) { } func TestObserveDNSResponse(t *testing.T) { - b := newTestBackend(t) + for _, shouldStore := range []bool{true, false} { + b := newTestBackend(t) - // ensure no error when no app connector is configured - b.ObserveDNSResponse(dnsResponse("example.com.", "192.0.0.8")) + // ensure no error when no app connector is configured + b.ObserveDNSResponse(dnsResponse("example.com.", "192.0.0.8")) - rc := &appctest.RouteCollector{} - b.appConnector = appc.NewAppConnector(t.Logf, rc) - b.appConnector.UpdateDomains([]string{"example.com"}) - b.appConnector.Wait(context.Background()) + rc := &appctest.RouteCollector{} + b.appConnector = appc.NewAppConnector(t.Logf, rc, shouldStore) + b.appConnector.UpdateDomains([]string{"example.com"}) + b.appConnector.Wait(context.Background()) - b.ObserveDNSResponse(dnsResponse("example.com.", "192.0.0.8")) - b.appConnector.Wait(context.Background()) - wantRoutes := []netip.Prefix{netip.MustParsePrefix("192.0.0.8/32")} - if !slices.Equal(rc.Routes(), wantRoutes) { - t.Fatalf("got routes %v, want %v", rc.Routes(), wantRoutes) + b.ObserveDNSResponse(dnsResponse("example.com.", "192.0.0.8")) + b.appConnector.Wait(context.Background()) + wantRoutes := []netip.Prefix{netip.MustParsePrefix("192.0.0.8/32")} + if !slices.Equal(rc.Routes(), wantRoutes) { + t.Fatalf("got routes %v, want %v", rc.Routes(), wantRoutes) + } } } diff --git a/ipn/ipnlocal/peerapi_test.go b/ipn/ipnlocal/peerapi_test.go index 435ce5017..7d50023fd 100644 --- a/ipn/ipnlocal/peerapi_test.go +++ b/ipn/ipnlocal/peerapi_test.go @@ -687,185 +687,191 @@ func TestPeerAPIReplyToDNSQueries(t *testing.T) { } func TestPeerAPIPrettyReplyCNAME(t *testing.T) { - var h peerAPIHandler - h.remoteAddr = netip.MustParseAddrPort("100.150.151.152:12345") + for _, shouldStore := range []bool{true, false} { + var h peerAPIHandler + h.remoteAddr = netip.MustParseAddrPort("100.150.151.152:12345") - eng, _ := wgengine.NewFakeUserspaceEngine(logger.Discard, 0) - pm := must.Get(newProfileManager(new(mem.Store), t.Logf)) - h.ps = &peerAPIServer{ - b: &LocalBackend{ - e: eng, - pm: pm, - store: pm.Store(), - // configure as an app connector just to enable the API. - appConnector: appc.NewAppConnector(t.Logf, &appctest.RouteCollector{}), - }, - } + eng, _ := wgengine.NewFakeUserspaceEngine(logger.Discard, 0) + pm := must.Get(newProfileManager(new(mem.Store), t.Logf)) + h.ps = &peerAPIServer{ + b: &LocalBackend{ + e: eng, + pm: pm, + store: pm.Store(), + // configure as an app connector just to enable the API. + appConnector: appc.NewAppConnector(t.Logf, &appctest.RouteCollector{}, shouldStore), + }, + } - h.ps.resolver = &fakeResolver{build: func(b *dnsmessage.Builder) { - b.CNAMEResource( - dnsmessage.ResourceHeader{ - Name: dnsmessage.MustNewName("www.example.com."), - Type: dnsmessage.TypeCNAME, - Class: dnsmessage.ClassINET, - TTL: 0, - }, - dnsmessage.CNAMEResource{ - CNAME: dnsmessage.MustNewName("example.com."), - }, - ) - b.AResource( - dnsmessage.ResourceHeader{ - Name: dnsmessage.MustNewName("example.com."), - Type: dnsmessage.TypeA, - Class: dnsmessage.ClassINET, - TTL: 0, - }, - dnsmessage.AResource{ - A: [4]byte{192, 0, 0, 8}, - }, - ) - }} - f := filter.NewAllowAllForTest(logger.Discard) - h.ps.b.setFilter(f) + h.ps.resolver = &fakeResolver{build: func(b *dnsmessage.Builder) { + b.CNAMEResource( + dnsmessage.ResourceHeader{ + Name: dnsmessage.MustNewName("www.example.com."), + Type: dnsmessage.TypeCNAME, + Class: dnsmessage.ClassINET, + TTL: 0, + }, + dnsmessage.CNAMEResource{ + CNAME: dnsmessage.MustNewName("example.com."), + }, + ) + b.AResource( + dnsmessage.ResourceHeader{ + Name: dnsmessage.MustNewName("example.com."), + Type: dnsmessage.TypeA, + Class: dnsmessage.ClassINET, + TTL: 0, + }, + dnsmessage.AResource{ + A: [4]byte{192, 0, 0, 8}, + }, + ) + }} + f := filter.NewAllowAllForTest(logger.Discard) + h.ps.b.setFilter(f) - if !h.replyToDNSQueries() { - t.Errorf("unexpectedly deny; wanted to be a DNS server") - } + if !h.replyToDNSQueries() { + t.Errorf("unexpectedly deny; wanted to be a DNS server") + } - w := httptest.NewRecorder() - h.handleDNSQuery(w, httptest.NewRequest("GET", "/dns-query?q=www.example.com.", nil)) - if w.Code != http.StatusOK { - t.Errorf("unexpected status code: %v", w.Code) - } - var addrs []string - json.NewDecoder(w.Body).Decode(&addrs) - if len(addrs) == 0 { - t.Fatalf("no addresses returned") - } - for _, addr := range addrs { - netip.MustParseAddr(addr) + w := httptest.NewRecorder() + h.handleDNSQuery(w, httptest.NewRequest("GET", "/dns-query?q=www.example.com.", nil)) + if w.Code != http.StatusOK { + t.Errorf("unexpected status code: %v", w.Code) + } + var addrs []string + json.NewDecoder(w.Body).Decode(&addrs) + if len(addrs) == 0 { + t.Fatalf("no addresses returned") + } + for _, addr := range addrs { + netip.MustParseAddr(addr) + } } } func TestPeerAPIReplyToDNSQueriesAreObserved(t *testing.T) { - ctx := context.Background() - var h peerAPIHandler - h.remoteAddr = netip.MustParseAddrPort("100.150.151.152:12345") + for _, shouldStore := range []bool{true, false} { + ctx := context.Background() + var h peerAPIHandler + h.remoteAddr = netip.MustParseAddrPort("100.150.151.152:12345") - rc := &appctest.RouteCollector{} - eng, _ := wgengine.NewFakeUserspaceEngine(logger.Discard, 0) - pm := must.Get(newProfileManager(new(mem.Store), t.Logf)) - h.ps = &peerAPIServer{ - b: &LocalBackend{ - e: eng, - pm: pm, - store: pm.Store(), - appConnector: appc.NewAppConnector(t.Logf, rc), - }, - } - h.ps.b.appConnector.UpdateDomains([]string{"example.com"}) - h.ps.b.appConnector.Wait(ctx) - - h.ps.resolver = &fakeResolver{build: func(b *dnsmessage.Builder) { - b.AResource( - dnsmessage.ResourceHeader{ - Name: dnsmessage.MustNewName("example.com."), - Type: dnsmessage.TypeA, - Class: dnsmessage.ClassINET, - TTL: 0, + rc := &appctest.RouteCollector{} + eng, _ := wgengine.NewFakeUserspaceEngine(logger.Discard, 0) + pm := must.Get(newProfileManager(new(mem.Store), t.Logf)) + h.ps = &peerAPIServer{ + b: &LocalBackend{ + e: eng, + pm: pm, + store: pm.Store(), + appConnector: appc.NewAppConnector(t.Logf, rc, shouldStore), }, - dnsmessage.AResource{ - A: [4]byte{192, 0, 0, 8}, - }, - ) - }} - f := filter.NewAllowAllForTest(logger.Discard) - h.ps.b.setFilter(f) + } + h.ps.b.appConnector.UpdateDomains([]string{"example.com"}) + h.ps.b.appConnector.Wait(ctx) - if !h.ps.b.OfferingAppConnector() { - t.Fatal("expecting to be offering app connector") - } - if !h.replyToDNSQueries() { - t.Errorf("unexpectedly deny; wanted to be a DNS server") - } + h.ps.resolver = &fakeResolver{build: func(b *dnsmessage.Builder) { + b.AResource( + dnsmessage.ResourceHeader{ + Name: dnsmessage.MustNewName("example.com."), + Type: dnsmessage.TypeA, + Class: dnsmessage.ClassINET, + TTL: 0, + }, + dnsmessage.AResource{ + A: [4]byte{192, 0, 0, 8}, + }, + ) + }} + f := filter.NewAllowAllForTest(logger.Discard) + h.ps.b.setFilter(f) - w := httptest.NewRecorder() - h.handleDNSQuery(w, httptest.NewRequest("GET", "/dns-query?q=example.com.", nil)) - if w.Code != http.StatusOK { - t.Errorf("unexpected status code: %v", w.Code) - } - h.ps.b.appConnector.Wait(ctx) + if !h.ps.b.OfferingAppConnector() { + t.Fatal("expecting to be offering app connector") + } + if !h.replyToDNSQueries() { + t.Errorf("unexpectedly deny; wanted to be a DNS server") + } - wantRoutes := []netip.Prefix{netip.MustParsePrefix("192.0.0.8/32")} - if !slices.Equal(rc.Routes(), wantRoutes) { - t.Errorf("got %v; want %v", rc.Routes(), wantRoutes) + w := httptest.NewRecorder() + h.handleDNSQuery(w, httptest.NewRequest("GET", "/dns-query?q=example.com.", nil)) + if w.Code != http.StatusOK { + t.Errorf("unexpected status code: %v", w.Code) + } + h.ps.b.appConnector.Wait(ctx) + + wantRoutes := []netip.Prefix{netip.MustParsePrefix("192.0.0.8/32")} + if !slices.Equal(rc.Routes(), wantRoutes) { + t.Errorf("got %v; want %v", rc.Routes(), wantRoutes) + } } } func TestPeerAPIReplyToDNSQueriesAreObservedWithCNAMEFlattening(t *testing.T) { - ctx := context.Background() - var h peerAPIHandler - h.remoteAddr = netip.MustParseAddrPort("100.150.151.152:12345") + for _, shouldStore := range []bool{true, false} { + ctx := context.Background() + var h peerAPIHandler + h.remoteAddr = netip.MustParseAddrPort("100.150.151.152:12345") - rc := &appctest.RouteCollector{} - eng, _ := wgengine.NewFakeUserspaceEngine(logger.Discard, 0) - pm := must.Get(newProfileManager(new(mem.Store), t.Logf)) - h.ps = &peerAPIServer{ - b: &LocalBackend{ - e: eng, - pm: pm, - store: pm.Store(), - appConnector: appc.NewAppConnector(t.Logf, rc), - }, - } - h.ps.b.appConnector.UpdateDomains([]string{"www.example.com"}) - h.ps.b.appConnector.Wait(ctx) - - h.ps.resolver = &fakeResolver{build: func(b *dnsmessage.Builder) { - b.CNAMEResource( - dnsmessage.ResourceHeader{ - Name: dnsmessage.MustNewName("www.example.com."), - Type: dnsmessage.TypeCNAME, - Class: dnsmessage.ClassINET, - TTL: 0, + rc := &appctest.RouteCollector{} + eng, _ := wgengine.NewFakeUserspaceEngine(logger.Discard, 0) + pm := must.Get(newProfileManager(new(mem.Store), t.Logf)) + h.ps = &peerAPIServer{ + b: &LocalBackend{ + e: eng, + pm: pm, + store: pm.Store(), + appConnector: appc.NewAppConnector(t.Logf, rc, shouldStore), }, - dnsmessage.CNAMEResource{ - CNAME: dnsmessage.MustNewName("example.com."), - }, - ) - b.AResource( - dnsmessage.ResourceHeader{ - Name: dnsmessage.MustNewName("example.com."), - Type: dnsmessage.TypeA, - Class: dnsmessage.ClassINET, - TTL: 0, - }, - dnsmessage.AResource{ - A: [4]byte{192, 0, 0, 8}, - }, - ) - }} - f := filter.NewAllowAllForTest(logger.Discard) - h.ps.b.setFilter(f) + } + h.ps.b.appConnector.UpdateDomains([]string{"www.example.com"}) + h.ps.b.appConnector.Wait(ctx) - if !h.ps.b.OfferingAppConnector() { - t.Fatal("expecting to be offering app connector") - } - if !h.replyToDNSQueries() { - t.Errorf("unexpectedly deny; wanted to be a DNS server") - } + h.ps.resolver = &fakeResolver{build: func(b *dnsmessage.Builder) { + b.CNAMEResource( + dnsmessage.ResourceHeader{ + Name: dnsmessage.MustNewName("www.example.com."), + Type: dnsmessage.TypeCNAME, + Class: dnsmessage.ClassINET, + TTL: 0, + }, + dnsmessage.CNAMEResource{ + CNAME: dnsmessage.MustNewName("example.com."), + }, + ) + b.AResource( + dnsmessage.ResourceHeader{ + Name: dnsmessage.MustNewName("example.com."), + Type: dnsmessage.TypeA, + Class: dnsmessage.ClassINET, + TTL: 0, + }, + dnsmessage.AResource{ + A: [4]byte{192, 0, 0, 8}, + }, + ) + }} + f := filter.NewAllowAllForTest(logger.Discard) + h.ps.b.setFilter(f) - w := httptest.NewRecorder() - h.handleDNSQuery(w, httptest.NewRequest("GET", "/dns-query?q=www.example.com.", nil)) - if w.Code != http.StatusOK { - t.Errorf("unexpected status code: %v", w.Code) - } - h.ps.b.appConnector.Wait(ctx) + if !h.ps.b.OfferingAppConnector() { + t.Fatal("expecting to be offering app connector") + } + if !h.replyToDNSQueries() { + t.Errorf("unexpectedly deny; wanted to be a DNS server") + } - wantRoutes := []netip.Prefix{netip.MustParsePrefix("192.0.0.8/32")} - if !slices.Equal(rc.Routes(), wantRoutes) { - t.Errorf("got %v; want %v", rc.Routes(), wantRoutes) + w := httptest.NewRecorder() + h.handleDNSQuery(w, httptest.NewRequest("GET", "/dns-query?q=www.example.com.", nil)) + if w.Code != http.StatusOK { + t.Errorf("unexpected status code: %v", w.Code) + } + h.ps.b.appConnector.Wait(ctx) + + wantRoutes := []netip.Prefix{netip.MustParsePrefix("192.0.0.8/32")} + if !slices.Equal(rc.Routes(), wantRoutes) { + t.Errorf("got %v; want %v", rc.Routes(), wantRoutes) + } } } diff --git a/tailcfg/tailcfg.go b/tailcfg/tailcfg.go index 0c521ce05..04cd89672 100644 --- a/tailcfg/tailcfg.go +++ b/tailcfg/tailcfg.go @@ -2232,8 +2232,13 @@ const ( // NodeAttrDisableWebClient disables using the web client. NodeAttrDisableWebClient NodeCapability = "disable-web-client" +<<<<<<< HEAD // NodeAttrExitDstNetworkFlowLog enables exit node destinations in network flow logs. NodeAttrExitDstNetworkFlowLog NodeCapability = "exit-dst-network-flow-log" +======= + // NodeAttrStoreAppCRoutes enables storing app connector routes persistently. + NodeAttrStoreAppCRoutes NodeCapability = "store-appc-routes" +>>>>>>> 61f7b83bd (Add a control knob to toggle writing RouteInfo to StateStore) ) // SetDNSRequest is a request to add a DNS record.