fix(airgap): make push-images idempotent against tag-immutable registries (#5940)

* fix(airgap): skip pushing manifests that already exist on destination

Add a manifest precheck before each image copy and enable
OptimizeDestinationImageAlreadyExists on copy.Options. KOTS
push-images now succeeds against tag-immutable registries
(Artifactory, Harbor, JFrog, Quay, WORM-backed OCI) when
re-pushing a release whose tags are unchanged from a prior push.

The precheck opens the destination ref, fetches its current
manifest, and compares the digest against the source. If they
match, the copy is skipped — no PUT is issued, so the registry
never sees an overwrite attempt to reject. Destination errors
fall through to the normal push path; only source failures bubble
up. OptimizeDestinationImageAlreadyExists adds per-child coverage
in case the parent manifest list differs but children are present.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(airgap): log swallowed destination errors and document precheck gap

Greptile review feedback on #5940:

- Surface transient/auth/rate-limit failures on the destination side at
  debug level so operators have a trail when the precheck isn't firing
  as expected. Behavior is unchanged: these errors still fall through
  to the normal push path.
- Document the known byte-level comparison gap on destinationManifestMatches:
  if a prior push succeeded through the copy library and the library
  silently mutated the manifest bytes, the digests will never match and
  subsequent pushes to tag-immutable destinations still fail.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Changes

Signed-off-by: Evans Mungai <evans@replicated.com>

* Attempt comparison after library modifications of manifest

Signed-off-by: Evans Mungai <evans@replicated.com>

* fix(airgap): make idempotent-push behavior opt-in

The always-on precheck and OptimizeDestinationImageAlreadyExists short-circuited
the copy library's manifest write events, which the online smoke-test e2e
asserts on ("Writing manifest to image destination" never appears in the UI).

Gate both behind a new SkipExistingImages flag on CopyImageOptions /
PushImagesOptions / ProcessImageOptions / RewriteOptions. Wire it through:

- CLI: --skip-existing-images on `kots admin-console push-images`
- Admin Console: "Skip Pushing Images That Already Exist" checkbox in
  AirgapRegistrySettings, plumbed via UpdateAppRegistryRequest →
  registry.RewriteImages → rewrite.Rewrite → CopyOnlineImages.

Default behavior is unchanged; opt-in only when re-pushing to registries
that enforce tag immutability (Artifactory, Harbor, JFrog, Quay, WORM-backed
OCI).

Signed-off-by: Evans Mungai <evans@replicated.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Remove unwanted registry refs

Signed-off-by: Evans Mungai <evans@replicated.com>

* Log level comment fix

Signed-off-by: Evans Mungai <evans@replicated.com>

---------

Signed-off-by: Evans Mungai <evans@replicated.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: banjoh

cmd/kots/cli/admin-console-push-images.go

@@ -69,6 +69,7 @@ func AdminPushImagesCmd() *cobra.Command {
 	cmd.Flags().String("registry-username", "", "user name to use to authenticate with the registry")
 	cmd.Flags().String("registry-password", "", "password to use to authenticate with the registry")
 	cmd.Flags().Bool("skip-registry-check", false, "skip the connectivity test and validation of the provided registry information")
+	cmd.Flags().Bool("skip-existing-images", false, "skip pushing images whose manifest is already present at the destination tag. Required when re-pushing to registries that enforce tag immutability.")
 
 	cmd.Flags().String("kotsadm-tag", "", "set to override the tag of kotsadm. this may create an incompatible deployment because the version of kots and kotsadm are designed to work together")
 	cmd.Flags().MarkHidden("kotsadm-tag")
@@ -125,7 +126,8 @@ func genAndCheckPushOptions(endpoint string, namespace string, log *logger.CLILo
 			Username:  username,
 			Password:  password,
 		},
-		ProgressWriter: os.Stdout,
+		ProgressWriter:     os.Stdout,
+		SkipExistingImages: v.GetBool("skip-existing-images"),
 	}
 
 	return &options, nil

pkg/handlers/registry.go

@@ -27,11 +27,12 @@ import (
 )
 
 type UpdateAppRegistryRequest struct {
-	Hostname   string `json:"hostname"`
-	Username   string `json:"username"`
-	Password   string `json:"password"`
-	Namespace  string `json:"namespace"`
-	IsReadOnly bool   `json:"isReadOnly"`
+	Hostname           string `json:"hostname"`
+	Username           string `json:"username"`
+	Password           string `json:"password"`
+	Namespace          string `json:"namespace"`
+	IsReadOnly         bool   `json:"isReadOnly"`
+	SkipExistingImages bool   `json:"skipExistingImages"`
 }
 
 type UpdateAppRegistryResponse struct {
@@ -237,7 +238,8 @@ func (h *Handler) UpdateAppRegistry(w http.ResponseWriter, r *http.Request) {
 		appDir, err := registry.RewriteImages(
 			foundApp.ID, latestSequence, updateAppRegistryRequest.Hostname,
 			updateAppRegistryRequest.Username, registryPassword,
-			updateAppRegistryRequest.Namespace, skipImagePush)
+			updateAppRegistryRequest.Namespace, skipImagePush,
+			updateAppRegistryRequest.SkipExistingImages)
 		if err != nil {
 			// log credential errors at info level
 			causeErr := errors.Cause(err)

pkg/image/airgap.go

@@ -124,9 +124,10 @@ func CopyAirgapImages(opts imagetypes.ProcessImageOptions, log *logger.CLILogger
 			Username:  opts.RegistrySettings.Username,
 			Password:  opts.RegistrySettings.Password,
 		},
-		Log:            log,
-		ProgressWriter: opts.ReportWriter,
-		LogForUI:       true,
+		Log:                log,
+		ProgressWriter:     opts.ReportWriter,
+		LogForUI:           true,
+		SkipExistingImages: opts.SkipExistingImages,
 	}
 
 	err := TagAndPushImagesFromBundle(opts.AirgapBundle, pushOpts)
@@ -270,13 +271,14 @@ func PushECImagesFromTempRegistry(airgapRootDir string, airgap *kotsv1beta1.Airg
 					Username: options.Registry.Username,
 					Password: options.Registry.Password,
 				},
-				CopyAll:           true,
-				PreserveDigests:   isDigestPinned,
-				SrcDisableV1Ping:  true,
-				SrcSkipTLSVerify:  true,
-				DestDisableV1Ping: true,
-				DestSkipTLSVerify: true,
-				ReportWriter:      reportWriter,
+				CopyAll:            true,
+				PreserveDigests:    isDigestPinned,
+				SrcDisableV1Ping:   true,
+				SrcSkipTLSVerify:   true,
+				DestDisableV1Ping:  true,
+				DestSkipTLSVerify:  true,
+				ReportWriter:       reportWriter,
+				SkipExistingImages: options.SkipExistingImages,
 			},
 		}
 		imageCounter++
@@ -387,13 +389,14 @@ func PushImagesFromTempRegistry(airgapRootDir string, imageList []string, option
 					Username: options.Registry.Username,
 					Password: options.Registry.Password,
 				},
-				CopyAll:           copyAll,
-				PreserveDigests:   isDigestPinned,
-				SrcDisableV1Ping:  true,
-				SrcSkipTLSVerify:  true,
-				DestDisableV1Ping: true,
-				DestSkipTLSVerify: true,
-				ReportWriter:      reportWriter,
+				CopyAll:            copyAll,
+				PreserveDigests:    isDigestPinned,
+				SrcDisableV1Ping:   true,
+				SrcSkipTLSVerify:   true,
+				DestDisableV1Ping:  true,
+				DestSkipTLSVerify:  true,
+				ReportWriter:       reportWriter,
+				SkipExistingImages: options.SkipExistingImages,
 			},
 		}
 		imageCounter++
@@ -488,10 +491,11 @@ func PushImagesFromDockerArchivePath(airgapRootDir string, options imagetypes.Pu
 					Username: options.Registry.Username,
 					Password: options.Registry.Password,
 				},
-				CopyAll:           false, // docker-archive format does not support multi-arch images
-				DestSkipTLSVerify: true,
-				DestDisableV1Ping: true,
-				ReportWriter:      reportWriter,
+				CopyAll:            false, // docker-archive format does not support multi-arch images
+				DestSkipTLSVerify:  true,
+				DestDisableV1Ping:  true,
+				ReportWriter:       reportWriter,
+				SkipExistingImages: options.SkipExistingImages,
 			},
 		}
 		if err := pushImage(pushImageOpts); err != nil {
@@ -614,10 +618,11 @@ func PushImagesFromDockerArchiveBundle(airgapBundle string, options imagetypes.P
 					Username: options.Registry.Username,
 					Password: options.Registry.Password,
 				},
-				CopyAll:           false, // docker-archive format does not support multi-arch images
-				DestSkipTLSVerify: true,
-				DestDisableV1Ping: true,
-				ReportWriter:      reportWriter,
+				CopyAll:            false, // docker-archive format does not support multi-arch images
+				DestSkipTLSVerify:  true,
+				DestDisableV1Ping:  true,
+				ReportWriter:       reportWriter,
+				SkipExistingImages: options.SkipExistingImages,
 			},
 		}
 		if err := pushImage(pushImageOpts); err != nil {

pkg/image/online.go

@@ -1,6 +1,7 @@
 package image
 
 import (
+	"bytes"
 	"context"
 	"fmt"
 	"io"
@@ -182,7 +183,7 @@ func CopyOnlineImages(opts imagetypes.ProcessImageOptions, images []string, kots
 		if _, copied := copiedImages[img]; copied {
 			continue
 		}
-		if err := copyOnlineImage(sourceRegistry, destRegistry, img, opts.AppSlug, opts.ReportWriter, log, installationImages, dockerHubRegistry); err != nil {
+		if err := copyOnlineImage(sourceRegistry, destRegistry, img, opts.AppSlug, opts.SkipExistingImages, opts.ReportWriter, log, installationImages, dockerHubRegistry); err != nil {
 			return errors.Wrapf(err, "failed to copy online image %s", img)
 		}
 		copiedImages[img] = true
@@ -191,7 +192,7 @@ func CopyOnlineImages(opts imagetypes.ProcessImageOptions, images []string, kots
 	return nil
 }
 
-func copyOnlineImage(srcRegistry, destRegistry dockerregistrytypes.RegistryOptions, image string, appSlug string, reportWriter io.Writer, log *logger.CLILogger, installationImages map[string]types.InstallationImageInfo, dockerHubRegistry dockerregistrytypes.RegistryOptions) error {
+func copyOnlineImage(srcRegistry, destRegistry dockerregistrytypes.RegistryOptions, image string, appSlug string, skipExistingImages bool, reportWriter io.Writer, log *logger.CLILogger, installationImages map[string]types.InstallationImageInfo, dockerHubRegistry dockerregistrytypes.RegistryOptions) error {
 	// TODO: This reaches out to internet in airgap installs. It shouldn't.
 	sourceImage := image
 	srcAuth := imagetypes.RegistryAuth{}
@@ -243,13 +244,14 @@ func copyOnlineImage(srcRegistry, destRegistry dockerregistrytypes.RegistryOptio
 			Username: destRegistry.Username,
 			Password: destRegistry.Password,
 		},
-		CopyAll:           copyAll,
-		PreserveDigests:   copyAll,
-		SrcDisableV1Ping:  true,
-		SrcSkipTLSVerify:  os.Getenv("KOTSADM_INSECURE_SRCREGISTRY") == "true",
-		DestDisableV1Ping: true,
-		DestSkipTLSVerify: true,
-		ReportWriter:      reportWriter,
+		CopyAll:            copyAll,
+		PreserveDigests:    copyAll,
+		SrcDisableV1Ping:   true,
+		SrcSkipTLSVerify:   os.Getenv("KOTSADM_INSECURE_SRCREGISTRY") == "true",
+		DestDisableV1Ping:  true,
+		DestSkipTLSVerify:  true,
+		ReportWriter:       reportWriter,
+		SkipExistingImages: skipExistingImages,
 	}
 	if err := CopyImage(copyImageOpts); err != nil {
 		return errors.Wrapf(err, "failed to copy %s to %s", sourceImage, destImage)
@@ -306,6 +308,25 @@ func CopyImage(opts types.CopyImageOptions) error {
 		imageListSelection = copy.CopyAllImages
 	}
 
+	// Opt-in idempotency precheck: if the destination tag already holds a manifest
+	// matching the source, skip the copy. Avoids re-pushing manifests to tag-immutable
+	// registries. Off by default — callers (CLI flag, admin-console checkbox) enable
+	// it when targeting registries that enforce tag immutability.
+	if opts.SkipExistingImages {
+		matches, err := destinationManifestMatches(context.Background(), opts, srcCtx, destCtx)
+		if err != nil {
+			return errors.Wrap(err, "failed manifest precheck")
+		}
+		if matches {
+			destRefName := ""
+			if opts.DestRef.DockerReference() != nil {
+				destRefName = opts.DestRef.DockerReference().String()
+			}
+			logger.Infof("Skipping push: destination manifest already matches source (dest=%s)", destRefName)
+			return nil
+		}
+	}
+
 	_, err := CopyImageWithGC(context.Background(), opts.DestRef, opts.SrcRef, &copy.Options{
 		RemoveSignatures:      true,
 		SignBy:                "",
@@ -315,6 +336,13 @@ func CopyImage(opts types.CopyImageOptions) error {
 		ForceManifestMIMEType: "",
 		ImageListSelection:    imageListSelection,
 		PreserveDigests:       opts.PreserveDigests,
+		// Skip per-child copies when the destination already has the same manifest.
+		// The precheck above handles the parent manifest list; this covers individual
+		// child images and any case where the parent differs but children are already
+		// present. The upstream library skips this for the parent manifest-list write
+		// in the multi-arch path, so the precheck is still required.
+		// known bug: https://github.com/podman-container-tools/container-libs/issues/918
+		OptimizeDestinationImageAlreadyExists: opts.SkipExistingImages,
 	})
 	if err != nil {
 		// When copying multi-arch images with PreserveDigests, the library may fail to write
@@ -438,6 +466,96 @@ func getPolicyContext() (*signature.PolicyContext, error) {
 	return policyContext, nil
 }
 
+// destinationManifestMatches reports whether the destination tag already holds a
+// manifest equivalent to the source. When true, the push can be skipped —
+// this makes the copy idempotent against registries that enforce tag
+// immutability and reject overwrite regardless of digest equality.
+//
+// Equivalence is checked in two passes:
+//  1. Raw bytes equal — covers PreserveDigests=true and the pushSourceManifestList
+//     fallback path, where destination bytes match source bytes verbatim.
+//  2. Canonical bytes equal — both manifests are parsed and re-serialized
+//     through the copy library's own parser. Two manifests with the same
+//     image content but different on-the-wire form (whitespace, field order,
+//     small library-normalization differences) round-trip to identical bytes.
+//     This catches the case where a prior push wrote a library-normalized
+//     form to the destination so the raw source bytes no longer match.
+//
+// If the destination is unreachable or the tag does not exist, returns (false, nil)
+// so the caller proceeds with the normal push. Only source-side failures surface as
+// errors, since a missing or unreadable destination should not block the push.
+func destinationManifestMatches(ctx context.Context, opts types.CopyImageOptions, srcCtx, destCtx *containerstypes.SystemContext) (bool, error) {
+	destImg, err := opts.DestRef.NewImageSource(ctx, destCtx)
+	if err != nil {
+		// Destination does not exist or is unreachable; fall through to the normal push.
+		// Most commonly this is a 404 on first push, but it also masks transient network
+		// errors, 401/403 from misconfigured destination auth, and 429 rate limits — log
+		// at debug to leave a trail when the optimization isn't firing as expected.
+		logger.Errorf("manifest precheck: opening destination failed, falling through to push: %v", err)
+		return false, nil
+	}
+	defer destImg.Close()
+
+	destManifest, destMIME, err := destImg.GetManifest(ctx, nil)
+	if err != nil {
+		// Destination manifest unreadable; fall through to the normal push.
+		logger.Errorf("manifest precheck: reading destination manifest failed, falling through to push: %v", err)
+		return false, nil
+	}
+
+	srcImg, err := opts.SrcRef.NewImageSource(ctx, srcCtx)
+	if err != nil {
+		return false, errors.Wrap(err, "failed to open source for manifest precheck")
+	}
+	defer srcImg.Close()
+
+	srcManifest, srcMIME, err := srcImg.GetManifest(ctx, nil)
+	if err != nil {
+		return false, errors.Wrap(err, "failed to get source manifest for precheck")
+	}
+
+	if bytes.Equal(destManifest, srcManifest) {
+		return true, nil
+	}
+
+	// Second pass: round-trip both manifests through the library's parser+serializer.
+	// If they describe the same image content, the canonical forms are identical
+	// even when the raw bytes differ.
+	canonicalSrc, srcErr := canonicalManifestBytes(srcManifest, srcMIME)
+	canonicalDest, destErr := canonicalManifestBytes(destManifest, destMIME)
+	if srcErr == nil && destErr == nil && bytes.Equal(canonicalSrc, canonicalDest) {
+		logger.Infof("manifest precheck: raw bytes differ but canonical forms match (dest=%s)", opts.DestRef.DockerReference())
+		return true, nil
+	}
+	if srcErr != nil {
+		logger.Errorf("manifest precheck: canonicalizing source failed: %v", srcErr)
+	}
+	if destErr != nil {
+		logger.Errorf("manifest precheck: canonicalizing destination failed: %v", destErr)
+	}
+
+	return false, nil
+}
+
+// canonicalManifestBytes returns the manifest bytes after round-tripping
+// through the copy library's parser and serializer. Two semantically equivalent
+// manifests with different raw bytes (different whitespace, field order, etc.)
+// produce identical canonical bytes.
+func canonicalManifestBytes(b []byte, mime string) ([]byte, error) {
+	if manifest.MIMETypeIsMultiImage(mime) {
+		list, err := manifest.ListFromBlob(b, mime)
+		if err != nil {
+			return nil, err
+		}
+		return list.Serialize()
+	}
+	m, err := manifest.FromBlob(b, mime)
+	if err != nil {
+		return nil, err
+	}
+	return m.Serialize()
+}
+
 // pushSourceManifestList fetches the raw manifest list from the source and pushes it
 // directly to the destination registry. This is used as a fallback when the copy library
 // fails to write the manifest list due to spurious byte differences caused by annotation