diff --git a/appc/appconnector.go b/appc/appconnector.go index 8d7dd54e8..8c1d49d22 100644 --- a/appc/appconnector.go +++ b/appc/appconnector.go @@ -162,17 +162,36 @@ type AppConnector struct { writeRateDay *rateLogger } +// Config carries the settings for an [AppConnector]. +type Config struct { + // Logf is the logger to which debug logs from the connector will be sent. + // It must be non-nil. + Logf logger.Logf + + // RouteAdvertiser allows the connector to update the set of advertised routes. + // It must be non-nil. + RouteAdvertiser RouteAdvertiser + + // RouteInfo, if non-nil, use used as the initial set of routes for the + // connector. If nil, the connector starts empty. + RouteInfo *RouteInfo + + // StoreRoutesFunc, if non-nil, is called when the connector's routes + // change, to allow the routes to be persisted. + StoreRoutesFunc func(*RouteInfo) error +} + // NewAppConnector creates a new AppConnector. -func NewAppConnector(logf logger.Logf, routeAdvertiser RouteAdvertiser, routeInfo *RouteInfo, storeRoutesFunc func(*RouteInfo) error) *AppConnector { +func NewAppConnector(c Config) *AppConnector { ac := &AppConnector{ - logf: logger.WithPrefix(logf, "appc: "), - routeAdvertiser: routeAdvertiser, - storeRoutesFunc: storeRoutesFunc, + logf: logger.WithPrefix(c.Logf, "appc: "), + routeAdvertiser: c.RouteAdvertiser, + storeRoutesFunc: c.StoreRoutesFunc, } - if routeInfo != nil { - ac.domains = routeInfo.Domains - ac.wildcards = routeInfo.Wildcards - ac.controlRoutes = routeInfo.Control + if c.RouteInfo != nil { + ac.domains = c.RouteInfo.Domains + ac.wildcards = c.RouteInfo.Wildcards + ac.controlRoutes = c.RouteInfo.Control } ac.writeRateMinute = newRateLogger(time.Now, time.Minute, func(c int64, s time.Time, l int64) { ac.logf("routeInfo write rate: %d in minute starting at %v (%d routes)", c, s, l) diff --git a/appc/appconnector_test.go b/appc/appconnector_test.go index c13835f39..12a39f040 100644 --- a/appc/appconnector_test.go +++ b/appc/appconnector_test.go @@ -28,9 +28,14 @@ func TestUpdateDomains(t *testing.T) { ctx := context.Background() var a *AppConnector if shouldStore { - a = NewAppConnector(t.Logf, &appctest.RouteCollector{}, &RouteInfo{}, fakeStoreRoutes) + a = NewAppConnector(Config{ + Logf: t.Logf, + RouteAdvertiser: &appctest.RouteCollector{}, + RouteInfo: &RouteInfo{}, + StoreRoutesFunc: fakeStoreRoutes, + }) } else { - a = NewAppConnector(t.Logf, &appctest.RouteCollector{}, nil, nil) + a = NewAppConnector(Config{Logf: t.Logf, RouteAdvertiser: &appctest.RouteCollector{}}) } a.UpdateDomains([]string{"example.com"}) @@ -63,9 +68,13 @@ func TestUpdateRoutes(t *testing.T) { rc := &appctest.RouteCollector{} var a *AppConnector if shouldStore { - a = NewAppConnector(t.Logf, rc, &RouteInfo{}, fakeStoreRoutes) + a = NewAppConnector(Config{ + Logf: t.Logf, + RouteAdvertiser: rc, + RouteInfo: &RouteInfo{}, StoreRoutesFunc: fakeStoreRoutes, + }) } else { - a = NewAppConnector(t.Logf, rc, nil, nil) + a = NewAppConnector(Config{Logf: t.Logf, RouteAdvertiser: rc}) } a.updateDomains([]string{"*.example.com"}) @@ -112,9 +121,14 @@ func TestUpdateRoutesUnadvertisesContainedRoutes(t *testing.T) { rc := &appctest.RouteCollector{} var a *AppConnector if shouldStore { - a = NewAppConnector(t.Logf, rc, &RouteInfo{}, fakeStoreRoutes) + a = NewAppConnector(Config{ + Logf: t.Logf, + RouteAdvertiser: rc, + RouteInfo: &RouteInfo{}, + StoreRoutesFunc: fakeStoreRoutes, + }) } else { - a = NewAppConnector(t.Logf, rc, nil, nil) + a = NewAppConnector(Config{Logf: t.Logf, RouteAdvertiser: 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")}) @@ -133,9 +147,14 @@ func TestDomainRoutes(t *testing.T) { rc := &appctest.RouteCollector{} var a *AppConnector if shouldStore { - a = NewAppConnector(t.Logf, rc, &RouteInfo{}, fakeStoreRoutes) + a = NewAppConnector(Config{ + Logf: t.Logf, + RouteAdvertiser: rc, + RouteInfo: &RouteInfo{}, + StoreRoutesFunc: fakeStoreRoutes, + }) } else { - a = NewAppConnector(t.Logf, rc, nil, nil) + a = NewAppConnector(Config{Logf: t.Logf, RouteAdvertiser: rc}) } a.updateDomains([]string{"example.com"}) if err := a.ObserveDNSResponse(dnsResponse("example.com.", "192.0.0.8")); err != nil { @@ -159,9 +178,14 @@ func TestObserveDNSResponse(t *testing.T) { rc := &appctest.RouteCollector{} var a *AppConnector if shouldStore { - a = NewAppConnector(t.Logf, rc, &RouteInfo{}, fakeStoreRoutes) + a = NewAppConnector(Config{ + Logf: t.Logf, + RouteAdvertiser: rc, + RouteInfo: &RouteInfo{}, + StoreRoutesFunc: fakeStoreRoutes, + }) } else { - a = NewAppConnector(t.Logf, rc, nil, nil) + a = NewAppConnector(Config{Logf: t.Logf, RouteAdvertiser: rc}) } // a has no domains configured, so it should not advertise any routes @@ -248,9 +272,14 @@ func TestWildcardDomains(t *testing.T) { rc := &appctest.RouteCollector{} var a *AppConnector if shouldStore { - a = NewAppConnector(t.Logf, rc, &RouteInfo{}, fakeStoreRoutes) + a = NewAppConnector(Config{ + Logf: t.Logf, + RouteAdvertiser: rc, + RouteInfo: &RouteInfo{}, + StoreRoutesFunc: fakeStoreRoutes, + }) } else { - a = NewAppConnector(t.Logf, rc, nil, nil) + a = NewAppConnector(Config{Logf: t.Logf, RouteAdvertiser: rc}) } a.updateDomains([]string{"*.example.com"}) @@ -408,9 +437,14 @@ func TestUpdateRouteRouteRemoval(t *testing.T) { var a *AppConnector if shouldStore { - a = NewAppConnector(t.Logf, rc, &RouteInfo{}, fakeStoreRoutes) + a = NewAppConnector(Config{ + Logf: t.Logf, + RouteAdvertiser: rc, + RouteInfo: &RouteInfo{}, + StoreRoutesFunc: fakeStoreRoutes, + }) } else { - a = NewAppConnector(t.Logf, rc, nil, nil) + a = NewAppConnector(Config{Logf: t.Logf, RouteAdvertiser: rc}) } // nothing has yet been advertised assertRoutes("appc init", []netip.Prefix{}, []netip.Prefix{}) @@ -453,9 +487,14 @@ func TestUpdateDomainRouteRemoval(t *testing.T) { var a *AppConnector if shouldStore { - a = NewAppConnector(t.Logf, rc, &RouteInfo{}, fakeStoreRoutes) + a = NewAppConnector(Config{ + Logf: t.Logf, + RouteAdvertiser: rc, + RouteInfo: &RouteInfo{}, + StoreRoutesFunc: fakeStoreRoutes, + }) } else { - a = NewAppConnector(t.Logf, rc, nil, nil) + a = NewAppConnector(Config{Logf: t.Logf, RouteAdvertiser: rc}) } assertRoutes("appc init", []netip.Prefix{}, []netip.Prefix{}) @@ -508,9 +547,14 @@ func TestUpdateWildcardRouteRemoval(t *testing.T) { var a *AppConnector if shouldStore { - a = NewAppConnector(t.Logf, rc, &RouteInfo{}, fakeStoreRoutes) + a = NewAppConnector(Config{ + Logf: t.Logf, + RouteAdvertiser: rc, + RouteInfo: &RouteInfo{}, + StoreRoutesFunc: fakeStoreRoutes, + }) } else { - a = NewAppConnector(t.Logf, rc, nil, nil) + a = NewAppConnector(Config{Logf: t.Logf, RouteAdvertiser: rc}) } assertRoutes("appc init", []netip.Prefix{}, []netip.Prefix{}) @@ -649,7 +693,12 @@ func TestMetricBucketsAreSorted(t *testing.T) { func TestUpdateRoutesDeadlock(t *testing.T) { ctx := context.Background() rc := &appctest.RouteCollector{} - a := NewAppConnector(t.Logf, rc, &RouteInfo{}, fakeStoreRoutes) + a := NewAppConnector(Config{ + Logf: t.Logf, + RouteAdvertiser: rc, + RouteInfo: &RouteInfo{}, + StoreRoutesFunc: fakeStoreRoutes, + }) advertiseCalled := new(atomic.Bool) unadvertiseCalled := new(atomic.Bool) diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 09f317f0f..5e738572f 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -4802,7 +4802,12 @@ func (b *LocalBackend) reconfigAppConnectorLocked(nm *netmap.NetworkMap, prefs i } storeFunc = b.storeRouteInfo } - b.appConnector = appc.NewAppConnector(b.logf, b, ri, storeFunc) + b.appConnector = appc.NewAppConnector(appc.Config{ + Logf: b.logf, + RouteAdvertiser: b, + RouteInfo: ri, + StoreRoutesFunc: storeFunc, + }) } if nm == nil { return diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go index a984d66bf..571f472cc 100644 --- a/ipn/ipnlocal/local_test.go +++ b/ipn/ipnlocal/local_test.go @@ -2309,9 +2309,11 @@ func TestOfferingAppConnector(t *testing.T) { t.Fatal("unexpected offering app connector") } if shouldStore { - b.appConnector = appc.NewAppConnector(t.Logf, nil, &appc.RouteInfo{}, fakeStoreRoutes) + b.appConnector = appc.NewAppConnector(appc.Config{ + Logf: t.Logf, RouteInfo: &appc.RouteInfo{}, StoreRoutesFunc: fakeStoreRoutes, + }) } else { - b.appConnector = appc.NewAppConnector(t.Logf, nil, nil, nil) + b.appConnector = appc.NewAppConnector(appc.Config{Logf: t.Logf}) } if !b.OfferingAppConnector() { t.Fatal("unexpected not offering app connector") @@ -2370,9 +2372,14 @@ func TestObserveDNSResponse(t *testing.T) { rc := &appctest.RouteCollector{} if shouldStore { - b.appConnector = appc.NewAppConnector(t.Logf, rc, &appc.RouteInfo{}, fakeStoreRoutes) + b.appConnector = appc.NewAppConnector(appc.Config{ + Logf: t.Logf, + RouteAdvertiser: rc, + RouteInfo: &appc.RouteInfo{}, + StoreRoutesFunc: fakeStoreRoutes, + }) } else { - b.appConnector = appc.NewAppConnector(t.Logf, rc, nil, nil) + b.appConnector = appc.NewAppConnector(appc.Config{Logf: t.Logf, RouteAdvertiser: rc}) } b.appConnector.UpdateDomains([]string{"example.com"}) b.appConnector.Wait(context.Background()) diff --git a/ipn/ipnlocal/peerapi_test.go b/ipn/ipnlocal/peerapi_test.go index db01dd608..a6a5f6ff5 100644 --- a/ipn/ipnlocal/peerapi_test.go +++ b/ipn/ipnlocal/peerapi_test.go @@ -257,9 +257,14 @@ func TestPeerAPIPrettyReplyCNAME(t *testing.T) { pm := must.Get(newProfileManager(new(mem.Store), t.Logf, ht)) var a *appc.AppConnector if shouldStore { - a = appc.NewAppConnector(t.Logf, &appctest.RouteCollector{}, &appc.RouteInfo{}, fakeStoreRoutes) + a = appc.NewAppConnector(appc.Config{ + Logf: t.Logf, + RouteAdvertiser: &appctest.RouteCollector{}, + RouteInfo: &appc.RouteInfo{}, + StoreRoutesFunc: fakeStoreRoutes, + }) } else { - a = appc.NewAppConnector(t.Logf, &appctest.RouteCollector{}, nil, nil) + a = appc.NewAppConnector(appc.Config{Logf: t.Logf, RouteAdvertiser: &appctest.RouteCollector{}}) } sys.Set(pm.Store()) sys.Set(eng) @@ -332,9 +337,14 @@ func TestPeerAPIReplyToDNSQueriesAreObserved(t *testing.T) { eng, _ := wgengine.NewFakeUserspaceEngine(logger.Discard, 0, ht, reg, sys.Bus.Get(), sys.Set) var a *appc.AppConnector if shouldStore { - a = appc.NewAppConnector(t.Logf, rc, &appc.RouteInfo{}, fakeStoreRoutes) + a = appc.NewAppConnector(appc.Config{ + Logf: t.Logf, + RouteAdvertiser: rc, + RouteInfo: &appc.RouteInfo{}, + StoreRoutesFunc: fakeStoreRoutes, + }) } else { - a = appc.NewAppConnector(t.Logf, rc, nil, nil) + a = appc.NewAppConnector(appc.Config{Logf: t.Logf, RouteAdvertiser: rc}) } sys.Set(pm.Store()) sys.Set(eng) @@ -399,9 +409,14 @@ func TestPeerAPIReplyToDNSQueriesAreObservedWithCNAMEFlattening(t *testing.T) { pm := must.Get(newProfileManager(new(mem.Store), t.Logf, ht)) var a *appc.AppConnector if shouldStore { - a = appc.NewAppConnector(t.Logf, rc, &appc.RouteInfo{}, fakeStoreRoutes) + a = appc.NewAppConnector(appc.Config{ + Logf: t.Logf, + RouteAdvertiser: rc, + RouteInfo: &appc.RouteInfo{}, + StoreRoutesFunc: fakeStoreRoutes, + }) } else { - a = appc.NewAppConnector(t.Logf, rc, nil, nil) + a = appc.NewAppConnector(appc.Config{Logf: t.Logf, RouteAdvertiser: rc}) } sys.Set(pm.Store()) sys.Set(eng)