Merge pull request #612 from stevvooe/simplify-resolution-flow

cmd/dist, remotes: simplify resolution flow

cmd/dist/fetch.go

@@ -46,8 +46,7 @@ Most of this is experimental and there are few leaps to make this work.`,
 	Action: func(clicontext *cli.Context) error {
 		var (
 			ctx = background
-			locator = clicontext.Args().First()
+			ref = clicontext.Args().First()
-			object  = clicontext.Args().Get(1)
 		)
 
 		conn, err := connectGRPC(clicontext)
@@ -59,11 +58,7 @@ Most of this is experimental and there are few leaps to make this work.`,
 		if err != nil {
 			return err
 		}
-
+		ctx = withJobsContext(ctx)
-		fetcher, err := resolver.Resolve(ctx, locator)
-		if err != nil {
-			return err
-		}
 
 		ingester := contentservice.NewIngesterFromClient(contentapi.NewContentClient(conn))
 		cs, err := resolveContentStore(clicontext)
@@ -73,10 +68,17 @@ Most of this is experimental and there are few leaps to make this work.`,
 
 		eg, ctx := errgroup.WithContext(ctx)
 
-		ctx = withJobsContext(ctx)
+		resolved := make(chan struct{})
-
 		eg.Go(func() error {
-			return fetchManifest(ctx, ingester, fetcher, object)
+			addJob(ctx, ref)
+			name, desc, fetcher, err := resolver.Resolve(ctx, ref)
+			if err != nil {
+				return err
+			}
+			log.G(ctx).WithField("image", name).Debug("fetching")
+			close(resolved)
+
+			return dispatch(ctx, ingester, fetcher, desc)
 		})
 
 		errs := make(chan error)
@@ -96,8 +98,7 @@ Most of this is experimental and there are few leaps to make this work.`,
 			case <-ticker.C:
 				fw.Flush()
 
-				tw := tabwriter.NewWriter(fw, 1, 8, 1, '\t', 0)
+				tw := tabwriter.NewWriter(fw, 1, 8, 1, ' ', 0)
-				// fmt.Fprintln(tw, "REF\tSIZE\tAGE")
 				var total int64
 
 				js := getJobs(ctx)
@@ -137,10 +138,20 @@ Most of this is experimental and there are few leaps to make this work.`,
 					if _, ok := activeSeen[j]; ok {
 						continue
 					}
+					status := "done"
+
+					if j == ref {
+						select {
+						case <-resolved:
+							status = "resolved"
+						default:
+							status = "resolving"
+						}
+					}
 
 					statuses[j] = statusInfo{
 						Ref:    j,
-						Status: "done", // for now!
+						Status: status, // for now!
 					}
 				}
 
@@ -151,21 +162,27 @@ Most of this is experimental and there are few leaps to make this work.`,
 					switch status.Status {
 					case "downloading":
 						bar := progress.Bar(float64(status.Offset) / float64(status.Total))
-						fmt.Fprintf(tw, "%s:\t%s\t%40r\t%8.8s/%s\n",
+						fmt.Fprintf(tw, "%s:\t%s\t%40r\t%8.8s/%s\t\n",
 							status.Ref,
 							status.Status,
 							bar,
 							progress.Bytes(status.Offset), progress.Bytes(status.Total))
-					case "done":
+					case "resolving":
+						bar := progress.Bar(0.0)
+						fmt.Fprintf(tw, "%s:\t%s\t%40r\t\n",
+							status.Ref,
+							status.Status,
+							bar)
+					default:
 						bar := progress.Bar(1.0)
-						fmt.Fprintf(tw, "%s:\t%s\t%40r\n",
+						fmt.Fprintf(tw, "%s:\t%s\t%40r\t\n",
 							status.Ref,
 							status.Status,
 							bar)
 					}
 				}
 
-				fmt.Fprintf(tw, "elapsed: %-4.1fs\ttotal: %7.6v\t(%v)\n",
+				fmt.Fprintf(tw, "elapsed: %-4.1fs\ttotal: %7.6v\t(%v)\t\n",
 					time.Since(start).Seconds(),
 					// TODO(stevvooe): These calculations are actually way off.
 					// Need to account for previously downloaded data. These
@@ -250,14 +267,11 @@ func (j *jobs) jobs() []string {
 	return jobs
 }
 
-func fetchManifest(ctx context.Context, ingester content.Ingester, fetcher remotes.Fetcher, object string, hints ...string) error {
+func fetchManifest(ctx context.Context, ingester content.Ingester, fetcher remotes.Fetcher, desc ocispec.Descriptor) error {
-	const manifestMediaType = "application/vnd.docker.distribution.manifest.v2+json"
+	ref := "manifest-" + desc.Digest.String()
-	hints = append(hints, "mediatype:"+manifestMediaType)
-
-	ref := "manifest-" + object
 	addJob(ctx, ref)
 
-	rc, err := fetcher.Fetch(ctx, object, hints...)
+	rc, err := fetcher.Fetch(ctx, desc)
 	if err != nil {
 		return err
 	}
@@ -291,18 +305,10 @@ func fetchManifest(ctx context.Context, ingester content.Ingester, fetcher remot
 }
 
 func fetch(ctx context.Context, ingester content.Ingester, fetcher remotes.Fetcher, desc ocispec.Descriptor) error {
-	var (
+	ref := "fetch-" + desc.Digest.String()
-		hints  []string
-		object = desc.Digest.String()
-	)
-	if desc.MediaType != "" {
-		hints = append(hints, "mediatype:"+desc.MediaType)
-	}
-
-	ref := "fetch-" + object
 	addJob(ctx, ref)
 	log.G(ctx).Debug("fetch")
-	rc, err := fetcher.Fetch(ctx, object, hints...)
+	rc, err := fetcher.Fetch(ctx, desc)
 	if err != nil {
 		log.G(ctx).WithError(err).Error("fetch error")
 		return err
@@ -328,7 +334,7 @@ func dispatch(ctx context.Context, ingester content.Ingester, fetcher remotes.Fe
 			switch desc.MediaType {
 			case MediaTypeDockerSchema2Manifest, ocispec.MediaTypeImageManifest:
 				eg.Go(func() error {
-					return fetchManifest(ctx, ingester, fetcher, desc.Digest.String(), "mediatype:"+desc.MediaType)
+					return fetchManifest(ctx, ingester, fetcher, desc)
 				})
 			case MediaTypeDockerSchema2ManifestList, ocispec.MediaTypeImageIndex:
 				return fmt.Errorf("%v not yet supported", desc.MediaType)

cmd/dist/fetchobject.go

@@ -10,10 +10,12 @@ import (
 	"net/url"
 	"os"
 	"path"
+	"strconv"
 	"strings"
 
 	"github.com/Sirupsen/logrus"
 	"github.com/docker/containerd/log"
+	"github.com/docker/containerd/reference"
 	"github.com/docker/containerd/remotes"
 	digest "github.com/opencontainers/go-digest"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
@@ -47,8 +49,7 @@ var fetchObjectCommand = cli.Command{
 		var (
 			ctx     = background
 			timeout = context.Duration("timeout")
-			locator = context.Args().First()
+			ref     = context.Args().First()
-			args    = context.Args().Tail()
 		)
 
 		if timeout > 0 {
@@ -57,35 +58,21 @@ var fetchObjectCommand = cli.Command{
 			defer cancel()
 		}
 
-		if locator == "" {
-			return errors.New("containerd: remote required")
-		}
-
-		if len(args) < 1 {
-			return errors.New("containerd: object required")
-		}
-
-		object := args[0]
-		hints := args[1:]
-
 		resolver, err := getResolver(ctx)
 		if err != nil {
 			return err
 		}
 
-		remote, err := resolver.Resolve(ctx, locator)
+		ctx = log.WithLogger(ctx, log.G(ctx).WithField("ref", ref))
+
+		log.G(ctx).Infof("resolving")
+		_, desc, fetcher, err := resolver.Resolve(ctx, ref)
 		if err != nil {
 			return err
 		}
 
-		ctx = log.WithLogger(ctx, log.G(ctx).WithFields(
-			logrus.Fields{
-				"remote": locator,
-				"object": object,
-			}))
-
 		log.G(ctx).Infof("fetching")
-		rc, err := remote.Fetch(ctx, object, hints...)
+		rc, err := fetcher.Fetch(ctx, desc)
 		if err != nil {
 			return err
 		}
@@ -99,73 +86,50 @@ var fetchObjectCommand = cli.Command{
 	},
 }
 
+// getResolver prepares the resolver from the environment and options.
+func getResolver(ctx contextpkg.Context) (remotes.Resolver, error) {
+	return newDockerResolver(), nil
+}
+
 // NOTE(stevvooe): Most of the code below this point is prototype code to
 // demonstrate a very simplified docker.io fetcher. We have a lot of hard coded
 // values but we leave many of the details down to the fetcher, creating a lot
 // of room for ways to fetch content.
 
-// getResolver prepares the resolver from the environment and options.
+type dockerFetcher struct {
-func getResolver(ctx contextpkg.Context) (remotes.Resolver, error) {
+	base  url.URL
-	return remotes.ResolverFunc(func(ctx contextpkg.Context, locator string) (remotes.Fetcher, error) {
+	token string
-		if !strings.HasPrefix(locator, "docker.io") && !strings.HasPrefix(locator, "localhost:5000") {
+}
-			return nil, errors.Errorf("unsupported locator: %q", locator)
-		}
 
-		var (
+func (r *dockerFetcher) url(ps ...string) string {
-			base = url.URL{
+	url := r.base
-				Scheme: "https",
+	url.Path = path.Join(url.Path, path.Join(ps...))
-				Host:   "registry-1.docker.io",
+	return url.String()
-			}
+}
-			prefix = strings.TrimPrefix(locator, "docker.io/")
-		)
 
-		if strings.HasPrefix(locator, "localhost:5000") {
+func (r *dockerFetcher) Fetch(ctx contextpkg.Context, desc ocispec.Descriptor) (io.ReadCloser, error) {
-			base.Scheme = "http"
-			base.Host = "localhost:5000"
-			prefix = strings.TrimPrefix(locator, "localhost:5000/")
-		}
-
-		token, err := getToken(ctx, "repository:"+prefix+":pull")
-		if err != nil {
-			return nil, err
-		}
-
-		return remotes.FetcherFunc(func(ctx contextpkg.Context, object string, hints ...string) (io.ReadCloser, error) {
 	ctx = log.WithLogger(ctx, log.G(ctx).WithFields(
 		logrus.Fields{
-					"prefix": prefix, // or repo?
+			"base":   r.base.String(),
-					"base":   base.String(),
+			"digest": desc.Digest,
-					"hints":  hints,
 		},
 	))
 
-			if _, err := digest.Parse(object); err != nil {
+	paths, err := getV2URLPaths(desc)
-				// in this case, we are seeking a manifest by a tag. Let's add some hints.
-				hints = append(hints, "mediatype:"+MediaTypeDockerSchema2Manifest)
-				hints = append(hints, "mediatype:"+MediaTypeDockerSchema2ManifestList)
-				hints = append(hints, "mediatype:"+ocispec.MediaTypeImageManifest)
-				hints = append(hints, "mediatype:"+ocispec.MediaTypeImageIndex)
-			}
-
-			paths, err := getV2URLPaths(prefix, object, hints...)
 	if err != nil {
 		return nil, err
 	}
 
 	for _, path := range paths {
-				url := base
+		u := r.url(path)
-				url.Path = path
-				log.G(ctx).WithField("url", url).Debug("fetch content")
 
-				req, err := http.NewRequest(http.MethodGet, url.String(), nil)
+		req, err := http.NewRequest(http.MethodGet, u, nil)
 		if err != nil {
 			return nil, err
 		}
 
-				req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", token))
+		req.Header.Set("Accept", strings.Join([]string{desc.MediaType, `*`}, ", "))
-				req.Header.Set("Accept", strings.Join(remotes.HintValues("mediatype", hints...), ", "))
+		resp, err := r.doRequest(ctx, req)
-
-				resp, err := ctxhttp.Do(ctx, http.DefaultClient, req)
 		if err != nil {
 			return nil, err
 		}
@@ -175,15 +139,164 @@ func getResolver(ctx contextpkg.Context) (remotes.Resolver, error) {
 				continue // try one of the other urls.
 			}
 			resp.Body.Close()
-					return nil, errors.Errorf("unexpected status code %v: %v", url, resp.Status)
+			return nil, errors.Errorf("unexpected status code %v: %v", u, resp.Status)
 		}
 
 		return resp.Body, nil
 	}
 
 	return nil, errors.New("not found")
-		}), nil
+}
-	}), nil
+
+func (r *dockerFetcher) doRequest(ctx contextpkg.Context, req *http.Request) (*http.Response, error) {
+	ctx = log.WithLogger(ctx, log.G(ctx).WithField("url", req.URL.String()))
+	log.G(ctx).WithField("request.headers", req.Header).Debug("fetch content")
+	if r.token != "" {
+		req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", r.token))
+	}
+
+	resp, err := ctxhttp.Do(ctx, http.DefaultClient, req)
+	if err != nil {
+		return nil, err
+	}
+	log.G(ctx).WithFields(logrus.Fields{
+		"status":           resp.Status,
+		"response.headers": resp.Header,
+	}).Debug("fetch response received")
+
+	return resp, err
+}
+
+type dockerResolver struct{}
+
+var _ remotes.Resolver = &dockerResolver{}
+
+func newDockerResolver() remotes.Resolver {
+	return &dockerResolver{}
+}
+
+func (r *dockerResolver) Resolve(ctx contextpkg.Context, ref string) (string, ocispec.Descriptor, remotes.Fetcher, error) {
+	refspec, err := reference.Parse(ref)
+	if err != nil {
+		return "", ocispec.Descriptor{}, nil, err
+	}
+
+	var (
+		base  url.URL
+		token string
+	)
+
+	switch refspec.Hostname() {
+	case "docker.io":
+		base.Scheme = "https"
+		base.Host = "registry-1.docker.io"
+		prefix := strings.TrimPrefix(refspec.Locator, "docker.io/")
+		base.Path = path.Join("/v2", prefix)
+		token, err = getToken(ctx, "repository:"+prefix+":pull")
+		if err != nil {
+			return "", ocispec.Descriptor{}, nil, err
+		}
+	case "localhost:5000":
+		base.Scheme = "http"
+		base.Host = "localhost:5000"
+		base.Path = path.Join("/v2", strings.TrimPrefix(refspec.Locator, "localhost:5000/"))
+	default:
+		return "", ocispec.Descriptor{}, nil, errors.Errorf("unsupported locator: %q", refspec.Locator)
+	}
+
+	fetcher := &dockerFetcher{
+		base:  base,
+		token: token,
+	}
+
+	var (
+		urls []string
+		dgst = refspec.Digest()
+	)
+
+	if dgst != "" {
+		if err := dgst.Validate(); err != nil {
+			// need to fail here, since we can't actually resolve the invalid
+			// digest.
+			return "", ocispec.Descriptor{}, nil, err
+		}
+
+		// turns out, we have a valid digest, make a url.
+		urls = append(urls, fetcher.url("manifests", dgst.String()))
+	} else {
+		urls = append(urls, fetcher.url("manifests", refspec.Object))
+	}
+
+	// fallback to blobs on not found.
+	urls = append(urls, fetcher.url("blobs", dgst.String()))
+
+	for _, u := range urls {
+		req, err := http.NewRequest(http.MethodHead, u, nil)
+		if err != nil {
+			return "", ocispec.Descriptor{}, nil, err
+		}
+
+		// set headers for all the types we support for resolution.
+		req.Header.Set("Accept", strings.Join([]string{
+			MediaTypeDockerSchema2Manifest,
+			MediaTypeDockerSchema2ManifestList,
+			ocispec.MediaTypeImageManifest,
+			ocispec.MediaTypeImageIndex, "*"}, ", "))
+
+		log.G(ctx).Debug("resolving")
+		resp, err := fetcher.doRequest(ctx, req)
+		if err != nil {
+			return "", ocispec.Descriptor{}, nil, err
+		}
+		resp.Body.Close() // don't care about body contents.
+
+		if resp.StatusCode > 299 {
+			if resp.StatusCode == http.StatusNotFound {
+				continue
+			}
+			return "", ocispec.Descriptor{}, nil, errors.Errorf("unexpected status code %v: %v", u, resp.Status)
+		}
+
+		// this is the only point at which we trust the registry. we use the
+		// content headers to assemble a descriptor for the name. when this becomes
+		// more robust, we mostly get this information from a secure trust store.
+		dgstHeader := digest.Digest(resp.Header.Get("Docker-Content-Digest"))
+
+		if dgstHeader != "" {
+			if err := dgstHeader.Validate(); err != nil {
+				if err == nil {
+					return "", ocispec.Descriptor{}, nil, errors.Errorf("%q in header not a valid digest", dgstHeader)
+				}
+				return "", ocispec.Descriptor{}, nil, errors.Wrapf(err, "%q in header not a valid digest", dgstHeader)
+			}
+			dgst = dgstHeader
+		}
+
+		if dgst == "" {
+			return "", ocispec.Descriptor{}, nil, errors.Wrapf(err, "could not resolve digest for %v", ref)
+		}
+
+		var (
+			size       int64
+			sizeHeader = resp.Header.Get("Content-Length")
+		)
+
+		size, err = strconv.ParseInt(sizeHeader, 10, 64)
+		if err != nil || size < 0 {
+			return "", ocispec.Descriptor{}, nil, errors.Wrapf(err, "%q in header not a valid size", sizeHeader)
+		}
+
+		desc := ocispec.Descriptor{
+			Digest:    dgst,
+			MediaType: resp.Header.Get("Content-Type"), // need to strip disposition?
+			Size:      size,
+		}
+
+		log.G(ctx).WithField("desc.digest", desc.Digest).Debug("resolved")
+		return ref, desc, fetcher, nil
+	}
+
+	return "", ocispec.Descriptor{}, nil, errors.Errorf("%v not found", ref)
 }
 
 func getToken(ctx contextpkg.Context, scopes ...string) (string, error) {
@@ -232,39 +345,17 @@ func getToken(ctx contextpkg.Context, scopes ...string) (string, error) {
 // getV2URLPaths generates the candidate urls paths for the object based on the
 // set of hints and the provided object id. URLs are returned in the order of
 // most to least likely succeed.
-func getV2URLPaths(prefix, object string, hints ...string) ([]string, error) {
+func getV2URLPaths(desc ocispec.Descriptor) ([]string, error) {
 	var urls []string
 
-	// TODO(stevvooe): We can probably define a higher-level "type" hint to
+	switch desc.MediaType {
-	// avoid having to do extra round trips to resolve content, as well as
+	case MediaTypeDockerSchema2Manifest, MediaTypeDockerSchema2ManifestList,
-	// avoid the tedium of providing media types.
+		ocispec.MediaTypeImageManifest, ocispec.MediaTypeImageIndex:
-
+		urls = append(urls, path.Join("manifests", desc.Digest.String()))
-	if remotes.HintExists("mediatype", MediaTypeDockerSchema2Manifest, hints...) ||
-		remotes.HintExists("mediatype", MediaTypeDockerSchema2ManifestList, hints...) ||
-		remotes.HintExists("mediatype", ocispec.MediaTypeImageManifest, hints...) ||
-		remotes.HintExists("mediatype", ocispec.MediaTypeImageIndex, hints...) {
-		// fast path out if we know we are getting a manifest. Arguably, we
-		// should fallback to blobs, just in case.
-		urls = append(urls, path.Join("/v2", prefix, "manifests", object))
 	}
 
-	// we have a digest, use blob or manifest path, depending on hints, may
+	// always fallback to attempting to get the object out of the blobs store.
-	// need to try both.
+	urls = append(urls, path.Join("blobs", desc.Digest.String()))
-	urls = append(urls, path.Join("/v2", prefix, "blobs", object))
 
-	// probably a take, so we go through the manifests endpoint
+	return urls, nil
-	urls = append(urls, path.Join("/v2", prefix, "manifests", object))
-
-	var (
-		noduplicates []string
-		seen         = map[string]struct{}{}
-	)
-	for _, u := range urls {
-		if _, ok := seen[u]; !ok {
-			seen[u] = struct{}{}
-			noduplicates = append(noduplicates, u)
-		}
-	}
-
-	return noduplicates, nil
 }

reference/reference.go

@@ -0,0 +1,139 @@
+package reference
+
+import (
+	"errors"
+	"fmt"
+	"net/url"
+	"path"
+	"regexp"
+	"strings"
+
+	digest "github.com/opencontainers/go-digest"
+)
+
+var (
+	ErrInvalid          = errors.New("invalid reference")
+	ErrObjectRequired   = errors.New("object required")
+	ErrHostnameRequired = errors.New("hostname required")
+)
+
+// Spec defines the main components of a reference specification.
+//
+// A reference specification is a schema-less URI parsed into common
+// components. The two main components, locator and object, are required to be
+// supported by remotes. It represents a superset of the naming define in
+// docker's reference schema. It aims to be compatible but not prescriptive.
+//
+// While the interpretation of the components, locator and object, are up to
+// the remote, we define a few common parts, accessible via helper methods.
+//
+// The first is the hostname, which is part of the locator. This doesn't need
+// to map to a physical resource, but it must parse as a hostname. We refer to
+// this as the namespace.
+//
+// The other component is the digest, which may be a part of the object
+// identifier, always prefixed with an '@'. If present, the remote may use the
+// digest portion directly or resolve it against a prefix. If the object
+type Spec struct {
+	// Locator is the host and path portion of the specification. The host
+	// portion may refer to an actual host or just a namespace of related
+	// images.
+	//
+	// Typically, the locator may used to resolve the remote to fetch specific
+	// resources.
+	Locator string
+
+	// Object contains the identifier for the remote resource. Classically,
+	// this is a tag but can refer to anything in a remote. By convention, any
+	// portion that may be a partial or whole digest will be preceeded by an
+	// `@`. Anything preceeding the `@` will be referred to as the "tag".
+	//
+	// In practice, we will see this broken down into the following formats:
+	//
+	// 1. <tag>
+	// 2. <tag>@<digest spec>
+	// 3. @<digest spec>
+	//
+	// We define the tag to be anything except '@' and ':'. <digest spec> may
+	// be a full valid digest or shortened version, possibly with elided
+	// algorithm.
+	Object string
+}
+
+var splitRe = regexp.MustCompile(`[:@]`)
+
+// Parse parses the string into a structured ref.
+func Parse(s string) (Spec, error) {
+	u, err := url.Parse("dummy://" + s)
+	if err != nil {
+		return Spec{}, err
+	}
+
+	if u.Scheme != "dummy" {
+		return Spec{}, ErrInvalid
+	}
+
+	if u.Host == "" {
+		return Spec{}, ErrHostnameRequired
+	}
+
+	parts := splitRe.Split(u.Path, 2)
+	if len(parts) < 2 {
+		return Spec{}, ErrObjectRequired
+	}
+
+	// This allows us to retain the @ to signify digests or shortend digests in
+	// the object.
+	object := u.Path[len(parts[0]):]
+	if object[:1] == ":" {
+		object = object[1:]
+	}
+
+	return Spec{
+		Locator: path.Join(u.Host, parts[0]),
+		Object:  object,
+	}, nil
+}
+
+// Hostname returns the hostname portion of the locator.
+//
+// Remotes are not required to directly access the resources at this host. This
+// method is provided for convenience.
+func (r Spec) Hostname() string {
+	i := strings.Index(r.Locator, "/")
+
+	if i < 0 {
+		i = len(r.Locator) + 1
+	}
+	return r.Locator[:i]
+}
+
+// Digest returns the digest portion of the reference spec. This may be a
+// partial or invalid digest, which may be used to lookup a complete digest.
+func (r Spec) Digest() digest.Digest {
+	_, dgst := SplitObject(r.Object)
+	return dgst
+}
+
+// String returns the normalized string for the ref.
+func (r Spec) String() string {
+	if r.Object[:1] == "@" {
+		return fmt.Sprintf("%v%v", r.Locator, r.Object)
+	}
+
+	return fmt.Sprintf("%v:%v", r.Locator, r.Object)
+}
+
+// SplitObject provides two parts of the object spec, delimiited by an `@`
+// symbol.
+//
+// Either may be empty and it is the callers job to validate them
+// appropriately.
+func SplitObject(obj string) (tag string, dgst digest.Digest) {
+	parts := strings.SplitAfterN(obj, "@", 2)
+	if len(parts) < 2 {
+		return parts[0], ""
+	} else {
+		return parts[0], digest.Digest(parts[1])
+	}
+}

reference/reference_test.go

@@ -0,0 +1,179 @@
+package reference
+
+import (
+	"testing"
+
+	digest "github.com/opencontainers/go-digest"
+)
+
+func TestReferenceParser(t *testing.T) {
+	for _, testcase := range []struct {
+		Skip       bool
+		Name       string
+		Input      string
+		Normalized string
+		Digest     digest.Digest
+		Hostname   string
+		Expected   Spec
+		Err        error
+	}{
+		{
+			Name:       "Basic",
+			Input:      "docker.io/library/redis:foo?fooo=asdf",
+			Normalized: "docker.io/library/redis:foo",
+			Hostname:   "docker.io",
+			Expected: Spec{
+				Locator: "docker.io/library/redis",
+				Object:  "foo",
+			},
+		},
+		{
+			Name:       "BasicWithDigest",
+			Input:      "docker.io/library/redis:foo@sha256:abcdef?fooo=asdf",
+			Normalized: "docker.io/library/redis:foo@sha256:abcdef",
+			Hostname:   "docker.io",
+			Digest:     "sha256:abcdef",
+			Expected: Spec{
+				Locator: "docker.io/library/redis",
+				Object:  "foo@sha256:abcdef",
+			},
+		},
+
+		{
+			Name:  "DigestOnly",
+			Input: "docker.io/library/redis@sha256:abcdef?fooo=asdf",
+			Expected: Spec{
+				Locator: "docker.io/library/redis",
+				Object:  "@sha256:abcdef",
+			},
+			Hostname:   "docker.io",
+			Normalized: "docker.io/library/redis@sha256:abcdef",
+			Digest:     "sha256:abcdef",
+		},
+		{
+			Name:       "AtShortDigest",
+			Input:      "docker.io/library/redis:obj@abcdef?fooo=asdf",
+			Normalized: "docker.io/library/redis:obj@abcdef",
+			Hostname:   "docker.io",
+			Digest:     "abcdef",
+			Expected: Spec{
+				Locator: "docker.io/library/redis",
+				Object:  "obj@abcdef",
+			},
+		},
+		{
+			Name:       "HostWithPort",
+			Input:      "localhost:5000/library/redis:obj@abcdef?fooo=asdf",
+			Normalized: "localhost:5000/library/redis:obj@abcdef",
+			Hostname:   "localhost:5000",
+			Digest:     "abcdef",
+			Expected: Spec{
+				Locator: "localhost:5000/library/redis",
+				Object:  "obj@abcdef",
+			},
+		},
+		{
+			Name:  "HostnameRequired",
+			Input: "/docker.io/library/redis:obj@abcdef?fooo=asdf",
+			Err:   ErrHostnameRequired,
+		},
+		{
+			Name:  "ErrObjectRequired",
+			Input: "docker.io/library/redis?fooo=asdf",
+			Err:   ErrObjectRequired,
+		},
+		{
+			Name:     "Subdomain",
+			Input:    "sub-dom1.foo.com/bar/baz/quux:latest",
+			Hostname: "sub-dom1.foo.com",
+			Expected: Spec{
+				Locator: "sub-dom1.foo.com/bar/baz/quux",
+				Object:  "latest",
+			},
+		},
+		{
+			Name:     "SubdomainWithLongTag",
+			Input:    "sub-dom1.foo.com/bar/baz/quux:some-long-tag",
+			Hostname: "sub-dom1.foo.com",
+			Expected: Spec{
+				Locator: "sub-dom1.foo.com/bar/baz/quux",
+				Object:  "some-long-tag",
+			},
+		},
+		{
+			Name:     "AGCRAppears",
+			Input:    "b.gcr.io/test.example.com/my-app:test.example.com",
+			Hostname: "b.gcr.io",
+			Expected: Spec{
+				Locator: "b.gcr.io/test.example.com/my-app",
+				Object:  "test.example.com",
+			},
+		},
+		{
+			Name:     "Punycode",
+			Input:    "xn--n3h.com/myimage:xn--n3h.com", // ☃.com in punycode
+			Hostname: "xn--n3h.com",
+			Expected: Spec{
+				Locator: "xn--n3h.com/myimage",
+				Object:  "xn--n3h.com",
+			},
+		},
+		{
+
+			Name:     "PunycodeWithDigest",
+			Input:    "xn--7o8h.com/myimage:xn--7o8h.com@sha512:fffffff",
+			Hostname: "xn--7o8h.com",
+			Digest:   "sha512:fffffff",
+			Expected: Spec{
+				Locator: "xn--7o8h.com/myimage",
+				Object:  "xn--7o8h.com@sha512:fffffff",
+			},
+		},
+		{
+			Skip:     true, // TODO(stevvooe): Implement this case.
+			Name:     "SchemeDefined",
+			Input:    "http://xn--7o8h.com/myimage:xn--7o8h.com@sha512:fffffff",
+			Hostname: "xn--7o8h.com",
+			Digest:   "sha512:fffffff",
+			Err:      ErrInvalid,
+		},
+	} {
+		t.Run(testcase.Name, func(t *testing.T) {
+			if testcase.Skip {
+				t.Skip("testcase disabled")
+				return
+			}
+
+			ref, err := Parse(testcase.Input)
+			if err != testcase.Err {
+				if testcase.Err != nil {
+					t.Fatalf("expected error %v for %q, got %v, %#v", testcase.Err, testcase.Input, err, ref)
+				} else {
+					t.Fatalf("unexpected error %v", err)
+				}
+			} else if testcase.Err != nil {
+				return
+			}
+
+			if ref != testcase.Expected {
+				t.Fatalf("%#v != %#v", ref, testcase.Expected)
+			}
+
+			if testcase.Normalized == "" {
+				testcase.Normalized = testcase.Input
+			}
+
+			if ref.String() != testcase.Normalized {
+				t.Fatalf("normalization failed: %v != %v", ref.String(), testcase.Normalized)
+			}
+
+			if ref.Digest() != testcase.Digest {
+				t.Fatalf("digest extraction failed: %v != %v", ref.Digest(), testcase.Digest)
+			}
+
+			if ref.Hostname() != testcase.Hostname {
+				t.Fatalf("unexpected hostname: %v != %v", ref.Hostname(), testcase.Hostname)
+			}
+		})
+	}
+}

remotes/remote.go

@@ -1,41 +0,0 @@
-package remotes
-
-import (
-	"context"
-	"io"
-)
-
-type Fetcher interface {
-	// Fetch the resource identified by id. The id is opaque to the remote, but
-	// may typically be a tag or a digest.
-	//
-	// Hints are provided to give instruction on how the resource may be
-	// fetched. They may provide information about the expected type or size.
-	// They may be protocol specific or help a protocol to identify the most
-	// efficient fetch methodology.
-	//
-	// Hints are the format of `<type>:<content>` where `<type>` is the type
-	// of the hint and `<content>` can be pretty much anything. For example, a
-	// media type hint would be the following:
-	//
-	// 	mediatype:application/vnd.docker.distribution.manifest.v2+json
-	//
-	// The following hint names are must be honored across all remote
-	// implementations:
-	//
-	// 	size: specify the expected size in bytes
-	// 	mediatype: specify the expected mediatype
-	//
-	// The caller should never expect the hints to be honored and should
-	// validate that returned content is as expected. They are only provided to
-	// help the remote retrieve the content.
-	Fetch(ctx context.Context, id string, hints ...string) (io.ReadCloser, error)
-}
-
-// FetcherFunc allows package users to implement a Fetcher with just a
-// function.
-type FetcherFunc func(context.Context, string, ...string) (io.ReadCloser, error)
-
-func (fn FetcherFunc) Fetch(ctx context.Context, object string, hints ...string) (io.ReadCloser, error) {
-	return fn(ctx, object, hints...)
-}

remotes/resolver.go

@@ -1,19 +1,37 @@
 package remotes
 
-import "context"
+import (
+	"context"
+	"io"
+
+	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
+)
 
 // Resolver provides a remote based on a locator.
 type Resolver interface {
-	// Resolve returns a remote from the locator.
+	// Resolve attempts to resolve the reference into a name and descriptor.
 	//
-	// A locator is a scheme-less URI representing the remote. Structurally, it
+	// The argument `ref` should be a scheme-less URI representing the remote.
-	// has a host and path. The "host" can be used to directly reference a
+	// Structurally, it has a host and path. The "host" can be used to directly
-	// specific host or be matched against a specific handler.
+	// reference a specific host or be matched against a specific handler.
-	Resolve(ctx context.Context, locator string) (Fetcher, error)
+	//
+	// The returned name should be used to identify the referenced entity.
+	// Dependending on the remote namespace, this may be immutable or mutable.
+	// While the name may differ from ref, it should itself be a valid ref.
+	//
+	// If the resolution fails, an error will be returned.
+	Resolve(ctx context.Context, ref string) (name string, desc ocispec.Descriptor, fetcher Fetcher, err error)
 }
 
-type ResolverFunc func(context.Context, string) (Fetcher, error)
+type Fetcher interface {
-
+	// Fetch the resource identified by the descriptor.
-func (fn ResolverFunc) Resolve(ctx context.Context, locator string) (Fetcher, error) {
+	Fetch(ctx context.Context, desc ocispec.Descriptor) (io.ReadCloser, error)
-	return fn(ctx, locator)
+}
+
+// FetcherFunc allows package users to implement a Fetcher with just a
+// function.
+type FetcherFunc func(ctx context.Context, desc ocispec.Descriptor) (io.ReadCloser, error)
+
+func (fn FetcherFunc) Fetch(ctx context.Context, desc ocispec.Descriptor) (io.ReadCloser, error) {
+	return fn(ctx, desc)
 }