Skip to content

fix(ghcr): recognise KSail's form of the same root credential - #5681

Merged
devantler merged 1 commit into
mainfrom
claude/ghcr-root-auth-form-5650
Nov 2, 2026
Merged

devantler merged 1 commit into
mainfrom
claude/ghcr-root-auth-form-5650

Conversation

@devantler

Copy link
Copy Markdown
Contributor

🤖 Generated by the Agentic Engineer

Why

Every production deploy runs the full credential transaction twice, about 3 to 4 minutes each, and the platform queue waits on it. In all six deploys read today, the second run stopped at the root credential check. The cluster update had just rewritten that same credential in KSail's own format, and the check only accepts the Git format.

What

The read-only path now also accepts exactly KSail's format of the Git credential. A different token, username or extra entry, in either format, still runs the full transaction. The second run may still stop at a later check, because the update also re-renders node markers, so the next deploys are read to measure it before the issue closes.

Part of #5650

cluster update rewrites the root Flux Secret with KSail's username, password and
auth fields, from the same Git/SOPS credential. The byte-exact check could never
match after it, so the second credential pass of every deploy ran the full fenced
transaction. Accept exactly that form, derived from the Git/SOPS credential; any
other payload still takes the fenced path.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor
Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 54 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration
Commits

Reviewing files that changed from the base of the PR and between 1dad8e0 and f425817.

Files selected for processing: 2
  • scripts/refresh-flux-ghcr-auth.sh
  • scripts/tests/refresh-flux-ghcr-auth/lazy_fence_test.go

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.

Share

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-01T13:51:02.185761Z f425817 Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Chef's kiss.

Reviewed commit: f425817363

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

Evaluation at f425817.

Exercised: the real script against the fake cluster: a root Secret in KSail's form of the same credential now takes the read-only path, with no controller restart. Before the change that same test failed. Six other payloads, including another token, another username, a disagreeing auth field and an extra registry, all end at check=root-auth and run the fenced path.

Format check: KSail's own buildDockerConfigJSON, copied from its main branch and run on throwaway values including characters its JSON encoder escapes, normalises to exactly the payload this script expects, 3 of 3. A different password does not match.

Not exercised: a production deploy. No live secret was read, so that the live payload is exactly this form is still inferred from the code. If it is not, the check keeps reporting root-auth and the full transaction runs as it does today. The next deploys after merge settle it, recorded on #5650.

Effect on production if it holds: after cluster update the root Secret stays in KSail's form rather than being written back. Later checks in the chain still run, and a read-only pass still needs all of them.

@devantler
devantler marked this pull request as ready for review November 1, 2026 13:52
@devantler
devantler added this pull request to the merge queue Nov 1, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Nov 1, 2026
@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

The merge queue removed this pull request at 17:46 UTC with failed_checks. This change did not cause it, so the fix stands as it is and only needs queueing again.

Run 41331622603 failed in the deploy because the policy engine's webhook timed out while infrastructure reconciled (validate-policy.kyverno.svc, context deadline exceeded on ClusterPolicy/restrict-arc-retirement). The same failure stopped the #5679 queue run at about 15:28 UTC. This pull request only changes the GHCR refresh script and its test, and #5692 tracks the timeout. The repair deploy succeeded at 18:08 UTC and live health reads OK.

This run did not queue it again: its one merge attempt, on #5422, was refused (sample on monorepo#3902), and it stopped merging there. Pass 2 of #5650 is still unread until this lands.

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

Queue re-entry checked at f42581736322929cbd812db304486e5c3293d9dc. The native failed deploy log confirms the policy-webhook timeout recorded on #5692. The restore job succeeded, including its explicit check for resources left by the failed deployment, and the current platform health check passes.

The source is unchanged, all reported checks pass, the exact-head Codex review is clean, and a fresh complete thread read is zero. No current claim or live local holder was found. Continuing with one normal head-pinned queue attempt. The recurring timeout remains open under #5692; pass-2 behavior and timing remain due on #5650 after merge.

@devantler
devantler added this pull request to the merge queue Nov 1, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Nov 1, 2026
@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

Removed from the queue again at about 21:06 UTC, for the same unrelated reason. Not re-queued. In merge-queue run 41347954355, 🚀 Deploy to Prod failed while Flux reported the policy webhook deadline on ClusterPolicy/add-security-context at 21:03:59. I read that from the live Flux condition; the job log stays empty until Heal Prod ends, so I have not read the log itself.

This pull request does not cause it: the same message also removed #5687, which changes one allow-list file. Details are on #5692.

Re-entry: wait until #5693 has deployed, then re-check the head (f42581736) and re-queue with a head-pinned merge. Putting it back sooner would likely hit the same failure.

@devantler
devantler added this pull request to the merge queue Nov 1, 2026
@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

Re-entered the queue at 22:06 UTC, head f425817, behind #5693 so its deploy follows the Kyverno fix. The earlier note said to wait for #5693 to deploy. Position in the queue gives the same order without the wait. The head is unchanged and Codex is clean at it. If #5693 is dropped again (see #5703), this PR no longer has the fix ahead of it: read the run before any further re-queue.

Merged via the queue into main with commit 187760c Nov 2, 2026
54 checks passed
@devantler
devantler deleted the claude/ghcr-root-auth-form-5650 branch November 2, 2026 00:52
@github-project-automation github-project-automation Bot moved this from 🫴 Ready to ✅ Done in 🌊 Project Board Nov 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

1 participant