From 8e4a42aa429c6dec0d5face7c69554d8a0677e96 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Wed, 7 Oct 2020 16:58:53 +0200 Subject: short-name aliasing Add support for short-name aliasing. Signed-off-by: Valentin Rothberg --- libpod/image/image.go | 28 +++++----- libpod/image/pull.go | 129 +++++++++++++++++++++++++--------------------- libpod/image/pull_test.go | 67 ++++++++---------------- 3 files changed, 104 insertions(+), 120 deletions(-) (limited to 'libpod') diff --git a/libpod/image/image.go b/libpod/image/image.go index 301954703..cecd64eb7 100644 --- a/libpod/image/image.go +++ b/libpod/image/image.go @@ -24,6 +24,7 @@ import ( "github.com/containers/image/v5/manifest" ociarchive "github.com/containers/image/v5/oci/archive" "github.com/containers/image/v5/oci/layout" + "github.com/containers/image/v5/pkg/shortnames" is "github.com/containers/image/v5/storage" "github.com/containers/image/v5/tarball" "github.com/containers/image/v5/transports" @@ -164,7 +165,7 @@ func (ir *Runtime) New(ctx context.Context, name, signaturePolicyPath, authfile } imageName, err := ir.pullImageFromHeuristicSource(ctx, name, writer, authfile, signaturePolicyPath, signingoptions, dockeroptions, &retry.RetryOptions{MaxRetry: maxRetry}, label) if err != nil { - return nil, errors.Wrapf(err, "unable to pull %s", name) + return nil, err } newImage, err := ir.NewFromLocal(imageName[0]) @@ -318,10 +319,8 @@ func (ir *Runtime) LoadAllImagesFromDockerArchive(ctx context.Context, fileName } goal := pullGoal{ - pullAllPairs: true, - usedSearchRegistries: false, - refPairs: refPairs, - searchedRegistries: nil, + pullAllPairs: true, + refPairs: refPairs, } defer goal.cleanUp() @@ -456,22 +455,19 @@ func (ir *Runtime) getLocalImage(inputName string) (string, *storage.Image, erro return "", nil, errors.Wrapf(ErrNoSuchImage, imageError) } - // "Short-name image", so let's try out certain prefixes: - // 1) DefaultLocalRegistry (i.e., "localhost/) - // 2) Unqualified-search registries from registries.conf - unqualifiedSearchRegistries, err := registries.GetRegistries() + sys := &types.SystemContext{ + SystemRegistriesConfPath: registries.SystemRegistriesConfPath(), + } + + candidates, err := shortnames.ResolveLocally(sys, inputName) if err != nil { return "", nil, err } - for _, candidate := range append([]string{DefaultLocalRegistry}, unqualifiedSearchRegistries...) { - ref, err := decomposedImage.referenceWithRegistry(candidate) - if err != nil { - return "", nil, err - } - img, err := ir.store.Image(reference.TagNameOnly(ref).String()) + for _, candidate := range candidates { + img, err := ir.store.Image(candidate.String()) if err == nil { - return ref.String(), img, nil + return candidate.String(), img, nil } } diff --git a/libpod/image/pull.go b/libpod/image/pull.go index 65acdf427..2a2d16252 100644 --- a/libpod/image/pull.go +++ b/libpod/image/pull.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "io" + "os" "path/filepath" "strings" @@ -15,13 +16,14 @@ import ( dockerarchive "github.com/containers/image/v5/docker/archive" ociarchive "github.com/containers/image/v5/oci/archive" oci "github.com/containers/image/v5/oci/layout" + "github.com/containers/image/v5/pkg/shortnames" is "github.com/containers/image/v5/storage" "github.com/containers/image/v5/transports" "github.com/containers/image/v5/transports/alltransports" "github.com/containers/image/v5/types" "github.com/containers/podman/v2/libpod/events" + "github.com/containers/podman/v2/pkg/errorhandling" "github.com/containers/podman/v2/pkg/registries" - "github.com/hashicorp/go-multierror" "github.com/opentracing/opentracing-go" "github.com/pkg/errors" "github.com/sirupsen/logrus" @@ -56,9 +58,10 @@ var ( // pullRefPair records a pair of prepared image references to pull. type pullRefPair struct { - image string - srcRef types.ImageReference - dstRef types.ImageReference + image string + srcRef types.ImageReference + dstRef types.ImageReference + resolvedShortname *shortnames.PullCandidate // if set, must be recorded after successful pull } // cleanUpFunc is a function prototype for clean-up functions. @@ -66,11 +69,11 @@ type cleanUpFunc func() error // pullGoal represents the prepared image references and decided behavior to be executed by imagePull type pullGoal struct { - refPairs []pullRefPair - pullAllPairs bool // Pull all refPairs instead of stopping on first success. - usedSearchRegistries bool // refPairs construction has depended on registries.GetRegistries() - searchedRegistries []string // The list of search registries used; set only if usedSearchRegistries - cleanUpFuncs []cleanUpFunc // Mainly used to close long-lived objects (e.g., an archive.Reader) + refPairs []pullRefPair + pullAllPairs bool // Pull all refPairs instead of stopping on first success. + cleanUpFuncs []cleanUpFunc // Mainly used to close long-lived objects (e.g., an archive.Reader) + shortName string // Set when pulling a short name + resolved *shortnames.Resolved // Set when pulling a short name } // cleanUp invokes all cleanUpFuncs. Certain resources may not be available @@ -86,10 +89,8 @@ func (p *pullGoal) cleanUp() { // singlePullRefPairGoal returns a no-frills pull goal for the specified reference pair. func singlePullRefPairGoal(rp pullRefPair) *pullGoal { return &pullGoal{ - refPairs: []pullRefPair{rp}, - pullAllPairs: false, // Does not really make a difference. - usedSearchRegistries: false, - searchedRegistries: nil, + refPairs: []pullRefPair{rp}, + pullAllPairs: false, // Does not really make a difference. } } @@ -193,11 +194,9 @@ func (ir *Runtime) pullGoalFromImageReference(ctx context.Context, srcRef types. } return &pullGoal{ - pullAllPairs: true, - usedSearchRegistries: false, - refPairs: pairs, - searchedRegistries: nil, - cleanUpFuncs: []cleanUpFunc{reader.Close}, + pullAllPairs: true, + refPairs: pairs, + cleanUpFuncs: []cleanUpFunc{reader.Close}, }, nil case OCIArchive: @@ -267,7 +266,7 @@ func (ir *Runtime) pullImageFromHeuristicSource(ctx context.Context, inputName s if srcTransport != nil && srcTransport.Name() != DockerTransport { return nil, err } - goal, err = ir.pullGoalFromPossiblyUnqualifiedName(inputName) + goal, err = ir.pullGoalFromPossiblyUnqualifiedName(sc, writer, inputName) if err != nil { return nil, errors.Wrap(err, "error getting default registries to try") } @@ -325,7 +324,7 @@ func (ir *Runtime) doPullImage(ctx context.Context, sc *types.SystemContext, goa var ( images []string - pullErrors *multierror.Error + pullErrors []error ) for _, imageInfo := range goal.refPairs { @@ -348,12 +347,17 @@ func (ir *Runtime) doPullImage(ctx context.Context, sc *types.SystemContext, goa _, err = cp.Image(ctx, policyContext, imageInfo.dstRef, imageInfo.srcRef, copyOptions) return err }, retryOptions); err != nil { - pullErrors = multierror.Append(pullErrors, err) + pullErrors = append(pullErrors, err) logrus.Debugf("Error pulling image ref %s: %v", imageInfo.srcRef.StringWithinTransport(), err) if writer != nil { _, _ = io.WriteString(writer, cleanErrorMessage(err)) } } else { + if imageInfo.resolvedShortname != nil { + if err := imageInfo.resolvedShortname.Record(); err != nil { + logrus.Errorf("Error recording short-name alias %q: %v", imageInfo.resolvedShortname.Value.String(), err) + } + } if !goal.pullAllPairs { ir.newImageEvent(events.Pull, "") return []string{imageInfo.image}, nil @@ -361,68 +365,75 @@ func (ir *Runtime) doPullImage(ctx context.Context, sc *types.SystemContext, goa images = append(images, imageInfo.image) } } - // If no image was found, we should handle. Lets be nicer to the user and see if we can figure out why. + // If no image was found, we should handle. Lets be nicer to the user + // and see if we can figure out why. if len(images) == 0 { - if goal.usedSearchRegistries && len(goal.searchedRegistries) == 0 { - return nil, errors.Errorf("image name provided is a short name and no search registries are defined in the registries config file.") - } - // If the image passed in was fully-qualified, we will have 1 refpair. Bc the image is fq'd, we don't need to yap about registries. - if !goal.usedSearchRegistries { - if pullErrors != nil && len(pullErrors.Errors) > 0 { // this should always be true - return nil, pullErrors.Errors[0] - } - return nil, errors.Errorf("unable to pull image, or you do not have pull access") + if goal.resolved != nil { + return nil, goal.resolved.FormatPullErrors(pullErrors) } - return nil, errors.Cause(pullErrors) - } - if len(images) > 0 { - ir.newImageEvent(events.Pull, images[0]) + return nil, errorhandling.JoinErrors(pullErrors) } + + ir.newImageEvent(events.Pull, images[0]) return images, nil } +// getShortNameMode looks up the `CONTAINERS_SHORT_NAME_ALIASING` environment +// variable. If it's "on", return `nil` to use the defaults from +// containers/image and the registries.conf files on the system. If it's +// "off", empty or unset, return types.ShortNameModeDisabled to turn off +// short-name aliasing by default. +// +// TODO: remove this function once we want to default to short-name aliasing. +func getShortNameMode() *types.ShortNameMode { + env := os.Getenv("CONTAINERS_SHORT_NAME_ALIASING") + if strings.ToLower(env) == "on" { + return nil // default to whatever registries.conf and c/image decide + } + mode := types.ShortNameModeDisabled + return &mode +} + // pullGoalFromPossiblyUnqualifiedName looks at inputName and determines the possible // image references to try pulling in combination with the registries.conf file as well -func (ir *Runtime) pullGoalFromPossiblyUnqualifiedName(inputName string) (*pullGoal, error) { - decomposedImage, err := decompose(inputName) +func (ir *Runtime) pullGoalFromPossiblyUnqualifiedName(sys *types.SystemContext, writer io.Writer, inputName string) (*pullGoal, error) { + if sys == nil { + sys = &types.SystemContext{} + } + sys.ShortNameMode = getShortNameMode() + + resolved, err := shortnames.Resolve(sys, inputName) if err != nil { return nil, err } - if decomposedImage.hasRegistry { - srcRef, err := docker.ParseReference("//" + inputName) - if err != nil { - return nil, errors.Wrapf(err, "unable to parse '%s'", inputName) + if desc := resolved.Description(); len(desc) > 0 { + logrus.Debug(desc) + if writer != nil { + if _, err := writer.Write([]byte(desc + "\n")); err != nil { + return nil, err + } } - return ir.getSinglePullRefPairGoal(srcRef, inputName) } - searchRegistries, err := registries.GetRegistries() - if err != nil { - return nil, err - } - refPairs := make([]pullRefPair, 0, len(searchRegistries)) - for _, registry := range searchRegistries { - ref, err := decomposedImage.referenceWithRegistry(registry) + refPairs := []pullRefPair{} + for i, candidate := range resolved.PullCandidates { + srcRef, err := docker.NewReference(candidate.Value) if err != nil { return nil, err } - imageName := ref.String() - srcRef, err := docker.ParseReference("//" + imageName) - if err != nil { - return nil, errors.Wrapf(err, "unable to parse '%s'", imageName) - } - ps, err := ir.getPullRefPair(srcRef, imageName) + ps, err := ir.getPullRefPair(srcRef, candidate.Value.String()) if err != nil { return nil, err } + ps.resolvedShortname = &resolved.PullCandidates[i] refPairs = append(refPairs, ps) } return &pullGoal{ - refPairs: refPairs, - pullAllPairs: false, - usedSearchRegistries: true, - searchedRegistries: searchRegistries, + refPairs: refPairs, + pullAllPairs: false, + shortName: inputName, + resolved: resolved, }, nil } diff --git a/libpod/image/pull_test.go b/libpod/image/pull_test.go index 6cb80e8b5..2e1464ad3 100644 --- a/libpod/image/pull_test.go +++ b/libpod/image/pull_test.go @@ -278,15 +278,11 @@ func TestPullGoalFromImageReference(t *testing.T) { assert.Equal(t, e.dstName, storageReferenceWithoutLocation(res.refPairs[i].dstRef), testDescription) } assert.Equal(t, c.expectedPullAllPairs, res.pullAllPairs, c.srcName) - assert.False(t, res.usedSearchRegistries, c.srcName) - assert.Nil(t, res.searchedRegistries, c.srcName) } } } -const registriesConfWithSearch = `[registries.search] -registries = ['example.com', 'docker.io'] -` +const registriesConfWithSearch = `unqualified-search-registries = ['example.com', 'docker.io']` func TestPullGoalFromPossiblyUnqualifiedName(t *testing.T) { const digestSuffix = "@sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" @@ -303,69 +299,58 @@ func TestPullGoalFromPossiblyUnqualifiedName(t *testing.T) { ir, cleanup := newTestRuntime(t) defer cleanup() - // Environment is per-process, so this looks very unsafe; actually it seems fine because tests are not - // run in parallel unless they opt in by calling t.Parallel(). So don’t do that. - oldRCP, hasRCP := os.LookupEnv("REGISTRIES_CONFIG_PATH") - defer func() { - if hasRCP { - os.Setenv("REGISTRIES_CONFIG_PATH", oldRCP) - } else { - os.Unsetenv("REGISTRIES_CONFIG_PATH") - } - }() - os.Setenv("REGISTRIES_CONFIG_PATH", registriesConf.Name()) + sc := GetSystemContext("", "", false) + + aliasesConf, err := ioutil.TempFile("", "short-name-aliases.conf") + require.NoError(t, err) + defer aliasesConf.Close() + defer os.Remove(aliasesConf.Name()) + sc.UserShortNameAliasConfPath = aliasesConf.Name() + sc.SystemRegistriesConfPath = registriesConf.Name() for _, c := range []struct { - input string - expected []pullRefStrings - expectedUsedSearchRegistries bool + input string + expected []pullRefStrings }{ - {"#", nil, false}, // Clearly invalid. + {"#", nil}, // Clearly invalid. { // Fully-explicit docker.io, name-only. "docker.io/library/busybox", // (The docker:// representation is shortened by c/image/docker.Reference but it refers to "docker.io/library".) - []pullRefStrings{{"docker.io/library/busybox", "docker://busybox:latest", "docker.io/library/busybox:latest"}}, - false, + []pullRefStrings{{"docker.io/library/busybox:latest", "docker://busybox:latest", "docker.io/library/busybox:latest"}}, }, { // docker.io with implied /library/, name-only. "docker.io/busybox", // (The docker:// representation is shortened by c/image/docker.Reference but it refers to "docker.io/library".) - []pullRefStrings{{"docker.io/busybox", "docker://busybox:latest", "docker.io/library/busybox:latest"}}, - false, + []pullRefStrings{{"docker.io/library/busybox:latest", "docker://busybox:latest", "docker.io/library/busybox:latest"}}, }, { // Qualified example.com, name-only. "example.com/ns/busybox", - []pullRefStrings{{"example.com/ns/busybox", "docker://example.com/ns/busybox:latest", "example.com/ns/busybox:latest"}}, - false, + []pullRefStrings{{"example.com/ns/busybox:latest", "docker://example.com/ns/busybox:latest", "example.com/ns/busybox:latest"}}, }, { // Qualified example.com, name:tag. "example.com/ns/busybox:notlatest", []pullRefStrings{{"example.com/ns/busybox:notlatest", "docker://example.com/ns/busybox:notlatest", "example.com/ns/busybox:notlatest"}}, - false, }, { // Qualified example.com, name@digest. "example.com/ns/busybox" + digestSuffix, []pullRefStrings{{"example.com/ns/busybox" + digestSuffix, "docker://example.com/ns/busybox" + digestSuffix, "example.com/ns/busybox" + digestSuffix}}, - false, }, // Qualified example.com, name:tag@digest. This code is happy to try, but .srcRef parsing currently rejects such input. - {"example.com/ns/busybox:notlatest" + digestSuffix, nil, false}, + {"example.com/ns/busybox:notlatest" + digestSuffix, nil}, { // Unqualified, single-name, name-only "busybox", []pullRefStrings{ - {"example.com/busybox", "docker://example.com/busybox:latest", "example.com/busybox:latest"}, + {"example.com/busybox:latest", "docker://example.com/busybox:latest", "example.com/busybox:latest"}, // (The docker:// representation is shortened by c/image/docker.Reference but it refers to "docker.io/library".) - {"docker.io/library/busybox", "docker://busybox:latest", "docker.io/library/busybox:latest"}, + {"docker.io/library/busybox:latest", "docker://busybox:latest", "docker.io/library/busybox:latest"}, }, - true, }, { // Unqualified, namespaced, name-only "ns/busybox", []pullRefStrings{ - {"example.com/ns/busybox", "docker://example.com/ns/busybox:latest", "example.com/ns/busybox:latest"}, + {"example.com/ns/busybox:latest", "docker://example.com/ns/busybox:latest", "example.com/ns/busybox:latest"}, }, - true, }, { // Unqualified, name:tag "busybox:notlatest", @@ -374,7 +359,6 @@ func TestPullGoalFromPossiblyUnqualifiedName(t *testing.T) { // (The docker:// representation is shortened by c/image/docker.Reference but it refers to "docker.io/library".) {"docker.io/library/busybox:notlatest", "docker://busybox:notlatest", "docker.io/library/busybox:notlatest"}, }, - true, }, { // Unqualified, name@digest "busybox" + digestSuffix, @@ -383,29 +367,22 @@ func TestPullGoalFromPossiblyUnqualifiedName(t *testing.T) { // (The docker:// representation is shortened by c/image/docker.Reference but it refers to "docker.io/library".) {"docker.io/library/busybox" + digestSuffix, "docker://busybox" + digestSuffix, "docker.io/library/busybox" + digestSuffix}, }, - true, }, // Unqualified, name:tag@digest. This code is happy to try, but .srcRef parsing currently rejects such input. - {"busybox:notlatest" + digestSuffix, nil, false}, + {"busybox:notlatest" + digestSuffix, nil}, } { - res, err := ir.pullGoalFromPossiblyUnqualifiedName(c.input) + res, err := ir.pullGoalFromPossiblyUnqualifiedName(sc, nil, c.input) if len(c.expected) == 0 { assert.Error(t, err, c.input) } else { assert.NoError(t, err, c.input) for i, e := range c.expected { - testDescription := fmt.Sprintf("%s #%d", c.input, i) + testDescription := fmt.Sprintf("%s #%d (%v)", c.input, i, res.refPairs) assert.Equal(t, e.image, res.refPairs[i].image, testDescription) assert.Equal(t, e.srcRef, transports.ImageName(res.refPairs[i].srcRef), testDescription) assert.Equal(t, e.dstName, storageReferenceWithoutLocation(res.refPairs[i].dstRef), testDescription) } assert.False(t, res.pullAllPairs, c.input) - assert.Equal(t, c.expectedUsedSearchRegistries, res.usedSearchRegistries, c.input) - if !c.expectedUsedSearchRegistries { - assert.Nil(t, res.searchedRegistries, c.input) - } else { - assert.Equal(t, []string{"example.com", "docker.io"}, res.searchedRegistries, c.input) - } } } } -- cgit v1.2.3-54-g00ecf