diff --git a/api/bases/keystone.openstack.org_keystoneapplicationcredentials.yaml b/api/bases/keystone.openstack.org_keystoneapplicationcredentials.yaml index 13c513cf..02ed45d9 100644 --- a/api/bases/keystone.openstack.org_keystoneapplicationcredentials.yaml +++ b/api/bases/keystone.openstack.org_keystoneapplicationcredentials.yaml @@ -209,6 +209,10 @@ spec: for this ApplicationCredential. format: int64 type: integer + previousSecretName: + description: PreviousSecretName - name of the previous AC secret. + Only current and previous are protected by finalizer. + type: string rotationEligibleAt: description: |- RotationEligibleAt indicates when rotation becomes eligible (start of grace period window). diff --git a/api/v1beta1/keystoneapplicationcredential.go b/api/v1beta1/keystoneapplicationcredential.go index d4df1d86..5644060f 100644 --- a/api/v1beta1/keystoneapplicationcredential.go +++ b/api/v1beta1/keystoneapplicationcredential.go @@ -18,26 +18,16 @@ package v1beta1 import ( "context" - "errors" "fmt" corev1 "k8s.io/api/core/v1" k8s_errors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" -) - -// ApplicationCredentialData contains AC ID/Secret extracted from a Secret -// Used by service operators to get AC data from the Secret -type ApplicationCredentialData struct { - ID string - Secret string -} -// GetACSecretName returns the standard AC Secret name for a service -func GetACSecretName(serviceName string) string { - return fmt.Sprintf("ac-%s-secret", serviceName) -} + "github.com/openstack-k8s-operators/lib-common/modules/common/helper" + "github.com/openstack-k8s-operators/lib-common/modules/common/object" +) // GetACCRName returns the standard AC CR name for a service func GetACCRName(serviceName string) string { @@ -51,37 +41,68 @@ const ( ACSecretSecretKey = "AC_SECRET" ) -var ( - // ErrACIDMissing indicates AC_ID key missing or empty in the Secret - ErrACIDMissing = errors.New("applicationcredential secret missing AC_ID") - // ErrACSecretMissing indicates AC_SECRET key missing or empty in the Secret - ErrACSecretMissing = errors.New("applicationcredential secret missing AC_SECRET") -) - -// GetApplicationCredentialFromSecret fetches and validates AC data from the Secret -func GetApplicationCredentialFromSecret( +// ManageACSecretFinalizer ensures consumerFinalizer is present on the AC secret +// identified by newSecretName and absent from the one identified by +// oldSecretName. It is a no-op when both names are equal. +func ManageACSecretFinalizer( ctx context.Context, - c client.Client, + h *helper.Helper, namespace string, - serviceName string, -) (*ApplicationCredentialData, error) { - secret := &corev1.Secret{} - key := types.NamespacedName{Namespace: namespace, Name: GetACSecretName(serviceName)} - if err := c.Get(ctx, key, secret); err != nil { - if k8s_errors.IsNotFound(err) { - return nil, nil + newSecretName string, + oldSecretName string, + consumerFinalizer string, +) error { + if newSecretName == oldSecretName { + return nil + } + + var newObj, oldObj client.Object + + if newSecretName != "" { + secret := &corev1.Secret{} + key := types.NamespacedName{Name: newSecretName, Namespace: namespace} + if err := h.GetClient().Get(ctx, key, secret); err != nil { + return fmt.Errorf("failed to get new AC secret %s: %w", newSecretName, err) } - return nil, fmt.Errorf("get applicationcredential secret %s: %w", key, err) + newObj = secret } - acID, okID := secret.Data[ACIDSecretKey] - if !okID || len(acID) == 0 { - return nil, fmt.Errorf("%w: %s", ErrACIDMissing, key.String()) + if oldSecretName != "" { + secret := &corev1.Secret{} + key := types.NamespacedName{Name: oldSecretName, Namespace: namespace} + if err := h.GetClient().Get(ctx, key, secret); err != nil { + if !k8s_errors.IsNotFound(err) { + return fmt.Errorf("failed to get old AC secret %s: %w", oldSecretName, err) + } + } else { + oldObj = secret + } } - acSecret, okSecret := secret.Data[ACSecretSecretKey] - if !okSecret || len(acSecret) == 0 { - return nil, fmt.Errorf("%w: %s", ErrACSecretMissing, key.String()) + + return object.ManageConsumerFinalizer(ctx, h, newObj, oldObj, consumerFinalizer) +} + +// RemoveACSecretConsumerFinalizer removes consumerFinalizer from the AC secret +// identified by secretName. It is a no-op when secretName is empty or the +// secret no longer exists. +func RemoveACSecretConsumerFinalizer( + ctx context.Context, + h *helper.Helper, + namespace string, + secretName string, + consumerFinalizer string, +) error { + if secretName == "" { + return nil } - return &ApplicationCredentialData{ID: string(acID), Secret: string(acSecret)}, nil + secret := &corev1.Secret{} + key := types.NamespacedName{Name: secretName, Namespace: namespace} + if err := h.GetClient().Get(ctx, key, secret); err != nil { + if k8s_errors.IsNotFound(err) { + return nil + } + return err + } + return object.RemoveConsumerFinalizer(ctx, h, secret, consumerFinalizer) } diff --git a/api/v1beta1/keystoneapplicationcredential_types.go b/api/v1beta1/keystoneapplicationcredential_types.go index 1daae9f7..88382b1c 100644 --- a/api/v1beta1/keystoneapplicationcredential_types.go +++ b/api/v1beta1/keystoneapplicationcredential_types.go @@ -91,6 +91,9 @@ type KeystoneApplicationCredentialStatus struct { // SecretName - name of the k8s Secret storing the ApplicationCredential secret SecretName string `json:"secretName,omitempty"` + // PreviousSecretName - name of the previous AC secret. Only current and previous are protected by finalizer. + PreviousSecretName string `json:"previousSecretName,omitempty"` + // Conditions Conditions condition.Conditions `json:"conditions,omitempty"` diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index e789f345..e91c6686 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -66,21 +66,6 @@ func (in *APIOverrideSpec) DeepCopy() *APIOverrideSpec { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *ApplicationCredentialData) DeepCopyInto(out *ApplicationCredentialData) { - *out = *in -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ApplicationCredentialData. -func (in *ApplicationCredentialData) DeepCopy() *ApplicationCredentialData { - if in == nil { - return nil - } - out := new(ApplicationCredentialData) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Endpoint) DeepCopyInto(out *Endpoint) { *out = *in diff --git a/config/crd/bases/keystone.openstack.org_keystoneapplicationcredentials.yaml b/config/crd/bases/keystone.openstack.org_keystoneapplicationcredentials.yaml index 13c513cf..02ed45d9 100644 --- a/config/crd/bases/keystone.openstack.org_keystoneapplicationcredentials.yaml +++ b/config/crd/bases/keystone.openstack.org_keystoneapplicationcredentials.yaml @@ -209,6 +209,10 @@ spec: for this ApplicationCredential. format: int64 type: integer + previousSecretName: + description: PreviousSecretName - name of the previous AC secret. + Only current and previous are protected by finalizer. + type: string rotationEligibleAt: description: |- RotationEligibleAt indicates when rotation becomes eligible (start of grace period window). diff --git a/docs/applicationcredentials.md b/docs/applicationcredentials.md index 1531544c..de94114e 100644 --- a/docs/applicationcredentials.md +++ b/docs/applicationcredentials.md @@ -53,7 +53,8 @@ status: # ACID - the ID in Keystone for this ApplicationCredential ACID: "7b23dbac20bc4f048f937415c84bb329" # SecretName - name of the k8s Secret storing the ApplicationCredential secret - secretName: "ac-barbican-secret" + # Format: ac---secret + secretName: "ac-barbican-7b23d-secret" # CreatedAt - timestamp of creation createdAt: "2025-05-29T09:02:28Z" # ExpiresAt - time of validity expiration @@ -120,17 +121,29 @@ the AC controller: - Includes access rules if specified in the CR 8. Store Secret - - Creates a k8s `Secret` named `ac-barbican-secret` + - Creates a new **immutable** k8s `Secret` with a unique name: `ac---secret` + - The name includes the first 5 characters of the Keystone AC ID for uniqueness - Adds `openstack.org/ac-secret-protection` finalizer to the Secret + - Sets owner reference to the AC CR (for garbage collection on CR deletion) ```yaml apiVersion: v1 kind: Secret metadata: - name: ac-barbican-secret + name: ac-barbican-7b23d-secret namespace: openstack + labels: + application-credentials: "true" + application-credential-service: barbican finalizers: - openstack.org/ac-secret-protection + ownerReferences: + - apiVersion: keystone.openstack.org/v1beta1 + kind: KeystoneApplicationCredential + name: ac-barbican + controller: true + blockOwnerDeletion: true +immutable: true data: AC_ID: AC_SECRET: @@ -138,13 +151,8 @@ data: 9. Update CR status - Sets `.status.ACID`, `.status.secretName`, `.status.createdAt`, `.status.expiresAt`, `.status.rotationEligibleAt` - - Sets `.status.lastRotated` (only during rotation, not initial creation) + - Sets `.status.lastRotated` and emits `ApplicationCredentialRotated` event (only during rotation, not initial creation) - Marks AC CR ready - - Emits an event for rotation to notify EDPM nodes - -10. Requeue for Next Check - - Calculates next reconcile at `expiresAt - gracePeriod` - - If already in grace window, requeues immediately, otherwise requeues after 24 h AC in Keystone side: ``` @@ -174,15 +182,20 @@ When the next reconcile hits the grace window (`now ≥ expiresAt - gracePeriodD - Generates a new Keystone AC with a fresh 5-char suffix - Uses the same roles, unrestricted flag, access rules, and expirationDays - Does _not_ revoke the old AC, the old credential naturally expires -- Store Updated Secret - - Overwrites the existing `ac-barbican-secret` with the new `AC_ID` and `AC_SECRET` +- Create New Immutable Secret + - Creates a **new** immutable Secret with a unique name (e.g. `ac-barbican-d38dc-secret`) + - The previous Secret (e.g. `ac-barbican-7b23d-secret`) is **retained** — it is not deleted + - Both secrets are owned by the AC CR and will be garbage-collected when the CR is deleted - Update Status + - Sets `.status.secretName` to the new Secret name - Replaces `.status.ACID`, `.status.createdAt`, `.status.expiresAt`, and `.status.rotationEligibleAt` with the new values - Sets `.status.lastRotated` to current timestamp - Re-marks AC CR ready - - Emits an event to notify EDPM nodes about the rotation -- Requeue - - Schedules the next check at `(newExpiresAt - gracePeriodDays)` + - Emits `ApplicationCredentialRotated` event for EDPM visibility +- Propagation + - The openstack-operator `Owns` the AC CR, so the status change triggers re-reconciliation + - It reads the new `.status.secretName` and updates the service CR's `ApplicationCredentialSecret` + - The service operator detects the spec change and reads credentials from the new Secret ## Manual Rotation @@ -203,8 +216,8 @@ This triggers seamless rotation with one pod restart and no authentication fallb ApplicationCredentials in Keystone are **not automatically deleted** by the controller. This design decision prevents disrupting running services, especially EDPM nodes that actively use these credentials. **Cleanup behavior:** -- **During rotation:** The old AC remains in Keystone and expires naturally based on its `expiresAt` timestamp. The new AC is created with fresh credentials. -- **When AC CR is deleted:** The ApplicationCredential remains in Keystone and continues to be valid until natural expiration. +- **During rotation:** The old AC remains in Keystone and expires naturally based on its `expiresAt` timestamp. The old K8s Secret is also retained (immutable). A new AC and a new immutable Secret are created. +- **When AC CR is deleted:** The controller removes the `openstack.org/ac-secret-protection` finalizer from **all** AC Secrets for the service (found by label), allowing owner-reference garbage collection to delete them. The ApplicationCredential in Keystone remains valid until natural expiration. - **Manual cleanup:** If immediate cleanup is required, operators can manually delete the AC from Keystone: ```bash @@ -213,30 +226,23 @@ openstack application credential delete This approach ensures that deleting the AC CR (intentionally or accidentally) does not cause immediate authentication failures across the control plane and EDPM deployments. -## Client-Side Helper Functions +## Exported API Helpers -Service operators can use these helper functions to consume ApplicationCredential data: +The `keystone-operator/api/v1beta1` package exports the following helpers for use by other operators: ```go import keystonev1 "github.com/openstack-k8s-operators/keystone-operator/api/v1beta1" -// Get standard AC Secret name for a service -secretName := keystonev1.GetACSecretName("barbican") // Returns "ac-barbican-secret" - // Get standard AC CR name for a service crName := keystonev1.GetACCRName("barbican") // Returns "ac-barbican" -// Fetch AC data directly from the Secret -acData, err := keystonev1.GetApplicationCredentialFromSecret( - ctx, client, namespace, serviceName) -if err != nil { - // Handle error -} -if acData != nil { - // Use acData.ID and acData.Secret -} +// Secret data keys +keystonev1.ACIDSecretKey // "AC_ID" +keystonev1.ACSecretSecretKey // "AC_SECRET" ``` +Service operators read AC data directly from the Secret referenced by the service CR's `ApplicationCredentialSecret` field, using `ACIDSecretKey` and `ACSecretSecretKey` as the data keys. + ## Validation Rules The API includes validation constraints: diff --git a/internal/controller/keystoneapplicationcredential_controller.go b/internal/controller/keystoneapplicationcredential_controller.go index c30be17f..390472b1 100644 --- a/internal/controller/keystoneapplicationcredential_controller.go +++ b/internal/controller/keystoneapplicationcredential_controller.go @@ -19,6 +19,8 @@ package controller import ( "context" "fmt" + "net/http" + "strings" "time" "github.com/go-logr/logr" @@ -29,6 +31,7 @@ import ( "github.com/openstack-k8s-operators/keystone-operator/internal/keystone" "github.com/openstack-k8s-operators/lib-common/modules/common/condition" "github.com/openstack-k8s-operators/lib-common/modules/common/helper" + oko_secret "github.com/openstack-k8s-operators/lib-common/modules/common/secret" "github.com/openstack-k8s-operators/lib-common/modules/common/util" corev1 "k8s.io/api/core/v1" k8s_errors "k8s.io/apimachinery/pkg/api/errors" @@ -39,14 +42,19 @@ import ( "k8s.io/client-go/kubernetes" "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/event" ctrlLog "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/predicate" ) const acSecretFinalizer = "openstack.org/ac-secret-protection" // #nosec G101 const finalizer = "openstack.org/applicationcredential" // #nosec G101 +var errACIDMismatch = fmt.Errorf("AC secret already exists with a different ACID") + // ApplicationCredentialReconciler reconciles an ApplicationCredential object type ApplicationCredentialReconciler struct { client.Client @@ -59,6 +67,7 @@ type ApplicationCredentialReconciler struct { //+kubebuilder:rbac:groups=keystone.openstack.org,resources=keystoneapplicationcredentials,verbs=get;list;watch;create;update;patch;delete //+kubebuilder:rbac:groups=keystone.openstack.org,resources=keystoneapplicationcredentials/status,verbs=get;update;patch //+kubebuilder:rbac:groups=keystone.openstack.org,resources=keystoneapplicationcredentials/finalizers,verbs=update;patch +//+kubebuilder:rbac:groups=core,resources=secrets,verbs=delete //+kubebuilder:rbac:groups=core,resources=secrets/finalizers,verbs=get;list;create;update;delete;patch //+kubebuilder:rbac:groups=core,resources=events,verbs=create;patch @@ -140,7 +149,7 @@ func (r *ApplicationCredentialReconciler) Reconcile(ctx context.Context, req ctr // any actual ApplicationCredential, just redirect execution to the "reconcileDelete()" // logic to avoid potentially hanging on waiting for a KeystoneAPI to appear if !instance.DeletionTimestamp.IsZero() { - return r.reconcileDelete(ctx, instance, helperObj) + return r.reconcileDelete(ctx, instance, helperObj, nil) } instance.Status.Conditions.Set(condition.FalseCondition( @@ -162,10 +171,11 @@ func (r *ApplicationCredentialReconciler) Reconcile(ctx context.Context, req ctr return ctrl.Result{}, err } - // If this KeystoneApplicationCredential CR is being deleted, perform delete cleanup without waiting for KeystoneAPI readiness - // NOTE: We don't talk to KeystoneAPI during delete, so KeystoneAPI readiness/deletion check is not needed here + // Deletion skips the KeystoneAPI IsReady() check below, so the AC CR is not blocked on readiness + // reconcileDelete receives keystoneAPI to attempt best-effort Keystone AC revocation + // If Keystone is unreachable (e.g. full teardown), revocation is skipped and finalizers are still removed so the CR deletion is never blocked if !instance.DeletionTimestamp.IsZero() { - return r.reconcileDelete(ctx, instance, helperObj) + return r.reconcileDelete(ctx, instance, helperObj, keystoneAPI) } if !keystoneAPI.IsReady() { @@ -203,7 +213,8 @@ func (r *ApplicationCredentialReconciler) reconcileNormal( return ctrl.Result{}, err } - // Inspect current Secret status + // If the current secret was deleted (e.g. manual cleanup, accidental removal), + // fall through to rotation so the controller self-heals. if instance.Status.SecretName != "" { secret := &corev1.Secret{} key := types.NamespacedName{Namespace: instance.Namespace, Name: instance.Status.SecretName} @@ -283,9 +294,23 @@ func (r *ApplicationCredentialReconciler) reconcileNormal( return ctrl.Result{}, err } - // Store it in a Secret (create or update) - secretName := fmt.Sprintf("%s-secret", instance.Name) - if err := r.storeACSecret(ctx, helperObj, instance, secretName, newID, newSecret); err != nil { + // Create a new immutable Secret with a unique name + secretName, err := r.createImmutableACSecret(ctx, helperObj, instance, newID, newSecret) + if err != nil { + // The Keystone AC was already created above but its secret cannot be stored. + // Revoke it so it doesn't become a permanently orphaned credential in Keystone. + if revokeErr := revokeKeystoneAC(ctx, userOS.GetOSClient(), userID, newID); revokeErr != nil { + logger.Error(revokeErr, "Failed to revoke orphaned Keystone AC after secret creation failure", "ACID", newID) + } else { + logger.Info("Revoked orphaned Keystone AC after secret creation failure", "ACID", newID) + } + instance.Status.Conditions.Set(condition.FalseCondition( + keystonev1.KeystoneApplicationCredentialReadyCondition, + condition.ErrorReason, + condition.SeverityWarning, + keystonev1.KeystoneApplicationCredentialReadyErrorMessage, + fmt.Sprintf("Failed to create AC secret: %s", err.Error()), + )) return ctrl.Result{}, err } @@ -295,7 +320,10 @@ func (r *ApplicationCredentialReconciler) reconcileNormal( previousExpiresAt = instance.Status.ExpiresAt.Format(time.RFC3339) } - // Update status + // Update status, capture previous secret before rotation for rollback tracking + if isRotation && instance.Status.SecretName != "" { + instance.Status.PreviousSecretName = instance.Status.SecretName + } instance.Status.ACID = newID instance.Status.SecretName = secretName instance.Status.CreatedAt = &metav1.Time{Time: time.Now().UTC()} @@ -335,7 +363,23 @@ func (r *ApplicationCredentialReconciler) reconcileNormal( } logger.Info("ApplicationCredential ready", "secret", secretName, "ACID", newID, "expiresAt", expiresAt) - return ctrl.Result{}, nil + + // Return early after creation/rotation so the defer patches the status. + // The status patch triggers a re-reconcile via the For() watch, at which + // point the cache is fresh and cleanup of unused rotated secrets can run. + // RequeueAfter is a safety net in case the watch event is delayed. + return ctrl.Result{RequeueAfter: time.Second * 10}, nil + } + + // Migrate old mutable secrets: add the application-credential-service label + // if missing, so they become visible to label-based cleanup and deletion queries + serviceName := strings.TrimPrefix(instance.Name, "ac-") + for _, sn := range []string{instance.Status.SecretName, instance.Status.PreviousSecretName} { + if sn != "" { + if err := r.ensureServiceLabel(ctx, helperObj, sn, instance.Namespace, serviceName); err != nil { + logger.Info("Could not ensure service label on secret", "secret", sn, "error", err) + } + } } // ApplicationCredential already exists and is valid @@ -346,44 +390,248 @@ func (r *ApplicationCredentialReconciler) reconcileNormal( instance.Status.RotationEligibleAt = &metav1.Time{Time: rotationEligibleAt} } + // Unused rotated AC secrets (not current/previous, no consumer finalizer), best effort + // Failures are logged but do not block the AC CR from reaching Ready, since the current credentials + // are valid regardless. Cleanup will be retried on the next reconcile. + userOS, userRes, userErr := keystonev1.GetUserServiceClient( + ctx, helperObj, keystoneAPI, + instance.Spec.UserName, + instance.Spec.Secret, + instance.Spec.PasswordSelector, + ) + if userErr != nil { + logger.Info("Could not build Keystone client; skipping unused rotated AC secret cleanup", "error", userErr) + } else if userRes != (ctrl.Result{}) { + logger.Info("Keystone client not ready; skipping unused rotated AC secret cleanup") + } else { + userID, err := r.getUserIDFromToken(ctx, userOS.GetOSClient(), instance.Spec.UserName) + if err != nil { + logger.Info("Could not get user ID; skipping unused rotated AC secret cleanup", "error", err) + } else { + if err := r.cleanupUnusedRotatedSecrets(ctx, instance, helperObj, userOS.GetOSClient(), userID); err != nil { + logger.Error(err, "Unused rotated AC secret cleanup failed, will retry on next reconcile") + } + } + } + instance.Status.Conditions.MarkTrue(keystonev1.KeystoneApplicationCredentialReadyCondition, keystonev1.KeystoneApplicationCredentialReadyMessage) return ctrl.Result{}, nil } +// reconcileDelete runs when the AC CR is deleted (removed from OpenStackControlPlane or manually) +// Best-effort: try to revoke Keystone ACs, then remove all protection finalizers func (r *ApplicationCredentialReconciler) reconcileDelete( ctx context.Context, instance *keystonev1.KeystoneApplicationCredential, helperObj *helper.Helper, + keystoneAPI *keystonev1.KeystoneAPI, ) (ctrl.Result, error) { logger := r.GetLogger(ctx) logger.Info("Reconciling ApplicationCredential delete") - // NOTE: We intentionally do NOT delete the ApplicationCredential from Keystone. - // This prevents breaking services (especially EDPM nodes) that are actively using these credentials. - // The AC will expire naturally based on its expiration time. If immediate cleanup is needed, - // operators can manually delete the AC from Keystone using: openstack application credential delete + serviceName := strings.TrimPrefix(instance.Name, "ac-") - // Before removing our CR finalizer, allow ApplicationCredential Secret to be deleted by removing its protection finalizer - if instance.Status.SecretName != "" { - key := types.NamespacedName{Namespace: instance.Namespace, Name: instance.Status.SecretName} - secret := &corev1.Secret{} - if err := helperObj.GetClient().Get(ctx, key, secret); err == nil { - if controllerutil.RemoveFinalizer(secret, acSecretFinalizer) { - if err := helperObj.GetClient().Update(ctx, secret); err != nil { - return ctrl.Result{}, err + // Migrate old mutable secrets before the label-based list query + for _, sn := range []string{instance.Status.SecretName, instance.Status.PreviousSecretName} { + if sn != "" { + if err := r.ensureServiceLabel(ctx, helperObj, sn, instance.Namespace, serviceName); err != nil { + logger.Info("Could not ensure service label on secret during delete", "secret", sn, "error", err) + } + } + } + + acLabels := map[string]string{ + "application-credentials": "true", + "application-credential-service": serviceName, + } + secretList, err := oko_secret.GetSecrets(ctx, helperObj, instance.Namespace, acLabels) + if err != nil { + return ctrl.Result{}, err + } + + // Build a Keystone client for best-effort revocation (nil if unavailable) + var identClient *gophercloud.ServiceClient + var userID string + if keystoneAPI != nil { + userOS, userRes, userErr := keystonev1.GetUserServiceClient( + ctx, helperObj, keystoneAPI, + instance.Spec.UserName, + instance.Spec.Secret, + instance.Spec.PasswordSelector, + ) + if userErr != nil || userRes != (ctrl.Result{}) { + logger.Info("Could not build Keystone client, skipping revocation during AC CR delete", "error", userErr) + } else if uid, err := r.getUserIDFromToken(ctx, userOS.GetOSClient(), instance.Spec.UserName); err != nil { + logger.Info("Could not get user ID, skipping revocation during AC CR delete", "error", err) + } else { + identClient = userOS.GetOSClient() + userID = uid + } + } else { + logger.Info("KeystoneAPI not present, skipping Keystone revocation during AC CR delete") + } + + // Single pass: revoke ACs in Keystone (best-effort) and strip protection finalizers + seen := make(map[string]bool) + processed := make(map[string]bool, len(secretList.Items)) + for i := range secretList.Items { + s := &secretList.Items[i] + processed[s.Name] = true + + if identClient != nil { + acID := string(s.Data[keystonev1.ACIDSecretKey]) + if acID != "" && !seen[acID] { + seen[acID] = true + if err := revokeKeystoneAC(ctx, identClient, userID, acID); err != nil { + logger.Info("Keystone revocation failed during AC CR delete, continuing", "ACID", acID, "error", err) + } else { + logger.Info("Revoked AC in Keystone during AC CR delete", "ACID", acID) } } - } else if !k8s_errors.IsNotFound(err) { + } + + if controllerutil.RemoveFinalizer(s, acSecretFinalizer) { + logger.Info("Removing protection finalizer from AC secret", "secret", s.Name) + if err := helperObj.GetClient().Update(ctx, s); err != nil { + return ctrl.Result{}, err + } + } + } + + // Fallback: status-referenced secrets may not appear in the label-based + // list if ensureServiceLabel just added the label and the cache hasn't + // synced yet. Process them directly to avoid leaving orphaned finalizers + for _, sn := range []string{instance.Status.SecretName, instance.Status.PreviousSecretName} { + if sn == "" || processed[sn] { + continue + } + fallbackSecret, _, err := oko_secret.GetSecret(ctx, helperObj, sn, instance.Namespace) + if err != nil { + if k8s_errors.IsNotFound(err) { + continue + } return ctrl.Result{}, err } + if controllerutil.RemoveFinalizer(fallbackSecret, acSecretFinalizer) { + logger.Info("Removing protection finalizer from status-referenced AC secret", "secret", sn) + if err := helperObj.GetClient().Update(ctx, fallbackSecret); err != nil { + return ctrl.Result{}, err + } + } } - // Remove finalizer from the ApplicationCredential CR, patching is done in the defer function controllerutil.RemoveFinalizer(instance, finalizer) - return ctrl.Result{}, nil } +// revokeKeystoneAC deletes one application credential in Keystone +// 404 is ignored (already revoked) +func revokeKeystoneAC( + ctx context.Context, + identClient *gophercloud.ServiceClient, + userID, acID string, +) error { + res := applicationcredentials.Delete(ctx, identClient, userID, acID) + if res.Err != nil && !gophercloud.ResponseCodeIs(res.Err, http.StatusNotFound) { + return fmt.Errorf("failed to revoke application credential %s in Keystone: %w", acID, res.Err) + } + return nil +} + +// ensureServiceLabel adds the application-credential-service label to a secret if it's missing +// This migrates old mutable secrets, so they become visible to label-based queries used by cleanup and deletion +func (r *ApplicationCredentialReconciler) ensureServiceLabel( + ctx context.Context, + helperObj *helper.Helper, + secretName, namespace, serviceName string, +) error { + secret := &corev1.Secret{} + key := types.NamespacedName{Namespace: namespace, Name: secretName} + if err := helperObj.GetClient().Get(ctx, key, secret); err != nil { + return client.IgnoreNotFound(err) + } + if secret.Labels != nil && secret.Labels["application-credential-service"] == serviceName { + return nil + } + if secret.Labels == nil { + secret.Labels = map[string]string{} + } + secret.Labels["application-credential-service"] = serviceName + return helperObj.GetClient().Update(ctx, secret) +} + +// hasConsumerFinalizer returns true if the secret has any finalizer matching the AC consumer convention (suffix: -ac-consumer) +// +// Currently the controlplane service operators (barbican, cinder, etc.) place +// finalizers like "openstack.org/barbican-ac-consumer" on the AC secret they +// are actively using +// +// TODO(OSPRH-28176): EDPM services (nova, ceilometer, aodh) that consume AC +// secrets on dataplane nodes must also place an EDPM-scoped consumer finalizer +// (e.g. "openstack.org/edpm-nova-ac-consumer") on the secret. This will +// prevent the keystone-operator from revoking/deleting a secret that is still +// deployed to dataplane nodes that have not yet been updated. The EDPM +// finalizer should only be removed once all nodes across all NodeSets have +// been redeployed with the new credentials. This depends on per-node secret +// rotation tracking: https://github.com/openstack-k8s-operators/openstack-operator/pull/1781 +func hasConsumerFinalizer(secret *corev1.Secret) bool { + for _, f := range secret.Finalizers { + if strings.HasPrefix(f, "openstack.org/") && strings.HasSuffix(f, "-ac-consumer") { + return true + } + } + return false +} + +// cleanupUnusedRotatedSecrets runs during reconcileNormal (AC CR is not being deleted) +// Finds rotated secrets that are neither current nor previous and have no service consumer finalizer +// For each: revoke its AC in Keystone, remove the protection finalizer, delete the K8s Secret +func (r *ApplicationCredentialReconciler) cleanupUnusedRotatedSecrets( + ctx context.Context, + instance *keystonev1.KeystoneApplicationCredential, + helperObj *helper.Helper, + identClient *gophercloud.ServiceClient, + userID string, +) error { + logger := r.GetLogger(ctx) + serviceName := strings.TrimPrefix(instance.Name, "ac-") + + secretList, err := oko_secret.GetSecrets(ctx, helperObj, instance.Namespace, map[string]string{ + "application-credentials": "true", + "application-credential-service": serviceName, + }) + if err != nil { + return err + } + + for i := range secretList.Items { + s := &secretList.Items[i] + if s.Name == instance.Status.SecretName || s.Name == instance.Status.PreviousSecretName || hasConsumerFinalizer(s) { + continue + } + + acID := string(s.Data[keystonev1.ACIDSecretKey]) + if acID != "" { + if err := revokeKeystoneAC(ctx, identClient, userID, acID); err != nil { + return err + } + logger.Info("Revoked AC in Keystone", "ACID", acID, "secret", s.Name) + } + + if controllerutil.RemoveFinalizer(s, acSecretFinalizer) { + if err := helperObj.GetClient().Update(ctx, s); err != nil { + return fmt.Errorf("failed to remove protection finalizer from %s: %w", s.Name, err) + } + logger.Info("Removed protection finalizer from AC secret", "secret", s.Name) + } + if err := helperObj.GetClient().Delete(ctx, s); err != nil && !k8s_errors.IsNotFound(err) { + return fmt.Errorf("failed to delete AC secret %s: %w", s.Name, err) + } + logger.Info("Deleted unused rotated AC secret", "secret", s.Name) + } + return nil +} + // createACWithName creates a new ApplicationCredential in Keystone func (r *ApplicationCredentialReconciler) createACWithName( ctx context.Context, @@ -440,41 +688,76 @@ func (r *ApplicationCredentialReconciler) createACWithName( return created.ID, created.Secret, created.ExpiresAt, nil } -func (r *ApplicationCredentialReconciler) storeACSecret( +// acSecretName returns the unique K8s Secret name for a given service name and +// Keystone AC ID: ac---secret. +func acSecretName(serviceName, acID string) string { + idPrefix := acID + if len(idPrefix) > 5 { + idPrefix = idPrefix[:5] + } + return fmt.Sprintf("ac-%s-%s-secret", serviceName, idPrefix) +} + +// createImmutableACSecret creates a new immutable K8s Secret with a unique name +// derived from the AC CR name and the first 5 characters of the Keystone AC ID +// Each rotation produces a distinct secret; old secrets are retained +// The protection finalizer is set atomically during creation to avoid a +// read-after-write cache race that would occur with a separate Get+Update +func (r *ApplicationCredentialReconciler) createImmutableACSecret( ctx context.Context, helperObj *helper.Helper, ac *keystonev1.KeystoneApplicationCredential, - secretName, newID, newSecret string, -) error { + newID, newSecret string, +) (string, error) { + logger := r.GetLogger(ctx) + + serviceName := strings.TrimPrefix(ac.Name, "ac-") + secretName := acSecretName(serviceName, newID) + immutable := true + secret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: secretName, Namespace: ac.Namespace, + Labels: map[string]string{ + "application-credentials": "true", + "application-credential-service": serviceName, + }, + Finalizers: []string{acSecretFinalizer}, }, - } - - op, err := controllerutil.CreateOrPatch(ctx, helperObj.GetClient(), secret, func() error { - secret.Labels = map[string]string{ - "application-credentials": "true", - } - secret.Data = map[string][]byte{ + Immutable: &immutable, + Data: map[string][]byte{ keystonev1.ACIDSecretKey: []byte(newID), keystonev1.ACSecretSecretKey: []byte(newSecret), - } - // Add protection finalizer - controllerutil.AddFinalizer(secret, acSecretFinalizer) - // Set owner reference - return controllerutil.SetControllerReference(ac, secret, helperObj.GetScheme()) - }) - if err != nil { - return err + }, + } + if err := controllerutil.SetControllerReference(ac, secret, helperObj.GetScheme()); err != nil { + return "", fmt.Errorf("failed to set controller reference on AC secret %s: %w", secretName, err) } - if op != controllerutil.OperationResultNone { - r.GetLogger(ctx).Info("Secret operation completed", "secret", secretName, "operation", op) + if err := helperObj.GetClient().Create(ctx, secret); err != nil { + if k8s_errors.IsAlreadyExists(err) { + // A secret with this name already exists - most likely a previous reconcile + // created it but crashed before patching the status. Validate that the + // existing secret's ACID matches the one we intended to store, a mismatch + // would indicate an ACID prefix collision (two different Keystone AC IDs + // whose first 5 characters are identical, producing the same secret name). + existing, _, getErr := oko_secret.GetSecret(ctx, helperObj, secretName, ac.Namespace) + if getErr != nil { + return "", fmt.Errorf("AC secret %s already exists but failed to fetch for validation: %w", secretName, getErr) + } + existingACID := string(existing.Data[keystonev1.ACIDSecretKey]) + if existingACID != newID { + return "", fmt.Errorf("%w: secret=%s existingACID=%s expectedACID=%s", errACIDMismatch, secretName, existingACID, newID) + } + logger.Info("Immutable AC secret already exists with matching ACID, proceeding", "secret", secretName, "ACID", newID) + return secretName, nil + } + return "", fmt.Errorf("failed to create immutable AC secret %s: %w", secretName, err) } + logger.Info("Created immutable AC secret", "secret", secretName, "ACID", newID) - return nil + return secretName, nil } // getUserIDFromToken extracts the user ID from the authenticated token @@ -525,7 +808,14 @@ func needsRotation(ac *keystonev1.KeystoneApplicationCredential) (bool, string, func (r *ApplicationCredentialReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). For(&keystonev1.KeystoneApplicationCredential{}). - Owns(&corev1.Secret{}). + // Suppress Create events on owned secrets to prevent a race condition: + // when a reconcile creates a new immutable secret, the Owns() watch would + // immediately enqueue another reconcile. That second reconcile could read + // a stale AC CR from the informer cache (status not yet patched) and + // duplicate the AC+secret creation. Update and Delete events still trigger reconciles. + Owns(&corev1.Secret{}, builder.WithPredicates(predicate.Funcs{ + CreateFunc: func(_ event.CreateEvent) bool { return false }, + })). Complete(r) } diff --git a/test/functional/base_test.go b/test/functional/base_test.go index 93cef179..6f23361a 100644 --- a/test/functional/base_test.go +++ b/test/functional/base_test.go @@ -23,13 +23,14 @@ import ( batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" + k8s_errors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" keystonev1 "github.com/openstack-k8s-operators/keystone-operator/api/v1beta1" keystone_base "github.com/openstack-k8s-operators/keystone-operator/internal/keystone" condition "github.com/openstack-k8s-operators/lib-common/modules/common/condition" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) func GetKeystoneAPISpec(fernetMaxKeys int32) map[string]any { @@ -208,6 +209,13 @@ func GetExtraMounts(kemName string, kemPath string) []map[string]any { } // ApplicationCredential helper functions + +// Finalizer constants used across AC tests +const ( + ACCRFinalizer = "openstack.org/applicationcredential" + ACSecretProtectionFinalizer = "openstack.org/ac-secret-protection" //nolint:gosec +) + func CreateACWithSpec(name types.NamespacedName, spec map[string]interface{}) client.Object { raw := map[string]interface{}{ "apiVersion": "keystone.openstack.org/v1beta1", @@ -221,6 +229,98 @@ func CreateACWithSpec(name types.NamespacedName, spec map[string]interface{}) cl return th.CreateUnstructured(raw) } +// GetDefaultACSpec returns a minimal, valid AC spec. Callers that need +// non-default values (roles, accessRules, etc.) should build their own map +func GetDefaultACSpec(userName, secretName string) map[string]interface{} { + return map[string]interface{}{ + "userName": userName, + "secret": secretName, + "passwordSelector": "ServicePassword", + "expirationDays": 30, + "gracePeriodDays": 7, + "roles": []string{"service"}, + } +} + +// CreateACServiceUserSecret creates the standard service-user Secret +// (key: ServicePassword) used by AC tests and returns its NamespacedName +func CreateACServiceUserSecret(namespace string) types.NamespacedName { + name := types.NamespacedName{Name: "osp-secret", Namespace: namespace} + th.CreateSecret(name, map[string][]byte{ + "ServicePassword": []byte("service-password"), + }) + return name +} + +// CreateProtectedACSecret creates an immutable Kubernetes Secret with the +// AC label set and the protection finalizer already applied, simulating a +// secret that was created during a previous reconcile or rotation +func CreateProtectedACSecret(name types.NamespacedName, serviceName string) *corev1.Secret { + immutable := true + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: name.Name, + Namespace: name.Namespace, + Labels: map[string]string{ + "application-credentials": "true", + "application-credential-service": serviceName, + }, + Finalizers: []string{ACSecretProtectionFinalizer}, + }, + Immutable: &immutable, + Data: map[string][]byte{ + keystonev1.ACIDSecretKey: []byte("fake-ac-id"), + keystonev1.ACSecretSecretKey: []byte("fake-ac-secret"), + }, + } + Expect(k8sClient.Create(ctx, secret)).To(Succeed()) + return secret +} + +// CreateOldStyleACSecret creates a mutable secret with only the +// application-credentials label (no application-credential-service label), +// simulating a secret created by the pre-immutable secret logic +func CreateOldStyleACSecret(name types.NamespacedName, acID string) *corev1.Secret { + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: name.Name, + Namespace: name.Namespace, + Labels: map[string]string{ + "application-credentials": "true", + }, + Finalizers: []string{ACSecretProtectionFinalizer}, + }, + Data: map[string][]byte{ + keystonev1.ACIDSecretKey: []byte(acID), + keystonev1.ACSecretSecretKey: []byte("fake-ac-secret"), + }, + } + Expect(k8sClient.Create(ctx, secret)).To(Succeed()) + return secret +} + +// WaitForACFinalizer blocks until the AC CR carries its controller finalizer +func WaitForACFinalizer(name types.NamespacedName) { + Eventually(func(g Gomega) { + ac := GetApplicationCredential(name) + g.Expect(ac.Finalizers).To(ContainElement(ACCRFinalizer)) + }, timeout, interval).Should(Succeed()) +} + +// DeleteACCR issues a delete on the AC CR. It does not wait for the CR to disappear +func DeleteACCR(name types.NamespacedName) { + acInstance := GetApplicationCredential(name) + Expect(k8sClient.Delete(ctx, acInstance)).To(Succeed()) +} + +// WaitForACGone blocks until the AC CR is fully removed from the API server +func WaitForACGone(name types.NamespacedName) { + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, name, &keystonev1.KeystoneApplicationCredential{}) + g.Expect(k8s_errors.IsNotFound(err)).To(BeTrue()) + }, timeout, interval).Should(Succeed()) +} + func GetApplicationCredential(name types.NamespacedName) *keystonev1.KeystoneApplicationCredential { instance := &keystonev1.KeystoneApplicationCredential{} Eventually(func(g Gomega) { diff --git a/test/functional/keystoneapi_controller_test.go b/test/functional/keystoneapi_controller_test.go index 330602fb..421a808e 100644 --- a/test/functional/keystoneapi_controller_test.go +++ b/test/functional/keystoneapi_controller_test.go @@ -2614,39 +2614,127 @@ OIDCRedirectURI "{{ .KeystoneEndpointPublic }}/v3/auth/OS-FEDERATION/websso/open }) It("should handle deletion with finalizer cleanup", func() { - serviceUserSecret := types.NamespacedName{Name: "osp-secret", Namespace: namespace} - th.CreateSecret(serviceUserSecret, map[string][]byte{ - "ServicePassword": []byte("service-password"), - }) + userSecret := CreateACServiceUserSecret(namespace) deleteACName := types.NamespacedName{Name: "ac-delete-test", Namespace: namespace} - CreateACWithSpec(deleteACName, map[string]interface{}{ - "userName": "test-service", - "secret": serviceUserSecret.Name, - "passwordSelector": "ServicePassword", - "expirationDays": 30, - "gracePeriodDays": 7, - "roles": []string{"service"}, - }) + CreateACWithSpec(deleteACName, GetDefaultACSpec("test-service", userSecret.Name)) + WaitForACFinalizer(deleteACName) + // The controller removes its own finalizer during reconcileDelete, so the CR should be fully deleted + DeleteACCR(deleteACName) + WaitForACGone(deleteACName) + }) + + It("should remove the protection finalizer from all AC secrets for the service on deletion", func() { + userSecret := CreateACServiceUserSecret(namespace) + + deleteACName := types.NamespacedName{Name: "ac-multi-secret-svc", Namespace: namespace} + CreateACWithSpec(deleteACName, GetDefaultACSpec("multi-secret-svc", userSecret.Name)) + WaitForACFinalizer(deleteACName) + + // Simulate two immutable AC secrets left from an initial creation and one rotation + firstSecretName := types.NamespacedName{Name: "ac-multi-secret-svc-aaaaa-secret", Namespace: namespace} + secondSecretName := types.NamespacedName{Name: "ac-multi-secret-svc-bbbbb-secret", Namespace: namespace} + CreateProtectedACSecret(firstSecretName, "multi-secret-svc") + CreateProtectedACSecret(secondSecretName, "multi-secret-svc") + + // Both secrets must start protected + for _, sName := range []types.NamespacedName{firstSecretName, secondSecretName} { + s := &corev1.Secret{} + Expect(k8sClient.Get(ctx, sName, s)).To(Succeed()) + Expect(s.Finalizers).To(ContainElement(ACSecretProtectionFinalizer)) + } + + DeleteACCR(deleteACName) + + // The controller should strip the protection finalizer from every secret carrying the service label, and not just Status.SecretName Eventually(func(g Gomega) { - ac := GetApplicationCredential(deleteACName) - g.Expect(ac.Finalizers).To(ContainElement("openstack.org/applicationcredential")) + for _, sName := range []types.NamespacedName{firstSecretName, secondSecretName} { + s := &corev1.Secret{} + g.Expect(k8sClient.Get(ctx, sName, s)).To(Succeed()) + g.Expect(s.Finalizers).NotTo(ContainElement(ACSecretProtectionFinalizer), + "secret %s should have its protection finalizer removed", sName.Name) + } + }, timeout, interval).Should(Succeed()) + + WaitForACGone(deleteACName) + }) + + It("should not remove AC secrets belonging to a different service on deletion", func() { + userSecret := CreateACServiceUserSecret(namespace) + + acSvc1Name := types.NamespacedName{Name: "ac-svc-alpha", Namespace: namespace} + acSvc2Name := types.NamespacedName{Name: "ac-svc-beta", Namespace: namespace} + for _, name := range []types.NamespacedName{acSvc1Name, acSvc2Name} { + CreateACWithSpec(name, GetDefaultACSpec(name.Name, userSecret.Name)) + } + for _, name := range []types.NamespacedName{acSvc1Name, acSvc2Name} { + WaitForACFinalizer(name) + } + + svc1SecretName := types.NamespacedName{Name: "ac-svc-alpha-ccccc-secret", Namespace: namespace} + svc2SecretName := types.NamespacedName{Name: "ac-svc-beta-ddddd-secret", Namespace: namespace} + CreateProtectedACSecret(svc1SecretName, "svc-alpha") + CreateProtectedACSecret(svc2SecretName, "svc-beta") + + // Delete only ac-svc-alpha + DeleteACCR(acSvc1Name) + + // svc-alpha secret must lose its finalizer + Eventually(func(g Gomega) { + s := &corev1.Secret{} + g.Expect(k8sClient.Get(ctx, svc1SecretName, s)).To(Succeed()) + g.Expect(s.Finalizers).NotTo(ContainElement(ACSecretProtectionFinalizer)) }, timeout, interval).Should(Succeed()) - acInstance := GetApplicationCredential(deleteACName) - err := k8sClient.Delete(ctx, acInstance) - Expect(err).NotTo(HaveOccurred()) + // svc-beta secret must keep its finalizer throughout + Consistently(func(g Gomega) { + s := &corev1.Secret{} + g.Expect(k8sClient.Get(ctx, svc2SecretName, s)).To(Succeed()) + g.Expect(s.Finalizers).To(ContainElement(ACSecretProtectionFinalizer), + "svc-beta secret should retain its protection finalizer") + }, "3s", interval).Should(Succeed()) + }) + + It("should migrate old mutable secrets by adding the service label on deletion", func() { + userSecret := CreateACServiceUserSecret(namespace) + + acName := types.NamespacedName{Name: "ac-migrate-svc", Namespace: namespace} + CreateACWithSpec(acName, GetDefaultACSpec("migrate-svc", userSecret.Name)) + WaitForACFinalizer(acName) + + oldSecretName := types.NamespacedName{Name: "ac-migrate-svc-secret", Namespace: namespace} + CreateOldStyleACSecret(oldSecretName, "old-ac-id-12345") + // Patch status to point to the old-style secret (simulating upgrade from old controller) Eventually(func(g Gomega) { ac := &keystonev1.KeystoneApplicationCredential{} - err := k8sClient.Get(ctx, deleteACName, ac) - if k8s_errors.IsNotFound(err) { - return - } - g.Expect(err).NotTo(HaveOccurred()) - g.Expect(ac.DeletionTimestamp).NotTo(BeNil()) + g.Expect(k8sClient.Get(ctx, acName, ac)).To(Succeed()) + ac.Status.ACID = "old-ac-id-12345" + ac.Status.SecretName = oldSecretName.Name + g.Expect(k8sClient.Status().Update(ctx, ac)).To(Succeed()) }, timeout, interval).Should(Succeed()) + + // Verify the old secret does not have the service label yet + oldSecret := &corev1.Secret{} + Expect(k8sClient.Get(ctx, oldSecretName, oldSecret)).To(Succeed()) + Expect(oldSecret.Labels).NotTo(HaveKey("application-credential-service")) + + DeleteACCR(acName) + + // reconcileDelete calls ensureServiceLabel before the label-based + // list query, so the old secret should get the service label added + // and then have its protection finalizer removed + Eventually(func(g Gomega) { + s := &corev1.Secret{} + g.Expect(k8sClient.Get(ctx, oldSecretName, s)).To(Succeed()) + g.Expect(s.Labels).To(HaveKeyWithValue( + "application-credential-service", "migrate-svc")) + g.Expect(s.Finalizers).NotTo(ContainElement(ACSecretProtectionFinalizer), + "old-style secret should have its protection finalizer removed after migration") + }, timeout, interval).Should(Succeed()) + + WaitForACGone(acName) }) It("should validate expiration vs grace period constraints", func() {