Add credential rotation helpers and EnsureFresh cache bypass

Add shared helpers for transport URL secret rotation across all
openstack-k8s-operators consumer operators:

- object.ManageSecretConsumerFinalizer: adds a consumer finalizer
  to a secret so the provider knows consumers still depend on it
- object.RemoveSecretConsumerFinalizer: removes the finalizer
- object.FinalizeSecretRotation: the rotation guard — if old != new
  and guard is ready, removes finalizer from old and returns new;
  otherwise returns old to preserve the reference
- condition.CredentialRotationGuardReady: returns true when all
  sub-CR specs are stable and all mirrored conditions are True
- helper.EnsureFresh: re-reads a sub-CR directly from the API server
  (bypassing the informer cache) when CreateOrPatch returns None
  during rotation, preventing false-positive guard evaluation from
  stale cached Generation/ObservedGeneration
- helper.SetAPIReader: stores the cache-bypassing reader
- statefulset.IsReady / deployment.IsReady: check UpdatedReplicas,
  CurrentRevision, and ObservedGeneration so DeploymentReady is only
  True when all pods have rolled to the new config
- StatefulSet/Deployment CreateOrPatch cache fix: when op != None,
  use the server-returned object instead of the stale cache

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: lmiccini

modules/common/condition/credential_rotation_guard.md

@@ -0,0 +1,151 @@
+# Credential rotation consumer finalizer guard
+
+OpenStack service operators that consume rotating credential secrets
+(transport URL, application credentials, or similar) attach a consumer
+finalizer to the old secret so it is not deleted until every sub-service
+has rolled out with the new credential.
+
+## Shared helpers
+
+Use helpers from these packages instead of duplicating guard logic in each
+operator:
+
+- `object.ManageSecretConsumerFinalizer` — add finalizer to current secret
+- `object.RemoveSecretConsumerFinalizer` — remove finalizer from old secret
+- `object.FinalizeSecretRotation` — rotation guard: hold old finalizer
+  until all sub-services are ready, then release
+- `condition.CredentialRotationGuardReady` — compute `guardReady` for the
+  rotation guard
+- `helper.EnsureFresh` — bypass informer cache during rotation so the
+  parent reads accurate sub-CR conditions from the API server
+- `helper.SetAPIReader` — configure the cache-bypassing reader
+- `statefulset.IsReady` / `deployment.IsReady` — check UpdatedReplicas,
+  CurrentRevision, and ObservedGeneration so DeploymentReady is only
+  True when all pods have rolled to the new config
+
+## Parent controller integration
+
+### 1. Wire up APIReader
+
+Add `APIReader client.Reader` to the reconciler struct and pass
+`mgr.GetAPIReader()` from `cmd/main.go`. In `Reconcile`, call
+`helper.SetAPIReader(r.APIReader)` after creating the helper.
+
+### 2. Pass transport URL directly — never through Status
+
+Pass `transportURL.Status.SecretName` as a parameter to sub-CR creation
+functions and config generation. Never read from
+`instance.Status.TransportURLSecret` when building sub-CR specs — the
+status field is only used as the "old" value for `FinalizeSecretRotation`.
+
+Set `instance.Status.TransportURLSecret` early only for first-time setup:
+
+```go
+if instance.Status.TransportURLSecret == "" ||
+    instance.Status.TransportURLSecret == transportURL.Status.SecretName {
+    instance.Status.TransportURLSecret = transportURL.Status.SecretName
+}
+```
+
+During rotation (old != current), the status is updated solely by
+`FinalizeSecretRotation` at the end of reconcile.
+
+### 3. Track stability and call EnsureFresh
+
+Use a simple boolean instead of StabilityTracker:
+
+```go
+allSubCRsStable := true
+rotationInProgress := instance.Status.TransportURLSecret != "" &&
+    instance.Status.TransportURLSecret != transportURL.Status.SecretName
+
+// After each sub-CR CreateOrPatch:
+subCR, op, err := r.subCRCreateOrUpdate(ctx, instance, transportURL.Status.SecretName)
+if err != nil { return ctrl.Result{}, err }
+if err := helper.EnsureFresh(ctx, op, subCR, rotationInProgress); err != nil {
+    return ctrl.Result{}, err
+}
+if op != controllerutil.OperationResultNone {
+    allSubCRsStable = false
+}
+```
+
+`EnsureFresh` re-reads the sub-CR directly from the API server when
+`op == None` and rotation is in progress. Without this, the informer
+cache may return stale data where `Generation == ObservedGeneration`
+from before the spec change, causing the guard to pass prematurely.
+
+### 4. Transport URL annotation (operators without TransportURLSecret in sub-CR spec)
+
+For operators whose sub-CR specs do not include a `TransportURLSecret`
+field (e.g. nova, watcher, glance, designate, octavia, telemetry), add
+an annotation inside the `CreateOrPatch` mutate function to force a
+spec change when the transport URL rotates:
+
+```go
+op, err := controllerutil.CreateOrPatch(ctx, r.Client, subCR, func() error {
+    subCR.Spec = spec
+    if subCR.Annotations == nil {
+        subCR.Annotations = map[string]string{}
+    }
+    subCR.Annotations["openstack.org/transport-url-secret"] = transportURLSecretName
+    return controllerutil.SetControllerReference(instance, subCR, r.Scheme)
+})
+```
+
+This is not needed for operators that pass `TransportURLSecret` directly
+in the sub-CR spec (cinder, manila, ironic, barbican, heat) — the spec
+field change already makes `CreateOrPatch` return `"updated"`.
+
+### 5. Compute the rotation guard and finalize
+
+```go
+guardReady := condition.CredentialRotationGuardReady(
+    allSubCRsStable,
+    &instance.Status.Conditions,
+)
+
+instance.Status.TransportURLSecret, err = object.FinalizeSecretRotation(
+    ctx, helper, instance.Namespace,
+    instance.Status.TransportURLSecret,
+    transportURL.Status.SecretName,
+    myTransportConsumerFinalizer,
+    guardReady,
+)
+```
+
+The same pattern works for application credential secrets:
+
+```go
+instance.Status.ApplicationCredentialSecret, err = object.FinalizeSecretRotation(
+    ctx, helper, instance.Namespace,
+    instance.Status.ApplicationCredentialSecret,
+    instance.Spec.Auth.ApplicationCredentialSecret,
+    myACConsumerFinalizer,
+    guardReady,
+)
+```
+
+## Sub-CR integration
+
+Sub-CR controllers should use `statefulset.IsReady()` or
+`deployment.IsReady()` for the `DeploymentReadyCondition` check:
+
+```go
+if statefulset.IsReady(ssData) {
+    instance.Status.Conditions.MarkTrue(
+        condition.DeploymentReadyCondition,
+        condition.DeploymentReadyMessage)
+} else if *instance.Spec.Replicas > 0 {
+    instance.Status.Conditions.Set(condition.FalseCondition(
+        condition.DeploymentReadyCondition,
+        condition.RequestedReason,
+        condition.SeverityInfo,
+        condition.DeploymentReadyRunningMessage))
+}
+```
+
+These helpers check `*Replicas == ReadyReplicas`, `*Replicas == UpdatedReplicas`,
+`Generation == ObservedGeneration`, and `CurrentRevision == UpdateRevision`,
+ensuring DeploymentReady is only True when all pods have rolled to the
+new config.

modules/common/condition/funcs.go

@@ -166,6 +166,34 @@ func (conditions *Conditions) IsUnknown(t Type) bool {
 	return true
 }
 
+// ServiceInstanceIsReady reports whether a service sub-CR has finished
+// reconciling its current spec and its deployment is ready to serve
+// requests. It implements the generation/observedGeneration guard used
+// by OpenStack service operators during credential rotation rollouts.
+func ServiceInstanceIsReady(generation, observedGeneration int64, readyCount, replicas int32, conditions *Conditions) bool {
+	if conditions == nil {
+		return false
+	}
+	return generation == observedGeneration &&
+		readyCount == replicas &&
+		(conditions.IsTrue(DeploymentReadyCondition) ||
+			(conditions.IsFalse(DeploymentReadyCondition) && replicas == 0))
+}
+
+// CredentialRotationGuardReady reports whether it is safe to remove a
+// consumer finalizer from an old transport URL or application credential
+// secret. The guard requires all sub-CR specs to be stable (no pending
+// CreateOrPatch updates) and all parent sub-conditions to be True.
+// Note: ReadyCondition is NOT checked because operators reset conditions
+// via Init() at the top of each Reconcile, so ReadyCondition is Unknown
+// until the very end. AllSubConditionIsTrue covers the same safety.
+func CredentialRotationGuardReady(allSubCRsStable bool, conditions *Conditions) bool {
+	if conditions == nil {
+		return false
+	}
+	return allSubCRsStable && conditions.AllSubConditionIsTrue()
+}
+
 // AllSubConditionIsTrue validates if all subconditions are True
 // It assumes that all conditions report success via the True status
 func (conditions *Conditions) AllSubConditionIsTrue() bool {

modules/common/condition/funcs_test.go

@@ -351,6 +351,41 @@ func TestIsMethods(t *testing.T) {
 	g.Expect(conditions.IsUnknown("unknownB")).To(BeTrue())
 }
 
+func TestServiceInstanceIsReady(t *testing.T) {
+	g := NewWithT(t)
+
+	readyConditions := CreateList(
+		TrueCondition(DeploymentReadyCondition, "deployment ready"),
+	)
+
+	notReadyConditions := CreateList(
+		FalseCondition(DeploymentReadyCondition, InitReason, SeverityInfo, "creating"),
+	)
+
+	g.Expect(ServiceInstanceIsReady(2, 2, 3, 3, &readyConditions)).To(BeTrue())
+	g.Expect(ServiceInstanceIsReady(2, 1, 3, 3, &readyConditions)).To(BeFalse())
+	g.Expect(ServiceInstanceIsReady(2, 2, 2, 3, &readyConditions)).To(BeFalse())
+	g.Expect(ServiceInstanceIsReady(2, 2, 3, 3, &notReadyConditions)).To(BeFalse())
+	g.Expect(ServiceInstanceIsReady(1, 1, 0, 0, &notReadyConditions)).To(BeTrue())
+	g.Expect(ServiceInstanceIsReady(1, 1, 0, 0, nil)).To(BeFalse())
+}
+
+func TestCredentialRotationGuardReady(t *testing.T) {
+	g := NewWithT(t)
+
+	conditions := Conditions{}
+	conditions.Init(nil)
+	conditions.MarkTrue("a", "ready")
+	conditions.MarkTrue("b", "ready")
+
+	g.Expect(CredentialRotationGuardReady(true, &conditions)).To(BeTrue())
+	conditions.MarkFalse("a", InitReason, SeverityInfo, "not ready")
+	g.Expect(CredentialRotationGuardReady(true, &conditions)).To(BeFalse())
+	conditions.MarkTrue("a", "ready")
+	g.Expect(CredentialRotationGuardReady(false, &conditions)).To(BeFalse())
+	g.Expect(CredentialRotationGuardReady(true, nil)).To(BeFalse())
+}
+
 func TestAllSubConditionIsTrue(t *testing.T) {
 	conditions := Conditions{}
 

modules/common/deployment/deployment.go

@@ -87,10 +87,20 @@ func (d *Deployment) CreateOrPatch(
 		h.GetLogger().Info(fmt.Sprintf("Deployment %s - %s", deployment.Name, op))
 	}
 
-	// update the deployment object of the deployment type
-	d.deployment, err = GetDeploymentWithName(ctx, h, deployment.GetName(), deployment.GetNamespace())
-	if err != nil {
-		return ctrl.Result{}, err
+	if op == controllerutil.OperationResultNone {
+		// Re-read from cache to pick up status updates from the
+		// Deployment controller (e.g. updated ReadyReplicas).
+		d.deployment, err = GetDeploymentWithName(ctx, h, deployment.GetName(), deployment.GetNamespace())
+		if err != nil {
+			return ctrl.Result{}, err
+		}
+	} else {
+		// After a create/update the informer cache may still hold the
+		// previous version where Generation == ObservedGeneration. Using
+		// the server-returned object preserves the correct (bumped)
+		// Generation so that callers' readiness checks do not pass on
+		// stale data.
+		d.deployment = deployment
 	}
 
 	return ctrl.Result{}, nil

modules/common/helper/helper.go

@@ -47,8 +47,7 @@ type Helper struct {
 	after        *unstructured.Unstructured
 	changes      map[string]bool
 	finalizer    string
-
-	logger logr.Logger
+	logger       logr.Logger
 }
 
 // NewHelper returns an initialized Helper.