From 9608fe1f2e4b57dc3c1128dd909ff6ee061db738 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 13 Feb 2026 19:27:47 +0000 Subject: [PATCH 1/2] aggregated/coder: add namespace lister to client providers --- .../aggregated/coder/controlplane_provider.go | 41 +++++ .../coder/controlplane_provider_test.go | 155 ++++++++++++++++++ internal/aggregated/coder/provider.go | 19 +++ internal/aggregated/coder/provider_test.go | 52 ++++++ 4 files changed, 267 insertions(+) diff --git a/internal/aggregated/coder/controlplane_provider.go b/internal/aggregated/coder/controlplane_provider.go index 23e4f309..acf06650 100644 --- a/internal/aggregated/coder/controlplane_provider.go +++ b/internal/aggregated/coder/controlplane_provider.go @@ -5,6 +5,7 @@ import ( "fmt" "log" "net/url" + "sort" "strings" "time" @@ -26,6 +27,7 @@ type ControlPlaneClientProvider struct { var ( _ ClientProvider = (*ControlPlaneClientProvider)(nil) _ NamespaceResolver = (*ControlPlaneClientProvider)(nil) + _ NamespaceLister = (*ControlPlaneClientProvider)(nil) ) // NewControlPlaneClientProvider constructs a dynamic ClientProvider backed by CoderControlPlane resources. @@ -206,6 +208,45 @@ func (p *ControlPlaneClientProvider) DefaultNamespace(ctx context.Context) (stri } } +// EligibleNamespaces returns namespaces served by eligible CoderControlPlane instances. +func (p *ControlPlaneClientProvider) EligibleNamespaces(ctx context.Context) ([]string, error) { + if p == nil { + return nil, fmt.Errorf("assertion failed: control plane client provider must not be nil") + } + if ctx == nil { + return nil, fmt.Errorf("assertion failed: context must not be nil") + } + + eligible, err := p.findEligibleControlPlanes(ctx, "") + if err != nil { + return nil, err + } + if len(eligible) == 0 { + return nil, apierrors.NewServiceUnavailable(noEligibleControlPlaneMessage("")) + } + + byNamespace := make(map[string]int, len(eligible)) + for i := range eligible { + namespace := strings.TrimSpace(eligible[i].Namespace) + if namespace == "" { + return nil, fmt.Errorf("assertion failed: eligible CoderControlPlane namespace must not be empty") + } + byNamespace[namespace]++ + } + + namespaces := make([]string, 0, len(byNamespace)) + for namespace, count := range byNamespace { + if count > 1 { + return nil, apierrors.NewBadRequest(multipleEligibleControlPlaneMessage(namespace)) + } + namespaces = append(namespaces, namespace) + } + + sort.Strings(namespaces) + + return namespaces, nil +} + func (p *ControlPlaneClientProvider) findEligibleControlPlanes( ctx context.Context, namespace string, diff --git a/internal/aggregated/coder/controlplane_provider_test.go b/internal/aggregated/coder/controlplane_provider_test.go index 701ff94e..90bfc68b 100644 --- a/internal/aggregated/coder/controlplane_provider_test.go +++ b/internal/aggregated/coder/controlplane_provider_test.go @@ -266,6 +266,161 @@ func TestControlPlaneClientProviderDefaultNamespaceReturnsBadRequestForMultipleE } } +func TestControlPlaneClientProviderEligibleNamespacesMultipleNamespaces(t *testing.T) { + t.Parallel() + + ignoredControlPlane := eligibleControlPlane("team-a", "coder-disabled") + ignoredControlPlane.Spec.OperatorAccess.Disabled = true + + provider, secretReader := newControlPlaneProviderForTest( + t, + []coderv1alpha1.CoderControlPlane{ + eligibleControlPlane("team-a", "coder-a"), + eligibleControlPlane("team-b", "coder-b"), + ignoredControlPlane, + }, + nil, + ) + + namespaces, err := provider.EligibleNamespaces(context.Background()) + if err != nil { + t.Fatalf("resolve eligible namespaces: %v", err) + } + if got, want := secretReader.getCalls, 0; got != want { + t.Fatalf("expected %d secret reads, got %d", want, got) + } + if got, want := len(namespaces), 2; got != want { + t.Fatalf("expected %d namespaces, got %d: %v", want, got, namespaces) + } + if got, want := namespaces[0], "team-a"; got != want { + t.Fatalf("expected first namespace %q, got %q", want, got) + } + if got, want := namespaces[1], "team-b"; got != want { + t.Fatalf("expected second namespace %q, got %q", want, got) + } +} + +func TestControlPlaneClientProviderEligibleNamespacesNoEligible(t *testing.T) { + t.Parallel() + + provider, secretReader := newControlPlaneProviderForTest(t, nil, nil) + + namespaces, err := provider.EligibleNamespaces(context.Background()) + if err == nil { + t.Fatal("expected error") + } + if namespaces != nil { + t.Fatalf("expected nil namespaces on error, got %v", namespaces) + } + if got, want := secretReader.getCalls, 0; got != want { + t.Fatalf("expected %d secret reads, got %d", want, got) + } + if !apierrors.IsServiceUnavailable(err) { + t.Fatalf("expected ServiceUnavailable, got %v", err) + } + if !strings.Contains(err.Error(), "no eligible CoderControlPlane") { + t.Fatalf("expected no-eligible message, got %v", err) + } +} + +func TestControlPlaneClientProviderEligibleNamespacesDuplicateInNamespace(t *testing.T) { + t.Parallel() + + provider, secretReader := newControlPlaneProviderForTest( + t, + []coderv1alpha1.CoderControlPlane{ + eligibleControlPlane("team-a", "coder-a"), + eligibleControlPlane("team-a", "coder-b"), + }, + nil, + ) + + namespaces, err := provider.EligibleNamespaces(context.Background()) + if err == nil { + t.Fatal("expected error") + } + if namespaces != nil { + t.Fatalf("expected nil namespaces on error, got %v", namespaces) + } + if got, want := secretReader.getCalls, 0; got != want { + t.Fatalf("expected %d secret reads, got %d", want, got) + } + if !apierrors.IsBadRequest(err) { + t.Fatalf("expected BadRequest, got %v", err) + } + if !strings.Contains(err.Error(), "multi-instance support is planned") { + t.Fatalf("expected multi-instance message, got %v", err) + } +} + +func TestControlPlaneClientProviderEligibleNamespacesSingleNamespace(t *testing.T) { + t.Parallel() + + provider, secretReader := newControlPlaneProviderForTest( + t, + []coderv1alpha1.CoderControlPlane{eligibleControlPlane("team-a", "coder-a")}, + nil, + ) + + namespaces, err := provider.EligibleNamespaces(context.Background()) + if err != nil { + t.Fatalf("resolve eligible namespaces: %v", err) + } + if got, want := secretReader.getCalls, 0; got != want { + t.Fatalf("expected %d secret reads, got %d", want, got) + } + if got, want := len(namespaces), 1; got != want { + t.Fatalf("expected %d namespace, got %d", want, got) + } + if got, want := namespaces[0], "team-a"; got != want { + t.Fatalf("expected namespace %q, got %q", want, got) + } +} + +func TestControlPlaneClientProviderEligibleNamespacesAssertions(t *testing.T) { + t.Parallel() + + provider, _ := newControlPlaneProviderForTest(t, nil, nil) + + tests := []struct { + name string + provider *ControlPlaneClientProvider + ctx context.Context + wantErrContains string + }{ + { + name: "rejects nil provider", + provider: nil, + ctx: context.Background(), + wantErrContains: "assertion failed: control plane client provider must not be nil", + }, + { + name: "rejects nil context", + provider: provider, + ctx: nil, + wantErrContains: "assertion failed: context must not be nil", + }, + } + + for _, testCase := range tests { + testCase := testCase + t.Run(testCase.name, func(t *testing.T) { + t.Parallel() + + namespaces, err := testCase.provider.EligibleNamespaces(testCase.ctx) + if err == nil { + t.Fatalf("expected error containing %q, got nil", testCase.wantErrContains) + } + if namespaces != nil { + t.Fatalf("expected nil namespaces on error, got %v", namespaces) + } + if !strings.Contains(err.Error(), testCase.wantErrContains) { + t.Fatalf("expected error containing %q, got %q", testCase.wantErrContains, err.Error()) + } + }) + } +} + func TestControlPlaneClientProviderClientForNamespaceDefaultsSecretKeyToToken(t *testing.T) { t.Parallel() diff --git a/internal/aggregated/coder/provider.go b/internal/aggregated/coder/provider.go index a50a6795..58a5397b 100644 --- a/internal/aggregated/coder/provider.go +++ b/internal/aggregated/coder/provider.go @@ -22,6 +22,12 @@ type NamespaceResolver interface { DefaultNamespace(ctx context.Context) (string, error) } +// NamespaceLister can enumerate namespaces served by a ClientProvider. +// Used to implement all-namespaces LIST by fanning out across instances. +type NamespaceLister interface { + EligibleNamespaces(ctx context.Context) ([]string, error) +} + // StaticClientProvider returns one static client, optionally restricted to one namespace. type StaticClientProvider struct { Client *codersdk.Client @@ -31,6 +37,7 @@ type StaticClientProvider struct { var ( _ ClientProvider = (*StaticClientProvider)(nil) _ NamespaceResolver = (*StaticClientProvider)(nil) + _ NamespaceLister = (*StaticClientProvider)(nil) ) // ClientForNamespace returns the static client. @@ -77,6 +84,18 @@ func (p *StaticClientProvider) DefaultNamespace(_ context.Context) (string, erro return p.Namespace, nil } +// EligibleNamespaces returns namespaces served by the static provider. +func (p *StaticClientProvider) EligibleNamespaces(_ context.Context) ([]string, error) { + if p == nil { + return nil, fmt.Errorf("assertion failed: static client provider must not be nil") + } + if p.Namespace == "" { + return nil, apierrors.NewServiceUnavailable("static provider has no default namespace") + } + + return []string{p.Namespace}, nil +} + // NewStaticClientProvider creates a StaticClientProvider from cfg and optional namespace restriction. func NewStaticClientProvider(cfg Config, namespace string) (*StaticClientProvider, error) { client, err := NewSDKClient(cfg) diff --git a/internal/aggregated/coder/provider_test.go b/internal/aggregated/coder/provider_test.go index dadd46f8..c3e8ef7a 100644 --- a/internal/aggregated/coder/provider_test.go +++ b/internal/aggregated/coder/provider_test.go @@ -209,6 +209,58 @@ func TestStaticClientProviderDefaultNamespaceAssertions(t *testing.T) { } } +func TestStaticClientProviderEligibleNamespaces(t *testing.T) { + t.Parallel() + + provider := &StaticClientProvider{Namespace: "control-plane"} + namespaces, err := provider.EligibleNamespaces(context.Background()) + if err != nil { + t.Fatalf("resolve eligible namespaces: %v", err) + } + if got, want := len(namespaces), 1; got != want { + t.Fatalf("expected %d namespace, got %d", want, got) + } + if got, want := namespaces[0], "control-plane"; got != want { + t.Fatalf("expected namespace %q, got %q", want, got) + } +} + +func TestStaticClientProviderEligibleNamespacesNoNamespace(t *testing.T) { + t.Parallel() + + provider := &StaticClientProvider{} + namespaces, err := provider.EligibleNamespaces(context.Background()) + if err == nil { + t.Fatal("expected error") + } + if namespaces != nil { + t.Fatalf("expected nil namespaces on error, got %v", namespaces) + } + if !apierrors.IsServiceUnavailable(err) { + t.Fatalf("expected ServiceUnavailable, got %v", err) + } + if !strings.Contains(err.Error(), "static provider has no default namespace") { + t.Fatalf("expected missing namespace message, got %v", err) + } +} + +func TestStaticClientProviderEligibleNamespacesNilReceiver(t *testing.T) { + t.Parallel() + + var provider *StaticClientProvider + namespaces, err := provider.EligibleNamespaces(context.Background()) + if err == nil { + t.Fatal("expected error") + } + if namespaces != nil { + t.Fatalf("expected nil namespaces on error, got %v", namespaces) + } + wantErrContains := "assertion failed: static client provider must not be nil" + if !strings.Contains(err.Error(), wantErrContains) { + t.Fatalf("expected error containing %q, got %q", wantErrContains, err.Error()) + } +} + func TestNewStaticClientProvider(t *testing.T) { t.Parallel() From eb6b150f8988154f7765a88697e09405d520cf6a Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 13 Feb 2026 19:34:44 +0000 Subject: [PATCH 2/2] storage: fan out all-namespace list across providers --- internal/aggregated/storage/doc.go | 2 + internal/aggregated/storage/storage_test.go | 170 +++++++++++++++++++- internal/aggregated/storage/template.go | 43 +++++ internal/aggregated/storage/workspace.go | 43 +++++ 4 files changed, 256 insertions(+), 2 deletions(-) diff --git a/internal/aggregated/storage/doc.go b/internal/aggregated/storage/doc.go index 37855a7d..dcbbb885 100644 --- a/internal/aggregated/storage/doc.go +++ b/internal/aggregated/storage/doc.go @@ -9,4 +9,6 @@ // while Kubernetes object names allow dots (DNS-1123 subdomains). // - A single admin session token is used for all API calls (no per-request impersonation in v1). // - Storage resolves the backing codersdk.Client via a ClientProvider interface. +// - All-namespaces LIST aggregates results across eligible CoderControlPlane namespaces +// when the provider implements NamespaceLister. package storage diff --git a/internal/aggregated/storage/storage_test.go b/internal/aggregated/storage/storage_test.go index 5ce6eb77..3792dad7 100644 --- a/internal/aggregated/storage/storage_test.go +++ b/internal/aggregated/storage/storage_test.go @@ -1115,6 +1115,88 @@ func TestTemplateStorageListAllowsAllNamespacesRequest(t *testing.T) { } } +func TestTemplateStorageListAggregatesAcrossNamespaces(t *testing.T) { + t.Parallel() + + serverA, _ := newMockCoderServer(t) + defer serverA.Close() + serverB, _ := newMockCoderServer(t) + defer serverB.Close() + + provider := &multiNamespaceTestProvider{ + clients: map[string]*codersdk.Client{ + "ns-a": newTestSDKClient(t, serverA.URL), + "ns-b": newTestSDKClient(t, serverB.URL), + }, + namespaces: []string{"ns-a", "ns-b"}, + } + + templateStorage := NewTemplateStorage(provider) + + listObj, err := templateStorage.List(namespacedContext(""), nil) + if err != nil { + t.Fatalf("expected all-namespaces list to aggregate successfully, got %v", err) + } + + list, ok := listObj.(*aggregationv1alpha1.CoderTemplateList) + if !ok { + t.Fatalf("expected *CoderTemplateList, got %T", listObj) + } + if len(list.Items) != 2 { + t.Fatalf("expected two templates in aggregated list, got %d", len(list.Items)) + } + if list.Items[0].Namespace != "ns-a" || list.Items[1].Namespace != "ns-b" { + t.Fatalf( + "expected namespaces [ns-a ns-b], got [%s %s]", + list.Items[0].Namespace, + list.Items[1].Namespace, + ) + } + if !sort.SliceIsSorted(list.Items, func(i, j int) bool { + if list.Items[i].Namespace != list.Items[j].Namespace { + return list.Items[i].Namespace < list.Items[j].Namespace + } + return list.Items[i].Name < list.Items[j].Name + }) { + t.Fatal("expected template list items to be sorted by namespace then name") + } +} + +func TestTemplateStorageListNamespacedRequestBypassesFanOut(t *testing.T) { + t.Parallel() + + serverA, _ := newMockCoderServer(t) + defer serverA.Close() + serverB, _ := newMockCoderServer(t) + defer serverB.Close() + + provider := &multiNamespaceTestProvider{ + clients: map[string]*codersdk.Client{ + "ns-a": newTestSDKClient(t, serverA.URL), + "ns-b": newTestSDKClient(t, serverB.URL), + }, + namespaces: []string{"ns-a", "ns-b"}, + } + + templateStorage := NewTemplateStorage(provider) + + listObj, err := templateStorage.List(namespacedContext("ns-a"), nil) + if err != nil { + t.Fatalf("expected namespaced list to succeed, got %v", err) + } + + list, ok := listObj.(*aggregationv1alpha1.CoderTemplateList) + if !ok { + t.Fatalf("expected *CoderTemplateList, got %T", listObj) + } + if len(list.Items) != 1 { + t.Fatalf("expected one template from namespaced list, got %d", len(list.Items)) + } + if list.Items[0].Namespace != "ns-a" { + t.Fatalf("expected namespaced list item to use ns-a, got %q", list.Items[0].Namespace) + } +} + func TestTemplateStorageListPreservesProviderStatusErrors(t *testing.T) { t.Parallel() @@ -2497,6 +2579,53 @@ func TestWorkspaceStorageListAllowsAllNamespacesRequest(t *testing.T) { } } +func TestWorkspaceStorageListAggregatesAcrossNamespaces(t *testing.T) { + t.Parallel() + + serverA, _ := newMockCoderServer(t) + defer serverA.Close() + serverB, _ := newMockCoderServer(t) + defer serverB.Close() + + provider := &multiNamespaceTestProvider{ + clients: map[string]*codersdk.Client{ + "ns-a": newTestSDKClient(t, serverA.URL), + "ns-b": newTestSDKClient(t, serverB.URL), + }, + namespaces: []string{"ns-a", "ns-b"}, + } + + workspaceStorage := NewWorkspaceStorage(provider) + + listObj, err := workspaceStorage.List(namespacedContext(""), nil) + if err != nil { + t.Fatalf("expected all-namespaces list to aggregate successfully, got %v", err) + } + + list, ok := listObj.(*aggregationv1alpha1.CoderWorkspaceList) + if !ok { + t.Fatalf("expected *CoderWorkspaceList, got %T", listObj) + } + if len(list.Items) != 2 { + t.Fatalf("expected two workspaces in aggregated list, got %d", len(list.Items)) + } + if list.Items[0].Namespace != "ns-a" || list.Items[1].Namespace != "ns-b" { + t.Fatalf( + "expected namespaces [ns-a ns-b], got [%s %s]", + list.Items[0].Namespace, + list.Items[1].Namespace, + ) + } + if !sort.SliceIsSorted(list.Items, func(i, j int) bool { + if list.Items[i].Namespace != list.Items[j].Namespace { + return list.Items[i].Namespace < list.Items[j].Namespace + } + return list.Items[i].Name < list.Items[j].Name + }) { + t.Fatal("expected workspace list items to be sorted by namespace then name") + } +} + func TestWorkspaceStorageListPreservesProviderStatusErrors(t *testing.T) { t.Parallel() @@ -2534,6 +2663,37 @@ func assertTopLevelStatusError(t *testing.T, err error) { } } +type multiNamespaceTestProvider struct { + clients map[string]*codersdk.Client + namespaces []string +} + +var ( + _ coder.ClientProvider = (*multiNamespaceTestProvider)(nil) + _ coder.NamespaceLister = (*multiNamespaceTestProvider)(nil) +) + +func (p *multiNamespaceTestProvider) ClientForNamespace(_ context.Context, namespace string) (*codersdk.Client, error) { + if p == nil { + return nil, fmt.Errorf("assertion failed: multi namespace provider must not be nil") + } + + client, ok := p.clients[namespace] + if !ok { + return nil, apierrors.NewNotFound(aggregationv1alpha1.Resource("namespace"), namespace) + } + + return client, nil +} + +func (p *multiNamespaceTestProvider) EligibleNamespaces(_ context.Context) ([]string, error) { + if p == nil { + return nil, fmt.Errorf("assertion failed: multi namespace provider must not be nil") + } + + return p.namespaces, nil +} + type testUpdatedObjectInfo struct { obj runtime.Object err error @@ -3622,7 +3782,7 @@ func zipEntryMode(t *testing.T, sourceZip []byte, path string) (fs.FileMode, boo return 0, false } -func newTestClientProvider(t *testing.T, serverURL string) coder.ClientProvider { +func newTestSDKClient(t *testing.T, serverURL string) *codersdk.Client { t.Helper() parsedURL, err := url.Parse(serverURL) @@ -3633,7 +3793,13 @@ func newTestClientProvider(t *testing.T, serverURL string) coder.ClientProvider client := codersdk.New(parsedURL) client.SetSessionToken("test-session-token") - return &coder.StaticClientProvider{Client: client, Namespace: "control-plane"} + return client +} + +func newTestClientProvider(t *testing.T, serverURL string) coder.ClientProvider { + t.Helper() + + return &coder.StaticClientProvider{Client: newTestSDKClient(t, serverURL), Namespace: "control-plane"} } func namespacedContext(namespace string) context.Context { diff --git a/internal/aggregated/storage/template.go b/internal/aggregated/storage/template.go index 55e25049..4ccf78bc 100644 --- a/internal/aggregated/storage/template.go +++ b/internal/aggregated/storage/template.go @@ -7,6 +7,7 @@ import ( "fmt" "os" "reflect" + "sort" "strings" "sync" "time" @@ -188,6 +189,48 @@ func (s *TemplateStorage) List(ctx context.Context, _ *metainternalversion.ListO return nil, badNamespaceErr } + if namespace == "" { + if lister, ok := s.provider.(coder.NamespaceLister); ok { + namespaces, err := lister.EligibleNamespaces(ctx) + if err != nil { + return nil, err + } + + list := &aggregationv1alpha1.CoderTemplateList{ + TypeMeta: metav1.TypeMeta{ + Kind: "CoderTemplateList", + APIVersion: aggregationv1alpha1.SchemeGroupVersion.String(), + }, + Items: make([]aggregationv1alpha1.CoderTemplate, 0), + } + + for _, eligibleNamespace := range namespaces { + sdk, err := s.clientForNamespace(ctx, eligibleNamespace) + if err != nil { + return nil, wrapClientError(err) + } + + templates, err := sdk.Templates(ctx, codersdk.TemplateFilter{}) + if err != nil { + return nil, coder.MapCoderError(err, aggregationv1alpha1.Resource("codertemplates"), "") + } + + for _, template := range templates { + list.Items = append(list.Items, *convert.TemplateToK8s(eligibleNamespace, template)) + } + } + + sort.Slice(list.Items, func(i, j int) bool { + if list.Items[i].Namespace != list.Items[j].Namespace { + return list.Items[i].Namespace < list.Items[j].Namespace + } + return list.Items[i].Name < list.Items[j].Name + }) + + return list, nil + } + } + responseNamespace, responseNamespaceErr := namespaceForListConversion(ctx, namespace, s.provider) if responseNamespaceErr != nil { return nil, responseNamespaceErr diff --git a/internal/aggregated/storage/workspace.go b/internal/aggregated/storage/workspace.go index 0c168523..29b572a6 100644 --- a/internal/aggregated/storage/workspace.go +++ b/internal/aggregated/storage/workspace.go @@ -3,6 +3,7 @@ package storage import ( "context" "fmt" + "sort" "sync" "time" @@ -152,6 +153,48 @@ func (s *WorkspaceStorage) List(ctx context.Context, _ *metainternalversion.List return nil, badNamespaceErr } + if namespace == "" { + if lister, ok := s.provider.(coder.NamespaceLister); ok { + namespaces, err := lister.EligibleNamespaces(ctx) + if err != nil { + return nil, err + } + + list := &aggregationv1alpha1.CoderWorkspaceList{ + TypeMeta: metav1.TypeMeta{ + Kind: "CoderWorkspaceList", + APIVersion: aggregationv1alpha1.SchemeGroupVersion.String(), + }, + Items: make([]aggregationv1alpha1.CoderWorkspace, 0), + } + + for _, eligibleNamespace := range namespaces { + sdk, err := s.clientForNamespace(ctx, eligibleNamespace) + if err != nil { + return nil, wrapClientError(err) + } + + workspacesResponse, err := sdk.Workspaces(ctx, codersdk.WorkspaceFilter{}) + if err != nil { + return nil, coder.MapCoderError(err, aggregationv1alpha1.Resource("coderworkspaces"), "") + } + + for _, workspace := range workspacesResponse.Workspaces { + list.Items = append(list.Items, *convert.WorkspaceToK8s(eligibleNamespace, workspace)) + } + } + + sort.Slice(list.Items, func(i, j int) bool { + if list.Items[i].Namespace != list.Items[j].Namespace { + return list.Items[i].Namespace < list.Items[j].Namespace + } + return list.Items[i].Name < list.Items[j].Name + }) + + return list, nil + } + } + responseNamespace, responseNamespaceErr := namespaceForListConversion(ctx, namespace, s.provider) if responseNamespaceErr != nil { return nil, responseNamespaceErr