Merge pull request #19640 from flouthoc/force-compression

push/manifest-push: add support for `--force-compression` to prevent reusing other blobs

cmd/podman/images/push.go

@@ -102,6 +102,8 @@ func pushFlags(cmd *cobra.Command) {
 	flags.StringVar(&pushOptions.DigestFile, digestfileFlagName, "", "Write the digest of the pushed image to the specified file")
 	_ = cmd.RegisterFlagCompletionFunc(digestfileFlagName, completion.AutocompleteDefault)
 
+	flags.BoolVar(&pushOptions.ForceCompressionFormat, "force-compression", false, "Use the specified compression algorithm even if the destination contains a differently-compressed variant already")
+
 	formatFlagName := "format"
 	flags.StringVarP(&pushOptions.Format, formatFlagName, "f", "", "Manifest type (oci, v2s2, or v2s1) to use in the destination (default is manifest type of source, with fallbacks)")
 	_ = cmd.RegisterFlagCompletionFunc(formatFlagName, common.AutocompleteManifestFormat)
@@ -214,6 +216,14 @@ func imagePush(cmd *cobra.Command, args []string) error {
 		pushOptions.CompressionLevel = &val
 	}
 
+	if cmd.Flags().Changed("compression-format") {
+		if !cmd.Flags().Changed("force-compression") {
+			// If `compression-format` is set and no value for `--force-compression`
+			// is selected then defaults to `true`.
+			pushOptions.ForceCompressionFormat = true
+		}
+	}
+
 	// Let's do all the remaining Yoga in the API to prevent us from scattering
 	// logic across (too) many parts of the code.
 	report, err := registry.ImageEngine().Push(registry.GetContext(), source, destination, pushOptions.ImagePushOptions)

cmd/podman/manifest/push.go

@@ -72,6 +72,8 @@ func init() {
 	flags.StringVar(&manifestPushOpts.DigestFile, digestfileFlagName, "", "after copying the image, write the digest of the resulting digest to the file")
 	_ = pushCmd.RegisterFlagCompletionFunc(digestfileFlagName, completion.AutocompleteDefault)
 
+	flags.BoolVar(&manifestPushOpts.ForceCompressionFormat, "force-compression", false, "Use the specified compression algorithm even if the destination contains a differently-compressed variant already")
+
 	formatFlagName := "format"
 	flags.StringVarP(&manifestPushOpts.Format, formatFlagName, "f", "", "manifest type (oci or v2s2) to attempt to use when pushing the manifest list (default is manifest type of source)")
 	_ = pushCmd.RegisterFlagCompletionFunc(formatFlagName, common.AutocompleteManifestFormat)
@@ -174,6 +176,14 @@ func push(cmd *cobra.Command, args []string) error {
 		manifestPushOpts.CompressionLevel = &val
 	}
 
+	if cmd.Flags().Changed("compression-format") {
+		if !cmd.Flags().Changed("force-compression") {
+			// If `compression-format` is set and no value for `--force-compression`
+			// is selected then defaults to `true`.
+			manifestPushOpts.ForceCompressionFormat = true
+		}
+	}
+
 	digest, err := registry.ImageEngine().ManifestPush(registry.Context(), listImageSpec, destSpec, manifestPushOpts.ImagePushOptions)
 	if err != nil {
 		return err

docs/source/markdown/options/force-compression.md

@@ -0,0 +1,8 @@
+####> This option file is used in:
+####>   podman manifest push, push
+####> If file is edited, make sure the changes
+####> are applicable to all of those.
+#### **--force-compression**
+
+If set, push uses the specified compression algorithm even if the destination contains a differently-compressed variant already.
+Defaults to `true` if `--compression-format` is explicitly specified on the command-line, `false` otherwise.

docs/source/markdown/podman-manifest-push.1.md.in

@@ -42,6 +42,8 @@ the list or index itself. (Default true)
 
 @@option digestfile
 
+@@option force-compression
+
 #### **--format**, **-f**=*format*
 
 Manifest list type (oci or v2s2) to use when pushing the list (default is oci).

docs/source/markdown/podman-push.1.md.in

@@ -70,6 +70,8 @@ Layer(s) to encrypt: 0-indexed layer indices with support for negative indexing
 
 The [protocol:keyfile] specifies the encryption protocol, which can be JWE (RFC7516), PGP (RFC4880), and PKCS7 (RFC2315) and the key material required for image encryption. For instance, jwe:/path/to/key.pem or pgp:admin@example.com or pkcs7:/path/to/x509-file.
 
+@@option force-compression
+
 #### **--format**, **-f**=*format*
 
 Manifest Type (oci, v2s2, or v2s1) to use when pushing an image.

pkg/api/handlers/libpod/images_push.go

@@ -25,14 +25,15 @@ func PushImage(w http.ResponseWriter, r *http.Request) {
 	runtime := r.Context().Value(api.RuntimeKey).(*libpod.Runtime)
 
 	query := struct {
-		All               bool   `schema:"all"`
-		CompressionFormat string `schema:"compressionFormat"`
-		CompressionLevel  *int   `schema:"compressionLevel"`
-		Destination       string `schema:"destination"`
-		Format            string `schema:"format"`
-		RemoveSignatures  bool   `schema:"removeSignatures"`
-		TLSVerify         bool   `schema:"tlsVerify"`
-		Quiet             bool   `schema:"quiet"`
+		All                    bool   `schema:"all"`
+		CompressionFormat      string `schema:"compressionFormat"`
+		CompressionLevel       *int   `schema:"compressionLevel"`
+		ForceCompressionFormat bool   `schema:"forceCompressionFormat"`
+		Destination            string `schema:"destination"`
+		Format                 string `schema:"format"`
+		RemoveSignatures       bool   `schema:"removeSignatures"`
+		TLSVerify              bool   `schema:"tlsVerify"`
+		Quiet                  bool   `schema:"quiet"`
 	}{
 		TLSVerify: true,
 		// #14971: older versions did not sent *any* data, so we need
@@ -73,15 +74,24 @@ func PushImage(w http.ResponseWriter, r *http.Request) {
 		password = authconf.Password
 	}
 	options := entities.ImagePushOptions{
-		All:               query.All,
-		Authfile:          authfile,
-		CompressionFormat: query.CompressionFormat,
-		CompressionLevel:  query.CompressionLevel,
-		Format:            query.Format,
-		Password:          password,
-		Quiet:             true,
-		RemoveSignatures:  query.RemoveSignatures,
-		Username:          username,
+		All:                    query.All,
+		Authfile:               authfile,
+		CompressionFormat:      query.CompressionFormat,
+		CompressionLevel:       query.CompressionLevel,
+		ForceCompressionFormat: query.ForceCompressionFormat,
+		Format:                 query.Format,
+		Password:               password,
+		Quiet:                  true,
+		RemoveSignatures:       query.RemoveSignatures,
+		Username:               username,
+	}
+
+	if _, found := r.URL.Query()["compressionFormat"]; found {
+		if _, foundForceCompression := r.URL.Query()["forceCompressionFormat"]; !foundForceCompression {
+			// If `compressionFormat` is set and no value for `forceCompressionFormat`
+			// is selected then default has to be `true`.
+			options.ForceCompressionFormat = true
+		}
 	}
 
 	if _, found := r.URL.Query()["tlsVerify"]; found {

pkg/api/handlers/libpod/manifests.go

@@ -333,14 +333,15 @@ func ManifestPush(w http.ResponseWriter, r *http.Request) {
 	decoder := r.Context().Value(api.DecoderKey).(*schema.Decoder)
 
 	query := struct {
-		All               bool     `schema:"all"`
-		CompressionFormat string   `schema:"compressionFormat"`
-		CompressionLevel  *int     `schema:"compressionLevel"`
-		Format            string   `schema:"format"`
-		RemoveSignatures  bool     `schema:"removeSignatures"`
-		TLSVerify         bool     `schema:"tlsVerify"`
-		Quiet             bool     `schema:"quiet"`
-		AddCompression    []string `schema:"addCompression"`
+		All                    bool     `schema:"all"`
+		CompressionFormat      string   `schema:"compressionFormat"`
+		CompressionLevel       *int     `schema:"compressionLevel"`
+		ForceCompressionFormat bool     `schema:"forceCompressionFormat"`
+		Format                 string   `schema:"format"`
+		RemoveSignatures       bool     `schema:"removeSignatures"`
+		TLSVerify              bool     `schema:"tlsVerify"`
+		Quiet                  bool     `schema:"quiet"`
+		AddCompression         []string `schema:"addCompression"`
 	}{
 		// Add defaults here once needed.
 		TLSVerify: true,
@@ -372,16 +373,24 @@ func ManifestPush(w http.ResponseWriter, r *http.Request) {
 		password = authconf.Password
 	}
 	options := entities.ImagePushOptions{
-		All:               query.All,
-		Authfile:          authfile,
-		AddCompression:    query.AddCompression,
-		CompressionFormat: query.CompressionFormat,
-		CompressionLevel:  query.CompressionLevel,
-		Format:            query.Format,
-		Password:          password,
-		Quiet:             true,
-		RemoveSignatures:  query.RemoveSignatures,
-		Username:          username,
+		All:                    query.All,
+		Authfile:               authfile,
+		AddCompression:         query.AddCompression,
+		CompressionFormat:      query.CompressionFormat,
+		CompressionLevel:       query.CompressionLevel,
+		ForceCompressionFormat: query.ForceCompressionFormat,
+		Format:                 query.Format,
+		Password:               password,
+		Quiet:                  true,
+		RemoveSignatures:       query.RemoveSignatures,
+		Username:               username,
+	}
+	if _, found := r.URL.Query()["compressionFormat"]; found {
+		if _, foundForceCompression := r.URL.Query()["forceCompressionFormat"]; !foundForceCompression {
+			// If `compressionFormat` is set and no value for `forceCompressionFormat`
+			// is selected then default has to be `true`.
+			options.ForceCompressionFormat = true
+		}
 	}
 	if sys := runtime.SystemContext(); sys != nil {
 		options.CertDir = sys.DockerCertPath

pkg/api/server/register_images.go

@@ -726,6 +726,11 @@ func (s *APIServer) registerImagesHandlers(r *mux.Router) error {
 	//    type: string
 	//    description: Allows for pushing the image to a different destination than the image refers to.
 	//  - in: query
+	//    name: forceCompressionFormat
+	//    description: Enforce compressing the layers with the specified --compression and do not reuse differently compressed blobs on the registry.
+	//    type: boolean
+	//    default: false
+	//  - in: query
 	//    name: tlsVerify
 	//    description: Require TLS verification.
 	//    type: boolean

pkg/api/server/register_manifest.go

@@ -67,6 +67,11 @@ func (s *APIServer) registerManifestHandlers(r *mux.Router) error {
 	//    type: array
 	//    items:
 	//      type: string
+	//  - in: query
+	//    name: forceCompressionFormat
+	//    description: Enforce compressing the layers with the specified --compression and do not reuse differently compressed blobs on the registry.
+	//    type: boolean
+	//    default: false
 	//  - in: path
 	//    name: destination
 	//    type: string

pkg/bindings/images/types.go

@@ -144,6 +144,10 @@ type PushOptions struct {
 	CompressionFormat *string
 	// CompressionLevel is the level to use for the compression of the blobs
 	CompressionLevel *int
+	// ForceCompressionFormat ensures that the compression algorithm set in
+	// CompressionFormat is used exclusively, and blobs of other compression
+	// algorithms are not reused.
+	ForceCompressionFormat *bool
 	// Add existing instances with requested compression algorithms to manifest list
 	AddCompression []string
 	// Manifest type of the pushed image

pkg/bindings/images/types_push_options.go

@@ -93,6 +93,21 @@ func (o *PushOptions) GetCompressionLevel() int {
 	return *o.CompressionLevel
 }
 
+// WithForceCompressionFormat set field ForceCompressionFormat to given value
+func (o *PushOptions) WithForceCompressionFormat(value bool) *PushOptions {
+	o.ForceCompressionFormat = &value
+	return o
+}
+
+// GetForceCompressionFormat returns value of field ForceCompressionFormat
+func (o *PushOptions) GetForceCompressionFormat() bool {
+	if o.ForceCompressionFormat == nil {
+		var z bool
+		return z
+	}
+	return *o.ForceCompressionFormat
+}
+
 // WithAddCompression set field AddCompression to given value
 func (o *PushOptions) WithAddCompression(value []string) *PushOptions {
 	o.AddCompression = value

pkg/domain/entities/images.go

@@ -247,6 +247,10 @@ type ImagePushOptions struct {
 	//  If necessary, add clones of existing instances with requested compression algorithms to manifest list
 	// Note: Following option is only valid for `manifest push`
 	AddCompression []string
+	// ForceCompressionFormat ensures that the compression algorithm set in
+	// CompressionFormat is used exclusively, and blobs of other compression
+	// algorithms are not reused.
+	ForceCompressionFormat bool
 }
 
 // ImagePushReport is the response from pushing an image.

pkg/domain/infra/abi/images.go

@@ -317,6 +317,7 @@ func (ir *ImageEngine) Push(ctx context.Context, source string, destination stri
 	pushOptions.OciEncryptConfig = options.OciEncryptConfig
 	pushOptions.OciEncryptLayers = options.OciEncryptLayers
 	pushOptions.CompressionLevel = options.CompressionLevel
+	pushOptions.ForceCompressionFormat = options.ForceCompressionFormat
 
 	compressionFormat := options.CompressionFormat
 	if compressionFormat == "" {

pkg/domain/infra/abi/manifest.go

@@ -346,6 +346,7 @@ func (ir *ImageEngine) ManifestPush(ctx context.Context, name, destination strin
 	pushOptions.Writer = opts.Writer
 	pushOptions.CompressionLevel = opts.CompressionLevel
 	pushOptions.AddCompression = opts.AddCompression
+	pushOptions.ForceCompressionFormat = opts.ForceCompressionFormat
 
 	compressionFormat := opts.CompressionFormat
 	if compressionFormat == "" {

pkg/domain/infra/tunnel/images.go

@@ -252,7 +252,7 @@ func (ir *ImageEngine) Push(ctx context.Context, source string, destination stri
 	}
 
 	options := new(images.PushOptions)
-	options.WithAll(opts.All).WithCompress(opts.Compress).WithUsername(opts.Username).WithPassword(opts.Password).WithAuthfile(opts.Authfile).WithFormat(opts.Format).WithRemoveSignatures(opts.RemoveSignatures).WithQuiet(opts.Quiet).WithCompressionFormat(opts.CompressionFormat).WithProgressWriter(opts.Writer)
+	options.WithAll(opts.All).WithCompress(opts.Compress).WithUsername(opts.Username).WithPassword(opts.Password).WithAuthfile(opts.Authfile).WithFormat(opts.Format).WithRemoveSignatures(opts.RemoveSignatures).WithQuiet(opts.Quiet).WithCompressionFormat(opts.CompressionFormat).WithProgressWriter(opts.Writer).WithForceCompressionFormat(opts.ForceCompressionFormat)
 
 	if opts.CompressionLevel != nil {
 		options.WithCompressionLevel(*opts.CompressionLevel)

pkg/domain/infra/tunnel/manifest.go

@@ -135,7 +135,7 @@ func (ir *ImageEngine) ManifestPush(ctx context.Context, name, destination strin
 	}
 
 	options := new(images.PushOptions)
-	options.WithUsername(opts.Username).WithPassword(opts.Password).WithAuthfile(opts.Authfile).WithRemoveSignatures(opts.RemoveSignatures).WithAll(opts.All).WithFormat(opts.Format).WithCompressionFormat(opts.CompressionFormat).WithQuiet(opts.Quiet).WithProgressWriter(opts.Writer).WithAddCompression(opts.AddCompression)
+	options.WithUsername(opts.Username).WithPassword(opts.Password).WithAuthfile(opts.Authfile).WithRemoveSignatures(opts.RemoveSignatures).WithAll(opts.All).WithFormat(opts.Format).WithCompressionFormat(opts.CompressionFormat).WithQuiet(opts.Quiet).WithProgressWriter(opts.Writer).WithAddCompression(opts.AddCompression).WithForceCompressionFormat(opts.ForceCompressionFormat)
 
 	if s := opts.SkipTLSVerify; s != types.OptionalBoolUndefined {
 		if s == types.OptionalBoolTrue {

test/e2e/manifest_test.go

@@ -154,7 +154,7 @@ var _ = Describe("Podman manifest", func() {
 		Expect(session2.OutputToString()).To(Equal(session.OutputToString()))
 	})
 
-	It("push with --add-compression", func() {
+	It("push with --add-compression and --force-compression", func() {
 		if podmanTest.Host.Arch == "ppc64le" {
 			Skip("No registry image for ppc64le")
 		}
@@ -209,6 +209,49 @@ var _ = Describe("Podman manifest", func() {
 		Expect(verifyInstanceCompression(index.Manifests, "zstd", "arm64")).Should(BeTrue())
 		Expect(verifyInstanceCompression(index.Manifests, "gzip", "arm64")).Should(BeTrue())
 		Expect(verifyInstanceCompression(index.Manifests, "gzip", "amd64")).Should(BeTrue())
+
+		// Note: Pushing again with --force-compression should produce the correct response the since blobs will be correctly force-pushed again.
+		push = podmanTest.Podman([]string{"manifest", "push", "--all", "--add-compression", "zstd", "--tls-verify=false", "--compression-format", "gzip", "--force-compression", "--remove-signatures", "foobar", "localhost:5000/list"})
+		push.WaitWithDefaultTimeout()
+		Expect(push).Should(Exit(0))
+		output = push.ErrorToString()
+		// 4 images must be pushed two for gzip and two for zstd
+		Expect(output).To(ContainSubstring("Copying 4 images generated from 2 images in list"))
+
+		session = podmanTest.Podman([]string{"run", "--rm", "--net", "host", "quay.io/skopeo/stable", "inspect", "--tls-verify=false", "--raw", "docker://localhost:5000/list:latest"})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+		inspectData = []byte(session.OutputToString())
+		err = json.Unmarshal(inspectData, &index)
+		Expect(err).ToNot(HaveOccurred())
+
+		Expect(verifyInstanceCompression(index.Manifests, "zstd", "amd64")).Should(BeTrue())
+		Expect(verifyInstanceCompression(index.Manifests, "zstd", "arm64")).Should(BeTrue())
+		Expect(verifyInstanceCompression(index.Manifests, "gzip", "arm64")).Should(BeTrue())
+		Expect(verifyInstanceCompression(index.Manifests, "gzip", "amd64")).Should(BeTrue())
+
+		// Note: Pushing again with --force-compression=false should produce in-correct/wrong result since blobs are already present in registry so they will be reused
+		// ignoring our compression priority ( this is expected behaviour of c/image and --force-compression is introduced to mitigate this behaviour ).
+		push = podmanTest.Podman([]string{"manifest", "push", "--all", "--add-compression", "zstd", "--force-compression=false", "--tls-verify=false", "--remove-signatures", "foobar", "localhost:5000/list"})
+		push.WaitWithDefaultTimeout()
+		Expect(push).Should(Exit(0))
+		output = push.ErrorToString()
+		// 4 images must be pushed two for gzip and two for zstd
+		Expect(output).To(ContainSubstring("Copying 4 images generated from 2 images in list"))
+
+		session = podmanTest.Podman([]string{"run", "--rm", "--net", "host", "quay.io/skopeo/stable", "inspect", "--tls-verify=false", "--raw", "docker://localhost:5000/list:latest"})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+		inspectData = []byte(session.OutputToString())
+		err = json.Unmarshal(inspectData, &index)
+		Expect(err).ToNot(HaveOccurred())
+
+		Expect(verifyInstanceCompression(index.Manifests, "zstd", "amd64")).Should(BeTrue())
+		Expect(verifyInstanceCompression(index.Manifests, "zstd", "arm64")).Should(BeTrue())
+		// blobs of zstd will be wrongly reused for gzip instances without --force-compression
+		Expect(verifyInstanceCompression(index.Manifests, "gzip", "arm64")).Should(BeFalse())
+		// blobs of zstd will be wrongly reused for gzip instances without --force-compression
+		Expect(verifyInstanceCompression(index.Manifests, "gzip", "amd64")).Should(BeFalse())
 	})
 
 	It("add --all", func() {

test/e2e/push_test.go

@@ -84,6 +84,63 @@ var _ = Describe("Podman push", func() {
 		Expect(foundZstdFile).To(BeTrue(), "found zstd file")
 	})
 
+	It("push test --force-compression", func() {
+		if podmanTest.Host.Arch == "ppc64le" {
+			Skip("No registry image for ppc64le")
+		}
+		if isRootless() {
+			err := podmanTest.RestoreArtifact(REGISTRY_IMAGE)
+			Expect(err).ToNot(HaveOccurred())
+		}
+		lock := GetPortLock("5000")
+		defer lock.Unlock()
+		session := podmanTest.Podman([]string{"run", "-d", "--name", "registry", "-p", "5000:5000", REGISTRY_IMAGE, "/entrypoint.sh", "/etc/docker/registry/config.yml"})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+
+		if !WaitContainerReady(podmanTest, "registry", "listening on", 20, 1) {
+			Skip("Cannot start docker registry.")
+		}
+
+		session = podmanTest.Podman([]string{"build", "-t", "imageone", "build/basicalpine"})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+
+		push := podmanTest.Podman([]string{"push", "--tls-verify=false", "--remove-signatures", "imageone", "localhost:5000/image"})
+		push.WaitWithDefaultTimeout()
+		Expect(push).Should(Exit(0))
+
+		session = podmanTest.Podman([]string{"run", "--rm", "--net", "host", "quay.io/skopeo/stable", "inspect", "--tls-verify=false", "--raw", "docker://localhost:5000/image:latest"})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+		output := session.OutputToString()
+		// Default compression is gzip and push with `--force-compression=false` no traces of `zstd` should be there.
+		Expect(output).ToNot(ContainSubstring("zstd"))
+
+		push = podmanTest.Podman([]string{"push", "--tls-verify=false", "--force-compression=false", "--compression-format", "zstd", "--remove-signatures", "imageone", "localhost:5000/image"})
+		push.WaitWithDefaultTimeout()
+		Expect(push).Should(Exit(0))
+
+		session = podmanTest.Podman([]string{"run", "--rm", "--net", "host", "quay.io/skopeo/stable", "inspect", "--tls-verify=false", "--raw", "docker://localhost:5000/image:latest"})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+		output = session.OutputToString()
+		// Although `--compression-format` is `zstd` but still no traces of `zstd` should be in image
+		// since blobs must be reused from last `gzip` image.
+		Expect(output).ToNot(ContainSubstring("zstd"))
+
+		push = podmanTest.Podman([]string{"push", "--tls-verify=false", "--compression-format", "zstd", "--force-compression", "--remove-signatures", "imageone", "localhost:5000/image"})
+		push.WaitWithDefaultTimeout()
+		Expect(push).Should(Exit(0))
+
+		session = podmanTest.Podman([]string{"run", "--rm", "--net", "host", "quay.io/skopeo/stable", "inspect", "--tls-verify=false", "--raw", "docker://localhost:5000/image:latest"})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+		output = session.OutputToString()
+		// Should contain `zstd` layer, substring `zstd` is enough to confirm in skopeo inspect output that `zstd` layer is present.
+		Expect(output).To(ContainSubstring("zstd"))
+	})
+
 	It("podman push to local registry", func() {
 		if podmanTest.Host.Arch == "ppc64le" {
 			Skip("No registry image for ppc64le")