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   

@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
Contributor

qodo-code-review bot commented Dec 4, 2025

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

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

@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Dec 4, 2025

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
  • Update

@Acepresso Acepresso force-pushed the exclude-components-EC-1513 branch from 207bd33 to 2d7c700 Compare December 9, 2025 13:15
@Acepresso
Copy link
Contributor Author

The CI tests should pass once the dependent PR that updates the CRD is merged.

@Acepresso Acepresso force-pushed the exclude-components-EC-1513 branch from 2d7c700 to 23adb75 Compare December 11, 2025 13:06
@codecov
Copy link

codecov bot commented Dec 11, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.

Flag Coverage Δ
acceptance 55.80% <57.14%> (-0.03%) ⬇️
generative 18.97% <0.00%> (-0.03%) ⬇️
integration 28.48% <68.57%> (+0.55%) ⬆️
unit 67.73% <94.28%> (+0.12%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
internal/evaluator/conftest_evaluator.go 88.65% <100.00%> (+1.54%) ⬆️
internal/evaluator/criteria.go 96.70% <100.00%> (+1.76%) ⬆️
internal/evaluator/filters.go 82.63% <100.00%> (ø)
internal/image/validate.go 70.14% <100.00%> (+0.45%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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 ec93ad1 to 67d081b Compare December 14, 2025 14:47
@Acepresso
Copy link
Contributor Author

Rebased and pushed fixes to address the reduction of code coverage.

Copy link
Contributor

@st3penta st3penta left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Member

@simonbaird simonbaird left a comment

Choose a reason for hiding this comment

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

Lgtm.

@Acepresso Acepresso merged commit 4dd2ecf into conforma:main Dec 16, 2025
13 checks passed
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.

3 participants