Maintain manifest link compatibility

Unfortunately, the 2.1 releease has written manfiest links into the wrong
directory. This doesn't affect new 2.1 deployments but fixing this to be 2.0
backwards compatible has broken 2.1.0 compatibility. To ensure we have
compatibility between 2.0, 2.1.0 and future releases, we now check one of
several locations to identify a manifest link.

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

docs/storage/linkedblobstore.go

@@ -27,11 +27,13 @@ type linkedBlobStore struct {
 	deleteEnabled          bool
 	resumableDigestEnabled bool
 
-	// linkPath allows one to control the repository blob link set to which
+	// linkPathFns specifies one or more path functions allowing one to
-	// the blob store dispatches. This is required because manifest and layer
+	// control the repository blob link set to which the blob store
-	// blobs have not yet been fully merged. At some point, this functionality
+	// dispatches. This is required because manifest and layer blobs have not
-	// should be removed an the blob links folder should be merged.
+	// yet been fully merged. At some point, this functionality should be
-	linkPath func(pm *pathMapper, name string, dgst digest.Digest) (string, error)
+	// removed an the blob links folder should be merged. The first entry is
+	// treated as the "canonical" link location and will be used for writes.
+	linkPathFns []linkPathFunc
 }
 
 var _ distribution.BlobStore = &linkedBlobStore{}
@@ -217,13 +219,16 @@ func (lbs *linkedBlobStore) linkBlob(ctx context.Context, canonical distribution
 	// Don't make duplicate links.
 	seenDigests := make(map[digest.Digest]struct{}, len(dgsts))
 
+	// only use the first link
+	linkPathFn := lbs.linkPathFns[0]
+
 	for _, dgst := range dgsts {
 		if _, seen := seenDigests[dgst]; seen {
 			continue
 		}
 		seenDigests[dgst] = struct{}{}
 
-		blobLinkPath, err := lbs.linkPath(lbs.pm, lbs.repository.Name(), dgst)
+		blobLinkPath, err := linkPathFn(lbs.pm, lbs.repository.Name(), dgst)
 		if err != nil {
 			return err
 		}
@@ -240,33 +245,43 @@ type linkedBlobStatter struct {
 	*blobStore
 	repository distribution.Repository
 
-	// linkPath allows one to control the repository blob link set to which
+	// linkPathFns specifies one or more path functions allowing one to
-	// the blob store dispatches. This is required because manifest and layer
+	// control the repository blob link set to which the blob store
-	// blobs have not yet been fully merged. At some point, this functionality
+	// dispatches. This is required because manifest and layer blobs have not
-	// should be removed an the blob links folder should be merged.
+	// yet been fully merged. At some point, this functionality should be
-	linkPath func(pm *pathMapper, name string, dgst digest.Digest) (string, error)
+	// removed an the blob links folder should be merged. The first entry is
+	// treated as the "canonical" link location and will be used for writes.
+	linkPathFns []linkPathFunc
 }
 
 var _ distribution.BlobDescriptorService = &linkedBlobStatter{}
 
 func (lbs *linkedBlobStatter) Stat(ctx context.Context, dgst digest.Digest) (distribution.Descriptor, error) {
-	blobLinkPath, err := lbs.linkPath(lbs.pm, lbs.repository.Name(), dgst)
+	var (
-	if err != nil {
+		resolveErr error
-		return distribution.Descriptor{}, err
+		target     digest.Digest
-	}
+	)
+
+	// try the many link path functions until we get success or an error that
+	// is not PathNotFoundError.
+	for _, linkPathFn := range lbs.linkPathFns {
+		var err error
+		target, err = lbs.resolveWithLinkFunc(ctx, dgst, linkPathFn)
+
+		if err == nil {
+			break // success!
+		}
 
-	target, err := lbs.blobStore.readlink(ctx, blobLinkPath)
-	if err != nil {
 		switch err := err.(type) {
 		case driver.PathNotFoundError:
-			return distribution.Descriptor{}, distribution.ErrBlobUnknown
+			resolveErr = distribution.ErrBlobUnknown // move to the next linkPathFn, saving the error
 		default:
 			return distribution.Descriptor{}, err
 		}
+	}
 
-		// TODO(stevvooe): For backwards compatibility with data in "_layers", we
+	if resolveErr != nil {
-		// need to hit layerLinkPath, as well. Or, somehow migrate to the new path
+		return distribution.Descriptor{}, resolveErr
-		// layout.
 	}
 
 	if target != dgst {
@@ -280,13 +295,38 @@ func (lbs *linkedBlobStatter) Stat(ctx context.Context, dgst digest.Digest) (dis
 	return lbs.blobStore.statter.Stat(ctx, target)
 }
 
-func (lbs *linkedBlobStatter) Clear(ctx context.Context, dgst digest.Digest) error {
+func (lbs *linkedBlobStatter) Clear(ctx context.Context, dgst digest.Digest) (err error) {
-	blobLinkPath, err := lbs.linkPath(lbs.pm, lbs.repository.Name(), dgst)
+	// clear any possible existence of a link described in linkPathFns
-	if err != nil {
+	for _, linkPathFn := range lbs.linkPathFns {
-		return err
+		blobLinkPath, err := linkPathFn(lbs.pm, lbs.repository.Name(), dgst)
+		if err != nil {
+			return err
+		}
+
+		err = lbs.blobStore.driver.Delete(ctx, blobLinkPath)
+		if err != nil {
+			switch err := err.(type) {
+			case driver.PathNotFoundError:
+				continue // just ignore this error and continue
+			default:
+				return err
+			}
+		}
 	}
 
-	return lbs.blobStore.driver.Delete(ctx, blobLinkPath)
+	return nil
+}
+
+// resolveTargetWithFunc allows us to read a link to a resource with different
+// linkPathFuncs to let us try a few different paths before returning not
+// found.
+func (lbs *linkedBlobStatter) resolveWithLinkFunc(ctx context.Context, dgst digest.Digest, linkPathFn linkPathFunc) (digest.Digest, error) {
+	blobLinkPath, err := linkPathFn(lbs.pm, lbs.repository.Name(), dgst)
+	if err != nil {
+		return "", err
+	}
+
+	return lbs.blobStore.readlink(ctx, blobLinkPath)
 }
 
 func (lbs *linkedBlobStatter) SetDescriptor(ctx context.Context, dgst digest.Digest, desc distribution.Descriptor) error {

docs/storage/registry.go

@@ -108,6 +108,13 @@ func (repo *repository) Name() string {
 // may be context sensitive in the future. The instance should be used similar
 // to a request local.
 func (repo *repository) Manifests(ctx context.Context, options ...distribution.ManifestServiceOption) (distribution.ManifestService, error) {
+	manifestLinkPathFns := []linkPathFunc{
+		// NOTE(stevvooe): Need to search through multiple locations since
+		// 2.1.0 unintentionally linked into  _layers.
+		manifestRevisionLinkPath,
+		blobLinkPath,
+	}
+
 	ms := &manifestStore{
 		ctx:        ctx,
 		repository: repo,
@@ -120,14 +127,14 @@ func (repo *repository) Manifests(ctx context.Context, options ...distribution.M
 				repository:    repo,
 				deleteEnabled: repo.registry.deleteEnabled,
 				blobAccessController: &linkedBlobStatter{
-					blobStore:  repo.blobStore,
+					blobStore:   repo.blobStore,
-					repository: repo,
+					repository:  repo,
-					linkPath:   manifestRevisionLinkPath,
+					linkPathFns: manifestLinkPathFns,
 				},
 
 				// TODO(stevvooe): linkPath limits this blob store to only
 				// manifests. This instance cannot be used for blob checks.
-				linkPath: manifestRevisionLinkPath,
+				linkPathFns: manifestLinkPathFns,
 			},
 		},
 		tagStore: &tagStore{
@@ -153,9 +160,9 @@ func (repo *repository) Manifests(ctx context.Context, options ...distribution.M
 // to a request local.
 func (repo *repository) Blobs(ctx context.Context) distribution.BlobStore {
 	var statter distribution.BlobDescriptorService = &linkedBlobStatter{
-		blobStore:  repo.blobStore,
+		blobStore:   repo.blobStore,
-		repository: repo,
+		repository:  repo,
-		linkPath:   blobLinkPath,
+		linkPathFns: []linkPathFunc{blobLinkPath},
 	}
 
 	if repo.descriptorCache != nil {
@@ -171,7 +178,7 @@ func (repo *repository) Blobs(ctx context.Context) distribution.BlobStore {
 
 		// TODO(stevvooe): linkPath limits this blob store to only layers.
 		// This instance cannot be used for manifest checks.
-		linkPath:      blobLinkPath,
+		linkPathFns:   []linkPathFunc{blobLinkPath},
 		deleteEnabled: repo.registry.deleteEnabled,
 	}
 }

docs/storage/signaturestore.go

@@ -132,10 +132,10 @@ func (s *signatureStore) linkedBlobStore(ctx context.Context, revision digest.Di
 		repository: s.repository,
 		blobStore:  s.blobStore,
 		blobAccessController: &linkedBlobStatter{
-			blobStore:  s.blobStore,
+			blobStore:   s.blobStore,
-			repository: s.repository,
+			repository:  s.repository,
-			linkPath:   linkpath,
+			linkPathFns: []linkPathFunc{linkpath},
 		},
-		linkPath: linkpath,
+		linkPathFns: []linkPathFunc{linkpath},
 	}
 }

docs/storage/tagstore.go

@@ -122,7 +122,7 @@ func (ts *tagStore) delete(tag string) error {
 	return ts.blobStore.driver.Delete(ts.ctx, tagPath)
 }
 
-// namedBlobStore returns the namedBlobStore for the named tag, allowing one
+// linkedBlobStore returns the linkedBlobStore for the named tag, allowing one
 // to index manifest blobs by tag name. While the tag store doesn't map
 // precisely to the linked blob store, using this ensures the links are
 // managed via the same code path.
@@ -131,13 +131,12 @@ func (ts *tagStore) linkedBlobStore(ctx context.Context, tag string) *linkedBlob
 		blobStore:  ts.blobStore,
 		repository: ts.repository,
 		ctx:        ctx,
-		linkPath: func(pm *pathMapper, name string, dgst digest.Digest) (string, error) {
+		linkPathFns: []linkPathFunc{func(pm *pathMapper, name string, dgst digest.Digest) (string, error) {
 			return pm.path(manifestTagIndexEntryLinkPathSpec{
 				name:     name,
 				tag:      tag,
 				revision: dgst,
 			})
-		},
+		}},
 	}
-
 }