diff --git a/cmd/podman/containers/create.go b/cmd/podman/containers/create.go index f06869c4eb..52cf8fea41 100644 --- a/cmd/podman/containers/create.go +++ b/cmd/podman/containers/create.go @@ -8,7 +8,6 @@ import ( "strings" "github.com/containers/common/pkg/config" - storageTransport "github.com/containers/image/v5/storage" "github.com/containers/image/v5/transports/alltransports" "github.com/containers/podman/v3/cmd/podman/common" "github.com/containers/podman/v3/cmd/podman/registry" @@ -16,9 +15,7 @@ import ( "github.com/containers/podman/v3/pkg/domain/entities" "github.com/containers/podman/v3/pkg/specgen" "github.com/containers/podman/v3/pkg/util" - "github.com/containers/storage" "github.com/pkg/errors" - "github.com/sirupsen/logrus" "github.com/spf13/cobra" ) @@ -238,30 +235,12 @@ func createInit(c *cobra.Command) error { return nil } -// TODO: we should let the backend take care of the pull policy (which it -// does!). The code below is at risk of causing regression and code divergence. func pullImage(imageName string) (string, error) { pullPolicy, err := config.ValidatePullPolicy(cliVals.Pull) if err != nil { return "", err } - // Check if the image is missing and hence if we need to pull it. - imageMissing := true - imageRef, err := alltransports.ParseImageName(imageName) - switch { - case err != nil: - // Assume we specified a local image without the explicit storage transport. - fallthrough - - case imageRef.Transport().Name() == storageTransport.Transport.Name(): - br, err := registry.ImageEngine().Exists(registry.GetContext(), imageName) - if err != nil { - return "", err - } - imageMissing = !br.Value - } - if cliVals.Platform != "" || cliVals.Arch != "" || cliVals.OS != "" { if cliVals.Platform != "" { if cliVals.Arch != "" || cliVals.OS != "" { @@ -273,31 +252,28 @@ func pullImage(imageName string) (string, error) { cliVals.Arch = split[1] } } + } - if pullPolicy != config.PullPolicyAlways { - logrus.Info("--platform --arch and --os causes the pull policy to be \"always\"") - pullPolicy = config.PullPolicyAlways - } + pullReport, pullErr := registry.ImageEngine().Pull(registry.GetContext(), imageName, entities.ImagePullOptions{ + Authfile: cliVals.Authfile, + Quiet: cliVals.Quiet, + Arch: cliVals.Arch, + OS: cliVals.OS, + Variant: cliVals.Variant, + SignaturePolicy: cliVals.SignaturePolicy, + PullPolicy: pullPolicy, + }) + if pullErr != nil { + return "", pullErr } - if imageMissing || pullPolicy == config.PullPolicyAlways { - if pullPolicy == config.PullPolicyNever { - return "", errors.Wrap(storage.ErrImageUnknown, imageName) - } - pullReport, pullErr := registry.ImageEngine().Pull(registry.GetContext(), imageName, entities.ImagePullOptions{ - Authfile: cliVals.Authfile, - Quiet: cliVals.Quiet, - Arch: cliVals.Arch, - OS: cliVals.OS, - Variant: cliVals.Variant, - SignaturePolicy: cliVals.SignaturePolicy, - PullPolicy: pullPolicy, - }) - if pullErr != nil { - return "", pullErr - } + // Return the input name such that the image resolves to correct + // repo/tag in the backend (see #8082). Unless we're referring to + // the image via a transport. + if _, err := alltransports.ParseImageName(imageName); err == nil { imageName = pullReport.Images[0] } + return imageName, nil } diff --git a/pkg/api/handlers/compat/containers_create.go b/pkg/api/handlers/compat/containers_create.go index 162a98135b..15eb86422d 100644 --- a/pkg/api/handlers/compat/containers_create.go +++ b/pkg/api/handlers/compat/containers_create.go @@ -71,13 +71,12 @@ func CreateContainer(w http.ResponseWriter, r *http.Request) { imgNameOrID := newImage.ID() // if the img had multi names with the same sha256 ID, should use the InputName, not the ID if len(newImage.Names()) > 1 { - imageRef, err := utils.ParseDockerReference(resolvedName) - if err != nil { + if err := utils.IsRegistryReference(resolvedName); err != nil { utils.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest, err) return } // maybe the InputName has no tag, so use full name to display - imgNameOrID = imageRef.DockerReference().String() + imgNameOrID = resolvedName } sg := specgen.NewSpecGenerator(imgNameOrID, cliOpts.RootFS) diff --git a/pkg/api/handlers/libpod/images.go b/pkg/api/handlers/libpod/images.go index a90408bfdd..fc6ab4b4c4 100644 --- a/pkg/api/handlers/libpod/images.go +++ b/pkg/api/handlers/libpod/images.go @@ -482,7 +482,7 @@ func PushImage(w http.ResponseWriter, r *http.Request) { destination = source } - if _, err := utils.ParseDockerReference(destination); err != nil { + if err := utils.IsRegistryReference(destination); err != nil { utils.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest, err) return } diff --git a/pkg/api/handlers/libpod/images_pull.go b/pkg/api/handlers/libpod/images_pull.go index 73d08a26e1..04b4156386 100644 --- a/pkg/api/handlers/libpod/images_pull.go +++ b/pkg/api/handlers/libpod/images_pull.go @@ -26,14 +26,16 @@ func ImagesPull(w http.ResponseWriter, r *http.Request) { runtime := r.Context().Value("runtime").(*libpod.Runtime) decoder := r.Context().Value("decoder").(*schema.Decoder) query := struct { - Reference string `schema:"reference"` - OS string `schema:"OS"` - Arch string `schema:"Arch"` - Variant string `schema:"Variant"` - TLSVerify bool `schema:"tlsVerify"` - AllTags bool `schema:"allTags"` + Reference string `schema:"reference"` + OS string `schema:"OS"` + Arch string `schema:"Arch"` + Variant string `schema:"Variant"` + TLSVerify bool `schema:"tlsVerify"` + AllTags bool `schema:"allTags"` + PullPolicy string `schema:"policy"` }{ - TLSVerify: true, + TLSVerify: true, + PullPolicy: "always", } if err := decoder.Decode(&query, r.URL.Query()); err != nil { @@ -48,7 +50,7 @@ func ImagesPull(w http.ResponseWriter, r *http.Request) { } // Make sure that the reference has no transport or the docker one. - if _, err := utils.ParseDockerReference(query.Reference); err != nil { + if err := utils.IsRegistryReference(query.Reference); err != nil { utils.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest, err) return } @@ -83,12 +85,18 @@ func ImagesPull(w http.ResponseWriter, r *http.Request) { pullOptions.Writer = writer + pullPolicy, err := config.ParsePullPolicy(query.PullPolicy) + if err != nil { + utils.Error(w, "failed to parse pull policy", http.StatusBadRequest, err) + return + } + var pulledImages []*libimage.Image var pullError error runCtx, cancel := context.WithCancel(r.Context()) go func() { defer cancel() - pulledImages, pullError = runtime.LibimageRuntime().Pull(runCtx, query.Reference, config.PullPolicyAlways, pullOptions) + pulledImages, pullError = runtime.LibimageRuntime().Pull(runCtx, query.Reference, pullPolicy, pullOptions) }() flush := func() { diff --git a/pkg/api/handlers/libpod/manifests.go b/pkg/api/handlers/libpod/manifests.go index f21eb2e80f..2f36db583a 100644 --- a/pkg/api/handlers/libpod/manifests.go +++ b/pkg/api/handlers/libpod/manifests.go @@ -169,7 +169,7 @@ func ManifestPush(w http.ResponseWriter, r *http.Request) { errors.Wrapf(err, "failed to parse parameters for %s", r.URL.String())) return } - if _, err := utils.ParseDockerReference(query.Destination); err != nil { + if err := utils.IsRegistryReference(query.Destination); err != nil { utils.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest, err) return } diff --git a/pkg/api/handlers/utils/images.go b/pkg/api/handlers/utils/images.go index 2662cd368b..2a1908d63e 100644 --- a/pkg/api/handlers/utils/images.go +++ b/pkg/api/handlers/utils/images.go @@ -15,22 +15,19 @@ import ( "github.com/pkg/errors" ) -// ParseDockerReference parses the specified image name to a -// `types.ImageReference` and enforces it to refer to a docker-transport -// reference. -func ParseDockerReference(name string) (types.ImageReference, error) { - dockerPrefix := fmt.Sprintf("%s://", docker.Transport.Name()) +// IsRegistryReference checks if the specified name points to the "docker://" +// transport. If it points to no supported transport, we'll assume a +// non-transport reference pointing to an image (e.g., "fedora:latest"). +func IsRegistryReference(name string) error { imageRef, err := alltransports.ParseImageName(name) - if err == nil && imageRef.Transport().Name() != docker.Transport.Name() { - return nil, errors.Errorf("reference %q must be a docker reference", name) - } else if err != nil { - origErr := err - imageRef, err = alltransports.ParseImageName(fmt.Sprintf("%s%s", dockerPrefix, name)) - if err != nil { - return nil, errors.Wrapf(origErr, "reference %q must be a docker reference", name) - } + if err != nil { + // No supported transport -> assume a docker-stype reference. + return nil } - return imageRef, nil + if imageRef.Transport().Name() == docker.Transport.Name() { + return nil + } + return errors.Errorf("unsupport transport %s in %q: only docker transport is supported", imageRef.Transport().Name(), name) } // ParseStorageReference parses the specified image name to a diff --git a/pkg/api/server/register_images.go b/pkg/api/server/register_images.go index b32c0df206..2641809ee5 100644 --- a/pkg/api/server/register_images.go +++ b/pkg/api/server/register_images.go @@ -964,6 +964,10 @@ func (s *APIServer) registerImagesHandlers(r *mux.Router) error { // description: Pull image for the specified variant. // type: string // - in: query + // name: policy + // description: Pull policy, "always" (default), "missing", "newer", "never". + // type: string + // - in: query // name: tlsVerify // description: Require TLS verification. // type: boolean diff --git a/pkg/bindings/images/pull.go b/pkg/bindings/images/pull.go index 9780c3bfff..7dfe9560c2 100644 --- a/pkg/bindings/images/pull.go +++ b/pkg/bindings/images/pull.go @@ -13,7 +13,7 @@ import ( "github.com/containers/podman/v3/pkg/auth" "github.com/containers/podman/v3/pkg/bindings" "github.com/containers/podman/v3/pkg/domain/entities" - "github.com/hashicorp/go-multierror" + "github.com/containers/podman/v3/pkg/errorhandling" "github.com/pkg/errors" ) @@ -65,7 +65,7 @@ func Pull(ctx context.Context, rawImage string, options *PullOptions) ([]string, dec := json.NewDecoder(response.Body) var images []string - var mErr error + var pullErrors []error for { var report entities.ImagePullReport if err := dec.Decode(&report); err != nil { @@ -77,7 +77,7 @@ func Pull(ctx context.Context, rawImage string, options *PullOptions) ([]string, select { case <-response.Request.Context().Done(): - return images, mErr + break default: // non-blocking select } @@ -86,7 +86,7 @@ func Pull(ctx context.Context, rawImage string, options *PullOptions) ([]string, case report.Stream != "": fmt.Fprint(stderr, report.Stream) case report.Error != "": - mErr = multierror.Append(mErr, errors.New(report.Error)) + pullErrors = append(pullErrors, errors.New(report.Error)) case len(report.Images) > 0: images = report.Images case report.ID != "": @@ -94,5 +94,5 @@ func Pull(ctx context.Context, rawImage string, options *PullOptions) ([]string, return images, errors.Errorf("failed to parse pull results stream, unexpected input: %v", report) } } - return images, mErr + return images, errorhandling.JoinErrors(pullErrors) } diff --git a/pkg/bindings/images/types.go b/pkg/bindings/images/types.go index 1f3e46729f..0aa75a81e1 100644 --- a/pkg/bindings/images/types.go +++ b/pkg/bindings/images/types.go @@ -147,6 +147,9 @@ type PullOptions struct { // OS will overwrite the local operating system (OS) for image // pulls. OS *string + // Policy is the pull policy. Supported values are "missing", "never", + // "newer", "always". An empty string defaults to "always". + Policy *string // Password for authenticating against the registry. Password *string // Quiet can be specified to suppress pull progress when pulling. Ignored diff --git a/pkg/bindings/images/types_pull_options.go b/pkg/bindings/images/types_pull_options.go index 0611c4447d..8fcf499eb9 100644 --- a/pkg/bindings/images/types_pull_options.go +++ b/pkg/bindings/images/types_pull_options.go @@ -84,6 +84,22 @@ func (o *PullOptions) GetOS() string { return *o.OS } +// WithPolicy +func (o *PullOptions) WithPolicy(value string) *PullOptions { + v := &value + o.Policy = v + return o +} + +// GetPolicy +func (o *PullOptions) GetPolicy() string { + var policy string + if o.Policy == nil { + return policy + } + return *o.Policy +} + // WithPassword func (o *PullOptions) WithPassword(value string) *PullOptions { v := &value diff --git a/pkg/domain/infra/tunnel/images.go b/pkg/domain/infra/tunnel/images.go index 3fd9a755d3..42027a2dc3 100644 --- a/pkg/domain/infra/tunnel/images.go +++ b/pkg/domain/infra/tunnel/images.go @@ -107,7 +107,7 @@ func (ir *ImageEngine) Pull(ctx context.Context, rawImage string, opts entities. options := new(images.PullOptions) options.WithAllTags(opts.AllTags).WithAuthfile(opts.Authfile).WithArch(opts.Arch).WithOS(opts.OS) options.WithVariant(opts.Variant).WithPassword(opts.Password) - options.WithQuiet(opts.Quiet).WithUsername(opts.Username) + options.WithQuiet(opts.Quiet).WithUsername(opts.Username).WithPolicy(opts.PullPolicy.String()) if s := opts.SkipTLSVerify; s != types.OptionalBoolUndefined { if s == types.OptionalBoolTrue { options.WithSkipTLSVerify(true) diff --git a/pkg/errorhandling/errorhandling.go b/pkg/errorhandling/errorhandling.go index 9b1740006f..6adbc9f344 100644 --- a/pkg/errorhandling/errorhandling.go +++ b/pkg/errorhandling/errorhandling.go @@ -15,6 +15,12 @@ func JoinErrors(errs []error) error { return nil } + // If there's just one error, return it. This prevents the "%d errors + // occurred:" header plus list from the multierror package. + if len(errs) == 1 { + return errs[0] + } + // `multierror` appends new lines which we need to remove to prevent // blank lines when printing the error. var multiE *multierror.Error @@ -24,9 +30,6 @@ func JoinErrors(errs []error) error { if finalErr == nil { return finalErr } - if len(multiE.WrappedErrors()) == 1 && logrus.IsLevelEnabled(logrus.TraceLevel) { - return multiE.WrappedErrors()[0] - } return errors.New(strings.TrimSpace(finalErr.Error())) } diff --git a/pkg/specgen/generate/container.go b/pkg/specgen/generate/container.go index d00e51e825..e7276892d8 100644 --- a/pkg/specgen/generate/container.go +++ b/pkg/specgen/generate/container.go @@ -24,7 +24,8 @@ func CompleteSpec(ctx context.Context, r *libpod.Runtime, s *specgen.SpecGenerat var inspectData *libimage.ImageData var err error if s.Image != "" { - newImage, _, err = r.LibimageRuntime().LookupImage(s.Image, nil) + lookupOptions := &libimage.LookupImageOptions{IgnorePlatform: true} + newImage, _, err = r.LibimageRuntime().LookupImage(s.Image, lookupOptions) if err != nil { return nil, err } diff --git a/pkg/specgen/generate/container_create.go b/pkg/specgen/generate/container_create.go index 7682367b7c..e53032ebea 100644 --- a/pkg/specgen/generate/container_create.go +++ b/pkg/specgen/generate/container_create.go @@ -92,7 +92,8 @@ func MakeContainer(ctx context.Context, rt *libpod.Runtime, s *specgen.SpecGener options = append(options, libpod.WithRootFS(s.Rootfs)) } else { var resolvedImageName string - newImage, resolvedImageName, err = rt.LibimageRuntime().LookupImage(s.Image, nil) + lookupOptions := &libimage.LookupImageOptions{IgnorePlatform: true} + newImage, resolvedImageName, err = rt.LibimageRuntime().LookupImage(s.Image, lookupOptions) if err != nil { return nil, err }