diff --git a/k8s/apps/key-rotation-controller/controller/api/types.go b/k8s/apps/key-rotation-controller/controller/api/types.go index aabbe8a..887a844 100644 --- a/k8s/apps/key-rotation-controller/controller/api/types.go +++ b/k8s/apps/key-rotation-controller/controller/api/types.go @@ -70,8 +70,12 @@ type ConsumerRef struct { type RotationTarget struct { // ZitadelProjectID is the project that owns the app or service user. 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"` + // 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 string `json:"machineUserID,omitempty"` // SecretName is the Secret the controller writes. 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 bde612f..0af16df 100644 --- a/k8s/apps/key-rotation-controller/controller/internal/reconcile/reconciler.go +++ b/k8s/apps/key-rotation-controller/controller/internal/reconcile/reconciler.go @@ -74,14 +74,26 @@ 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. + // Return 0 to rotate NOW and a positive duration to wait. A failed attempt must + // 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 { - 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 == "" { return 0 // rotate now } + if kr.Status.Phase != iamv1.PhaseSucceeded { + return 0 // an interrupted attempt starts over + } due := kr.Status.LastRotation.Add(interval) wait := due.Sub(r.now()) if wait < 0 { @@ -102,6 +114,9 @@ func (r *Reconciler) Reconcile(ctx context.Context, req types.NamespacedName) (t 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 { 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 { 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) } diff --git a/k8s/apps/key-rotation-controller/controller/internal/reconcile/reconciler_test.go b/k8s/apps/key-rotation-controller/controller/internal/reconcile/reconciler_test.go index 5a51e4d..955add5 100644 --- a/k8s/apps/key-rotation-controller/controller/internal/reconcile/reconciler_test.go +++ b/k8s/apps/key-rotation-controller/controller/internal/reconcile/reconciler_test.go @@ -19,10 +19,28 @@ func rotation(last *metav1.Time, activeID string, hours int32) *iamv1.KeyRotatio Status: iamv1.KeyRotationStatus{ LastRotation: last, 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) { r := &Reconciler{Now: func() time.Time { return time.Unix(1_700_000_000, 0) }} if got := r.RequeueAfter(rotation(nil, "", 24)); got != 0 { diff --git a/k8s/apps/key-rotation-controller/controller/main.go b/k8s/apps/key-rotation-controller/controller/main.go index 7192fef..f62bd35 100644 --- a/k8s/apps/key-rotation-controller/controller/main.go +++ b/k8s/apps/key-rotation-controller/controller/main.go @@ -79,6 +79,9 @@ func main() { }, iamv1.ProviderZitadelOIDCClient: &keyreconcile.OIDCClientProvider{ Client: zitadelClient, + ClientIDFunc: func(kr *iamv1.KeyRotation) string { + return kr.Spec.Target.ZitadelClientID + }, }, }, } diff --git a/k8s/apps/key-rotation-controller/crd.yaml b/k8s/apps/key-rotation-controller/crd.yaml index d36b07a..52f34af 100644 --- a/k8s/apps/key-rotation-controller/crd.yaml +++ b/k8s/apps/key-rotation-controller/crd.yaml @@ -60,6 +60,7 @@ spec: properties: zitadelProjectID: {type: string} zitadelAppID: {type: string} + zitadelClientID: {type: string} machineUserID: {type: string} secretName: {type: string} secretNamespace: {type: string} diff --git a/k8s/apps/key-rotation-controller/rotations.yaml b/k8s/apps/key-rotation-controller/rotations.yaml index 32dd066..558ddab 100644 --- a/k8s/apps/key-rotation-controller/rotations.yaml +++ b/k8s/apps/key-rotation-controller/rotations.yaml @@ -25,6 +25,7 @@ spec: target: zitadelProjectID: "396757796898471937" zitadelAppID: "396757801948413953" # app resource id, not the client id 396757801965191169 + zitadelClientID: "396757801965191169" secretName: grafana-oidc secretNamespace: logging secretKeys: @@ -50,6 +51,7 @@ spec: target: zitadelProjectID: "396757796898471937" zitadelAppID: "396757801210216831" # app resource id, not the client id 396757801210282367 + zitadelClientID: "396757801210282367" secretName: argocd-oidc secretNamespace: argocd secretKeys: @@ -75,6 +77,7 @@ spec: target: zitadelProjectID: "396757796898471937" zitadelAppID: "397015743994003854" # app resource id, not the client id 397015744010781070 + zitadelClientID: "397015744010781070" secretName: comfyui-oauth2-proxy secretNamespace: comfyui secretKeys: @@ -100,13 +103,14 @@ spec: target: zitadelProjectID: "396757796898471937" zitadelAppID: "396757803240259585" # app resource id, not the client id 396757803257036801 + zitadelClientID: "396757803257036801" secretName: minio-oidc - secretNamespace: minio + secretNamespace: storage secretKeys: secret: client-secret reload: hot consumers: - - {kind: Deployment, name: minio, namespace: minio} + - {kind: StatefulSet, name: minio-cluster-az-a, namespace: storage} verify: introspect: true --- @@ -125,6 +129,7 @@ spec: target: zitadelProjectID: "396757796898471937" zitadelAppID: "396757803542249855" # app resource id, not the client id 396757803559027071 + zitadelClientID: "396757803559027071" secretName: paperless-oidc secretNamespace: paperless secretKeys: @@ -150,6 +155,7 @@ spec: target: zitadelProjectID: "396757796898471937" zitadelAppID: "396757802602725759" # app resource id, not the client id 396757802602791295 + zitadelClientID: "396757802602791295" secretName: immich-oidc secretNamespace: immich secretKeys: @@ -175,13 +181,14 @@ spec: target: zitadelProjectID: "396757796898471937" zitadelAppID: "396757801562538367" # app resource id, not the client id 396757801579315583 + zitadelClientID: "396757801579315583" secretName: forgejo-oidc - secretNamespace: forgejo + secretNamespace: cicd secretKeys: - secret: key + secret: CLIENT_SECRET reload: restart consumers: - - {kind: StatefulSet, name: forgejo, namespace: forgejo} + - {kind: Deployment, name: forgejo-gitea, namespace: cicd} verify: introspect: true --- @@ -200,6 +207,7 @@ spec: target: zitadelProjectID: "396757796898471937" zitadelAppID: "396757800857895295" # app resource id, not the client id 396757800874672511 + zitadelClientID: "396757800874672511" secretName: api-gw-oidc secretNamespace: api-gw secretKeys: