Merge pull request #3018 from joejstuart/fix-fallback-results

Results difference

Author: joejstuart

cmd/validate/vsa.go

@@ -452,32 +452,18 @@ func performVSAValidationForSingle(ctx context.Context, identifier string, data
 }
 
 // handleFallbackValidation handles the fallback validation logic
-func handleFallbackValidation(ctx context.Context, identifier string, result *vsa.ValidationResult, data *validateVSAData, fs afero.Fs) error {
+func handleFallbackValidation(ctx context.Context, imageRef string, result *vsa.ValidationResult, data *validateVSAData, fs afero.Fs) error {
 	printVSAInfo(os.Stdout, "Falling back to image validation...")
-
-	// Extract image reference from VSA identifier for fallback
-	imageRef, extractErr := vsa.ExtractImageFromVSAIdentifier(identifier)
-	if extractErr != nil {
-		return fmt.Errorf("fallback validation not supported for file paths: %s", identifier)
-	}
+	printVSAInfo(os.Stdout, fmt.Sprintf("Image reference: %s", imageRef))
 
 	// Create worker context for fallback validation
 	workerFallbackContext, workerErr := vsa.CreateWorkerFallbackContext(ctx, data.fallbackContext.FallbackPolicy)
 	if workerErr != nil {
 		return fmt.Errorf("failed to create fallback context: %w", workerErr)
 	}
 
-	// Create fallback config
-	fallbackConfig := &vsa.FallbackConfig{
-		FallbackToImageValidation: data.fallbackToImageValidation,
-		FallbackPublicKey:         data.fallbackPublicKey,
-		PolicyConfig:              data.policyConfig,
-		EffectiveTime:             data.effectiveTime,
-		Info:                      data.info,
-	}
-
 	// Use the common fallback validation logic
-	fallbackResult := vsa.PerformFallbackValidation(ctx, fallbackConfig, data.fallbackContext, imageRef, "single-vsa-component", result, "", workerFallbackContext)
+	fallbackResult := vsa.PerformFallbackValidation(result, "")
 	if fallbackResult.Error != nil {
 		return fallbackResult.Error
 	}
@@ -831,6 +817,7 @@ 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)
 	result, err := image.ValidateImage(ctx, comp, spec, data.fallbackContext.FallbackPolicy, workerFallbackContext.Evaluators, data.info)
 	if err != nil {
 		log.Debugf("🔄 Fallback: Image validation failed with error: %v", err)
@@ -910,17 +897,8 @@ func handleComponentFallback(ctx context.Context, component app.SnapshotComponen
 		predicateStatus = result.PredicateOutcome
 	}
 
-	// Create fallback config
-	fallbackConfig := &vsa.FallbackConfig{
-		FallbackToImageValidation: data.fallbackToImageValidation,
-		FallbackPublicKey:         data.fallbackPublicKey,
-		PolicyConfig:              data.policyConfig,
-		EffectiveTime:             data.effectiveTime,
-		Info:                      data.info,
-	}
-
 	// Use the common fallback validation logic
-	fallbackResult := vsa.PerformFallbackValidation(ctx, fallbackConfig, data.fallbackContext, imageRef, component.Name, result, predicateStatus, workerFallbackContext)
+	fallbackResult := vsa.PerformFallbackValidation(result, predicateStatus)
 	if fallbackResult.Error != nil {
 		return createErrorResult(component, fallbackResult.Error)
 	}

cmd/validate/vsa_test.go

@@ -35,7 +35,6 @@ import (
 	"github.com/spf13/cobra"
 	"github.com/stretchr/testify/assert"
 
-	"github.com/conforma/cli/internal/evaluator"
 	"github.com/conforma/cli/internal/output"
 	"github.com/conforma/cli/internal/validate/vsa"
 )
@@ -1779,25 +1778,15 @@ func (m *mockOutputFormatter) PrintText(writer io.Writer) error {
 
 // TestPerformFallbackValidation tests the extracted fallback validation function
 func TestPerformFallbackValidation(t *testing.T) {
-	ctx := context.Background()
-
-	// Create a mock worker fallback context
-	workerContext := &vsa.WorkerFallbackContext{
-		Evaluators: []evaluator.Evaluator{}, // Empty for testing
-	}
 
 	tests := []struct {
 		name            string
-		imageRef        string
-		componentName   string
 		result          *vsa.ValidationResult
 		predicateStatus string
 		expectError     bool
 	}{
 		{
-			name:          "successful fallback with VSA result",
-			imageRef:      "test-image:latest",
-			componentName: "test-component",
+			name: "successful fallback with VSA result",
 			result: &vsa.ValidationResult{
 				Passed:  false,
 				Message: "VSA validation failed",
@@ -1807,16 +1796,12 @@ func TestPerformFallbackValidation(t *testing.T) {
 		},
 		{
 			name:            "successful fallback without VSA result",
-			imageRef:        "test-image:latest",
-			componentName:   "test-component",
 			result:          nil,
 			predicateStatus: "failed",
 			expectError:     false,
 		},
 		{
 			name:            "fallback with empty predicate status",
-			imageRef:        "test-image:latest",
-			componentName:   "test-component",
 			result:          nil,
 			predicateStatus: "",
 			expectError:     false,
@@ -1827,16 +1812,7 @@ func TestPerformFallbackValidation(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			// Note: This test will fail in practice because it requires actual
 			// fallback context and evaluators, but it tests the function structure
-			// Create fallback config
-			fallbackConfig := &vsa.FallbackConfig{
-				FallbackToImageValidation: true,
-				FallbackPublicKey:         "test-key",
-				PolicyConfig:              "test-policy",
-				EffectiveTime:             "2023-01-01T00:00:00Z",
-				Info:                      false,
-			}
-
-			fallbackResult := vsa.PerformFallbackValidation(ctx, fallbackConfig, nil, tt.imageRef, tt.componentName, tt.result, tt.predicateStatus, workerContext)
+			fallbackResult := vsa.PerformFallbackValidation(tt.result, tt.predicateStatus)
 
 			if tt.expectError {
 				assert.Error(t, fallbackResult.Error)
@@ -1852,19 +1828,8 @@ func TestPerformFallbackValidation(t *testing.T) {
 
 // TestPerformFallbackValidation_ErrorHandling tests error handling in fallback validation
 func TestPerformFallbackValidation_ErrorHandling(t *testing.T) {
-	ctx := context.Background()
-
 	// Test with nil worker context - should not cause error as function only handles VSA result logic
-	// Create fallback config
-	fallbackConfig := &vsa.FallbackConfig{
-		FallbackToImageValidation: true,
-		FallbackPublicKey:         "test-key",
-		PolicyConfig:              "test-policy",
-		EffectiveTime:             "2023-01-01T00:00:00Z",
-		Info:                      false,
-	}
-
-	fallbackResult := vsa.PerformFallbackValidation(ctx, fallbackConfig, nil, "test-image:latest", "test-component", nil, "failed", nil)
+	fallbackResult := vsa.PerformFallbackValidation(nil, "failed")
 	// The function now only handles VSA result logic, so it should not return an error
 	assert.NoError(t, fallbackResult.Error)
 	assert.NotNil(t, fallbackResult.VSAResult)

internal/validate/vsa/fallback.go

@@ -83,7 +83,7 @@ func ShouldTriggerFallback(err error, result *ValidationResult) bool {
 
 // performFallbackValidation performs the common fallback validation logic
 // Note: This function now only handles the VSA result logic, image validation is handled in CLI layer
-func PerformFallbackValidation(ctx context.Context, config *FallbackConfig, fallbackContext *FallbackValidationContext, imageRef string, componentName string, result *ValidationResult, predicateStatus string, workerFallbackContext *WorkerFallbackContext) *FallbackResult {
+func PerformFallbackValidation(result *ValidationResult, predicateStatus string) *FallbackResult {
 	// Use the actual VSA result for fallback case
 	// If we have a result, use it; otherwise create a minimal result
 	var vsaResult *ValidationResult

internal/validate/vsa/fallback_test.go

@@ -113,44 +113,22 @@ func TestShouldTriggerFallback(t *testing.T) {
 }
 
 func TestPerformFallbackValidation(t *testing.T) {
-	ctx := context.Background()
-
 	tests := []struct {
-		name                  string
-		config                *FallbackConfig
-		fallbackContext       *FallbackValidationContext
-		imageRef              string
-		componentName         string
-		result                *ValidationResult
-		predicateStatus       string
-		workerFallbackContext *WorkerFallbackContext
-		expectedVSAResult     *ValidationResult
-		expectedError         error
+		name              string
+		result            *ValidationResult
+		predicateStatus   string
+		expectedVSAResult *ValidationResult
+		expectedError     error
 	}{
 		{
 			name: "With VSA result - should use provided result",
-			config: &FallbackConfig{
-				FallbackToImageValidation: true,
-				FallbackPublicKey:         "test-key",
-				PolicyConfig:              "test-policy",
-				EffectiveTime:             "2023-01-01T00:00:00Z",
-				Info:                      false,
-			},
-			fallbackContext: &FallbackValidationContext{
-				PolicyConfiguration: "test-policy",
-			},
-			imageRef:      "test-image:latest",
-			componentName: "test-component",
 			result: &ValidationResult{
 				Passed:            false,
 				Message:           "VSA validation failed",
 				SignatureVerified: false,
 				PredicateOutcome:  "failed",
 			},
 			predicateStatus: "failed",
-			workerFallbackContext: &WorkerFallbackContext{
-				Evaluators: []evaluator.Evaluator{},
-			},
 			expectedVSAResult: &ValidationResult{
 				Passed:            false,
 				Message:           "VSA validation failed",
@@ -160,24 +138,9 @@ func TestPerformFallbackValidation(t *testing.T) {
 			expectedError: nil,
 		},
 		{
-			name: "Without VSA result - should create minimal result",
-			config: &FallbackConfig{
-				FallbackToImageValidation: true,
-				FallbackPublicKey:         "test-key",
-				PolicyConfig:              "test-policy",
-				EffectiveTime:             "2023-01-01T00:00:00Z",
-				Info:                      false,
-			},
-			fallbackContext: &FallbackValidationContext{
-				PolicyConfiguration: "test-policy",
-			},
-			imageRef:        "test-image:latest",
-			componentName:   "test-component",
+			name:            "Without VSA result - should create minimal result",
 			result:          nil, // No VSA result
 			predicateStatus: "failed",
-			workerFallbackContext: &WorkerFallbackContext{
-				Evaluators: []evaluator.Evaluator{},
-			},
 			expectedVSAResult: &ValidationResult{
 				Passed:            false,
 				Message:           "VSA validation failed",
@@ -188,28 +151,13 @@ func TestPerformFallbackValidation(t *testing.T) {
 		},
 		{
 			name: "With successful VSA result - should use provided result",
-			config: &FallbackConfig{
-				FallbackToImageValidation: true,
-				FallbackPublicKey:         "test-key",
-				PolicyConfig:              "test-policy",
-				EffectiveTime:             "2023-01-01T00:00:00Z",
-				Info:                      false,
-			},
-			fallbackContext: &FallbackValidationContext{
-				PolicyConfiguration: "test-policy",
-			},
-			imageRef:      "test-image:latest",
-			componentName: "test-component",
 			result: &ValidationResult{
 				Passed:            true,
 				Message:           "VSA validation passed",
 				SignatureVerified: true,
 				PredicateOutcome:  "passed",
 			},
 			predicateStatus: "passed",
-			workerFallbackContext: &WorkerFallbackContext{
-				Evaluators: []evaluator.Evaluator{},
-			},
 			expectedVSAResult: &ValidationResult{
 				Passed:            true,
 				Message:           "VSA validation passed",
@@ -223,14 +171,8 @@ func TestPerformFallbackValidation(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			fallbackResult := PerformFallbackValidation(
-				ctx,
-				tt.config,
-				tt.fallbackContext,
-				tt.imageRef,
-				tt.componentName,
 				tt.result,
 				tt.predicateStatus,
-				tt.workerFallbackContext,
 			)
 
 			// Verify the result structure
@@ -426,7 +368,6 @@ func TestShouldTriggerFallback_EdgeCases(t *testing.T) {
 
 // Test PerformFallbackValidation with different predicate statuses
 func TestPerformFallbackValidation_PredicateStatuses(t *testing.T) {
-	ctx := context.Background()
 
 	tests := []struct {
 		name            string
@@ -461,23 +402,9 @@ func TestPerformFallbackValidation_PredicateStatuses(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			config := &FallbackConfig{
-				FallbackToImageValidation: true,
-				FallbackPublicKey:         "test-key",
-				PolicyConfig:              "test-policy",
-				EffectiveTime:             "2023-01-01T00:00:00Z",
-				Info:                      false,
-			}
-
 			fallbackResult := PerformFallbackValidation(
-				ctx,
-				config,
-				nil, // fallbackContext
-				"test-image:latest",
-				"test-component",
 				tt.result,
 				tt.predicateStatus,
-				nil, // workerFallbackContext
 			)
 
 			require.NotNil(t, fallbackResult)

internal/validate/vsa/vsa.go

@@ -931,36 +931,6 @@ func ExtractDigestFromImageRef(imageRef string) (string, error) {
 	return imageRef, nil
 }
 
-// ExtractImageFromVSAIdentifier extracts the image reference from VSA identifier
-// This function is used for fallback validation when VSA validation fails
-func ExtractImageFromVSAIdentifier(identifier string) (string, error) {
-	// Handle empty identifier specially
-	if identifier == "" {
-		return "", nil
-	}
-
-	identifierType := DetectIdentifierType(identifier)
-
-	switch identifierType {
-	case IdentifierImageDigest:
-		// Convert digest to image reference
-		// e.g., registry/image@sha256:abc123 -> registry/image:tag
-		return ConvertDigestToImageRef(identifier)
-	case IdentifierImageReference:
-		// Check if it's actually a file path that was incorrectly detected as image reference
-		if IsFilePathLike(identifier) {
-			return "", fmt.Errorf("fallback validation not supported for file paths: %s", identifier)
-		}
-		// Already an image reference
-		return identifier, nil
-	case IdentifierFile:
-		// Cannot extract image from file path - fallback not supported
-		return "", fmt.Errorf("fallback validation not supported for file paths: %s", identifier)
-	default:
-		return "", fmt.Errorf("fallback validation not supported for identifier type: %s", identifier)
-	}
-}
-
 // isFilePathLike checks if an identifier looks like a file path
 // This handles the case where name.ParseReference incorrectly accepts file paths as valid image references
 func IsFilePathLike(identifier string) bool {
@@ -987,28 +957,3 @@ func IsFilePathLike(identifier string) bool {
 
 	return false
 }
-
-// ConvertDigestToImageRef converts a digest to an image reference
-// This is a simplified implementation that attempts to construct a reasonable image reference
-func ConvertDigestToImageRef(digest string) (string, error) {
-	// If the digest is already in the format registry/image@sha256:abc123,
-	// we can extract the repository part and construct a tag reference
-	if strings.Contains(digest, "@") {
-		parts := strings.Split(digest, "@")
-		if len(parts) == 2 {
-			repository := parts[0]
-			// Try to construct a reasonable tag reference
-			// This is a best-effort approach since we don't have the original tag
-			imageRef := repository + ":latest"
-
-			// Validate that the constructed reference is valid
-			if _, err := name.ParseReference(imageRef); err == nil {
-				return imageRef, nil
-			}
-		}
-	}
-
-	// If we can't convert the digest to an image reference,
-	// return an error as fallback is not supported for pure digests
-	return "", fmt.Errorf("fallback validation: cannot convert digest to image reference: %s", digest)
-}