ROX-35910: add observability for scanner-type divergence in compliance profile dispatchers - #21965
ROX-35910: add observability for scanner-type divergence in compliance profile dispatchers#21965guzalv wants to merge 2 commits into
Conversation
CEL profiles have no XCCDF ID. The compliance operator deliberately sets ComplianceScan.Spec.Profile to the k8s object name for CEL profiles, while complianceProfile.ID contains only the short content ID (e.g. 'cis-vm-extension' vs 'ocp4virt-cis-vm-extension'). This caused BuildProfileRefID to produce different UUIDs on the profile and scan sides, breaking the SQL JOIN in the coverage view (0 results shown). Apply the same fix that already exists for tailored profiles with custom rules: use complianceProfile.Name when the scanner-type annotation is 'CEL', in both the V1 and V2 proto blocks. Add tests covering CEL vs OpenSCAP ProfileId selection in both V1 and V2 event paths. Verified on ga-ocp4-cron cluster: API returned 0 results before fix, 5 ERROR checks after. Coverage UI confirms all checks visible. Co-authored-by: AI assistant (Claude Sonnet)
…tation for CEL TPs In the compliance operator, when a TailoredProfile contains custom rules, both ScannerTypeAnnotation=CEL and CustomRuleProfileAnnotation=true are set atomically. Checking either condition is equivalent for that case, but checking ScannerTypeAnnotation is semantically more correct and covers two additional cases introduced in newer CO versions: - TPs with CEL-typed rules but no CustomRules (ScannerTypeAnnotation=CEL only) - TPs extending a CEL base profile (inherit ScannerTypeAnnotation=CEL) The companion fix for regular Profiles in complianceoperatorprofiles.go (PR #21904) already uses ScannerTypeAnnotation. This change brings complianceoperatortailoredprofiles.go into alignment. Add tests covering OpenSCAP and CEL TP ProfileId selection explicitly. Co-authored-by: AI assistant (Claude Sonnet)
|
Skipping CI for Draft Pull Request. |
|
PR needs rebase. DetailsInstructions 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. |
📝 WalkthroughSummary by CodeRabbit
WalkthroughProfile dispatchers now derive ChangesScanner-aware dispatch
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
sensor/kubernetes/complianceoperator/dispatchers/complianceoperatorprofiles.go (1)
47-77: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated scanner-type → ProfileId dispatch logic across both dispatchers.
Both files independently implement the same CEL/OpenSCAP/unknown switch with parallel debug/warn messages. Any future scanner type must be added in two places in lockstep, risking drift.
sensor/kubernetes/complianceoperator/dispatchers/complianceoperatorprofiles.go#L47-L77: extract the scanner-type switch (CEL → k8s name, OpenSCAP/empty → XCCDF ID, default → warn+fallback) into a shared helper, e.g.resolveProfileID(annotations map[string]string, k8sName, xccdfID, entityName string) string.sensor/kubernetes/complianceoperator/dispatchers/complianceoperatortailoredprofiles.go#L71-L94: reuse the same shared helper instead of re-implementing the switch, passingtailoredProfile.Status.IDas the XCCDF ID.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sensor/kubernetes/complianceoperator/dispatchers/complianceoperatorprofiles.go` around lines 47 - 77, Duplicate scanner-type-to-ProfileId resolution should be centralized to prevent the two dispatchers from drifting. In sensor/kubernetes/complianceoperator/dispatchers/complianceoperatorprofiles.go lines 47-77, extract the CEL, OpenSCAP/empty, and unknown-type handling into a shared resolveProfileID helper that preserves the existing logging and fallback behavior; in sensor/kubernetes/complianceoperator/dispatchers/complianceoperatortailoredprofiles.go lines 71-94, remove the parallel switch and reuse that helper, passing tailoredProfile.Status.ID as the XCCDF ID.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@sensor/kubernetes/complianceoperator/dispatchers/complianceoperatorprofiles.go`:
- Around line 47-77: Duplicate scanner-type-to-ProfileId resolution should be
centralized to prevent the two dispatchers from drifting. In
sensor/kubernetes/complianceoperator/dispatchers/complianceoperatorprofiles.go
lines 47-77, extract the CEL, OpenSCAP/empty, and unknown-type handling into a
shared resolveProfileID helper that preserves the existing logging and fallback
behavior; in
sensor/kubernetes/complianceoperator/dispatchers/complianceoperatortailoredprofiles.go
lines 71-94, remove the parallel switch and reuse that helper, passing
tailoredProfile.Status.ID as the XCCDF ID.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 8f0a25c7-66af-485a-bb7f-834d1b7b5dd3
📒 Files selected for processing (4)
sensor/kubernetes/complianceoperator/dispatchers/complianceoperatorprofiles.gosensor/kubernetes/complianceoperator/dispatchers/complianceoperatorprofiles_test.gosensor/kubernetes/complianceoperator/dispatchers/complianceoperatortailoredprofiles.gosensor/kubernetes/complianceoperator/dispatchers/complianceoperatortailoredprofiles_test.go
Description
Follow-up to #21904 and #21905. The compliance coverage join relies on
BuildProfileRefIDproducing identical UUIDs from the profile dispatcher and the scan dispatcher. If they diverge, the result is always the same silent symptom: 0 checks shown. This adds targeted logs to make such divergence detectable without a code change.Edge cases that are now observable:
Warn): if CO introduces a new scanner type that also uses the k8s name inComplianceScan.Spec.Profile(e.g. a hypothetical future type), the dispatcher falls back to XCCDF ID, causing a mismatch. The Warn log names the unknown type and the profile, making root cause obvious.Warn): ifcomplianceProfile.IDis empty for a non-CEL profile,BuildProfileRefIDsilently produces a wrong UUID. Now surfaced.Debug): confirms the k8s-name branch was taken, with both the name and the XCCDF ID logged for cross-referencing.Also fixes a latent issue in the TP dispatcher: the `Status.ID == """ guard skipped CEL TPs that had no XCCDF Status.ID, even though CEL TPs use the k8s name and don't need Status.ID. Now CEL TPs without Status.ID are allowed through.
User-facing documentation
Testing and quality
Automated testing
How I validated my change
Unit tests pass. Build clean.