Skip to content

fix(security): supply the C-0211 baseline context to snapshot-controller - #3741

Merged
devantler merged 4 commits into
mainfrom
claude/snapshot-controller-c0211-baseline-3239
Sep 13, 2026
Merged

devantler merged 4 commits into
mainfrom
claude/snapshot-controller-c0211-baseline-3239

Conversation

@devantler

Copy link
Copy Markdown
Contributor

🤖 Generated by the Agentic Engineer

Why

The cluster snapshot controller is one of the remaining kube-system workloads whose stored template misses the C-0211 baseline context. The admission mutation deliberately skips that namespace, so the fix has to come from the workload's own source. It is the same low-risk shape as the descheduler slice (#3733): no volumes and no group ownership, so there is nothing for the new settings to change at runtime.

What

Sets the ownership-change policy and an empty SELinux options object through the chart's own values. Rendering the pinned chart confirms that these two fields are the only change and that all existing hardening stays intact.

Merge order: this moves the EKS authorization fingerprint, like every HelmRelease change. The approved value is taken from CI, and it has to be re-derived after whichever of this PR or #3736 merges second (#3740). Merges on this repo are also held until the production fence in #3065 is released.

Part of #3239.

🤖 Generated with Claude Code

kube-system is excluded from the add-security-context mutation, so the
stored controller template is supplied through the chart's own values.
Rendering the pinned chart shows the two fields are the entire delta and
the existing hardening is preserved.

Part of #3239.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ot-controller

CI run 34716590753 rendered this branch under the approved toolchain and
reported only the aggregate as unapproved, with zero missing and zero
duplicate resources. A two-tree render of all five authorization roots
shows the 86 grant-bearing identities, role rules and binding subjects
unchanged; the only hunk is the two values fields.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

@coderabbitai review

All required checks are green at this head. The EKS authorization fingerprint was taken from CI's own validator output for this tree, with the two-tree conservation check recorded in the commit.

@coderabbitai

coderabbitai Bot commented Sep 12, 2026 •

Copy link
Copy Markdown
Contributor

@devantler: I will review the changes.

✅ Action performed

Review finished.

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 Sep 12, 2026 •

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

Warning

Review limit reached

Next included review available in 27 minutes.

Check out review usage here.

View limit details

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

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

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 83eff0c2-7b03-41da-8c9b-1082063ad09a

📥 Commits

Reviewing files that changed from the base of the PR and between 65e833d and 11691a0.

📒 Files selected for processing (1)
  • scripts/validate-eks-ci-role-policy/main.go

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 913586bf-d7df-41c3-95d8-b005a4c9aef5

📥 Commits

Reviewing files that changed from the base of the PR and between 491d5ef and 65e833d.

📒 Files selected for processing (2)
  • k8s/providers/hetzner/infrastructure/controllers/snapshot-controller/helm-release.yaml
  • scripts/validate-eks-ci-role-policy/main.go

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

📜 Recent review details
🔇 Additional comments (2)
k8s/providers/hetzner/infrastructure/controllers/snapshot-controller/helm-release.yaml (1)

70-81: LGTM!

scripts/validate-eks-ci-role-policy/main.go (1)

1973-2009: LGTM!


📝 Walkthrough

Walkthrough

The snapshot-controller HelmRelease now sets fsGroupChangePolicy: OnRootMismatch and an empty seLinuxOptions object through chart values. The authorization-surface approval record now documents these additions and updates the expected rendered-surface fingerprint. Existing authorization identities and RBAC rules remain unchanged.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 65e83

The snapshot-controller baseline values and approval fingerprint are consistent, with no unresolved merge-blocking risk identified.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description check ✅ Passed The description clearly explains the snapshot-controller security-context changes and the related EKS authorization fingerprint update.
Title check ✅ Passed The title clearly and concisely identifies the main change: supplying the C-0211 baseline context to snapshot-controller.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
⚔️ Resolve merge conflicts 💡
  • Resolve merge conflict in branch claude/snapshot-controller-c0211-baseline-3239

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

Comment @coderabbitai help to get the list of available commands.

devantler and others added 2 commits September 12, 2026 23:13
Resolve the EKS rendered-surface fingerprint conflict with the Wedding data-loss repair (#3738). Both changes re-approved from 814debc4; the merged tree's aggregate is pending CI's validator on this commit, so the constant holds main's 8756f2ad as the recorded previous value.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CI's validator on c61aac1 reported the merged tree's aggregate as its only authorization error (0 missing, 0 duplicate). Previous aggregate 8756f2ad (#3738).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

@coderabbitai review

The earlier review at 65e833d is stale: this head merges main (the Wedding data-loss repair) and re-pins the EKS authorization fingerprint to the value CI's validator reported for the merged tree (no missing or duplicate identities). All checks are green here.

@coderabbitai

coderabbitai Bot commented Sep 12, 2026 •

Copy link
Copy Markdown
Contributor

@devantler: I will review the current pull request head.

⚠️ 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.

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

@codex review

CodeRabbit declined this head under its rate limit, so this moves to the next review lane. This head merges main (the Wedding data-loss repair) and re-pins the EKS authorization fingerprint to the value CI's validator reported for the merged tree; all checks are green.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 12, 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-09-12T21:40:15.693981Z 11691a0 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. What shall we delve into next?

Reviewed commit: 11691a0ff5

ℹ️ 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
devantler marked this pull request as ready for review September 12, 2026 21:41
@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

Ready — merge deliberately held

Readiness at 11691a0ff54d57bbfd3376495bf2e2a746d64dd9:

  • Tested: all 21 checks pass (7 skipped), including the manifest validation, the Kubescape scan and the production authorization validator. The fingerprint was taken from that validator's own report for the merged tree, which found no missing or duplicate identities. Main's aggregate is still the recorded previous value after fix(wedding-db): correct pre-loss recovery witness #3742, which changed only the recovery script and its test.
  • Reviewed: Codex found no major issues at this head, with no finding comments and no review threads. CodeRabbit declined this round under its rate limit.
  • Evaluated: this change has no behaviour to exercise before it deploys. I checked what it produces instead: rendered, the chart gains exactly the two intended security-context fields on the snapshot-controller, both overlays build, and every grant-bearing identity and rule body is identical before and after.

Why it is not queued yet: merging here runs a production deploy on the same prod-deploy lock the Wedding database recovery workflow uses. That recovery is being worked right now (#3742 merged at 21:33Z, a recovery run was cancelled at 21:35Z), so a queued deploy would make the next recovery attempt wait. A security-baseline change can wait instead. It goes into the merge queue once no recovery run is active or pending.

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

Still ready — merge still deliberately held (re-verified 23:4xZ)

Readiness at 11691a0ff54d57bbfd3376495bf2e2a746d64dd9 is unchanged: Codex green at this head, 0 review threads, all checks green, CLEAN. No push or comment since the 21:42Z note.

The EKS fingerprint pin is still valid: main's expectedRenderedSurfaceSHA is still 8756f2ad…, the Previous aggregate this PR re-pinned against.

Why it is not enqueued yet: the Wedding database recovery is still in flight, and a merge-queue deploy would take the shared prod-deploy concurrency slot.

  • Recovery run 34724909168 (dispatched 23:14:51Z) failed at 23:18Z in its restore-and-prove job.
  • The follow-up recovery fix, fix(wedding-db): fence annotation-free resources #3745, touches only the recovery script and its test, and is going through the merge queue now. The next recovery attempt most plausibly follows it.

Enqueueing this snapshot-controller hardening in between would make that recovery wait behind it.

Enqueue condition: no recovery run is queued or in progress and the recovery sequence has settled. Then enqueue with gh pr merge 3741 --repo devantler-tech/platform --match-head-commit 11691a0ff54d57bbfd3376495bf2e2a746d64dd9, after re-checking that main's fingerprint is still 8756f2ad…. If another fingerprint-moving PR lands first (#3736, #3735), this PR needs a merge from main and a CI-derived re-pin per #3740.

@devantler
devantler added this pull request to the merge queue Sep 13, 2026
Merged via the queue into main with commit 61a8c31 Sep 13, 2026
29 checks passed
@github-project-automation github-project-automation Bot moved this from 🫴 Ready to ✅ Done in 🌊 Project Board Sep 13, 2026
@devantler
devantler deleted the claude/snapshot-controller-c0211-baseline-3239 branch September 13, 2026 02:26
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