diff --git a/internal/service/access_controls_service.go b/internal/service/access_controls_service.go index c6ffff7d..f8816a1f 100644 --- a/internal/service/access_controls_service.go +++ b/internal/service/access_controls_service.go @@ -11,7 +11,7 @@ import ( ) type LabelProvider interface { - GetLabels(appDomain string) (*model.App, error) + Lookup(locator func(name string, app *model.App) bool) error } type AccessControlsService struct { @@ -37,35 +37,74 @@ func NewAccessControlsService(i AccessControlServiceInput) *AccessControlsServic } } -func (service *AccessControlsService) lookupStaticACLs(domain string) *model.App { - var nameMatch *model.App - +func (service *AccessControlsService) getACLs(domain string, lookup func(locator func(name string, app *model.App) bool) error) (*model.App, error) { v := validators.NewDomainValidator(validators.DomainValidatorOptions{}) - // First try to find a matching app by domain, then fallback to matching by app name (subdomain) - for app, config := range service.config.Apps { - if config.Config.Domain != "" { - err := v.Validate(config.Config.Domain, domain) + var domainMatch *model.App + var nameMatch *model.App + var nameMatchedApps []string + + locatorFunc := func(name string, app *model.App) bool { + if app.Config.Domain != "" { + err := v.Validate(app.Config.Domain, domain) if err == nil { - service.log.App.Debug().Str("name", app).Msg("Found matching container by domain") - return &config - } - if !errors.Is(err, validators.ErrHostnameMismatch) { - service.log.App.Debug().Str("name", app).Err(err).Msg("Domain validation failed") + service.log.App.Debug().Str("name", name).Msg("Found matching container by domain") + domainMatch = app + return true + } else if !errors.Is(err, validators.ErrHostnameMismatch) { + service.log.App.Debug().Str("name", name).Err(err).Msg("Domain validation failed") } } - if strings.HasPrefix(strings.ToLower(domain), strings.ToLower(app+".")) { - service.log.App.Debug().Str("name", app).Msg("Found matching container by app name") - nameMatch = &config + if strings.HasPrefix(strings.ToLower(domain), strings.ToLower(name+".")) { + service.log.App.Debug().Str("name", name).Msg("Found matching container by app name") + nameMatch = app + nameMatchedApps = append(nameMatchedApps, name) } + return false } - return nameMatch + err := lookup(locatorFunc) + if err != nil { + return nil, err + } + + if domainMatch != nil { + service.log.App.Debug().Str("domain", domain).Msg("Found matching app by domain") + return domainMatch, nil + } + + if nameMatch == nil { + service.log.App.Debug().Str("domain", domain).Msg("No match found for domain, skipping") + return nil, nil + } + + if len(nameMatchedApps) > 1 { + service.log.App.Warn().Str("domain", domain).Strs("apps", nameMatchedApps).Msg("Multiple apps matched domain by name, app names must be unique, using last match") + } + + service.log.App.Debug().Str("domain", domain).Msg("Found matching app by app name") + return nameMatch, nil +} + +func (service *AccessControlsService) lookupStaticACLs(domain string) (*model.App, error) { + return service.getACLs(domain, func(locator func(name string, app *model.App) bool) error { + for app, config := range service.config.Apps { + if ok := locator(app, &config); ok { + return nil + } + } + return nil + }) } func (service *AccessControlsService) GetAccessControls(domain string) (*model.App, error) { // First check in the static config - app := service.lookupStaticACLs(domain) + app, err := service.lookupStaticACLs(domain) + + // Will never return an error here, but we need to check it + if err != nil { + return nil, err + } if app != nil { service.log.App.Debug().Msg("Using static ACLs for app") @@ -74,9 +113,9 @@ 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.labelProvider.GetLabels(domain) + return service.getACLs(domain, service.labelProvider.Lookup) } - // no labels + // No labels return nil, nil } diff --git a/internal/service/access_controls_service_test.go b/internal/service/access_controls_service_test.go index f4f4d24c..30415933 100644 --- a/internal/service/access_controls_service_test.go +++ b/internal/service/access_controls_service_test.go @@ -4,224 +4,184 @@ import ( "errors" "testing" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/tinyauthapp/tinyauth/internal/model" "github.com/tinyauthapp/tinyauth/internal/utils/logger" ) -type mockLabelProvider struct { - getLabelsFn func(appDomain string) (*model.App, error) - calledWith string - callCount int +type mockProvider struct { + acls map[string]model.App + shouldError bool } -func (m *mockLabelProvider) GetLabels(appDomain string) (*model.App, error) { - m.calledWith = appDomain - m.callCount++ - if m.getLabelsFn != nil { - return m.getLabelsFn(appDomain) +func newMockProvider(acls map[string]model.App, shouldError bool) *mockProvider { + return &mockProvider{acls: acls, shouldError: shouldError} +} + +func (m *mockProvider) Lookup(locator func(name string, app *model.App) bool) error { + if m.shouldError { + return errors.New("mock error") + } + for name, app := range m.acls { + if ok := locator(name, &app); ok { + return nil + } } - return nil, nil + return nil } -func TestLookupStaticACLs(t *testing.T) { +func TestAccessControlsService(t *testing.T) { log := logger.NewLogger().WithTestConfig() log.Init() tests := []struct { - name string - apps map[string]model.App - domain string - expectNil bool - expectedDomain string + name string + domain string + acls map[string]model.App + want *model.App }{ { - name: "returns nil when no apps are configured", - apps: nil, - domain: "foo.example.com", - expectNil: true, + name: "returns ACLs for domain", + domain: "example.com", + acls: map[string]model.App{ + "foo": {Config: model.AppConfig{Domain: "example.com"}}, + }, + want: &model.App{Config: model.AppConfig{Domain: "example.com"}}, }, { - name: "returns nil when no app matches", - apps: map[string]model.App{ - "foo": {Config: model.AppConfig{Domain: "foo.example.com"}}, + name: "returns ACLs for domain with port", + domain: "example.com:8080", + acls: map[string]model.App{ + "foo": {Config: model.AppConfig{Domain: "example.com"}}, }, - domain: "bar.example.com", - expectNil: true, + want: &model.App{Config: model.AppConfig{Domain: "example.com"}}, }, { - name: "matches by exact domain", - apps: map[string]model.App{ - "foo": {Config: model.AppConfig{Domain: "foo.example.com"}}, + name: "returns ACLs for domain with trailing dot", + domain: "example.com.", + acls: map[string]model.App{ + "foo": {Config: model.AppConfig{Domain: "example.com"}}, }, - domain: "foo.example.com", - expectedDomain: "foo.example.com", + want: &model.App{Config: model.AppConfig{Domain: "example.com"}}, }, { - name: "matches by app name when domain does not match any app", - apps: map[string]model.App{ - "foo": {Config: model.AppConfig{Domain: "configured.example.com"}}, + name: "returns ACLs for non-ascii domain", + domain: "bücher.example.com", + acls: map[string]model.App{ + "foo": {Config: model.AppConfig{Domain: "bücher.example.com"}}, }, - domain: "foo.example.com", - expectedDomain: "configured.example.com", + want: &model.App{Config: model.AppConfig{Domain: "bücher.example.com"}}, }, { - name: "matches by app name for nested subdomains", - apps: map[string]model.App{ - "foo": {Config: model.AppConfig{Domain: "configured.example.com"}}, + name: "returns ACLs for punycode domain and non-ascii config", + domain: "bücher.example.com", + acls: map[string]model.App{ + "foo": {Config: model.AppConfig{Domain: "xn--bcher-kva.example.com"}}, }, - domain: "foo.sub.example.com", - expectedDomain: "configured.example.com", + want: &model.App{Config: model.AppConfig{Domain: "xn--bcher-kva.example.com"}}, }, { - name: "selects the app matching by domain among multiple apps", - apps: map[string]model.App{ - "unrelated": {Config: model.AppConfig{Domain: "other.example.com"}}, - "target": {Config: model.AppConfig{Domain: "foo.example.com"}}, + name: "returns ACLs with case-insensitive matching", + domain: "Example.com", + acls: map[string]model.App{ + "foo": {Config: model.AppConfig{Domain: "example.com"}}, }, - domain: "foo.example.com", - expectedDomain: "foo.example.com", + want: &model.App{Config: model.AppConfig{Domain: "example.com"}}, + }, + { + name: "falls back to name matching when domain fails", + domain: "app.example.com", + acls: map[string]model.App{ + "app": {Path: model.AppPath{Allow: "/foo"}}, + }, + want: &model.App{Path: model.AppPath{Allow: "/foo"}}, + }, + { + name: "name matching is case-insensitive", + domain: "aPp.example.com", + acls: map[string]model.App{ + "APP": {Path: model.AppPath{Allow: "/foo"}}, + }, + want: &model.App{Path: model.AppPath{Allow: "/foo"}}, + }, + { + name: "returns nil when no ACLs are found", + domain: "example.com", + acls: map[string]model.App{}, + want: nil, }, } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - svc := NewAccessControlsService(AccessControlServiceInput{ + // run once for a mock provider + for _, test := range tests { + t.Run(test.name+"(getACLs)", func(t *testing.T) { + mock := newMockProvider(test.acls, false) + acls := NewAccessControlsService(AccessControlServiceInput{ Log: log, - Config: &model.Config{Apps: tt.apps}, - LabelProvider: nil, + Config: &model.Config{}, + LabelProvider: mock, }) - got := svc.lookupStaticACLs(tt.domain) - if tt.expectNil { - assert.Nil(t, got) - return - } - require.NotNil(t, got) - assert.Equal(t, tt.expectedDomain, got.Config.Domain) + app, err := acls.getACLs(test.domain, mock.Lookup) + require.NoError(t, err) + require.Equal(t, test.want, app) }) } -} -func TestGetAccessControls(t *testing.T) { - log := logger.NewLogger().WithTestConfig() - log.Init() - - t.Run("returns static ACLs when domain matches", func(t *testing.T) { - config := model.Config{ - Apps: map[string]model.App{ - "foo": { - Config: model.AppConfig{Domain: "foo.example.com"}, - Users: model.AppUsers{Allow: "alice"}, + // run again for static acls + for _, test := range tests { + t.Run(test.name+"(staticACLs)", func(t *testing.T) { + acls := NewAccessControlsService(AccessControlServiceInput{ + Log: log, + Config: &model.Config{ + Apps: test.acls, }, - }, - } - svc := NewAccessControlsService(AccessControlServiceInput{ - Log: log, - Config: &config, - LabelProvider: nil, - }) - - got, err := svc.GetAccessControls("foo.example.com") - - require.NoError(t, err) - require.NotNil(t, got) - assert.Equal(t, "foo.example.com", got.Config.Domain) - assert.Equal(t, "alice", got.Users.Allow) - }) - - t.Run("returns nil when no static match and no label provider", func(t *testing.T) { - svc := NewAccessControlsService(AccessControlServiceInput{ - Log: log, - Config: &model.Config{}, - LabelProvider: nil, - }) - - got, err := svc.GetAccessControls("unknown.example.com") - - require.NoError(t, err) - assert.Nil(t, got) - }) - - t.Run("returns nil when label provider pointer wraps a nil interface", func(t *testing.T) { - var provider LabelProvider - svc := NewAccessControlsService(AccessControlServiceInput{ - Log: log, - Config: &model.Config{}, - LabelProvider: provider, // nil provider - }) - - got, err := svc.GetAccessControls("unknown.example.com") - - require.NoError(t, err) - assert.Nil(t, got) - }) - - t.Run("falls back to label provider when no static match", func(t *testing.T) { - expected := &model.App{ - Config: model.AppConfig{Domain: "dynamic.example.com"}, - Users: model.AppUsers{Allow: "bob"}, - } - mock := &mockLabelProvider{ - getLabelsFn: func(appDomain string) (*model.App, error) { - return expected, nil - }, - } - var provider LabelProvider = mock - svc := NewAccessControlsService(AccessControlServiceInput{ - Log: log, - Config: &model.Config{}, - LabelProvider: provider, + }) + app, err := acls.lookupStaticACLs(test.domain) + require.NoError(t, err) + require.Equal(t, test.want, app) }) + } - got, err := svc.GetAccessControls("dynamic.example.com") - - require.NoError(t, err) - assert.Same(t, expected, got) - assert.Equal(t, "dynamic.example.com", mock.calledWith) - assert.Equal(t, 1, mock.callCount) + // get acls should return an error when the provider fails + mock := newMockProvider(map[string]model.App{}, true) + acls := NewAccessControlsService(AccessControlServiceInput{ + Log: log, + Config: &model.Config{}, }) - - t.Run("does not call label provider when static match found", func(t *testing.T) { - mock := &mockLabelProvider{} - var provider LabelProvider = mock - config := model.Config{ + _, err := acls.getACLs("example.com", mock.Lookup) + require.Error(t, err) + + // get access controls should get acls from + // static when static acls are configured + acls = NewAccessControlsService(AccessControlServiceInput{ + Log: log, + Config: &model.Config{ Apps: map[string]model.App{ "foo": {Config: model.AppConfig{Domain: "foo.example.com"}}, }, - } - svc := NewAccessControlsService(AccessControlServiceInput{ - Log: log, - Config: &config, - LabelProvider: provider, - }) - - got, err := svc.GetAccessControls("foo.example.com") - - require.NoError(t, err) - require.NotNil(t, got) - assert.Equal(t, "foo.example.com", got.Config.Domain) - assert.Equal(t, 0, mock.callCount) + }, }) - - t.Run("propagates label provider errors", func(t *testing.T) { - providerErr := errors.New("provider boom") - mock := &mockLabelProvider{ - getLabelsFn: func(appDomain string) (*model.App, error) { - return nil, providerErr - }, - } - var provider LabelProvider = mock - svc := NewAccessControlsService(AccessControlServiceInput{ - Log: log, - Config: &model.Config{}, - LabelProvider: provider, - }) - - got, err := svc.GetAccessControls("dynamic.example.com") - - assert.Nil(t, got) - assert.ErrorIs(t, err, providerErr) - assert.Equal(t, 1, mock.callCount) + app, err := acls.GetAccessControls("foo.example.com") + require.NoError(t, err) + require.Equal(t, &model.App{Config: model.AppConfig{Domain: "foo.example.com"}}, app) + + // should return nil for no apps + app, err = acls.GetAccessControls("bar.example.com") + require.NoError(t, err) + require.Nil(t, app) + + // Should use label provider if available + mock = newMockProvider(map[string]model.App{ + "bar": { + Config: model.AppConfig{Domain: "bar.example.com"}, + }, + }, false) + acls = NewAccessControlsService(AccessControlServiceInput{ + Log: log, + Config: &model.Config{}, + LabelProvider: mock, }) + app, err = acls.GetAccessControls("bar.example.com") + require.NoError(t, err) + require.Equal(t, &model.App{Config: model.AppConfig{Domain: "bar.example.com"}}, app) } diff --git a/internal/service/docker_service.go b/internal/service/docker_service.go index 49708b0d..21265a2e 100644 --- a/internal/service/docker_service.go +++ b/internal/service/docker_service.go @@ -2,7 +2,7 @@ package service import ( "context" - "strings" + "fmt" "github.com/steveiliop56/ding" "github.com/tinyauthapp/tinyauth/internal/model" @@ -31,7 +31,6 @@ type DockerServiceInput struct { } func NewDockerService(i DockerServiceInput) (*DockerService, error) { - client, err := client.NewClientWithOpts(client.FromEnv) if err != nil { return nil, err @@ -68,49 +67,38 @@ func (docker *DockerService) inspectContainer(containerId string) (container.Ins return docker.client.ContainerInspect(docker.context, containerId) } -func (docker *DockerService) GetLabels(appDomain string) (*model.App, 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, nil + return nil } containers, err := docker.getContainers() if err != nil { - return nil, err + return fmt.Errorf("failed to get containers: %w", err) } for _, ctr := range containers { inspect, err := docker.inspectContainer(ctr.ID) if err != nil { - return nil, err + docker.log.App.Error().Err(err).Msgf("Failed to inspect container %s", ctr.ID) + continue } labels, err := decoders.DecodeLabels[model.Apps](inspect.Config.Labels, "apps") if err != nil { - return nil, err + docker.log.App.Warn().Err(err).Msgf("Failed to decode labels for container %s", ctr.ID) + continue } - var nameMatch *model.App - - // First try to find a matching app by domain, then fallback to matching by app name (subdomain) - for appName, appLabels := range labels.Apps { - if appLabels.Config.Domain == appDomain { - docker.log.App.Debug().Str("id", inspect.ID).Str("name", inspect.Name).Msg("Found matching container by domain") - return &appLabels, nil + for app, config := range labels.Apps { + if ok := locator(app, &config); ok { + return nil } - if strings.SplitN(appDomain, ".", 2)[0] == appName { - docker.log.App.Debug().Str("id", inspect.ID).Str("name", inspect.Name).Msg("Found matching container by app name") - nameMatch = &appLabels - } - } - - if nameMatch != nil { - return nameMatch, nil } } - docker.log.App.Debug().Str("domain", appDomain).Msg("No matching container found for domain") - return nil, nil + return nil } func (docker *DockerService) watchAndClose(ctx context.Context) { diff --git a/internal/service/kubernetes_service.go b/internal/service/kubernetes_service.go index f065be72..81229d68 100644 --- a/internal/service/kubernetes_service.go +++ b/internal/service/kubernetes_service.go @@ -12,6 +12,7 @@ import ( "github.com/tinyauthapp/tinyauth/internal/model" "github.com/tinyauthapp/tinyauth/internal/utils/decoders" "github.com/tinyauthapp/tinyauth/internal/utils/logger" + "github.com/tinyauthapp/tinyauth/pkg/validators" "go.uber.org/dig" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -22,31 +23,23 @@ import ( "k8s.io/client-go/rest" ) +type ingressEntry struct { + name string + app model.App +} + type ingressKey struct { namespace string name string } -type ingressAppKey struct { - ingressKey - appName string -} - -type ingressApp struct { - domain string - appName string - app model.App -} - type KubernetesService struct { log *logger.Logger - client dynamic.Interface - started bool - mu sync.RWMutex - ingressApps map[ingressKey][]ingressApp - domainIndex map[string]ingressAppKey - appNameIndex map[string]ingressAppKey + client dynamic.Interface + connected bool + mu sync.RWMutex + ingressEntries map[ingressKey][]ingressEntry } type KubernetesServiceInput struct { @@ -86,90 +79,45 @@ func NewKubernetesService(i KubernetesServiceInput) (*KubernetesService, error) i.Log.App.Debug().Str("api", gvr.GroupVersion().String()).Msg("Successfully accessed Ingress API, starting watcher") service := &KubernetesService{ - log: i.Log, - client: client, - ingressApps: make(map[ingressKey][]ingressApp), - domainIndex: make(map[string]ingressAppKey), - appNameIndex: make(map[string]ingressAppKey), + log: i.Log, + client: client, + ingressEntries: make(map[ingressKey][]ingressEntry), } i.Ding.Go(func(ctx context.Context) { service.watchGVR(gvr, ctx) }, ding.RingMajor) - service.started = true + service.connected = true i.Log.App.Debug().Msg("Kubernetes label provider started successfully") return service, nil } -func (k *KubernetesService) addIngressApps(namespace, name string, apps []ingressApp) { +func (k *KubernetesService) addIngressEntries(key ingressKey, entries []ingressEntry) { k.mu.Lock() defer k.mu.Unlock() - - key := ingressKey{namespace, name} - // Remove existing entries for this ingress - if existing, ok := k.ingressApps[key]; ok { - for _, app := range existing { - delete(k.domainIndex, app.domain) - delete(k.appNameIndex, app.appName) - } - } - // Add new entries - k.ingressApps[key] = apps - for _, app := range apps { - appKey := ingressAppKey{key, app.appName} - k.domainIndex[app.domain] = appKey - k.appNameIndex[app.appName] = appKey - } + k.ingressEntries[key] = entries } -func (k *KubernetesService) removeIngress(namespace, name string) { +func (k *KubernetesService) removeIngress(key ingressKey) { k.mu.Lock() defer k.mu.Unlock() - - key := ingressKey{namespace, name} - if apps, ok := k.ingressApps[key]; ok { - for _, app := range apps { - delete(k.domainIndex, app.domain) - delete(k.appNameIndex, app.appName) - } - delete(k.ingressApps, key) - } + delete(k.ingressEntries, key) } -func (k *KubernetesService) getByDomain(domain string) *model.App { +func (k *KubernetesService) getEntry(locator func(name string, app *model.App) bool) { k.mu.RLock() defer k.mu.RUnlock() - if appKey, ok := k.domainIndex[domain]; ok { - if apps, ok := k.ingressApps[appKey.ingressKey]; ok { - for i := range apps { - app := &apps[i] - if app.domain == domain && app.appName == appKey.appName { - return &app.app - } - } - } - } - return nil -} - -func (k *KubernetesService) getByAppName(appName string) *model.App { - k.mu.RLock() - defer k.mu.RUnlock() - - if appKey, ok := k.appNameIndex[appName]; ok { - if apps, ok := k.ingressApps[appKey.ingressKey]; ok { - for i := range apps { - app := &apps[i] - if app.appName == appName { - return &app.app - } + // O(n^2) is not great but the number of ingress entries is expected to be small + for _, entries := range k.ingressEntries { + for _, entry := range entries { + if ok := locator(entry.name, &entry.app); ok { + return } } } - return nil } func (k *KubernetesService) extractPaths(rule map[string]any) ([]string, error) { @@ -219,7 +167,8 @@ func (k *KubernetesService) extractHosts(item *unstructured.Unstructured) ([]str } paths, err := k.extractPaths(rule) if err != nil { - // This is purely to warn users, it doesn't affect our ability to extract hosts so we won't fail the whole operation + // This is purely to warn users + // It doesn't affect our ability to extract hosts, so we won't fail the whole operation k.log.App.Warn().Err(err).Str("namespace", item.GetNamespace()).Str("name", item.GetName()).Msg("Failed to extract paths from ingress rule") continue } @@ -235,44 +184,72 @@ func (k *KubernetesService) extractHosts(item *unstructured.Unstructured) ([]str } func (k *KubernetesService) updateFromItem(item *unstructured.Unstructured) { - namespace := item.GetNamespace() - name := item.GetName() + key := ingressKey{ + namespace: item.GetNamespace(), + name: item.GetName(), + } + annotations := item.GetAnnotations() if annotations == nil { - k.removeIngress(namespace, name) + k.removeIngress(key) return } + hosts, err := k.extractHosts(item) if err != nil { - k.removeIngress(namespace, name) + k.removeIngress(key) return } + labels, err := decoders.DecodeLabels[model.Apps](annotations, "apps") if err != nil { - k.log.App.Warn().Err(err).Str("namespace", namespace).Str("name", name).Msg("Failed to decode ingress labels, skipping") - k.removeIngress(namespace, name) + k.log.App.Warn().Err(err).Str("namespace", key.namespace).Str("name", key.name).Msg("Failed to decode ingress labels, skipping") + k.removeIngress(key) return } - var apps []ingressApp - for appName, appLabels := range labels.Apps { - if appLabels.Config.Domain == "" { - continue + + var entries []ingressEntry + + v := validators.NewDomainValidator(validators.DomainValidatorOptions{}) + + for name, config := range labels.Apps { + registerApp := len(hosts) == 0 + + if config.Config.Domain != "" { + hostname, err := v.SafeHostname(config.Config.Domain) + if err != nil { + k.log.App.Warn().Err(err).Str("namespace", key.namespace).Str("name", key.name).Str("domain", config.Config.Domain).Msg("Domain is invalid, matching will rely on app name") + } else if slices.Contains(hosts, hostname) { + registerApp = true + } } - if len(hosts) > 0 && !slices.Contains(hosts, appLabels.Config.Domain) { - k.log.App.Warn().Str("namespace", namespace).Str("name", name).Str("appName", appName).Str("domain", appLabels.Config.Domain).Msg("App domain does not match any hosts defined in ingress rules, skipping") + + if !registerApp { + for _, host := range hosts { + if strings.HasPrefix(strings.ToLower(host), strings.ToLower(name+".")) { + registerApp = true + break + } + } + } + + if !registerApp { + k.log.App.Warn().Str("namespace", key.namespace).Str("name", name).Str("appName", name).Msg("App name or domain does not match with ingress") continue } - apps = append(apps, ingressApp{ - domain: appLabels.Config.Domain, - appName: appName, - app: appLabels, + + entries = append(entries, ingressEntry{ + name: name, + app: config, }) } - if len(apps) == 0 { - k.removeIngress(namespace, name) - } else { - k.addIngressApps(namespace, name, apps) + + if len(entries) == 0 { + k.removeIngress(key) + return } + + k.addIngressEntries(key, entries) } func (k *KubernetesService) resyncGVR(gvr schema.GroupVersionResource, ctx context.Context) error { @@ -315,7 +292,10 @@ func (k *KubernetesService) runWatcher(gvr schema.GroupVersionResource, w watch. case watch.Added, watch.Modified: k.updateFromItem(item) case watch.Deleted: - k.removeIngress(item.GetNamespace(), item.GetName()) + k.removeIngress(ingressKey{ + namespace: item.GetNamespace(), + name: item.GetName(), + }) } case <-resyncTicker.C: if err := k.resyncGVR(gvr, ctx); err != nil { @@ -362,25 +342,13 @@ func (k *KubernetesService) watchGVR(gvr schema.GroupVersionResource, ctx contex } } -func (k *KubernetesService) GetLabels(appDomain string) (*model.App, error) { - if !k.started { - k.log.App.Debug().Str("domain", appDomain).Msg("Kubernetes label provider not started, skipping") - return nil, nil +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 } - // First check cache - app := k.getByDomain(appDomain) - if app != nil { - k.log.App.Debug().Str("domain", appDomain).Msg("Found labels in cache by domain") - return app, nil - } - appName := strings.SplitN(appDomain, ".", 2)[0] - app = k.getByAppName(appName) - if app != nil { - k.log.App.Debug().Str("domain", appDomain).Str("appName", appName).Msg("Found labels in cache by app name") - return app, nil - } + k.getEntry(locator) - k.log.App.Debug().Str("domain", appDomain).Msg("No labels found for domain") - return nil, nil + return nil } diff --git a/internal/service/kubernetes_service_test.go b/internal/service/kubernetes_service_test.go index 702fe0f8..ce7b6a79 100644 --- a/internal/service/kubernetes_service_test.go +++ b/internal/service/kubernetes_service_test.go @@ -1,6 +1,7 @@ package service import ( + "strings" "testing" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -25,47 +26,66 @@ func TestKubernetesService(t *testing.T) { description: "Cache by domain returns app and misses unknown domain", run: func(t *testing.T, svc *KubernetesService) { app := model.App{Config: model.AppConfig{Domain: "foo.example.com"}} - svc.addIngressApps("default", "my-ingress", []ingressApp{ - {domain: "foo.example.com", appName: "foo", app: app}, + svc.addIngressEntries(ingressKey{ + namespace: "default", + name: "my-ingress", + }, []ingressEntry{ + { + app: app, + name: "foo", + }, }) - got := svc.getByDomain("foo.example.com") + var got *model.App + svc.getEntry(func(name string, app *model.App) bool { + if app.Config.Domain == "foo.example.com" { + got = app + return true + } + return false + }) require.NotNil(t, got) assert.Equal(t, "foo.example.com", got.Config.Domain) - - got = svc.getByDomain("notfound.example.com") - assert.Nil(t, got) }, }, { - description: "Cache by app name returns app and misses unknown name", + description: "RemoveIngress clears domain and app name entries", run: func(t *testing.T, svc *KubernetesService) { - app := model.App{Config: model.AppConfig{Domain: "bar.example.com"}} - svc.addIngressApps("default", "my-ingress", []ingressApp{ - {domain: "bar.example.com", appName: "bar", app: app}, + app := model.App{Config: model.AppConfig{Domain: "foo.example.com"}} + svc.addIngressEntries(ingressKey{ + namespace: "default", + name: "my-ingress", + }, []ingressEntry{ + { + app: app, + name: "foo", + }, }) - got := svc.getByAppName("bar") + var got *model.App + svc.getEntry(func(name string, app *model.App) bool { + if app.Config.Domain == "foo.example.com" { + got = app + return true + } + return false + }) require.NotNil(t, got) - assert.Equal(t, "bar.example.com", got.Config.Domain) + assert.Equal(t, "foo.example.com", got.Config.Domain) - got = svc.getByAppName("notfound") - assert.Nil(t, got) - }, - }, - { - description: "RemoveIngress clears domain and app name entries", - run: func(t *testing.T, svc *KubernetesService) { - app := model.App{Config: model.AppConfig{Domain: "baz.example.com"}} - svc.addIngressApps("default", "my-ingress", []ingressApp{ - {domain: "baz.example.com", appName: "baz", app: app}, + got = nil + svc.removeIngress(ingressKey{ + namespace: "default", + name: "my-ingress", }) - svc.removeIngress("default", "my-ingress") - - got := svc.getByDomain("baz.example.com") - assert.Nil(t, got) - got = svc.getByAppName("baz") + svc.getEntry(func(name string, app *model.App) bool { + if app.Config.Domain == "foo.example.com" { + got = app + return true + } + return false + }) assert.Nil(t, got) }, }, @@ -73,67 +93,130 @@ func TestKubernetesService(t *testing.T) { description: "AddIngressApps replaces stale entries for the same ingress", run: func(t *testing.T, svc *KubernetesService) { old := model.App{Config: model.AppConfig{Domain: "old.example.com"}} - svc.addIngressApps("default", "my-ingress", []ingressApp{ - {domain: "old.example.com", appName: "old", app: old}, + svc.addIngressEntries(ingressKey{ + namespace: "default", + name: "my-ingress", + }, []ingressEntry{ + { + app: old, + name: "foo", + }, }) updated := model.App{Config: model.AppConfig{Domain: "new.example.com"}} - svc.addIngressApps("default", "my-ingress", []ingressApp{ - {domain: "new.example.com", appName: "new", app: updated}, + svc.addIngressEntries(ingressKey{ + namespace: "default", + name: "my-ingress", + }, []ingressEntry{ + { + app: updated, + name: "foo", + }, }) - got := svc.getByDomain("old.example.com") + var got *model.App + svc.getEntry(func(name string, app *model.App) bool { + if app.Config.Domain == "old.example.com" { + got = app + return true + } + return false + }) assert.Nil(t, got) - got = svc.getByDomain("new.example.com") + svc.getEntry(func(name string, app *model.App) bool { + if app.Config.Domain == "new.example.com" { + got = app + return true + } + return false + }) require.NotNil(t, got) assert.Equal(t, "new.example.com", got.Config.Domain) }, }, { - description: "GetLabels returns app from cache when started", + description: "GetLabels returns app from cache when connected", run: func(t *testing.T, svc *KubernetesService) { - svc.started = true + svc.connected = true app := model.App{Config: model.AppConfig{Domain: "hit.example.com"}} - svc.addIngressApps("default", "ing", []ingressApp{ - {domain: "hit.example.com", appName: "hit", app: app}, + svc.addIngressEntries(ingressKey{ + namespace: "default", + name: "my-ingress", + }, []ingressEntry{ + { + app: app, + name: "foo", + }, }) - got, err := svc.GetLabels("hit.example.com") + var got *model.App + err := svc.Lookup(func(name string, app *model.App) bool { + if app.Config.Domain == "hit.example.com" { + got = app + return true + } + return false + }) require.NoError(t, err) + require.NotNil(t, got) assert.Equal(t, "hit.example.com", got.Config.Domain) }, }, { description: "GetLabels returns empty app on cache miss when started", run: func(t *testing.T, svc *KubernetesService) { - svc.started = true + svc.connected = true - got, err := svc.GetLabels("notfound.example.com") + var got *model.App + err := svc.Lookup(func(name string, app *model.App) bool { + if app.Config.Domain == "notfound.example.com" { + got = app + return true + } + return false + }) require.NoError(t, err) - assert.Nil(t, got) + require.Nil(t, got) }, }, { description: "GetLabels resolves app by app name", run: func(t *testing.T, svc *KubernetesService) { - svc.started = true + svc.connected = true - app := model.App{Config: model.AppConfig{Domain: "myapp.internal.example.com"}} - svc.addIngressApps("default", "ing", []ingressApp{ - {domain: "myapp.internal.example.com", appName: "myapp", app: app}, + app := model.App{Path: model.AppPath{Allow: "/foo"}} + svc.addIngressEntries(ingressKey{ + namespace: "default", + name: "my-ingress", + }, []ingressEntry{ + { + app: app, + name: "foo", + }, }) - got, err := svc.GetLabels("myapp.internal.example.com") + var got *model.App + err := svc.Lookup(func(name string, app *model.App) bool { + if strings.HasPrefix("foo.internal.example.com", "foo.") { + got = app + return true + } + return false + }) require.NoError(t, err) - assert.Equal(t, "myapp.internal.example.com", got.Config.Domain) + require.NotNil(t, got) + assert.Equal(t, "/foo", got.Path.Allow) }, }, { description: "GetLabels returns empty app when service not yet started", run: func(t *testing.T, svc *KubernetesService) { - got, err := svc.GetLabels("anything.example.com") + var got *model.App + err := svc.Lookup(func(name string, app *model.App) bool { + return false + }) require.NoError(t, err) assert.Nil(t, got) }, @@ -151,7 +234,15 @@ func TestKubernetesService(t *testing.T) { svc.updateFromItem(&item) - got := svc.getByDomain("myapp.example.com") + var got *model.App + svc.getEntry(func(name string, app *model.App) bool { + if app.Config.Domain == "myapp.example.com" { + got = app + return true + } + return false + }) + require.NotNil(t, got) assert.Equal(t, "myapp.example.com", got.Config.Domain) assert.Equal(t, "alice", got.Users.Allow) @@ -161,17 +252,354 @@ func TestKubernetesService(t *testing.T) { description: "UpdateFromItem with no annotations removes existing cache entries", run: func(t *testing.T, svc *KubernetesService) { app := model.App{Config: model.AppConfig{Domain: "todelete.example.com"}} - svc.addIngressApps("default", "test-ingress", []ingressApp{ - {domain: "todelete.example.com", appName: "todelete", app: app}, + svc.addIngressEntries(ingressKey{ + namespace: "default", + name: "my-ingress", + }, []ingressEntry{ + { + app: app, + name: "foo", + }, + }) + + item := unstructured.Unstructured{} + item.SetNamespace("default") + item.SetName("my-ingress") + + svc.updateFromItem(&item) + + var got *model.App + svc.getEntry(func(name string, app *model.App) bool { + if app.Config.Domain == "todelete.example.com" { + got = app + return true + } + return false + }) + assert.Nil(t, got) + }, + }, + { + description: "ExtractPaths returns all non empty paths from a rule", + run: func(t *testing.T, svc *KubernetesService) { + rule := map[string]any{ + "http": map[string]any{ + "paths": []any{ + map[string]any{"path": "/"}, + map[string]any{"path": "/api"}, + map[string]any{"path": ""}, + map[string]any{"pathType": "Prefix"}, + "not-a-map", + }, + }, + } + + paths, err := svc.extractPaths(rule) + require.NoError(t, err) + assert.Equal(t, []string{"/", "/api"}, paths) + }, + }, + { + description: "ExtractPaths returns nothing when http or paths are missing", + run: func(t *testing.T, svc *KubernetesService) { + paths, err := svc.extractPaths(map[string]any{}) + require.NoError(t, err) + assert.Empty(t, paths) + + paths, err = svc.extractPaths(map[string]any{ + "http": map[string]any{}, + }) + require.NoError(t, err) + assert.Empty(t, paths) + }, + }, + { + description: "ExtractPaths errors when http is not a map", + run: func(t *testing.T, svc *KubernetesService) { + paths, err := svc.extractPaths(map[string]any{ + "http": "invalid", + }) + require.Error(t, err) + assert.Nil(t, paths) + }, + }, + { + description: "ExtractPaths errors when paths is not a slice", + run: func(t *testing.T, svc *KubernetesService) { + paths, err := svc.extractPaths(map[string]any{ + "http": map[string]any{ + "paths": "invalid", + }, + }) + require.Error(t, err) + assert.Nil(t, paths) + }, + }, + { + description: "ExtractHosts returns hosts from all rules", + run: func(t *testing.T, svc *KubernetesService) { + item := unstructured.Unstructured{} + item.SetNamespace("default") + item.SetName("test-ingress") + require.NoError(t, unstructured.SetNestedSlice(item.Object, []any{ + map[string]any{ + "host": "foo.example.com", + "http": map[string]any{ + "paths": []any{ + map[string]any{"path": "/"}, + }, + }, + }, + map[string]any{ + "host": "bar.example.com", + }, + map[string]any{ + "host": "", + }, + "not-a-map", + }, "spec", "rules")) + + hosts, err := svc.extractHosts(&item) + require.NoError(t, err) + assert.Equal(t, []string{"foo.example.com", "bar.example.com"}, hosts) + }, + }, + { + description: "ExtractHosts still returns hosts when a rule has no catch all path", + run: func(t *testing.T, svc *KubernetesService) { + item := unstructured.Unstructured{} + item.SetNamespace("default") + item.SetName("test-ingress") + require.NoError(t, unstructured.SetNestedSlice(item.Object, []any{ + map[string]any{ + "host": "foo.example.com", + "http": map[string]any{ + "paths": []any{ + map[string]any{"path": "/api"}, + }, + }, + }, + }, "spec", "rules")) + + hosts, err := svc.extractHosts(&item) + require.NoError(t, err) + assert.Equal(t, []string{"foo.example.com"}, hosts) + }, + }, + { + description: "ExtractHosts still returns hosts when path extraction fails", + run: func(t *testing.T, svc *KubernetesService) { + item := unstructured.Unstructured{} + item.SetNamespace("default") + item.SetName("test-ingress") + require.NoError(t, unstructured.SetNestedSlice(item.Object, []any{ + map[string]any{ + "host": "foo.example.com", + "http": "invalid", + }, + }, "spec", "rules")) + + hosts, err := svc.extractHosts(&item) + require.NoError(t, err) + assert.Equal(t, []string{"foo.example.com"}, hosts) + }, + }, + { + description: "ExtractHosts returns nothing when spec.rules is missing", + run: func(t *testing.T, svc *KubernetesService) { + item := unstructured.Unstructured{} + item.SetNamespace("default") + item.SetName("test-ingress") + + hosts, err := svc.extractHosts(&item) + require.NoError(t, err) + assert.Empty(t, hosts) + }, + }, + { + description: "ExtractHosts errors when spec.rules is not a slice", + run: func(t *testing.T, svc *KubernetesService) { + item := unstructured.Unstructured{} + item.SetNamespace("default") + item.SetName("test-ingress") + require.NoError(t, unstructured.SetNestedField(item.Object, "invalid", "spec", "rules")) + + hosts, err := svc.extractHosts(&item) + require.Error(t, err) + assert.Nil(t, hosts) + }, + }, + { + description: "UpdateFromItem registers app when its domain matches an ingress host", + run: func(t *testing.T, svc *KubernetesService) { + item := unstructured.Unstructured{} + item.SetNamespace("default") + item.SetName("test-ingress") + item.SetAnnotations(map[string]string{ + "tinyauth.apps.myapp.config.domain": "myapp.example.com", + }) + require.NoError(t, unstructured.SetNestedSlice(item.Object, []any{ + map[string]any{ + "host": "myapp.example.com", + }, + }, "spec", "rules")) + + svc.updateFromItem(&item) + + var got *model.App + svc.getEntry(func(name string, app *model.App) bool { + if name == "myapp" { + got = app + return true + } + return false + }) + require.NotNil(t, got) + assert.Equal(t, "myapp.example.com", got.Config.Domain) + }, + }, + { + description: "UpdateFromItem registers app when its name matches an ingress host prefix", + run: func(t *testing.T, svc *KubernetesService) { + item := unstructured.Unstructured{} + item.SetNamespace("default") + item.SetName("test-ingress") + item.SetAnnotations(map[string]string{ + "tinyauth.apps.myapp.users.allow": "alice", + }) + require.NoError(t, unstructured.SetNestedSlice(item.Object, []any{ + map[string]any{ + "host": "MyApp.example.com", + }, + }, "spec", "rules")) + + svc.updateFromItem(&item) + + var got *model.App + svc.getEntry(func(name string, app *model.App) bool { + if name == "myapp" { + got = app + return true + } + return false + }) + require.NotNil(t, got) + assert.Equal(t, "alice", got.Users.Allow) + }, + }, + { + description: "UpdateFromItem skips apps that match neither host nor name", + run: func(t *testing.T, svc *KubernetesService) { + item := unstructured.Unstructured{} + item.SetNamespace("default") + item.SetName("test-ingress") + item.SetAnnotations(map[string]string{ + "tinyauth.apps.myapp.config.domain": "myapp.example.com", }) + require.NoError(t, unstructured.SetNestedSlice(item.Object, []any{ + map[string]any{ + "host": "other.example.com", + }, + }, "spec", "rules")) + svc.updateFromItem(&item) + + var got *model.App + svc.getEntry(func(name string, app *model.App) bool { + got = app + return true + }) + assert.Nil(t, got) + }, + }, + { + description: "UpdateFromItem falls back to app name when the domain is invalid", + run: func(t *testing.T, svc *KubernetesService) { item := unstructured.Unstructured{} item.SetNamespace("default") item.SetName("test-ingress") + item.SetAnnotations(map[string]string{ + "tinyauth.apps.myapp.config.domain": "not a domain", + }) + require.NoError(t, unstructured.SetNestedSlice(item.Object, []any{ + map[string]any{ + "host": "myapp.example.com", + }, + }, "spec", "rules")) + + svc.updateFromItem(&item) + + var got *model.App + svc.getEntry(func(name string, app *model.App) bool { + if name == "myapp" { + got = app + return true + } + return false + }) + require.NotNil(t, got) + }, + }, + { + description: "UpdateFromItem removes entries when host extraction fails", + run: func(t *testing.T, svc *KubernetesService) { + key := ingressKey{ + namespace: "default", + name: "test-ingress", + } + svc.addIngressEntries(key, []ingressEntry{ + { + app: model.App{Config: model.AppConfig{Domain: "stale.example.com"}}, + name: "foo", + }, + }) + + item := unstructured.Unstructured{} + item.SetNamespace(key.namespace) + item.SetName(key.name) + item.SetAnnotations(map[string]string{ + "tinyauth.apps.myapp.config.domain": "myapp.example.com", + }) + require.NoError(t, unstructured.SetNestedField(item.Object, "invalid", "spec", "rules")) svc.updateFromItem(&item) - got := svc.getByDomain("todelete.example.com") + var got *model.App + svc.getEntry(func(name string, app *model.App) bool { + got = app + return true + }) + assert.Nil(t, got) + }, + }, + { + description: "UpdateFromItem removes entries when annotations are not decodable", + run: func(t *testing.T, svc *KubernetesService) { + key := ingressKey{ + namespace: "default", + name: "test-ingress", + } + svc.addIngressEntries(key, []ingressEntry{ + { + app: model.App{Config: model.AppConfig{Domain: "stale.example.com"}}, + name: "foo", + }, + }) + + item := unstructured.Unstructured{} + item.SetNamespace(key.namespace) + item.SetName(key.name) + item.SetAnnotations(map[string]string{ + "tinyauth.apps.myapp.config.oauthWhitelist": "[", + }) + + svc.updateFromItem(&item) + + var got *model.App + svc.getEntry(func(name string, app *model.App) bool { + got = app + return true + }) assert.Nil(t, got) }, }, @@ -180,10 +608,8 @@ func TestKubernetesService(t *testing.T) { for _, test := range tests { t.Run(test.description, func(t *testing.T) { svc := &KubernetesService{ - ingressApps: make(map[ingressKey][]ingressApp), - domainIndex: make(map[string]ingressAppKey), - appNameIndex: make(map[string]ingressAppKey), - log: log, + ingressEntries: make(map[ingressKey][]ingressEntry), + log: log, } test.run(t, svc) })