manifest: references should cover all children

To allow generic manifest walking, we define an interface method of
`References` that returns the referenced items in the manifest. The
current implementation does not return the config target from schema2,
making this useless for most applications.

The garbage collector has been modified to show the utility of this
correctly formed `References` method. We may be able to make more
generic traversal methods with this, as well.

Signed-off-by: Stephen J Day <stephen.day@docker.com>

registry/storage/garbagecollect.go

@@ -6,7 +6,6 @@ import (
 	"github.com/docker/distribution"
 	"github.com/docker/distribution/context"
 	"github.com/docker/distribution/digest"
-	"github.com/docker/distribution/manifest/schema2"
 	"github.com/docker/distribution/reference"
 	"github.com/docker/distribution/registry/storage/driver"
 )
@@ -63,14 +62,6 @@ func MarkAndSweep(ctx context.Context, storageDriver driver.StorageDriver, regis
 				emit("%s: marking blob %s", repoName, descriptor.Digest)
 			}
 
-			switch manifest.(type) {
-			case *schema2.DeserializedManifest:
-				config := manifest.(*schema2.DeserializedManifest).Config
-				emit("%s: marking configuration %s", repoName, config.Digest)
-				markSet[config.Digest] = struct{}{}
-				break
-			}
-
 			return nil
 		})
 

registry/storage/schema2manifesthandler.go

@@ -9,6 +9,7 @@ import (
 	"github.com/docker/distribution"
 	"github.com/docker/distribution/context"
 	"github.com/docker/distribution/digest"
+	"github.com/docker/distribution/manifest/schema1"
 	"github.com/docker/distribution/manifest/schema2"
 )
 
@@ -71,53 +72,62 @@ func (ms *schema2ManifestHandler) Put(ctx context.Context, manifest distribution
 func (ms *schema2ManifestHandler) verifyManifest(ctx context.Context, mnfst schema2.DeserializedManifest, skipDependencyVerification bool) error {
 	var errs distribution.ErrManifestVerification
 
-	if !skipDependencyVerification {
-		target := mnfst.Target()
-		_, err := ms.repository.Blobs(ctx).Stat(ctx, target.Digest)
+	if skipDependencyVerification {
+		return nil
+	}
+
+	manifestService, err := ms.repository.Manifests(ctx)
+	if err != nil {
+		return err
+	}
+
+	blobsService := ms.repository.Blobs(ctx)
+
+	for _, descriptor := range mnfst.References() {
+		var err error
+
+		switch descriptor.MediaType {
+		case schema2.MediaTypeForeignLayer:
+			// Clients download this layer from an external URL, so do not check for
+			// its presense.
+			if len(descriptor.URLs) == 0 {
+				err = errMissingURL
+			}
+			allow := ms.manifestURLs.allow
+			deny := ms.manifestURLs.deny
+			for _, u := range descriptor.URLs {
+				var pu *url.URL
+				pu, err = url.Parse(u)
+				if err != nil || (pu.Scheme != "http" && pu.Scheme != "https") || pu.Fragment != "" || (allow != nil && !allow.MatchString(u)) || (deny != nil && deny.MatchString(u)) {
+					err = errInvalidURL
+					break
+				}
+			}
+		case schema2.MediaTypeManifest, schema1.MediaTypeManifest:
+			var exists bool
+			exists, err = manifestService.Exists(ctx, descriptor.Digest)
+			if err != nil || !exists {
+				err = distribution.ErrBlobUnknown // just coerce to unknown.
+			}
+
+			fallthrough // double check the blob store.
+		default:
+			// forward all else to blob storage
+			if len(descriptor.URLs) == 0 {
+				_, err = blobsService.Stat(ctx, descriptor.Digest)
+			}
+		}
+
 		if err != nil {
 			if err != distribution.ErrBlobUnknown {
 				errs = append(errs, err)
 			}
 
 			// On error here, we always append unknown blob errors.
-			errs = append(errs, distribution.ErrManifestBlobUnknown{Digest: target.Digest})
-		}
-
-		for _, fsLayer := range mnfst.References() {
-			var err error
-			if fsLayer.MediaType != schema2.MediaTypeForeignLayer {
-				if len(fsLayer.URLs) == 0 {
-					_, err = ms.repository.Blobs(ctx).Stat(ctx, fsLayer.Digest)
-				} else {
-					err = errUnexpectedURL
-				}
-			} else {
-				// Clients download this layer from an external URL, so do not check for
-				// its presense.
-				if len(fsLayer.URLs) == 0 {
-					err = errMissingURL
-				}
-				allow := ms.manifestURLs.allow
-				deny := ms.manifestURLs.deny
-				for _, u := range fsLayer.URLs {
-					var pu *url.URL
-					pu, err = url.Parse(u)
-					if err != nil || (pu.Scheme != "http" && pu.Scheme != "https") || pu.Fragment != "" || (allow != nil && !allow.MatchString(u)) || (deny != nil && deny.MatchString(u)) {
-						err = errInvalidURL
-						break
-					}
-				}
-			}
-			if err != nil {
-				if err != distribution.ErrBlobUnknown {
-					errs = append(errs, err)
-				}
-
-				// On error here, we always append unknown blob errors.
-				errs = append(errs, distribution.ErrManifestBlobUnknown{Digest: fsLayer.Digest})
-			}
+			errs = append(errs, distribution.ErrManifestBlobUnknown{Digest: descriptor.Digest})
 		}
 	}
+
 	if len(errs) != 0 {
 		return errs
 	}

registry/storage/schema2manifesthandler_test.go

@@ -57,9 +57,10 @@ func TestVerifyManifestForeignLayer(t *testing.T) {
 			errMissingURL,
 		},
 		{
+			// regular layers may have foreign urls
 			layer,
 			[]string{"http://foo/bar"},
-			errUnexpectedURL,
+			nil,
 		},
 		{
 			foreignLayer,