From 951a943d16b4b18e3c9c04ff4c8bb024419ddf4c Mon Sep 17 00:00:00 2001 From: baude Date: Thu, 31 Aug 2017 08:04:02 -0500 Subject: [PATCH] libkpod/image/copy.go: Add pull by short-name If the user provides kpod pull a short name like 'debian', we still want the pull to be sucessful. As such, when a short name is provided, we get the list of searchable registries via the systemregistries code in containers-storage. We then append a tag of 'latest' (if not provided) and we formulate a list of possible fully-qualified image names to try. Vendor update for containers-storage to bring in the system_registries code. Also includes a patch from Nalin to fix compilation errors. Signed-off-by: baude --- cmd/kpod/pull.go | 103 +++++++++++++++++- libpod/image.go | 2 +- libpod/images/copy_ref.go | 18 +-- test/helpers.bash | 3 + test/kpod.bats | 46 -------- test/kpod_pull.bats | 74 +++++++++++++ test/registries.conf | 9 ++ .../pkg/sysregistries/system_registries.go | 86 +++++++++++++++ 8 files changed, 274 insertions(+), 67 deletions(-) create mode 100644 test/kpod_pull.bats create mode 100644 test/registries.conf create mode 100644 vendor/github.com/containers/image/pkg/sysregistries/system_registries.go diff --git a/cmd/kpod/pull.go b/cmd/kpod/pull.go index 69f31def..b2495726 100644 --- a/cmd/kpod/pull.go +++ b/cmd/kpod/pull.go @@ -3,6 +3,11 @@ package main import ( "os" + "fmt" + "github.com/containers/image/docker/reference" + "github.com/containers/image/pkg/sysregistries" + "github.com/containers/image/transports/alltransports" + "github.com/containers/image/types" "github.com/pkg/errors" "github.com/sirupsen/logrus" "github.com/urfave/cli" @@ -31,9 +36,84 @@ var ( } ) +// struct for when a user passes a short or incomplete +// image name +type imagePullStruct struct { + imageName string + tag string + registry string + hasRegistry bool + transport string +} + +func (ips imagePullStruct) returnFQName() string { + return fmt.Sprintf("%s%s/%s:%s", ips.transport, ips.registry, ips.imageName, ips.tag) +} + +func getRegistriesToTry(image string) ([]string, error) { + var registries []string + var imageError = fmt.Sprintf("unable to parse '%s'\n", image) + imgRef, err := reference.Parse(image) + if err != nil { + return nil, errors.Wrapf(err, imageError) + } + tagged, isTagged := imgRef.(reference.NamedTagged) + tag := "latest" + if isTagged { + tag = tagged.Tag() + } + hasDomain := true + registry := reference.Domain(imgRef.(reference.Named)) + if registry == "" { + hasDomain = false + } + imageName := reference.Path(imgRef.(reference.Named)) + pImage := imagePullStruct{ + imageName, + tag, + registry, + hasDomain, + "docker://", + } + if pImage.hasRegistry { + // If input has a registry, we have to assume they included an image + // name but maybe not a tag + pullRef, err := alltransports.ParseImageName(pImage.returnFQName()) + if err != nil { + return nil, errors.Errorf(imageError) + } + registries = append(registries, pullRef.DockerReference().String()) + } else { + // No registry means we check the globals registries configuration file + // and assemble a list of candidate sources to try + registryConfigPath := "" + envOverride := os.Getenv("REGISTRIES_CONFIG_PATH") + if len(envOverride) > 0 { + registryConfigPath = envOverride + } + searchRegistries, err := sysregistries.GetRegistries(&types.SystemContext{SystemRegistriesConfPath: registryConfigPath}) + if err != nil { + fmt.Println(err) + return nil, errors.Errorf("unable to parse the registries.conf file and"+ + " the image name '%s' is incomplete.", imageName) + } + for _, searchRegistry := range searchRegistries { + pImage.registry = searchRegistry + pullRef, err := alltransports.ParseImageName(pImage.returnFQName()) + if err != nil { + return nil, errors.Errorf("unable to parse '%s'", pImage.returnFQName()) + } + registries = append(registries, pullRef.DockerReference().String()) + } + } + return registries, nil +} + // pullCmd gets the data from the command line and calls pullImage // to copy an image from a registry to a local machine func pullCmd(c *cli.Context) error { + var fqRegistries []string + args := c.Args() if len(args) == 0 { logrus.Errorf("an image name must be specified") @@ -44,13 +124,28 @@ func pullCmd(c *cli.Context) error { return nil } image := args[0] - + srcRef, err := alltransports.ParseImageName(image) + if err != nil { + fqRegistries, err = getRegistriesToTry(image) + if err != nil { + fmt.Println(err) + } + } else { + fqRegistries = append(fqRegistries, srcRef.DockerReference().String()) + } runtime, err := getRuntime(c) + defer runtime.Shutdown(false) + if err != nil { return errors.Wrapf(err, "could not create runtime") } - if err := runtime.PullImage(image, c.Bool("all-tags"), os.Stdout); err != nil { - return errors.Errorf("error pulling image from %q: %v", image, err) + for _, fqname := range fqRegistries { + fmt.Printf("Trying to pull %s...", fqname) + if err := runtime.PullImage(fqname, c.Bool("all-tags"), os.Stdout); err != nil { + fmt.Printf(" Failed\n") + } else { + return nil + } } - return nil + return errors.Errorf("error pulling image from %q", image) } diff --git a/libpod/image.go b/libpod/image.go index cbc52d49..be7cb445 100644 --- a/libpod/image.go +++ b/libpod/image.go @@ -121,7 +121,7 @@ func (r *Runtime) PullImage(imgName string, allTags bool, reportWriter io.Writer copyOptions := common.GetCopyOptions(reportWriter, "", nil, nil, common.SigningOptions{}) for _, image := range images { - destRef, err := is.Transport.ParseStoreReference(r.store, image) + destRef, err := is.Transport.ParseStoreReference(r.store, srcRef.DockerReference().String()) if err != nil { return errors.Errorf("error parsing dest reference name: %v", err) } diff --git a/libpod/images/copy_ref.go b/libpod/images/copy_ref.go index 2391a242..a65d0050 100644 --- a/libpod/images/copy_ref.go +++ b/libpod/images/copy_ref.go @@ -66,26 +66,12 @@ func (c *CopyRef) NewImage(sc *types.SystemContext) (types.Image, error) { return image.FromSource(src) } -func selectManifestType(preferred string, acceptable, supported []string) string { - selected := preferred - for _, accept := range acceptable { - if preferred == accept { - return preferred - } - for _, support := range supported { - if accept == support { - selected = accept - } - } - } - return selected -} - // NewImageSource creates a new image source from the given system context and manifest func (c *CopyRef) NewImageSource(sc *types.SystemContext) (src types.ImageSource, err error) { // Decide which type of manifest and configuration output we're going to provide. - manifestType := selectManifestType(c.preferredManifestType, nil, nil) + manifestType := c.preferredManifestType // If it's not a format we support, return an error. + // Try to provide a manifest and configuration in the same format the current ones are in. if manifestType != v1.MediaTypeImageManifest && manifestType != docker.V2S2MediaTypeManifest { return nil, errors.Errorf("no supported manifest types (attempted to use %q, only know %q and %q)", manifestType, v1.MediaTypeImageManifest, docker.V2S2MediaTypeManifest) diff --git a/test/helpers.bash b/test/helpers.bash index f7c0517b..b7f61bfd 100644 --- a/test/helpers.bash +++ b/test/helpers.bash @@ -59,6 +59,9 @@ PIDS_LIMIT=${PIDS_LIMIT:-1024} TESTDIR=$(mktemp -d) +# kpod pull needs a configuration file for shortname pulls +export REGISTRIES_CONFIG_PATH="$INTEGRATION_ROOT/registries.conf" + # Setup default hooks dir HOOKSDIR=$TESTDIR/hooks mkdir ${HOOKSDIR} diff --git a/test/kpod.bats b/test/kpod.bats index 8ac5feda..b88b65be 100644 --- a/test/kpod.bats +++ b/test/kpod.bats @@ -17,52 +17,6 @@ function teardown() { [ "$status" -eq 0 ] } -@test "kpod pull from docker with tag" { - run ${KPOD_BINARY} ${KPOD_OPTIONS} pull debian:6.0.10 - echo "$output" - [ "$status" -eq 0 ] - run ${KPOD_BINARY} $KPOD_OPTIONS rmi debian:6.0.10 - [ "$status" -eq 0 ] -} - -@test "kpod pull from docker without tag" { - run ${KPOD_BINARY} $KPOD_OPTIONS pull debian - echo "$output" - [ "$status" -eq 0 ] - run ${KPOD_BINARY} $KPOD_OPTIONS rmi debian - [ "$status" -eq 0 ] -} - -@test "kpod pull from a non-docker registry with tag" { - run ${KPOD_BINARY} $KPOD_OPTIONS pull registry.fedoraproject.org/fedora:rawhide - echo "$output" - [ "$status" -eq 0 ] - run ${KPOD_BINARY} $KPOD_OPTIONS rmi registry.fedoraproject.org/fedora:rawhide - [ "$status" -eq 0 ] -} - -@test "kpod pull from a non-docker registry without tag" { - run ${KPOD_BINARY} $KPOD_OPTIONS pull registry.fedoraproject.org/fedora - echo "$output" - [ "$status" -eq 0 ] - run ${KPOD_BINARY} $KPOD_OPTIONS rmi registry.fedoraproject.org/fedora - [ "$status" -eq 0 ] -} - -@test "kpod pull using digest" { - run ${KPOD_BINARY} $KPOD_OPTIONS pull alpine@sha256:1072e499f3f655a032e88542330cf75b02e7bdf673278f701d7ba61629ee3ebe - echo "$output" - [ "$status" -eq 0 ] - run ${KPOD_BINARY} $KPOD_OPTIONS rmi alpine:latest - [ "$status" -eq 0 ] -} - -@test "kpod pull from a non existent image" { - run ${KPOD_BINARY} $KPOD_OPTIONS pull umohnani/get-started - echo "$output" - [ "$status" -ne 0 ] -} - @test "kpod history default" { run ${KPOD_BINARY} ${KPOD_OPTIONS} pull $IMAGE [ "$status" -eq 0 ] diff --git a/test/kpod_pull.bats b/test/kpod_pull.bats new file mode 100644 index 00000000..2103eecc --- /dev/null +++ b/test/kpod_pull.bats @@ -0,0 +1,74 @@ +#!/usr/bin/env bats + +load helpers + +IMAGE="alpine:latest" +ROOT="$TESTDIR/crio" +RUNROOT="$TESTDIR/crio-run" +KPOD_OPTIONS="--root $ROOT --runroot $RUNROOT ${STORAGE_OPTS}" + +function teardown() { + cleanup_test +} + +@test "kpod pull from docker with tag" { + run ${KPOD_BINARY} ${KPOD_OPTIONS} pull debian:6.0.10 + echo "$output" + [ "$status" -eq 0 ] + run ${KPOD_BINARY} $KPOD_OPTIONS rmi debian:6.0.10 + [ "$status" -eq 0 ] +} + +@test "kpod pull from docker without tag" { + run ${KPOD_BINARY} $KPOD_OPTIONS pull debian + echo "$output" + [ "$status" -eq 0 ] + run ${KPOD_BINARY} $KPOD_OPTIONS rmi debian + [ "$status" -eq 0 ] +} + +@test "kpod pull from a non-docker registry with tag" { + run ${KPOD_BINARY} $KPOD_OPTIONS pull registry.fedoraproject.org/fedora:rawhide + echo "$output" + [ "$status" -eq 0 ] + run ${KPOD_BINARY} $KPOD_OPTIONS rmi registry.fedoraproject.org/fedora:rawhide + [ "$status" -eq 0 ] +} + +@test "kpod pull from a non-docker registry without tag" { + run ${KPOD_BINARY} $KPOD_OPTIONS pull registry.fedoraproject.org/fedora + echo "$output" + [ "$status" -eq 0 ] + run ${KPOD_BINARY} $KPOD_OPTIONS rmi registry.fedoraproject.org/fedora + [ "$status" -eq 0 ] +} + +@test "kpod pull using digest" { + run ${KPOD_BINARY} $KPOD_OPTIONS pull alpine@sha256:1072e499f3f655a032e88542330cf75b02e7bdf673278f701d7ba61629ee3ebe + echo "$output" + [ "$status" -eq 0 ] + run ${KPOD_BINARY} $KPOD_OPTIONS rmi alpine:latest + [ "$status" -eq 0 ] +} + +@test "kpod pull from a non existent image" { + run ${KPOD_BINARY} $KPOD_OPTIONS pull umohnani/get-started + echo "$output" + [ "$status" -ne 0 ] +} + +@test "kpod pull from docker with shortname" { + run ${KPOD_BINARY} ${KPOD_OPTIONS} pull debian + echo "$output" + [ "$status" -eq 0 ] + run ${KPOD_BINARY} $KPOD_OPTIONS rmi docker.io/debian:latest + [ "$status" -eq 0 ] +} + +@test "kpod pull from docker with shortname and tag" { + run ${KPOD_BINARY} ${KPOD_OPTIONS} pull debian:6.0.10 + echo "$output" + [ "$status" -eq 0 ] + run ${KPOD_BINARY} $KPOD_OPTIONS rmi docker.io/debian:6.0.10 + [ "$status" -eq 0 ] +} diff --git a/test/registries.conf b/test/registries.conf new file mode 100644 index 00000000..f3bf092b --- /dev/null +++ b/test/registries.conf @@ -0,0 +1,9 @@ +[registries.search] +registries = ['registry.access.redhat.com', 'registry.fedoraproject.org', 'docker.io'] + +[registries.insecure] +registries = [] + +#blocked (docker only) +[registries.block] +registries = [] diff --git a/vendor/github.com/containers/image/pkg/sysregistries/system_registries.go b/vendor/github.com/containers/image/pkg/sysregistries/system_registries.go new file mode 100644 index 00000000..e5564a2a --- /dev/null +++ b/vendor/github.com/containers/image/pkg/sysregistries/system_registries.go @@ -0,0 +1,86 @@ +package sysregistries + +import ( + "github.com/BurntSushi/toml" + "github.com/containers/image/types" + "io/ioutil" + "path/filepath" +) + +// systemRegistriesConfPath is the path to the system-wide registry configuration file +// and is used to add/subtract potential registries for obtaining images. +// You can override this at build time with +// -ldflags '-X github.com/containers/image/sysregistries.systemRegistriesConfPath=$your_path' +var systemRegistriesConfPath = builtinRegistriesConfPath + +// builtinRegistriesConfPath is the path to registry configuration file +// DO NOT change this, instead see systemRegistriesConfPath above. +const builtinRegistriesConfPath = "/etc/containers/registries.conf" + +type registries struct { + Registries []string `toml:"registries"` +} + +type tomlConfig struct { + Registries struct { + Search registries `toml:"search"` + Insecure registries `toml:"insecure"` + Block registries `toml:"block"` + } `toml:"registries"` +} + +// Reads the global registry file from the filesystem. Returns +// a byte array +func readRegistryConf(ctx *types.SystemContext) ([]byte, error) { + dirPath := systemRegistriesConfPath + if ctx != nil { + if ctx.SystemRegistriesConfPath != "" { + dirPath = ctx.SystemRegistriesConfPath + } else if ctx.RootForImplicitAbsolutePaths != "" { + dirPath = filepath.Join(ctx.RootForImplicitAbsolutePaths, systemRegistriesConfPath) + } + } + configBytes, err := ioutil.ReadFile(dirPath) + return configBytes, err +} + +// For mocking in unittests +var readConf = readRegistryConf + +// Loads the registry configuration file from the filesystem and +// then unmarshals it. Returns the unmarshalled object. +func loadRegistryConf(ctx *types.SystemContext) (*tomlConfig, error) { + config := &tomlConfig{} + + configBytes, err := readConf(ctx) + if err != nil { + return nil, err + } + + err = toml.Unmarshal(configBytes, &config) + return config, err +} + +// GetRegistries returns an array of strings that contain the names +// of the registries as defined in the system-wide +// registries file. it returns an empty array if none are +// defined +func GetRegistries(ctx *types.SystemContext) ([]string, error) { + config, err := loadRegistryConf(ctx) + if err != nil { + return nil, err + } + return config.Registries.Search.Registries, nil +} + +// GetInsecureRegistries returns an array of strings that contain the names +// of the insecure registries as defined in the system-wide +// registries file. it returns an empty array if none are +// defined +func GetInsecureRegistries(ctx *types.SystemContext) ([]string, error) { + config, err := loadRegistryConf(ctx) + if err != nil { + return nil, err + } + return config.Registries.Insecure.Registries, nil +}