refactor: remove domain arg from label provider

Author: steveiliop56

internal/service/access_controls_service.go

@@ -12,12 +12,8 @@ import (
 	"go.uber.org/dig"
 )
 
-// LabelProvider looks up the apps it knows about for the given domain. A
-// provider that knows which hosts its apps are served on MUST only yield the
-// ones that are actually served on domain, so that an unrelated app cannot
-// claim it by name.
 type LabelProvider interface {
-	Lookup(domain string, locator func(name string, app *model.App) bool) error
+	Lookup(locator func(name string, app *model.App) bool) error
 }
 
 type AccessControlsService struct {
@@ -149,9 +145,7 @@ func (service *AccessControlsService) GetAccessControls(domain string) (*model.A
 
 	// If we have a label provider configured, try to get ACLs from it
 	if service.labelProvider != nil {
-		return service.getACLs(domain, func(locator func(name string, app *model.App) bool) error {
-			return service.labelProvider.Lookup(domain, locator)
-		})
+		return service.getACLs(domain, service.labelProvider.Lookup)
 	}
 
 	// No labels

internal/service/access_controls_service_test.go

@@ -20,7 +20,7 @@ func newMockProvider(acls map[string]model.App, shouldError bool) *mockProvider
 	return &mockProvider{acls: acls, shouldError: shouldError}
 }
 
-func (m *mockProvider) Lookup(_ string, locator func(name string, app *model.App) bool) error {
+func (m *mockProvider) Lookup(locator func(name string, app *model.App) bool) error {
 	if m.shouldError {
 		return errors.New("mock error")
 	}
@@ -153,9 +153,7 @@ func TestAccessControlsService(t *testing.T) {
 				Config:        &model.Config{},
 				LabelProvider: mock,
 			})
-			app, err := acls.getACLs(test.domain, func(locator func(name string, app *model.App) bool) error {
-				return mock.Lookup(test.domain, locator)
-			})
+			app, err := acls.getACLs(test.domain, mock.Lookup)
 			if test.errorFunc != nil {
 				test.errorFunc(t, err)
 				return
@@ -193,9 +191,7 @@ func TestAccessControlsService(t *testing.T) {
 		Config:        &model.Config{},
 		LabelProvider: mock,
 	})
-	_, err := acls.getACLs("example.com", func(locator func(name string, app *model.App) bool) error {
-		return mock.Lookup("example.com", locator)
-	})
+	_, err := acls.getACLs("example.com", mock.Lookup)
 	assert.Error(t, err)
 
 	// get acls should return an error when multiple apps with the same domain exist

internal/service/docker_service.go

@@ -119,10 +119,7 @@ func (docker *DockerService) inspectContainer(containerId string) (container.Ins
 	return docker.client.ContainerInspect(docker.context, containerId)
 }
 
-// Lookup yields every app labelled on a running container. Container labels
-// carry no routing information, so the domain cannot be used to narrow the
-// results down and the caller is left to match them.
-func (docker *DockerService) Lookup(_ string, locator func(name string, app *model.App) bool) error {
+func (docker *DockerService) Lookup(locator func(name string, app *model.App) bool) error {
 	if !docker.isConnected {
 		docker.log.App.Debug().Msg("Docker service not connected, returning empty labels")
 		return nil

internal/service/kubernetes_service.go

@@ -211,22 +211,12 @@ func (k *KubernetesService) removeResource(key resourceKey) {
 	delete(k.apps, key)
 }
 
-func (k *KubernetesService) getEntry(domain string, locator func(name string, app *model.App) bool) {
-	if !ensureAscii(domain) {
-		k.log.App.Debug().Str("domain", domain).Msg("Domain is invalid, skipping lookup")
-		return
-	}
-
+func (k *KubernetesService) getEntry(locator func(name string, app *model.App) bool) {
 	k.mu.RLock()
 	defer k.mu.RUnlock()
 
 	// O(n^2) is not great but the number of resource entries is expected to be small
 	for _, app := range k.apps {
-		if !slices.ContainsFunc(app.hosts, func(host string) bool {
-			return hostMatchesHostname(host, domain)
-		}) {
-			continue
-		}
 		for _, entry := range app.entries {
 			if ok := locator(entry.name, &entry.app); ok {
 				return
@@ -412,13 +402,13 @@ func (k *KubernetesService) watchGVR(res watchedResource, ctx context.Context) {
 	}
 }
 
-func (k *KubernetesService) Lookup(domain string, locator func(name string, app *model.App) bool) error {
+func (k *KubernetesService) Lookup(locator func(name string, app *model.App) bool) error {
 	if !k.connected {
 		k.log.App.Debug().Msg("Kubernetes label provider not started, skipping")
 		return nil
 	}
 
-	k.getEntry(domain, locator)
+	k.getEntry(locator)
 
 	return nil
 }

internal/service/kubernetes_service_test.go

@@ -49,7 +49,7 @@ func testIngress(name string, annotations map[string]string, hosts ...string) *t
 
 func lookupApp(service *KubernetesService, domain string) *model.App {
 	var app *model.App
-	service.getEntry(domain, func(name string, candidate *model.App) bool {
+	service.getEntry(func(name string, candidate *model.App) bool {
 		if candidate.Config.Domain == domain || strings.HasPrefix(domain, name+".") {
 			app = candidate
 			return true
@@ -175,7 +175,6 @@ func TestKubernetesServiceLookup(t *testing.T) {
 	}{
 		{"Returns a matching app when connected", true, "app.example.com", true},
 		{"Skips the cache before the service is connected", false, "app.example.com", false},
-		{"Skips an invalid domain", true, "app.example.com\xC3\xA9", false},
 	}
 
 	for _, test := range tests {
@@ -188,7 +187,7 @@ func TestKubernetesServiceLookup(t *testing.T) {
 			}})
 
 			var app *model.App
-			err := service.Lookup(test.domain, func(_ string, candidate *model.App) bool {
+			err := service.Lookup(func(_ string, candidate *model.App) bool {
 				app = candidate
 				return true
 			})