Skip to content

CMP-4447: Fix the five shipped CIS OCP-Virt CEL rules (false positives and eval errors) - #14916

Merged
Mab879 merged 6 commits into
ComplianceAsCode:masterfrom
Vincent056:cmp-4447-fix-cel-rules
Aug 7, 2026
Merged

CMP-4447: Fix the five shipped CIS OCP-Virt CEL rules (false positives and eval errors)#14916
Mab879 merged 6 commits into
ComplianceAsCode:masterfrom
Vincent056:cmp-4447-fix-cel-rules

Conversation

@Vincent056

Copy link
Copy Markdown
Contributor

Summary

Fixes all five shipped CIS OCP-Virt (cis-vm-extension) CEL rules — one commit per Jira:

Commit CIS Bug fixed
CMP-4447 1.1 false FAIL when the host-device allowlist is only partially present (also covers usbHostDevices)
CMP-4448 1.2 false positive on every OCP 4.18+ cluster (nonRoot field removed; absence is compliant)
CMP-4449 1.3 false FAIL when persistentReservation is absent (the semantic default "disabled")
CMP-4450 1.5 eval ERROR (no such key: insecureRegistries) when storageImport lacks the key
CMP-4451 2.2 hard ERROR on any cluster with VMs (vms.all on a List wrapper instead of vms.items.all) + plural resource name

Testing

Every rule ships cel/tests/cases.yaml fixtures (26 cases total, each rule with compliant AND non-compliant cases) evaluated through celctl — the compliance-operator scanner engine (cel-go). All validated three ways:

  • celctl cac lint — passes for all 5 (2.2 failed before this PR)
  • celctl cac test — 26/26 cases pass
  • celctl cac live on an OCP 4.22 + CNV cluster with running VMs — all 5 rules PASS; before this PR the same cluster reported 1.2 FAIL (false positive) and 2.2 ERROR

Notes

🤖 Generated with Claude Code

@openshift-ci openshift-ci Bot added the do-not-merge/work-in-progress Used by openshift-ci bot. label Jul 23, 2026
@openshift-ci

openshift-ci Bot commented Jul 23, 2026

Copy link
Copy Markdown

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@Vincent056
Vincent056 marked this pull request as ready for review July 24, 2026 06:29
@openshift-ci openshift-ci Bot removed the do-not-merge/work-in-progress Used by openshift-ci bot. label Jul 24, 2026
@Vincent056
Vincent056 force-pushed the cmp-4447-fix-cel-rules branch from da7c3e3 to b1f1977 Compare July 24, 2026 06:48
@jan-cerny jan-cerny added the OpenShift OpenShift product related. label Jul 28, 2026
@Vincent056

Copy link
Copy Markdown
Contributor Author

/test e2e-aws-openshift-platform-compliance

@rhmdnd rhmdnd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like the overall structure of the test cases, and that we're adding CEL test coverage support here.

Some recommendations inline.

@rhmdnd rhmdnd added the CIS CIS Benchmark related. label Aug 5, 2026
Vincent056 and others added 3 commits August 5, 2026 10:56
…l allowlist

CIS OCP-Virt 1.1. The expression required BOTH pciHostDevices and
mediatedDevices to be present-and-empty, so an HCO with only
'pciHostDevices: []' (no devices permitted - compliant) reported FAIL.
Treat a missing sub-key as empty, and also cover usbHostDevices, which the
benchmark remediation lists.

Fixtures: 7 cases (absent/all-empty/partial-empty compliant; pci/usb/mediated
device configured non-compliant; missing HCO non-compliant). celctl cac
lint/test 7/7; live on OCP 4.22 + CNV: PASS.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CIS OCP-Virt 1.2. The benchmark scopes this control to 'prior to 4.18':
from 4.18 the nonRoot field is removed from HyperConverged featureGates and
non-root execution is always enforced, so requiring the field made the rule
FAIL on every 4.18+ cluster. Treat an absent field as compliant; only FAIL
when nonRoot is present and not true. rule.yml ocil updated to match.

Fixtures: 5 cases (true/absent/no-featureGates compliant; false and missing
HCO non-compliant). celctl cac test 5/5; live on OCP 4.22 + CNV: PASS
(was FAIL before this fix).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…d as compliant

CIS OCP-Virt 1.3. Persistent reservations are disabled by default; the rule
required featureGates.persistentReservation to be present AND false, so the
semantic default (key absent) reported a false FAIL. Absent now passes; only
an explicit 'true' fails.

Fixtures: 5 cases. celctl cac test 5/5; live on OCP 4.22 + CNV: PASS.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Vincent056 and others added 3 commits August 5, 2026 10:56
…cureRegistries key

CIS OCP-Virt 1.5. When storageImport exists without an insecureRegistries
key, the expression raised 'no such key: insecureRegistries', which the
scanner maps to FAIL - a false non-compliant for a compliant configuration.
Guard the nested key so absence passes.

Fixtures: 4 cases (no storageImport / missing key / empty list compliant;
populated list non-compliant). celctl cac test 4/4; live on OCP 4.22 + CNV:
PASS.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CIS OCP-Virt 2.2. The input 'vms' has no resource_name, so the scanner binds
it as a List wrapper - but the expression iterated 'vms.all(...)' instead of
'vms.items.all(...)', walking the wrapper's map keys and erroring with
'no such key: spec' on any cluster with VMs. Also normalize the input
resource to the plural 'virtualmachines' for consistency. Scanner RBAC for
virtualmachines.kubevirt.io ships with compliance-operator#1293 (merged).

Fixtures: 5 cases (no VMs / plain VM / explicit false / no resources block
compliant; overcommit true non-compliant). celctl cac lint passes (failed
before); cac test 5/5; live on OCP 4.22 + CNV with running VMs: PASS
(was ERROR before this fix).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Rename all CEL test cases to plain prose without Jira tickets or
parenthetical qualifiers, simplify the provenance fetched_with note, and
unwrap the parenthetical failure-reason sentences in the nonroot and
persistentReservation rules. Requested by rhmdnd in review.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@Vincent056
Vincent056 force-pushed the cmp-4447-fix-cel-rules branch from b1f1977 to 944237a Compare August 5, 2026 17:58
@Vincent056
Vincent056 requested a review from rhmdnd August 5, 2026 18:15

@rhmdnd rhmdnd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@openshift-ci

openshift-ci Bot commented Aug 5, 2026

Copy link
Copy Markdown

@Vincent056: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-aws-openshift-node-compliance 944237a link false /test e2e-aws-openshift-node-compliance

Full PR test history. Your PR dashboard.

Details

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 kubernetes-sigs/prow repository. I understand the commands that are listed here.

@Mab879 Mab879 added this to the 0.1.82 milestone Aug 7, 2026
@Mab879
Mab879 merged commit b5e125e into ComplianceAsCode:master Aug 7, 2026
71 of 73 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CIS CIS Benchmark related. OpenShift OpenShift product related.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants