diff options
| author | Fran Bull <fran@tailscale.com> | 2024-07-01 08:53:59 -0700 |
|---|---|---|
| committer | Fran Bull <fran@tailscale.com> | 2024-07-01 08:53:59 -0700 |
| commit | 1c8e060628ff7d49a610303c2e982eb6b10cb5b6 (patch) | |
| tree | ed86719445471ef4d1197a1c2fd880c7a5a152d9 | |
| parent | 8965e87fa857d8bb7103d091f8b33bdbc97f4c04 (diff) | |
| download | tailscale-fran/fix-appc-write-new-domain.tar.xz tailscale-fran/fix-appc-write-new-domain.zip | |
appc: fix writing route infofran/fix-appc-write-new-domain
Before the fix in 7eb8a77ac80b78a2917f0a78a6f4a9189d739797 we were
writing route info on every DNS request. Since that fix we have only
been writing when we advertise new routes. This means we haven't been
writing domain->IP Address associations if the IP Address is covered by
a control route. We do want to persist the info associating the domain
and IP Address so make sure we also store routes in that case.
Fixes #12673
Signed-off-by: Fran Bull <fran@tailscale.com>
| -rw-r--r-- | appc/appconnector.go | 1 | ||||
| -rw-r--r-- | appc/appconnector_test.go | 32 |
2 files changed, 33 insertions, 0 deletions
diff --git a/appc/appconnector.go b/appc/appconnector.go index 9dab4764c..2f2550189 100644 --- a/appc/appconnector.go +++ b/appc/appconnector.go @@ -493,6 +493,7 @@ func (e *AppConnector) isAddrKnownLocked(domain string, addr netip.Addr) bool { // record the new address associated with the domain for faster matching in subsequent // requests and for diagnostic records. e.addDomainAddrLocked(domain, addr) + e.storeRoutesLocked() return true } } diff --git a/appc/appconnector_test.go b/appc/appconnector_test.go index cf5efeff5..07d32231e 100644 --- a/appc/appconnector_test.go +++ b/appc/appconnector_test.go @@ -569,3 +569,35 @@ func TestRateLogger(t *testing.T) { t.Fatalf("wasCalled: got false, want true") } } + +func TestWriteRoutesCoveredByControlRoutes(t *testing.T) { + var writeCount int + write := func(*RouteInfo) error { + writeCount++ + return nil + } + // even if there are routes that cover a domain's ip, if we haven't see the domain before we should write + ctx := context.Background() + rc := &appctest.RouteCollector{} + a := NewAppConnector(t.Logf, rc, &RouteInfo{}, write) + a.UpdateDomainsAndRoutes([]string{"*.example.com"}, []netip.Prefix{netip.MustParsePrefix("192.1.1.0/31")}) + a.Wait(ctx) + // now we have an app connector that is wanting to learn routes for *.example.com, and is preconfigured with + // a route range, when it observes a dns response within the route range it should not publish a new route, but + // it should write it's RouteInfo, so that it remembers the domain<->ip addr association. + writeCount = 0 + a.ObserveDNSResponse(dnsResponse("a.example.com.", "192.1.1.1")) + a.Wait(ctx) + want := 1 + if writeCount != want { + t.Fatalf("writeCount new ip: got %d, want %d", writeCount, want) + } + // we should NOT write, if we are observing the same ip address again + writeCount = 0 + a.ObserveDNSResponse(dnsResponse("a.example.com.", "192.1.1.1")) + a.Wait(ctx) + want = 0 + if writeCount != want { + t.Fatalf("writeCount old ip: got %d, want %d", writeCount, want) + } +} |
