Repository: project-copacetic/copacetic (go)

Issue:
copy over windows digests (don't skip over them) in multi arch patching
since copa cannot patch windows containers, we need to keep them as is. this means we need to keep the existing windows container digests copied over to the new patched container index. this only applies to index and manifests list that includes non-host process windows containers, and it is not applicable to host-process containers.

this is a must before we can release multi platform functionality as this might lead to existing containers breaking.

Pull request description:
fix: preseve all  platforms in manifest
<!--
Please include the type of change in the PR title as "<type>: <summary>"
Valid <type> include: build, ci, docs, feat, fix, perf, refactor, test
See https://github.com/project-copacetic/copacetic/blob/master/CONTRIBUTING.md#pull-requests) for guidelines.
-->

Preserves digests of unpatched images (including Windows). I can modify this for just windows if we want, but thought it would be good to keep all digests in the resulting manifest.

Describe the changes in this pull request using active verbs such as _Add_, _Remove_, _Replace_ ...

Closes #1112


Patch:
diff --git a/pkg/buildkit/buildkit.go b/pkg/buildkit/buildkit.go
index ccdcc673..8700dcb4 100644
--- a/pkg/buildkit/buildkit.go
+++ b/pkg/buildkit/buildkit.go
@@ -184,7 +184,9 @@ func DiscoverPlatformsFromReference(manifestRef string) ([]types.PatchPlatform,
 		for i := range manifest.Manifests {
 			m := &manifest.Manifests[i]
 
-			if m.Platform.OS != linux {
+			// Skip manifests with unknown platforms
+			if m.Platform == nil || m.Platform.OS == "unknown" || m.Platform.Architecture == "unknown" {
+				log.Debugf("Skipping manifest with unknown platform: %s/%s", m.Platform.OS, m.Platform.Architecture)
 				continue
 			}
 
@@ -193,6 +195,8 @@ func DiscoverPlatformsFromReference(manifestRef string) ([]types.PatchPlatform,
 					OS:           m.Platform.OS,
 					Architecture: m.Platform.Architecture,
 					Variant:      m.Platform.Variant,
+					OSVersion:    m.Platform.OSVersion,
+					OSFeatures:   m.Platform.OSFeatures,
 				},
 			}
 			if m.Platform.Architecture == arm64 && m.Platform.Variant == "v8" {
@@ -216,6 +220,10 @@ func PlatformKey(pl ispec.Platform) string {
 	if pl.Variant != "" {
 		key += "/" + pl.Variant
 	}
+	// Include OS version for platforms like Windows that have multiple versions
+	if pl.OSVersion != "" {
+		key += "@" + pl.OSVersion
+	}
 	return key
 }
 
@@ -238,6 +246,7 @@ func DiscoverPlatforms(manifestRef, reportDir, scanner string) ([]types.PatchPla
 		}
 		log.WithField("platforms", p2).Debug("Discovered platforms from report")
 
+		// include all platforms from original manifest, patching only those with reports
 		reportSet := make(map[string]string, len(p2))
 		for _, pl := range p2 {
 			reportSet[PlatformKey(pl.Platform)] = pl.ReportFile
@@ -245,8 +254,14 @@ func DiscoverPlatforms(manifestRef, reportDir, scanner string) ([]types.PatchPla
 
 		for _, pl := range p {
 			if rp, ok := reportSet[PlatformKey(pl.Platform)]; ok {
+				// Platform has a report - will be patched
 				pl.ReportFile = rp
 				platforms = append(platforms, pl)
+			} else {
+				// Platform has no report - preserve original without patching
+				log.Debugf("No report found for platform %s, preserving original", PlatformKey(pl.Platform))
+				pl.ReportFile = ""
+				platforms = append(platforms, pl)
 			}
 		}
 
diff --git a/pkg/patch/patch.go b/pkg/patch/patch.go
index ede49ee0..35dfa346 100644
--- a/pkg/patch/patch.go
+++ b/pkg/patch/patch.go
@@ -24,6 +24,8 @@ import (
 
 	"github.com/distribution/reference"
 	"github.com/docker/buildx/util/imagetools"
+	"github.com/google/go-containerregistry/pkg/name"
+	"github.com/google/go-containerregistry/pkg/v1/remote"
 	"github.com/moby/buildkit/client"
 	"github.com/moby/buildkit/client/llb"
 	"github.com/moby/buildkit/exporter/containerimage/exptypes"
@@ -31,6 +33,8 @@ import (
 	"github.com/moby/buildkit/session"
 	"github.com/moby/buildkit/session/auth/authprovider"
 	"github.com/moby/buildkit/util/progress/progressui"
+	"github.com/opencontainers/go-digest"
+	ispec "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/project-copacetic/copacetic/pkg/buildkit"
 	"github.com/project-copacetic/copacetic/pkg/buildkit/connhelpers"
 	"github.com/project-copacetic/copacetic/pkg/pkgmgr"
@@ -50,6 +54,7 @@ const (
 	defaultRegistry = "docker.io"
 	defaultTag      = "latest"
 	LINUX           = "linux"
+	ARM64           = "arm64"
 )
 
 // for testing.
@@ -783,6 +788,51 @@ func patchMultiPlatformImage(
 			}
 			defer func() { <-sem }()
 
+			if p.ReportFile == "" {
+				// No report for this platform - preserve original
+				log.Infof("No report for platform %s, preserving original in manifest", p.OS+"/"+p.Architecture)
+
+				// Handle Windows platform without push enabled
+				if !push && p.OS == "windows" {
+					if !ignoreError {
+						return errors.New("cannot save Windows platform image without pushing to registry. Use --push flag to save Windows images to a registry or run with --ignore-errors")
+					}
+					log.Warn("Cannot save Windows platform image without pushing to registry. Use --push flag to save Windows images to a registry.")
+				}
+
+				// Get the original platform descriptor from the manifest
+				originalDesc, err := getPlatformDescriptorFromManifest(image, &p)
+				if err != nil {
+					return fmt.Errorf("failed to get original descriptor for platform %s: %w", p.OS+"/"+p.Architecture, err)
+				}
+
+				// Parse the original image reference for the result
+				originalRef, err := reference.ParseNormalizedNamed(image)
+				if err != nil {
+					return fmt.Errorf("failed to parse original image reference: %w", err)
+				}
+
+				// For platforms without reports, use the original image digest/reference
+				result := types.PatchResult{
+					OriginalRef: originalRef,
+					PatchedRef:  originalRef,
+					PatchedDesc: originalDesc,
+				}
+
+				mu.Lock()
+				patchResults = append(patchResults, result)
+				// Add summary entry for unpatched platform
+				summaryMap[platformKey] = &types.MultiArchSummary{
+					Platform: platformKey,
+					Status:   "Not Patched",
+					Ref:      originalRef.String() + " (original reference)",
+					Error:    "",
+				}
+				mu.Unlock()
+				log.Infof("Preserved original image (%s): %s\n", p.OS+"/"+p.Architecture, originalRef.String())
+				return nil
+			}
+
 			res, err := patchSingleArchImage(gctx, ch, image, p.ReportFile, patchedTag, suffix, workingFolder, scanner, format, output, p, ignoreError, push, bkOpts, true)
 			mu.Lock()
 			defer mu.Unlock()
@@ -850,18 +900,39 @@ func patchMultiPlatformImage(
 	}
 
 	if !push {
-		if len(patchResults) > 0 {
+		// Show push commands only for actually patched images (not preserved originals)
+		patchedOnlyResults := make([]types.PatchResult, 0)
+		for _, result := range patchResults {
+			// Only include results where the patched ref differs from original ref
+			if result.PatchedRef.String() != result.OriginalRef.String() {
+				patchedOnlyResults = append(patchedOnlyResults, result)
+			}
+		}
+
+		if len(patchedOnlyResults) > 0 {
 			log.Info("To push the individual architecture images, run:")
-			for _, result := range patchResults {
+			for _, result := range patchedOnlyResults {
 				log.Infof("  docker push %s", result.PatchedRef.String())
 			}
 			log.Infof("To create and push the multi-platform manifest, run:")
+
+			// Include all platforms (both patched and preserved) in the manifest create command
 			refs := make([]string, len(patchResults))
 			for i, result := range patchResults {
-				refs[i] = result.PatchedRef.String()
+				if result.PatchedRef.String() != result.OriginalRef.String() {
+					// Use the patched reference for actually patched platforms
+					refs[i] = result.PatchedRef.String()
+				} else {
+					// Use the original reference with digest for preserved platforms
+					if result.PatchedDesc != nil && result.PatchedDesc.Digest.String() != "" {
+						refs[i] = result.OriginalRef.String() + "@" + result.PatchedDesc.Digest.String()
+					} else {
+						refs[i] = result.OriginalRef.String()
+					}
+				}
 			}
-			log.Infof("  docker manifest create %s %s", patchedImageName.String(), strings.Join(refs, " "))
-			log.Infof("  docker manifest push %s", patchedImageName.String())
+
+			log.Infof("  docker buildx imagetools create --tag %s %s", patchedImageName.String(), strings.Join(refs, " "))
 		} else {
 			return fmt.Errorf("no patched images were created, check the logs for errors")
 		}
@@ -887,3 +958,70 @@ func patchMultiPlatformImage(
 
 	return nil
 }
+
+// Gets the descriptor for a specific platform from a multi-arch manifest.
+func getPlatformDescriptorFromManifest(imageRef string, targetPlatform *types.PatchPlatform) (*ispec.Descriptor, error) {
+	ref, err := name.ParseReference(imageRef)
+	if err != nil {
+		return nil, fmt.Errorf("error parsing reference %q: %w", imageRef, err)
+	}
+
+	desc, err := remote.Get(ref)
+	if err != nil {
+		return nil, fmt.Errorf("error fetching descriptor for %q: %w", imageRef, err)
+	}
+
+	if !desc.MediaType.IsIndex() {
+		return nil, fmt.Errorf("expected multi-platform image but got single-arch image")
+	}
+
+	index, err := desc.ImageIndex()
+	if err != nil {
+		return nil, fmt.Errorf("error getting image index: %w", err)
+	}
+
+	manifest, err := index.IndexManifest()
+	if err != nil {
+		return nil, fmt.Errorf("error getting manifest: %w", err)
+	}
+
+	// Find the descriptor for the target platform
+	for i := range manifest.Manifests {
+		m := &manifest.Manifests[i]
+		if m.Platform == nil {
+			continue
+		}
+
+		// Normalize the variant comparison - treat missing variant as empty string
+		manifestVariant := m.Platform.Variant
+		targetVariant := targetPlatform.Variant
+		if m.Platform.Architecture == "arm64" && manifestVariant == "v8" {
+			manifestVariant = ""
+		}
+		if targetPlatform.Architecture == "arm64" && targetVariant == "v8" {
+			targetVariant = ""
+		}
+
+		if m.Platform.OS == targetPlatform.OS &&
+			m.Platform.Architecture == targetPlatform.Architecture &&
+			manifestVariant == targetVariant &&
+			m.Platform.OSVersion == targetPlatform.OSVersion {
+			// Convert the descriptor to the expected format
+			ociDesc := &ispec.Descriptor{
+				MediaType: string(m.MediaType),
+				Size:      m.Size,
+				Digest:    digest.Digest(m.Digest.String()),
+				Platform: &ispec.Platform{
+					OS:           m.Platform.OS,
+					Architecture: m.Platform.Architecture,
+					Variant:      m.Platform.Variant,
+					OSVersion:    m.Platform.OSVersion,
+					OSFeatures:   m.Platform.OSFeatures,
+				},
+			}
+			return ociDesc, nil
+		}
+	}
+
+	return nil, fmt.Errorf("platform %s/%s not found in manifest", targetPlatform.OS, targetPlatform.Architecture)
+}


Test patch:
diff --git a/integration/multiarch/patch_test.go b/integration/multiarch/patch_test.go
index 24ee2b58..a7a5d51f 100644
--- a/integration/multiarch/patch_test.go
+++ b/integration/multiarch/patch_test.go
@@ -126,6 +126,174 @@ func TestPatch(t *testing.T) {
 	}
 }
 
+// Tests patching only some architectures while preserving others.
+func TestPatchPartialArchitectures(t *testing.T) {
+	// Test image with multiple platforms including Windows
+	originalImage := "registry.k8s.io/csi-secrets-store/driver"
+	tag := "v1.4.8"
+	localImage := "localhost:5000/secrets-store-test"
+
+	originalRef := fmt.Sprintf("%s:%s", originalImage, tag)
+	localRef := fmt.Sprintf("%s:%s", localImage, tag)
+
+	// Copy the original multi-arch image to local registry
+	copyImage(t, originalRef, localRef)
+
+	// Create a temporary directory for reports
+	reportDir := t.TempDir()
+
+	// Only scan amd64 platform
+	platformToScan := "linux/amd64"
+	suffix := strings.ReplaceAll(platformToScan, "/", "-")
+	reportPath := filepath.Join(reportDir, "report-"+suffix+".json")
+
+	t.Logf("scanning original image for platform %s only", platformToScan)
+	scanner().
+		withOutput(reportPath).
+		withSkipDBUpdate().
+		withPlatform(platformToScan).
+		scan(t, localRef, false)
+
+	// Patch the image
+	patchedTag := tag + "-partial-patched"
+	patchedRef := fmt.Sprintf("%s:%s", localImage, patchedTag)
+
+	t.Log("patching image with only linux/amd64 platform report")
+	patchMultiPlatform(t, localRef, patchedTag, reportDir, false, true)
+
+	// Verify the patched manifest still contains all original platforms
+	t.Log("verifying manifest contains all original platforms")
+
+	// Get original manifest platforms
+	originalPlatforms := getManifestPlatforms(t, originalRef)
+	t.Logf("original platforms: %v", originalPlatforms)
+
+	// Get patched manifest platforms
+	patchedPlatforms := getManifestPlatforms(t, patchedRef)
+	t.Logf("patched platforms: %v", patchedPlatforms)
+
+	// Verify all original platforms are preserved
+	require.Equal(t, len(originalPlatforms), len(patchedPlatforms),
+		"patched manifest should have same number of platforms as original")
+
+	// Count expected platforms from original manifest
+	expectedLinuxCount := 0
+	expectedWindowsCount := 0
+	for _, p := range originalPlatforms {
+		switch p.OS {
+		case "linux":
+			expectedLinuxCount++
+		case "windows":
+			expectedWindowsCount++
+		}
+	}
+
+	// Verify we have the expected number of Linux and Windows platforms
+	actualLinuxCount := 0
+	actualWindowsCount := 0
+	for _, p := range patchedPlatforms {
+		switch p.OS {
+		case "linux":
+			actualLinuxCount++
+		case "windows":
+			actualWindowsCount++
+		}
+	}
+
+	require.Equal(t, expectedLinuxCount, actualLinuxCount, "should contain %d Linux platforms", expectedLinuxCount)
+	require.Equal(t, expectedWindowsCount, actualWindowsCount, "should contain %d Windows platforms", expectedWindowsCount)
+
+	// Verify that all original platforms are preserved in the patched manifest
+	for _, originalPlatform := range originalPlatforms {
+		found := false
+		for _, patchedPlatform := range patchedPlatforms {
+			if originalPlatform.OS == patchedPlatform.OS &&
+				originalPlatform.Architecture == patchedPlatform.Architecture &&
+				originalPlatform.Variant == patchedPlatform.Variant &&
+				originalPlatform.OSVersion == patchedPlatform.OSVersion {
+				found = true
+				break
+			}
+		}
+		require.True(t, found, "original platform %s/%s (variant: %s, osversion: %s) should be preserved",
+			originalPlatform.OS, originalPlatform.Architecture, originalPlatform.Variant, originalPlatform.OSVersion)
+	}
+}
+
+// getManifestPlatforms extracts platform information from a manifest.
+func getManifestPlatforms(t *testing.T, imageRef string) []Platform {
+	// For localhost registry, use registry API since docker manifest inspect
+	// doesn't work well with local insecure registries
+	if strings.HasPrefix(imageRef, "localhost:5000/") {
+		return getManifestPlatformsFromRegistry(t, imageRef)
+	}
+
+	// For external registries, use docker manifest inspect
+	cmd := exec.Command("docker", "manifest", "inspect", imageRef)
+	output, err := cmd.CombinedOutput()
+	require.NoError(t, err, "failed to inspect manifest: %s", string(output))
+
+	var manifest struct {
+		Manifests []struct {
+			Platform Platform `json:"platform"`
+		} `json:"manifests"`
+	}
+
+	err = json.Unmarshal(output, &manifest)
+	require.NoError(t, err, "failed to parse manifest JSON")
+
+	platforms := make([]Platform, len(manifest.Manifests))
+	for i, m := range manifest.Manifests {
+		platforms[i] = m.Platform
+	}
+
+	return platforms
+}
+
+// Gets platform info directly from registry API.
+func getManifestPlatformsFromRegistry(t *testing.T, imageRef string) []Platform {
+	// Parse image reference: localhost:5000/repo:tag
+	parts := strings.SplitN(imageRef, "/", 2)
+	require.Len(t, parts, 2, "invalid image reference format")
+
+	repoParts := strings.SplitN(parts[1], ":", 2)
+	repo := repoParts[0]
+	tag := "latest"
+	if len(repoParts) == 2 {
+		tag = repoParts[1]
+	}
+
+	// Get manifest from registry
+	url := fmt.Sprintf("http://localhost:5000/v2/%s/manifests/%s", repo, tag)
+	cmd := exec.Command("curl", "-s", "-H", "Accept: application/vnd.docker.distribution.manifest.list.v2+json", url)
+	output, err := cmd.CombinedOutput()
+	require.NoError(t, err, "failed to get manifest from registry: %s", string(output))
+
+	var manifest struct {
+		Manifests []struct {
+			Platform Platform `json:"platform"`
+		} `json:"manifests"`
+	}
+
+	err = json.Unmarshal(output, &manifest)
+	require.NoError(t, err, "failed to parse manifest JSON: %s", string(output))
+
+	platforms := make([]Platform, len(manifest.Manifests))
+	for i, m := range manifest.Manifests {
+		platforms[i] = m.Platform
+	}
+
+	return platforms
+}
+
+// Platform represents a platform in a manifest.
+type Platform struct {
+	OS           string `json:"os"`
+	Architecture string `json:"architecture"`
+	OSVersion    string `json:"os.version,omitempty"`
+	Variant      string `json:"variant,omitempty"`
+}
+
 type addrWrapper struct {
 	m       sync.Mutex
 	address *string
diff --git a/pkg/patch/patch_test.go b/pkg/patch/patch_test.go
index b98a2bfd..49b2dd94 100644
--- a/pkg/patch/patch_test.go
+++ b/pkg/patch/patch_test.go
@@ -534,7 +534,7 @@ func TestArchTag(t *testing.T) {
 	cases := []struct {
 		base, arch, variant, want string
 	}{
-		{"patched", "arm64", "", "patched-arm64"},
+		{"patched", ARM64, "", "patched-arm64"},
 		{"patched", "arm", "v7", "patched-arm-v7"},
 		{"patched", "mips64", "n32", "patched-mips64-n32"},
 	}
@@ -552,7 +552,7 @@ func TestNormalizeConfigForPlatform(t *testing.T) {
 
 	plat := &types.PatchPlatform{}
 	plat.OS = LINUX
-	plat.Architecture = "arm64"
+	plat.Architecture = ARM64
 	plat.Variant = "v8"
 
 	fixed, err := normalizeConfigForPlatform(orig, plat)
@@ -565,7 +565,7 @@ func TestNormalizeConfigForPlatform(t *testing.T) {
 		t.Fatalf("unmarshal: %v", err)
 	}
 
-	if m["architecture"] != "arm64" || m["os"] != LINUX || m["variant"] != "v8" {
+	if m["architecture"] != ARM64 || m["os"] != LINUX || m["variant"] != "v8" {
 		t.Fatalf("fields not normalised correctly: %#v", m)
 	}
 
@@ -589,7 +589,7 @@ func TestMultiArchSummaryTable(t *testing.T) {
 		Variant      string
 	}{
 		{"linux", "amd64", ""},
-		{"linux", "arm64", ""},
+		{"linux", ARM64, ""},
 		{"linux", "arm", "v7"},
 	}