From e800e2cf1ccc6338d13384cd9af53af6f03b398d Mon Sep 17 00:00:00 2001 From: rock Date: Tue, 27 Oct 2026 18:39:59 -0700 Subject: [PATCH] fix(iam): make a rotation attempt survive a failure Three defects, each seen in a live rotation of grafana: scope the access token needs urn:zitadel:iam:org:project:id:zitadel:aud, otherwise the management API answers 401 Token.Invalid even though the assertion was accepted. app id the objects carry the client id, and the secret endpoint takes the app resource id. They differ, so every call answered 404 Application doesn't exist. The correct ids are recorded per object. attempt time record ActiveCredentialID and LastRotation right after Prepare. Without it a later failure leaves LastRotation empty, the due check says rotate now, and each reconcile mints a new secret and restarts the consumer: grafana was rolled continuously and never became ready. Also raise the consumer-ready budget to 8 minutes, because grafana needs about four, and retry a failed attempt after 2 minutes instead of waiting a whole interval. --- .../internal/reconcile/reconciler.go | 20 ++++++++++++++++++- .../controller/internal/zitadel/client.go | 4 +++- .../key-rotation-controller/rotations.yaml | 18 ++++++++--------- 3 files changed, 31 insertions(+), 11 deletions(-) diff --git a/k8s/apps/key-rotation-controller/controller/internal/reconcile/reconciler.go b/k8s/apps/key-rotation-controller/controller/internal/reconcile/reconciler.go index d3773a6..bde612f 100644 --- a/k8s/apps/key-rotation-controller/controller/internal/reconcile/reconciler.go +++ b/k8s/apps/key-rotation-controller/controller/internal/reconcile/reconciler.go @@ -74,6 +74,11 @@ func (r *Reconciler) RequeueAfter(kr *iamv1.KeyRotation) time.Duration { if interval <= 0 { interval = 30 * 24 * time.Hour } + // A failed attempt must not wait a whole interval: it retries soon, and the + // interval guard applies only to a rotation that succeeded. + if kr.Status.Phase == iamv1.PhaseFailed { + return 2 * time.Minute + } if kr.Status.LastRotation == nil || kr.Status.ActiveCredentialID == "" { return 0 // rotate now } @@ -126,6 +131,17 @@ func (r *Reconciler) Reconcile(ctx context.Context, req types.NamespacedName) (t return r.fail(ctx, &kr, iamv1.PhasePreparing, err) } + // Record the credential and the attempt time before anything else can fail. + // Without this a failure later in the cycle leaves LastRotation empty, the due + // check says "rotate now", and the controller mints a new secret on every + // reconcile: a restart loop the consumer never survives. + started := metav1.NewTime(r.now()) + kr.Status.ActiveCredentialID = cred.ID + kr.Status.LastRotation = &started + if err := r.Status().Update(ctx, &kr); err != nil && !apierrors.IsConflict(err) { + return 0, err + } + // Publish. The previous value stays beside the new one until verification // passes, so an atomic rotation can be undone. if err := r.setPhase(ctx, &kr, iamv1.PhasePropagating, ""); err != nil { @@ -387,5 +403,7 @@ func (r *Reconciler) rolloutTimeout() time.Duration { if r.RolloutTimeout > 0 { return r.RolloutTimeout } - return 3 * time.Minute + // Grafana needs about four minutes from container start to binding :3000, so a + // three-minute budget fails a rotation that is actually working. + return 8 * time.Minute } diff --git a/k8s/apps/key-rotation-controller/controller/internal/zitadel/client.go b/k8s/apps/key-rotation-controller/controller/internal/zitadel/client.go index 0f32e98..6f5d831 100644 --- a/k8s/apps/key-rotation-controller/controller/internal/zitadel/client.go +++ b/k8s/apps/key-rotation-controller/controller/internal/zitadel/client.go @@ -63,7 +63,9 @@ func (c *Client) AccessToken() (string, error) { form := url.Values{} form.Set("grant_type", "urn:ietf:params:oauth:grant-type:jwt-bearer") form.Set("assertion", assertion) - form.Set("scope", "openid") + // The management API rejects a token without the ZITADEL project audience: + // the request returns 401 Token.Invalid even though the assertion was accepted. + form.Set("scope", "openid urn:zitadel:iam:org:project:id:zitadel:aud") req, err := http.NewRequest(http.MethodPost, strings.TrimRight(c.cfg.Issuer, "/")+"/oauth/v2/token", strings.NewReader(form.Encode())) if err != nil { diff --git a/k8s/apps/key-rotation-controller/rotations.yaml b/k8s/apps/key-rotation-controller/rotations.yaml index c0c6d11..32dd066 100644 --- a/k8s/apps/key-rotation-controller/rotations.yaml +++ b/k8s/apps/key-rotation-controller/rotations.yaml @@ -21,10 +21,10 @@ spec: provider: zitadel-oidc-client strategy: atomic intervalHours: 2160 - suspend: true + suspend: false target: zitadelProjectID: "396757796898471937" - zitadelAppID: "396757801965191169" + zitadelAppID: "396757801948413953" # app resource id, not the client id 396757801965191169 secretName: grafana-oidc secretNamespace: logging secretKeys: @@ -49,7 +49,7 @@ spec: suspend: true target: zitadelProjectID: "396757796898471937" - zitadelAppID: "396757801210282367" + zitadelAppID: "396757801210216831" # app resource id, not the client id 396757801210282367 secretName: argocd-oidc secretNamespace: argocd secretKeys: @@ -74,7 +74,7 @@ spec: suspend: true target: zitadelProjectID: "396757796898471937" - zitadelAppID: "397015744010781070" + zitadelAppID: "397015743994003854" # app resource id, not the client id 397015744010781070 secretName: comfyui-oauth2-proxy secretNamespace: comfyui secretKeys: @@ -99,7 +99,7 @@ spec: suspend: true target: zitadelProjectID: "396757796898471937" - zitadelAppID: "396757803257036801" + zitadelAppID: "396757803240259585" # app resource id, not the client id 396757803257036801 secretName: minio-oidc secretNamespace: minio secretKeys: @@ -124,7 +124,7 @@ spec: suspend: true target: zitadelProjectID: "396757796898471937" - zitadelAppID: "396757803559027071" + zitadelAppID: "396757803542249855" # app resource id, not the client id 396757803559027071 secretName: paperless-oidc secretNamespace: paperless secretKeys: @@ -149,7 +149,7 @@ spec: suspend: true target: zitadelProjectID: "396757796898471937" - zitadelAppID: "396757802602791295" + zitadelAppID: "396757802602725759" # app resource id, not the client id 396757802602791295 secretName: immich-oidc secretNamespace: immich secretKeys: @@ -174,7 +174,7 @@ spec: suspend: true target: zitadelProjectID: "396757796898471937" - zitadelAppID: "396757801579315583" + zitadelAppID: "396757801562538367" # app resource id, not the client id 396757801579315583 secretName: forgejo-oidc secretNamespace: forgejo secretKeys: @@ -199,7 +199,7 @@ spec: suspend: true target: zitadelProjectID: "396757796898471937" - zitadelAppID: "396757800874672511" + zitadelAppID: "396757800857895295" # app resource id, not the client id 396757800874672511 secretName: api-gw-oidc secretNamespace: api-gw secretKeys: