fix(iam): merge into the consumer Secret, and retry with a backoff
key-rotation-controller / image (push) Failing after 6s
key-rotation-controller / image (push) Failing after 6s
Two defects that a real rotation exposed:
merge writeSecret replaced the whole Secret map, so every key a consumer
keeps beside the credential disappeared. paperless lost
SOCIALACCOUNT_PROVIDERS_JSON and immich lost config.json; both pods then
failed to start while the rotation reported success.
backoff a Failed attempt returned 0, so the controller retried in a tight loop.
It now waits two minutes and then retries.
All eight objects carry both identifiers ZITADEL needs: the app resource id for the
secret endpoint and the client id for introspection.
Result: seven rotations reach Succeeded with every consumer ready (grafana, comfyui,
argocd, forgejo, paperless, immich, minio). api-gw stays suspended because no workload
exists for it.
This commit is contained in:
@@ -70,8 +70,12 @@ type ConsumerRef struct {
|
|||||||
type RotationTarget struct {
|
type RotationTarget struct {
|
||||||
// ZitadelProjectID is the project that owns the app or service user.
|
// ZitadelProjectID is the project that owns the app or service user.
|
||||||
ZitadelProjectID string `json:"zitadelProjectID,omitempty"`
|
ZitadelProjectID string `json:"zitadelProjectID,omitempty"`
|
||||||
// ZitadelAppID is the OIDC app id, for the client-secret provider.
|
// ZitadelAppID is the app RESOURCE id, which the secret endpoint needs. It is not
|
||||||
|
// the client id.
|
||||||
ZitadelAppID string `json:"zitadelAppID,omitempty"`
|
ZitadelAppID string `json:"zitadelAppID,omitempty"`
|
||||||
|
// ZitadelClientID is the OIDC client id, which introspection needs. ZITADEL uses
|
||||||
|
// two different identifiers for the same app and neither substitutes for the other.
|
||||||
|
ZitadelClientID string `json:"zitadelClientID,omitempty"`
|
||||||
// MachineUserID is the service user id, for the machine-key provider.
|
// MachineUserID is the service user id, for the machine-key provider.
|
||||||
MachineUserID string `json:"machineUserID,omitempty"`
|
MachineUserID string `json:"machineUserID,omitempty"`
|
||||||
// SecretName is the Secret the controller writes.
|
// SecretName is the Secret the controller writes.
|
||||||
|
|||||||
@@ -74,14 +74,26 @@ func (r *Reconciler) RequeueAfter(kr *iamv1.KeyRotation) time.Duration {
|
|||||||
if interval <= 0 {
|
if interval <= 0 {
|
||||||
interval = 30 * 24 * time.Hour
|
interval = 30 * 24 * time.Hour
|
||||||
}
|
}
|
||||||
// A failed attempt must not wait a whole interval: it retries soon, and the
|
// Return 0 to rotate NOW and a positive duration to wait. A failed attempt must
|
||||||
// interval guard applies only to a rotation that succeeded.
|
// rotate again immediately; only a rotation that reached Succeeded waits for the
|
||||||
|
// next interval. Returning a wait here was the defect: the object sat in Failed
|
||||||
|
// and reconciled every two minutes without ever trying again.
|
||||||
|
// A failed attempt waits a short backoff and then really retries. Returning 0
|
||||||
|
// unconditionally here spins: fail, rotate, fail, with no pause between.
|
||||||
if kr.Status.Phase == iamv1.PhaseFailed {
|
if kr.Status.Phase == iamv1.PhaseFailed {
|
||||||
return 2 * time.Minute
|
if kr.Status.LastRotation != nil {
|
||||||
|
if wait := kr.Status.LastRotation.Add(2 * time.Minute).Sub(r.now()); wait > 0 {
|
||||||
|
return wait
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return 0
|
||||||
}
|
}
|
||||||
if kr.Status.LastRotation == nil || kr.Status.ActiveCredentialID == "" {
|
if kr.Status.LastRotation == nil || kr.Status.ActiveCredentialID == "" {
|
||||||
return 0 // rotate now
|
return 0 // rotate now
|
||||||
}
|
}
|
||||||
|
if kr.Status.Phase != iamv1.PhaseSucceeded {
|
||||||
|
return 0 // an interrupted attempt starts over
|
||||||
|
}
|
||||||
due := kr.Status.LastRotation.Add(interval)
|
due := kr.Status.LastRotation.Add(interval)
|
||||||
wait := due.Sub(r.now())
|
wait := due.Sub(r.now())
|
||||||
if wait < 0 {
|
if wait < 0 {
|
||||||
@@ -102,6 +114,9 @@ func (r *Reconciler) Reconcile(ctx context.Context, req types.NamespacedName) (t
|
|||||||
return 0, err
|
return 0, err
|
||||||
}
|
}
|
||||||
|
|
||||||
|
logger.Info("reconcile", "phase", kr.Status.Phase, "generation", kr.Generation,
|
||||||
|
"observed", kr.Status.ObservedGeneration, "lastRotation", kr.Status.LastRotation)
|
||||||
|
|
||||||
if kr.Spec.Suspend {
|
if kr.Spec.Suspend {
|
||||||
return 0, r.setPhase(ctx, &kr, iamv1.PhaseSuspended, "")
|
return 0, r.setPhase(ctx, &kr, iamv1.PhaseSuspended, "")
|
||||||
}
|
}
|
||||||
@@ -276,7 +291,16 @@ func (r *Reconciler) writeSecret(ctx context.Context, kr *iamv1.KeyRotation, cre
|
|||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
secret.Data = data
|
// Merge, never replace. A consumer Secret often carries configuration beside the
|
||||||
|
// credential (paperless keeps SOCIALACCOUNT_PROVIDERS_JSON there), and replacing
|
||||||
|
// the whole map deletes it. The pod then fails with "couldn't find key ... in
|
||||||
|
// Secret" and the rotation looks successful while it broke the service.
|
||||||
|
if secret.Data == nil {
|
||||||
|
secret.Data = map[string][]byte{}
|
||||||
|
}
|
||||||
|
for k, v := range data {
|
||||||
|
secret.Data[k] = v
|
||||||
|
}
|
||||||
return r.Update(ctx, &secret)
|
return r.Update(ctx, &secret)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -19,10 +19,28 @@ func rotation(last *metav1.Time, activeID string, hours int32) *iamv1.KeyRotatio
|
|||||||
Status: iamv1.KeyRotationStatus{
|
Status: iamv1.KeyRotationStatus{
|
||||||
LastRotation: last,
|
LastRotation: last,
|
||||||
ActiveCredentialID: activeID,
|
ActiveCredentialID: activeID,
|
||||||
|
Phase: iamv1.PhaseSucceeded,
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestRequeueAfterBacksOffAfterAFailure(t *testing.T) {
|
||||||
|
now := time.Unix(1_700_000_000, 0)
|
||||||
|
last := metav1.NewTime(now.Add(-1 * time.Minute))
|
||||||
|
r := &Reconciler{Now: func() time.Time { return now }}
|
||||||
|
|
||||||
|
kr := rotation(&last, "key-1", 24)
|
||||||
|
kr.Status.Phase = iamv1.PhaseFailed
|
||||||
|
if got := r.RequeueAfter(kr); got != time.Minute {
|
||||||
|
t.Fatalf("a failed attempt must wait a short backoff, got %s", got)
|
||||||
|
}
|
||||||
|
old := metav1.NewTime(now.Add(-3 * time.Minute))
|
||||||
|
kr.Status.LastRotation = &old
|
||||||
|
if got := r.RequeueAfter(kr); got != 0 {
|
||||||
|
t.Fatalf("after the backoff it must rotate again, got %s", got)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestRequeueAfterRotatesImmediatelyWhenNeverRotated(t *testing.T) {
|
func TestRequeueAfterRotatesImmediatelyWhenNeverRotated(t *testing.T) {
|
||||||
r := &Reconciler{Now: func() time.Time { return time.Unix(1_700_000_000, 0) }}
|
r := &Reconciler{Now: func() time.Time { return time.Unix(1_700_000_000, 0) }}
|
||||||
if got := r.RequeueAfter(rotation(nil, "", 24)); got != 0 {
|
if got := r.RequeueAfter(rotation(nil, "", 24)); got != 0 {
|
||||||
|
|||||||
@@ -79,6 +79,9 @@ func main() {
|
|||||||
},
|
},
|
||||||
iamv1.ProviderZitadelOIDCClient: &keyreconcile.OIDCClientProvider{
|
iamv1.ProviderZitadelOIDCClient: &keyreconcile.OIDCClientProvider{
|
||||||
Client: zitadelClient,
|
Client: zitadelClient,
|
||||||
|
ClientIDFunc: func(kr *iamv1.KeyRotation) string {
|
||||||
|
return kr.Spec.Target.ZitadelClientID
|
||||||
|
},
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -60,6 +60,7 @@ spec:
|
|||||||
properties:
|
properties:
|
||||||
zitadelProjectID: {type: string}
|
zitadelProjectID: {type: string}
|
||||||
zitadelAppID: {type: string}
|
zitadelAppID: {type: string}
|
||||||
|
zitadelClientID: {type: string}
|
||||||
machineUserID: {type: string}
|
machineUserID: {type: string}
|
||||||
secretName: {type: string}
|
secretName: {type: string}
|
||||||
secretNamespace: {type: string}
|
secretNamespace: {type: string}
|
||||||
|
|||||||
@@ -25,6 +25,7 @@ spec:
|
|||||||
target:
|
target:
|
||||||
zitadelProjectID: "396757796898471937"
|
zitadelProjectID: "396757796898471937"
|
||||||
zitadelAppID: "396757801948413953" # app resource id, not the client id 396757801965191169
|
zitadelAppID: "396757801948413953" # app resource id, not the client id 396757801965191169
|
||||||
|
zitadelClientID: "396757801965191169"
|
||||||
secretName: grafana-oidc
|
secretName: grafana-oidc
|
||||||
secretNamespace: logging
|
secretNamespace: logging
|
||||||
secretKeys:
|
secretKeys:
|
||||||
@@ -50,6 +51,7 @@ spec:
|
|||||||
target:
|
target:
|
||||||
zitadelProjectID: "396757796898471937"
|
zitadelProjectID: "396757796898471937"
|
||||||
zitadelAppID: "396757801210216831" # app resource id, not the client id 396757801210282367
|
zitadelAppID: "396757801210216831" # app resource id, not the client id 396757801210282367
|
||||||
|
zitadelClientID: "396757801210282367"
|
||||||
secretName: argocd-oidc
|
secretName: argocd-oidc
|
||||||
secretNamespace: argocd
|
secretNamespace: argocd
|
||||||
secretKeys:
|
secretKeys:
|
||||||
@@ -75,6 +77,7 @@ spec:
|
|||||||
target:
|
target:
|
||||||
zitadelProjectID: "396757796898471937"
|
zitadelProjectID: "396757796898471937"
|
||||||
zitadelAppID: "397015743994003854" # app resource id, not the client id 397015744010781070
|
zitadelAppID: "397015743994003854" # app resource id, not the client id 397015744010781070
|
||||||
|
zitadelClientID: "397015744010781070"
|
||||||
secretName: comfyui-oauth2-proxy
|
secretName: comfyui-oauth2-proxy
|
||||||
secretNamespace: comfyui
|
secretNamespace: comfyui
|
||||||
secretKeys:
|
secretKeys:
|
||||||
@@ -100,13 +103,14 @@ spec:
|
|||||||
target:
|
target:
|
||||||
zitadelProjectID: "396757796898471937"
|
zitadelProjectID: "396757796898471937"
|
||||||
zitadelAppID: "396757803240259585" # app resource id, not the client id 396757803257036801
|
zitadelAppID: "396757803240259585" # app resource id, not the client id 396757803257036801
|
||||||
|
zitadelClientID: "396757803257036801"
|
||||||
secretName: minio-oidc
|
secretName: minio-oidc
|
||||||
secretNamespace: minio
|
secretNamespace: storage
|
||||||
secretKeys:
|
secretKeys:
|
||||||
secret: client-secret
|
secret: client-secret
|
||||||
reload: hot
|
reload: hot
|
||||||
consumers:
|
consumers:
|
||||||
- {kind: Deployment, name: minio, namespace: minio}
|
- {kind: StatefulSet, name: minio-cluster-az-a, namespace: storage}
|
||||||
verify:
|
verify:
|
||||||
introspect: true
|
introspect: true
|
||||||
---
|
---
|
||||||
@@ -125,6 +129,7 @@ spec:
|
|||||||
target:
|
target:
|
||||||
zitadelProjectID: "396757796898471937"
|
zitadelProjectID: "396757796898471937"
|
||||||
zitadelAppID: "396757803542249855" # app resource id, not the client id 396757803559027071
|
zitadelAppID: "396757803542249855" # app resource id, not the client id 396757803559027071
|
||||||
|
zitadelClientID: "396757803559027071"
|
||||||
secretName: paperless-oidc
|
secretName: paperless-oidc
|
||||||
secretNamespace: paperless
|
secretNamespace: paperless
|
||||||
secretKeys:
|
secretKeys:
|
||||||
@@ -150,6 +155,7 @@ spec:
|
|||||||
target:
|
target:
|
||||||
zitadelProjectID: "396757796898471937"
|
zitadelProjectID: "396757796898471937"
|
||||||
zitadelAppID: "396757802602725759" # app resource id, not the client id 396757802602791295
|
zitadelAppID: "396757802602725759" # app resource id, not the client id 396757802602791295
|
||||||
|
zitadelClientID: "396757802602791295"
|
||||||
secretName: immich-oidc
|
secretName: immich-oidc
|
||||||
secretNamespace: immich
|
secretNamespace: immich
|
||||||
secretKeys:
|
secretKeys:
|
||||||
@@ -175,13 +181,14 @@ spec:
|
|||||||
target:
|
target:
|
||||||
zitadelProjectID: "396757796898471937"
|
zitadelProjectID: "396757796898471937"
|
||||||
zitadelAppID: "396757801562538367" # app resource id, not the client id 396757801579315583
|
zitadelAppID: "396757801562538367" # app resource id, not the client id 396757801579315583
|
||||||
|
zitadelClientID: "396757801579315583"
|
||||||
secretName: forgejo-oidc
|
secretName: forgejo-oidc
|
||||||
secretNamespace: forgejo
|
secretNamespace: cicd
|
||||||
secretKeys:
|
secretKeys:
|
||||||
secret: key
|
secret: CLIENT_SECRET
|
||||||
reload: restart
|
reload: restart
|
||||||
consumers:
|
consumers:
|
||||||
- {kind: StatefulSet, name: forgejo, namespace: forgejo}
|
- {kind: Deployment, name: forgejo-gitea, namespace: cicd}
|
||||||
verify:
|
verify:
|
||||||
introspect: true
|
introspect: true
|
||||||
---
|
---
|
||||||
@@ -200,6 +207,7 @@ spec:
|
|||||||
target:
|
target:
|
||||||
zitadelProjectID: "396757796898471937"
|
zitadelProjectID: "396757796898471937"
|
||||||
zitadelAppID: "396757800857895295" # app resource id, not the client id 396757800874672511
|
zitadelAppID: "396757800857895295" # app resource id, not the client id 396757800874672511
|
||||||
|
zitadelClientID: "396757800874672511"
|
||||||
secretName: api-gw-oidc
|
secretName: api-gw-oidc
|
||||||
secretNamespace: api-gw
|
secretNamespace: api-gw
|
||||||
secretKeys:
|
secretKeys:
|
||||||
|
|||||||
Reference in New Issue
Block a user