Merge pull request #3365 from brackendawson/3122-remove-workaround

Remove workaround from 2.1.1 for faulty 2.1.0 manifest links
This commit is contained in:
Milos Gajdos 2022-10-19 09:04:24 +01:00 committed by GitHub
commit fb2188868d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
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,