Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 34 additions & 13 deletions cmd/manager/cel-scanner.go
Original file line number Diff line number Diff line change
Expand Up @@ -348,11 +348,41 @@ func (c *CelScanner) runPlatformScan() {
celVariables = append(celVariables, celVar)
}

// Build SDK rule list, skipping rules with empty expressions
// Convert SDK results to compliance operator results
evalResultList := []*cmpv1alpha1.ComplianceCheckResult{}
// Cache custom metadata per result so we can merge it with the same
// precedence logic the SCAP/aggregator path uses (operator keys win).
type customMeta struct {
labels map[string]string
annotations map[string]string
}
customMetadataByName := make(map[string]customMeta)

// Build SDK rule list; produce MANUAL results directly for rules without expressions
sdkRules := make([]scanner.Rule, 0, len(selectedRules))
for _, rw := range selectedRules {
if rw.payload.Expression == "" {
cmdLog.Info("Warning: Skipping rule with empty expression", "rule", rw.scannerRule.Identifier())
// Manual rule — produce CheckResultManual directly, bypass SDK scanner
checkResultName := fmt.Sprintf("%s-%s", c.celConfig.ScanName, utils.IDToDNSFriendlyName(rw.payload.ID))
cl, ca := utils.GetCustomMetadata(rw.labels, rw.annotations)
customMetadataByName[checkResultName] = customMeta{labels: cl, annotations: ca}
evalResultList = append(evalResultList, &cmpv1alpha1.ComplianceCheckResult{
TypeMeta: metav1.TypeMeta{
APIVersion: "compliance.openshift.io/v1alpha1",
Kind: "ComplianceCheckResult",
},
ObjectMeta: metav1.ObjectMeta{
Name: checkResultName,
Namespace: c.celConfig.NameSpace,
},
ID: rw.payload.ID,
Description: rw.payload.Description,
Rationale: rw.payload.Rationale,
Severity: cmpv1alpha1.ComplianceCheckResultSeverity(rw.payload.Severity),
Instructions: rw.payload.Instructions,
Status: cmpv1alpha1.CheckResultManual,
})
cmdLog.Info("Manual rule — no CEL expression, result is MANUAL", "rule", rw.scannerRule.Identifier())
continue
}
sdkRules = append(sdkRules, rw.scannerRule)
Expand Down Expand Up @@ -380,16 +410,6 @@ func (c *CelScanner) runPlatformScan() {
for i := range selectedRules {
ruleByID[selectedRules[i].scannerRule.Identifier()] = &selectedRules[i]
}

// Convert SDK results to compliance operator results
evalResultList := []*cmpv1alpha1.ComplianceCheckResult{}
// Cache custom metadata per result so we can merge it with the same
// precedence logic the SCAP/aggregator path uses (operator keys win).
type customMeta struct {
labels map[string]string
annotations map[string]string
}
customMetadataByName := make(map[string]customMeta)
for _, result := range checkResults {
rw, found := ruleByID[result.ID]
if !found {
Expand Down Expand Up @@ -737,7 +757,8 @@ func (c *CelScanner) getCELRulesFromProfile(profileName, namespace string) ([]ce
// validateCELRulePayload validates that a RulePayload has the required CEL fields.
func (c *CelScanner) validateCELRulePayload(name string, payload *cmpv1alpha1.RulePayload) error {
if payload.Expression == "" {
return fmt.Errorf("CEL expression is empty")
// Manual rule — no expression to validate
return nil

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would it make sense to output a log message?

}

if len(payload.Inputs) == 0 {
Expand Down
30 changes: 12 additions & 18 deletions cmd/manager/cel_scanner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,35 +114,30 @@ var _ = Describe("getCELRulesFromProfile", func() {
Expect(err.Error()).To(ContainSubstring("not found"))
})

It("returns error for CEL rule with empty expression", func() {
It("accepts CEL rule with empty expression as manual rule", func() {
scheme := newTestScheme()
profile := &cmpv1alpha1.Profile{
ObjectMeta: metav1.ObjectMeta{Name: "prof", Namespace: "ns"},
ProfilePayload: cmpv1alpha1.ProfilePayload{
Rules: []cmpv1alpha1.ProfileRule{"bad-rule"},
Rules: []cmpv1alpha1.ProfileRule{"manual-rule"},
},
}
badRule := &cmpv1alpha1.Rule{
ObjectMeta: metav1.ObjectMeta{Name: "bad-rule", Namespace: "ns"},
manualRule := &cmpv1alpha1.Rule{
ObjectMeta: metav1.ObjectMeta{Name: "manual-rule", Namespace: "ns"},
RulePayload: cmpv1alpha1.RulePayload{
ID: "bad-rule",
ID: "manual-rule",
ScannerType: cmpv1alpha1.ScannerTypeCEL,
Expression: "",
Inputs: []cmpv1alpha1.InputPayload{{
Name: "pods",
KubernetesInputSpec: cmpv1alpha1.KubernetesInputSpec{
APIVersion: "v1", Resource: "pods",
},
}},
Inputs: nil,
},
}
client := fake.NewClientBuilder().WithScheme(scheme).
WithObjects(profile, badRule).Build()
WithObjects(profile, manualRule).Build()
cs = &CelScanner{client: client, scheme: scheme}

_, err := cs.getCELRulesFromProfile("prof", "ns")
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("invalid Rule"))
rules, err := cs.getCELRulesFromProfile("prof", "ns")
Expect(err).NotTo(HaveOccurred())
Expect(rules).To(HaveLen(1))
})

It("returns empty slice when profile has only non-CEL rules", func() {
Expand Down Expand Up @@ -272,7 +267,7 @@ var _ = Describe("validateCELRulePayload", func() {
Expect(cs.validateCELRulePayload("test", payload)).To(Succeed())
})

It("rejects empty expression", func() {
It("accepts empty expression as manual rule", func() {
payload := &cmpv1alpha1.RulePayload{
Expression: "",
Inputs: []cmpv1alpha1.InputPayload{{
Expand All @@ -283,8 +278,7 @@ var _ = Describe("validateCELRulePayload", func() {
}},
}
err := cs.validateCELRulePayload("test", payload)
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("expression is empty"))
Expect(err).NotTo(HaveOccurred())
})

It("rejects no inputs", func() {
Expand Down
9 changes: 5 additions & 4 deletions coverage-baseline.txt
Original file line number Diff line number Diff line change
@@ -1,19 +1,20 @@
# Coverage baseline for compliance-operator
# Generated by: make update-coverage-baseline
# Do not edit manually.
github.com/ComplianceAsCode/compliance-operator/cmd/manager 24.2
github.com/ComplianceAsCode/compliance-operator/cmd/celctl 45.6
github.com/ComplianceAsCode/compliance-operator/cmd/manager 24.1
github.com/ComplianceAsCode/compliance-operator/pkg/apis/compliance/v1alpha1 7.5
github.com/ComplianceAsCode/compliance-operator/pkg/celcontent 83.3
github.com/ComplianceAsCode/compliance-operator/pkg/controller/common 33.7
github.com/ComplianceAsCode/compliance-operator/pkg/controller/complianceremediation 53.2
github.com/ComplianceAsCode/compliance-operator/pkg/controller/compliancescan 40.0
github.com/ComplianceAsCode/compliance-operator/pkg/controller/compliancesuite 19.3
github.com/ComplianceAsCode/compliance-operator/pkg/controller/compliancesuite 22.9
github.com/ComplianceAsCode/compliance-operator/pkg/controller/customrule 65.7
github.com/ComplianceAsCode/compliance-operator/pkg/controller/metrics 44.4
github.com/ComplianceAsCode/compliance-operator/pkg/controller/metrics 42.9
github.com/ComplianceAsCode/compliance-operator/pkg/controller/profilebundle 5.1
github.com/ComplianceAsCode/compliance-operator/pkg/controller/scansettingbinding 53.6
github.com/ComplianceAsCode/compliance-operator/pkg/controller/tailoredprofile 59.5
github.com/ComplianceAsCode/compliance-operator/pkg/profileparser 78.1
github.com/ComplianceAsCode/compliance-operator/pkg/profileparser 78.2
github.com/ComplianceAsCode/compliance-operator/pkg/utils 67.5
github.com/ComplianceAsCode/compliance-operator/pkg/utils/celvalidation 100.0
github.com/ComplianceAsCode/compliance-operator/pkg/xccdf 48.1
4 changes: 2 additions & 2 deletions pkg/celcontent/bundler.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,10 +142,10 @@ func loadRules(dir string) ([]CELRuleContent, error) {
if rule.Name == "" {
return nil, fmt.Errorf("rule in %s has no name", f)
}
if rule.Expression == "" {
if rule.CheckType != "Manual" && rule.Expression == "" {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Pasting here what I commented on ComplianceAsCode/content#14920

I don't agree with the Manual check type.

The check type is about what is being checked, whether the Kubernetes resource in the Platform or the file or service configuration in the Node.

A Manual rule is just missing the automated check.

return nil, fmt.Errorf("rule %q in %s has no expression", rule.Name, f)
}
if len(rule.Inputs) == 0 {
if rule.CheckType != "Manual" && len(rule.Inputs) == 0 {
return nil, fmt.Errorf("rule %q in %s has no inputs", rule.Name, f)
}
rules = append(rules, rule)
Expand Down
10 changes: 6 additions & 4 deletions pkg/profileparser/cel_content.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,10 +121,12 @@ func ParseCELBundle(celPath string, pb *cmpv1alpha1.ProfileBundle, pcfg *ParserC
Instructions: celRule.Instructions,
}

// Validate CEL expression at parse time
if err := celvalidation.ValidateCELRule(celRule.Name, &rulePayload); err != nil {
errChan <- fmt.Errorf("CEL rule '%s' validation failed: %w", celRule.Name, err)
return
// Validate CEL expression at parse time (skip for Manual rules)
if celRule.CheckType != "Manual" {
if err := celvalidation.ValidateCELRule(celRule.Name, &rulePayload); err != nil {
errChan <- fmt.Errorf("CEL rule '%s' validation failed: %w", celRule.Name, err)
return
}
}

annotations := map[string]string{
Expand Down
Loading