diff --git a/cmd/manager/cel-scanner.go b/cmd/manager/cel-scanner.go index bc7a856f73..8a2a65defe 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") + cmdLog.Info("Rule has no CEL expression, treating as manual rule", "rule", name) + 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..953bc6488b 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/celctl 45.6 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 -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..1f861078c0 100644 --- a/pkg/celcontent/bundler.go +++ b/pkg/celcontent/bundler.go @@ -142,6 +142,11 @@ func loadRules(dir string) ([]CELRuleContent, error) { if rule.Name == "" { return nil, fmt.Errorf("rule in %s has no name", f) } + 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) } 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 9e4d8eb357..b247c9fabb 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 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 + } } annotations := map[string]string{ 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())