Repository navigation
refactor(policy): preserve Helm release defaults on CEL policies - #5360
Conversation
Local validation at The pinned CLI's CEL UPDATE processor copies the incoming object into oldObject, so the old-label fixture alone cannot prove the UPDATE boundary. A dedicated control opens only the selector on a temporary policy copy, modeling an already-admitted old-label request: the real incoming conditions skip both opt-outs, and removing those conditions makes the same fixtures fail. This runs in the existing policy job. Native run 40267919289 correctly rejected two unregistered policy identities and unknown CEL surfaces. The follow-up records only those two authorization-inventory replacements, with every other ledger entry unchanged. Consumer discovery separately recognizes their complete rendered API/kind/spec fingerprints; names alone grant nothing. Its full regression suite passes with zero failures, including both positives and 26 changed-scope, side-effect, patch-path, evaluation and foreign-kind/API refusals. The real conservation guard passes on Bash 3.2 and 5. The unchanged complete authorization regression suite and rendered equality check pass with the pinned renderer. ShellCheck, actionlint and the policy-job contract pass. As the platform operator, I inspected all ten prescribed configuration layers. Both provider infrastructure renders contain exactly the two new MutatingPolicies and no corresponding legacy ClusterPolicy. The defaults and unrelated settings match the legacy output. Local/prod KSail validation exits zero, but its Helm rendering reports missing local credential-helper support for 20 charts; that portion is not claimed as complete validation, including after a private anonymous-client retry. No shared credentials or installed tools changed. Full current-head native CI, substantive review, protected deployment and live policy readback remain required. Parent #4997 remains open for its other policies. |
The manifest failure at The complete real Crossplane/Helm/effective-policy suite passes on Bash 3.2 and 5, including both approved definitions and 26 schema-valid altered-scope, side-effect, value-path, extra-selector and evaluation controls. An initial unknown-field fixture failed schema validation before reaching the intended gate; it was replaced with a schema-valid control, preserving both validators. No schema check or admission gate was disabled. Both local and production configuration validation also completed using an isolated anonymous public-chart client that reads no host credentials, resolving the earlier missing-helper limitation. The known offline namespace-label warnings remain unproven admission cases. Those runs precede the latest independent main synchronization; fresh native CI must judge the combined head. The final combined authorization suite passes uncached and consumer conservation remains exact. No production deployment or live policy acceptance is claimed. Parent #4997 remains open. |
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
The acknowledged request's original 30-minute observation window has expired. A complete fresh read at 06:37 UTC still shows only the running acknowledgement, with no submitted review, completed verdict or finding. The current head and all finding surfaces were checked again before advancing the ordered review loop. This records a stalled request, not a successful review or permission to merge without one. A later finding remains actionable. |
|
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 3ed20e3. Configure here.
@coderabbitai full review |
|
|
No actionable comments were generated in the recent review.
 |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Linked Issues check |  |
The implementation matches the coding objectives in [#5357]. Both resources are MutatingPolicy objects with CREATE/UPDATE matching, incoming-label conditions, failurePolicy: Fail, and JSON pat… |
Provide current-head results for the required policy tests, fixture-evaluation guard, and manifest/layer validation. No conclusion is made here from the separately outstanding deployment, review, or live-readback tasks because those are non… |
-
Check name Status Explanation Title check The title clearly summarizes the main change: migrating Helm release default policies to CEL-based policies while preserving their behavior. Description check The description directly explains the need to migrate the two release defaults and preserve testing, retry settings, opt-outs, and unrelated settings. Out of Scope Changes check The changes stay within [ #5357]. They migrate onlyhelm-release-enable-testsandhelm-release-remediation-retriesand add directly related fixtures, regression tests, CI wiring, consumer-discovery…Docstring Coverage Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 4 files. (18 skipped: 1… Full details: Out of Scope Changes check
-
Explanation
The changes stay within [
#5357]. They migrate onlyhelm-release-enable-testsandhelm-release-remediation-retriesand add directly related fixtures, regression tests, CI wiring, consumer-discovery checks, and authorization-surface records. The CRD-install default and other migration work remain unchanged.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 4 files. (18 skipped: 18 unsupported.)
-
Full details: Linked Issues check
-
Explanation
The implementation matches the coding objectives in [
#5357]. Both resources areMutatingPolicyobjects withCREATE/UPDATEmatching, incoming-label conditions,failurePolicy: Fail, and JSON patches that preserve unrelated settings. The PR adds fixture coverage, repeated-application and composition checks, fixture-evaluation guards, CI wiring, and policy-surface validation. However, the available record says current-head native CI and post-synchronization manifest/layer validation remain outstanding, so complete automated-test compliance cannot be established.Resolution
Provide current-head results for the required policy tests, fixture-evaluation guard, and manifest/layer validation. No conclusion is made here from the separately outstanding deployment, review, or live-readback tasks because those are non-coding tasks.
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
CodeRabbit explicitly declined this exact-head request under the included review limit. No green review was produced. The earlier Codex request already has its authenticated acknowledgement-timeout record, and Bugbot remains neutral after the sole finding was refuted with native execution evidence and its thread resolved. The draft still needs a qualifying current-head green review; collect the next included review opportunity after the provider’s reset rather than repeat a request inside the same limit window. |
@codex review |
|
Codex Review: Didn't find any major issues. 👍 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 full review |
✅ Action performed
|
Current-head readiness at Native CI 40272630712 passed the complete policy fixtures, consumer-discovery regressions, production authorization, repository guards, and local/production manifest validation. These exercise the migrated defaults and preservation boundaries described in the earlier operator evaluation. The current-head security and Code Quality workflow runs also completed successfully; the separate native Code Quality rule remains independently unqueryable, so it is not represented as independently verified. The Codex receipt matches this head. The refreshed CodeRabbit summary passes the reviewed exact-head verdict helper. All review/comment surfaces were read completely; the sole Bugbot finding has native refutation evidence and its thread is resolved. The summary's inconclusive linked-issue assessment requested current-head validation; the native run above supplies those results. The base remains |
Why
Two automatic release defaults still rely on a retiring policy format. They need to keep working through the platform upgrade.
What
Preserve automatic release testing and retry settings on the supported format, including existing opt-outs and unrelated settings. The broader migration stays open.
Fixes #5357
Part of #4997