Merge pull request #864 from stevvooe/use-correct-manifest-link

registry/storage: use correct manifest link
This commit is contained in:
Richard Scothern 2015-08-12 15:13:45 -07:00
commit 9174fc62d2
5 changed files with 127 additions and 43 deletions

View file

@ -11,6 +11,10 @@ import (
"github.com/docker/distribution/uuid"
)
// linkPathFunc describes a function that can resolve a link based on the
// repository name and digest.
type linkPathFunc func(pm *pathMapper, name string, dgst digest.Digest) (string, error)
// linkedBlobStore provides a full BlobService that namespaces the blobs to a
// given repository. Effectively, it manages the links in a given repository
// that grant access to the global blob store.
@ -23,11 +27,13 @@ type linkedBlobStore struct {
deleteEnabled bool
resumableDigestEnabled bool
// linkPath allows one to control the repository blob link set to which
// the blob store dispatches. This is required because manifest and layer
// blobs have not yet been fully merged. At some point, this functionality
// should be removed an the blob links folder should be merged.
linkPath func(pm *pathMapper, name string, dgst digest.Digest) (string, error)
// linkPathFns specifies one or more path functions allowing one to
// control the repository blob link set to which the blob store
// dispatches. This is required because manifest and layer blobs have not
// yet been fully merged. At some point, this functionality should be
// 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{}
@ -213,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
}
@ -236,33 +245,43 @@ type linkedBlobStatter struct {
*blobStore
repository distribution.Repository
// linkPath allows one to control the repository blob link set to which
// the blob store dispatches. This is required because manifest and layer
// blobs have not yet been fully merged. At some point, this functionality
// should be removed an the blob links folder should be merged.
linkPath func(pm *pathMapper, name string, dgst digest.Digest) (string, error)
// linkPathFns specifies one or more path functions allowing one to
// control the repository blob link set to which the blob store
// dispatches. This is required because manifest and layer blobs have not
// yet been fully merged. At some point, this functionality should be
// 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)
if err != nil {
return distribution.Descriptor{}, err
}
var (
resolveErr error
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
// need to hit layerLinkPath, as well. Or, somehow migrate to the new path
// layout.
if resolveErr != nil {
return distribution.Descriptor{}, resolveErr
}
if target != dgst {
@ -276,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 {
blobLinkPath, err := lbs.linkPath(lbs.pm, lbs.repository.Name(), dgst)
if err != nil {
return err
func (lbs *linkedBlobStatter) Clear(ctx context.Context, dgst digest.Digest) (err error) {
// clear any possible existence of a link described in linkPathFns
for _, linkPathFn := range lbs.linkPathFns {
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 {
@ -297,5 +341,5 @@ func blobLinkPath(pm *pathMapper, name string, dgst digest.Digest) (string, erro
// manifestRevisionLinkPath provides the path to the manifest revision link.
func manifestRevisionLinkPath(pm *pathMapper, name string, dgst digest.Digest) (string, error) {
return pm.path(layerLinkPathSpec{name: name, digest: dgst})
return pm.path(manifestRevisionLinkPathSpec{name: name, revision: dgst})
}

View file

@ -362,3 +362,37 @@ func TestManifestStorage(t *testing.T) {
t.Errorf("Unexpected success deleting while disabled")
}
}
// TestLinkPathFuncs ensures that the link path functions behavior are locked
// down and implemented as expected.
func TestLinkPathFuncs(t *testing.T) {
for _, testcase := range []struct {
repo string
digest digest.Digest
linkPathFn linkPathFunc
expected string
}{
{
repo: "foo/bar",
digest: "sha256:deadbeaf",
linkPathFn: blobLinkPath,
expected: "/docker/registry/v2/repositories/foo/bar/_layers/sha256/deadbeaf/link",
},
{
repo: "foo/bar",
digest: "sha256:deadbeaf",
linkPathFn: manifestRevisionLinkPath,
expected: "/docker/registry/v2/repositories/foo/bar/_manifests/revisions/sha256/deadbeaf/link",
},
} {
p, err := testcase.linkPathFn(defaultPathMapper, testcase.repo, testcase.digest)
if err != nil {
t.Fatalf("unexpected error calling linkPathFn(pm, %q, %q): %v", testcase.repo, testcase.digest, err)
}
if p != testcase.expected {
t.Fatalf("incorrect path returned: %q != %q", p, testcase.expected)
}
}
}

View file

@ -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,
repository: repo,
linkPath: manifestRevisionLinkPath,
blobStore: repo.blobStore,
repository: repo,
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,
repository: repo,
linkPath: blobLinkPath,
blobStore: repo.blobStore,
repository: repo,
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,
}
}

View file

@ -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,
repository: s.repository,
linkPath: linkpath,
blobStore: s.blobStore,
repository: s.repository,
linkPathFns: []linkPathFunc{linkpath},
},
linkPath: linkpath,
linkPathFns: []linkPathFunc{linkpath},
}
}

View file

@ -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,
})
},
}},
}
}