Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

3.2 backports #10763

Merged
merged 2 commits into from
Jun 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 17 additions & 41 deletions cmd/podman/containers/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,14 @@ 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"
"github.com/containers/podman/v3/cmd/podman/utils"
"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"
)

Expand Down Expand Up @@ -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 != "" {
Expand All @@ -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
}

Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ require (
github.com/containernetworking/cni v0.8.1
github.com/containernetworking/plugins v0.9.1
github.com/containers/buildah v1.21.0
github.com/containers/common v0.38.9
github.com/containers/common v0.38.10
github.com/containers/conmon v2.0.20+incompatible
github.com/containers/image/v5 v5.12.0
github.com/containers/ocicrypt v1.1.1
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -218,8 +218,8 @@ github.com/containernetworking/plugins v0.9.1/go.mod h1:xP/idU2ldlzN6m4p5LmGiwRD
github.com/containers/buildah v1.21.0 h1:LuwuqRPjan3X3AIdGwfkEkqMgmrDMNpQznFqNdHgCz8=
github.com/containers/buildah v1.21.0/go.mod h1:yPdlpVd93T+i91yGxrJbW1YOWrqN64j5ZhHOZmHUejs=
github.com/containers/common v0.38.4/go.mod h1:egfpX/Y3+19Dz4Wa1eRZDdgzoEOeneieF9CQppKzLBg=
github.com/containers/common v0.38.9 h1:73TUUqIIMRU6hNqrgmZy4vlWPgMz4A/oFfi+v2VEdkg=
github.com/containers/common v0.38.9/go.mod h1:egfpX/Y3+19Dz4Wa1eRZDdgzoEOeneieF9CQppKzLBg=
github.com/containers/common v0.38.10 h1:X3spMNjrqKYQ25Lc+Z1wpc0t7KIrwO6mT2S5J1TDiTI=
github.com/containers/common v0.38.10/go.mod h1:egfpX/Y3+19Dz4Wa1eRZDdgzoEOeneieF9CQppKzLBg=
github.com/containers/conmon v2.0.20+incompatible h1:YbCVSFSCqFjjVwHTPINGdMX1F6JXHGTUje2ZYobNrkg=
github.com/containers/conmon v2.0.20+incompatible/go.mod h1:hgwZ2mtuDrppv78a/cOBNiCm6O0UMWGx1mu7P00nu5I=
github.com/containers/image/v5 v5.12.0 h1:1hNS2QkzFQ4lH3GYQLyAXB0acRMhS1Ubm6oV++8vw4w=
Expand Down
5 changes: 2 additions & 3 deletions pkg/api/handlers/compat/containers_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion pkg/api/handlers/libpod/images.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
26 changes: 17 additions & 9 deletions pkg/api/handlers/libpod/images_pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
}
Expand Down Expand Up @@ -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() {
Expand Down
2 changes: 1 addition & 1 deletion pkg/api/handlers/libpod/manifests.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
25 changes: 11 additions & 14 deletions pkg/api/handlers/utils/images.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions pkg/api/server/register_images.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 5 additions & 5 deletions pkg/bindings/images/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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 {
Expand All @@ -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
}
Expand All @@ -86,13 +86,13 @@ 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 != "":
default:
return images, errors.Errorf("failed to parse pull results stream, unexpected input: %v", report)
}
}
return images, mErr
return images, errorhandling.JoinErrors(pullErrors)
}
3 changes: 3 additions & 0 deletions pkg/bindings/images/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 16 additions & 0 deletions pkg/bindings/images/types_pull_options.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pkg/domain/infra/tunnel/images.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
9 changes: 6 additions & 3 deletions pkg/errorhandling/errorhandling.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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()))
}

Expand Down
3 changes: 2 additions & 1 deletion pkg/specgen/generate/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/specgen/generate/container_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
Loading