Skip to content

Commit eb31dc5

Browse files
lmicciniclaude
andcommitted
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>
1 parent fe8e60d commit eb31dc5

9 files changed

Lines changed: 1040 additions & 12 deletions

File tree

Lines changed: 189 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,189 @@
1+
# Credential rotation consumer finalizer guard
2+
3+
OpenStack service operators that consume rotating credential secrets
4+
(transport URL, application credentials, or similar) attach a consumer
5+
finalizer to the old secret so it is not deleted until every sub-service
6+
has rolled out with the new credential.
7+
8+
## Shared helpers
9+
10+
Use helpers from these packages instead of duplicating guard logic in each
11+
operator:
12+
13+
- `object.ManageSecretConsumerFinalizer` — add finalizer to current secret
14+
- `object.RemoveSecretConsumerFinalizer` — remove finalizer from old secret
15+
- `object.FinalizeSecretRotation` — rotation guard: hold old finalizer
16+
until all sub-services are ready, then release
17+
- `object.ManageRotationGracePeriod` — time-based grace period that gives
18+
sub-CRs time to detect config changes, roll pods, and update conditions
19+
before the guard evaluates readiness
20+
- `condition.CredentialRotationGuardReady` — compute `guardReady` for the
21+
rotation guard
22+
- `condition.ServiceInstanceIsReady` — check whether a sub-CR has
23+
finished reconciling (generation/observedGeneration guard, replica
24+
counts, and DeploymentReadyCondition)
25+
- `statefulset.IsReady` — check `*Replicas == ReadyReplicas`,
26+
`*Replicas == UpdatedReplicas`, `Generation == ObservedGeneration`, and
27+
`CurrentRevision == UpdateRevision` so DeploymentReady is only True when
28+
all pods have rolled to the new config
29+
- `deployment.IsReady` — check `*Replicas == ReadyReplicas`,
30+
`*Replicas == UpdatedReplicas`, `Status.Replicas == ReadyReplicas`, and
31+
`Generation == ObservedGeneration`
32+
33+
## Parent controller integration
34+
35+
### 1. Pass transport URL directly — never through Status
36+
37+
Pass `transportURL.Status.SecretName` as a parameter to sub-CR creation
38+
functions and config generation. Never read from
39+
`instance.Status.TransportURLSecret` when building sub-CR specs — the
40+
status field is only used as the "old" value for `FinalizeSecretRotation`.
41+
42+
Set `instance.Status.TransportURLSecret` early only for first-time setup:
43+
44+
```go
45+
if instance.Status.TransportURLSecret == "" ||
46+
instance.Status.TransportURLSecret == transportURL.Status.SecretName {
47+
instance.Status.TransportURLSecret = transportURL.Status.SecretName
48+
}
49+
```
50+
51+
During rotation (old != current), the status is updated solely by
52+
`FinalizeSecretRotation` at the end of reconcile.
53+
54+
### 2. Track stability
55+
56+
Use a simple boolean to track whether all sub-CRs are stable:
57+
58+
```go
59+
allSubCRsStable := true
60+
61+
// After each sub-CR CreateOrPatch:
62+
subCR, op, err := r.subCRCreateOrUpdate(ctx, instance, transportURL.Status.SecretName)
63+
if err != nil { return ctrl.Result{}, err }
64+
if op != controllerutil.OperationResultNone {
65+
allSubCRsStable = false
66+
}
67+
```
68+
69+
### 3. Transport URL annotation (operators without TransportURLSecret in sub-CR spec)
70+
71+
For operators whose sub-CR specs do not include a `TransportURLSecret`
72+
field (e.g. nova, watcher, glance, designate, octavia, telemetry), add
73+
an annotation inside the `CreateOrPatch` mutate function to force a
74+
spec change when the transport URL rotates:
75+
76+
```go
77+
op, err := controllerutil.CreateOrPatch(ctx, r.Client, subCR, func() error {
78+
subCR.Spec = spec
79+
if subCR.Annotations == nil {
80+
subCR.Annotations = map[string]string{}
81+
}
82+
subCR.Annotations["openstack.org/transport-url-secret"] = transportURLSecretName
83+
return controllerutil.SetControllerReference(instance, subCR, r.Scheme)
84+
})
85+
```
86+
87+
This is not needed for operators that pass `TransportURLSecret` directly
88+
in the sub-CR spec (cinder, manila, ironic, barbican, heat) — the spec
89+
field change already makes `CreateOrPatch` return `"updated"`.
90+
91+
### 4. Compute the rotation guard and finalize
92+
93+
```go
94+
guardReady := condition.CredentialRotationGuardReady(
95+
allSubCRsStable,
96+
&instance.Status.Conditions,
97+
)
98+
99+
instance.Status.TransportURLSecret, err = object.FinalizeSecretRotation(
100+
ctx, helper, instance.Namespace,
101+
instance.Status.TransportURLSecret,
102+
transportURL.Status.SecretName,
103+
myTransportConsumerFinalizer,
104+
guardReady,
105+
)
106+
```
107+
108+
The same pattern works for application credential secrets:
109+
110+
```go
111+
instance.Status.ApplicationCredentialSecret, err = object.FinalizeSecretRotation(
112+
ctx, helper, instance.Namespace,
113+
instance.Status.ApplicationCredentialSecret,
114+
instance.Spec.Auth.ApplicationCredentialSecret,
115+
myACConsumerFinalizer,
116+
guardReady,
117+
)
118+
```
119+
120+
### 5. Rotation grace period (optional)
121+
122+
Use `ManageRotationGracePeriod` to give sub-CRs time to detect config
123+
changes, update their Deployments/StatefulSets, and roll pods before the
124+
guard releases the old secret's consumer finalizer:
125+
126+
```go
127+
rotationPending := instance.Status.TransportURLSecret != "" &&
128+
instance.Status.TransportURLSecret != transportURL.Status.SecretName
129+
130+
result, graceActive, err := object.ManageRotationGracePeriod(
131+
ctx, r.Client, instance,
132+
rotationPending,
133+
30*time.Second,
134+
)
135+
if err != nil { return ctrl.Result{}, err }
136+
if graceActive {
137+
return result, nil
138+
}
139+
```
140+
141+
When `rotationPending` is true and no grace period is active, it sets the
142+
`openstack.org/rotation-grace-until` annotation and returns a requeue.
143+
While the grace period is active, it continues to requeue. After the
144+
grace period expires, it returns `graceActive == false` so the caller
145+
can proceed to evaluate the rotation guard. When `rotationPending` is
146+
false, it clears any existing annotation.
147+
148+
## Sub-CR integration
149+
150+
Sub-CR controllers should use `statefulset.IsReady()` or
151+
`deployment.IsReady()` for the `DeploymentReadyCondition` check:
152+
153+
```go
154+
if statefulset.IsReady(sts) {
155+
instance.Status.Conditions.MarkTrue(
156+
condition.DeploymentReadyCondition,
157+
condition.DeploymentReadyMessage)
158+
} else if *instance.Spec.Replicas > 0 {
159+
instance.Status.Conditions.Set(condition.FalseCondition(
160+
condition.DeploymentReadyCondition,
161+
condition.RequestedReason,
162+
condition.SeverityInfo,
163+
condition.DeploymentReadyRunningMessage))
164+
}
165+
```
166+
167+
`statefulset.IsReady` takes an `appsv1.StatefulSet` and checks
168+
`*Replicas == ReadyReplicas`, `*Replicas == UpdatedReplicas`,
169+
`Generation == ObservedGeneration`, and `CurrentRevision == UpdateRevision`.
170+
171+
`deployment.IsReady` takes an `appsv1.Deployment` and checks
172+
`*Replicas == ReadyReplicas`, `*Replicas == UpdatedReplicas`,
173+
`Status.Replicas == ReadyReplicas`, and `Generation == ObservedGeneration`.
174+
175+
Both ensure DeploymentReady is only True when all pods have rolled to
176+
the new config.
177+
178+
For parent operators that need to check sub-CR readiness directly (e.g.
179+
to feed into the stability tracker), use `ServiceInstanceIsReady`:
180+
181+
```go
182+
ready := condition.ServiceInstanceIsReady(
183+
subCR.Generation,
184+
subCR.Status.ObservedGeneration,
185+
subCR.Status.ReadyCount,
186+
*subCR.Spec.Replicas,
187+
&subCR.Status.Conditions,
188+
)
189+
```

modules/common/condition/funcs.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,34 @@ func (conditions *Conditions) IsUnknown(t Type) bool {
166166
return true
167167
}
168168

169+
// ServiceInstanceIsReady reports whether a service sub-CR has finished
170+
// reconciling its current spec and its deployment is ready to serve
171+
// requests. It implements the generation/observedGeneration guard used
172+
// by OpenStack service operators during credential rotation rollouts.
173+
func ServiceInstanceIsReady(generation, observedGeneration int64, readyCount, replicas int32, conditions *Conditions) bool {
174+
if conditions == nil {
175+
return false
176+
}
177+
return generation == observedGeneration &&
178+
readyCount == replicas &&
179+
(conditions.IsTrue(DeploymentReadyCondition) ||
180+
(conditions.IsFalse(DeploymentReadyCondition) && replicas == 0))
181+
}
182+
183+
// CredentialRotationGuardReady reports whether it is safe to remove a
184+
// consumer finalizer from an old transport URL or application credential
185+
// secret. The guard requires all sub-CR specs to be stable (no pending
186+
// CreateOrPatch updates) and all parent sub-conditions to be True.
187+
// Note: ReadyCondition is NOT checked because operators reset conditions
188+
// via Init() at the top of each Reconcile, so ReadyCondition is Unknown
189+
// until the very end. AllSubConditionIsTrue covers the same safety.
190+
func CredentialRotationGuardReady(allSubCRsStable bool, conditions *Conditions) bool {
191+
if conditions == nil {
192+
return false
193+
}
194+
return allSubCRsStable && conditions.AllSubConditionIsTrue()
195+
}
196+
169197
// AllSubConditionIsTrue validates if all subconditions are True
170198
// It assumes that all conditions report success via the True status
171199
func (conditions *Conditions) AllSubConditionIsTrue() bool {

modules/common/condition/funcs_test.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -351,6 +351,41 @@ func TestIsMethods(t *testing.T) {
351351
g.Expect(conditions.IsUnknown("unknownB")).To(BeTrue())
352352
}
353353

354+
func TestServiceInstanceIsReady(t *testing.T) {
355+
g := NewWithT(t)
356+
357+
readyConditions := CreateList(
358+
TrueCondition(DeploymentReadyCondition, "deployment ready"),
359+
)
360+
361+
notReadyConditions := CreateList(
362+
FalseCondition(DeploymentReadyCondition, InitReason, SeverityInfo, "creating"),
363+
)
364+
365+
g.Expect(ServiceInstanceIsReady(2, 2, 3, 3, &readyConditions)).To(BeTrue())
366+
g.Expect(ServiceInstanceIsReady(2, 1, 3, 3, &readyConditions)).To(BeFalse())
367+
g.Expect(ServiceInstanceIsReady(2, 2, 2, 3, &readyConditions)).To(BeFalse())
368+
g.Expect(ServiceInstanceIsReady(2, 2, 3, 3, &notReadyConditions)).To(BeFalse())
369+
g.Expect(ServiceInstanceIsReady(1, 1, 0, 0, &notReadyConditions)).To(BeTrue())
370+
g.Expect(ServiceInstanceIsReady(1, 1, 0, 0, nil)).To(BeFalse())
371+
}
372+
373+
func TestCredentialRotationGuardReady(t *testing.T) {
374+
g := NewWithT(t)
375+
376+
conditions := Conditions{}
377+
conditions.Init(nil)
378+
conditions.MarkTrue("a", "ready")
379+
conditions.MarkTrue("b", "ready")
380+
381+
g.Expect(CredentialRotationGuardReady(true, &conditions)).To(BeTrue())
382+
conditions.MarkFalse("a", InitReason, SeverityInfo, "not ready")
383+
g.Expect(CredentialRotationGuardReady(true, &conditions)).To(BeFalse())
384+
conditions.MarkTrue("a", "ready")
385+
g.Expect(CredentialRotationGuardReady(false, &conditions)).To(BeFalse())
386+
g.Expect(CredentialRotationGuardReady(true, nil)).To(BeFalse())
387+
}
388+
354389
func TestAllSubConditionIsTrue(t *testing.T) {
355390
conditions := Conditions{}
356391

modules/common/deployment/deployment.go

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,10 +87,20 @@ func (d *Deployment) CreateOrPatch(
8787
h.GetLogger().Info(fmt.Sprintf("Deployment %s - %s", deployment.Name, op))
8888
}
8989

90-
// update the deployment object of the deployment type
91-
d.deployment, err = GetDeploymentWithName(ctx, h, deployment.GetName(), deployment.GetNamespace())
92-
if err != nil {
93-
return ctrl.Result{}, err
90+
if op == controllerutil.OperationResultNone {
91+
// Re-read from cache to pick up status updates from the
92+
// Deployment controller (e.g. updated ReadyReplicas).
93+
d.deployment, err = GetDeploymentWithName(ctx, h, deployment.GetName(), deployment.GetNamespace())
94+
if err != nil {
95+
return ctrl.Result{}, err
96+
}
97+
} else {
98+
// After a create/update the informer cache may still hold the
99+
// previous version where Generation == ObservedGeneration. Using
100+
// the server-returned object preserves the correct (bumped)
101+
// Generation so that callers' readiness checks do not pass on
102+
// stale data.
103+
d.deployment = deployment
94104
}
95105

96106
return ctrl.Result{}, nil

0 commit comments

Comments
 (0)