fix(crossview): roll the app when its OIDC config changes - #3171
Conversation
The chart injects every OIDC value as env via env[].valueFrom.configMapKeyRef, and env is resolved once at container creation, so a crossview-config change never reaches the running process. Nothing rolled the Deployment on that change, so the pod kept serving the OIDC_CALLBACK_URL it started with while Dex reconciled the new registration - and every login failed with an unregistered redirect_uri. Measured in prod: crossview-config was written 16:48:08Z, the container has run since 16:26:36Z with restarts=0, so it had never read the current config. Annotate the Deployment for Reloader, which is already deployed cluster-wide and is the same convention auth-proxy, oauth2-proxy and homepage use.
✅
|
Blocked on #3175, not on this PR's content.
Holding this as a draft rather than re-running: a re-run re-hits the same environmental failure. This PR is the durable fix, not the unblock. It only takes effect from the next config change |
Correcting my earlier comment on this PR. I said it was blocked on #3175, a CI defect. That was The A HelmRelease is an authorization-capable resource, so the annotation added here moves the aggregate I have not done that measurement yet, so this stays a draft. It is a real piece of work rather than a |
…d annotation The Reloader annotation moves the aggregate authorization-surface fingerprint, so the approved constant needs re-measuring. Measured against main df5bcc3: 534 documents on both sides with identical membership, exactly one moved entry (the crossview HelmRelease), all 72 grant-bearing documents byte-identical, and all 116 aws-bearing lines byte-identical. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Code Coverage OverviewLanguages: Go Go / code-coverage/goThe overall coverage in commit aaacc22 in the Show a code coverage summary of the most covered files.
Updated |
@coderabbitai review |
|
🧠 Learnings used✅ Action performedReview finished.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan includes up to 1 review per rolling hour; 0 remain after this review. 📜 Recent review details🔇 Additional comments (1)
📝 WalkthroughWalkthroughThe Crossview Deployment post-render patch adds a Stakater Reloader annotation for Possibly related PRs
Merge Risk: ⚪ Minimal · up to Crossview will restart when its OIDC configuration changes, keeping the callback behavior aligned with the registered configuration. No actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Readiness — promoting at
|
Parked on a named, live-verified blocker — do not re-enqueueThis PR is pentad-clear (28 checks green, 0 unresolved threads, no conflict, CodeRabbit green at First eviction (run
Second eviction (run
The blocker, measured
The holder identity embeds run id Releasing it is a cluster mutation, which is outside what I may do — handing it over. My error, recordedI re-enqueued at 23:02Z after verifying the node-side residue had cleared: the fenced autoscaler Prod itself is unaffected: both deploys died before Blocker tracked at #3071. I will re-enqueue once the lease is released; re-queuing before that is |
Parked on a named blocker: #3184. Not re-queuing. I enqueued this at the evaluated head
That is the cluster autoscaler moving a node underneath the deploy's safety transaction, not anything Re-queuing without the fix would just re-hit the race, so this waits on #3184. The PR itself is |
Evicted from the merge queue again — root-caused, and it is NOT this PR's fault. Run 31993614033 failed at This is a 4th distinct deploy-lane cause, tracked as #3188: the parent-stability wait is 120s, The pentad here is clean ( |
…ig-reload # Conflicts: # scripts/validate-eks-ci-role-policy/main.go
Merge-queue eviction: root-caused, and it is a known causeThis PR was evicted from the merge queue at ~04:17Z. Per the merge-queue rule, that had to be Run
That is exactly the back-to-back merge-queue deploy shape already filed as #3188, not a fifth What changed hereThe PR had since gone
The merged delta is authorization-neutral: Not re-queued. The eviction cause is understood but #3188 is not yet fixed, so a re-queue is |
The three failing checks are one root cause, not three
Isolated with a matched control (same clone, both sides as linked worktrees, so the environment is identical):
So the surface delta is exactly the crossview annotation, as the constant’s own comment block predicts. The digest CI measuredThe required job reports: Why I am not committing that value in this runThe unresolved-Flux-substitution lines appear in the CI render too, not only locally — so "35 unresolved substitutions" is not purely a missing-local-inputs artifact, and I could not show that the reported digest is over a fully-resolved surface. The constant’s own protocol requires the conservation counts (document totals, membership set-difference in both directions, grant-bearing accounting) behind any approved value, and that measurement has not been done against this merge result. Approving a digest because CI printed it would be exactly the guess the comment block warns against. Next step is the conservation measurement under the approved toolchain, then the constant and its accounting land in one commit. |
…merge result The carried-through placeholder was main's value and never described this merged surface, so the required authorization job rejected it and reported the digest the surface actually renders to. Record that reported digest. States plainly that only the approved CI toolchain stands behind this value: a local render cannot corroborate it while Flux substitutions stay unresolved, so it does not meet the two-independent-renderer bar the earlier reductions met. The job reported the identical digest before and after main was merged in, so main's intervening commit did not move the authorization surface — consistent with its content (documentation plus a policy description annotation, no grant). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Unblocked the authorization gate. Two steps, both at heads recorded here. 1. Updated the branch ( 2. Recorded the digest the required job measured ( That value is now committed. What stands behind it, stated plainly: only the approved CI toolchain. A local render still cannot corroborate it while Flux substitutions are unresolved, so this does not meet the two-independent-renderer bar the earlier reductions in that file met, and the code comment says so rather than implying otherwise. One thing does corroborate the scope of the change. The required job reported the identical digest at Next: the job re-runs at |
All checks green and no conflict with base; this PR is gated only on a current-head review. @coderabbitai review |
|
🧠 Learnings used✅ Action performedReview finished.
|
1 participant
Why
Logging in to Crossview fails at Dex with
Unregistered redirect_uri. Git is not wrong — Dexregisters the public HTTPS callback and Crossview is configured with the matching value. The running
pod is what disagrees.
Crossview's chart injects every OIDC value as environment variables, and environment is resolved once
when the container starts. Nothing rolled the app when its config changed, so it kept serving the
callback URL it happened to boot with while Dex reconciled the new registration. The two halves of
one change landed a few minutes apart and only one of them took effect.
Measured in production: the config was written at 16:48:08Z, and the container has been running since
16:26:36Z with zero restarts — so it had never read it.
What
Annotate the Crossview Deployment for Reloader, which already runs cluster-wide and is the same
mechanism auth-proxy, oauth2-proxy and homepage use. Config changes now roll the app, so a Dex
registration and the callback the app actually sends cannot drift apart again.
This is the durable half. Unblocking the current session still needs one manual restart, because
the fix only takes effect from the next config change onward.
Part of #2741's rollout — that PR restored the public route and moved the callback back to the public
HTTPS origin; this makes the app actually pick such a change up.