Skip to content

Conversation

@Acepresso
Copy link
Contributor

@Acepresso Acepresso commented Dec 3, 2025

User description

Add an optional componentNames field to allow filtering volatile config include/exclude rules by component name from ApplicationSnapshot. This allows filtering in scenarios where multiple components share the same image repository.

Example usage:

  volatileConfig:
    exclude:
      - value: "lint.has_failures"
        componentNames: ["component1", "component2"]

** Depends on: conforma/crds#21 to be merged first (updated ECP CRD with the new ComponentNames field).

Ref: https://issues.redhat.com/browse/EC-1513
Assisted-by: Cursor (using claude-4.5-sonnet)


PR Type

Enhancement


Description

  • Add ComponentName field to EvaluationTarget for component-specific filtering

  • Implement componentItems map in Criteria to store component-name-based rules

  • Update Criteria.get() method to merge component-specific and image-specific rules

  • Pass component name through evaluation pipeline to filters and matchers

  • Add comprehensive tests for component-based volatile config filtering


Diagram Walkthrough

flowchart LR
  A["EvaluationTarget<br/>+ ComponentName"] -->|passes| B["Criteria.get<br/>imageRef, componentName"]
  B -->|merges| C["componentItems<br/>+ imageRef items<br/>+ defaults"]
  C -->|filters| D["FilterResults<br/>with component context"]
  D -->|applies| E["Include/Exclude<br/>Rules"]
  E -->|produces| F["Filtered<br/>Outcomes"]
Loading

File Walkthrough

Relevant files
Enhancement
5 files
evaluator.go
Add ComponentName field to EvaluationTarget struct             
+3/-2     
criteria.go
Implement component-based filtering in Criteria                   
+32/-7   
conftest_evaluator.go
Pass component name through evaluation pipeline                   
+9/-8     
filters.go
Update filter signatures to accept component name               
+16/-13 
validate.go
Extract and pass component name to evaluator                         
+4/-1     
Tests
4 files
criteria_test.go
Add comprehensive tests for component filtering                   
+615/-31
conftest_evaluator_integration_basic_test.go
Add integration test for component-based exclusions           
+201/-0 
filters_test.go
Update filter tests with component name parameter               
+5/-5     
validate_test.go
Add test verifying component name propagation                       
+61/-0   

Add optional componentNames field to allow filtering volatile config
include/exclude rules by component name from ApplicationSnapshot.
This allows filtering in scenarios where multiple components share the
same image repository.

Example usage:

```
  volatileConfig:
    exclude:
      - value: "lint.has_failures"
        componentNames: ["component1", "component2"]
```

**Depends on: github.com/conforma/crds with ComponentNames field

Ref: https://issues.redhat.com/browse/EC-1513
Assisted-by: Cursor (using claude-4.5-sonnet)
@Acepresso Acepresso force-pushed the exclude-components-EC-1513 branch from b410dce to 207bd33 Compare December 4, 2025 13:31
@Acepresso Acepresso marked this pull request as ready for review December 4, 2025 13:37
@qodo-code-review
Copy link

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Audit Logging: The new component-based filtering logic and evaluation path do not include any explicit
audit logging of filter decisions or component context, which may be acceptable if handled
elsewhere but is not visible in the added code.

Referred Code
// Debug: Print all failures
t.Logf("comp1 results: %d outcomes", len(result1))
for i, outcome := range result1 {
	t.Logf("  Outcome %d: %d failures, %d successes", i, len(outcome.Failures), len(outcome.Successes))
	for _, failure := range outcome.Failures {
		t.Logf("    Failure: %s", failure.Metadata["code"])
	}
	for _, success := range outcome.Successes {
		t.Logf("    Success: %s", success.Metadata["code"])
	}
}

// Verify check_a is excluded, check_b is not
hasCheckA := false
hasCheckB := false
for _, outcome := range result1 {
	for _, failure := range outcome.Failures {
		if codeStr, ok := failure.Metadata["code"].(string); ok {
			if codeStr == "test.check_a" {
				hasCheckA = true
			}


 ... (clipped 6 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status:
Log Sensitivity: Debug logs added for skipping results reference entire success/failure objects and codes,
which may be acceptable but should be reviewed to ensure no sensitive data is logged at
debug level.

Referred Code
	if len(filteredResults) == 0 {
		log.Debugf("Skipping result success: %#v", success)
		continue
	}
} else {
	// Fallback to legacy filtering for backward compatibility
	if !c.isResultIncluded(success, imageRef, componentName, missingIncludes) {
		log.Debugf("Skipping result success: %#v", success)
		continue

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Component filtering is not implemented for all filter types

The component-based filtering logic is missing from the ECPolicyResolver, which
handles pipeline intention filtering. To fix this, update the ResolvePolicy
method and related functions in filters.go to accept and utilize the
componentName.

Examples:

internal/evaluator/filters.go [522-524]
func (r *ECPolicyResolver) ResolvePolicy(rules policyRules, target string) PolicyResolutionResult {
	return r.baseResolvePolicy(rules, target, r.processPackage)
}
internal/evaluator/filters.go [1033-1035]
	if ecResolver, ok := f.policyResolver.(*ECPolicyResolver); ok {
		// Use policy resolution for ECPolicyResolver to handle pipeline intentions
		policyResolution := ecResolver.ResolvePolicy(rules, imageRef)

Solution Walkthrough:

Before:

// internal/evaluator/filters.go

func (r *ECPolicyResolver) ResolvePolicy(rules policyRules, target string) PolicyResolutionResult {
  // componentName is not available here
  return r.baseResolvePolicy(rules, target, r.processPackage)
}

func (r *basePolicyResolver) baseResolvePolicy(rules policyRules, target string, ...) PolicyResolutionResult {
  // ...
  // componentName is hardcoded to ""
  for _, include := range r.include.get(target, "") {
    // ...
  }
  // ...
}

After:

// internal/evaluator/filters.go

func (r *ECPolicyResolver) ResolvePolicy(rules policyRules, imageRef, componentName string) PolicyResolutionResult {
  // componentName is now available
  return r.baseResolvePolicy(rules, imageRef, componentName, r.processPackage)
}

func (r *basePolicyResolver) baseResolvePolicy(rules policyRules, imageRef, componentName string, ...) PolicyResolutionResult {
  // ...
  // componentName is now passed through
  for _, include := range r.include.get(imageRef, componentName) {
    // ...
  }
  // ...
}
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies a significant flaw where the new component-based filtering is not implemented for the ECPolicyResolver path, causing the feature to fail silently when pipeline intentions are used.

High
Possible issue
Apply component criteria on invalid image

Modify the get function to prevent an early return when imageRef parsing fails.
This ensures that component-specific and default criteria are still applied even
if the image reference is invalid.

internal/evaluator/criteria.go [82-113]

 func (c *Criteria) get(imageRef string, componentName string) []string {
+	var items []string
+
 	ref, err := name.ParseReference(imageRef)
 	if err != nil {
 		log.Debugf("error parsing target image url: %q", imageRef)
-		// Return only global defaults if image ref is invalid
-		return c.defaultItems
-	}
+	} else {
+		// Collect keys to look up: always the repository name,
+		// and if available, the digest string.
+		keys := []string{ref.Context().Name()}
+		if digestRef, ok := ref.(name.Digest); ok {
+			keys = append(keys, digestRef.DigestStr())
+		} else {
+			log.Debugf("no digest found for reference: %q", ref)
+		}
 
-	// Collect keys to look up: always the repository name,
-	// and if available, the digest string.
-	keys := []string{ref.Context().Name()}
-	if digestRef, ok := ref.(name.Digest); ok {
-		keys = append(keys, digestRef.DigestStr())
-	} else {
-		log.Debugf("no digest found for reference: %q", ref)
-	}
-
-	var items []string
-	for _, k := range keys {
-		items = append(items, c.getWithKey(k)...)
+		for _, k := range keys {
+			items = append(items, c.getWithKey(k)...)
+		}
 	}
 
 	// Add component-specific items if component name is provided
 	if componentName != "" {
 		if componentItems, ok := c.componentItems[componentName]; ok {
 			items = append(items, componentItems...)
 		}
 	}
 
 	// Add any exceptions that pertain to all images.
 	return append(items, c.defaultItems...)
 }
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies a logic flaw in the new get function where an invalid imageRef causes an early return, skipping the newly added component-specific criteria. This fix is important for ensuring component-based rules are applied consistently, even with image parsing errors.

Medium
  • More

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant