Repository navigation
Share the self-signer gate, securityContext and Role rules across the install Job and rotation CronJobs - #691
Open
isaactwong wants to merge 2 commits into
Open
isaactwong wants to merge 2 commits into
isaactwong wants to merge 2 commits into
Conversation
The install Job and the two certificate-rotation CronJobs are three hand-written copies of one pod spec, in both charts. This test renders them side by side and compares what must agree: the pod and container securityContext, the TLS gate, and the rules of the Role each runs under. It currently records the drift. The CronJobs render without either securityContext block, the legacy chart's client/node CronJob renders with TLS off while its ServiceAccount and Role do not, and the rotate Role is a narrower copy of the install Role. The pre-delete cleaner Job is a fourth copy of the pod spec and still matches the install Job; the test pins that too so it cannot drift later. The next commit makes the three share one definition and flips these assertions to equality. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The install Job applied a pod and container securityContext under selfSigner.securityContext.enabled; the two rotation CronJobs did not. On a namespace enforcing Pod Security Admission "restricted" the install succeeded and every rotation pod was rejected, so certificates expired on schedule. The client/node CronJob also rendered without the tls.enabled check the Job and its own RBAC objects have, and the rotate Role was a narrower copy of the install Role (missing the OpenShift securitycontextconstraints rule in the operator chart). Each chart now defines the gate, both securityContext blocks and the Role rules once in _helpers.tpl, and the install Job, the pre-delete cleaner Job, both CronJobs, both Roles and the self-signer RBAC templates include them. The install Job and install Role render byte-for-byte as before. The rotation CronJobs gain the securityContext blocks and the tls.enabled gate; the rotate Role gains the install Role's remaining rules. The RoleBinding, ServiceAccount and cleaner Job edits are one-line gate swaps; their output is unchanged. In the operator chart the rotation ServiceAccount now holds the securitycontextconstraints grant permanently. The pair test now compares all four workloads against the install Job with the securityContext on and off, checks the client/node CronJob gate from both sides, and asserts the rules the rotate Role used to lack. Fixes #690 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In both charts the self-signer install Job, the pre-delete cleaner Job and the two certificate-rotation CronJobs were four hand-written copies of one pod spec, and the copies had drifted. The rotation CronJobs rendered without the pod and container
securityContextthe install Job sets, so a namespace enforcing Pod Security Admissionrestrictedadmitted the install and rejected every rotation pod; the legacy chart's client/node CronJob rendered without thetls.enabledcheck its own ServiceAccount and Role have; and the rotate Role was a narrower copy of the install Role.Each chart now defines the gate, both
securityContextblocks and the Role rules once in_helpers.tpl(selfcerts.enabled,selfcerts.podSecurityContext,selfcerts.containerSecurityContext,selfcerts.roleRules), and the four workloads plus the self-signer Role, RoleBinding and ServiceAccount templates include them. The install Job, cleaner Job and install Role render byte-for-byte as before; the CronJobs gain thesecurityContextblocks and the gate, and the rotate Role gains the install Role's remaining rules.The first commit adds a template test that renders the four workloads and both Roles for each chart and compares them. It passes on
masterby recording the drift; the second commit flips it to equality, with thesecurityContexton and off and the gate checked from both sides.One behavior worth a reviewer's eye: in the operator chart the install Role's unscoped
securitycontextconstraintsuserule now sits on the permanent rotation ServiceAccount as well (the install ServiceAccount holds it only for the pre-install hook). The CronJobs need an SCC to run as user 1000 on OpenShift, so this mirrors how the install Job already works; scoping the grant withresourceNameswould change the install path too and is left to a follow-up.Fixes #690
🤖 Generated with Claude Code