From 056a1ba60bdaf4cf9faf96c92aa33b3b9d29e09b Mon Sep 17 00:00:00 2001 From: taimurhafeez Date: Fri, 31 Jul 2026 11:08:14 +0100 Subject: [PATCH 1/2] Add support for manual rules in CEL scanner Allow CEL profiles to include manual rules (checkType: Manual) that have no CEL expression. The bundler and profile parser skip validation for Manual rules, and the CEL scanner produces CheckResultManual directly instead of sending them to the SDK scanner. --- cmd/manager/cel-scanner.go | 47 +++++++++++++++++++++++--------- cmd/manager/cel_scanner_test.go | 30 ++++++++------------ coverage-baseline.txt | 9 +++--- pkg/celcontent/bundler.go | 4 +-- pkg/profileparser/cel_content.go | 10 ++++--- 5 files changed, 59 insertions(+), 41 deletions(-) diff --git a/cmd/manager/cel-scanner.go b/cmd/manager/cel-scanner.go index bc7a856f73..ddb7461330 100644 --- a/cmd/manager/cel-scanner.go +++ b/cmd/manager/cel-scanner.go @@ -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) @@ -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 { @@ -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 } if len(payload.Inputs) == 0 { diff --git a/cmd/manager/cel_scanner_test.go b/cmd/manager/cel_scanner_test.go index 89f17bb5b5..9a287fc6b8 100644 --- a/cmd/manager/cel_scanner_test.go +++ b/cmd/manager/cel_scanner_test.go @@ -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() { @@ -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{{ @@ -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() { diff --git a/coverage-baseline.txt b/coverage-baseline.txt index df8ab5b279..580028ca36 100644 --- a/coverage-baseline.txt +++ b/coverage-baseline.txt @@ -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 diff --git a/pkg/celcontent/bundler.go b/pkg/celcontent/bundler.go index 1b25f4bc59..a84dbe19db 100644 --- a/pkg/celcontent/bundler.go +++ b/pkg/celcontent/bundler.go @@ -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 == "" { 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) diff --git a/pkg/profileparser/cel_content.go b/pkg/profileparser/cel_content.go index 9e4d8eb357..6bdf13c5db 100644 --- a/pkg/profileparser/cel_content.go +++ b/pkg/profileparser/cel_content.go @@ -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{ From 364ae6be290b70209dcb3b534a60a78486d1df16 Mon Sep 17 00:00:00 2001 From: taimurhafeez Date: Tue, 4 Aug 2026 19:38:24 +0100 Subject: [PATCH 2/2] Replace checkType == Manual checks with expression-based detection: bundler accepts rules with both empty expression and inputs as manual, parser skips CEL validation when expression is empty, and scanner logs manual rule detection. Add unit tests for bundler and parser. --- cmd/manager/cel-scanner.go | 2 +- coverage-baseline.txt | 4 +- pkg/celcontent/bundler.go | 9 ++- pkg/celcontent/bundler_test.go | 67 +++++++++++++++++++++ pkg/profileparser/cel_content.go | 4 +- pkg/profileparser/cel_content_test.go | 86 +++++++++++++++++++++++++++ 6 files changed, 165 insertions(+), 7 deletions(-) diff --git a/cmd/manager/cel-scanner.go b/cmd/manager/cel-scanner.go index ddb7461330..8a2a65defe 100644 --- a/cmd/manager/cel-scanner.go +++ b/cmd/manager/cel-scanner.go @@ -757,7 +757,7 @@ 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 == "" { - // Manual rule — no expression to validate + cmdLog.Info("Rule has no CEL expression, treating as manual rule", "rule", name) return nil } diff --git a/coverage-baseline.txt b/coverage-baseline.txt index 580028ca36..953bc6488b 100644 --- a/coverage-baseline.txt +++ b/coverage-baseline.txt @@ -2,9 +2,9 @@ # Generated by: make update-coverage-baseline # Do not edit manually. github.com/ComplianceAsCode/compliance-operator/cmd/celctl 45.6 -github.com/ComplianceAsCode/compliance-operator/cmd/manager 24.1 +github.com/ComplianceAsCode/compliance-operator/cmd/manager 24.2 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/celcontent 84.0 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 diff --git a/pkg/celcontent/bundler.go b/pkg/celcontent/bundler.go index a84dbe19db..1f861078c0 100644 --- a/pkg/celcontent/bundler.go +++ b/pkg/celcontent/bundler.go @@ -142,10 +142,15 @@ func loadRules(dir string) ([]CELRuleContent, error) { if rule.Name == "" { return nil, fmt.Errorf("rule in %s has no name", f) } - if rule.CheckType != "Manual" && rule.Expression == "" { + if rule.Expression == "" && len(rule.Inputs) == 0 { + // Manual rule — no automated check, skip validation + rules = append(rules, rule) + continue + } + if rule.Expression == "" { return nil, fmt.Errorf("rule %q in %s has no expression", rule.Name, f) } - if rule.CheckType != "Manual" && len(rule.Inputs) == 0 { + if len(rule.Inputs) == 0 { return nil, fmt.Errorf("rule %q in %s has no inputs", rule.Name, f) } rules = append(rules, rule) diff --git a/pkg/celcontent/bundler_test.go b/pkg/celcontent/bundler_test.go index d0bfbc7e78..c83f533610 100644 --- a/pkg/celcontent/bundler_test.go +++ b/pkg/celcontent/bundler_test.go @@ -313,6 +313,73 @@ func TestBundleFromDirs_MissingFields(t *testing.T) { } } +func TestBundleFromDirs_ManualRule(t *testing.T) { + dir := t.TempDir() + rulesDir := filepath.Join(dir, "rules") + profilesDir := filepath.Join(dir, "profiles") + os.MkdirAll(rulesDir, 0755) + os.MkdirAll(profilesDir, 0755) + + celRuleYAML := `name: cel-rule +id: cel_rule +title: CEL Rule +severity: medium +checkType: Platform +expression: "x.items.size() > 0" +inputs: + - name: x + kubernetesInputSpec: + apiVersion: v1 + resource: pods +` + manualRuleYAML := `name: manual-rule +id: manual_rule +title: Manual Rule +severity: medium +checkType: Platform +` + os.WriteFile(filepath.Join(rulesDir, "cel.yaml"), []byte(celRuleYAML), 0644) + os.WriteFile(filepath.Join(rulesDir, "manual.yaml"), []byte(manualRuleYAML), 0644) + + profileYAML := `name: p +id: p_id +title: P +rules: + - cel-rule + - manual-rule +` + os.WriteFile(filepath.Join(profilesDir, "p.yaml"), []byte(profileYAML), 0644) + + bundle, err := BundleFromDirs(rulesDir, profilesDir) + if err != nil { + t.Fatalf("BundleFromDirs failed: %v", err) + } + if len(bundle.Rules) != 2 { + t.Fatalf("Expected 2 rules (CEL + manual), got %d", len(bundle.Rules)) + } + + ruleMap := make(map[string]CELRuleContent) + for _, r := range bundle.Rules { + ruleMap[r.Name] = r + } + + celRule := ruleMap["cel-rule"] + if celRule.Expression == "" { + t.Error("CEL rule should have expression") + } + if len(celRule.Inputs) == 0 { + t.Error("CEL rule should have inputs") + } + + manualRule := ruleMap["manual-rule"] + if manualRule.Expression != "" { + t.Errorf("Manual rule should have empty expression, got %q", manualRule.Expression) + } + if len(manualRule.Inputs) != 0 { + t.Errorf("Manual rule should have no inputs, got %d", len(manualRule.Inputs)) + } +} + func TestBundleFromDirs_EmptyProfile(t *testing.T) { dir := t.TempDir() rulesDir := filepath.Join(dir, "rules") diff --git a/pkg/profileparser/cel_content.go b/pkg/profileparser/cel_content.go index 6bdf13c5db..b247c9fabb 100644 --- a/pkg/profileparser/cel_content.go +++ b/pkg/profileparser/cel_content.go @@ -121,8 +121,8 @@ func ParseCELBundle(celPath string, pb *cmpv1alpha1.ProfileBundle, pcfg *ParserC Instructions: celRule.Instructions, } - // Validate CEL expression at parse time (skip for Manual rules) - if celRule.CheckType != "Manual" { + // Validate CEL expression at parse time (skip for rules without expressions) + if rulePayload.Expression != "" { if err := celvalidation.ValidateCELRule(celRule.Name, &rulePayload); err != nil { errChan <- fmt.Errorf("CEL rule '%s' validation failed: %w", celRule.Name, err) return diff --git a/pkg/profileparser/cel_content_test.go b/pkg/profileparser/cel_content_test.go index a4c05fc688..cb062abc50 100644 --- a/pkg/profileparser/cel_content_test.go +++ b/pkg/profileparser/cel_content_test.go @@ -384,6 +384,92 @@ func TestCELBundleCISVMExtension(t *testing.T) { var _ = Describe("ParseCELBundle integration", func() { const pbName = "cel-e2e-pb" + It("accepts manual rules without expressions and creates Rule CRs", func() { + bundleYAML := `rules: + - name: cel-rule + id: cel_rule + title: CEL Rule + description: A rule with CEL checks + rationale: Testing + severity: medium + checkType: Platform + expression: "pods.items.size() > 0" + inputs: + - name: pods + kubernetesInputSpec: + apiVersion: v1 + resource: pods + - name: manual-rule + id: manual_rule + title: Manual Rule + description: A rule without automated checks + rationale: Requires manual verification + severity: medium + checkType: Platform + instructions: Run oc adm policy who-can create vmim +profiles: + - name: test-profile + id: test_profile + title: Test Profile + productType: Platform + rules: + - cel-rule + - manual-rule +` + outPath := filepath.Join(GinkgoT().TempDir(), "manual-bundle.yaml") + Expect(os.WriteFile(outPath, []byte(bundleYAML), 0644)).To(Succeed()) + + pb := &cmpv1alpha1.ProfileBundle{ + ObjectMeta: metav1.ObjectMeta{ + Name: pbName, + Namespace: testNamespace, + }, + } + Expect(client.Create(context.TODO(), pb)).To(Succeed()) + defer client.Delete(context.TODO(), pb) + + pcfg := &ParserConfig{ + Client: client, + Scheme: client.Scheme(), + } + + err := ParseCELBundle(outPath, pb, pcfg) + Expect(err).NotTo(HaveOccurred()) + + // Verify CEL rule CR was created with expression + celRule := &cmpv1alpha1.Rule{} + Expect(client.Get(context.TODO(), types.NamespacedName{ + Name: GetPrefixedName(pbName, "cel-rule"), + Namespace: testNamespace, + }, celRule)).To(Succeed()) + Expect(celRule.RulePayload.Expression).NotTo(BeEmpty()) + Expect(celRule.RulePayload.Inputs).To(HaveLen(1)) + + // Verify manual rule CR was created without expression + manualRule := &cmpv1alpha1.Rule{} + Expect(client.Get(context.TODO(), types.NamespacedName{ + Name: GetPrefixedName(pbName, "manual-rule"), + Namespace: testNamespace, + }, manualRule)).To(Succeed()) + Expect(manualRule.RulePayload.Expression).To(BeEmpty()) + Expect(manualRule.RulePayload.Inputs).To(BeEmpty()) + Expect(manualRule.RulePayload.Instructions).To(ContainSubstring("who-can")) + Expect(manualRule.RulePayload.ScannerType).To(Equal(cmpv1alpha1.ScannerTypeCEL)) + + // Verify profile references both rules + profile := &cmpv1alpha1.Profile{} + Expect(client.Get(context.TODO(), types.NamespacedName{ + Name: GetPrefixedName(pbName, "test-profile"), + Namespace: testNamespace, + }, profile)).To(Succeed()) + Expect(profile.Rules).To(HaveLen(2)) + + // Cleanup + client.Delete(context.TODO(), celRule) + client.Delete(context.TODO(), manualRule) + client.Delete(context.TODO(), profile) + }) + It("creates Rule and Profile CRs from bundler-generated file", func() { outPath := filepath.Join(GinkgoT().TempDir(), "cel-bundle.yaml") Expect(celcontent.BundleToFile(celTestRulesDir, celTestProfilesDir, outPath)).To(Succeed())