appc: factor app connector arguments into a Config type (#17389)

Replace the positional arguments to NewAppConnector with a Config struct.
Update the existing uses. Other than the API change, there are no functional
changes in this commit.

Updates #15160
Updates #17192

Change-Id: Ibf37f021372155a4db8aaf738f4b4f2c746bf623
Signed-off-by: M. J. Fromberger <fromberger@tailscale.com>
This commit is contained in:
M. J. Fromberger 2025-10-01 11:39:01 -07:00 committed by GitHub
parent 05a4c8e839
commit 6f7ce5eb5d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 133 additions and 38 deletions

View File

@ -162,17 +162,36 @@ type AppConnector struct {
writeRateDay *rateLogger 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. // 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{ ac := &AppConnector{
logf: logger.WithPrefix(logf, "appc: "), logf: logger.WithPrefix(c.Logf, "appc: "),
routeAdvertiser: routeAdvertiser, routeAdvertiser: c.RouteAdvertiser,
storeRoutesFunc: storeRoutesFunc, storeRoutesFunc: c.StoreRoutesFunc,
} }
if routeInfo != nil { if c.RouteInfo != nil {
ac.domains = routeInfo.Domains ac.domains = c.RouteInfo.Domains
ac.wildcards = routeInfo.Wildcards ac.wildcards = c.RouteInfo.Wildcards
ac.controlRoutes = routeInfo.Control ac.controlRoutes = c.RouteInfo.Control
} }
ac.writeRateMinute = newRateLogger(time.Now, time.Minute, func(c int64, s time.Time, l int64) { 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) ac.logf("routeInfo write rate: %d in minute starting at %v (%d routes)", c, s, l)

View File

@ -28,9 +28,14 @@ func TestUpdateDomains(t *testing.T) {
ctx := context.Background() ctx := context.Background()
var a *AppConnector var a *AppConnector
if shouldStore { if shouldStore {
a = NewAppConnector(t.Logf, &appctest.RouteCollector{}, &RouteInfo{}, fakeStoreRoutes) a = NewAppConnector(Config{
Logf: t.Logf,
RouteAdvertiser: &appctest.RouteCollector{},
RouteInfo: &RouteInfo{},
StoreRoutesFunc: fakeStoreRoutes,
})
} else { } else {
a = NewAppConnector(t.Logf, &appctest.RouteCollector{}, nil, nil) a = NewAppConnector(Config{Logf: t.Logf, RouteAdvertiser: &appctest.RouteCollector{}})
} }
a.UpdateDomains([]string{"example.com"}) a.UpdateDomains([]string{"example.com"})
@ -63,9 +68,13 @@ func TestUpdateRoutes(t *testing.T) {
rc := &appctest.RouteCollector{} rc := &appctest.RouteCollector{}
var a *AppConnector var a *AppConnector
if shouldStore { if shouldStore {
a = NewAppConnector(t.Logf, rc, &RouteInfo{}, fakeStoreRoutes) a = NewAppConnector(Config{
Logf: t.Logf,
RouteAdvertiser: rc,
RouteInfo: &RouteInfo{}, StoreRoutesFunc: fakeStoreRoutes,
})
} else { } else {
a = NewAppConnector(t.Logf, rc, nil, nil) a = NewAppConnector(Config{Logf: t.Logf, RouteAdvertiser: rc})
} }
a.updateDomains([]string{"*.example.com"}) a.updateDomains([]string{"*.example.com"})
@ -112,9 +121,14 @@ func TestUpdateRoutesUnadvertisesContainedRoutes(t *testing.T) {
rc := &appctest.RouteCollector{} rc := &appctest.RouteCollector{}
var a *AppConnector var a *AppConnector
if shouldStore { if shouldStore {
a = NewAppConnector(t.Logf, rc, &RouteInfo{}, fakeStoreRoutes) a = NewAppConnector(Config{
Logf: t.Logf,
RouteAdvertiser: rc,
RouteInfo: &RouteInfo{},
StoreRoutesFunc: fakeStoreRoutes,
})
} else { } 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")}) 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")}) rc.SetRoutes([]netip.Prefix{netip.MustParsePrefix("192.0.2.1/32")})
@ -133,9 +147,14 @@ func TestDomainRoutes(t *testing.T) {
rc := &appctest.RouteCollector{} rc := &appctest.RouteCollector{}
var a *AppConnector var a *AppConnector
if shouldStore { if shouldStore {
a = NewAppConnector(t.Logf, rc, &RouteInfo{}, fakeStoreRoutes) a = NewAppConnector(Config{
Logf: t.Logf,
RouteAdvertiser: rc,
RouteInfo: &RouteInfo{},
StoreRoutesFunc: fakeStoreRoutes,
})
} else { } else {
a = NewAppConnector(t.Logf, rc, nil, nil) a = NewAppConnector(Config{Logf: t.Logf, RouteAdvertiser: rc})
} }
a.updateDomains([]string{"example.com"}) a.updateDomains([]string{"example.com"})
if err := a.ObserveDNSResponse(dnsResponse("example.com.", "192.0.0.8")); err != nil { 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{} rc := &appctest.RouteCollector{}
var a *AppConnector var a *AppConnector
if shouldStore { if shouldStore {
a = NewAppConnector(t.Logf, rc, &RouteInfo{}, fakeStoreRoutes) a = NewAppConnector(Config{
Logf: t.Logf,
RouteAdvertiser: rc,
RouteInfo: &RouteInfo{},
StoreRoutesFunc: fakeStoreRoutes,
})
} else { } 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 // a has no domains configured, so it should not advertise any routes
@ -248,9 +272,14 @@ func TestWildcardDomains(t *testing.T) {
rc := &appctest.RouteCollector{} rc := &appctest.RouteCollector{}
var a *AppConnector var a *AppConnector
if shouldStore { if shouldStore {
a = NewAppConnector(t.Logf, rc, &RouteInfo{}, fakeStoreRoutes) a = NewAppConnector(Config{
Logf: t.Logf,
RouteAdvertiser: rc,
RouteInfo: &RouteInfo{},
StoreRoutesFunc: fakeStoreRoutes,
})
} else { } else {
a = NewAppConnector(t.Logf, rc, nil, nil) a = NewAppConnector(Config{Logf: t.Logf, RouteAdvertiser: rc})
} }
a.updateDomains([]string{"*.example.com"}) a.updateDomains([]string{"*.example.com"})
@ -408,9 +437,14 @@ func TestUpdateRouteRouteRemoval(t *testing.T) {
var a *AppConnector var a *AppConnector
if shouldStore { if shouldStore {
a = NewAppConnector(t.Logf, rc, &RouteInfo{}, fakeStoreRoutes) a = NewAppConnector(Config{
Logf: t.Logf,
RouteAdvertiser: rc,
RouteInfo: &RouteInfo{},
StoreRoutesFunc: fakeStoreRoutes,
})
} else { } else {
a = NewAppConnector(t.Logf, rc, nil, nil) a = NewAppConnector(Config{Logf: t.Logf, RouteAdvertiser: rc})
} }
// nothing has yet been advertised // nothing has yet been advertised
assertRoutes("appc init", []netip.Prefix{}, []netip.Prefix{}) assertRoutes("appc init", []netip.Prefix{}, []netip.Prefix{})
@ -453,9 +487,14 @@ func TestUpdateDomainRouteRemoval(t *testing.T) {
var a *AppConnector var a *AppConnector
if shouldStore { if shouldStore {
a = NewAppConnector(t.Logf, rc, &RouteInfo{}, fakeStoreRoutes) a = NewAppConnector(Config{
Logf: t.Logf,
RouteAdvertiser: rc,
RouteInfo: &RouteInfo{},
StoreRoutesFunc: fakeStoreRoutes,
})
} else { } else {
a = NewAppConnector(t.Logf, rc, nil, nil) a = NewAppConnector(Config{Logf: t.Logf, RouteAdvertiser: rc})
} }
assertRoutes("appc init", []netip.Prefix{}, []netip.Prefix{}) assertRoutes("appc init", []netip.Prefix{}, []netip.Prefix{})
@ -508,9 +547,14 @@ func TestUpdateWildcardRouteRemoval(t *testing.T) {
var a *AppConnector var a *AppConnector
if shouldStore { if shouldStore {
a = NewAppConnector(t.Logf, rc, &RouteInfo{}, fakeStoreRoutes) a = NewAppConnector(Config{
Logf: t.Logf,
RouteAdvertiser: rc,
RouteInfo: &RouteInfo{},
StoreRoutesFunc: fakeStoreRoutes,
})
} else { } else {
a = NewAppConnector(t.Logf, rc, nil, nil) a = NewAppConnector(Config{Logf: t.Logf, RouteAdvertiser: rc})
} }
assertRoutes("appc init", []netip.Prefix{}, []netip.Prefix{}) assertRoutes("appc init", []netip.Prefix{}, []netip.Prefix{})
@ -649,7 +693,12 @@ func TestMetricBucketsAreSorted(t *testing.T) {
func TestUpdateRoutesDeadlock(t *testing.T) { func TestUpdateRoutesDeadlock(t *testing.T) {
ctx := context.Background() ctx := context.Background()
rc := &appctest.RouteCollector{} 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) advertiseCalled := new(atomic.Bool)
unadvertiseCalled := new(atomic.Bool) unadvertiseCalled := new(atomic.Bool)

View File

@ -4802,7 +4802,12 @@ func (b *LocalBackend) reconfigAppConnectorLocked(nm *netmap.NetworkMap, prefs i
} }
storeFunc = b.storeRouteInfo 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 { if nm == nil {
return return

View File

@ -2309,9 +2309,11 @@ func TestOfferingAppConnector(t *testing.T) {
t.Fatal("unexpected offering app connector") t.Fatal("unexpected offering app connector")
} }
if shouldStore { 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 { } else {
b.appConnector = appc.NewAppConnector(t.Logf, nil, nil, nil) b.appConnector = appc.NewAppConnector(appc.Config{Logf: t.Logf})
} }
if !b.OfferingAppConnector() { if !b.OfferingAppConnector() {
t.Fatal("unexpected not offering app connector") t.Fatal("unexpected not offering app connector")
@ -2370,9 +2372,14 @@ func TestObserveDNSResponse(t *testing.T) {
rc := &appctest.RouteCollector{} rc := &appctest.RouteCollector{}
if shouldStore { 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 { } 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.UpdateDomains([]string{"example.com"})
b.appConnector.Wait(context.Background()) b.appConnector.Wait(context.Background())

View File

@ -257,9 +257,14 @@ func TestPeerAPIPrettyReplyCNAME(t *testing.T) {
pm := must.Get(newProfileManager(new(mem.Store), t.Logf, ht)) pm := must.Get(newProfileManager(new(mem.Store), t.Logf, ht))
var a *appc.AppConnector var a *appc.AppConnector
if shouldStore { 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 { } 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(pm.Store())
sys.Set(eng) 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) eng, _ := wgengine.NewFakeUserspaceEngine(logger.Discard, 0, ht, reg, sys.Bus.Get(), sys.Set)
var a *appc.AppConnector var a *appc.AppConnector
if shouldStore { 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 { } 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(pm.Store())
sys.Set(eng) sys.Set(eng)
@ -399,9 +409,14 @@ func TestPeerAPIReplyToDNSQueriesAreObservedWithCNAMEFlattening(t *testing.T) {
pm := must.Get(newProfileManager(new(mem.Store), t.Logf, ht)) pm := must.Get(newProfileManager(new(mem.Store), t.Logf, ht))
var a *appc.AppConnector var a *appc.AppConnector
if shouldStore { 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 { } 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(pm.Store())
sys.Set(eng) sys.Set(eng)