diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index a0919919..2a15c1cc 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -27,6 +27,7 @@ import ( corev1 "k8s.io/api/core/v1" policyv1 "k8s.io/api/policy/v1" rbacv1 "k8s.io/api/rbac/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" @@ -181,7 +182,6 @@ func NewController(ctx context.Context, registry *typed.Registry, dclient dynami for _, gvr := range []schema.GroupVersionResource{ appsv1.SchemeGroupVersion.WithResource("deployments"), - corev1.SchemeGroupVersion.WithResource("secrets"), corev1.SchemeGroupVersion.WithResource("serviceaccounts"), corev1.SchemeGroupVersion.WithResource("services"), corev1.SchemeGroupVersion.WithResource("pods"), @@ -202,6 +202,27 @@ func NewController(ctx context.Context, registry *typed.Registry, dclient dynami return nil, err } } + // Secrets get per-type indexes (one per credential role) so that each + // adoption handler only sees secrets of its own type and never + // misidentifies another role's secret as stale. OwningClusterIndex is + // kept for the legacy SecretRef path and for syncExternalResource, which + // uses it to re-enqueue the owning cluster when any secret changes. + secretsInf := externalInformerFactory.ForResource(corev1.SchemeGroupVersion.WithResource("secrets")).Informer() + if err := secretsInf.AddIndexers(cache.Indexers{ + metadata.OwningClusterIndex: metadata.GetClusterKeyFromMeta, + metadata.OwningClusterDatastoreURIIndex: metadata.GetClusterKeyFromMetaForType(metadata.CredentialTypeDatastoreURI), + metadata.OwningClusterPresharedKeyIndex: metadata.GetClusterKeyFromMetaForType(metadata.CredentialTypePresharedKey), + metadata.OwningClusterMigrationSecretsIndex: metadata.GetClusterKeyFromMetaForType(metadata.CredentialTypeMigrationSecrets), + }); err != nil { + return nil, err + } + if _, err := secretsInf.AddEventHandler(cache.ResourceEventHandlerFuncs{ + AddFunc: func(obj any) { c.syncExternalResource(obj) }, + UpdateFunc: func(_, obj any) { c.syncExternalResource(obj) }, + DeleteFunc: func(obj any) { c.syncExternalResource(obj) }, + }); err != nil { + return nil, err + } externalInformerFactories = append(externalInformerFactories, externalInformerFactory) } @@ -414,6 +435,40 @@ func credentialSecretNames(cluster *v1alpha1.SpiceDBCluster) []string { return names } +type credentialSecret struct { + name string + credType string +} + +// credentialSecretsForCluster returns one entry per non-skipped credential ref, +// preserving the credential type for each. Unlike credentialSecretNames, the +// same name may appear more than once when a secret is shared across multiple +// roles — each role runs its own adoption handler with its own type label. +func credentialSecretsForCluster(cluster *v1alpha1.SpiceDBCluster) []credentialSecret { + creds := cluster.Spec.Credentials + if creds == nil && cluster.Spec.SecretRef != "" { + return []credentialSecret{{name: cluster.Spec.SecretRef, credType: ""}} + } + if creds == nil { + return nil + } + roleSecrets := []struct { + ref *v1alpha1.CredentialRef + credType string + }{ + {creds.DatastoreURI, metadata.CredentialTypeDatastoreURI}, + {creds.PresharedKey, metadata.CredentialTypePresharedKey}, + {creds.MigrationSecrets, metadata.CredentialTypeMigrationSecrets}, + } + var result []credentialSecret + for _, rs := range roleSecrets { + if rs.ref != nil && !rs.ref.Skip && rs.ref.SecretName != "" { + result = append(result, credentialSecret{name: rs.ref.SecretName, credType: rs.credType}) + } + } + return result +} + // syncExternalResource is called when a dependent resource is updated: // It queues the owning SpiceDBCluster for reconciliation based on the labels. // No other reconciliation should take place here; we keep a single state @@ -576,21 +631,35 @@ func (c *Controller) selfPauseCluster(...handler.Handler) handler.Handler { func (c *Controller) secretAdopter(next ...handler.Handler) handler.Handler { secretsGVR := corev1.SchemeGroupVersion.WithResource("secrets") return handler.NewHandlerFromFunc(func(ctx context.Context) { - names := CtxSecretNames.Value(ctx) - if len(names) == 0 { + cluster := CtxCluster.MustValue(ctx) + pairs := credentialSecretsForCluster(cluster) + if len(pairs) == 0 { handler.Handlers(next).MustOne().Handle(ctx) return } - cluster := CtxCluster.MustValue(ctx) - for _, name := range names { + for _, cs := range pairs { + credType := cs.credType secretCtx := CtxSecretNN.WithValue(ctx, types.NamespacedName{ - Name: name, + Name: cs.name, Namespace: cluster.Namespace, }) NewSecretAdoptionHandler( c.Recorder, func(ctx context.Context) (*corev1.Secret, error) { - return typed.MustListerForKey[*corev1.Secret](c.Registry, typed.NewRegistryKey(DependentFactoryKey(CtxCacheNamespace.Value(ctx)), secretsGVR)).ByNamespace(CtxSecretNN.MustValue(ctx).Namespace).Get(CtxSecretNN.MustValue(ctx).Name) + secret, err := typed.MustListerForKey[*corev1.Secret](c.Registry, typed.NewRegistryKey(DependentFactoryKey(CtxCacheNamespace.Value(ctx)), secretsGVR)).ByNamespace(CtxSecretNN.MustValue(ctx).Namespace).Get(CtxSecretNN.MustValue(ctx).Name) + if err != nil { + return nil, err + } + // If the secret lacks this role's per-type label key, return + // NotFound so the adoption handler applies the full label set. + // This migrates secrets from operator versions that did not + // set per-role labels. + if lk := metadata.LabelKeyForCredentialType(credType); lk != "" { + if _, ok := secret.Labels[lk]; !ok { + return nil, apierrors.NewNotFound(corev1.SchemeGroupVersion.WithResource("secrets").GroupResource(), CtxSecretNN.MustValue(ctx).Name) + } + } + return secret, nil }, func(ctx context.Context, err error) { cluster := CtxCluster.MustValue(ctx) @@ -618,6 +687,7 @@ func (c *Controller) secretAdopter(next ...handler.Handler) handler.Handler { _, err := c.kclient.CoreV1().Secrets(nn.Namespace).Get(ctx, nn.Name, metav1.GetOptions{}) return err }, + credType, handler.NoopHandler, ).Handle(secretCtx) if errors.Is(secretCtx.Err(), context.Canceled) { diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index f62ba6c5..a493a1ce 100644 --- a/pkg/controller/controller_test.go +++ b/pkg/controller/controller_test.go @@ -204,3 +204,87 @@ func TestControllerNamespacing(t *testing.T) { }) } } + +func TestCredentialSecretsForCluster(t *testing.T) { + ref := func(name string) *v1alpha1.CredentialRef { return &v1alpha1.CredentialRef{SecretName: name} } + skipped := func(name string) *v1alpha1.CredentialRef { + return &v1alpha1.CredentialRef{SecretName: name, Skip: true} + } + + tests := []struct { + name string + credentials *v1alpha1.ClusterCredentials + secretRef string + expect []credentialSecret + }{ + { + name: "nil credentials with no SecretRef returns nil", + expect: nil, + }, + { + name: "nil credentials with SecretRef returns legacy entry with empty type", + secretRef: "my-secret", + expect: []credentialSecret{{name: "my-secret", credType: ""}}, + }, + { + name: "datastoreURI only", + credentials: &v1alpha1.ClusterCredentials{DatastoreURI: ref("ds-secret")}, + expect: []credentialSecret{{name: "ds-secret", credType: metadata.CredentialTypeDatastoreURI}}, + }, + { + name: "presharedKey only", + credentials: &v1alpha1.ClusterCredentials{PresharedKey: ref("psk-secret")}, + expect: []credentialSecret{{name: "psk-secret", credType: metadata.CredentialTypePresharedKey}}, + }, + { + name: "migrationSecrets only", + credentials: &v1alpha1.ClusterCredentials{MigrationSecrets: ref("mig-secret")}, + expect: []credentialSecret{{name: "mig-secret", credType: metadata.CredentialTypeMigrationSecrets}}, + }, + { + name: "shared secret across two roles returns one entry per role", + credentials: &v1alpha1.ClusterCredentials{ + DatastoreURI: ref("shared"), + PresharedKey: ref("shared"), + }, + expect: []credentialSecret{ + {name: "shared", credType: metadata.CredentialTypeDatastoreURI}, + {name: "shared", credType: metadata.CredentialTypePresharedKey}, + }, + }, + { + name: "shared secret across all three roles returns three entries", + credentials: &v1alpha1.ClusterCredentials{ + DatastoreURI: ref("shared"), + PresharedKey: ref("shared"), + MigrationSecrets: ref("shared"), + }, + expect: []credentialSecret{ + {name: "shared", credType: metadata.CredentialTypeDatastoreURI}, + {name: "shared", credType: metadata.CredentialTypePresharedKey}, + {name: "shared", credType: metadata.CredentialTypeMigrationSecrets}, + }, + }, + { + name: "skipped ref is excluded", + credentials: &v1alpha1.ClusterCredentials{ + DatastoreURI: skipped("shared"), + PresharedKey: ref("shared"), + }, + expect: []credentialSecret{ + {name: "shared", credType: metadata.CredentialTypePresharedKey}, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cluster := &v1alpha1.SpiceDBCluster{ + Spec: v1alpha1.ClusterSpec{ + Credentials: tt.credentials, + SecretRef: tt.secretRef, + }, + } + require.Equal(t, tt.expect, credentialSecretsForCluster(cluster)) + }) + } +} diff --git a/pkg/controller/secret_adoption.go b/pkg/controller/secret_adoption.go index 114fd302..7629a4db 100644 --- a/pkg/controller/secret_adoption.go +++ b/pkg/controller/secret_adoption.go @@ -18,11 +18,21 @@ import ( const EventSecretAdoptedBySpiceDBCluster = "SecretAdoptedBySpiceDB" -func NewSecretAdoptionHandler(recorder record.EventRecorder, getFromCache func(ctx context.Context) (*corev1.Secret, error), missingFunc func(ctx context.Context, err error), secretIndexer *typed.Indexer[*corev1.Secret], secretApplyFunc adopt.ApplyFunc[*corev1.Secret, *applycorev1.SecretApplyConfiguration], existsFunc func(ctx context.Context, name types.NamespacedName) error, next handler.Handler) handler.Handler { +func NewSecretAdoptionHandler(recorder record.EventRecorder, getFromCache func(ctx context.Context) (*corev1.Secret, error), missingFunc func(ctx context.Context, err error), secretIndexer *typed.Indexer[*corev1.Secret], secretApplyFunc adopt.ApplyFunc[*corev1.Secret, *applycorev1.SecretApplyConfiguration], existsFunc func(ctx context.Context, name types.NamespacedName) error, credentialType string, next handler.Handler) handler.Handler { ctxSecret := typedctx.WithDefault[*corev1.Secret](nil) + labels := map[string]string{metadata.OperatorManagedLabelKey: metadata.OperatorManagedLabelValue} + if lk := metadata.LabelKeyForCredentialType(credentialType); lk != "" { + labels[lk] = "true" + } + // Use a role-qualified field manager so that two handlers adopting the same + // shared secret do not release each other's type label via SSA ownership. + fieldManager := metadata.FieldManager + if credentialType != "" { + fieldManager = metadata.FieldManager + "-" + credentialType + } return handler.NewHandler(&adopt.AdoptionHandler[*corev1.Secret, *applycorev1.SecretApplyConfiguration]{ OperationsContext: QueueOps, - ControllerFieldManager: metadata.FieldManager, + ControllerFieldManager: fieldManager, AdopteeCtx: CtxSecretNN, OwnerCtx: CtxClusterNN, AdoptedCtx: ctxSecret, @@ -32,8 +42,8 @@ func NewSecretAdoptionHandler(recorder record.EventRecorder, getFromCache func(c ObjectMissingFunc: missingFunc, GetFromCache: getFromCache, Indexer: secretIndexer, - IndexName: metadata.OwningClusterIndex, - Labels: map[string]string{metadata.OperatorManagedLabelKey: metadata.OperatorManagedLabelValue}, + IndexName: metadata.IndexNameForCredentialType(credentialType), + Labels: labels, NewPatch: func(nn types.NamespacedName) *applycorev1.SecretApplyConfiguration { return applycorev1.Secret(nn.Name, nn.Namespace) }, diff --git a/pkg/controller/secret_adoption_test.go b/pkg/controller/secret_adoption_test.go index 63b1825e..0928e662 100644 --- a/pkg/controller/secret_adoption_test.go +++ b/pkg/controller/secret_adoption_test.go @@ -24,10 +24,11 @@ import ( func TestSecretAdopterHandler(t *testing.T) { type applyCall struct { - called bool - input *applycorev1.SecretApplyConfiguration - result *corev1.Secret - err error + called bool + input *applycorev1.SecretApplyConfiguration + wantFieldManager string // if non-empty, the ApplyOptions.FieldManager must equal this + result *corev1.Secret + err error } secretNotFound := func(_ string) error { @@ -51,6 +52,7 @@ func TestSecretAdopterHandler(t *testing.T) { tests := []struct { name string secretName string + credentialType string cluster types.NamespacedName secretInCache *corev1.Secret cacheErr error @@ -134,6 +136,112 @@ func TestSecretAdopterHandler(t *testing.T) { expectEvents: []string{"Normal SecretAdoptedBySpiceDB Secret was referenced as the secret source for SpiceDBCluster test/test; it has been labelled to mark it as part of the configuration for that controller."}, expectNext: true, }, + { + name: "secret needs adopting with credential type label", + secretName: "secret", + credentialType: metadata.CredentialTypeDatastoreURI, + cluster: types.NamespacedName{ + Namespace: "test", + Name: "test", + }, + cacheErr: secretNotFound("test"), + secretsInIndex: []*corev1.Secret{}, + applyCalls: []*applyCall{ + { + input: applycorev1.Secret("secret", "test"). + WithLabels(map[string]string{ + metadata.OperatorManagedLabelKey: metadata.OperatorManagedLabelValue, + metadata.CredentialTypeDatastoreURILabelKey: "true", + }), + result: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "secret", + Namespace: "test", + Labels: map[string]string{ + metadata.OperatorManagedLabelKey: metadata.OperatorManagedLabelValue, + metadata.CredentialTypeDatastoreURILabelKey: "true", + }, + }, + }, + }, + { + input: applycorev1.Secret("secret", "test"). + WithAnnotations(map[string]string{ + metadata.OwnerAnnotationKeyPrefix + "test": "owned", + }), + result: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "secret", + Namespace: "test", + Labels: map[string]string{ + metadata.OperatorManagedLabelKey: metadata.OperatorManagedLabelValue, + metadata.CredentialTypeDatastoreURILabelKey: "true", + }, + Annotations: map[string]string{ + metadata.OwnerAnnotationKeyPrefix + "test": "owned", + }, + }, + }, + }, + }, + expectEvents: []string{"Normal SecretAdoptedBySpiceDB Secret was referenced as the secret source for SpiceDBCluster test/test; it has been labelled to mark it as part of the configuration for that controller."}, + expectNext: true, + }, + { + // Each credential role must use a role-qualified field manager so that + // two handlers adopting the same shared secret don't release each + // other's labels via SSA field-manager ownership. + name: "labels apply uses role-qualified field manager", + secretName: "secret", + credentialType: metadata.CredentialTypeDatastoreURI, + cluster: types.NamespacedName{ + Namespace: "test", + Name: "test", + }, + cacheErr: secretNotFound("test"), + secretsInIndex: []*corev1.Secret{}, + applyCalls: []*applyCall{ + { + wantFieldManager: metadata.FieldManager + "-" + metadata.CredentialTypeDatastoreURI, + input: applycorev1.Secret("secret", "test"). + WithLabels(map[string]string{ + metadata.OperatorManagedLabelKey: metadata.OperatorManagedLabelValue, + metadata.CredentialTypeDatastoreURILabelKey: "true", + }), + result: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "secret", + Namespace: "test", + Labels: map[string]string{ + metadata.OperatorManagedLabelKey: metadata.OperatorManagedLabelValue, + metadata.CredentialTypeDatastoreURILabelKey: "true", + }, + }, + }, + }, + { + input: applycorev1.Secret("secret", "test"). + WithAnnotations(map[string]string{ + metadata.OwnerAnnotationKeyPrefix + "test": "owned", + }), + result: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "secret", + Namespace: "test", + Labels: map[string]string{ + metadata.OperatorManagedLabelKey: metadata.OperatorManagedLabelValue, + metadata.CredentialTypeDatastoreURILabelKey: "true", + }, + Annotations: map[string]string{ + metadata.OwnerAnnotationKeyPrefix + "test": "owned", + }, + }, + }, + }, + }, + expectEvents: []string{"Normal SecretAdoptedBySpiceDB Secret was referenced as the secret source for SpiceDBCluster test/test; it has been labelled to mark it as part of the configuration for that controller."}, + expectNext: true, + }, { name: "secret already adopted", cluster: types.NamespacedName{ @@ -282,7 +390,12 @@ func TestSecretAdopterHandler(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { ctrls := &fake.FakeInterface{} - indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{metadata.OwningClusterIndex: metadata.GetClusterKeyFromMeta}) + indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{ + metadata.OwningClusterIndex: metadata.GetClusterKeyFromMeta, + metadata.OwningClusterDatastoreURIIndex: metadata.GetClusterKeyFromMetaForType(metadata.CredentialTypeDatastoreURI), + metadata.OwningClusterPresharedKeyIndex: metadata.GetClusterKeyFromMetaForType(metadata.CredentialTypePresharedKey), + metadata.OwningClusterMigrationSecretsIndex: metadata.GetClusterKeyFromMetaForType(metadata.CredentialTypeMigrationSecrets), + }) IndexAddUnstructured(t, indexer, tt.secretsInIndex) recorder := record.NewFakeRecorder(1) @@ -297,16 +410,20 @@ func TestSecretAdopterHandler(t *testing.T) { require.Equal(t, tt.expectObjectMissingErr, err) }, typed.NewIndexer[*corev1.Secret](indexer), - func(_ context.Context, secret *applycorev1.SecretApplyConfiguration, _ metav1.ApplyOptions) (result *corev1.Secret, err error) { + func(_ context.Context, secret *applycorev1.SecretApplyConfiguration, opts metav1.ApplyOptions) (result *corev1.Secret, err error) { defer func() { applyCallIndex++ }() call := tt.applyCalls[applyCallIndex] call.called = true require.Equal(t, call.input, secret, "error on call %d", applyCallIndex) + if call.wantFieldManager != "" { + require.Equal(t, call.wantFieldManager, opts.FieldManager, "field manager on call %d", applyCallIndex) + } return call.result, call.err }, func(_ context.Context, _ types.NamespacedName) error { return tt.secretExistsErr }, + tt.credentialType, handler.NewHandlerFromFunc(func(_ context.Context) { nextCalled = true }, "testnext"), diff --git a/pkg/controller/secretadopter_test.go b/pkg/controller/secretadopter_test.go index f6bcafcc..4d284f9e 100644 --- a/pkg/controller/secretadopter_test.go +++ b/pkg/controller/secretadopter_test.go @@ -10,6 +10,7 @@ import ( "github.com/authzed/controller-idioms/manager" "github.com/authzed/controller-idioms/queue" "github.com/authzed/controller-idioms/typed" + "github.com/samber/lo" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -36,6 +37,7 @@ func newTestControllerForSecretAdopter( t *testing.T, testNamespace string, secrets []*corev1.Secret, + extraIndexers cache.Indexers, ) (*Controller, chan *v1alpha1.SpiceDBCluster) { t.Helper() @@ -81,11 +83,13 @@ func newTestControllerForSecretAdopter( }, ) - // Add the OwningClusterIndex, matching production setup in NewController. + // Add the OwningClusterIndex plus any extra indexes (e.g. per-type). + idxs := cache.Indexers{metadata.OwningClusterIndex: metadata.GetClusterKeyFromMeta} + for k, v := range extraIndexers { + idxs[k] = v + } inf := informerFactory.ForResource(secretsGVR).Informer() - if err := inf.AddIndexers(cache.Indexers{ - metadata.OwningClusterIndex: metadata.GetClusterKeyFromMeta, - }); err != nil { + if err := inf.AddIndexers(idxs); err != nil { t.Fatalf("failed to add indexers: %v", err) } @@ -127,17 +131,22 @@ func TestControllerSecretAdopter(t *testing.T) { // OwnerAnnotationKeyFunc in NewSecretAdoptionHandler. ownerAnnotationKey := metadata.OwnerAnnotationKeyPrefix + clusterName - // adoptedSecret returns a secret already adopted: it has both the managed - // label (visible to the filtered informer) and the owner annotation (found - // in the OwningClusterIndex). - adoptedSecret := func(name string) *corev1.Secret { + // adoptedSecret returns a secret already adopted with an optional credential + // type. When credType is non-empty the corresponding per-role label key is + // set so the secret appears in the per-type index. When credType is "" (the + // legacy SecretRef path) only the managed label is set. + adoptedSecret := func(name, credType string) *corev1.Secret { + labels := map[string]string{ + metadata.OperatorManagedLabelKey: metadata.OperatorManagedLabelValue, + } + if lk := metadata.LabelKeyForCredentialType(credType); lk != "" { + labels[lk] = "true" + } return &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: testNamespace, - Labels: map[string]string{ - metadata.OperatorManagedLabelKey: metadata.OperatorManagedLabelValue, - }, + Labels: labels, Annotations: map[string]string{ ownerAnnotationKey: "owned", }, @@ -157,11 +166,13 @@ func TestControllerSecretAdopter(t *testing.T) { } } + ref := func(name string) *v1alpha1.CredentialRef { return &v1alpha1.CredentialRef{SecretName: name} } + tests := []struct { name string - // secretNames is placed into CtxSecretNames. - secretNames []string + // cluster is the SpiceDBCluster whose credentials drive secretAdopter. + cluster *v1alpha1.SpiceDBCluster // secretsInRegistry are pre-loaded into the fake dynamic informer AND // the fake kclient. Only secrets with the managed label are visible // through the filtered factory; unlabelled secrets are only reachable @@ -174,26 +185,47 @@ func TestControllerSecretAdopter(t *testing.T) { expectPatchedSecret types.NamespacedName }{ { - name: "no secrets - calls next", - secretNames: []string{}, + name: "no secrets - calls next", + cluster: &v1alpha1.SpiceDBCluster{ + ObjectMeta: metav1.ObjectMeta{Name: clusterName, Namespace: testNamespace}, + }, expectNextCalled: true, }, { - name: "single secret already adopted - calls next", - secretNames: []string{"secret1"}, - secretsInRegistry: []*corev1.Secret{adoptedSecret("secret1")}, + name: "single secret already adopted via SecretRef - calls next", + cluster: &v1alpha1.SpiceDBCluster{ + ObjectMeta: metav1.ObjectMeta{Name: clusterName, Namespace: testNamespace}, + Spec: v1alpha1.ClusterSpec{SecretRef: "secret1"}, + }, + // SecretRef path uses the legacy OwningClusterIndex; no type label needed. + secretsInRegistry: []*corev1.Secret{adoptedSecret("secret1", "")}, expectNextCalled: true, }, { - name: "two secrets both present - calls next", - secretNames: []string{"secret1", "secret2"}, - secretsInRegistry: []*corev1.Secret{adoptedSecret("secret1"), adoptedSecret("secret2")}, - expectNextCalled: true, + name: "two secrets both present via Credentials - calls next", + cluster: &v1alpha1.SpiceDBCluster{ + ObjectMeta: metav1.ObjectMeta{Name: clusterName, Namespace: testNamespace}, + Spec: v1alpha1.ClusterSpec{Credentials: &v1alpha1.ClusterCredentials{ + DatastoreURI: ref("secret1"), + PresharedKey: ref("secret2"), + }}, + }, + secretsInRegistry: []*corev1.Secret{ + adoptedSecret("secret1", metadata.CredentialTypeDatastoreURI), + adoptedSecret("secret2", metadata.CredentialTypePresharedKey), + }, + expectNextCalled: true, }, { - name: "first secret missing - does not call next, sets MissingSecret condition", - secretNames: []string{"missing-first", "secret2"}, - secretsInRegistry: []*corev1.Secret{adoptedSecret("secret2")}, + name: "first secret missing - does not call next, sets MissingSecret condition", + cluster: &v1alpha1.SpiceDBCluster{ + ObjectMeta: metav1.ObjectMeta{Name: clusterName, Namespace: testNamespace}, + Spec: v1alpha1.ClusterSpec{Credentials: &v1alpha1.ClusterCredentials{ + DatastoreURI: ref("missing-first"), + PresharedKey: ref("secret2"), + }}, + }, + secretsInRegistry: []*corev1.Secret{adoptedSecret("secret2", metadata.CredentialTypePresharedKey)}, expectNextCalled: false, expectPatchedSecret: types.NamespacedName{ Namespace: testNamespace, @@ -201,9 +233,15 @@ func TestControllerSecretAdopter(t *testing.T) { }, }, { - name: "second secret missing - does not call next, sets MissingSecret condition", - secretNames: []string{"secret1", "missing-second"}, - secretsInRegistry: []*corev1.Secret{adoptedSecret("secret1")}, + name: "second secret missing - does not call next, sets MissingSecret condition", + cluster: &v1alpha1.SpiceDBCluster{ + ObjectMeta: metav1.ObjectMeta{Name: clusterName, Namespace: testNamespace}, + Spec: v1alpha1.ClusterSpec{Credentials: &v1alpha1.ClusterCredentials{ + DatastoreURI: ref("secret1"), + PresharedKey: ref("missing-second"), + }}, + }, + secretsInRegistry: []*corev1.Secret{adoptedSecret("secret1", metadata.CredentialTypeDatastoreURI)}, expectNextCalled: false, expectPatchedSecret: types.NamespacedName{ Namespace: testNamespace, @@ -215,8 +253,11 @@ func TestControllerSecretAdopter(t *testing.T) { // (not visible in the filtered informer cache). The adoption handler // should call Apply to add the managed label and owner annotation, // then call next. - name: "unlabelled secret gets adopted - calls next", - secretNames: []string{"new-secret"}, + name: "unlabelled secret gets adopted via SecretRef - calls next", + cluster: &v1alpha1.SpiceDBCluster{ + ObjectMeta: metav1.ObjectMeta{Name: clusterName, Namespace: testNamespace}, + Spec: v1alpha1.ClusterSpec{SecretRef: "new-secret"}, + }, secretsInRegistry: []*corev1.Secret{unlabelledSecret("new-secret")}, expectNextCalled: true, }, @@ -224,7 +265,11 @@ func TestControllerSecretAdopter(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - c, patchedCh := newTestControllerForSecretAdopter(t, testNamespace, tt.secretsInRegistry) + c, patchedCh := newTestControllerForSecretAdopter(t, testNamespace, tt.secretsInRegistry, cache.Indexers{ + metadata.OwningClusterDatastoreURIIndex: metadata.GetClusterKeyFromMetaForType(metadata.CredentialTypeDatastoreURI), + metadata.OwningClusterPresharedKeyIndex: metadata.GetClusterKeyFromMetaForType(metadata.CredentialTypePresharedKey), + metadata.OwningClusterMigrationSecretsIndex: metadata.GetClusterKeyFromMetaForType(metadata.CredentialTypeMigrationSecrets), + }) // Use a real queue.Operations wired to a cancelable context so that // QueueOps.RequeueErr / RequeueAPIErr properly cancel secretCtx, @@ -234,12 +279,7 @@ func TestControllerSecretAdopter(t *testing.T) { t.Cleanup(cancel) ops := queue.NewOperations(cancel, func(_ time.Duration) { cancel() }, cancel) - cluster := &v1alpha1.SpiceDBCluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: clusterName, - Namespace: testNamespace, - }, - } + cluster := tt.cluster ctx := handlerCtx ctx = QueueOps.WithValue(ctx, ops) @@ -249,7 +289,9 @@ func TestControllerSecretAdopter(t *testing.T) { Namespace: testNamespace, Name: clusterName, }) - ctx = CtxSecretNames.WithValue(ctx, tt.secretNames) + // CtxSecretNames is still set for collectSecrets (which runs after + // secretAdopter); secretAdopter itself derives pairs from the cluster spec. + ctx = CtxSecretNames.WithValue(ctx, credentialSecretNames(cluster)) nextCalled := false next := handler.NewHandlerFromFunc(func(_ context.Context) { @@ -287,3 +329,239 @@ func TestControllerSecretAdopter(t *testing.T) { }) } } + +// TestSecretAdopterPerTypeIndexIsolation is a regression test for the bug +// where N sequential AdoptionHandlers sharing OwningClusterIndex would each +// remove the other handler's valid adoptee as if it were stale, causing an +// infinite re-adoption loop. +// +// With per-type indexes, each adoption handler only sees secrets of its own +// credential type in the index, so it never treats another type's secret as +// an extra to clean up. +func TestSecretAdopterPerTypeIndexIsolation(t *testing.T) { + const testNamespace = "test" + const clusterName = "mycluster" + + ownerAnnotationKey := metadata.OwnerAnnotationKeyPrefix + clusterName + + makeAdoptedSecret := func(name, credType string) *corev1.Secret { + labels := map[string]string{ + metadata.OperatorManagedLabelKey: metadata.OperatorManagedLabelValue, + } + if lk := metadata.LabelKeyForCredentialType(credType); lk != "" { + labels[lk] = "true" + } + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: testNamespace, + Labels: labels, + Annotations: map[string]string{ownerAnnotationKey: "owned"}, + }, + } + } + + extraIndexers := cache.Indexers{ + metadata.OwningClusterDatastoreURIIndex: metadata.GetClusterKeyFromMetaForType(metadata.CredentialTypeDatastoreURI), + metadata.OwningClusterPresharedKeyIndex: metadata.GetClusterKeyFromMetaForType(metadata.CredentialTypePresharedKey), + metadata.OwningClusterMigrationSecretsIndex: metadata.GetClusterKeyFromMetaForType(metadata.CredentialTypeMigrationSecrets), + } + c, _ := newTestControllerForSecretAdopter(t, testNamespace, []*corev1.Secret{ + makeAdoptedSecret("secret-ds", metadata.CredentialTypeDatastoreURI), + makeAdoptedSecret("secret-psk", metadata.CredentialTypePresharedKey), + }, extraIndexers) + + handlerCtx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + ops := queue.NewOperations(cancel, func(_ time.Duration) { cancel() }, cancel) + + cluster := &v1alpha1.SpiceDBCluster{ + ObjectMeta: metav1.ObjectMeta{Name: clusterName, Namespace: testNamespace}, + Spec: v1alpha1.ClusterSpec{ + Credentials: &v1alpha1.ClusterCredentials{ + DatastoreURI: &v1alpha1.CredentialRef{SecretName: "secret-ds"}, + PresharedKey: &v1alpha1.CredentialRef{SecretName: "secret-psk"}, + }, + }, + } + + ctx := handlerCtx + ctx = QueueOps.WithValue(ctx, ops) + ctx = CtxCacheNamespace.WithValue(ctx, testNamespace) + ctx = CtxCluster.WithValue(ctx, cluster) + ctx = CtxClusterNN.WithValue(ctx, types.NamespacedName{Namespace: testNamespace, Name: clusterName}) + ctx = CtxSecretNames.WithValue(ctx, []string{"secret-ds", "secret-psk"}) + + nextCalled := false + c.secretAdopter(handler.NewHandlerFromFunc(func(_ context.Context) { + nextCalled = true + }, "testnext")).Handle(ctx) + + require.True(t, nextCalled, "next should be called when both secrets are already adopted") + require.Empty(t, c.kclient.(*kfake.Clientset).Actions(), + "secretAdopter must not issue any API calls when all secrets are already adopted with correct type labels") +} + +// TestSecretAdopterSharedSecret verifies that a single secret used for multiple +// credential roles is handled independently for each role: both type labels must +// be applied, and once they are no adoption API calls should be issued. +func TestSecretAdopterSharedSecret(t *testing.T) { + const testNamespace = "test" + const clusterName = "mycluster" + + ownerAnnotationKey := metadata.OwnerAnnotationKeyPrefix + clusterName + extraIndexers := cache.Indexers{ + metadata.OwningClusterDatastoreURIIndex: metadata.GetClusterKeyFromMetaForType(metadata.CredentialTypeDatastoreURI), + metadata.OwningClusterPresharedKeyIndex: metadata.GetClusterKeyFromMetaForType(metadata.CredentialTypePresharedKey), + metadata.OwningClusterMigrationSecretsIndex: metadata.GetClusterKeyFromMetaForType(metadata.CredentialTypeMigrationSecrets), + } + + cluster := &v1alpha1.SpiceDBCluster{ + ObjectMeta: metav1.ObjectMeta{Name: clusterName, Namespace: testNamespace}, + Spec: v1alpha1.ClusterSpec{ + Credentials: &v1alpha1.ClusterCredentials{ + DatastoreURI: &v1alpha1.CredentialRef{SecretName: "shared"}, + PresharedKey: &v1alpha1.CredentialRef{SecretName: "shared"}, + }, + }, + } + + // sharedSecret carries both per-role label keys and the owner annotation. + sharedSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "shared", + Namespace: testNamespace, + Labels: map[string]string{ + metadata.OperatorManagedLabelKey: metadata.OperatorManagedLabelValue, + metadata.CredentialTypeDatastoreURILabelKey: "true", + metadata.CredentialTypePresharedKeyLabelKey: "true", + }, + Annotations: map[string]string{ownerAnnotationKey: "owned"}, + }, + } + + c, _ := newTestControllerForSecretAdopter(t, testNamespace, []*corev1.Secret{sharedSecret}, extraIndexers) + + handlerCtx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + ops := queue.NewOperations(cancel, func(_ time.Duration) { cancel() }, cancel) + + ctx := handlerCtx + ctx = QueueOps.WithValue(ctx, ops) + ctx = CtxCacheNamespace.WithValue(ctx, testNamespace) + ctx = CtxCluster.WithValue(ctx, cluster) + ctx = CtxClusterNN.WithValue(ctx, types.NamespacedName{Namespace: testNamespace, Name: clusterName}) + ctx = CtxSecretNames.WithValue(ctx, credentialSecretNames(cluster)) + + nextCalled := false + c.secretAdopter(handler.NewHandlerFromFunc(func(_ context.Context) { + nextCalled = true + }, "testnext")).Handle(ctx) + + require.True(t, nextCalled, "next should be called when the shared secret carries all role labels") + require.Empty(t, c.kclient.(*kfake.Clientset).Actions(), + "no API calls should be issued when the shared secret already has all required type labels") +} + +// TestSecretAdopterSharedSecretMigration is a regression test for the SSA +// field-manager collision that occurs when the same secret is used for two +// credential roles and needs migration (has no per-role type labels yet). +// +// Both adoption handlers run during the same reconcile. If they use the same +// SSA field manager for the labels patch, the second handler's apply releases +// ownership of the first handler's type label — Kubernetes removes it and the +// next reconcile sees the label missing again, causing an infinite loop. +// +// The fix is to use a role-qualified field manager per handler so each handler +// exclusively owns its own type label and the two never interfere. +func TestSecretAdopterSharedSecretMigration(t *testing.T) { + const testNamespace = "test" + const clusterName = "mycluster" + + extraIndexers := cache.Indexers{ + metadata.OwningClusterDatastoreURIIndex: metadata.GetClusterKeyFromMetaForType(metadata.CredentialTypeDatastoreURI), + metadata.OwningClusterPresharedKeyIndex: metadata.GetClusterKeyFromMetaForType(metadata.CredentialTypePresharedKey), + metadata.OwningClusterMigrationSecretsIndex: metadata.GetClusterKeyFromMetaForType(metadata.CredentialTypeMigrationSecrets), + } + + cluster := &v1alpha1.SpiceDBCluster{ + ObjectMeta: metav1.ObjectMeta{Name: clusterName, Namespace: testNamespace}, + Spec: v1alpha1.ClusterSpec{ + Credentials: &v1alpha1.ClusterCredentials{ + DatastoreURI: &v1alpha1.CredentialRef{SecretName: "shared"}, + PresharedKey: &v1alpha1.CredentialRef{SecretName: "shared"}, + }, + }, + } + + // unlabelledSecret exists in the cluster (visible via kclient.Get) but has + // no managed label so it is not visible through the filtered informer cache. + unlabelledSharedSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "shared", Namespace: testNamespace}, + } + + c, _ := newTestControllerForSecretAdopter(t, testNamespace, []*corev1.Secret{unlabelledSharedSecret}, extraIndexers) + + // Intercept every Apply (SSA patch) on secrets and record the field manager + // used. Two handlers adopting the same secret must use distinct field + // managers so their label ownership does not overlap. + type applyRecord struct { + fieldManager string + labels map[string]string + } + var labelApplies []applyRecord + c.kclient.(*kfake.Clientset).PrependReactor("patch", "secrets", + func(action kubetesting.Action) (bool, runtime.Object, error) { + pa, ok := action.(kubetesting.PatchActionImpl) + if !ok || pa.GetPatchType() != types.ApplyPatchType { + return false, nil, nil + } + // Parse labels out of the SSA patch JSON. + var obj struct { + Metadata struct { + Labels map[string]string `json:"labels"` + } `json:"metadata"` + } + _ = json.Unmarshal(pa.GetPatch(), &obj) + if obj.Metadata.Labels != nil { + labelApplies = append(labelApplies, applyRecord{ + fieldManager: pa.PatchOptions.FieldManager, + labels: obj.Metadata.Labels, + }) + } + // Return the unlabelled secret so the handler doesn't error out. + return true, unlabelledSharedSecret.DeepCopy(), nil + }) + + handlerCtx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + ops := queue.NewOperations(cancel, func(_ time.Duration) { cancel() }, cancel) + + ctx := handlerCtx + ctx = QueueOps.WithValue(ctx, ops) + ctx = CtxCacheNamespace.WithValue(ctx, testNamespace) + ctx = CtxCluster.WithValue(ctx, cluster) + ctx = CtxClusterNN.WithValue(ctx, types.NamespacedName{Namespace: testNamespace, Name: clusterName}) + ctx = CtxSecretNames.WithValue(ctx, credentialSecretNames(cluster)) + + nextCalled := false + c.secretAdopter(handler.NewHandlerFromFunc(func(_ context.Context) { + nextCalled = true + }, "testnext")).Handle(ctx) + + require.True(t, nextCalled, "secretAdopter should call next after adopting both roles") + + // Both role labels must be applied via distinct field managers. If the same + // manager is used for both, the second apply would release ownership of the + // first role's label, causing Kubernetes to remove it (SSA semantics). + require.Len(t, labelApplies, 2, "expected one labels Apply per credential role") + require.NotEqual(t, labelApplies[0].fieldManager, labelApplies[1].fieldManager, + "each credential role must use a distinct SSA field manager to avoid label ownership collisions") + + // Merge both applies' label maps so the assertions are order-independent. + allLabels := lo.Assign(labelApplies[0].labels, labelApplies[1].labels) + require.Contains(t, allLabels, metadata.CredentialTypeDatastoreURILabelKey, + "one apply must carry the datastore-uri type label") + require.Contains(t, allLabels, metadata.CredentialTypePresharedKeyLabelKey, + "one apply must carry the preshared-key type label") +} diff --git a/pkg/metadata/keys.go b/pkg/metadata/keys.go index 6f47fe2b..6874dfb4 100644 --- a/pkg/metadata/keys.go +++ b/pkg/metadata/keys.go @@ -8,7 +8,6 @@ import ( "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" - "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/tools/cache" @@ -16,7 +15,25 @@ import ( ) const ( - OwningClusterIndex = "owning-cluster" + OwningClusterIndex = "owning-cluster" + OwningClusterDatastoreURIIndex = "owning-cluster-datastore-uri" + OwningClusterPresharedKeyIndex = "owning-cluster-preshared-key" + OwningClusterMigrationSecretsIndex = "owning-cluster-migration-secrets" + + // CredentialType* are the internal credential-role identifiers. They are + // used to derive per-role label keys and index names; they are not stored + // directly on Kubernetes objects. + CredentialTypeDatastoreURI = "datastore-uri" // nolint: gosec + CredentialTypePresharedKey = "preshared-key" // nolint: gosec + CredentialTypeMigrationSecrets = "migration-secrets" // nolint: gosec + + // Per-role label keys. A secret carries exactly the keys for the roles it + // serves. Key presence (not value) is what the index functions check, so a + // shared secret can carry all applicable keys simultaneously. + CredentialTypeDatastoreURILabelKey = "authzed.com/credential-type-datastore-uri" // nolint: gosec + CredentialTypePresharedKeyLabelKey = "authzed.com/credential-type-preshared-key" // nolint: gosec + CredentialTypeMigrationSecretsLabelKey = "authzed.com/credential-type-migration-secrets" // nolint: gosec + OperatorManagedLabelKey = "authzed.com/managed-by" OperatorManagedLabelValue = "operator" OwnerLabelKey = "authzed.com/cluster" @@ -89,14 +106,73 @@ func SplitGVRMetaNamespaceKey(key string) (gvr *schema.GroupVersionResource, nam return gvr, namespace, name, err } -func GetClusterKeyFromMeta(in interface{}) ([]string, error) { - obj := in.(runtime.Object) - objMeta, err := meta.Accessor(obj) +// LabelKeyForCredentialType returns the per-role label key for the given +// credential type, or "" for unknown types (including the legacy empty-string +// SecretRef type). +func LabelKeyForCredentialType(credType string) string { + switch credType { + case CredentialTypeDatastoreURI: + return CredentialTypeDatastoreURILabelKey + case CredentialTypePresharedKey: + return CredentialTypePresharedKeyLabelKey + case CredentialTypeMigrationSecrets: + return CredentialTypeMigrationSecretsLabelKey + default: + return "" + } +} + +// IndexNameForCredentialType maps a credential type to its dedicated index +// name. The empty string (legacy SecretRef) falls back to OwningClusterIndex. +func IndexNameForCredentialType(credType string) string { + switch credType { + case CredentialTypeDatastoreURI: + return OwningClusterDatastoreURIIndex + case CredentialTypePresharedKey: + return OwningClusterPresharedKeyIndex + case CredentialTypeMigrationSecrets: + return OwningClusterMigrationSecretsIndex + default: + return OwningClusterIndex + } +} + +// GetClusterKeyFromMetaForType returns a cache.IndexFunc that indexes objects +// by owning cluster, but only for objects that carry the per-role label key for +// the given credential type. Key presence (not value) is checked, so a single +// secret can carry multiple role labels and appear in multiple indexes without +// any handler treating another role's secret as stale. +func GetClusterKeyFromMetaForType(credentialType string) cache.IndexFunc { + labelKey := LabelKeyForCredentialType(credentialType) + return func(in any) ([]string, error) { + if d, ok := in.(cache.DeletedFinalStateUnknown); ok { + in = d.Obj + } + objMeta, err := meta.Accessor(in) + if err != nil { + return nil, err + } + // When labelKey is "" (legacy SecretRef / unknown type) skip the type + // filter and index all owned objects, matching OwningClusterIndex. + if labelKey != "" { + if _, ok := objMeta.GetLabels()[labelKey]; !ok { + return nil, nil + } + } + return adopt.OwnerKeysFromMeta(OwnerAnnotationKeyPrefix)(in) + } +} + +func GetClusterKeyFromMeta(in any) ([]string, error) { + if d, ok := in.(cache.DeletedFinalStateUnknown); ok { + in = d.Obj + } + objMeta, err := meta.Accessor(in) if err != nil { return nil, err } - clusterNames, err := adopt.OwnerKeysFromMeta(OwnerAnnotationKeyPrefix)(obj) + clusterNames, err := adopt.OwnerKeysFromMeta(OwnerAnnotationKeyPrefix)(in) if err != nil { return nil, err } diff --git a/pkg/metadata/keys_test.go b/pkg/metadata/keys_test.go new file mode 100644 index 00000000..cb46e397 --- /dev/null +++ b/pkg/metadata/keys_test.go @@ -0,0 +1,124 @@ +package metadata + +import ( + "testing" + + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/tools/cache" +) + +func TestGetClusterKeyFromMetaForType(t *testing.T) { + ownerAnnotation := OwnerAnnotationKeyPrefix + "mycluster" + // secret creates a secret whose label set marks it as carrying the given + // credential type role. Key presence (not value) is what matters. + secret := func(credType string) *corev1.Secret { + labels := map[string]string{} + if lk := LabelKeyForCredentialType(credType); lk != "" { + labels[lk] = "true" + } + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mysecret", + Namespace: "test", + Labels: labels, + Annotations: map[string]string{ownerAnnotation: "owned"}, + }, + } + } + secretNoTypeLabel := func() *corev1.Secret { + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mysecret", + Namespace: "test", + Labels: map[string]string{}, + Annotations: map[string]string{ownerAnnotation: "owned"}, + }, + } + } + + tests := []struct { + name string + indexType string + obj any + expectKeys []string + expectErr bool + }{ + { + name: "matching type label returns owner key", + indexType: CredentialTypeDatastoreURI, + obj: secret(CredentialTypeDatastoreURI), + expectKeys: []string{"test/mycluster"}, + }, + { + name: "non-matching type label returns nil", + indexType: CredentialTypeDatastoreURI, + obj: secret(CredentialTypePresharedKey), + }, + { + name: "missing type label returns nil", + indexType: CredentialTypeDatastoreURI, + obj: secretNoTypeLabel(), + }, + { + name: "tombstone with matching type label returns owner key", + indexType: CredentialTypeDatastoreURI, + obj: cache.DeletedFinalStateUnknown{Key: "test/mysecret", Obj: secret(CredentialTypeDatastoreURI)}, + expectKeys: []string{"test/mycluster"}, + }, + { + name: "non-object tombstone returns error without panic", + indexType: CredentialTypeDatastoreURI, + obj: cache.DeletedFinalStateUnknown{Key: "test/mysecret", Obj: "not-an-object"}, + expectErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + keys, err := GetClusterKeyFromMetaForType(tt.indexType)(tt.obj) + if tt.expectErr { + require.Error(t, err) + return + } + require.NoError(t, err) + require.Equal(t, tt.expectKeys, keys) + }) + } +} + +func TestLabelKeyForCredentialType(t *testing.T) { + tests := []struct { + credType string + expectKey string + }{ + {CredentialTypeDatastoreURI, CredentialTypeDatastoreURILabelKey}, + {CredentialTypePresharedKey, CredentialTypePresharedKeyLabelKey}, + {CredentialTypeMigrationSecrets, CredentialTypeMigrationSecretsLabelKey}, + {"", ""}, + {"unknown", ""}, + } + for _, tt := range tests { + t.Run(tt.credType, func(t *testing.T) { + require.Equal(t, tt.expectKey, LabelKeyForCredentialType(tt.credType)) + }) + } +} + +func TestIndexNameForCredentialType(t *testing.T) { + tests := []struct { + credType string + expectIdx string + }{ + {CredentialTypeDatastoreURI, OwningClusterDatastoreURIIndex}, + {CredentialTypePresharedKey, OwningClusterPresharedKeyIndex}, + {CredentialTypeMigrationSecrets, OwningClusterMigrationSecretsIndex}, + {"", OwningClusterIndex}, + {"unknown", OwningClusterIndex}, + } + for _, tt := range tests { + t.Run(tt.credType, func(t *testing.T) { + require.Equal(t, tt.expectIdx, IndexNameForCredentialType(tt.credType)) + }) + } +}