Implement immutable secrets for application credentials

Deleted unused `GetApplicationCredentialFromSecret` function and introduce immutable per-rotation AC secrets with deterministic names,
add Keystone-side revocation of unused rotated ACs, and suppress Owns() create events on the secret
watch to prevent a race condition caused by stale informer cach and sometimes causing additional AC secret to be created and deleted immediately during rotation.

Signed-off-by: Veronika Fisarova <vfisarov@redhat.com>
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>

Author: Deydra71

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).

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)
 }

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"`
 

api/v1beta1/zz_generated.deepcopy.go

@@ -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).

config/crd/bases/keystone.openstack.org_keystoneapplicationcredentials.yaml

@@ -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-<service>-<first5ofACID>-secret
+  secretName: "ac-barbican-7b23d-secret"
   # CreatedAt - timestamp of creation
   createdAt: "2025-05-29T09:02:28Z"
   # ExpiresAt - time of validity expiration
@@ -120,31 +121,38 @@ 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-<service>-<first5ofACID>-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:     <base64-of-AC-ID>
   AC_SECRET: <base64-of-AC-secret>
 ```
 
 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 <ac-id>
 
 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: