Remove workaround from 2.1.1 for faulty 2.1.0 manifest links

This reverts commit 06a098c632

This changes the function of linkedBlobStatter.Clear(). It was either removing the first of two possible manifest links or returning nil if none were found. Now it once again it removes only the valid manifest link or returns an error if none are found.

Signed-off-by: Bracken Dawson <abdawson@gmail.com>
This commit is contained in:
Bracken Dawson 2021-02-15 18:36:19 +00:00
parent 5fe693474e
commit b2b3f86039
No known key found for this signature in database
GPG key ID: 0A29122E6FE51E41
3 changed files with 37 additions and 90 deletions

View file

@ -32,13 +32,11 @@ type linkedBlobStore struct {
deleteEnabled bool deleteEnabled bool
resumableDigestEnabled bool resumableDigestEnabled bool
// linkPathFns specifies one or more path functions allowing one to // linkPath allows one to control the repository blob link set to which
// control the repository blob link set to which the blob store // the blob store dispatches. This is required because manifest and layer
// dispatches. This is required because manifest and layer blobs have not // blobs have not yet been fully merged. At some point, this functionality
// yet been fully merged. At some point, this functionality should be // should be removed an the blob links folder should be merged.
// removed the blob links folder should be merged. The first entry is linkPath linkPathFunc
// treated as the "canonical" link location and will be used for writes.
linkPathFns []linkPathFunc
// linkDirectoryPathSpec locates the root directories in which one might find links // linkDirectoryPathSpec locates the root directories in which one might find links
linkDirectoryPathSpec pathSpec linkDirectoryPathSpec pathSpec
@ -338,16 +336,13 @@ func (lbs *linkedBlobStore) linkBlob(ctx context.Context, canonical distribution
// Don't make duplicate links. // Don't make duplicate links.
seenDigests := make(map[digest.Digest]struct{}, len(dgsts)) seenDigests := make(map[digest.Digest]struct{}, len(dgsts))
// only use the first link
linkPathFn := lbs.linkPathFns[0]
for _, dgst := range dgsts { for _, dgst := range dgsts {
if _, seen := seenDigests[dgst]; seen { if _, seen := seenDigests[dgst]; seen {
continue continue
} }
seenDigests[dgst] = struct{}{} seenDigests[dgst] = struct{}{}
blobLinkPath, err := linkPathFn(lbs.repository.Named().Name(), dgst) blobLinkPath, err := lbs.linkPath(lbs.repository.Named().Name(), dgst)
if err != nil { if err != nil {
return err return err
} }
@ -364,46 +359,31 @@ type linkedBlobStatter struct {
*blobStore *blobStore
repository distribution.Repository repository distribution.Repository
// linkPathFns specifies one or more path functions allowing one to // linkPath allows one to control the repository blob link set to which
// control the repository blob link set to which the blob store // the blob store dispatches. This is required because manifest and layer
// dispatches. This is required because manifest and layer blobs have not // blobs have not yet been fully merged. At some point, this functionality
// yet been fully merged. At some point, this functionality should be // should be removed an the blob links folder should be merged.
// removed an the blob links folder should be merged. The first entry is linkPath linkPathFunc
// treated as the "canonical" link location and will be used for writes.
linkPathFns []linkPathFunc
} }
var _ distribution.BlobDescriptorService = &linkedBlobStatter{} var _ distribution.BlobDescriptorService = &linkedBlobStatter{}
func (lbs *linkedBlobStatter) Stat(ctx context.Context, dgst digest.Digest) (distribution.Descriptor, error) { func (lbs *linkedBlobStatter) Stat(ctx context.Context, dgst digest.Digest) (distribution.Descriptor, error) {
var ( blobLinkPath, err := lbs.linkPath(lbs.repository.Named().Name(), dgst)
found bool if err != nil {
target digest.Digest return distribution.Descriptor{}, err
)
// 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 {
found = true
break // success!
} }
target, err := lbs.blobStore.readlink(ctx, blobLinkPath)
if err != nil {
switch err := err.(type) { switch err := err.(type) {
case driver.PathNotFoundError: case driver.PathNotFoundError:
// do nothing, just move to the next linkPathFn return distribution.Descriptor{}, distribution.ErrBlobUnknown
default: default:
return distribution.Descriptor{}, err return distribution.Descriptor{}, err
} }
} }
if !found {
return distribution.Descriptor{}, distribution.ErrBlobUnknown
}
if target != dgst { if target != dgst {
// Track when we are doing cross-digest domain lookups. ie, sha512 to sha256. // Track when we are doing cross-digest domain lookups. ie, sha512 to sha256.
dcontext.GetLogger(ctx).Warnf("looking up blob with canonical target: %v -> %v", dgst, target) dcontext.GetLogger(ctx).Warnf("looking up blob with canonical target: %v -> %v", dgst, target)
@ -416,37 +396,12 @@ func (lbs *linkedBlobStatter) Stat(ctx context.Context, dgst digest.Digest) (dis
} }
func (lbs *linkedBlobStatter) Clear(ctx context.Context, dgst digest.Digest) (err error) { func (lbs *linkedBlobStatter) Clear(ctx context.Context, dgst digest.Digest) (err error) {
// clear any possible existence of a link described in linkPathFns blobLinkPath, err := lbs.linkPath(lbs.repository.Named().Name(), dgst)
for _, linkPathFn := range lbs.linkPathFns {
blobLinkPath, err := linkPathFn(lbs.repository.Named().Name(), dgst)
if err != nil { if err != nil {
return err return err
} }
err = lbs.blobStore.driver.Delete(ctx, blobLinkPath) return 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 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.repository.Named().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 { func (lbs *linkedBlobStatter) SetDescriptor(ctx context.Context, dgst digest.Digest, desc distribution.Descriptor) error {

View file

@ -215,19 +215,12 @@ func (repo *repository) Tags(ctx context.Context) distribution.TagService {
// may be context sensitive in the future. The instance should be used similar // may be context sensitive in the future. The instance should be used similar
// to a request local. // to a request local.
func (repo *repository) Manifests(ctx context.Context, options ...distribution.ManifestServiceOption) (distribution.ManifestService, error) { 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,
}
manifestDirectoryPathSpec := manifestRevisionsPathSpec{name: repo.name.Name()} manifestDirectoryPathSpec := manifestRevisionsPathSpec{name: repo.name.Name()}
var statter distribution.BlobDescriptorService = &linkedBlobStatter{ var statter distribution.BlobDescriptorService = &linkedBlobStatter{
blobStore: repo.blobStore, blobStore: repo.blobStore,
repository: repo, repository: repo,
linkPathFns: manifestLinkPathFns, linkPath: manifestRevisionLinkPath,
} }
if repo.registry.blobDescriptorServiceFactory != nil { if repo.registry.blobDescriptorServiceFactory != nil {
@ -243,7 +236,7 @@ func (repo *repository) Manifests(ctx context.Context, options ...distribution.M
// TODO(stevvooe): linkPath limits this blob store to only // TODO(stevvooe): linkPath limits this blob store to only
// manifests. This instance cannot be used for blob checks. // manifests. This instance cannot be used for blob checks.
linkPathFns: manifestLinkPathFns, linkPath: manifestRevisionLinkPath,
linkDirectoryPathSpec: manifestDirectoryPathSpec, linkDirectoryPathSpec: manifestDirectoryPathSpec,
} }
@ -308,7 +301,7 @@ func (repo *repository) Blobs(ctx context.Context) distribution.BlobStore {
var statter distribution.BlobDescriptorService = &linkedBlobStatter{ var statter distribution.BlobDescriptorService = &linkedBlobStatter{
blobStore: repo.blobStore, blobStore: repo.blobStore,
repository: repo, repository: repo,
linkPathFns: []linkPathFunc{blobLinkPath}, linkPath: blobLinkPath,
} }
if repo.descriptorCache != nil { if repo.descriptorCache != nil {
@ -329,7 +322,7 @@ func (repo *repository) Blobs(ctx context.Context) distribution.BlobStore {
// TODO(stevvooe): linkPath limits this blob store to only layers. // TODO(stevvooe): linkPath limits this blob store to only layers.
// This instance cannot be used for manifest checks. // This instance cannot be used for manifest checks.
linkPathFns: []linkPathFunc{blobLinkPath}, linkPath: blobLinkPath,
linkDirectoryPathSpec: layersPathSpec{name: repo.name.Name()}, linkDirectoryPathSpec: layersPathSpec{name: repo.name.Name()},
deleteEnabled: repo.registry.deleteEnabled, deleteEnabled: repo.registry.deleteEnabled,
resumableDigestEnabled: repo.resumableDigestEnabled, resumableDigestEnabled: repo.resumableDigestEnabled,

View file

@ -124,14 +124,13 @@ func (ts *tagStore) linkedBlobStore(ctx context.Context, tag string) *linkedBlob
blobStore: ts.blobStore, blobStore: ts.blobStore,
repository: ts.repository, repository: ts.repository,
ctx: ctx, ctx: ctx,
linkPathFns: []linkPathFunc{func(name string, dgst digest.Digest) (string, error) { linkPath: func(name string, dgst digest.Digest) (string, error) {
return pathFor(manifestTagIndexEntryLinkPathSpec{ return pathFor(manifestTagIndexEntryLinkPathSpec{
name: name, name: name,
tag: tag, tag: tag,
revision: dgst, revision: dgst,
}) })
},
}},
} }
} }
@ -187,11 +186,11 @@ func (ts *tagStore) ManifestDigests(ctx context.Context, tag string) ([]digest.D
blobAccessController: &linkedBlobStatter{ blobAccessController: &linkedBlobStatter{
blobStore: ts.blobStore, blobStore: ts.blobStore,
repository: ts.repository, repository: ts.repository,
linkPathFns: []linkPathFunc{manifestRevisionLinkPath}, linkPath: manifestRevisionLinkPath,
}, },
repository: ts.repository, repository: ts.repository,
ctx: ctx, ctx: ctx,
linkPathFns: []linkPathFunc{tagLinkPath}, linkPath: tagLinkPath,
linkDirectoryPathSpec: manifestTagIndexPathSpec{ linkDirectoryPathSpec: manifestTagIndexPathSpec{
name: ts.repository.Named().Name(), name: ts.repository.Named().Name(),
tag: tag, tag: tag,