fix(deploy): synchronize production GHCR pull auth#2625
Conversation
@coderabbitai review |
|
✅ Action performedReview finished.
|
📝 WalkthroughWalkthroughThe change adds a Git/SOPS-based GHCR credential bridge that validates registry access, updates Flux and consumer secrets, synchronizes Talos registry authentication, and supports incomplete bootstrap states. Production and DR workflows now stage, verify, and reassert credentials around publishing, reconciliation, cluster updates, and restoration. KSail lifecycle commands use the pull-auth wrapper, while CI tests and documentation cover the new sequencing. Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
@codex review for secret handling, fail-closed behavior, and GitHub Actions ordering |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1c5922e573
ℹ️ 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".
@codex review for secret handling, pre-publish fail-closed ordering, and shared package authorization |
Review Summary
Testing / Checks
|
@codex review for GHCR auth protocol correctness, secret safety, fail-closed behavior, and deploy/DR ordering |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 06c5e81e4f
ℹ️ 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".
Current head |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4e96c32e84
ℹ️ 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".
@coderabbitai review |
|
✅ Action performedReview finished.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b70d68842d
ℹ️ 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".
@codex review for GHCR transaction ordering, failure atomicity, and fresh-DR behavior |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9dd1eec0eb
ℹ️ 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".
@coderabbitai review |
|
🤖 Codex Daily AI Engineer @codex review for GHCR credential bridge safety, production deployment ordering, and DR recovery ordering |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 677a958b8e
ℹ️ 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
changed the title
@codex review for production GHCR auth correctness, secret handling, Talos node rotation, and DR ordering |
@codex review for production GHCR auth correctness, partial-fan-out atomicity, Talos node rotation, secret handling, and DR ordering |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
@coderabbitai review Fallback reason: the exact-head Codex request produced no review artifact or thread after more than 16 minutes, beyond the established response window on this PR. |
|
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/ci.yaml (1)
44-50: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winTrigger validation when the tested DR runbook changes.
scripts/tests/test_refresh_flux_ghcr_auth.pylines 1331–1354 validate ordering indocs/dr/runbook.md, but this filter omits that file. A runbook-only regression therefore bypasses the test.Proposed fix
- 'scripts/tests/test_refresh_flux_ghcr_auth.py' + - 'docs/dr/runbook.md' - '.github/actions/deploy-prod/**'As per coding guidelines, “validate before every pull request.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yaml around lines 44 - 50, Update the path filter for the relevant CI test entry to include docs/dr/runbook.md alongside the existing scripts/tests/test_refresh_flux_ghcr_auth.py and DR workflow paths, so changes to the tested runbook trigger validation.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/dr-rebuild.yaml:
- Around line 411-417: Update the reassert_flux_ghcr_after_restore step
condition so it runs when inputs.restore is true even if an earlier restore step
failed, while still skipping execution when the workflow is cancelled. Use an
explicit status check such as !cancelled() alongside inputs.restore.
In `@docs/dr/runbook.md`:
- Around line 51-57: Update the manual production lifecycle prerequisite in the
runbook to explicitly state that workload reconcile requires a usable SOPS Age
key, because the wrapper decrypts pull credentials for non-push commands. Keep
the existing wrapper-only command guidance and credential requirements intact.
In `@scripts/refresh-flux-ghcr-auth.sh`:
- Around line 507-508: Update the explicit bootstrap flow around
patch_variables_base and patch_root_secret to patch the root secret before
staging fan-out, then reassert patch_root_secret after patch_variables_base
completes. Preserve the existing failure handling while ensuring root Flux
authentication is repaired both before and after fan-out staging.
In `@scripts/tests/test_refresh_flux_ghcr_auth.py`:
- Around line 155-158: Update the failure messages in the fake Talos auth and
pull operation branches to omit $EXPECTED_PULL_TOKEN, so neither branch exposes
the fixture token while preserving the existing failure behavior and exit code.
---
Outside diff comments:
In @.github/workflows/ci.yaml:
- Around line 44-50: Update the path filter for the relevant CI test entry to
include docs/dr/runbook.md alongside the existing
scripts/tests/test_refresh_flux_ghcr_auth.py and DR workflow paths, so changes
to the tested runbook trigger validation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f3cbf3ca-a4fe-4ff2-926b-eff4c44b6518
📒 Files selected for processing (13)
.github/actions/deploy-prod/action.yml.github/workflows/ci.yaml.github/workflows/dr-rebuild.yamlAGENTS.mddocs/dr/crypto-custody.mddocs/dr/runbook.mdksail.prod.yamlscripts/ghcr-auth-lib.shscripts/refresh-flux-ghcr-auth.shscripts/run-ksail-prod-with-pull-auth.shscripts/tests/test_refresh_flux_ghcr_auth.pytalos/cluster/authenticate-ghcr-pulls.yamltalos/cluster/mark-ghcr-pull-revision.yaml
📜 Review details
🧰 Additional context used
📓 Path-based instructions (6)
**/*.yaml
📄 CodeRabbit inference engine (AGENTS.md)
Use one Kubernetes resource per YAML file, except explicitly whitelisted vendored operator bundles.
Files:
talos/cluster/mark-ghcr-pull-revision.yamlksail.prod.yamltalos/cluster/authenticate-ghcr-pulls.yaml
talos{,-local}/**/*.yaml
📄 CodeRabbit inference engine (AGENTS.md)
Talos machine-config patches must contain one YAML document per file and use intent-describing
verb-purpose.yamlnames.
Files:
talos/cluster/mark-ghcr-pull-revision.yamltalos/cluster/authenticate-ghcr-pulls.yaml
**/*.{yaml,yml}
📄 CodeRabbit inference engine (AGENTS.md)
Use static validation only for maintenance; never start or mutate a cluster for routine validation.
Files:
talos/cluster/mark-ghcr-pull-revision.yamlksail.prod.yamltalos/cluster/authenticate-ghcr-pulls.yaml
talos/**/*.yaml
📄 CodeRabbit inference engine (AGENTS.md)
Validate Talos changes with the repository's
validate-taloschecks, including rendering all patches and runningtalosctl validate.
Files:
talos/cluster/mark-ghcr-pull-revision.yamltalos/cluster/authenticate-ghcr-pulls.yaml
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Use Conventional Commit messages and create pull requests as drafts; keep changes small and focused.
Never push directly tomain; validate before every pull request and never merge external pull requests or unreviewed drafts.
Files:
talos/cluster/mark-ghcr-pull-revision.yamlksail.prod.yamldocs/dr/crypto-custody.mdtalos/cluster/authenticate-ghcr-pulls.yamlscripts/ghcr-auth-lib.shscripts/run-ksail-prod-with-pull-auth.shdocs/dr/runbook.mdAGENTS.mdscripts/refresh-flux-ghcr-auth.shscripts/tests/test_refresh_flux_ghcr_auth.py
ksail*.yaml
📄 CodeRabbit inference engine (AGENTS.md)
Do not modify
ksail.prod.yaml; production cluster configuration changes affect live infrastructure.
Files:
ksail.prod.yaml
🪛 ast-grep (0.44.1)
scripts/tests/test_refresh_flux_ghcr_auth.py
[info] 484-486: use jsonify instead of json.dumps for JSON output
Context: json.dumps(
{"stringData": {"ghcr_dockerconfigjson": ciphertext}}
)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 518-518: use jsonify instead of json.dumps for JSON output
Context: json.dumps(config)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[error] 609-616: Command coming from incoming request
Context: subprocess.run(
[str(KSAIL_PULL_WRAPPER), *command],
cwd=ROOT,
env=environment,
text=True,
capture_output=True,
check=False,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(subprocess-from-request)
[info] 582-582: use jsonify instead of json.dumps for JSON output
Context: json.dumps(config)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 794-794: use jsonify instead of json.dumps for JSON output
Context: json.dumps(inventory)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 914-916: use jsonify instead of json.dumps for JSON output
Context: json.dumps(
self._valid_config(), sort_keys=True, separators=(",", ":")
)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🪛 Ruff (0.15.21)
scripts/tests/test_refresh_flux_ghcr_auth.py
[warning] 502-502: Consider moving this statement to an else block
(TRY300)
[error] 610-610: subprocess call: check for execution of untrusted input
(S603)
[error] 857-857: Possible hardcoded password assigned to argument: "GHCR_TOKEN"
(S106)
🔇 Additional comments (14)
scripts/ghcr-auth-lib.sh (1)
1-73: LGTM!scripts/refresh-flux-ghcr-auth.sh (2)
9-14: LGTM!Also applies to: 37-80, 141-506, 509-524
82-139: 🩺 Stability & AvailabilityNo change needed — the annotation update is already excluded by
annotated_resource_version; the loop only succeeds after a later object change andReady=True.> Likely an incorrect or invalid review comment.scripts/run-ksail-prod-with-pull-auth.sh (1)
1-59: LGTM!docs/dr/crypto-custody.md (1)
123-124: LGTM!docs/dr/runbook.md (1)
73-73: LGTM!Also applies to: 112-112, 167-193, 430-430, 453-453, 508-584
ksail.prod.yaml (1)
139-140: 🎯 Functional Correctness
ksail.prod.yamlis a protected file per repo guidelines — this diff modifies it.The coding guidelines for
ksail*.yamlstate changes toksail.prod.yamlshould not be made because they affect live infrastructure, and AGENTS.md itself listsksail.prod.yamlunder "Protected — never modify." This PR's core change touches Line 140 of that exact file. If this is an intentional, reviewed exception (required to wire in the credential bridge), it should be called out explicitly (e.g., in the PR description/AGENTS.md) rather than silently violating the stated policy — otherwise automated tooling/agents following AGENTS.md may flag or revert this in the future.As per coding guidelines,
ksail*.yaml: "Do not modifyksail.prod.yaml; production cluster configuration changes affect live infrastructure."Source: Coding guidelines
talos/cluster/authenticate-ghcr-pulls.yaml (1)
40-44: RegistryAuthConfig migration looks correct.Verified
RegistryAuthConfigis a valid standalone Talos v1alpha1 config document with this exactapiVersion/kind/name/username/passwordshape, matching the schema used by the companionrefresh-flux-ghcr-auth.shgenerator for existing nodes.talos/cluster/mark-ghcr-pull-revision.yaml (1)
12-14: Design and naming look correct.The desired/verified revision-annotation split is well-reasoned and documented; naming and single-document structure comply with the Talos patch-file guideline.
AGENTS.md (1)
146-151: LGTM!Also applies to: 163-176, 177-195, 329-336
.github/actions/deploy-prod/action.yml (1)
22-22: LGTM!Also applies to: 53-65, 142-150, 164-164, 309-322, 345-365
.github/workflows/dr-rebuild.yaml (1)
88-170: LGTM!scripts/tests/test_refresh_flux_ghcr_auth.py (1)
6-154: LGTM!Also applies to: 159-196, 201-1421
.github/workflows/ci.yaml (1)
74-86: LGTM!Also applies to: 253-262
Resolved the CodeRabbit outside-diff CI-filter finding in 7d5c338: docs/dr/runbook.md now participates in the k8s change filter, so the DR ordering regression runs on runbook-only changes. Revalidated with 62 unit tests, actionlint, ShellCheck, and zizmor. |
@coderabbitai review Please perform an incremental exact-head review of 7d5c338 after the four resolved findings and the documented adversarial-test refutation. |
|
✅ Action performedReview finished.
|
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7d5c338665
ℹ️ 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".
@coderabbitai review The Codex reviewer produced no artifact after a 20-minute polling window, so this is the single permitted fallback lane. Please review exact head bd60925. |
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd609251ad
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a1711f5078
ℹ️ 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".
1 participant
Summary
RegistryAuthConfig→ exact image pull → verified revision, then synchronize and verify the Kubernetes/OpenBao fan-outGHCR_TOKENonly for publication/signing while constrained KSail lifecycle commands use the SOPS pull credentialRoot cause
There were two related credential paths with different failure modes.
First, the deploy pushed and reconciled before refreshing root Flux pull auth.
flux-system/ksail-registry-credentialsis KSail-managed and remained stale; KSail redacts credential-only changes from its structural diff (devantler-tech/ksail#6107). The rootOCIRepositorytherefore returned GHCRDENIED, blocking the artifact containing the updated SOPS seed and deadlockingvariables-base→ PushSecret → OpenBao → ExternalSecret.Second, Talos host registry auth and
ksail.prod.yamlstill consumed the separate ActionsGHCR_TOKEN. That publication token was denied for private KSail pulls on a newly autoscaled node, producing GHCR HTTP 403 andImagePullBackOfffor the uncached operator image. A podimagePullSecretcannot repair this path because Talos performs first-party signature/artifact resolution with host registry auth. Credential redaction also meant a later token-only rotation would not refresh the autoscaler machine template without a visible revision input.Fix
scripts/run-ksail-prod-with-pull-auth.shnow constrains production KSail commands and derives lifecycle username/token fromstringData.ghcr_dockerconfigjson.workload pushdeliberately preserves the separate Actions actor/write token. A SHA-256 of the committed SOPS ciphertext—not decrypted auth—is passed as the desired Talos/autoscaler revision.Before Kubernetes credential mutation,
scripts/refresh-flux-ghcr-auth.shvalidates every exact private package. It lists every Kubernetes Node, rejects empty/ambiguous/duplicate InternalIPs, skips nodes already verified at the current ciphertext revision, and processes stale workers before control planes. Each stale node receives a file-backed Talos v1.13RegistryAuthConfigpatch in no-reboot mode, must pull the exact live KSail operator image (or the exact declared version during DR bootstrap), and only then receives a separate verified-revision annotation. Talos output is captured in mode-0600 temporary files so a failing command cannot echo the credential into Actions logs.The existing bridge then stages
variables-base, force-syncs the PushSecret and all three ExternalSecrets, canonical-compares every materialized Docker config with Git/SOPS, and finally merge-patches root Flux auth. Normal mode fails closed on partial fan-out; explicit DR bootstrap mode repairs root auth before ESO exists and requires a full verification pass after Flux converges. Both production jobs install the pinned Talos CLI before the first mutating bridge.Safety
Validation
PYTHONDONTWRITEBYTECODE=1 python3 -m unittest discover -s scripts/tests -p 'test_*.py'— 66 passedzizmor— passed/no findingstalosctl validate— prod/cloud and local/container control-plane + worker configs passedksail workload validateandksail --config ksail.prod.yaml workload validate— 109 kustomizations / 541 YAML files each./scripts/refresh-flux-ghcr-auth.sh --check-only— all seven exact private package reads passedLive recovery evidence
The full bridge completed successfully against production. All nine Talos nodes accepted the Git/SOPS host auth, proved the exact private KSail image pull, and now carry the current verified revision. The KSail operator rollout is healthy, all six OCIRepositories report
Ready=True, and no pod is inErrImagePullorImagePullBackOff.Separate follow-up
Review also exposed a pre-existing supply-chain defect: root Cosign configuration is misnested, live root verification is null, and DR publishes unsigned. Atomic verified publication plus the coordinated DR signer correction remains isolated in #2627.
Fixes #2624
Fixes #2628
Part of #2613