Skip to content

Commit a7dc750

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 - 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 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent fe8e60d commit a7dc750

8 files changed

Lines changed: 490 additions & 14 deletions

File tree

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

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

modules/common/helper/helper.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,7 @@ type Helper struct {
4747
after *unstructured.Unstructured
4848
changes map[string]bool
4949
finalizer string
50-
51-
logger logr.Logger
50+
logger logr.Logger
5251
}
5352

5453
// NewHelper returns an initialized Helper.

0 commit comments

Comments
 (0)