mirrors
/
tailscale
mirror of https://github.com/tailscale/tailscale.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

ipn/ipnlocal: make app connector configuration concurrent 

If there are routes changes as a side effect of an app connector
configuration update, the connector configuration may want to reenter a
lock, so must be started asynchronously.

Updates tailscale/corp#16833
Signed-off-by: James Tucker <james@tailscale.com>
(cherry picked from commit 8250582fe6)
This commit is contained in:
James Tucker 2024-01-18 10:18:25 -08:00 committed by kari-ts
parent 36e3c418aa
commit d304bdcae8
6 changed files with 55 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

39
appc/appconnector.go
View File

@ -10,6 +10,7 @@
package appc
import (
"context"
"net/netip"
"slices"
"strings"
@ -20,6 +21,7 @@ import (
"tailscale.com/types/logger"
"tailscale.com/types/views"
"tailscale.com/util/dnsname"
"tailscale.com/util/execqueue"
)
// RouteAdvertiser is an interface that allows the AppConnector to advertise
@ -58,6 +60,9 @@ type AppConnector struct {
// wildcards is the list of domain strings that match subdomains.
wildcards []string
// queue provides ordering for update operations
queue execqueue.ExecQueue
}
// NewAppConnector creates a new AppConnector.
@ -68,11 +73,33 @@ func NewAppConnector(logf logger.Logf, routeAdvertiser RouteAdvertiser) *AppConn
}
}
// UpdateDomains 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 matches all
// subdomains of a domain.
// UpdateDomainsAndRoutes starts an asynchronous update of the configuration
// given the new domains and routes.
func (e *AppConnector) UpdateDomainsAndRoutes(domains []string, routes []netip.Prefix) {
e.queue.Add(func() {
// Add the new routes first.
e.updateRoutes(routes)
e.updateDomains(domains)
})
}
// 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
// matches all subdomains of a domain.
func (e *AppConnector) UpdateDomains(domains []string) {
e.queue.Add(func() {
e.updateDomains(domains)
})
}
// Wait waits for the currently scheduled asynchronous configuration changes to
// complete.
func (e *AppConnector) Wait(ctx context.Context) {
e.queue.Wait(ctx)
}
func (e *AppConnector) updateDomains(domains []string) {
e.mu.Lock()
defer e.mu.Unlock()
@ -104,11 +131,11 @@ func (e *AppConnector) UpdateDomains(domains []string) {
e.logf("handling domains: %v and wildcards: %v", xmaps.Keys(e.domains), e.wildcards)
}
// UpdateRoutes merges the supplied routes into the currently configured routes. The routes supplied
// updateRoutes merges the supplied routes into the currently configured routes. The routes supplied
// by control for UpdateRoutes are supplemental to the routes discovered by DNS resolution, but are
// also more often whole ranges. UpdateRoutes will remove any single address routes that are now
// covered by new ranges.
func (e *AppConnector) UpdateRoutes(routes []netip.Prefix) {
func (e *AppConnector) updateRoutes(routes []netip.Prefix) {
e.mu.Lock()
defer e.mu.Unlock()

22
appc/appconnector_test.go
View File

@ -4,6 +4,7 @@
package appc
import (
"context"
"net/netip"
"reflect"
"slices"
@ -16,8 +17,11 @@ import (
)
func TestUpdateDomains(t *testing.T) {
ctx := context.Background()
a := NewAppConnector(t.Logf, nil)
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)
}
@ -25,6 +29,7 @@ func TestUpdateDomains(t *testing.T) {
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)
@ -32,6 +37,7 @@ func TestUpdateDomains(t *testing.T) {
// 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)
}
@ -41,7 +47,7 @@ func TestUpdateRoutes(t *testing.T) {
rc := &routeCollector{}
a := NewAppConnector(t.Logf, rc)
routes := []netip.Prefix{netip.MustParsePrefix("192.0.2.0/24")}
a.UpdateRoutes(routes)
a.updateRoutes(routes)
if !slices.EqualFunc(routes, rc.routes, prefixEqual) {
t.Fatalf("got %v, want %v", rc.routes, routes)
@ -54,7 +60,7 @@ func TestUpdateRoutesUnadvertisesContainedRoutes(t *testing.T) {
mak.Set(&a.domains, "example.com", []netip.Addr{netip.MustParseAddr("192.0.2.1")})
rc.routes = []netip.Prefix{netip.MustParsePrefix("192.0.2.1/32")}
routes := []netip.Prefix{netip.MustParsePrefix("192.0.2.0/24")}
a.UpdateRoutes(routes)
a.updateRoutes(routes)
if !slices.EqualFunc(routes, rc.routes, prefixEqual) {
t.Fatalf("got %v, want %v", rc.routes, routes)
@ -64,7 +70,7 @@ func TestUpdateRoutesUnadvertisesContainedRoutes(t *testing.T) {
func TestDomainRoutes(t *testing.T) {
rc := &routeCollector{}
a := NewAppConnector(t.Logf, rc)
a.UpdateDomains([]string{"example.com"})
a.updateDomains([]string{"example.com"})
a.ObserveDNSResponse(dnsResponse("example.com.", "192.0.0.8"))
want := map[string][]netip.Addr{
@ -88,7 +94,7 @@ func TestObserveDNSResponse(t *testing.T) {
wantRoutes := []netip.Prefix{netip.MustParsePrefix("192.0.0.8/32")}
a.UpdateDomains([]string{"example.com"})
a.updateDomains([]string{"example.com"})
a.ObserveDNSResponse(dnsResponse("example.com.", "192.0.0.8"))
if got, want := rc.routes, wantRoutes; !slices.Equal(got, want) {
t.Errorf("got %v; want %v", got, want)
@ -109,7 +115,7 @@ func TestObserveDNSResponse(t *testing.T) {
// 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})
a.updateRoutes([]netip.Prefix{pfx})
wantRoutes = append(wantRoutes, pfx)
a.ObserveDNSResponse(dnsResponse("example.com.", "192.0.2.1"))
if !slices.Equal(rc.routes, wantRoutes) {
@ -124,7 +130,7 @@ func TestWildcardDomains(t *testing.T) {
rc := &routeCollector{}
a := NewAppConnector(t.Logf, rc)
a.UpdateDomains([]string{"*.example.com"})
a.updateDomains([]string{"*.example.com"})
a.ObserveDNSResponse(dnsResponse("foo.example.com.", "192.0.0.8"))
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)
@ -133,7 +139,7 @@ func TestWildcardDomains(t *testing.T) {
t.Errorf("wildcards: got %v; want %v", got, want)
}
a.UpdateDomains([]string{"*.example.com", "example.com"})
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")
}
@ -142,7 +148,7 @@ func TestWildcardDomains(t *testing.T) {
}
// There was an early regression where the wildcard domain was added repeatedly, this guards against that.
a.UpdateDomains([]string{"*.example.com", "example.com"})
a.updateDomains([]string{"*.example.com", "example.com"})
if len(a.wildcards) != 1 {
t.Errorf("expected only one wildcard domain, got %v", a.wildcards)
}

2
cmd/tailscaled/depaware.txt
View File

@ -348,7 +348,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
💣 tailscale.com/util/deephash from tailscale.com/ipn/ipnlocal+
L 💣 tailscale.com/util/dirwalk from tailscale.com/metrics+
tailscale.com/util/dnsname from tailscale.com/hostinfo+
tailscale.com/util/execqueue from tailscale.com/control/controlclient
tailscale.com/util/execqueue from tailscale.com/control/controlclient+
tailscale.com/util/goroutines from tailscale.com/ipn/ipnlocal
tailscale.com/util/groupmember from tailscale.com/ipn/ipnauth+
💣 tailscale.com/util/hashx from tailscale.com/util/deephash

3
ipn/ipnlocal/local.go
View File

@ -3460,8 +3460,7 @@ func (b *LocalBackend) reconfigAppConnectorLocked(nm *netmap.NetworkMap, prefs i
slices.SortFunc(routes, func(i, j netip.Prefix) int { return i.Addr().Compare(j.Addr()) })
domains = slices.Compact(domains)
routes = slices.Compact(routes)
b.appConnector.UpdateRoutes(routes)
b.appConnector.UpdateDomains(domains)
b.appConnector.UpdateDomainsAndRoutes(domains, routes)
}
// authReconfig pushes a new configuration into wgengine, if engine

3
ipn/ipnlocal/local_test.go
View File

@ -1207,8 +1207,10 @@ func TestObserveDNSResponse(t *testing.T) {
rc := &routeCollector{}
b.appConnector = appc.NewAppConnector(t.Logf, rc)
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)
@ -1250,6 +1252,7 @@ func TestReconfigureAppConnector(t *testing.T) {
}).View()
b.reconfigAppConnectorLocked(b.netMap, b.pm.prefs)
b.appConnector.Wait(context.Background())
want := []string{"example.com"}
if !slices.Equal(b.appConnector.Domains().AsSlice(), want) {

3
ipn/ipnlocal/peerapi_test.go
View File

@ -685,6 +685,7 @@ func TestPeerAPIReplyToDNSQueries(t *testing.T) {
}
func TestPeerAPIReplyToDNSQueriesAreObserved(t *testing.T) {
ctx := context.Background()
var h peerAPIHandler
h.remoteAddr = netip.MustParseAddrPort("100.150.151.152:12345")
@ -700,6 +701,7 @@ func TestPeerAPIReplyToDNSQueriesAreObserved(t *testing.T) {
},
}
h.ps.b.appConnector.UpdateDomains([]string{"example.com"})
h.ps.b.appConnector.Wait(ctx)
h.ps.resolver = &fakeResolver{}
f := filter.NewAllowAllForTest(logger.Discard)
@ -717,6 +719,7 @@ func TestPeerAPIReplyToDNSQueriesAreObserved(t *testing.T) {
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) {