From 1f1525f2e184882c2c3b0f02f353e44fd468238e Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Thu, 20 Apr 2023 14:13:59 +0200 Subject: [PATCH] support `--digestfile` for remote push Wire in support for writing the digest of the pushed image to a user-specified file. Requires some massaging of _internal_ APIs and the extension of the push endpoint to integrate the raw manifest (i.e., in bytes) in the stream. Closes: #18216 Signed-off-by: Valentin Rothberg --- cmd/podman/images/push.go | 15 ++++++- cmd/podman/manifest/push.go | 1 + docs/source/markdown/options/digestfile.md | 1 - pkg/api/handlers/compat/images_push.go | 50 ++++++++-------------- pkg/api/handlers/libpod/images_push.go | 9 +++- pkg/bindings/images/push.go | 2 + pkg/bindings/images/types.go | 3 ++ pkg/bindings/images/types_push_options.go | 15 +++++++ pkg/domain/entities/engine_image.go | 2 +- pkg/domain/entities/images.go | 11 +++-- pkg/domain/infra/abi/images.go | 31 ++++++-------- pkg/domain/infra/tunnel/images.go | 11 +++-- test/e2e/push_test.go | 18 ++++---- 13 files changed, 97 insertions(+), 72 deletions(-) diff --git a/cmd/podman/images/push.go b/cmd/podman/images/push.go index 51b53c3d07..490fb6b406 100644 --- a/cmd/podman/images/push.go +++ b/cmd/podman/images/push.go @@ -24,6 +24,7 @@ type pushOptionsWrapper struct { SignBySigstoreParamFileCLI string EncryptionKeys []string EncryptLayers []int + DigestFile string } var ( @@ -140,7 +141,6 @@ func pushFlags(cmd *cobra.Command) { if registry.IsRemote() { _ = flags.MarkHidden("cert-dir") _ = flags.MarkHidden("compress") - _ = flags.MarkHidden("digestfile") _ = flags.MarkHidden("quiet") _ = flags.MarkHidden(signByFlagName) _ = flags.MarkHidden(signBySigstoreFlagName) @@ -203,5 +203,16 @@ func imagePush(cmd *cobra.Command, args []string) error { // Let's do all the remaining Yoga in the API to prevent us from scattering // logic across (too) many parts of the code. - return registry.ImageEngine().Push(registry.GetContext(), source, destination, pushOptions.ImagePushOptions) + report, err := registry.ImageEngine().Push(registry.GetContext(), source, destination, pushOptions.ImagePushOptions) + if err != nil { + return err + } + + if pushOptions.DigestFile != "" { + if err := os.WriteFile(pushOptions.DigestFile, []byte(report.ManifestDigest), 0644); err != nil { + return err + } + } + + return nil } diff --git a/cmd/podman/manifest/push.go b/cmd/podman/manifest/push.go index 120e81a53b..a2d1eba8f6 100644 --- a/cmd/podman/manifest/push.go +++ b/cmd/podman/manifest/push.go @@ -25,6 +25,7 @@ type manifestPushOptsWrapper struct { CredentialsCLI string SignBySigstoreParamFileCLI string SignPassphraseFileCLI string + DigestFile string } var ( diff --git a/docs/source/markdown/options/digestfile.md b/docs/source/markdown/options/digestfile.md index ce569acac8..64166b94ff 100644 --- a/docs/source/markdown/options/digestfile.md +++ b/docs/source/markdown/options/digestfile.md @@ -5,4 +5,3 @@ #### **--digestfile**=*Digestfile* After copying the image, write the digest of the resulting image to the file. -(This option is not available with the remote Podman client, including Mac and Windows (excluding WSL2) machines) diff --git a/pkg/api/handlers/compat/images_push.go b/pkg/api/handlers/compat/images_push.go index e1655a3bc1..188ba9c678 100644 --- a/pkg/api/handlers/compat/images_push.go +++ b/pkg/api/handlers/compat/images_push.go @@ -4,9 +4,7 @@ import ( "encoding/json" "errors" "fmt" - "io" "net/http" - "os" "strings" "github.com/containers/image/v5/types" @@ -27,13 +25,6 @@ func PushImage(w http.ResponseWriter, r *http.Request) { decoder := r.Context().Value(api.DecoderKey).(*schema.Decoder) runtime := r.Context().Value(api.RuntimeKey).(*libpod.Runtime) - digestFile, err := os.CreateTemp("", "digest.txt") - if err != nil { - utils.Error(w, http.StatusInternalServerError, fmt.Errorf("unable to create tempfile: %w", err)) - return - } - defer digestFile.Close() - // Now use the ABI implementation to prevent us from having duplicate // code. imageEngine := abi.ImageEngine{Libpod: runtime} @@ -97,15 +88,14 @@ func PushImage(w http.ResponseWriter, r *http.Request) { password = authconf.Password } options := entities.ImagePushOptions{ - All: query.All, - Authfile: authfile, - Compress: query.Compress, - Format: query.Format, - Password: password, - Username: username, - DigestFile: digestFile.Name(), - Quiet: true, - Progress: make(chan types.ProgressProperties), + All: query.All, + Authfile: authfile, + Compress: query.Compress, + Format: query.Format, + Password: password, + Username: username, + Quiet: true, + Progress: make(chan types.ProgressProperties), } if _, found := r.URL.Query()["tlsVerify"]; found { options.SkipTLSVerify = types.NewOptionalBool(!query.TLSVerify) @@ -138,8 +128,11 @@ func PushImage(w http.ResponseWriter, r *http.Request) { flush() pushErrChan := make(chan error) + var pushReport *entities.ImagePushReport go func() { - pushErrChan <- imageEngine.Push(r.Context(), imageName, destination, options) + var err error + pushReport, err = imageEngine.Push(r.Context(), imageName, destination, options) + pushErrChan <- err }() loop: // break out of for/select infinite loop @@ -187,23 +180,16 @@ loop: // break out of for/select infinite loop break loop } - digestBytes, err := io.ReadAll(digestFile) - if err != nil { - report.Error = &jsonmessage.JSONError{ - Message: err.Error(), - } - report.ErrorMessage = err.Error() - if err := enc.Encode(report); err != nil { - logrus.Warnf("Failed to json encode error %q", err.Error()) - } - flush() - break loop - } tag := query.Tag if tag == "" { tag = "latest" } - report.Status = fmt.Sprintf("%s: digest: %s size: %d", tag, string(digestBytes), len(rawManifest)) + + var digestStr string + if pushReport != nil { + digestStr = pushReport.ManifestDigest + } + report.Status = fmt.Sprintf("%s: digest: %s size: %d", tag, digestStr, len(rawManifest)) if err := enc.Encode(report); err != nil { logrus.Warnf("Failed to json encode error %q", err.Error()) } diff --git a/pkg/api/handlers/libpod/images_push.go b/pkg/api/handlers/libpod/images_push.go index bd907a4458..1241a07759 100644 --- a/pkg/api/handlers/libpod/images_push.go +++ b/pkg/api/handlers/libpod/images_push.go @@ -90,7 +90,8 @@ func PushImage(w http.ResponseWriter, r *http.Request) { // Let's keep thing simple when running in quiet mode and push directly. if query.Quiet { - if err := imageEngine.Push(r.Context(), source, destination, options); err != nil { + _, err := imageEngine.Push(r.Context(), source, destination, options) + if err != nil { utils.Error(w, http.StatusBadRequest, fmt.Errorf("pushing image %q: %w", destination, err)) return } @@ -104,9 +105,10 @@ func PushImage(w http.ResponseWriter, r *http.Request) { pushCtx, pushCancel := context.WithCancel(r.Context()) var pushError error + var pushReport *entities.ImagePushReport go func() { defer pushCancel() - pushError = imageEngine.Push(pushCtx, source, destination, options) + pushReport, pushError = imageEngine.Push(pushCtx, source, destination, options) }() flush := func() { @@ -131,6 +133,9 @@ func PushImage(w http.ResponseWriter, r *http.Request) { } flush() case <-pushCtx.Done(): + if pushReport != nil { + stream.ManifestDigest = pushReport.ManifestDigest + } if pushError != nil { stream.Error = pushError.Error() if err := enc.Encode(stream); err != nil { diff --git a/pkg/bindings/images/push.go b/pkg/bindings/images/push.go index dae5da2b5f..ea1d96823e 100644 --- a/pkg/bindings/images/push.go +++ b/pkg/bindings/images/push.go @@ -87,6 +87,8 @@ LOOP: switch { case report.Stream != "": fmt.Fprint(writer, report.Stream) + case report.ManifestDigest != "": + options.ManifestDigest = &report.ManifestDigest case report.Error != "": // There can only be one error. return errors.New(report.Error) diff --git a/pkg/bindings/images/types.go b/pkg/bindings/images/types.go index a83ad206aa..36fc06ffcf 100644 --- a/pkg/bindings/images/types.go +++ b/pkg/bindings/images/types.go @@ -158,6 +158,9 @@ type PushOptions struct { Username *string `schema:"-"` // Quiet can be specified to suppress progress when pushing. Quiet *bool + + // Manifest of the pushed image. Set by images.Push. + ManifestDigest *string } // SearchOptions are optional options for searching images on registries diff --git a/pkg/bindings/images/types_push_options.go b/pkg/bindings/images/types_push_options.go index 817d873f8c..71f3d9e3eb 100644 --- a/pkg/bindings/images/types_push_options.go +++ b/pkg/bindings/images/types_push_options.go @@ -182,3 +182,18 @@ func (o *PushOptions) GetQuiet() bool { } return *o.Quiet } + +// WithManifestDigest set field ManifestDigest to given value +func (o *PushOptions) WithManifestDigest(value string) *PushOptions { + o.ManifestDigest = &value + return o +} + +// GetManifestDigest returns value of field ManifestDigest +func (o *PushOptions) GetManifestDigest() string { + if o.ManifestDigest == nil { + var z string + return z + } + return *o.ManifestDigest +} diff --git a/pkg/domain/entities/engine_image.go b/pkg/domain/entities/engine_image.go index 0de0a889ca..378da383dc 100644 --- a/pkg/domain/entities/engine_image.go +++ b/pkg/domain/entities/engine_image.go @@ -20,7 +20,7 @@ type ImageEngine interface { //nolint:interfacebloat Mount(ctx context.Context, images []string, options ImageMountOptions) ([]*ImageMountReport, error) Prune(ctx context.Context, opts ImagePruneOptions) ([]*reports.PruneReport, error) Pull(ctx context.Context, rawImage string, opts ImagePullOptions) (*ImagePullReport, error) - Push(ctx context.Context, source string, destination string, opts ImagePushOptions) error + Push(ctx context.Context, source string, destination string, opts ImagePushOptions) (*ImagePushReport, error) Remove(ctx context.Context, images []string, opts ImageRemoveOptions) (*ImageRemoveReport, []error) Save(ctx context.Context, nameOrID string, tags []string, options ImageSaveOptions) error Scp(ctx context.Context, src, dst string, parentFlags []string, quiet bool, sshMode ssh.EngineMode) error diff --git a/pkg/domain/entities/images.go b/pkg/domain/entities/images.go index f56cf15b44..4b65e6e310 100644 --- a/pkg/domain/entities/images.go +++ b/pkg/domain/entities/images.go @@ -195,9 +195,6 @@ type ImagePushOptions struct { Username string // Password for authenticating against the registry. Password string - // DigestFile, after copying the image, write the digest of the resulting - // image to the file. Ignored for remote calls. - DigestFile string // Format is the Manifest type (oci, v2s1, or v2s2) to use when pushing an // image. Default is manifest type of source, with fallbacks. // Ignored for remote calls. @@ -247,9 +244,17 @@ type ImagePushOptions struct { OciEncryptLayers *[]int } +// ImagePushReport is the response from pushing an image. +type ImagePushReport struct { + // The digest of the manifest of the pushed image. + ManifestDigest string +} + // ImagePushStream is the response from pushing an image. Only used in the // remote API. type ImagePushStream struct { + // ManifestDigest is the digest of the manifest of the pushed image. + ManifestDigest string `json:"manifestdigest,omitempty"` // Stream used to provide push progress Stream string `json:"stream,omitempty"` // Error contains text of errors from pushing diff --git a/pkg/domain/infra/abi/images.go b/pkg/domain/infra/abi/images.go index 3aaf4f4018..4e4ec294dd 100644 --- a/pkg/domain/infra/abi/images.go +++ b/pkg/domain/infra/abi/images.go @@ -281,7 +281,7 @@ func (ir *ImageEngine) Inspect(ctx context.Context, namesOrIDs []string, opts en return reports, errs, nil } -func (ir *ImageEngine) Push(ctx context.Context, source string, destination string, options entities.ImagePushOptions) error { +func (ir *ImageEngine) Push(ctx context.Context, source string, destination string, options entities.ImagePushOptions) (*entities.ImagePushReport, error) { var manifestType string switch options.Format { case "": @@ -293,7 +293,7 @@ func (ir *ImageEngine) Push(ctx context.Context, source string, destination stri case "v2s2", "docker": manifestType = manifest.DockerV2Schema2MediaType default: - return fmt.Errorf("unknown format %q. Choose on of the supported formats: 'oci', 'v2s1', or 'v2s2'", options.Format) + return nil, fmt.Errorf("unknown format %q. Choose on of the supported formats: 'oci', 'v2s1', or 'v2s2'", options.Format) } pushOptions := &libimage.PushOptions{} @@ -320,14 +320,14 @@ func (ir *ImageEngine) Push(ctx context.Context, source string, destination stri if compressionFormat == "" { config, err := ir.Libpod.GetConfigNoCopy() if err != nil { - return err + return nil, err } compressionFormat = config.Engine.CompressionFormat } if compressionFormat != "" { algo, err := compression.AlgorithmByName(compressionFormat) if err != nil { - return err + return nil, err } pushOptions.CompressionFormat = &algo } @@ -338,27 +338,24 @@ func (ir *ImageEngine) Push(ctx context.Context, source string, destination stri pushedManifestBytes, pushError := ir.Libpod.LibimageRuntime().Push(ctx, source, destination, pushOptions) if pushError == nil { - if options.DigestFile != "" { - manifestDigest, err := manifest.Digest(pushedManifestBytes) - if err != nil { - return err - } - - if err := os.WriteFile(options.DigestFile, []byte(manifestDigest.String()), 0644); err != nil { - return err - } + manifestDigest, err := manifest.Digest(pushedManifestBytes) + if err != nil { + return nil, err } - return nil + return &entities.ImagePushReport{ManifestDigest: manifestDigest.String()}, nil } // If the image could not be found, we may be referring to a manifest // list but could not find a matching image instance in the local // containers storage. In that case, fall back and attempt to push the // (entire) manifest. if _, err := ir.Libpod.LibimageRuntime().LookupManifestList(source); err == nil { - _, err := ir.ManifestPush(ctx, source, destination, options) - return err + pushedManifestString, err := ir.ManifestPush(ctx, source, destination, options) + if err != nil { + return nil, err + } + return &entities.ImagePushReport{ManifestDigest: pushedManifestString}, nil } - return pushError + return nil, pushError } func (ir *ImageEngine) Tag(ctx context.Context, nameOrID string, tags []string, options entities.ImageTagOptions) error { diff --git a/pkg/domain/infra/tunnel/images.go b/pkg/domain/infra/tunnel/images.go index f249769f73..0d92a53098 100644 --- a/pkg/domain/infra/tunnel/images.go +++ b/pkg/domain/infra/tunnel/images.go @@ -243,12 +243,12 @@ func (ir *ImageEngine) Import(ctx context.Context, opts entities.ImageImportOpti return images.Import(ir.ClientCtx, f, options) } -func (ir *ImageEngine) Push(ctx context.Context, source string, destination string, opts entities.ImagePushOptions) error { +func (ir *ImageEngine) Push(ctx context.Context, source string, destination string, opts entities.ImagePushOptions) (*entities.ImagePushReport, error) { if opts.Signers != nil { - return fmt.Errorf("forwarding Signers is not supported for remote clients") + return nil, fmt.Errorf("forwarding Signers is not supported for remote clients") } if opts.OciEncryptConfig != nil { - return fmt.Errorf("encryption is not supported for remote clients") + return nil, fmt.Errorf("encryption is not supported for remote clients") } options := new(images.PushOptions) @@ -261,7 +261,10 @@ func (ir *ImageEngine) Push(ctx context.Context, source string, destination stri options.WithSkipTLSVerify(false) } } - return images.Push(ir.ClientCtx, source, destination, options) + if err := images.Push(ir.ClientCtx, source, destination, options); err != nil { + return nil, err + } + return &entities.ImagePushReport{ManifestDigest: options.GetManifestDigest()}, nil } func (ir *ImageEngine) Save(ctx context.Context, nameOrID string, tags []string, opts entities.ImageSaveOptions) error { diff --git a/test/e2e/push_test.go b/test/e2e/push_test.go index 12507c7eaf..f52c887488 100644 --- a/test/e2e/push_test.go +++ b/test/e2e/push_test.go @@ -137,16 +137,14 @@ var _ = Describe("Podman push", func() { Expect(push).Should(Exit(0)) } - if !IsRemote() { // Remote does not support --digestfile - // Test --digestfile option - digestFile := filepath.Join(podmanTest.TempDir, "digestfile.txt") - push2 := podmanTest.Podman([]string{"push", "--tls-verify=false", "--digestfile=" + digestFile, "--remove-signatures", ALPINE, "localhost:5000/my-alpine"}) - push2.WaitWithDefaultTimeout() - fi, err := os.Lstat(digestFile) - Expect(err).ToNot(HaveOccurred()) - Expect(fi.Name()).To(Equal("digestfile.txt")) - Expect(push2).Should(Exit(0)) - } + // Test --digestfile option + digestFile := filepath.Join(podmanTest.TempDir, "digestfile.txt") + push2 := podmanTest.Podman([]string{"push", "--tls-verify=false", "--digestfile=" + digestFile, "--remove-signatures", ALPINE, "localhost:5000/my-alpine"}) + push2.WaitWithDefaultTimeout() + fi, err := os.Lstat(digestFile) + Expect(err).ToNot(HaveOccurred()) + Expect(fi.Name()).To(Equal("digestfile.txt")) + Expect(push2).Should(Exit(0)) if !IsRemote() { // Remote does not support signing By("pushing and pulling with --sign-by-sigstore-private-key")