From 9adad264d2bf17bee8ec7b8a29d7112b3f503771 Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Wed, 1 Jun 2022 18:26:35 -0700 Subject: [PATCH] distribution: match manifest list resolution with containerd Make finding the correct runtime image from image index more compliant with OCI spec and match containerd implementation. Changes: - Manifest list is allowed to contain manifest lists - Unknown mediatype inside manifest list is skipped instead of causing an error - Platform in descriptor is optional Signed-off-by: Tonis Tiigi --- distribution/pull_v2.go | 124 +++++++++++++++++++---------------- distribution/pull_v2_unix.go | 17 ++++- 2 files changed, 83 insertions(+), 58 deletions(-) diff --git a/distribution/pull_v2.go b/distribution/pull_v2.go index f4a55f83e7..c63735adec 100644 --- a/distribution/pull_v2.go +++ b/distribution/pull_v2.go @@ -833,64 +833,65 @@ func (p *puller) pullManifestList(ctx context.Context, ref reference.Named, mfst manifestMatches := filterManifests(mfstList.Manifests, platform) - if len(manifestMatches) == 0 { - errMsg := fmt.Sprintf("no matching manifest for %s in the manifest list entries", formatPlatform(platform)) - logrus.Debugf(errMsg) - return "", "", errors.New(errMsg) - } + for _, match := range manifestMatches { + if err := checkImageCompatibility(match.Platform.OS, match.Platform.OSVersion); err != nil { + return "", "", err + } - if len(manifestMatches) > 1 { - logrus.Debugf("found multiple matches in manifest list, choosing best match %s", manifestMatches[0].Digest.String()) - } - match := manifestMatches[0] - - if err := checkImageCompatibility(match.Platform.OS, match.Platform.OSVersion); err != nil { - return "", "", err - } - - desc := specs.Descriptor{ - Digest: match.Digest, - Size: match.Size, - MediaType: match.MediaType, - } - manifest, err := p.manifestStore.Get(ctx, desc) - if err != nil { - return "", "", err - } - - manifestRef, err := reference.WithDigest(reference.TrimNamed(ref), match.Digest) - if err != nil { - return "", "", err - } - - switch v := manifest.(type) { - case *schema1.SignedManifest: - msg := fmt.Sprintf("[DEPRECATION NOTICE] v2 schema1 manifests in manifest lists are not supported and will break in a future release. Suggest author of %s to upgrade to v2 schema2. More information at https://docs.docker.com/registry/spec/deprecated-schema-v1/", ref) - logrus.Warn(msg) - progress.Message(p.config.ProgressOutput, "", msg) - - platform := toOCIPlatform(manifestMatches[0].Platform) - id, _, err = p.pullSchema1(ctx, manifestRef, v, &platform) + desc := specs.Descriptor{ + Digest: match.Digest, + Size: match.Size, + MediaType: match.MediaType, + } + manifest, err := p.manifestStore.Get(ctx, desc) if err != nil { return "", "", err } - case *schema2.DeserializedManifest: - platform := toOCIPlatform(manifestMatches[0].Platform) - id, _, err = p.pullSchema2(ctx, manifestRef, v, &platform) - if err != nil { - return "", "", err - } - case *ocischema.DeserializedManifest: - platform := toOCIPlatform(manifestMatches[0].Platform) - id, _, err = p.pullOCI(ctx, manifestRef, v, &platform) - if err != nil { - return "", "", err - } - default: - return "", "", errors.New("unsupported manifest format") - } - return id, manifestListDigest, err + manifestRef, err := reference.WithDigest(reference.TrimNamed(ref), match.Digest) + if err != nil { + return "", "", err + } + + switch v := manifest.(type) { + case *schema1.SignedManifest: + msg := fmt.Sprintf("[DEPRECATION NOTICE] v2 schema1 manifests in manifest lists are not supported and will break in a future release. Suggest author of %s to upgrade to v2 schema2. More information at https://docs.docker.com/registry/spec/deprecated-schema-v1/", ref) + logrus.Warn(msg) + progress.Message(p.config.ProgressOutput, "", msg) + + platform := toOCIPlatform(match.Platform) + id, _, err = p.pullSchema1(ctx, manifestRef, v, platform) + if err != nil { + return "", "", err + } + case *schema2.DeserializedManifest: + platform := toOCIPlatform(match.Platform) + id, _, err = p.pullSchema2(ctx, manifestRef, v, platform) + if err != nil { + return "", "", err + } + case *ocischema.DeserializedManifest: + platform := toOCIPlatform(match.Platform) + id, _, err = p.pullOCI(ctx, manifestRef, v, platform) + if err != nil { + return "", "", err + } + case *manifestlist.DeserializedManifestList: + id, _, err = p.pullManifestList(ctx, manifestRef, v, pp) + if err != nil { + var noMatches noMatchesErr + if !errors.As(err, &noMatches) { + // test the next match + continue + } + } + default: + // OCI spec requires to skip unknown manifest types + continue + } + return id, manifestListDigest, err + } + return "", "", noMatchesErr{platform: platform} } const ( @@ -922,6 +923,14 @@ func (p *puller) pullSchema2Config(ctx context.Context, dgst digest.Digest) (con return configJSON, nil } +type noMatchesErr struct { + platform specs.Platform +} + +func (e noMatchesErr) Error() string { + return fmt.Sprintf("no matching manifest for %s in the manifest list entries", formatPlatform(e.platform)) +} + func retry(ctx context.Context, maxAttempts int, sleep time.Duration, f func(ctx context.Context) error) (err error) { attempt := 0 for ; attempt < maxAttempts; attempt++ { @@ -1054,8 +1063,13 @@ func createDownloadFile() (*os.File, error) { return os.CreateTemp("", "GetImageBlob") } -func toOCIPlatform(p manifestlist.PlatformSpec) specs.Platform { - return specs.Platform{ +func toOCIPlatform(p manifestlist.PlatformSpec) *specs.Platform { + // distribution pkg does define platform as pointer so this hack for empty struct + // is necessary. This is temporary until correct OCI image-spec package is used. + if p.OS == "" && p.Architecture == "" && p.Variant == "" && p.OSVersion == "" && p.OSFeatures == nil && p.Features == nil { + return nil + } + return &specs.Platform{ OS: p.OS, Architecture: p.Architecture, Variant: p.Variant, diff --git a/distribution/pull_v2_unix.go b/distribution/pull_v2_unix.go index ca4814a472..dad9a85136 100644 --- a/distribution/pull_v2_unix.go +++ b/distribution/pull_v2_unix.go @@ -24,14 +24,25 @@ func filterManifests(manifests []manifestlist.ManifestDescriptor, p specs.Platfo m := platforms.Only(p) var matches []manifestlist.ManifestDescriptor for _, desc := range manifests { - if m.Match(toOCIPlatform(desc.Platform)) { + descP := toOCIPlatform(desc.Platform) + if descP == nil || m.Match(*descP) { matches = append(matches, desc) - logrus.Debugf("found match for %s with media type %s, digest %s", platforms.Format(p), desc.MediaType, desc.Digest.String()) + if descP != nil { + logrus.Debugf("found match for %s with media type %s, digest %s", platforms.Format(p), desc.MediaType, desc.Digest.String()) + } } } sort.SliceStable(matches, func(i, j int) bool { - return m.Less(toOCIPlatform(matches[i].Platform), toOCIPlatform(matches[j].Platform)) + p1 := toOCIPlatform(matches[i].Platform) + if p1 == nil { + return false + } + p2 := toOCIPlatform(matches[j].Platform) + if p2 == nil { + return true + } + return m.Less(*p1, *p2) }) // deprecated: backwards compatibility with older versions that didn't compare variant