CMP-4550: Deliver Variable values to CEL rules, honoring TailoredProfile setValues - #1381
CMP-4550: Deliver Variable values to CEL rules, honoring TailoredProfile setValues#1381Vincent056 wants to merge 1 commit into
Conversation
|
@Vincent056: This pull request references CMP-4550 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target either version "5.1.0." or "openshift-5.1.0.", but it targets "compliance-operator-1.10.0" instead. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Vincent056 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
a170ebd to
ce1e830
Compare
ce1e830 to
1e1cb9b
Compare
|
🤖 To deploy this PR, run the following command: |
52146ce to
e3148c5
Compare
|
🤖 To deploy this PR, run the following command: |
|
🤖 To deploy this PR, run the following command: |
…ile setValues CEL rules could not consume Variable CRs: admission compiled expressions with inputs-only declarations, so any variable reference was rejected as UNDECLARED_REFERENCE, and a TailoredProfile's setValues were silently inert for CEL scans. Bind every Variable in the scan namespace under an auto-derived CEL identifier: dashes become underscores (ocp4-var-max-pods is referenced as ocp4_var_max_pods). Kubernetes names cannot contain underscores, so the mapping is unambiguous and reversible. Values resolve per scan with setValues taking precedence over the stored Variable value, and the Variable CR is never mutated. Each rule is evaluated with the variables filtered against its own input names, so an input always wins exactly the way admission validation declares it. CEL keywords and standard type names are never used as identifiers. Validation gains the same knowledge through the SDK's exported validator: the CustomRule controller and the ProfileBundle CEL parser pass the namespace's Variable names, so a rule referencing a Variable that exists admits Ready while an unknown identifier is rejected with UNDECLARED_REFERENCE. The CustomRule controller watches Variables and revalidates the rules whose expression references one — a rule created before its Variable becomes Ready once it appears, and a rule whose Variable disappears is marked Error — and a transient Variable listing failure is retried instead of being reported as an unknown identifier. No SDK or CRD schema changes. Adds unit coverage for the derivation, validation, per-rule variable filtering, controller revalidation paths, and the Variable watch mapper, plus an e2e test covering the full flow including the setValues override, the negative admission case, and late Variable creation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
e3148c5 to
7d20e68
Compare
|
🤖 To deploy this PR, run the following command: |
3 participants
What
CEL rules can now consume Variable CRs, and a TailoredProfile's
setValuesoverride those values per scan. Previously (CMP-4550) the CustomRule admission compiled expressions with inputs-only declarations, so any variable reference was rejected asUNDECLARED_REFERENCE, and setValues were silently inert for CEL scans — a rule keyed to the tailored value evaluated against nothing.How
Every Variable in the scan namespace is bound under an auto-derived CEL identifier: dashes become underscores, so the Variable
ocp4-var-max-podsis referenced in expressions asocp4_var_max_pods. Kubernetes resource names cannot contain underscores, so the mapping is unambiguous and reversible (the same munging kube-state-metrics applies to label keys for PromQL).cel-scanner):buildScanVariableslists the namespace's Variables, overlays the scan-resolved values (TailoredProfile setValues take precedence over the stored value; the Variable CR is never mutated), and feeds the derived bindings through the SDK's existingconfig.Variablespipe. Each rule is evaluated with the variables filtered against its own input names, so an input wins exactly as admission declared it for that rule. Names that derive to something unusable (digit-led, CEL keywords, CEL standard type names such aslist/intthat would shadow the type intype(x) == list) are skipped.celvalidation, CustomRule controller, ProfileBundle CEL parser): the namespace's Variable names are passed into validation, which declares the derived identifiers through the SDK's exportedValidateCELExpressionWithInputs. A rule referencing an existing Variable admitsReady; an unknown identifier is rejected withUNDECLARED_REFERENCE. Bundle CEL rules get the same treatment at parse time (their ownvariables:list is included), so content can adopt the convention without tripping the parser.No SDK changes and no CRD schema changes — the delivery uses only existing, exported surfaces.
Testing
TestCustomRuleVariableSetValues(parallel lane) — CustomRule referencing a Variable via the derived identifier admits Ready, an unknown identifier admits Error, a rule created before its Variable flips Error → Ready once the Variable is created, the scan passes with the stored value, addingsetValuesand rescanning flips the same check to FAIL, and the Variable CR is asserted unmutated afterwards.DONE COMPLIANTat the stored value (100000),DONE NON-COMPLIANTwith the check FAIL aftersetValues: "0", Variable CR still100000afterwards.Notes
ocp4-var-x→ocp4_var_x). This is the same couplingTailoredProfile.setValuesalready has (it names the prefixed CR too); a bundle-independent derivation would need a collision policy for two bundles of the same content and is left for the declarations follow-up.getDetailedCompilationError) re-validate with inputs only, so a rule whose input fetch fails is diagnosed as an undeclared reference to its (existing, bound) variable. The verdict is right, the message is misleading; fix belongs in the SDK.celvalidationmirrors the SDK'sCompileCELExpressionuntil the SDK exports a declaration-taking variant.pkg/profileparser/manual-bundle.yamlis added to.gitignore:cel_content_test.gowrites it into the package directory on every run (Ginkgo v1'sTempDir()is a no-op), which is how it nearly slipped into this PR.variables:declaration tier (chosen short names, defaults, bundle-portable content rules, typed binding) and a reservedvarsmap for existence checks / in-expression defaults.🤖 Generated with Claude Code