fix: move image archive validation to assemble (#4490)

Signed-off-by: Austin Abro <austinabro321@gmail.com>

Author: AustinAbro321

src/pkg/lint/validate.go

@@ -11,7 +11,6 @@ import (
 	"strings"
 
 	"github.com/zarf-dev/zarf/src/api/v1alpha1"
-	"github.com/zarf-dev/zarf/src/pkg/transform"
 	"k8s.io/apimachinery/pkg/util/validation"
 )
 
@@ -159,10 +158,6 @@ func ValidatePackage(pkg v1alpha1.ZarfPackage) error {
 		}
 	}
 
-	if archiveErr := validateImageArchivesNoDuplicates(pkg.Components); archiveErr != nil {
-		err = errors.Join(err, archiveErr)
-	}
-
 	return err
 }
 
@@ -340,43 +335,3 @@ func validateManifest(manifest v1alpha1.ZarfManifest) error {
 
 	return err
 }
-
-// validateImageArchivesNoDuplicates ensures no image appears in multiple image archives
-// and that images in image archives don't conflict with images in component.Images.
-func validateImageArchivesNoDuplicates(components []v1alpha1.ZarfComponent) error {
-	imageToArchive := make(map[string]string)
-
-	for _, comp := range components {
-		for _, archive := range comp.ImageArchives {
-			for _, image := range archive.Images {
-				refInfo, err := transform.ParseImageRef(image)
-				if err != nil {
-					return fmt.Errorf("failed to parse image ref %s in archive %s: %w", image, archive.Path, err)
-				}
-
-				if existingArchivePath, exists := imageToArchive[refInfo.Reference]; exists {
-					// A user may want to represent the same tar twice across components if both components need the same image
-					if existingArchivePath != archive.Path {
-						return fmt.Errorf("image %s appears in multiple image archives: %s and %s", refInfo.Reference, existingArchivePath, archive.Path)
-					}
-				} else {
-					imageToArchive[refInfo.Reference] = archive.Path
-				}
-			}
-		}
-	}
-
-	for _, comp := range components {
-		for _, image := range comp.Images {
-			refInfo, err := transform.ParseImageRef(image)
-			if err != nil {
-				return fmt.Errorf("failed to parse image ref %s in component %s: %w", image, comp.Name, err)
-			}
-			if archivePath, exists := imageToArchive[refInfo.Reference]; exists {
-				return fmt.Errorf("image %s from %s is also pulled by component %s", refInfo.Reference, archivePath, comp.Name)
-			}
-		}
-	}
-
-	return nil
-}

src/pkg/lint/validate_test.go

@@ -493,163 +493,3 @@ func TestValidateComponentAction(t *testing.T) {
 		})
 	}
 }
-
-func TestValidateImageArchivesNoDuplicates(t *testing.T) {
-	t.Parallel()
-
-	tests := []struct {
-		name          string
-		components    []v1alpha1.ZarfComponent
-		errorContains string
-	}{
-		{
-			name: "no duplicates",
-			components: []v1alpha1.ZarfComponent{
-				{
-					Name: "component1",
-					ImageArchives: []v1alpha1.ImageArchive{
-						{
-							Path:   "/path/to/archive1.tar",
-							Images: []string{"nginx:1.21"},
-						},
-					},
-					Images: []string{"redis:6.2"},
-				},
-			},
-		},
-		{
-			name: "duplicate in different image archive",
-			components: []v1alpha1.ZarfComponent{
-				{
-					Name: "component1",
-					ImageArchives: []v1alpha1.ImageArchive{
-						{
-							Path:   "/path/to/archive1.tar",
-							Images: []string{"postgres:13"},
-						},
-						{
-							Path:   "/path/to/archive2.tar",
-							Images: []string{"postgres:13"},
-						},
-					},
-				},
-			},
-			errorContains: "appears in multiple image archives",
-		},
-		{
-			name: "duplicate in component images",
-			components: []v1alpha1.ZarfComponent{
-				{
-					Name: "component1",
-					ImageArchives: []v1alpha1.ImageArchive{
-						{
-							Path:   "/path/to/archive1.tar",
-							Images: []string{"ghcr.io/zarf-dev/zarf/agent:0.65.0"},
-						},
-					},
-					Images: []string{"nginx:1.21", "ghcr.io/zarf-dev/zarf/agent:0.65.0"},
-				},
-			},
-			errorContains: "is also pulled by component",
-		},
-		{
-			name: "duplicate in component with docker ref",
-			components: []v1alpha1.ZarfComponent{
-				{
-					Name: "component1",
-					ImageArchives: []v1alpha1.ImageArchive{
-						{
-							Path:   "/path/to/archive1.tar",
-							Images: []string{"nginx:1.21"},
-						},
-					},
-					Images: []string{"nginx:1.21"},
-				},
-			},
-			errorContains: "is also pulled by component",
-		},
-		{
-			name: "duplicate across multiple components",
-			components: []v1alpha1.ZarfComponent{
-				{
-					Name: "component1",
-					ImageArchives: []v1alpha1.ImageArchive{
-						{
-							Path:   "/path/to/archive1.tar",
-							Images: []string{"nginx:1.21"},
-						},
-					},
-				},
-				{
-					Name: "component2",
-					ImageArchives: []v1alpha1.ImageArchive{
-						{
-							Path:   "/path/to/archive2.tar",
-							Images: []string{"nginx:1.21"},
-						},
-					},
-				},
-			},
-			errorContains: "appears in multiple image archives",
-		},
-		{
-			name: "empty components",
-			components: []v1alpha1.ZarfComponent{
-				{
-					Name: "component1",
-				},
-			},
-		},
-		{
-			name: "duplicate images in component.Images is allowed",
-			components: []v1alpha1.ZarfComponent{
-				{
-					Name:   "component1",
-					Images: []string{"nginx:1.21"},
-				},
-				{
-					Name:   "component2",
-					Images: []string{"nginx:1.21"},
-				},
-			},
-		},
-		{
-			name: "same archive path used by multiple components is allowed",
-			components: []v1alpha1.ZarfComponent{
-				{
-					Name: "component1",
-					ImageArchives: []v1alpha1.ImageArchive{
-						{
-							Path:   "/path/to/shared-archive.tar",
-							Images: []string{"nginx:1.21", "redis:6.2"},
-						},
-					},
-				},
-				{
-					Name: "component2",
-					ImageArchives: []v1alpha1.ImageArchive{
-						{
-							Path:   "/path/to/shared-archive.tar",
-							Images: []string{"nginx:1.21", "postgres:13"},
-						},
-					},
-				},
-			},
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-
-			err := validateImageArchivesNoDuplicates(tt.components)
-
-			if tt.errorContains != "" {
-				require.Error(t, err)
-				require.Contains(t, err.Error(), tt.errorContains)
-			} else {
-				require.NoError(t, err)
-			}
-		})
-	}
-}

src/pkg/packager/layout/assemble.go

@@ -61,6 +61,10 @@ func AssemblePackage(ctx context.Context, pkg v1alpha1.ZarfPackage, packagePath
 	l := logger.From(ctx)
 	l.Info("assembling package", "path", packagePath)
 
+	if err := validateImageArchivesNoDuplicates(pkg.Components); err != nil {
+		return nil, err
+	}
+
 	if opts.DifferentialPackage.Metadata.Name != "" {
 		l.Debug("creating differential package", "differential", opts.DifferentialPackage)
 		allIncludedImagesMap := map[string]bool{}
@@ -302,6 +306,46 @@ func AssembleSkeleton(ctx context.Context, pkg v1alpha1.ZarfPackage, packagePath
 	return pkgLayout, nil
 }
 
+// validateImageArchivesNoDuplicates ensures no image appears in multiple image archives
+// and that images in image archives don't conflict with images in component.Images.
+func validateImageArchivesNoDuplicates(components []v1alpha1.ZarfComponent) error {
+	imageToArchive := make(map[string]string)
+
+	for _, comp := range components {
+		for _, archive := range comp.ImageArchives {
+			for _, image := range archive.Images {
+				refInfo, err := transform.ParseImageRef(image)
+				if err != nil {
+					return fmt.Errorf("failed to parse image ref %s in archive %s: %w", image, archive.Path, err)
+				}
+
+				if existingArchivePath, exists := imageToArchive[refInfo.Reference]; exists {
+					// A user may want to represent the same tar twice across components if both components need the same image
+					if existingArchivePath != archive.Path {
+						return fmt.Errorf("image %s appears in multiple image archives: %s and %s", refInfo.Reference, existingArchivePath, archive.Path)
+					}
+				} else {
+					imageToArchive[refInfo.Reference] = archive.Path
+				}
+			}
+		}
+	}
+
+	for _, comp := range components {
+		for _, image := range comp.Images {
+			refInfo, err := transform.ParseImageRef(image)
+			if err != nil {
+				return fmt.Errorf("failed to parse image ref %s in component %s: %w", image, comp.Name, err)
+			}
+			if archivePath, exists := imageToArchive[refInfo.Reference]; exists {
+				return fmt.Errorf("image %s from %s is also pulled by component %s", refInfo.Reference, archivePath, comp.Name)
+			}
+		}
+	}
+
+	return nil
+}
+
 func assemblePackageComponent(ctx context.Context, component v1alpha1.ZarfComponent, packagePath, buildPath string) (err error) {
 	tmpBuildPath, err := utils.MakeTempDir(config.CommonOptions.TempDirectory)
 	if err != nil {