Skip to content

Commit

Permalink
Merge tag 'v1.58.2' into sunos-1.58
Browse files Browse the repository at this point in the history 
Release 1.58.2
  • Loading branch information
@nshalman
nshalman committed Jan 24, 2024
2 parents 60b2539 + b0e1bbb commit fab1c7e
Show file tree
Hide file tree
Showing 13 changed files with 629 additions and 216 deletions.
2 changes: 1 addition & 1 deletion VERSION.txt
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1.58.0
1.58.2
71 changes: 51 additions & 20 deletions appc/appconnector.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
package appc

import (
"context"
"net/netip"
"slices"
"strings"
Expand All @@ -20,17 +21,18 @@ 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
// newly discovered routes that need to be served through the AppConnector.
type RouteAdvertiser interface {
// AdvertiseRoute adds a new route advertisement if the route is not already
// being advertised.
AdvertiseRoute(netip.Prefix) error
// AdvertiseRoute adds one or more route advertisements skipping any that
// are already advertised.
AdvertiseRoute(...netip.Prefix) error

// UnadvertiseRoute removes a route advertisement.
UnadvertiseRoute(netip.Prefix) error
// UnadvertiseRoute removes any matching route advertisements.
UnadvertiseRoute(...netip.Prefix) error
}

// AppConnector is an implementation of an AppConnector that performs
Expand Down Expand Up @@ -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.
Expand All @@ -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()

Expand Down Expand Up @@ -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()

Expand All @@ -117,26 +144,30 @@ func (e *AppConnector) UpdateRoutes(routes []netip.Prefix) {
return
}

if err := e.routeAdvertiser.AdvertiseRoute(routes...); err != nil {
e.logf("failed to advertise routes: %v: %v", routes, err)
return
}

var toRemove []netip.Prefix

nextRoute:
for _, r := range routes {
if err := e.routeAdvertiser.AdvertiseRoute(r); err != nil {
e.logf("failed to advertise route: %v: %v", r, err)
continue
}

for _, addr := range e.domains {
for _, a := range addr {
if r.Contains(a) {
if r.Contains(a) && netip.PrefixFrom(a, a.BitLen()) != r {
pfx := netip.PrefixFrom(a, a.BitLen())
if err := e.routeAdvertiser.UnadvertiseRoute(pfx); err != nil {
e.logf("failed to unadvertise route: %v: %v", pfx, err)
}
toRemove = append(toRemove, pfx)
continue nextRoute
}
}
}
}

if err := e.routeAdvertiser.UnadvertiseRoute(toRemove...); err != nil {
e.logf("failed to unadvertise routes: %v: %v", toRemove, err)
}

e.controlRoutes = routes
}

Expand Down
116 changes: 66 additions & 50 deletions appc/appconnector_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,67 +4,100 @@
package appc

import (
"context"
"net/netip"
"reflect"
"slices"
"testing"

xmaps "golang.org/x/exp/maps"
"golang.org/x/net/dns/dnsmessage"
"tailscale.com/appc/appctest"
"tailscale.com/util/mak"
"tailscale.com/util/must"
)

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)
}

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)
}

// 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) {
rc := &routeCollector{}
ctx := context.Background()
rc := &appctest.RouteCollector{}
a := NewAppConnector(t.Logf, rc)
routes := []netip.Prefix{netip.MustParsePrefix("192.0.2.0/24")}
a.UpdateRoutes(routes)
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)

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)

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)

// 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)
}

if !slices.EqualFunc(routes, rc.routes, prefixEqual) {
t.Fatalf("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())
}
}

func TestUpdateRoutesUnadvertisesContainedRoutes(t *testing.T) {
rc := &routeCollector{}
rc := &appctest.RouteCollector{}
a := NewAppConnector(t.Logf, rc)
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")}
rc.SetRoutes([]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)
if !slices.EqualFunc(routes, rc.Routes(), prefixEqual) {
t.Fatalf("got %v, want %v", rc.Routes(), routes)
}
}

func TestDomainRoutes(t *testing.T) {
rc := &routeCollector{}
rc := &appctest.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{
Expand All @@ -77,63 +110,63 @@ func TestDomainRoutes(t *testing.T) {
}

func TestObserveDNSResponse(t *testing.T) {
rc := &routeCollector{}
rc := &appctest.RouteCollector{}
a := NewAppConnector(t.Logf, rc)

// 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) {
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")}

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) {
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"))

a.ObserveDNSResponse(dnsResponse("example.com.", "2001:db8::1"))
if got, want := rc.routes, wantRoutes; !slices.Equal(got, want) {
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"))
if !slices.Equal(rc.routes, wantRoutes) {
t.Errorf("rc.routes: got %v; want %v", rc.routes, wantRoutes)
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})
a.updateRoutes([]netip.Prefix{pfx})
wantRoutes = append(wantRoutes, pfx)
a.ObserveDNSResponse(dnsResponse("example.com.", "192.0.2.1"))
if !slices.Equal(rc.routes, wantRoutes) {
t.Errorf("rc.routes: got %v; want %v", rc.routes, wantRoutes)
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) {
rc := &routeCollector{}
rc := &appctest.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) {
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"})
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")
}
Expand All @@ -142,7 +175,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)
}
Expand Down Expand Up @@ -185,30 +218,13 @@ func dnsResponse(domain, address string) []byte {
return must.Get(b.Finish())
}

// routeCollector is a test helper that collects the list of routes advertised
type routeCollector struct {
routes []netip.Prefix
}

// routeCollector implements RouteAdvertiser
var _ RouteAdvertiser = (*routeCollector)(nil)

func (rc *routeCollector) AdvertiseRoute(pfx netip.Prefix) error {
rc.routes = append(rc.routes, pfx)
return nil
func prefixEqual(a, b netip.Prefix) bool {
return a == b
}

func (rc *routeCollector) UnadvertiseRoute(pfx netip.Prefix) error {
routes := rc.routes
rc.routes = rc.routes[:0]
for _, r := range routes {
if r != pfx {
rc.routes = append(rc.routes, r)
}
func prefixCompare(a, b netip.Prefix) int {
if a.Addr().Compare(b.Addr()) == 0 {
return a.Bits() - b.Bits()
}
return nil
}

func prefixEqual(a, b netip.Prefix) bool {
return a.Addr().Compare(b.Addr()) == 0 && a.Bits() == b.Bits()
return a.Addr().Compare(b.Addr())
}
Loading

0 comments on commit fab1c7e

Please sign in to comment.