From 469ace09103fec5b1f34569186815b68a46c5a0e Mon Sep 17 00:00:00 2001 From: Aditya R Date: Wed, 16 Aug 2023 12:20:12 +0530 Subject: [PATCH 1/3] push: add support for --force-compression Adds support for --force-compression which allows end-users to force push blobs with the selected compresison in --compression option, in order to make sure that blobs of other compression on registry are not reused. Is equivalent to: force-compression here: https://docs.docker.com/build/exporters/#compression Closes: https://github.com/containers/podman/issues/18660 Signed-off-by: Aditya R --- cmd/podman/images/push.go | 2 + .../markdown/podman-manifest-push.1.md.in | 4 ++ docs/source/markdown/podman-push.1.md.in | 4 ++ pkg/api/handlers/libpod/images_push.go | 36 ++++++------ pkg/api/server/register_images.go | 5 ++ pkg/bindings/images/types.go | 4 ++ pkg/bindings/images/types_push_options.go | 15 +++++ pkg/domain/entities/images.go | 4 ++ pkg/domain/infra/abi/images.go | 1 + pkg/domain/infra/tunnel/images.go | 2 +- test/e2e/push_test.go | 57 +++++++++++++++++++ 11 files changed, 116 insertions(+), 18 deletions(-) diff --git a/cmd/podman/images/push.go b/cmd/podman/images/push.go index 084550e43f..23e5437dca 100644 --- a/cmd/podman/images/push.go +++ b/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 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) diff --git a/docs/source/markdown/podman-manifest-push.1.md.in b/docs/source/markdown/podman-manifest-push.1.md.in index fcdcde66e5..84ea14ca75 100644 --- a/docs/source/markdown/podman-manifest-push.1.md.in +++ b/docs/source/markdown/podman-manifest-push.1.md.in @@ -42,6 +42,10 @@ the list or index itself. (Default true) @@option digestfile +#### **--force-compression** + +Use the specified compression algorithm even if the destination contains a differently-compressed variant already. + #### **--format**, **-f**=*format* Manifest list type (oci or v2s2) to use when pushing the list (default is oci). diff --git a/docs/source/markdown/podman-push.1.md.in b/docs/source/markdown/podman-push.1.md.in index b66ff4370d..b450bf9829 100644 --- a/docs/source/markdown/podman-push.1.md.in +++ b/docs/source/markdown/podman-push.1.md.in @@ -70,6 +70,10 @@ 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. +### **--force-compression** + +Use the specified compression algorithm even if the destination contains a differently-compressed variant already. + #### **--format**, **-f**=*format* Manifest Type (oci, v2s2, or v2s1) to use when pushing an image. diff --git a/pkg/api/handlers/libpod/images_push.go b/pkg/api/handlers/libpod/images_push.go index 4bd8393d5d..e0a5ac6664 100644 --- a/pkg/api/handlers/libpod/images_push.go +++ b/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,16 @@ 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()["tlsVerify"]; found { diff --git a/pkg/api/server/register_images.go b/pkg/api/server/register_images.go index 805af97f82..f3816d15f6 100644 --- a/pkg/api/server/register_images.go +++ b/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: Use the specified compression algorithm if the destination contains a differently-compressed variant already. + // type: boolean + // default: false + // - in: query // name: tlsVerify // description: Require TLS verification. // type: boolean diff --git a/pkg/bindings/images/types.go b/pkg/bindings/images/types.go index e8ac86ce5b..e5c58df00a 100644 --- a/pkg/bindings/images/types.go +++ b/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 diff --git a/pkg/bindings/images/types_push_options.go b/pkg/bindings/images/types_push_options.go index 7d5d38e1c9..770ffffd17 100644 --- a/pkg/bindings/images/types_push_options.go +++ b/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 diff --git a/pkg/domain/entities/images.go b/pkg/domain/entities/images.go index cd2062c928..e259f97503 100644 --- a/pkg/domain/entities/images.go +++ b/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. diff --git a/pkg/domain/infra/abi/images.go b/pkg/domain/infra/abi/images.go index 5b22158dd7..a5b163788c 100644 --- a/pkg/domain/infra/abi/images.go +++ b/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 == "" { diff --git a/pkg/domain/infra/tunnel/images.go b/pkg/domain/infra/tunnel/images.go index 6ed8d18b20..55a01a07c0 100644 --- a/pkg/domain/infra/tunnel/images.go +++ b/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) diff --git a/test/e2e/push_test.go b/test/e2e/push_test.go index 8c19a9e40e..bd59aeeda7 100644 --- a/test/e2e/push_test.go +++ b/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 no traces of `zstd` should be there. + Expect(output).ToNot(ContainSubstring("zstd")) + + push = podmanTest.Podman([]string{"push", "--tls-verify=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") From 82bd56be74d6f2b79ee7eb3fe2923562cad47a14 Mon Sep 17 00:00:00 2001 From: Aditya R Date: Wed, 16 Aug 2023 12:49:11 +0530 Subject: [PATCH 2/3] manifest-push: add support for --force-compression Adds support for --force-compression which allows end-users to force push blobs with the selected compresison in --compression option, in order to make sure that blobs of other compression on registry are not reused. Signed-off-by: Aditya R --- cmd/podman/manifest/push.go | 2 + .../markdown/podman-manifest-push.1.md.in | 2 + docs/source/markdown/podman-push.1.md.in | 4 +- pkg/api/handlers/libpod/manifests.go | 38 ++++++++-------- pkg/api/server/register_manifest.go | 5 +++ pkg/domain/infra/abi/manifest.go | 1 + pkg/domain/infra/tunnel/manifest.go | 2 +- test/e2e/manifest_test.go | 45 ++++++++++++++++++- 8 files changed, 78 insertions(+), 21 deletions(-) diff --git a/cmd/podman/manifest/push.go b/cmd/podman/manifest/push.go index 967bfbbd83..4bc8c6159c 100644 --- a/cmd/podman/manifest/push.go +++ b/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 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) diff --git a/docs/source/markdown/podman-manifest-push.1.md.in b/docs/source/markdown/podman-manifest-push.1.md.in index 84ea14ca75..4bfeccc83b 100644 --- a/docs/source/markdown/podman-manifest-push.1.md.in +++ b/docs/source/markdown/podman-manifest-push.1.md.in @@ -45,6 +45,8 @@ the list or index itself. (Default true) #### **--force-compression** Use the specified compression algorithm even if the destination contains a differently-compressed variant already. +Usually use for this flag arises when image is prior compressed and pushed using `--compression-format` with a different +compression algorithm and user now needs to overwrite those blobs with a new compression algorithm on the remote registry. #### **--format**, **-f**=*format* diff --git a/docs/source/markdown/podman-push.1.md.in b/docs/source/markdown/podman-push.1.md.in index b450bf9829..71a0c79052 100644 --- a/docs/source/markdown/podman-push.1.md.in +++ b/docs/source/markdown/podman-push.1.md.in @@ -70,9 +70,11 @@ 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. -### **--force-compression** +#### **--force-compression** Use the specified compression algorithm even if the destination contains a differently-compressed variant already. +Usually use for this flag arises when image is prior compressed and pushed using `--compression-format` with a different +compression algorithm and user now needs to overwrite those blobs with a new compression algorithm on the remote registry. #### **--format**, **-f**=*format* diff --git a/pkg/api/handlers/libpod/manifests.go b/pkg/api/handlers/libpod/manifests.go index 0434d5fbc0..5094796e43 100644 --- a/pkg/api/handlers/libpod/manifests.go +++ b/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,17 @@ 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 sys := runtime.SystemContext(); sys != nil { options.CertDir = sys.DockerCertPath diff --git a/pkg/api/server/register_manifest.go b/pkg/api/server/register_manifest.go index 282bcd8d85..624dcd4afe 100644 --- a/pkg/api/server/register_manifest.go +++ b/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: Use the specified compression algorithm if the destination contains a differently-compressed variant already. + // type: boolean + // default: false // - in: path // name: destination // type: string diff --git a/pkg/domain/infra/abi/manifest.go b/pkg/domain/infra/abi/manifest.go index 4acf09038a..b664f6678a 100644 --- a/pkg/domain/infra/abi/manifest.go +++ b/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 == "" { diff --git a/pkg/domain/infra/tunnel/manifest.go b/pkg/domain/infra/tunnel/manifest.go index d2bd4e762e..d1cb0274a1 100644 --- a/pkg/domain/infra/tunnel/manifest.go +++ b/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 { diff --git a/test/e2e/manifest_test.go b/test/e2e/manifest_test.go index 52ba578ba3..73d2978e28 100644 --- a/test/e2e/manifest_test.go +++ b/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 without --force-compression 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", "--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() { From 0938ee189939e50152b964344a316f5d747427ed Mon Sep 17 00:00:00 2001 From: Aditya R Date: Thu, 24 Aug 2023 13:34:25 +0530 Subject: [PATCH 3/3] push, manifest-push: --force-compression must be true with --compression-format Value of `--force-compression` should be already `true` is `--compression-format` is selected otherwise let users decide. Signed-off-by: Aditya R --- cmd/podman/images/push.go | 10 +++++++++- cmd/podman/manifest/push.go | 10 +++++++++- docs/source/markdown/options/force-compression.md | 8 ++++++++ docs/source/markdown/podman-manifest-push.1.md.in | 6 +----- docs/source/markdown/podman-push.1.md.in | 6 +----- pkg/api/handlers/libpod/images_push.go | 8 ++++++++ pkg/api/handlers/libpod/manifests.go | 7 +++++++ pkg/api/server/register_images.go | 2 +- pkg/api/server/register_manifest.go | 2 +- test/e2e/manifest_test.go | 4 ++-- test/e2e/push_test.go | 4 ++-- 11 files changed, 49 insertions(+), 18 deletions(-) create mode 100644 docs/source/markdown/options/force-compression.md diff --git a/cmd/podman/images/push.go b/cmd/podman/images/push.go index 23e5437dca..2cb721b2a7 100644 --- a/cmd/podman/images/push.go +++ b/cmd/podman/images/push.go @@ -102,7 +102,7 @@ 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 if the destination contains a differently-compressed variant already") + 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)") @@ -216,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) diff --git a/cmd/podman/manifest/push.go b/cmd/podman/manifest/push.go index 4bc8c6159c..baefb81564 100644 --- a/cmd/podman/manifest/push.go +++ b/cmd/podman/manifest/push.go @@ -72,7 +72,7 @@ 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 if the destination contains a differently-compressed variant already") + 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)") @@ -176,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 diff --git a/docs/source/markdown/options/force-compression.md b/docs/source/markdown/options/force-compression.md new file mode 100644 index 0000000000..6212320458 --- /dev/null +++ b/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. diff --git a/docs/source/markdown/podman-manifest-push.1.md.in b/docs/source/markdown/podman-manifest-push.1.md.in index 4bfeccc83b..d8cc8702ae 100644 --- a/docs/source/markdown/podman-manifest-push.1.md.in +++ b/docs/source/markdown/podman-manifest-push.1.md.in @@ -42,11 +42,7 @@ the list or index itself. (Default true) @@option digestfile -#### **--force-compression** - -Use the specified compression algorithm even if the destination contains a differently-compressed variant already. -Usually use for this flag arises when image is prior compressed and pushed using `--compression-format` with a different -compression algorithm and user now needs to overwrite those blobs with a new compression algorithm on the remote registry. +@@option force-compression #### **--format**, **-f**=*format* diff --git a/docs/source/markdown/podman-push.1.md.in b/docs/source/markdown/podman-push.1.md.in index 71a0c79052..49f08da6fe 100644 --- a/docs/source/markdown/podman-push.1.md.in +++ b/docs/source/markdown/podman-push.1.md.in @@ -70,11 +70,7 @@ 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. -#### **--force-compression** - -Use the specified compression algorithm even if the destination contains a differently-compressed variant already. -Usually use for this flag arises when image is prior compressed and pushed using `--compression-format` with a different -compression algorithm and user now needs to overwrite those blobs with a new compression algorithm on the remote registry. +@@option force-compression #### **--format**, **-f**=*format* diff --git a/pkg/api/handlers/libpod/images_push.go b/pkg/api/handlers/libpod/images_push.go index e0a5ac6664..2e587e650c 100644 --- a/pkg/api/handlers/libpod/images_push.go +++ b/pkg/api/handlers/libpod/images_push.go @@ -86,6 +86,14 @@ func PushImage(w http.ResponseWriter, r *http.Request) { 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 { options.SkipTLSVerify = types.NewOptionalBool(!query.TLSVerify) } diff --git a/pkg/api/handlers/libpod/manifests.go b/pkg/api/handlers/libpod/manifests.go index 5094796e43..4f5cb116de 100644 --- a/pkg/api/handlers/libpod/manifests.go +++ b/pkg/api/handlers/libpod/manifests.go @@ -385,6 +385,13 @@ func ManifestPush(w http.ResponseWriter, r *http.Request) { 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 } diff --git a/pkg/api/server/register_images.go b/pkg/api/server/register_images.go index f3816d15f6..1abe5a98f5 100644 --- a/pkg/api/server/register_images.go +++ b/pkg/api/server/register_images.go @@ -727,7 +727,7 @@ func (s *APIServer) registerImagesHandlers(r *mux.Router) error { // description: Allows for pushing the image to a different destination than the image refers to. // - in: query // name: forceCompressionFormat - // description: Use the specified compression algorithm if the destination contains a differently-compressed variant already. + // 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 diff --git a/pkg/api/server/register_manifest.go b/pkg/api/server/register_manifest.go index 624dcd4afe..a79d49340a 100644 --- a/pkg/api/server/register_manifest.go +++ b/pkg/api/server/register_manifest.go @@ -69,7 +69,7 @@ func (s *APIServer) registerManifestHandlers(r *mux.Router) error { // type: string // - in: query // name: forceCompressionFormat - // description: Use the specified compression algorithm if the destination contains a differently-compressed variant already. + // 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 diff --git a/test/e2e/manifest_test.go b/test/e2e/manifest_test.go index 73d2978e28..a20522aa10 100644 --- a/test/e2e/manifest_test.go +++ b/test/e2e/manifest_test.go @@ -230,9 +230,9 @@ var _ = Describe("Podman manifest", func() { Expect(verifyInstanceCompression(index.Manifests, "gzip", "arm64")).Should(BeTrue()) Expect(verifyInstanceCompression(index.Manifests, "gzip", "amd64")).Should(BeTrue()) - // Note: Pushing again without --force-compression should produce in-correct/wrong result since blobs are already present in registry so they will be reused + // 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", "--tls-verify=false", "--remove-signatures", "foobar", "localhost:5000/list"}) + 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() diff --git a/test/e2e/push_test.go b/test/e2e/push_test.go index bd59aeeda7..3073b161a6 100644 --- a/test/e2e/push_test.go +++ b/test/e2e/push_test.go @@ -114,10 +114,10 @@ var _ = Describe("Podman push", func() { session.WaitWithDefaultTimeout() Expect(session).Should(Exit(0)) output := session.OutputToString() - // Default compression is gzip no traces of `zstd` should be there. + // 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", "--compression-format", "zstd", "--remove-signatures", "imageone", "localhost:5000/image"}) + 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))