Skip to content

Commit

Permalink
Fix pulling of images within buildah
Browse files Browse the repository at this point in the history 
Change references to Transfer to transfer to make it internal only.
It should be determined from the image specification and only determined
in one place.

Make buildah.Pull use registries.conf

Currently buildah pull does not resolve images based on registries.conf
This does not match the behaviour of buildah from or buildah bud

Signed-off-by: Daniel J Walsh <[email protected]>
  • Loading branch information
@rhatdan
rhatdan committed Feb 17, 2019
1 parent 80fcb24 commit 654dcd7
Show file tree
Hide file tree
Showing 8 changed files with 105 additions and 157 deletions.
2 changes: 1 addition & 1 deletion buildah.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ type BuilderOptions struct {
// needs to be pulled and the image name alone, or the image name and
// the registry together, can not be resolved to a reference to a
// source image. No separator is implicitly added.
Transport string
transport string
// PullBlobDirectory is the name of a directory in which we'll attempt
// to store copies of layer blobs that we pull down, if any. It should
// already exist.
Expand Down
2 changes: 1 addition & 1 deletion cmd/buildah/commit.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ func commitCmd(c *cobra.Command, args []string, iopts commitInputOptions) error

dest, err := alltransports.ParseImageName(image)
if err != nil {
candidates, _, err := util.ResolveName(image, "", systemContext, store)
candidates, _, _, err := util.ResolveName(image, "", systemContext, store)
if err != nil {
return errors.Wrapf(err, "error parsing target image name %q", image)
}
Expand Down
10 changes: 0 additions & 10 deletions cmd/buildah/from.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"github.com/containers/buildah"
buildahcli "github.com/containers/buildah/pkg/cli"
"github.com/containers/buildah/pkg/parse"
"github.com/containers/buildah/util"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
"github.com/spf13/cobra"
Expand Down Expand Up @@ -188,14 +187,6 @@ func fromCmd(c *cobra.Command, args []string, iopts fromReply) error {
return err
}

transport := util.DefaultTransport
arr := strings.SplitN(args[0], ":", 2)
if len(arr) == 2 {
if _, ok := util.Transports[arr[0]]; ok {
transport = arr[0]
}
}

isolation, err := parse.IsolationOption(c)
if err != nil {
return err
Expand All @@ -218,7 +209,6 @@ func fromCmd(c *cobra.Command, args []string, iopts fromReply) error {

options := buildah.BuilderOptions{
FromImage: args[0],
Transport: transport,
Container: iopts.name,
PullPolicy: pullPolicy,
SignaturePolicyPath: signaturePolicy,
Expand Down
19 changes: 2 additions & 17 deletions cmd/buildah/pull.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,10 @@ package main

import (
"os"
"strings"

"github.com/containers/buildah"
buildahcli "github.com/containers/buildah/pkg/cli"
"github.com/containers/buildah/pkg/parse"
"github.com/containers/buildah/util"
"github.com/pkg/errors"
"github.com/spf13/cobra"
)
Expand Down Expand Up @@ -80,27 +78,14 @@ func pullCmd(c *cobra.Command, args []string, iopts pullResults) error {
return err
}

transport := util.DefaultTransport
arr := strings.SplitN(args[0], ":", 2)
if len(arr) == 2 {
if iopts.allTags {
return errors.Errorf("tag can't be used with --all-tags")
}
if _, ok := util.Transports[arr[0]]; ok {
transport = arr[0]
}
}

options := buildah.PullOptions{
Transport: transport,
SignaturePolicyPath: iopts.signaturePolicy,
Store: store,
SystemContext: systemContext,
BlobDirectory: iopts.blobCache,
AllTags: iopts.allTags,
}
if !iopts.quiet {
options.ReportWriter = os.Stderr
ReportWriter: os.Stderr,
Quiet: iopts.quiet,
}

return buildah.Pull(getContext(), args[0], options)
Expand Down
9 changes: 1 addition & 8 deletions imagebuildah/build.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,6 @@ type BuildOptions struct {
// needs to be pulled and the image name alone can not be resolved to a
// reference to a source image. No separator is implicitly added.
Registry string
// Transport is a value which is prepended to the image's name, if it
// needs to be pulled and the image name alone, or the image name and
// the registry together, can not be resolved to a reference to a
// source image. No separator is implicitly added.
Transport string
// IgnoreUnrecognizedInstructions tells us to just log instructions we
// don't recognize, and try to keep going.
IgnoreUnrecognizedInstructions bool
Expand Down Expand Up @@ -582,7 +577,6 @@ func NewExecutor(store storage.Store, options BuildOptions) (*Executor, error) {
contextDir: options.ContextDirectory,
pullPolicy: options.PullPolicy,
registry: options.Registry,
transport: options.Transport,
ignoreUnrecognizedInstructions: options.IgnoreUnrecognizedInstructions,
quiet: options.Quiet,
runtime: options.Runtime,
Expand Down Expand Up @@ -672,7 +666,6 @@ func (b *Executor) Prepare(ctx context.Context, stage imagebuilder.Stage, from s
FromImage: from,
PullPolicy: b.pullPolicy,
Registry: b.registry,
Transport: b.transport,
PullBlobDirectory: b.blobDirectory,
SignaturePolicyPath: b.signaturePolicyPath,
ReportWriter: b.reportWriter,
Expand Down Expand Up @@ -785,7 +778,7 @@ func (b *Executor) resolveNameToImageRef() (types.ImageReference, error) {
if b.output != "" {
imageRef, err = alltransports.ParseImageName(b.output)
if err != nil {
candidates, _, err := util.ResolveName(b.output, "", b.systemContext, b.store)
candidates, _, _, err := util.ResolveName(b.output, "", b.systemContext, b.store)
if err != nil {
return nil, errors.Wrapf(err, "error parsing target image name %q", b.output)
}
Expand Down
64 changes: 34 additions & 30 deletions new.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func pullAndFindImage(ctx context.Context, store storage.Store, imageName string
ReportWriter: options.ReportWriter,
Store: store,
SystemContext: options.SystemContext,
Transport: options.Transport,
transport: options.transport,
BlobDirectory: options.PullBlobDirectory,
}
ref, err := pullImage(ctx, store, imageName, pullOptions, sc)
Expand Down Expand Up @@ -101,55 +101,57 @@ func newContainerIDMappingOptions(idmapOptions *IDMappingOptions) storage.IDMapp
return options
}

func resolveImage(ctx context.Context, systemContext *types.SystemContext, store storage.Store, options BuilderOptions) (types.ImageReference, *storage.Image, error) {
func resolveImage(ctx context.Context, systemContext *types.SystemContext, store storage.Store, options BuilderOptions) (types.ImageReference, string, *storage.Image, error) {
type failure struct {
resolvedImageName string
err error
}

candidates, searchRegistriesWereUsedButEmpty, err := util.ResolveName(options.FromImage, options.Registry, systemContext, store)
candidates, transport, searchRegistriesWereUsedButEmpty, err := util.ResolveName(options.FromImage, options.Registry, systemContext, store)
if err != nil {
return nil, nil, errors.Wrapf(err, "error parsing reference to image %q", options.FromImage)
return nil, "", nil, errors.Wrapf(err, "error parsing reference to image %q", options.FromImage)
}

failures := []failure{}
for _, image := range candidates {
var err error
if len(image) >= minimumTruncatedIDLength {
if img, err := store.Image(image); err == nil && img != nil && strings.HasPrefix(img.ID, image) {
ref, err := is.Transport.ParseStoreReference(store, img.ID)
if err != nil {
return nil, nil, errors.Wrapf(err, "error parsing reference to image %q", img.ID)
return nil, "", nil, errors.Wrapf(err, "error parsing reference to image %q", img.ID)
}
return ref, img, nil
return ref, transport, img, nil
}
}

if options.PullPolicy == PullAlways {
options.transport = transport
pulledImg, pulledReference, err := pullAndFindImage(ctx, store, image, options, systemContext)
if err != nil {
logrus.Debugf("unable to pull and read image %q: %v", image, err)
failures = append(failures, failure{resolvedImageName: image, err: err})
continue
}
return pulledReference, pulledImg, nil
return pulledReference, transport, pulledImg, nil
}

srcRef, err := alltransports.ParseImageName(image)
if err != nil {
if options.Transport == "" {
if transport == "" {
logrus.Debugf("error parsing image name %q: %v", image, err)
failures = append(failures, failure{
resolvedImageName: image,
err: errors.Wrapf(err, "error parsing image name"),
})
continue
}
logrus.Debugf("error parsing image name %q as given, trying with transport %q: %v", image, options.Transport, err)
transport := options.Transport
logrus.Debugf("error parsing image name %q as given, trying with transport %q: %v", image, transport, err)

trans := transport
if transport != util.DefaultTransport {
transport = transport + ":"
trans = trans + ":"
}
srcRef2, err := alltransports.ParseImageName(transport + image)
srcRef2, err := alltransports.ParseImageName(trans + image)
if err != nil {
logrus.Debugf("error parsing image name %q: %v", transport+image, err)
failures = append(failures, failure{
Expand All @@ -163,19 +165,19 @@ func resolveImage(ctx context.Context, systemContext *types.SystemContext, store

destImage, err := localImageNameForReference(ctx, store, srcRef, options.FromImage)
if err != nil {
return nil, nil, errors.Wrapf(err, "error computing local image name for %q", transports.ImageName(srcRef))
return nil, "", nil, errors.Wrapf(err, "error computing local image name for %q", transports.ImageName(srcRef))
}
if destImage == "" {
return nil, nil, errors.Errorf("error computing local image name for %q", transports.ImageName(srcRef))
return nil, "", nil, errors.Errorf("error computing local image name for %q", transports.ImageName(srcRef))
}

ref, err := is.Transport.ParseStoreReference(store, destImage)
if err != nil {
return nil, nil, errors.Wrapf(err, "error parsing reference to image %q", destImage)
return nil, "", nil, errors.Wrapf(err, "error parsing reference to image %q", destImage)
}
img, err := is.Transport.GetStoreImage(store, ref)
if err == nil {
return ref, img, nil
return ref, transport, img, nil
}

if errors.Cause(err) == storage.ErrImageUnknown && options.PullPolicy != PullIfMissing {
Expand All @@ -187,26 +189,27 @@ func resolveImage(ctx context.Context, systemContext *types.SystemContext, store
continue
}

options.transport = transport
pulledImg, pulledReference, err := pullAndFindImage(ctx, store, image, options, systemContext)
if err != nil {
logrus.Debugf("unable to pull and read image %q: %v", image, err)
failures = append(failures, failure{resolvedImageName: image, err: err})
continue
}
return pulledReference, pulledImg, nil
return pulledReference, transport, pulledImg, nil
}

if len(failures) != len(candidates) {
return nil, nil, fmt.Errorf("internal error: %d candidates (%#v) vs. %d failures (%#v)", len(candidates), candidates, len(failures), failures)
return nil, "", nil, fmt.Errorf("internal error: %d candidates (%#v) vs. %d failures (%#v)", len(candidates), candidates, len(failures), failures)
}

registriesConfPath := sysregistries.RegistriesConfPath(systemContext)
switch len(failures) {
case 0:
if searchRegistriesWereUsedButEmpty {
return nil, nil, errors.Errorf("image name %q is a short name and no search registries are defined in %s.", options.FromImage, registriesConfPath)
return nil, "", nil, errors.Errorf("image name %q is a short name and no search registries are defined in %s.", options.FromImage, registriesConfPath)
}
return nil, nil, fmt.Errorf("internal error: no pull candidates were available for %q for an unknown reason", options.FromImage)
return nil, "", nil, fmt.Errorf("internal error: no pull candidates were available for %q for an unknown reason", options.FromImage)

case 1:
err := failures[0].err
Expand All @@ -216,15 +219,15 @@ func resolveImage(ctx context.Context, systemContext *types.SystemContext, store
if searchRegistriesWereUsedButEmpty {
err = errors.Wrapf(err, "(image name %q is a short name and no search registries are defined in %s)", options.FromImage, registriesConfPath)
}
return nil, nil, err
return nil, "", nil, err

default:
// NOTE: a multi-line error string:
e := fmt.Sprintf("The following failures happened while trying to pull image specified by %q based on search registries in %s:", options.FromImage, registriesConfPath)
for _, f := range failures {
e = e + fmt.Sprintf("\n* %q: %s", f.resolvedImageName, f.err.Error())
}
return nil, nil, errors.New(e)
return nil, "", nil, errors.New(e)
}
}

Expand All @@ -250,21 +253,22 @@ func findUnusedContainer(name string, containers []storage.Container) string {
}

func newBuilder(ctx context.Context, store storage.Store, options BuilderOptions) (*Builder, error) {
var ref types.ImageReference
var img *storage.Image
var err error

var (
ref types.ImageReference
img *storage.Image
err error
)
if options.FromImage == BaseImageFakeName {
options.FromImage = ""
}
if options.Transport == "" {
options.Transport = util.DefaultTransport
if options.transport == "" {
options.transport = util.DefaultTransport
}

systemContext := getSystemContext(options.SystemContext, options.SignaturePolicyPath)

if options.FromImage != "" && options.FromImage != "scratch" {
ref, img, err = resolveImage(ctx, systemContext, store, options)
ref, _, img, err = resolveImage(ctx, systemContext, store, options)
if err != nil {
return nil, err
}
Expand Down
Loading

0 comments on commit 654dcd7

Please sign in to comment.