Fix inconsistent violation reporting

Walk each data source directory returned by GetPolicy
These are the actual directories (possibly symlinks) where data was downloaded
Walking them directly ensures we find files even if they're symlinks

Author: joejstuart

cmd/validate/vsa.go

@@ -947,51 +947,6 @@ func displayComponentResults(allResults []vsa.ComponentResult, data *validateVSA
 	return nil
 }
 
-// displayComponentResultsOld displays the validation results for each component (old implementation)
-// Kept temporarily for reference - will be removed after new implementation is verified
-func displayComponentResultsOld(allResults []vsa.ComponentResult, data *validateVSAData) {
-	var successCount, failureCount, fallbackCount int
-
-	printVSAInfo(os.Stdout, "\n=== Component Validation Results ===")
-	for _, result := range allResults {
-		printVSAInfo(os.Stdout, fmt.Sprintf("\nComponent: %s", result.ComponentName))
-		printVSAInfo(os.Stdout, fmt.Sprintf("  Image: %s", result.ImageRef))
-
-		resultType := classifyResult(result)
-		switch resultType {
-		case ResultTypeError:
-			failureCount++
-			printVSAStatus(os.Stdout, fmt.Sprintf("Failed: %v", result.Error), "failure")
-		case ResultTypeFallback:
-			fallbackCount++
-			if result.FallbackResult != nil && result.FallbackResult.Component.Success {
-				successCount++
-			} else {
-				failureCount++
-			}
-			displayFallbackResult(result, data)
-		case ResultTypeVSASuccess:
-			successCount++
-			displayVSASuccessResult(result, data)
-		case ResultTypeVSAFailure:
-			failureCount++
-			displayVSAFailureResult(result, data)
-		case ResultTypeUnexpected:
-			failureCount++
-			printVSAStatus(os.Stdout, fmt.Sprintf("  Unexpected state: no validation result for component %s", result.ComponentName), "failure")
-		}
-	}
-
-	// Print summary statistics
-	printVSAInfo(os.Stdout, "\n=== Snapshot Validation Summary ===")
-	printVSAInfo(os.Stdout, fmt.Sprintf("Total components: %d", len(allResults)))
-	printVSAInfo(os.Stdout, fmt.Sprintf("Successful: %d", successCount))
-	printVSAInfo(os.Stdout, fmt.Sprintf("Failed: %d", failureCount))
-	if fallbackCount > 0 {
-		printVSAInfo(os.Stdout, fmt.Sprintf("Used fallback: %d", fallbackCount))
-	}
-}
-
 // displayFallbackResult displays fallback validation results
 // It shows VSA result first (why fallback was triggered), then the fallback image validation result
 func displayFallbackResult(result vsa.ComponentResult, data *validateVSAData) {
@@ -1204,12 +1159,15 @@ func validateImageFallbackWithWorkerContext(ctx context.Context, data *validateV
 	// Perform image validation using precomputed context and worker-specific evaluators
 	log.Debugf("🔄 Fallback: Starting image validation...")
 	log.Debugf("🔄 Fallback: Using fallback policy: %s", data.fallbackContext.FallbackPolicy)
+	fmt.Printf("Component: %+v\n", comp.ContainerImage)
 	result, err := image.ValidateImage(ctx, comp, data.snapshot, data.fallbackContext.FallbackPolicy, workerFallbackContext.Evaluators, data.info)
 	if err != nil {
 		log.Debugf("🔄 Fallback: Image validation failed with error: %v", err)
 		return nil, err
 	}
 
+	fmt.Printf("Component: %+v, violations: %+v\n", comp.ContainerImage, len(result.Violations()))
+
 	return result, nil
 }
 

internal/evaluator/conftest_evaluator.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"io/fs"
 	"net/url"
 	"os"
 	"path"
@@ -441,6 +442,8 @@ func (c conftestEvaluator) Evaluate(ctx context.Context, target EvaluationTarget
 	rules := policyRules{}
 	// Track non-annotated rules separately for filtering purposes only
 	nonAnnotatedRules := nonAnnotatedRules{}
+	// Track data source directories for prepareDataDirs
+	dataSourceDirs := []string{}
 	// Download all sources
 	for _, s := range c.policySources {
 		dir, err := s.GetPolicy(ctx, c.workDir, false)
@@ -449,6 +452,11 @@ func (c conftestEvaluator) Evaluate(ctx context.Context, target EvaluationTarget
 			// TODO do we want to download other policies instead of erroring out?
 			return nil, err
 		}
+		// Track data source directories - these are the actual directories returned by GetPolicy
+		// which may be symlinks, so we need to use them directly rather than walking
+		if s.Subdir() == "data" {
+			dataSourceDirs = append(dataSourceDirs, dir)
+		}
 		annotations := []*ast.AnnotationsRef{}
 		fs := utils.FS(ctx)
 		// We only want to inspect the directory of policy subdirs, not config or data subdirs.
@@ -584,13 +592,11 @@ func (c conftestEvaluator) Evaluate(ctx context.Context, target EvaluationTarget
 		log.Debugf("Namespaces to use: %v, allNamespaces: %v", namespacesToUse, allNamespaces)
 
 		// Prepare the list of data dirs
-		dataDirs, err := c.prepareDataDirs(ctx)
+		dataDirs, err := c.prepareDataDirs(ctx, dataSourceDirs)
 		if err != nil {
 			return nil, err
 		}
 
-		log.Debugf("Data dirs: %v", dataDirs)
-
 		r = &conftestRunner{
 			runner.TestRunner{
 				Data:          dataDirs,
@@ -700,15 +706,54 @@ func (c conftestEvaluator) Evaluate(ctx context.Context, target EvaluationTarget
 	return results, nil
 }
 
-// prepareDataDirs inspects the top level data dir and returns a list of the directories
-// that appear to have data files in them. That list will be passed to the conftest runner.
-func (c conftestEvaluator) prepareDataDirs(ctx context.Context) ([]string, error) {
+// prepareDataDirs inspects the data source directories and base data dir to return a list of
+// directories that contain data files. That list will be passed to the conftest runner.
+// dataSourceDirs contains the directories returned by GetPolicy for data sources.
+// These directories may be symlinks (from cached downloads), but we walk them directly
+// which ensures we find the files regardless of whether they're symlinks or not.
+func (c conftestEvaluator) prepareDataDirs(ctx context.Context, dataSourceDirs []string) ([]string, error) {
 	// The reason we do this is to avoid having the names of the subdirs under c.dataDir
 	// converted to keys in the data structure. We want the top level keys in the data files
 	// to be at the top level of the data structure visible to the rego rules.
 
 	dirsWithDataFiles := make(map[string]bool)
-	err := afero.Walk(c.fs, c.dataDir, func(path string, info os.FileInfo, err error) error {
+
+	// Walk each data source directory returned by GetPolicy
+	// These are the actual directories (possibly symlinks) where data was downloaded
+	// Walking them directly ensures we find files even if they're symlinks
+	for _, dataSourceDir := range dataSourceDirs {
+		// IMPORTANT: Use fs.WalkDir instead of afero.Walk because afero.Walk does not follow symlinks.
+		// When cached downloads create symlinks (as in getPolicyThroughCache), afero.Walk won't traverse
+		// into the symlinked directories, causing data files to be missed.
+		// See afero issue https://github.com/spf13/afero/issues/284 and internal/opa/inspect.go for reference.
+		err := fs.WalkDir(opaWrapperFs{afs: c.fs}, dataSourceDir, func(path string, d fs.DirEntry, err error) error {
+			if err != nil {
+				return err
+			}
+
+			// Only process files, not directories
+			if !d.IsDir() {
+				ext := filepath.Ext(d.Name())
+				// Check if this is a data file (.json, .yaml, .yml)
+				// Todo: Should probably recognize other supported types of data
+				if ext == ".json" || ext == ".yaml" || ext == ".yml" {
+					// Mark the directory containing this file as having data
+					dir := filepath.Dir(path)
+					dirsWithDataFiles[dir] = true
+				}
+			}
+
+			return nil
+		})
+		if err != nil {
+			// Continue with other directories even if one fails
+			continue
+		}
+	}
+
+	// Also walk the base data directory to find config.json and any other files
+	// This ensures we don't miss files that weren't from GetPolicy sources
+	err := fs.WalkDir(opaWrapperFs{afs: c.fs}, c.dataDir, func(path string, d fs.DirEntry, err error) error {
 		if err != nil {
 			return err
 		}
@@ -719,10 +764,9 @@ func (c conftestEvaluator) prepareDataDirs(ctx context.Context) ([]string, error
 		}
 
 		// Only process files, not directories
-		if !info.IsDir() {
-			ext := filepath.Ext(info.Name())
+		if !d.IsDir() {
+			ext := filepath.Ext(d.Name())
 			// Check if this is a data file (.json, .yaml, .yml)
-			// Todo: Should probably recognize other supported types of data
 			if ext == ".json" || ext == ".yaml" || ext == ".yml" {
 				// Mark the directory containing this file as having data
 				dir := filepath.Dir(path)
@@ -1228,3 +1272,15 @@ func extractCodeFromRuleBody(ruleRef *ast.Rule) string {
 
 	return ""
 }
+
+// opaWrapperFs wraps afero.Fs to implement fs.FS interface for use with fs.WalkDir.
+// This ensures symlinks are followed when walking directories, which is critical
+// when cached downloads create symlinks via getPolicyThroughCache.
+// See internal/opa/inspect.go for the original implementation.
+type opaWrapperFs struct {
+	afs afero.Fs
+}
+
+func (w opaWrapperFs) Open(name string) (fs.File, error) {
+	return w.afs.Open(name)
+}