Merge pull request #674 from runcom/additional-registries

*: implement additional pull registries

cmd/crio/config.go

@@ -135,6 +135,11 @@ image_volumes = "{{ .ImageVolumes }}"
 insecure_registries = [
 {{ range $opt := .InsecureRegistries }}{{ printf "\t%q,\n" $opt }}{{ end }}]
 
+# registries is used to specify a comma separated list of registries to be used
+# when pulling an unqualified image (e.g. fedora:rawhide).
+registries = [
+{{ range $opt := .Registries }}{{ printf "\t%q,\n" $opt }}{{ end }}]
+
 # The "crio.network" table contains settings pertaining to the
 # management of CNI plugins.
 [crio.network]

cmd/crio/main.go

@@ -81,6 +81,9 @@ func mergeConfig(config *server.Config, ctx *cli.Context) error {
 	if ctx.GlobalIsSet("insecure-registry") {
 		config.InsecureRegistries = ctx.GlobalStringSlice("insecure-registry")
 	}
+	if ctx.GlobalIsSet("registry") {
+		config.Registries = ctx.GlobalStringSlice("registry")
+	}
 	if ctx.GlobalIsSet("default-transport") {
 		config.DefaultTransport = ctx.GlobalString("default-transport")
 	}
@@ -227,6 +230,10 @@ func main() {
 			Name:  "insecure-registry",
 			Usage: "whether to disable TLS verification for the given registry",
 		},
+		cli.StringSliceFlag{
+			Name:  "registry",
+			Usage: "registry to be prepended when pulling unqualified images, can be specified multiple times",
+		},
 		cli.StringFlag{
 			Name:  "default-transport",
 			Usage: "default transport",

docs/crio.8.md

@@ -16,11 +16,13 @@ crio - OCI Kubernetes Container Runtime daemon
 [**--debug**]
 [**--default-transport**=[*value*]]
 [**--help**|**-h**]
+[**--insecure-registry**=[*value*]]
 [**--listen**=[*value*]]
 [**--log**=[*value*]]
 [**--log-format value**]
 [**--pause-command**=[*value*]]
 [**--pause-image**=[*value*]]
+[**--registry**=[*value*]]
 [**--root**=[*value*]]
 [**--runroot**=[*value*]]
 [**--runtime**=[*value*]]
@@ -73,6 +75,20 @@ set the CPU profile file path
 **--help, -h**
   Print usage statement
 
+**--insecure-registry=**
+  Enable insecure registry  communication,  i.e.,  enable  un-encrypted
+  and/or untrusted communication.
+
+  List  of  insecure registries can contain an element with CIDR notation
+  to specify a whole  subnet.  Insecure  registries  accept  HTTP  and/or
+  accept HTTPS with certificates from unknown CAs.
+
+  Enabling  --insecure-registry  is useful when running a local registry.
+  However, because its use creates  security  vulnerabilities  it  should
+  ONLY  be  enabled  for testing purposes.  For increased security, users
+  should add their CA to their system's list of trusted  CAs  instead  of
+  using --insecure-registry.
+
 **--image-volumes**=""
   Image volume handling ('mkdir' or 'ignore') (default: "mkdir")
 
@@ -97,6 +113,9 @@ set the CPU profile file path
 **--root**=""
   CRIO root dir (default: "/var/lib/containers/storage")
 
+**--registry**=""
+  Registry host which will be prepended to unqualified images, can be specified multiple times
+
 **--runroot**=""
   CRIO state dir (default: "/var/run/containers/storage")
 

docs/crio.conf.5.md

@@ -77,15 +77,33 @@ The `crio` table supports the following options:
 **default_transport**
   A prefix to prepend to image names that can't be pulled as-is (default: "docker://")
 
-**--image_volumes**=""
+**image_volumes**=""
   Image volume handling ('mkdir' or 'ignore') (default: "mkdir")
 
+**insecure_registries**=""
+  Enable insecure registry  communication,  i.e.,  enable  un-encrypted
+  and/or untrusted communication.
+
+  List  of  insecure registries can contain an element with CIDR notation
+  to specify a whole  subnet.  Insecure  registries  accept  HTTP  and/or
+  accept HTTPS with certificates from unknown CAs.
+
+  Enabling  --insecure-registry  is useful when running a local registry.
+  However, because its use creates  security  vulnerabilities  it  should
+  ONLY  be  enabled  for testing purposes.  For increased security, users
+  should add their CA to their system's list of trusted  CAs  instead  of
+  using --insecure-registry.
+
 **pause_command**=""
   Path to the pause executable in the pause image (default: "/pause")
 
 **pause_image**=""
   Image which contains the pause executable (default: "kubernetes/pause")
 
+**registries**=""
+  Comma separated list of registries that will be prepended when pulling
+  unqualified images
+
 ## CRIO.NETWORK TABLE
 
 **network_dir**=""

libkpod/config.go

@@ -160,6 +160,8 @@ type ImageConfig struct {
 	InsecureRegistries []string `toml:"insecure_registries"`
 	// ImageVolumes controls how volumes specified in image config are handled
 	ImageVolumes ImageVolumesType `toml:"image_volumes"`
+	// Registries holds a list of registries used to pull unqualified images
+	Registries []string `toml:"registries"`
 }
 
 // NetworkConfig represents the "crio.network" TOML config table

libkpod/container_server.go

@@ -103,7 +103,7 @@ func New(config *Config) (*ContainerServer, error) {
 		return nil, err
 	}
 
-	imageService, err := storage.GetImageService(store, config.DefaultTransport, config.InsecureRegistries)
+	imageService, err := storage.GetImageService(store, config.DefaultTransport, config.InsecureRegistries, config.Registries)
 	if err != nil {
 		return nil, err
 	}

pkg/storage/image.go

@@ -1,7 +1,10 @@
 package storage
 
 import (
+	"errors"
 	"net"
+	"path/filepath"
+	"strings"
 
 	"github.com/containers/image/copy"
 	"github.com/containers/image/docker/reference"
@@ -31,6 +34,7 @@ type imageService struct {
 	defaultTransport      string
 	insecureRegistryCIDRs []*net.IPNet
 	indexConfigs          map[string]*indexInfo
+	registries            []string
 }
 
 // ImageServer wraps up various CRI-related activities into a reusable
@@ -50,6 +54,9 @@ type ImageServer interface {
 	GetStore() storage.Store
 	// CanPull preliminary checks whether we're allowed to pull an image
 	CanPull(imageName string, options *copy.Options) (bool, error)
+	// ResolveNames takes an image reference and if it's unqualified (w/o hostname),
+	// it uses crio's default registries to qualify it.
+	ResolveNames(imageName string) ([]string, error)
 }
 
 func (svc *imageService) ListImages(filter string) ([]ImageResult, error) {
@@ -271,11 +278,47 @@ func (svc *imageService) isSecureIndex(indexName string) bool {
 	return true
 }
 
+func isValidHostname(hostname string) bool {
+	return hostname != "" && !strings.Contains(hostname, "/") &&
+		(strings.Contains(hostname, ".") ||
+			strings.Contains(hostname, ":") || hostname == "localhost")
+}
+
+func (svc *imageService) ResolveNames(imageName string) ([]string, error) {
+	r, err := reference.ParseNormalizedNamed(imageName)
+	if err != nil {
+		return nil, err
+	}
+	domain, rest := splitDomain(r.Name())
+	if len(domain) != 0 && isValidHostname(domain) {
+		// this means the image is already fully qualified
+		return []string{imageName}, nil
+	}
+	// we got an unqualified image here, we can't go ahead w/o registries configured
+	// properly.
+	if len(svc.registries) == 0 {
+		return nil, errors.New("no registries configured while trying to pull an unqualified image")
+	}
+	// this means we got an image in the form of "busybox"
+	// we need to use additional registries...
+	// normalize the unqualified image to be domain/repo/image...
+	images := []string{}
+	for _, r := range svc.registries {
+		path := rest
+		if !isValidHostname(domain) {
+			// This is the case where we have an image like "runcom/busybox"
+			path = imageName
+		}
+		images = append(images, filepath.Join(r, path))
+	}
+	return images, nil
+}
+
 // GetImageService returns an ImageServer that uses the passed-in store, and
 // which will prepend the passed-in defaultTransport value to an image name if
 // a name that's passed to its PullImage() method can't be resolved to an image
 // in the store and can't be resolved to a source on its own.
-func GetImageService(store storage.Store, defaultTransport string, insecureRegistries []string) (ImageServer, error) {
+func GetImageService(store storage.Store, defaultTransport string, insecureRegistries []string, registries []string) (ImageServer, error) {
 	if store == nil {
 		var err error
 		store, err = storage.GetStore(storage.DefaultStoreOptions)
@@ -284,11 +327,22 @@ func GetImageService(store storage.Store, defaultTransport string, insecureRegis
 		}
 	}
 
+	seenRegistries := make(map[string]bool, len(registries))
+	cleanRegistries := []string{}
+	for _, r := range registries {
+		if seenRegistries[r] {
+			continue
+		}
+		cleanRegistries = append(cleanRegistries, r)
+		seenRegistries[r] = true
+	}
+
 	is := &imageService{
 		store:                 store,
 		defaultTransport:      defaultTransport,
 		indexConfigs:          make(map[string]*indexInfo, 0),
 		insecureRegistryCIDRs: make([]*net.IPNet, 0),
+		registries:            cleanRegistries,
 	}
 
 	insecureRegistries = append(insecureRegistries, "127.0.0.0/8")

pkg/storage/image_regexp.go

@@ -0,0 +1,125 @@
+package storage
+
+// This is a fork of docker/distribution code to be used when manipulating image
+// references.
+// DO NOT EDIT THIS FILE.
+
+import "regexp"
+
+var (
+	// alphaNumericRegexp defines the alpha numeric atom, typically a
+	// component of names. This only allows lower case characters and digits.
+	alphaNumericRegexp = match(`[a-z0-9]+`)
+
+	// separatorRegexp defines the separators allowed to be embedded in name
+	// components. This allow one period, one or two underscore and multiple
+	// dashes.
+	separatorRegexp = match(`(?:[._]|__|[-]*)`)
+
+	// nameComponentRegexp restricts registry path component names to start
+	// with at least one letter or number, with following parts able to be
+	// separated by one period, one or two underscore and multiple dashes.
+	nameComponentRegexp = expression(
+		alphaNumericRegexp,
+		optional(repeated(separatorRegexp, alphaNumericRegexp)))
+
+	// domainComponentRegexp restricts the registry domain component of a
+	// repository name to start with a component as defined by domainRegexp
+	// and followed by an optional port.
+	domainComponentRegexp = match(`(?:[a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9-]*[a-zA-Z0-9])`)
+
+	// domainRegexp defines the structure of potential domain components
+	// that may be part of image names. This is purposely a subset of what is
+	// allowed by DNS to ensure backwards compatibility with Docker image
+	// names.
+	domainRegexp = expression(
+		domainComponentRegexp,
+		optional(repeated(literal(`.`), domainComponentRegexp)),
+		optional(literal(`:`), match(`[0-9]+`)))
+
+	// NameRegexp is the format for the name component of references. The
+	// regexp has capturing groups for the domain and name part omitting
+	// the separating forward slash from either.
+	NameRegexp = expression(
+		optional(domainRegexp, literal(`/`)),
+		nameComponentRegexp,
+		optional(repeated(literal(`/`), nameComponentRegexp)))
+
+	// anchoredNameRegexp is used to parse a name value, capturing the
+	// domain and trailing components.
+	anchoredNameRegexp = anchored(
+		optional(capture(domainRegexp), literal(`/`)),
+		capture(nameComponentRegexp,
+			optional(repeated(literal(`/`), nameComponentRegexp))))
+
+	// IdentifierRegexp is the format for string identifier used as a
+	// content addressable identifier using sha256. These identifiers
+	// are like digests without the algorithm, since sha256 is used.
+	IdentifierRegexp = match(`([a-f0-9]{64})`)
+
+	// ShortIdentifierRegexp is the format used to represent a prefix
+	// of an identifier. A prefix may be used to match a sha256 identifier
+	// within a list of trusted identifiers.
+	ShortIdentifierRegexp = match(`([a-f0-9]{6,64})`)
+)
+
+// match compiles the string to a regular expression.
+var match = regexp.MustCompile
+
+// literal compiles s into a literal regular expression, escaping any regexp
+// reserved characters.
+func literal(s string) *regexp.Regexp {
+	re := match(regexp.QuoteMeta(s))
+
+	if _, complete := re.LiteralPrefix(); !complete {
+		panic("must be a literal")
+	}
+
+	return re
+}
+
+func splitDomain(name string) (string, string) {
+	match := anchoredNameRegexp.FindStringSubmatch(name)
+	if len(match) != 3 {
+		return "", name
+	}
+	return match[1], match[2]
+}
+
+// expression defines a full expression, where each regular expression must
+// follow the previous.
+func expression(res ...*regexp.Regexp) *regexp.Regexp {
+	var s string
+	for _, re := range res {
+		s += re.String()
+	}
+
+	return match(s)
+}
+
+// optional wraps the expression in a non-capturing group and makes the
+// production optional.
+func optional(res ...*regexp.Regexp) *regexp.Regexp {
+	return match(group(expression(res...)).String() + `?`)
+}
+
+// repeated wraps the regexp in a non-capturing group to get one or more
+// matches.
+func repeated(res ...*regexp.Regexp) *regexp.Regexp {
+	return match(group(expression(res...)).String() + `+`)
+}
+
+// group wraps the regexp in a non-capturing group.
+func group(res ...*regexp.Regexp) *regexp.Regexp {
+	return match(`(?:` + expression(res...).String() + `)`)
+}
+
+// capture wraps the expression in a capturing group.
+func capture(res ...*regexp.Regexp) *regexp.Regexp {
+	return match(`(` + expression(res...).String() + `)`)
+}
+
+// anchored anchors the regular expression by adding start and end delimiters.
+func anchored(res ...*regexp.Regexp) *regexp.Regexp {
+	return match(`^` + expression(res...).String() + `$`)
+}

server/container_create.go

@@ -553,6 +553,16 @@ func (s *Server) createSandboxContainer(ctx context.Context, containerID string,
 	if image == "" {
 		return nil, fmt.Errorf("CreateContainerRequest.ContainerConfig.Image.Image is empty")
 	}
+	images, err := s.StorageImageServer().ResolveNames(image)
+	if err != nil {
+		// This means we got an image ID
+		if strings.Contains(err.Error(), "cannot specify 64-byte hexadecimal strings") {
+			images = append(images, image)
+		} else {
+			return nil, err
+		}
+	}
+	image = images[0]
 
 	// bind mount the pod shm
 	specgen.AddBindMount(sb.ShmPath(), "/dev/shm", []string{"rw"})

server/image_pull.go

@@ -14,7 +14,6 @@ import (
 // PullImage pulls a image with authentication config.
 func (s *Server) PullImage(ctx context.Context, req *pb.PullImageRequest) (*pb.PullImageResponse, error) {
 	logrus.Debugf("PullImageRequest: %+v", req)
-	// TODO(runcom?): deal with AuthConfig in req.GetAuth()
 	// TODO: what else do we need here? (Signatures when the story isn't just pulling from docker://)
 	image := ""
 	img := req.GetImage()
@@ -22,6 +21,16 @@ func (s *Server) PullImage(ctx context.Context, req *pb.PullImageRequest) (*pb.P
 		image = img.Image
 	}
 
+	var (
+		images []string
+		pulled string
+		err    error
+	)
+	images, err = s.StorageImageServer().ResolveNames(image)
+	if err != nil {
+		return nil, err
+	}
+	for _, img := range images {
 		var (
 			username string
 			password string
@@ -30,18 +39,17 @@ func (s *Server) PullImage(ctx context.Context, req *pb.PullImageRequest) (*pb.P
 			username = req.GetAuth().Username
 			password = req.GetAuth().Password
 			if req.GetAuth().Auth != "" {
-			var err error
 				username, password, err = decodeDockerAuth(req.GetAuth().Auth)
 				if err != nil {
-				return nil, err
+					logrus.Debugf("error decoding authentication for image %s: %v", img, err)
+					continue
 				}
 			}
 		}
 		options := &copy.Options{
 			SourceCtx: &types.SystemContext{},
 		}
-	// a not empty username should be sufficient to decide whether to send auth
+		// Specifiying a username indicates the user intends to send authentication to the registry.
-	// or not I guess
 		if username != "" {
 			options.SourceCtx = &types.SystemContext{
 				DockerAuthConfig: &types.DockerAuthConfig{
@@ -51,23 +59,34 @@ func (s *Server) PullImage(ctx context.Context, req *pb.PullImageRequest) (*pb.P
 			}
 		}
 
-	canPull, err := s.StorageImageServer().CanPull(image, options)
+		var canPull bool
+		canPull, err = s.StorageImageServer().CanPull(img, options)
 		if err != nil && !canPull {
-		return nil, err
+			logrus.Debugf("error checking image %s: %v", img, err)
+			continue
 		}
 
 		// let's be smart, docker doesn't repull if image already exists.
-	if _, err := s.StorageImageServer().ImageStatus(s.ImageContext(), image); err == nil {
+		_, err = s.StorageImageServer().ImageStatus(s.ImageContext(), img)
-		return &pb.PullImageResponse{
+		if err == nil {
-			ImageRef: image,
+			logrus.Debugf("image %s already in store, skipping pull", img)
-		}, nil
+			pulled = img
+			break
 		}
 
-	if _, err := s.StorageImageServer().PullImage(s.ImageContext(), image, options); err != nil {
+		_, err = s.StorageImageServer().PullImage(s.ImageContext(), img, options)
+		if err != nil {
+			logrus.Debugf("error pulling image %s: %v", img, err)
+			continue
+		}
+		pulled = img
+		break
+	}
+	if pulled == "" && err != nil {
 		return nil, err
 	}
 	resp := &pb.PullImageResponse{
-		ImageRef: image,
+		ImageRef: pulled,
 	}
 	logrus.Debugf("PullImageResponse: %+v", resp)
 	return resp, nil

server/image_remove.go

@@ -2,6 +2,7 @@ package server
 
 import (
 	"fmt"
+	"strings"
 
 	"github.com/Sirupsen/logrus"
 	"golang.org/x/net/context"
@@ -19,8 +20,30 @@ func (s *Server) RemoveImage(ctx context.Context, req *pb.RemoveImageRequest) (*
 	if image == "" {
 		return nil, fmt.Errorf("no image specified")
 	}
-	err := s.StorageImageServer().RemoveImage(s.ImageContext(), image)
+	var (
+		images  []string
+		err     error
+		deleted bool
+	)
+	images, err = s.StorageImageServer().ResolveNames(image)
 	if err != nil {
+		// This means we got an image ID
+		if strings.Contains(err.Error(), "cannot specify 64-byte hexadecimal strings") {
+			images = append(images, image)
+		} else {
+			return nil, err
+		}
+	}
+	for _, img := range images {
+		err = s.StorageImageServer().RemoveImage(s.ImageContext(), img)
+		if err != nil {
+			logrus.Debugf("error deleting image %s: %v", img, err)
+			continue
+		}
+		deleted = true
+		break
+	}
+	if !deleted && err != nil {
 		return nil, err
 	}
 	resp := &pb.RemoveImageResponse{}

server/image_status.go

@@ -2,6 +2,7 @@ package server
 
 import (
 	"fmt"
+	"strings"
 
 	"github.com/Sirupsen/logrus"
 	"github.com/containers/storage"
@@ -20,6 +21,17 @@ func (s *Server) ImageStatus(ctx context.Context, req *pb.ImageStatusRequest) (*
 	if image == "" {
 		return nil, fmt.Errorf("no image specified")
 	}
+	images, err := s.StorageImageServer().ResolveNames(image)
+	if err != nil {
+		// This means we got an image ID
+		if strings.Contains(err.Error(), "cannot specify 64-byte hexadecimal strings") {
+			images = append(images, image)
+		} else {
+			return nil, err
+		}
+	}
+	// match just the first registry as that's what kube meant
+	image = images[0]
 	status, err := s.StorageImageServer().ImageStatus(s.ImageContext(), image)
 	if err != nil {
 		if err == storage.ErrImageUnknown {

test/helpers.bash

@@ -215,7 +215,7 @@ function start_crio() {
 	"$COPYIMG_BINARY" --root "$TESTDIR/crio" $STORAGE_OPTS --runroot "$TESTDIR/crio-run" --image-name=mrunalp/image-volume-test --import-from=dir:"$ARTIFACTS_PATH"/image-volume-test-image --add-name=docker.io/library/mrunalp/image-volume-test --signature-policy="$INTEGRATION_ROOT"/policy.json
 	"$COPYIMG_BINARY" --root "$TESTDIR/crio" $STORAGE_OPTS --runroot "$TESTDIR/crio-run" --image-name=busybox:latest --import-from=dir:"$ARTIFACTS_PATH"/busybox-image --add-name=docker.io/library/busybox:latest --signature-policy="$INTEGRATION_ROOT"/policy.json
 	"$COPYIMG_BINARY" --root "$TESTDIR/crio" $STORAGE_OPTS --runroot "$TESTDIR/crio-run" --image-name=runcom/stderr-test:latest --import-from=dir:"$ARTIFACTS_PATH"/stderr-test --add-name=docker.io/runcom/stderr-test:latest --signature-policy="$INTEGRATION_ROOT"/policy.json
-	"$CRIO_BINARY" --conmon "$CONMON_BINARY" --listen "$CRIO_SOCKET" --cgroup-manager "$CGROUP_MANAGER" --runtime "$RUNTIME_BINARY" --root "$TESTDIR/crio" --runroot "$TESTDIR/crio-run" $STORAGE_OPTS --seccomp-profile "$seccomp" --apparmor-profile "$apparmor" --cni-config-dir "$CRIO_CNI_CONFIG" --signature-policy "$INTEGRATION_ROOT"/policy.json --image-volumes "$IMAGE_VOLUMES" --pids-limit "$PIDS_LIMIT" --config /dev/null config >$CRIO_CONFIG
+	"$CRIO_BINARY" --conmon "$CONMON_BINARY" --listen "$CRIO_SOCKET" --cgroup-manager "$CGROUP_MANAGER" --registry "docker.io" --runtime "$RUNTIME_BINARY" --root "$TESTDIR/crio" --runroot "$TESTDIR/crio-run" $STORAGE_OPTS --seccomp-profile "$seccomp" --apparmor-profile "$apparmor" --cni-config-dir "$CRIO_CNI_CONFIG" --signature-policy "$INTEGRATION_ROOT"/policy.json --image-volumes "$IMAGE_VOLUMES" --pids-limit "$PIDS_LIMIT" --config /dev/null config >$CRIO_CONFIG
 
 	# Prepare the CNI configuration files, we're running with non host networking by default
 	if [[ -n "$4" ]]; then