Add credential rotation helpers for secret rotation

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
- object.ManageRotationGracePeriod: time-based grace period that
  gives sub-CRs time to detect config changes, roll pods, and
  update conditions before the guard evaluates readiness
- condition.CredentialRotationGuardReady: returns true when all
  sub-CR specs are stable and all mirrored conditions are True
- condition.ServiceInstanceIsReady: generation/observedGeneration
  guard with replica count and DeploymentReadyCondition check
- statefulset.IsReady: checks ReadyReplicas, UpdatedReplicas,
  ObservedGeneration, and CurrentRevision == UpdateRevision so
  DeploymentReady is only True when all pods have rolled
- deployment.IsReady: checks ReadyReplicas, UpdatedReplicas,
  Status.Replicas, and ObservedGeneration

Note: statefulset.IsReady now additionally requires
CurrentRevision == UpdateRevision compared to the previous
implementation. This tightens the readiness check to prevent
declaring ready during in-progress rolling updates.

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

Author: lmiccini

modules/common/annotations/annotations.go

@@ -33,6 +33,10 @@ const (
 	// SkipValidationAnnotation is set on a resource to skip webhook validation.
 	// The annotation key presence is what matters; the value is ignored.
 	SkipValidationAnnotation = "openstack.org/skip-webhook-validation"
+
+	// RotationGraceAnnotation is set on an object to track when a
+	// credential rotation grace period started.
+	RotationGraceAnnotation = "openstack.org/rotation-grace-until"
 )
 
 // IsPaused returns true if the PausedAnnotation key is present on the object.

modules/common/condition/credential_rotation_guard.md

@@ -0,0 +1,189 @@
+# 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
+- `object.ManageRotationGracePeriod` — time-based grace period that gives
+  sub-CRs time to detect config changes, roll pods, and update conditions
+  before the guard evaluates readiness
+- `condition.CredentialRotationGuardReady` — compute `guardReady` for the
+  rotation guard
+- `condition.ServiceInstanceIsReady` — check whether a sub-CR has
+  finished reconciling (generation/observedGeneration guard, replica
+  counts, and DeploymentReadyCondition)
+- `statefulset.IsReady` — check `*Replicas == ReadyReplicas`,
+  `*Replicas == UpdatedReplicas`, `Generation == ObservedGeneration`, and
+  `CurrentRevision == UpdateRevision` so DeploymentReady is only True when
+  all pods have rolled to the new config
+- `deployment.IsReady` — check `*Replicas == ReadyReplicas`,
+  `*Replicas == UpdatedReplicas`, `Status.Replicas == ReadyReplicas`, and
+  `Generation == ObservedGeneration`
+
+## Parent controller integration
+
+### 1. 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.
+
+### 2. Track stability
+
+Use a simple boolean to track whether all sub-CRs are stable:
+
+```go
+allSubCRsStable := true
+
+// After each sub-CR CreateOrPatch:
+subCR, op, err := r.subCRCreateOrUpdate(ctx, instance, transportURL.Status.SecretName)
+if err != nil { return ctrl.Result{}, err }
+if op != controllerutil.OperationResultNone {
+    allSubCRsStable = false
+}
+```
+
+### 3. 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"`.
+
+### 4. 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,
+)
+```
+
+### 5. Rotation grace period (optional)
+
+Use `ManageRotationGracePeriod` to give sub-CRs time to detect config
+changes, update their Deployments/StatefulSets, and roll pods before the
+guard releases the old secret's consumer finalizer:
+
+```go
+rotationPending := instance.Status.TransportURLSecret != "" &&
+    instance.Status.TransportURLSecret != transportURL.Status.SecretName
+
+result, graceActive, err := object.ManageRotationGracePeriod(
+    ctx, r.Client, instance,
+    rotationPending,
+    30*time.Second,
+)
+if err != nil { return ctrl.Result{}, err }
+if graceActive {
+    return result, nil
+}
+```
+
+When `rotationPending` is true and no grace period is active, it sets the
+`openstack.org/rotation-grace-until` annotation and returns a requeue.
+While the grace period is active, it continues to requeue. After the
+grace period expires, it returns `graceActive == false` so the caller
+can proceed to evaluate the rotation guard. When `rotationPending` is
+false, it clears any existing annotation.
+
+## Sub-CR integration
+
+Sub-CR controllers should use `statefulset.IsReady()` or
+`deployment.IsReady()` for the `DeploymentReadyCondition` check:
+
+```go
+if statefulset.IsReady(sts) {
+    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))
+}
+```
+
+`statefulset.IsReady` takes an `appsv1.StatefulSet` and checks
+`*Replicas == ReadyReplicas`, `*Replicas == UpdatedReplicas`,
+`Generation == ObservedGeneration`, and `CurrentRevision == UpdateRevision`.
+
+`deployment.IsReady` takes an `appsv1.Deployment` and checks
+`*Replicas == ReadyReplicas`, `*Replicas == UpdatedReplicas`,
+`Status.Replicas == ReadyReplicas`, and `Generation == ObservedGeneration`.
+
+Both ensure DeploymentReady is only True when all pods have rolled to
+the new config.
+
+For parent operators that need to check sub-CR readiness directly (e.g.
+to feed into the stability tracker), use `ServiceInstanceIsReady`:
+
+```go
+ready := condition.ServiceInstanceIsReady(
+    subCR.Generation,
+    subCR.Status.ObservedGeneration,
+    subCR.Status.ReadyCount,
+    *subCR.Spec.Replicas,
+    &subCR.Status.Conditions,
+)
+```

modules/common/condition/funcs.go

@@ -166,6 +166,37 @@ 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
+	}
+	if generation != observedGeneration || readyCount != replicas {
+		return false
+	}
+	if replicas == 0 {
+		return true
+	}
+	return conditions.IsTrue(DeploymentReadyCondition)
+}
+
+// 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