diff --git a/registry/storage/linkedblobstore.go b/registry/storage/linkedblobstore.go index 2ba62a958..dc670542f 100644 --- a/registry/storage/linkedblobstore.go +++ b/registry/storage/linkedblobstore.go @@ -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}) } diff --git a/registry/storage/manifeststore_test.go b/registry/storage/manifeststore_test.go index a4ce9149f..0bb72fb05 100644 --- a/registry/storage/manifeststore_test.go +++ b/registry/storage/manifeststore_test.go @@ -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) + } + } + +} diff --git a/registry/storage/registry.go b/registry/storage/registry.go index c5058b801..b6e0ba4df 100644 --- a/registry/storage/registry.go +++ b/registry/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, - 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, } } diff --git a/registry/storage/signaturestore.go b/registry/storage/signaturestore.go index 78fd2e6cb..105d66f39 100644 --- a/registry/storage/signaturestore.go +++ b/registry/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, - repository: s.repository, - linkPath: linkpath, + blobStore: s.blobStore, + repository: s.repository, + linkPathFns: []linkPathFunc{linkpath}, }, - linkPath: linkpath, + linkPathFns: []linkPathFunc{linkpath}, } } diff --git a/registry/storage/tagstore.go b/registry/storage/tagstore.go index a74d9b094..a7ca3301a 100644 --- a/registry/storage/tagstore.go +++ b/registry/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, }) - }, + }}, } - }