From 24c2028a7709ca1b26bd2534e9b415e04a4f9865 Mon Sep 17 00:00:00 2001 From: Aleksei Sviridkin Date: Fri, 14 Aug 2026 15:33:48 +0300 Subject: [PATCH 1/2] fix(registry): select the requested chart from an OCI Image Index Pulling a reference that resolves to an Image Index collected every descriptor matching a fixed media-type allow-list, then assigned the config, chart and provenance descriptors with a switch that kept the last match of each kind. That slice is filled from oras PreCopy callbacks during a concurrent graph walk, so its order is neither the order of the index nor defined by anything else, and each kind is resolved independently of the others. For an index holding more than one chart the delivered chart is arbitrary, and because the metadata is read from the config blob while the bytes come from the chart layer, the two can describe different charts. Nothing on the path compares them, and the file name is taken from the reference rather than from either. Select the chart manifest before copying instead. An entry declaring the chart artifactType is a candidate, whether or not it also carries a platform, so that tooling stamping a platform on every index entry cannot hide the chart. Entries declaring no type are matched on their config mediaType, which keeps an index written before artifactType existed resolvable. That second pass runs when the first found nothing, and also when no single entry it found announces the whole requested name and version, since narrowing the candidates before that is checked is how a mixed index answers with the wrong chart. Asking per attribute instead would let one entry supply the name and another the version, and the pair they form belongs to neither. Several candidates are then narrowed by the requested name and, if that leaves more than one, by the requested version. Both are read from the descriptor annotations, falling back to the candidate's own config for whichever an index omits. Entries repeated in the index are collapsed first, so one chart is never called ambiguous with itself, and an answer that rests on nothing else matching is withheld when the pass could not read every entry, because absence is a fact about the index only while the whole index could be read. Reading a candidate's identity can fail the same way and now says so, so a chart whose name could not be fetched is no longer indistinguishable from a chart that is not there. A choice that name and version cannot settle is an error naming the candidates rather than an arbitrary pick. One consequence worth naming. Pulling an index reference reports the digest of the selected chart manifest where it previously reported the digest of the index. Selection runs inside the copy, on the root oras has already resolved, so narrowing an index costs no request of its own. The pass over the entries that declare no type is what costs requests, one per entry, and only when it runs. Signed-off-by: Aleksei Sviridkin Assisted-By: Claude --- pkg/registry/client.go | 34 +- pkg/registry/constants.go | 7 + pkg/registry/generic.go | 381 +++++++++++- pkg/registry/index_test.go | 1177 ++++++++++++++++++++++++++++++++++++ 4 files changed, 1594 insertions(+), 5 deletions(-) create mode 100644 pkg/registry/index_test.go diff --git a/pkg/registry/client.go b/pkg/registry/client.go index 4c7b9a6c8..009fc1b47 100644 --- a/pkg/registry/client.go +++ b/pkg/registry/client.go @@ -28,6 +28,7 @@ import ( "net/http" "net/url" "os" + "path" "sort" "strings" @@ -566,7 +567,6 @@ func (c *Client) Pull(ref string, options ...PullOption) (*PullResult, error) { // Build allowed media types for chart pull allowedMediaTypes := []string{ - ocispec.MediaTypeImageIndex, ocispec.MediaTypeImageManifest, ConfigMediaType, } @@ -577,10 +577,29 @@ func (c *Client) Pull(ref string, options ...PullOption) (*PullResult, error) { allowedMediaTypes = append(allowedMediaTypes, ProvLayerMediaType) } + // An Image Index may hold several chart manifests. Name them, so selection can + // pick the requested one instead of whichever descriptor happens to arrive last. + // The chart name is the reference's last path segment; the version, when the + // reference carries a tag, breaks a tie between same-named charts. + var selectors map[string]string + if parsed, perr := newReference(ref); perr == nil { + if name := path.Base(parsed.Repository); name != "" && name != "." { + selectors = map[string]string{ocispec.AnnotationTitle: name} + if parsed.Tag != "" { + // newReference stores "+" as "_" in a tag; undo it to compare against + // the chart's own version annotation. + selectors[ocispec.AnnotationVersion] = strings.ReplaceAll(parsed.Tag, "_", "+") + } + } + } + // Use generic client for the pull operation genericClient := c.Generic() genericResult, err := genericClient.PullGeneric(ref, GenericPullOptions{ AllowedMediaTypes: allowedMediaTypes, + ArtifactType: ChartArtifactType, + Selectors: selectors, + ParseIdentity: parseChartIdentity, }) if err != nil { return nil, err @@ -905,6 +924,19 @@ func (c *Client) ValidateReference(ref, version string, u *url.URL) (string, *ur return "", u, err } +// parseChartIdentity reads a chart's name and version out of its config blob, +// which is Chart.yaml serialised as JSON. +func parseChartIdentity(config []byte) (name, version string, err error) { + var meta struct { + Name string `json:"name"` + Version string `json:"version"` + } + if err := json.Unmarshal(config, &meta); err != nil { + return "", "", err + } + return meta.Name, meta.Version, nil +} + // tagManifest prepares and tags a manifest in memory storage func (c *Client) tagManifest(ctx context.Context, memoryStore *memory.Store, configDescriptor ocispec.Descriptor, layers []ocispec.Descriptor, diff --git a/pkg/registry/constants.go b/pkg/registry/constants.go index df31a18c4..ab17c4359 100644 --- a/pkg/registry/constants.go +++ b/pkg/registry/constants.go @@ -34,4 +34,11 @@ const ( // LegacyChartLayerMediaType is the legacy reserved media type for Helm chart package content. LegacyChartLayerMediaType = "application/tar+gzip" + + // ChartArtifactType identifies a Helm chart manifest inside an OCI Image Index. + // The OCI image spec defines an index descriptor's artifactType as the value of + // the referenced manifest's config mediaType, so the two are the same string by + // design rather than by coincidence. + // https://github.com/opencontainers/image-spec/blob/main/descriptor.md#properties + ChartArtifactType = ConfigMediaType ) diff --git a/pkg/registry/generic.go b/pkg/registry/generic.go index b46133d91..8f7ea3e44 100644 --- a/pkg/registry/generic.go +++ b/pkg/registry/generic.go @@ -18,10 +18,14 @@ package registry import ( "context" + "encoding/json" + "errors" + "fmt" "io" "net/http" "slices" "sort" + "strings" "sync" ocispec "github.com/opencontainers/image-spec/specs-go/v1" @@ -33,7 +37,9 @@ import ( "oras.land/oras-go/v2/registry/remote/credentials" ) -// GenericClient provides low-level OCI operations without artifact-specific assumptions +// GenericClient provides low-level OCI operations parameterised by artifact type +// rather than hardcoded to one artifact. Its diagnostics are not: index selection +// reports what it rejected in the vocabulary of Helm charts. type GenericClient struct { debug bool enableCache bool @@ -56,6 +62,28 @@ type GenericPullOptions struct { SkipMediaTypes []string // Custom PreCopy function for filtering PreCopy func(context.Context, ocispec.Descriptor) error + // ArtifactType to select from OCI Image Index (empty means no filtering). + // When pulling from an Image Index containing multiple manifests, + // this field is used to select the manifest with matching artifactType. + ArtifactType string + // Selectors disambiguate when more than one manifest in an Image Index matches + // ArtifactType. Two keys are honoured and the rest are ignored: + // org.opencontainers.image.title and org.opencontainers.image.version. Either + // annotation may be absent, and the missing one is then read out of the + // candidate's own config blob. + // + // Both choose between candidates rather than validate one: a reference that + // resolves to a single chart yields that chart whatever was asked for, matching + // what a reference to a plain manifest already does, and the version is consulted + // only once a name has left more than one candidate standing. Selectors are + // consulted for a single candidate in one case only, when entries were dropped + // and being the last one standing is therefore not a fact about the index. + Selectors map[string]string + // ParseIdentity reads a name and version out of an artifact's config blob. It is + // what lets selection fall back when an index descriptor carries no annotations, + // and it is the only place an artifact's own format is known here. Without it, + // selection matches on descriptor annotations alone. + ParseIdentity func(config []byte) (name, version string, err error) } // GenericPullResult contains the result of a generic pull operation @@ -83,7 +111,332 @@ func NewGenericClient(client *Client) *GenericClient { } } -// PullGeneric performs a generic OCI pull without artifact-specific assumptions +// resolveFromIndex selects one manifest from an OCI Image Index by artifactType. +// When nothing matches on artifactType it retries over the entries that declare +// none, matching their config mediaType instead, which is how indexes written +// before artifactType existed are still resolvable. More than one match is +// narrowed by the selectors; a choice they cannot settle is an error rather than +// a guess. +func resolveFromIndex(ctx context.Context, fetcher content.Fetcher, indexDesc ocispec.Descriptor, artifactType string, selectors map[string]string, parse func(config []byte) (name, version string, err error)) (ocispec.Descriptor, error) { + // Fetch the index manifest + indexData, err := content.FetchAll(ctx, fetcher, indexDesc) + if err != nil { + return ocispec.Descriptor{}, fmt.Errorf("unable to fetch image index: %w", err) + } + + var index ocispec.Index + if err := json.Unmarshal(indexData, &index); err != nil { + return ocispec.Descriptor{}, fmt.Errorf("unable to parse image index: %w", err) + } + + // First pass: entries that declare the artifact type. A declared type is taken + // at face value even on a descriptor that also carries a platform, since tooling + // that stamps a platform on every index entry would otherwise hide the artifact. + var candidates []ocispec.Descriptor + var availableTypes []string + var undeclared []ocispec.Descriptor + for _, manifest := range index.Manifests { + switch { + case manifest.ArtifactType == artifactType: + candidates = append(candidates, manifest) + case manifest.ArtifactType != "": + availableTypes = append(availableTypes, manifest.ArtifactType) + default: + undeclared = append(undeclared, manifest) + } + } + + wantName := selectors[ocispec.AnnotationTitle] + wantVersion := selectors[ocispec.AnnotationVersion] + + // Second pass: entries that declare no type at all, matched on the config + // mediaType instead, which is how an index written before artifactType existed + // stays resolvable. It runs when the first pass found nothing, and also when + // no single entry it found announces the whole requested name and version: + // narrowing the candidates before either is checked is how a mixed index answers + // with the wrong chart. Entries carrying a platform are fetched here too, at one + // fetch per undeclared entry. That cost is not confined to failures: an index + // builder may set artifactType without copying the manifest annotations onto the + // descriptor, and then the declared entries announce nothing, so pulls that go on + // to succeed pay it as well. + configCache := map[string]ocispec.Descriptor{} + var skipped []error + if len(candidates) == 0 || !announcesChart(candidates, wantName, wantVersion) { + for _, candidate := range undeclared { + manifestData, err := content.FetchAll(ctx, fetcher, candidate) + if err != nil { + skipped = append(skipped, fmt.Errorf("%s: %w", candidate.Digest, err)) + continue + } + var manifest ocispec.Manifest + if err := json.Unmarshal(manifestData, &manifest); err != nil { + skipped = append(skipped, fmt.Errorf("%s: %w", candidate.Digest, err)) + continue + } + if manifest.Config.MediaType == artifactType { + candidates = append(candidates, candidate) + configCache[candidate.Digest.String()] = manifest.Config + } + } + } + + // An index may legally list the same manifest twice. Left in, the repeats would + // be counted as separate candidates and one chart would be called ambiguous with + // itself, so they are collapsed before anything downstream counts them. + candidates = dedupeByDigest(candidates) + + // Every answer below rests on nothing else in the index matching. Absence is a + // fact about the index only while every entry could be read; once entries have + // been dropped it is a fact about the pass instead. So an incomplete pass may + // not hand back a candidate that merely survived, and its negative answers have + // to admit what they could not see. Reading an identity can fail too, and those + // failures join the same record, which is why it is consulted where it is used + // rather than captured once. + + // A multi-arch image repeats one artifactType per platform; listing it once is + // the whole content of the message. + slices.Sort(availableTypes) + availableTypes = slices.Compact(availableTypes) + + switch len(candidates) { + case 0: + if len(skipped) > 0 { + return ocispec.Descriptor{}, fmt.Errorf( + "no manifest with artifactType %q found in image index; available types: %v; %d entries could not be read: %v", + artifactType, availableTypes, len(skipped), skipped) + } + return ocispec.Descriptor{}, fmt.Errorf( + "no manifest with artifactType %q found in image index; available types: %v; %d entries declared none", + artifactType, availableTypes, len(undeclared)) + case 1: + // One candidate is taken without checking the name against it. Requiring a + // match would break publishing a chart under a repository named differently + // from the chart, which is legal and common; there is also nothing to choose + // between, so a wrong name here means the reference itself was wrong. + // + // That reasoning holds only while the candidate set is complete. Once entries + // have been dropped, "nothing to choose between" describes what could be read + // rather than what the index holds, and the survivor has to earn the answer. + // With nothing skipped the set is complete and this is the only chart in the + // index, so it answers whatever name was asked for. Only an incomplete pass + // makes that reasoning unsound, and only then is the identity worth reading. + if len(skipped) > 0 && wantName != "" { + name, version, idErr := descriptorIdentity(ctx, fetcher, candidates[0], configCache, parse) + switch { + case idErr != nil: + return ocispec.Descriptor{}, fmt.Errorf( + "the identity of the only remaining chart in the image index could not be read: %w; %d other entries could not be read either: %v", + idErr, len(skipped), skipped) + case name != wantName || (wantVersion != "" && version != wantVersion): + return ocispec.Descriptor{}, fmt.Errorf( + "the only readable chart in the image index is %q version %q, and the reference asks for %q version %q; %d entries could not be read, so it cannot be told whether the requested one is among them: %v", + name, version, wantName, wantVersion, len(skipped), skipped) + } + } + return candidates[0], nil + } + + // More than one chart in the index: the name is required to disambiguate and + // the version breaks a remaining tie. Every error below lists the candidates, + // because the caller cannot see the index and has no other way to find out + // what it would have to pick between. + resolved := make([]chartCandidate, 0, len(candidates)) + for _, d := range candidates { + name, version, idErr := descriptorIdentity(ctx, fetcher, d, configCache, parse) + if idErr != nil { + // An identity that could not be read matches nothing below, so losing it + // silently would turn "this chart is not here" into a claim about the + // index that only describes the read. + skipped = append(skipped, idErr) + } + resolved = append(resolved, chartCandidate{desc: d, name: name, version: version, idErr: idErr}) + } + + if wantName == "" { + return ocispec.Descriptor{}, fmt.Errorf( + "image index holds %d charts and no chart name was given to disambiguate; candidates: %s", + len(resolved), describeCandidates(resolved)) + } + + var named []chartCandidate + for _, cand := range resolved { + if cand.name == wantName { + named = append(named, cand) + } + } + switch len(named) { + case 0: + if len(skipped) > 0 { + return ocispec.Descriptor{}, fmt.Errorf( + "no chart named %q among the %d found in the image index; %d entries could not be read: %v; candidates: %s", + wantName, len(resolved), len(skipped), skipped, describeCandidates(resolved)) + } + return ocispec.Descriptor{}, fmt.Errorf( + "image index holds %d charts and none is named %q; candidates: %s", + len(resolved), wantName, describeCandidates(resolved)) + case 1: + if len(skipped) > 0 && wantVersion != "" && named[0].version != wantVersion { + if named[0].idErr != nil { + return ocispec.Descriptor{}, fmt.Errorf( + "the version of the only chart named %q could not be read: %w; %d entries could not be read either: %v", + wantName, named[0].idErr, len(skipped), skipped) + } + return ocispec.Descriptor{}, fmt.Errorf( + "the only readable chart named %q is version %q, not %q; %d entries could not be read: %v", + wantName, named[0].version, wantVersion, len(skipped), skipped) + } + return named[0].desc, nil + } + + // Several charts share the requested name: break the tie by version. + if wantVersion != "" { + var versioned []chartCandidate + for _, cand := range named { + if cand.version == wantVersion { + versioned = append(versioned, cand) + } + } + switch len(versioned) { + case 1: + return versioned[0].desc, nil + case 0: + if len(skipped) > 0 { + return ocispec.Descriptor{}, fmt.Errorf( + "cannot choose between the %d charts named %q, and none is version %q; %d entries could not be read: %v; candidates: %s", + len(named), wantName, wantVersion, len(skipped), skipped, describeCandidates(named)) + } + return ocispec.Descriptor{}, fmt.Errorf( + "cannot choose between the %d charts named %q, and none is version %q; candidates: %s", + len(named), wantName, wantVersion, describeCandidates(named)) + default: + return ocispec.Descriptor{}, fmt.Errorf( + "image index holds %d entries for %s:%s; they can only be told apart by digest: %s", + len(versioned), wantName, wantVersion, describeDigests(versioned)) + } + } + + return ocispec.Descriptor{}, fmt.Errorf( + "image index is ambiguous: %d charts named %q; specify a version; candidates: %s", + len(named), wantName, describeCandidates(named)) +} + +// dedupeByDigest keeps the first entry for each digest, preserving index order. +func dedupeByDigest(descs []ocispec.Descriptor) []ocispec.Descriptor { + seen := make(map[string]bool, len(descs)) + out := make([]ocispec.Descriptor, 0, len(descs)) + for _, d := range descs { + key := d.Digest.String() + if seen[key] { + continue + } + seen[key] = true + out = append(out, d) + } + return out +} + +// describeDigests renders candidates as digests, for the one case where their +// name and version are identical and nothing else tells them apart. +func describeDigests(candidates []chartCandidate) string { + parts := make([]string, 0, len(candidates)) + for _, c := range candidates { + parts = append(parts, c.desc.Digest.String()) + } + return strings.Join(parts, ", ") +} + +// announcesChart reports whether any single descriptor in the index announces the +// whole requested identity. Asking per attribute instead would let one candidate supply +// the name and another the version, and the pair they form belongs to neither. +// It only reads what the index already states: an identity that is knowable but +// not announced answers false, which costs the second pass a fetch it did not +// strictly need and never costs a wrong answer. +func announcesChart(descs []ocispec.Descriptor, name, version string) bool { + return slices.ContainsFunc(descs, func(d ocispec.Descriptor) bool { + if name != "" && d.Annotations[ocispec.AnnotationTitle] != name { + return false + } + return version == "" || d.Annotations[ocispec.AnnotationVersion] == version + }) +} + +// chartCandidate pairs an index descriptor with the chart identity used to +// disambiguate it. +type chartCandidate struct { + desc ocispec.Descriptor + name string + version string + // idErr is set when the identity could not be read. Kept on the candidate so a + // message about it cannot claim a name or version that was never fetched. + idErr error +} + +// descriptorIdentity returns a candidate's name and version, preferring the +// descriptor annotations and asking parse to read whichever of the two they omit +// out of the config blob. Both are needed: a name alone cannot break a tie between +// two candidates sharing it. Without a parser only the annotations are available. +func descriptorIdentity(ctx context.Context, fetcher content.Fetcher, desc ocispec.Descriptor, configCache map[string]ocispec.Descriptor, parse func(config []byte) (name, version string, err error)) (name, version string, err error) { + name = desc.Annotations[ocispec.AnnotationTitle] + version = desc.Annotations[ocispec.AnnotationVersion] + if (name != "" && version != "") || parse == nil { + return name, version, nil + } + + // Resolve the config descriptor, reusing the one captured during the fallback + // pass when present so the manifest is not fetched a second time. + config, ok := configCache[desc.Digest.String()] + if !ok { + manifestData, ferr := content.FetchAll(ctx, fetcher, desc) + if ferr != nil { + return name, version, ferr + } + var manifest ocispec.Manifest + if uerr := json.Unmarshal(manifestData, &manifest); uerr != nil { + return name, version, fmt.Errorf("%s: %w", desc.Digest, uerr) + } + config = manifest.Config + } + configData, ferr := content.FetchAll(ctx, fetcher, config) + if ferr != nil { + return name, version, ferr + } + parsedName, parsedVersion, perr := parse(configData) + if perr != nil { + return name, version, fmt.Errorf("%s: %w", desc.Digest, perr) + } + if name == "" { + name = parsedName + } + if version == "" { + version = parsedVersion + } + return name, version, nil +} + +// describeCandidates renders chart candidates as name:version, falling back to +// the digest, for disambiguation error messages. A candidate whose identity could +// not be read is marked as such: rendering it like one that simply carries no +// version would state as absent what was never looked at. +func describeCandidates(candidates []chartCandidate) string { + parts := make([]string, 0, len(candidates)) + for _, c := range candidates { + switch { + case c.idErr != nil && c.name == "": + parts = append(parts, c.desc.Digest.String()+" (identity unreadable)") + case c.idErr != nil: + parts = append(parts, c.name+" (version unreadable)") + case c.name == "": + parts = append(parts, c.desc.Digest.String()) + case c.version == "": + parts = append(parts, c.name) + default: + parts = append(parts, c.name+":"+c.version) + } + } + return strings.Join(parts, ", ") +} + +// PullGeneric performs an OCI pull parameterised by artifact type. func (c *GenericClient) PullGeneric(ref string, options GenericPullOptions) (*GenericPullResult, error) { parsedRef, err := newReference(ref) if err != nil { @@ -112,7 +465,7 @@ func (c *GenericClient) PullGeneric(ref string, options GenericPullOptions) (*Ge } var mu sync.Mutex - manifest, err := oras.Copy(ctx, repository, parsedRef.String(), memoryStore, "", oras.CopyOptions{ + copyOptions := oras.CopyOptions{ CopyGraphOptions: oras.CopyGraphOptions{ PreCopy: func(ctx context.Context, desc ocispec.Descriptor) error { // Apply a custom PreCopy function if provided @@ -142,8 +495,28 @@ func (c *GenericClient) PullGeneric(ref string, options GenericPullOptions) (*Ge return nil }, }, - }) + } + + // Select inside the copy rather than before it. oras resolves the root itself + // and hands it to MapRoot through a proxy that still serves the body it already + // read, so an index is recognised and narrowed without a request of our own. + if options.ArtifactType != "" { + copyOptions.MapRoot = func(ctx context.Context, src content.ReadOnlyStorage, root ocispec.Descriptor) (ocispec.Descriptor, error) { + if root.MediaType != ocispec.MediaTypeImageIndex { + return root, nil + } + return resolveFromIndex(ctx, src, root, options.ArtifactType, options.Selectors, options.ParseIdentity) + } + } + + manifest, err := oras.Copy(ctx, repository, parsedRef.String(), memoryStore, "", copyOptions) if err != nil { + // oras wraps a MapRoot failure as its own copy error. The selection message + // underneath is the part naming something the caller can act on. + var copyErr *oras.CopyError + if errors.As(err, ©Err) && copyErr.Op == "MapRoot" { + return nil, copyErr.Err + } return nil, err } diff --git a/pkg/registry/index_test.go b/pkg/registry/index_test.go new file mode 100644 index 000000000..b23978400 --- /dev/null +++ b/pkg/registry/index_test.go @@ -0,0 +1,1177 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package registry + +import ( + "bytes" + "compress/gzip" + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + + "github.com/opencontainers/go-digest" + ocispec "github.com/opencontainers/image-spec/specs-go/v1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// gzipBytes returns payload as a gzip stream, standing in for a chart archive. +func gzipBytes(payload string) []byte { + var buf bytes.Buffer + zw := gzip.NewWriter(&buf) + _, _ = zw.Write([]byte(payload)) + _ = zw.Close() + return buf.Bytes() +} + +func TestPullFromImageIndex(t *testing.T) { + // Build chart config and layer with real digests + chartConfigData := []byte(`{"name":"testchart","version":"1.0.0","apiVersion":"v2"}`) + chartConfigDigest := digest.FromBytes(chartConfigData) + + // Minimal valid gzipped content + chartLayerData := []byte{0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0x03, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00} + chartLayerDigest := digest.FromBytes(chartLayerData) + + // Create chart manifest with real digests + chartManifest := ocispec.Manifest{ + MediaType: ocispec.MediaTypeImageManifest, + Config: ocispec.Descriptor{ + MediaType: ConfigMediaType, + Digest: chartConfigDigest, + Size: int64(len(chartConfigData)), + }, + Layers: []ocispec.Descriptor{ + { + MediaType: ChartLayerMediaType, + Digest: chartLayerDigest, + Size: int64(len(chartLayerData)), + }, + }, + } + chartManifestBytes, _ := json.Marshal(chartManifest) + chartManifestDigest := digest.FromBytes(chartManifestBytes) + + // Container manifest (we won't actually serve the blobs, just need valid structure) + containerManifestDigest := digest.Digest("sha256:ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff") + + // Image Index containing both chart and container manifests + imageIndex := ocispec.Index{ + MediaType: ocispec.MediaTypeImageIndex, + Manifests: []ocispec.Descriptor{ + { + MediaType: ocispec.MediaTypeImageManifest, + Digest: containerManifestDigest, + Size: 500, + ArtifactType: "application/vnd.oci.image.config.v1+json", + Platform: &ocispec.Platform{ + Architecture: "amd64", + OS: "linux", + }, + }, + { + MediaType: ocispec.MediaTypeImageManifest, + Digest: chartManifestDigest, + Size: int64(len(chartManifestBytes)), + ArtifactType: ChartArtifactType, + }, + }, + } + imageIndexBytes, _ := json.Marshal(imageIndex) + imageIndexDigest := digest.FromBytes(imageIndexBytes) + + // Create test server that serves the Image Index + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + path := r.URL.Path + switch { + case path == "/v2/": + w.WriteHeader(http.StatusOK) + + case path == "/v2/testrepo/multichart/manifests/1.0.0": + w.Header().Set("Content-Type", ocispec.MediaTypeImageIndex) + w.Header().Set("Docker-Content-Digest", imageIndexDigest.String()) + w.WriteHeader(http.StatusOK) + _, _ = w.Write(imageIndexBytes) + + // Serve Image Index by digest (for resolveFromIndex FetchAll) + case path == "/v2/testrepo/multichart/blobs/"+imageIndexDigest.String(), + path == "/v2/testrepo/multichart/manifests/"+imageIndexDigest.String(): + w.Header().Set("Content-Type", ocispec.MediaTypeImageIndex) + w.Header().Set("Docker-Content-Digest", imageIndexDigest.String()) + w.WriteHeader(http.StatusOK) + _, _ = w.Write(imageIndexBytes) + + // Serve chart manifest by digest + case path == "/v2/testrepo/multichart/manifests/"+chartManifestDigest.String(): + w.Header().Set("Content-Type", ocispec.MediaTypeImageManifest) + w.Header().Set("Docker-Content-Digest", chartManifestDigest.String()) + w.WriteHeader(http.StatusOK) + _, _ = w.Write(chartManifestBytes) + + // Serve chart config blob + case strings.Contains(path, chartConfigDigest.Encoded()): + w.Header().Set("Content-Type", ConfigMediaType) + w.Header().Set("Docker-Content-Digest", chartConfigDigest.String()) + w.WriteHeader(http.StatusOK) + _, _ = w.Write(chartConfigData) + + // Serve chart layer blob + case strings.Contains(path, chartLayerDigest.Encoded()): + w.Header().Set("Content-Type", ChartLayerMediaType) + w.Header().Set("Docker-Content-Digest", chartLayerDigest.String()) + w.WriteHeader(http.StatusOK) + _, _ = w.Write(chartLayerData) + + default: + t.Logf("404 for path: %s", path) + w.WriteHeader(http.StatusNotFound) + } + })) + defer s.Close() + + u, _ := url.Parse(s.URL) + host := "localhost:" + u.Port() + ref := host + "/testrepo/multichart:1.0.0" + + client, err := NewClient(ClientOptPlainHTTP()) + require.NoError(t, err) + + // Pull should automatically select the chart manifest from the index + result, err := client.Pull(ref) + require.NoError(t, err) + require.NotNil(t, result) + assert.Equal(t, chartManifestDigest.String(), result.Manifest.Digest) + assert.Equal(t, "testchart", result.Chart.Meta.Name) + assert.Equal(t, "1.0.0", result.Chart.Meta.Version) + assert.Equal(t, chartLayerData, result.Chart.Data) +} + +func TestPullFromImageIndexNoMatchingArtifactType(t *testing.T) { + // Image Index with only container images, no Helm chart + imageIndex := ocispec.Index{ + MediaType: ocispec.MediaTypeImageIndex, + Manifests: []ocispec.Descriptor{ + { + MediaType: ocispec.MediaTypeImageManifest, + Digest: "sha256:2222222222222222222222222222222222222222222222222222222222222222", + Size: 500, + ArtifactType: "application/vnd.oci.image.config.v1+json", + Platform: &ocispec.Platform{ + Architecture: "amd64", + OS: "linux", + }, + }, + { + MediaType: ocispec.MediaTypeImageManifest, + Digest: "sha256:3333333333333333333333333333333333333333333333333333333333333333", + Size: 500, + ArtifactType: "application/vnd.oci.image.config.v1+json", + Platform: &ocispec.Platform{ + Architecture: "arm64", + OS: "linux", + }, + }, + }, + } + imageIndexBytes, _ := json.Marshal(imageIndex) + imageIndexDigest := digest.FromBytes(imageIndexBytes) + + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + path := r.URL.Path + switch { + case path == "/v2/": + w.WriteHeader(http.StatusOK) + case path == "/v2/testrepo/nohelm/manifests/1.0.0": + w.Header().Set("Content-Type", ocispec.MediaTypeImageIndex) + w.Header().Set("Docker-Content-Digest", imageIndexDigest.String()) + w.WriteHeader(http.StatusOK) + _, _ = w.Write(imageIndexBytes) + // Serve Image Index by digest + case path == "/v2/testrepo/nohelm/blobs/"+imageIndexDigest.String(), + path == "/v2/testrepo/nohelm/manifests/"+imageIndexDigest.String(): + w.Header().Set("Content-Type", ocispec.MediaTypeImageIndex) + w.Header().Set("Docker-Content-Digest", imageIndexDigest.String()) + w.WriteHeader(http.StatusOK) + _, _ = w.Write(imageIndexBytes) + default: + w.WriteHeader(http.StatusNotFound) + } + })) + defer s.Close() + + u, _ := url.Parse(s.URL) + host := "localhost:" + u.Port() + ref := host + "/testrepo/nohelm:1.0.0" + + client, err := NewClient(ClientOptPlainHTTP()) + require.NoError(t, err) + + _, err = client.Pull(ref) + require.Error(t, err) + assert.Contains(t, err.Error(), "no manifest with artifactType") + assert.Contains(t, err.Error(), ChartArtifactType) + // The types that were there have to be named, otherwise the message tells the + // user what is missing without telling them what the index actually holds. + assert.Contains(t, err.Error(), "application/vnd.oci.image.config.v1+json") +} + +func TestPullSingleManifestNotIndex(t *testing.T) { + // Regular manifest (not an Index) should work as before + // Build config and layer with real digests + configData := []byte(`{"name":"singlechart","version":"1.0.0","apiVersion":"v2"}`) + configDigest := digest.FromBytes(configData) + + layerData := []byte{0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0x03, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00} + layerDigest := digest.FromBytes(layerData) + + manifest := ocispec.Manifest{ + MediaType: ocispec.MediaTypeImageManifest, + Config: ocispec.Descriptor{ + MediaType: ConfigMediaType, + Digest: configDigest, + Size: int64(len(configData)), + }, + Layers: []ocispec.Descriptor{ + { + MediaType: ChartLayerMediaType, + Digest: layerDigest, + Size: int64(len(layerData)), + }, + }, + } + manifestBytes, _ := json.Marshal(manifest) + manifestDigest := digest.FromBytes(manifestBytes) + + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + path := r.URL.Path + switch { + case path == "/v2/": + w.WriteHeader(http.StatusOK) + + case path == "/v2/testrepo/singlechart/manifests/1.0.0": + w.Header().Set("Content-Type", ocispec.MediaTypeImageManifest) + w.Header().Set("Docker-Content-Digest", manifestDigest.String()) + w.WriteHeader(http.StatusOK) + _, _ = w.Write(manifestBytes) + + case strings.Contains(path, configDigest.Encoded()): + w.Header().Set("Content-Type", ConfigMediaType) + w.Header().Set("Docker-Content-Digest", configDigest.String()) + w.WriteHeader(http.StatusOK) + _, _ = w.Write(configData) + + case strings.Contains(path, layerDigest.Encoded()): + w.Header().Set("Content-Type", ChartLayerMediaType) + w.Header().Set("Docker-Content-Digest", layerDigest.String()) + w.WriteHeader(http.StatusOK) + _, _ = w.Write(layerData) + + default: + w.WriteHeader(http.StatusNotFound) + } + })) + defer s.Close() + + u, _ := url.Parse(s.URL) + host := "localhost:" + u.Port() + ref := host + "/testrepo/singlechart:1.0.0" + + client, err := NewClient(ClientOptPlainHTTP()) + require.NoError(t, err) + + result, err := client.Pull(ref) + require.NoError(t, err) + assert.NotNil(t, result) + assert.Equal(t, "singlechart", result.Chart.Meta.Name) +} + +// testChart is a chart manifest plus its config and layer blobs, for building +// multi-chart Image Index test fixtures. +type testChart struct { + name, version string + configData []byte + layerData []byte + manifestBytes []byte + configDigest digest.Digest + layerDigest digest.Digest + manifestDigest digest.Digest +} + +// newTestChart builds a chart fixture. The optional seed distinguishes two charts +// that share a name and version, the way two pushes of one chart version differ by +// their creation annotation. +func newTestChart(name, version string, seed ...string) testChart { + configData := []byte(`{"name":"` + name + `","version":"` + version + `","apiVersion":"v2"}`) + // Distinct payload per chart, so tests can tell the charts apart by layer. + layerData := gzipBytes(name + "-" + version + strings.Join(seed, "")) + configDigest := digest.FromBytes(configData) + layerDigest := digest.FromBytes(layerData) + manifest := ocispec.Manifest{ + MediaType: ocispec.MediaTypeImageManifest, + Config: ocispec.Descriptor{MediaType: ConfigMediaType, Digest: configDigest, Size: int64(len(configData))}, + Layers: []ocispec.Descriptor{{MediaType: ChartLayerMediaType, Digest: layerDigest, Size: int64(len(layerData))}}, + } + manifestBytes, _ := json.Marshal(manifest) + return testChart{ + name: name, version: version, + configData: configData, layerData: layerData, manifestBytes: manifestBytes, + configDigest: configDigest, layerDigest: layerDigest, manifestDigest: digest.FromBytes(manifestBytes), + } +} + +// assertSelected checks that a pull returned the requested chart: its manifest, +// its metadata and its archive. All three are needed. The manifest digest pins +// that a manifest was selected at all, since a pull that copies the index graph +// reports the index digest here; metadata and archive are checked separately +// because they are read from different descriptors and can disagree. +func assertSelected(t *testing.T, want testChart, got *PullResult) { + t.Helper() + assert.Equal(t, want.manifestDigest.String(), got.Manifest.Digest) + assert.Equal(t, want.name, got.Chart.Meta.Name) + assert.Equal(t, want.version, got.Chart.Meta.Version) + assert.Equal(t, want.layerData, got.Chart.Data) +} + +func (c testChart) indexDescriptor() ocispec.Descriptor { + return ocispec.Descriptor{ + MediaType: ocispec.MediaTypeImageManifest, + Digest: c.manifestDigest, + Size: int64(len(c.manifestBytes)), + ArtifactType: ChartArtifactType, + Annotations: map[string]string{ + ocispec.AnnotationTitle: c.name, + ocispec.AnnotationVersion: c.version, + }, + } +} + +// platformStampedIndexDescriptor declares the chart artifact type and also carries +// a platform, the way tooling that stamps one on every index entry leaves it. +func (c testChart) platformStampedIndexDescriptor() ocispec.Descriptor { + desc := c.indexDescriptor() + desc.Platform = &ocispec.Platform{OS: "unknown", Architecture: "unknown"} + return desc +} + +// declaredWithoutAnnotations announces the artifact type but no identity, so a +// selector has to read the chart's own config to learn its name and version. +func (c testChart) declaredWithoutAnnotations() ocispec.Descriptor { + desc := c.indexDescriptor() + desc.Annotations = nil + return desc +} + +// titleOnlyIndexDescriptor names the chart but omits the version annotation, +// which is what tooling that copies only the title leaves behind. +func (c testChart) titleOnlyIndexDescriptor() ocispec.Descriptor { + desc := c.indexDescriptor() + delete(desc.Annotations, ocispec.AnnotationVersion) + return desc +} + +// legacyIndexDescriptor is an index entry without artifactType or annotations, +// as produced before this feature; selection must fall back to the chart config. +func (c testChart) legacyIndexDescriptor() ocispec.Descriptor { + return ocispec.Descriptor{ + MediaType: ocispec.MediaTypeImageManifest, + Digest: c.manifestDigest, + Size: int64(len(c.manifestBytes)), + } +} + +// serveMultiChartIndex serves an Image Index (by tag 1.0.0 and by digest) plus +// every chart's manifest, config and layer (matched by digest), so a pull can +// resolve and select from it. Matching is repo-prefix agnostic. +func serveMultiChartIndex(indexBytes []byte, indexDigest digest.Digest, charts ...testChart) *httptest.Server { + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + p := r.URL.Path + switch { + case p == "/v2/": + w.WriteHeader(http.StatusOK) + return + case strings.Contains(p, indexDigest.Encoded()), + strings.Contains(p, "/manifests/") && !strings.Contains(p, "sha256:"): + w.Header().Set("Content-Type", ocispec.MediaTypeImageIndex) + w.Header().Set("Docker-Content-Digest", indexDigest.String()) + w.WriteHeader(http.StatusOK) + _, _ = w.Write(indexBytes) + return + } + for _, c := range charts { + switch { + case strings.Contains(p, c.manifestDigest.Encoded()): + w.Header().Set("Content-Type", ocispec.MediaTypeImageManifest) + w.Header().Set("Docker-Content-Digest", c.manifestDigest.String()) + w.WriteHeader(http.StatusOK) + _, _ = w.Write(c.manifestBytes) + return + case strings.Contains(p, c.configDigest.Encoded()): + w.Header().Set("Content-Type", ConfigMediaType) + w.Header().Set("Docker-Content-Digest", c.configDigest.String()) + w.WriteHeader(http.StatusOK) + _, _ = w.Write(c.configData) + return + case strings.Contains(p, c.layerDigest.Encoded()): + w.Header().Set("Content-Type", ChartLayerMediaType) + w.Header().Set("Docker-Content-Digest", c.layerDigest.String()) + w.WriteHeader(http.StatusOK) + _, _ = w.Write(c.layerData) + return + } + } + w.WriteHeader(http.StatusNotFound) + })) +} + +func TestPullFromImageIndexSelectsByName(t *testing.T) { + alpha := newTestChart("alpha", "1.0.0") + beta := newTestChart("beta", "1.0.0") + index := ocispec.Index{ + MediaType: ocispec.MediaTypeImageIndex, + // Order alpha, beta on purpose: selection must be by name, not position. + Manifests: []ocispec.Descriptor{alpha.indexDescriptor(), beta.indexDescriptor()}, + } + indexBytes, _ := json.Marshal(index) + indexDigest := digest.FromBytes(indexBytes) + + s := serveMultiChartIndex(indexBytes, indexDigest, alpha, beta) + defer s.Close() + u, _ := url.Parse(s.URL) + host := "localhost:" + u.Port() + + client, err := NewClient(ClientOptPlainHTTP()) + require.NoError(t, err) + + // Pulling the "beta" reference must return beta even though alpha is first. + result, err := client.Pull(host + "/testrepo/beta:1.0.0") + require.NoError(t, err) + assertSelected(t, beta, result) + + // And the "alpha" reference must return alpha from the same index. + result, err = client.Pull(host + "/testrepo/alpha:1.0.0") + require.NoError(t, err) + assertSelected(t, alpha, result) +} + +func TestPullFromImageIndexNoChartWithRequestedName(t *testing.T) { + alpha := newTestChart("alpha", "1.0.0") + beta := newTestChart("beta", "1.0.0") + index := ocispec.Index{ + MediaType: ocispec.MediaTypeImageIndex, + Manifests: []ocispec.Descriptor{alpha.indexDescriptor(), beta.indexDescriptor()}, + } + indexBytes, _ := json.Marshal(index) + indexDigest := digest.FromBytes(indexBytes) + + s := serveMultiChartIndex(indexBytes, indexDigest, alpha, beta) + defer s.Close() + u, _ := url.Parse(s.URL) + host := "localhost:" + u.Port() + + client, err := NewClient(ClientOptPlainHTTP()) + require.NoError(t, err) + + // The "gamma" reference matches neither chart: the wrong chart must not be + // returned silently; the error lists the available candidates. + _, err = client.Pull(host + "/testrepo/gamma:1.0.0") + require.Error(t, err) + assert.Contains(t, err.Error(), "none is named") + assert.Contains(t, err.Error(), "alpha") + assert.Contains(t, err.Error(), "beta") +} + +func TestPullFromImageIndexSelectsPlatformStampedChart(t *testing.T) { + alpha := newTestChart("alpha", "1.0.0") + index := ocispec.Index{ + MediaType: ocispec.MediaTypeImageIndex, + // The entry declares the chart artifact type and a platform at once. The + // declared type decides; the platform must not hide the chart. + Manifests: []ocispec.Descriptor{alpha.platformStampedIndexDescriptor()}, + } + indexBytes, _ := json.Marshal(index) + indexDigest := digest.FromBytes(indexBytes) + + s := serveMultiChartIndex(indexBytes, indexDigest, alpha) + defer s.Close() + u, _ := url.Parse(s.URL) + host := "localhost:" + u.Port() + + client, err := NewClient(ClientOptPlainHTTP()) + require.NoError(t, err) + + result, err := client.Pull(host + "/testrepo/alpha:1.0.0") + require.NoError(t, err) + assertSelected(t, alpha, result) +} + +func TestPullFromImageIndexRequestedVersionNotInIndex(t *testing.T) { + v1 := newTestChart("app", "1.0.0") + v2 := newTestChart("app", "2.0.0") + index := ocispec.Index{ + MediaType: ocispec.MediaTypeImageIndex, + Manifests: []ocispec.Descriptor{v1.indexDescriptor(), v2.indexDescriptor()}, + } + indexBytes, _ := json.Marshal(index) + indexDigest := digest.FromBytes(indexBytes) + + s := serveMultiChartIndex(indexBytes, indexDigest, v1, v2) + defer s.Close() + u, _ := url.Parse(s.URL) + host := "localhost:" + u.Port() + + client, err := NewClient(ClientOptPlainHTTP()) + require.NoError(t, err) + + // The name matches two charts and the requested version matches neither, so + // the error has to say the version is missing, not ask for one. + _, err = client.Pull(host + "/testrepo/app:3.0.0") + require.Error(t, err) + assert.Contains(t, err.Error(), `none is version "3.0.0"`) + assert.Contains(t, err.Error(), "app:1.0.0") + assert.Contains(t, err.Error(), "app:2.0.0") +} + +func TestPullFromImageIndexLegacyPlatformStampedChart(t *testing.T) { + alpha := newTestChart("alpha", "1.0.0") + desc := alpha.legacyIndexDescriptor() + desc.Platform = &ocispec.Platform{OS: "unknown", Architecture: "unknown"} + index := ocispec.Index{ + MediaType: ocispec.MediaTypeImageIndex, + // Neither an artifact type nor annotations, and a platform on top. The + // config mediaType is the only thing left that can identify the chart. + Manifests: []ocispec.Descriptor{desc}, + } + indexBytes, _ := json.Marshal(index) + indexDigest := digest.FromBytes(indexBytes) + + s := serveMultiChartIndex(indexBytes, indexDigest, alpha) + defer s.Close() + u, _ := url.Parse(s.URL) + host := "localhost:" + u.Port() + + client, err := NewClient(ClientOptPlainHTTP()) + require.NoError(t, err) + + result, err := client.Pull(host + "/testrepo/alpha:1.0.0") + require.NoError(t, err) + assertSelected(t, alpha, result) +} + +func TestPullFromImageIndexMixedArtifactTypeSelectsRequestedChart(t *testing.T) { + alpha := newTestChart("alpha", "1.0.0") + beta := newTestChart("beta", "1.0.0") + index := ocispec.Index{ + MediaType: ocispec.MediaTypeImageIndex, + // Mixed index: beta declares the artifact type, alpha predates it. Matching + // on artifactType alone leaves exactly one candidate, and returning that one + // unchecked would answer a request for alpha with beta. + Manifests: []ocispec.Descriptor{beta.indexDescriptor(), alpha.legacyIndexDescriptor()}, + } + indexBytes, _ := json.Marshal(index) + indexDigest := digest.FromBytes(indexBytes) + + s := serveMultiChartIndex(indexBytes, indexDigest, alpha, beta) + defer s.Close() + u, _ := url.Parse(s.URL) + host := "localhost:" + u.Port() + + client, err := NewClient(ClientOptPlainHTTP()) + require.NoError(t, err) + + result, err := client.Pull(host + "/testrepo/alpha:1.0.0") + require.NoError(t, err) + assertSelected(t, alpha, result) + + // The declared side of the same index still resolves. + result, err = client.Pull(host + "/testrepo/beta:1.0.0") + require.NoError(t, err) + assertSelected(t, beta, result) +} + +func TestPullFromImageIndexMixedArtifactTypeSelectsRequestedVersion(t *testing.T) { + v1 := newTestChart("app", "1.0.0") + v2 := newTestChart("app", "2.0.0") + index := ocispec.Index{ + MediaType: ocispec.MediaTypeImageIndex, + // Same name on both sides of the mixed index, so only the version tells them + // apart, and the declared one is not the version being asked for. + Manifests: []ocispec.Descriptor{v1.indexDescriptor(), v2.legacyIndexDescriptor()}, + } + indexBytes, _ := json.Marshal(index) + indexDigest := digest.FromBytes(indexBytes) + + s := serveMultiChartIndex(indexBytes, indexDigest, v1, v2) + defer s.Close() + u, _ := url.Parse(s.URL) + host := "localhost:" + u.Port() + + client, err := NewClient(ClientOptPlainHTTP()) + require.NoError(t, err) + + result, err := client.Pull(host + "/testrepo/app:2.0.0") + require.NoError(t, err) + assertSelected(t, v2, result) + + result, err = client.Pull(host + "/testrepo/app:1.0.0") + require.NoError(t, err) + assertSelected(t, v1, result) +} + +func TestPullFromImageIndexSplitIdentityDoesNotSuppressFallback(t *testing.T) { + appV1 := newTestChart("app", "1.0.0") + otherV2 := newTestChart("other", "2.0.0") + appV2 := newTestChart("app", "2.0.0") + index := ocispec.Index{ + MediaType: ocispec.MediaTypeImageIndex, + // Between them the declared entries announce the requested name and the + // requested version, but neither announces both. The chart that does is the + // undeclared one, so checking the two attributes separately would skip the + // fallback pass and answer with app 1.0.0. + Manifests: []ocispec.Descriptor{ + appV1.indexDescriptor(), + otherV2.indexDescriptor(), + appV2.legacyIndexDescriptor(), + }, + } + indexBytes, _ := json.Marshal(index) + indexDigest := digest.FromBytes(indexBytes) + + s := serveMultiChartIndex(indexBytes, indexDigest, appV1, otherV2, appV2) + defer s.Close() + u, _ := url.Parse(s.URL) + host := "localhost:" + u.Port() + + client, err := NewClient(ClientOptPlainHTTP()) + require.NoError(t, err) + + result, err := client.Pull(host + "/testrepo/app:2.0.0") + require.NoError(t, err) + assertSelected(t, appV2, result) +} + +func TestPullFromImageIndexUnreadableEntryDoesNotYieldAnotherChart(t *testing.T) { + alpha := newTestChart("alpha", "1.0.0") + beta := newTestChart("beta", "1.0.0") + index := ocispec.Index{ + MediaType: ocispec.MediaTypeImageIndex, + Manifests: []ocispec.Descriptor{beta.indexDescriptor(), alpha.legacyIndexDescriptor()}, + } + indexBytes, _ := json.Marshal(index) + indexDigest := digest.FromBytes(indexBytes) + + // beta is readable and declares the artifact type; alpha, the chart actually + // requested, cannot be fetched. One survivor must not become the answer. + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + p := r.URL.Path + switch { + case p == "/v2/": + w.WriteHeader(http.StatusOK) + case strings.Contains(p, alpha.manifestDigest.Encoded()): + // 404 rather than 500: both land in the skipped list, and this one does + // not spend the test on the client's retry backoff. + w.WriteHeader(http.StatusNotFound) + case strings.Contains(p, indexDigest.Encoded()), + strings.Contains(p, "/manifests/") && !strings.Contains(p, "sha256:"): + w.Header().Set("Content-Type", ocispec.MediaTypeImageIndex) + w.Header().Set("Docker-Content-Digest", indexDigest.String()) + w.WriteHeader(http.StatusOK) + _, _ = w.Write(indexBytes) + case strings.Contains(p, beta.manifestDigest.Encoded()): + w.Header().Set("Content-Type", ocispec.MediaTypeImageManifest) + w.Header().Set("Docker-Content-Digest", beta.manifestDigest.String()) + w.WriteHeader(http.StatusOK) + _, _ = w.Write(beta.manifestBytes) + case strings.Contains(p, beta.configDigest.Encoded()): + w.Header().Set("Content-Type", ConfigMediaType) + w.WriteHeader(http.StatusOK) + _, _ = w.Write(beta.configData) + default: + w.WriteHeader(http.StatusNotFound) + } + })) + defer s.Close() + u, _ := url.Parse(s.URL) + host := "localhost:" + u.Port() + + client, err := NewClient(ClientOptPlainHTTP()) + require.NoError(t, err) + + _, err = client.Pull(host + "/testrepo/alpha:1.0.0") + require.Error(t, err) + assert.Contains(t, err.Error(), "could not be read") + assert.NotContains(t, err.Error(), "no manifest with artifactType") +} + +func TestPullFromImageIndexUnreadableEntryDoesNotYieldAnotherVersion(t *testing.T) { + v1 := newTestChart("app", "1.0.0") + v2 := newTestChart("app", "2.0.0") + other := newTestChart("other", "9.9.9") + index := ocispec.Index{ + MediaType: ocispec.MediaTypeImageIndex, + // Two readable charts keep the count above one, so selection goes through the + // name filter; the version actually requested lives in the entry that cannot + // be read. The survivor named "app" must not be handed back for it. + Manifests: []ocispec.Descriptor{ + v1.indexDescriptor(), other.indexDescriptor(), v2.legacyIndexDescriptor(), + }, + } + indexBytes, _ := json.Marshal(index) + indexDigest := digest.FromBytes(indexBytes) + + base := serveMultiChartIndex(indexBytes, indexDigest, v1, other, v2) + defer base.Close() + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.Contains(r.URL.Path, v2.manifestDigest.Encoded()) { + w.WriteHeader(http.StatusNotFound) + return + } + base.Config.Handler.ServeHTTP(w, r) + })) + defer s.Close() + u, _ := url.Parse(s.URL) + host := "localhost:" + u.Port() + + client, err := NewClient(ClientOptPlainHTTP()) + require.NoError(t, err) + + _, err = client.Pull(host + "/testrepo/app:2.0.0") + require.Error(t, err) + assert.Contains(t, err.Error(), "could not be read") + // Naming both versions is the point: the reader has to see that 1.0.0 was found + // and 2.0.0 was asked for, rather than receive 1.0.0 as though it were the answer. + assert.Contains(t, err.Error(), `is version "1.0.0", not "2.0.0"`) +} + +func TestPullFromImageIndexUnreadableIdentityIsNotAnAbsentChart(t *testing.T) { + alpha := newTestChart("alpha", "1.0.0") + beta := newTestChart("beta", "1.0.0") + index := ocispec.Index{ + MediaType: ocispec.MediaTypeImageIndex, + // Both declare the artifact type, so both are candidates without any fallback. + // alpha carries no annotations, so learning that it is alpha needs a fetch, + // and that fetch is the one the registry refuses. + Manifests: []ocispec.Descriptor{alpha.declaredWithoutAnnotations(), beta.indexDescriptor()}, + } + indexBytes, _ := json.Marshal(index) + indexDigest := digest.FromBytes(indexBytes) + + base := serveMultiChartIndex(indexBytes, indexDigest, alpha, beta) + defer base.Close() + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.Contains(r.URL.Path, alpha.manifestDigest.Encoded()) { + w.WriteHeader(http.StatusNotFound) + return + } + base.Config.Handler.ServeHTTP(w, r) + })) + defer s.Close() + u, _ := url.Parse(s.URL) + host := "localhost:" + u.Port() + + client, err := NewClient(ClientOptPlainHTTP()) + require.NoError(t, err) + + // "no chart named alpha here" would be a claim about the index. What actually + // happened is that alpha's identity could not be read, and the message has to + // say so rather than report an absence it never established. + _, err = client.Pull(host + "/testrepo/alpha:1.0.0") + require.Error(t, err) + assert.Contains(t, err.Error(), "could not be read") +} + +func TestPullFromImageIndexOneNameMatchIgnoresRequestedVersion(t *testing.T) { + app := newTestChart("app", "1.0.0") + other := newTestChart("other", "1.0.0") + index := ocispec.Index{ + MediaType: ocispec.MediaTypeImageIndex, + Manifests: []ocispec.Descriptor{app.indexDescriptor(), other.indexDescriptor()}, + } + indexBytes, _ := json.Marshal(index) + indexDigest := digest.FromBytes(indexBytes) + + s := serveMultiChartIndex(indexBytes, indexDigest, app, other) + defer s.Close() + u, _ := url.Parse(s.URL) + host := "localhost:" + u.Port() + + client, err := NewClient(ClientOptPlainHTTP()) + require.NoError(t, err) + + // Only one chart is named "app", so there is no choice for the version to make + // and it is not used to reject the answer. This matches a reference to a plain + // manifest, where the tag decides and the chart's own version is not checked. + result, err := client.Pull(host + "/testrepo/app:2.0.0") + require.NoError(t, err) + assertSelected(t, app, result) +} + +func TestPullFromImageIndexUnreadableSoleIdentityIsNotDescribed(t *testing.T) { + alpha := newTestChart("alpha", "1.0.0") + beta := newTestChart("beta", "1.0.0") + index := ocispec.Index{ + MediaType: ocispec.MediaTypeImageIndex, + // beta declares no type, so the fallback runs and fails on it, leaving alpha + // as the sole candidate — and alpha's own identity cannot be read either. + Manifests: []ocispec.Descriptor{alpha.declaredWithoutAnnotations(), beta.legacyIndexDescriptor()}, + } + indexBytes, _ := json.Marshal(index) + indexDigest := digest.FromBytes(indexBytes) + + base := serveMultiChartIndex(indexBytes, indexDigest, alpha, beta) + defer base.Close() + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + p := r.URL.Path + if strings.Contains(p, alpha.manifestDigest.Encoded()) || strings.Contains(p, beta.manifestDigest.Encoded()) { + w.WriteHeader(http.StatusNotFound) + return + } + base.Config.Handler.ServeHTTP(w, r) + })) + defer s.Close() + u, _ := url.Parse(s.URL) + host := "localhost:" + u.Port() + + client, err := NewClient(ClientOptPlainHTTP()) + require.NoError(t, err) + + _, err = client.Pull(host + "/testrepo/alpha:1.0.0") + require.Error(t, err) + // The entry whose read failed must not be reported as a chart that was read. + assert.Contains(t, err.Error(), "could not be read") + assert.NotContains(t, err.Error(), `is "" version ""`) +} + +func TestPullFromImageIndexUnreadableNamedVersionIsNotDescribed(t *testing.T) { + alpha := newTestChart("alpha", "1.0.0") + beta := newTestChart("beta", "1.0.0") + titleOnly := alpha.indexDescriptor() + delete(titleOnly.Annotations, ocispec.AnnotationVersion) + index := ocispec.Index{ + MediaType: ocispec.MediaTypeImageIndex, + // alpha announces its name but not its version, so the version has to be + // fetched, and that fetch is the one that fails. beta keeps the candidate + // count above one so selection reaches the name filter. + Manifests: []ocispec.Descriptor{titleOnly, beta.indexDescriptor()}, + } + indexBytes, _ := json.Marshal(index) + indexDigest := digest.FromBytes(indexBytes) + + base := serveMultiChartIndex(indexBytes, indexDigest, alpha, beta) + defer base.Close() + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.Contains(r.URL.Path, alpha.manifestDigest.Encoded()) { + w.WriteHeader(http.StatusNotFound) + return + } + base.Config.Handler.ServeHTTP(w, r) + })) + defer s.Close() + u, _ := url.Parse(s.URL) + host := "localhost:" + u.Port() + + client, err := NewClient(ClientOptPlainHTTP()) + require.NoError(t, err) + + _, err = client.Pull(host + "/testrepo/alpha:2.0.0") + require.Error(t, err) + assert.Contains(t, err.Error(), "could not be read") + // Calling it readable and naming its version are both claims about an entry + // whose read is what failed. + assert.NotContains(t, err.Error(), "the only readable chart") + assert.NotContains(t, err.Error(), `is version ""`) +} + +func TestPullFromImageIndexRepeatedEntry(t *testing.T) { + alpha := newTestChart("alpha", "1.0.0") + desc := alpha.indexDescriptor() + index := ocispec.Index{ + MediaType: ocispec.MediaTypeImageIndex, + // The same manifest listed twice is one chart, not a choice between two. + Manifests: []ocispec.Descriptor{desc, desc}, + } + indexBytes, _ := json.Marshal(index) + indexDigest := digest.FromBytes(indexBytes) + + s := serveMultiChartIndex(indexBytes, indexDigest, alpha) + defer s.Close() + u, _ := url.Parse(s.URL) + host := "localhost:" + u.Port() + + client, err := NewClient(ClientOptPlainHTTP()) + require.NoError(t, err) + + result, err := client.Pull(host + "/testrepo/alpha:1.0.0") + require.NoError(t, err) + assertSelected(t, alpha, result) +} + +func TestPullFromImageIndexTwoChartsSameNameAndVersion(t *testing.T) { + // Re-packaging a chart gives the archive a new modification time, which push + // records as the creation annotation, so two builds of one chart version are + // distinct manifests with identical identities. + first := newTestChart("app", "1.0.0", "first") + second := newTestChart("app", "1.0.0", "second") + require.NotEqual(t, first.manifestDigest, second.manifestDigest) + + index := ocispec.Index{ + MediaType: ocispec.MediaTypeImageIndex, + Manifests: []ocispec.Descriptor{first.indexDescriptor(), second.indexDescriptor()}, + } + indexBytes, _ := json.Marshal(index) + indexDigest := digest.FromBytes(indexBytes) + + s := serveMultiChartIndex(indexBytes, indexDigest, first, second) + defer s.Close() + u, _ := url.Parse(s.URL) + host := "localhost:" + u.Port() + + client, err := NewClient(ClientOptPlainHTTP()) + require.NoError(t, err) + + // Name and version cannot separate them, so the error has to fall back to what + // can: the digests, which here genuinely differ. + _, err = client.Pull(host + "/testrepo/app:1.0.0") + require.Error(t, err) + assert.Contains(t, err.Error(), "told apart by digest") + assert.Contains(t, err.Error(), first.manifestDigest.String()) + assert.Contains(t, err.Error(), second.manifestDigest.String()) +} + +func TestPullGenericWithoutSelectors(t *testing.T) { + alpha := newTestChart("alpha", "1.0.0") + beta := newTestChart("beta", "1.0.0") + index := ocispec.Index{ + MediaType: ocispec.MediaTypeImageIndex, + Manifests: []ocispec.Descriptor{alpha.indexDescriptor(), beta.indexDescriptor()}, + } + indexBytes, _ := json.Marshal(index) + indexDigest := digest.FromBytes(indexBytes) + + s := serveMultiChartIndex(indexBytes, indexDigest, alpha, beta) + defer s.Close() + u, _ := url.Parse(s.URL) + host := "localhost:" + u.Port() + + client, err := NewClient(ClientOptPlainHTTP()) + require.NoError(t, err) + + // PullGeneric is exported and a caller may pass no selectors at all. Two charts + // and nothing to choose between them has to be reported, not guessed. + _, err = client.Generic().PullGeneric(host+"/testrepo/anything:1.0.0", GenericPullOptions{ + AllowedMediaTypes: []string{ocispec.MediaTypeImageIndex, ocispec.MediaTypeImageManifest, ConfigMediaType, ChartLayerMediaType}, + ArtifactType: ChartArtifactType, + }) + require.Error(t, err) + assert.Contains(t, err.Error(), "no chart name was given to disambiguate") +} + +func TestPullGenericWithoutIdentityParser(t *testing.T) { + alpha := newTestChart("alpha", "1.0.0") + beta := newTestChart("beta", "1.0.0") + index := ocispec.Index{ + MediaType: ocispec.MediaTypeImageIndex, + // Neither entry announces an identity, so without a parser there is nothing + // left to match on and the requested name cannot be found. + Manifests: []ocispec.Descriptor{alpha.declaredWithoutAnnotations(), beta.declaredWithoutAnnotations()}, + } + indexBytes, _ := json.Marshal(index) + indexDigest := digest.FromBytes(indexBytes) + + s := serveMultiChartIndex(indexBytes, indexDigest, alpha, beta) + defer s.Close() + u, _ := url.Parse(s.URL) + host := "localhost:" + u.Port() + + client, err := NewClient(ClientOptPlainHTTP()) + require.NoError(t, err) + + // PullGeneric is exported and ParseIdentity is optional: a caller that omits it + // gets annotation-only matching, not a panic and not a config read. + _, err = client.Generic().PullGeneric(host+"/testrepo/alpha:1.0.0", GenericPullOptions{ + AllowedMediaTypes: []string{ocispec.MediaTypeImageManifest, ConfigMediaType, ChartLayerMediaType}, + ArtifactType: ChartArtifactType, + Selectors: map[string]string{ocispec.AnnotationTitle: "alpha"}, + }) + require.Error(t, err) + assert.Contains(t, err.Error(), `none is named "alpha"`) +} + +func TestDescribeCandidates(t *testing.T) { + d := func(s string) ocispec.Descriptor { return ocispec.Descriptor{Digest: digest.Digest(s)} } + errFixture := errors.New("not found") + sha := "sha256:0000000000000000000000000000000000000000000000000000000000000000" + for _, tc := range []struct { + name string + in []chartCandidate + want string + }{ + {"name and version", []chartCandidate{{desc: d(sha), name: "app", version: "1.0.0"}}, "app:1.0.0"}, + {"name only", []chartCandidate{{desc: d(sha), name: "app"}}, "app"}, + {"no identity read at all", []chartCandidate{{desc: d(sha)}}, sha}, + {"identity read failed", []chartCandidate{{desc: d(sha), idErr: errFixture}}, sha + " (identity unreadable)"}, + {"version read failed", []chartCandidate{{desc: d(sha), name: "app", idErr: errFixture}}, "app (version unreadable)"}, + } { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.want, describeCandidates(tc.in)) + }) + } +} + +func TestPullFromImageIndexAmbiguousWithoutVersion(t *testing.T) { + v1 := newTestChart("app", "1.0.0") + v2 := newTestChart("app", "2.0.0") + index := ocispec.Index{ + MediaType: ocispec.MediaTypeImageIndex, + Manifests: []ocispec.Descriptor{v1.indexDescriptor(), v2.indexDescriptor()}, + } + indexBytes, _ := json.Marshal(index) + indexDigest := digest.FromBytes(indexBytes) + + s := serveMultiChartIndex(indexBytes, indexDigest, v1, v2) + defer s.Close() + u, _ := url.Parse(s.URL) + host := "localhost:" + u.Port() + + client, err := NewClient(ClientOptPlainHTTP()) + require.NoError(t, err) + + // A digest-pinned reference carries no tag, so no version reaches selection and + // two same-named charts cannot be told apart. That has to be said, not guessed. + _, err = client.Pull(host + "/testrepo/app@" + indexDigest.String()) + require.Error(t, err) + assert.Contains(t, err.Error(), "specify a version") + assert.Contains(t, err.Error(), "app:1.0.0") + assert.Contains(t, err.Error(), "app:2.0.0") +} + +func TestPullFromImageIndexReportsUnreadableEntries(t *testing.T) { + alpha := newTestChart("alpha", "1.0.0") + index := ocispec.Index{ + MediaType: ocispec.MediaTypeImageIndex, + Manifests: []ocispec.Descriptor{alpha.legacyIndexDescriptor()}, + } + indexBytes, _ := json.Marshal(index) + indexDigest := digest.FromBytes(indexBytes) + + // Serve the index but not the manifest it points at, so the only candidate the + // fallback pass has cannot be read. + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + p := r.URL.Path + switch { + case p == "/v2/": + w.WriteHeader(http.StatusOK) + case strings.Contains(p, indexDigest.Encoded()), + strings.Contains(p, "/manifests/") && !strings.Contains(p, "sha256:"): + w.Header().Set("Content-Type", ocispec.MediaTypeImageIndex) + w.Header().Set("Docker-Content-Digest", indexDigest.String()) + w.WriteHeader(http.StatusOK) + _, _ = w.Write(indexBytes) + default: + w.WriteHeader(http.StatusNotFound) + } + })) + defer s.Close() + u, _ := url.Parse(s.URL) + host := "localhost:" + u.Port() + + client, err := NewClient(ClientOptPlainHTTP()) + require.NoError(t, err) + + _, err = client.Pull(host + "/testrepo/alpha:1.0.0") + require.Error(t, err) + // "found nothing" and "could not read what was there" are different answers. + assert.Contains(t, err.Error(), "could not be read") + assert.Contains(t, err.Error(), alpha.manifestDigest.String()) +} + +func TestPullFromImageIndexSelectsByVersion(t *testing.T) { + v1 := newTestChart("app", "1.0.0") + v2 := newTestChart("app", "2.0.0") + index := ocispec.Index{ + MediaType: ocispec.MediaTypeImageIndex, + // Same name, different versions: the requested version breaks the tie. + Manifests: []ocispec.Descriptor{v1.indexDescriptor(), v2.indexDescriptor()}, + } + indexBytes, _ := json.Marshal(index) + indexDigest := digest.FromBytes(indexBytes) + + s := serveMultiChartIndex(indexBytes, indexDigest, v1, v2) + defer s.Close() + u, _ := url.Parse(s.URL) + host := "localhost:" + u.Port() + + client, err := NewClient(ClientOptPlainHTTP()) + require.NoError(t, err) + + result, err := client.Pull(host + "/testrepo/app:2.0.0") + require.NoError(t, err) + assertSelected(t, v2, result) +} + +func TestPullFromImageIndexSelectsByVersionWithoutVersionAnnotation(t *testing.T) { + v1 := newTestChart("app", "1.0.0") + v2 := newTestChart("app", "2.0.0") + index := ocispec.Index{ + MediaType: ocispec.MediaTypeImageIndex, + // Both entries name the chart and neither carries a version annotation, + // so the tie can only be broken on the version in each chart's config. + Manifests: []ocispec.Descriptor{v1.titleOnlyIndexDescriptor(), v2.titleOnlyIndexDescriptor()}, + } + indexBytes, _ := json.Marshal(index) + indexDigest := digest.FromBytes(indexBytes) + + s := serveMultiChartIndex(indexBytes, indexDigest, v1, v2) + defer s.Close() + u, _ := url.Parse(s.URL) + host := "localhost:" + u.Port() + + client, err := NewClient(ClientOptPlainHTTP()) + require.NoError(t, err) + + result, err := client.Pull(host + "/testrepo/app:2.0.0") + require.NoError(t, err) + assertSelected(t, v2, result) +} + +func TestPullFromImageIndexLegacySelectsByName(t *testing.T) { + // Legacy index: entries carry neither artifactType nor title annotations, so + // selection falls back to config.mediaType and the chart's own Chart.yaml. + alpha := newTestChart("alpha", "1.0.0") + beta := newTestChart("beta", "1.0.0") + index := ocispec.Index{ + MediaType: ocispec.MediaTypeImageIndex, + Manifests: []ocispec.Descriptor{alpha.legacyIndexDescriptor(), beta.legacyIndexDescriptor()}, + } + indexBytes, _ := json.Marshal(index) + indexDigest := digest.FromBytes(indexBytes) + + s := serveMultiChartIndex(indexBytes, indexDigest, alpha, beta) + defer s.Close() + u, _ := url.Parse(s.URL) + host := "localhost:" + u.Port() + + client, err := NewClient(ClientOptPlainHTTP()) + require.NoError(t, err) + + result, err := client.Pull(host + "/testrepo/beta:1.0.0") + require.NoError(t, err) + assertSelected(t, beta, result) +} From c24ba33a60dcb1ecece29fb50d80048b83b76e7f Mon Sep 17 00:00:00 2001 From: Aleksei Sviridkin Date: Fri, 14 Aug 2026 23:12:07 +0300 Subject: [PATCH 2/2] fix(registry): read what an index entry holds, not what it declares A descriptor's declaration decides nothing on its own. An entry may list manifests rather than hold an artifact, which a multi-arch image beside a chart does, and it may declare an artifact type that is not the chart's, which a builder setting the field wrongly leaves behind. Ruling on either from the descriptor loses charts that are there: the list is never searched, or the entry is dropped before its config is read, and in both cases the chart beside it becomes the only one in the index and answers in its place. Search what an entry lists, the same way the top index is searched, whether it carries the OCI index media type or the Docker manifest list one, and read the config of every entry whose declaration did not match, not only of those that declared nothing. A list is still never a candidate: the artifact is identified by the config blob its manifest carries and a list has none, so taking one would root the copy at a manifest the filter drops. Both readings happen under the condition that already governs the second pass, so an index whose declared entries answer the reference is resolved without either. Reading an entry means reading what a registry serves for it, so the reading is bounded twice over. An entry declaring more bytes than a manifest is goes unread, and so does one whose body holds neither the config nor the list of manifests its media type promised, since a body that parses as both shapes proves nothing about either. The search itself stops three lists down, counting from the one the reference resolves to, against a chain a registry is free to extend while it is walked. None of those three is dropped quietly. Each is recorded the way an entry that cannot be fetched is, because every answer resting on nothing else in the index matching has to know the difference between an index that holds one chart and a pass that read one. A manifest the copy will not store cannot be the answer either. A chart written to the Docker schema, which is what a registry-to-registry copy leaves behind, is copied like the OCI one it describes, and a selection that lands on anything else the pull does not accept says so and names the entry, rather than leaving a copy that quietly stores nothing and a message about descriptors that never arrived. Signed-off-by: Aleksei Sviridkin Assisted-By: Claude --- pkg/registry/client.go | 5 + pkg/registry/generic.go | 200 ++++++++++-- pkg/registry/index_test.go | 632 ++++++++++++++++++++++++++++++++++++- 3 files changed, 808 insertions(+), 29 deletions(-) diff --git a/pkg/registry/client.go b/pkg/registry/client.go index 009fc1b47..5d47e78f8 100644 --- a/pkg/registry/client.go +++ b/pkg/registry/client.go @@ -568,6 +568,11 @@ func (c *Client) Pull(ref string, options ...PullOption) (*PullResult, error) { // Build allowed media types for chart pull allowedMediaTypes := []string{ ocispec.MediaTypeImageManifest, + // A chart converted between registries comes back written to the Docker + // schema, which describes the same manifest. Copying it is what selecting it + // out of an index is for; leaving the type out drops the manifest the copy + // was pointed at and fails the pull on a blob it never stored. + dockerManifestMediaType, ConfigMediaType, } if operation.withChart { diff --git a/pkg/registry/generic.go b/pkg/registry/generic.go index 8f7ea3e44..7d7159d31 100644 --- a/pkg/registry/generic.go +++ b/pkg/registry/generic.go @@ -111,6 +111,57 @@ func NewGenericClient(client *Client) *GenericClient { } } +// maxIndexDepth is how many indexes deep the search follows, counting from the one +// the reference resolves to. A multi-arch image listed beside a chart is depth one, +// which is what tooling produces today, and an index aggregating such indexes is +// depth two. Three leaves a level beyond anything a publisher assembles. Past that +// the cost is one sequential request per level against a chain the registry is free +// to make up as it is walked. +const maxIndexDepth = 3 + +// nestedIndex is an index listed inside another one, carried with the depth it was +// found at, since that is what the limit above is applied to. +type nestedIndex struct { + desc ocispec.Descriptor + depth int +} + +// dockerManifestListMediaType is what Docker Hub and older buildx write where the +// OCI spec writes an image index. It lists manifests the same way, so an entry +// carrying it holds artifacts the same way too. +const dockerManifestListMediaType = "application/vnd.docker.distribution.manifest.list.v2+json" + +// dockerManifestMediaType is the same story one level down: a manifest copied +// between registries by tooling that writes the Docker schema describes its config +// and layers exactly as an image manifest does. +const dockerManifestMediaType = "application/vnd.docker.distribution.manifest.v2+json" + +// listsManifests reports whether an entry holds other entries rather than an +// artifact. Such an entry is never a candidate, since an artifact is identified by +// the config blob its manifest carries and a list has none, and it is searched +// rather than dropped, since it can hold the artifact that was asked for. +func listsManifests(mediaType string) bool { + return mediaType == ocispec.MediaTypeImageIndex || mediaType == dockerManifestListMediaType +} + +// maxEntryBytes bounds what the pass below reads out of an entry it was not asked +// for. The size is the index's own claim about content the registry serves, and a +// manifest that large is not a manifest, so an entry declaring more is left unread +// instead of streamed. The figure is what oras allows a manifest to be. Reading a +// candidate's own identity is bounded by oras instead, which caps what it allocates +// ahead of the content and verifies size and digest against it. +const maxEntryBytes = 4 * 1024 * 1024 + +// entryBody is what an entry turns out to hold once read, which is what decides how +// it is treated: a config makes it an artifact and a list of manifests makes it a +// container of artifacts. An entry declared as a manifest that holds no config is +// read as neither; one declared as a list that holds a config is too, while a list +// that holds nothing is a list, and searching it finds nothing to find. +type entryBody struct { + Config ocispec.Descriptor `json:"config"` + Manifests []ocispec.Descriptor `json:"manifests"` +} + // resolveFromIndex selects one manifest from an OCI Image Index by artifactType. // When nothing matches on artifactType it retries over the entries that declare // none, matching their config mediaType instead, which is how indexes written @@ -134,48 +185,126 @@ func resolveFromIndex(ctx context.Context, fetcher content.Fetcher, indexDesc oc // that stamps a platform on every index entry would otherwise hide the artifact. var candidates []ocispec.Descriptor var availableTypes []string - var undeclared []ocispec.Descriptor + var unmatched []ocispec.Descriptor + var nested []nestedIndex + var skipped []error for _, manifest := range index.Manifests { - switch { - case manifest.ArtifactType == artifactType: + // Set aside to be searched, per listsManifests: taken as a candidate the pull + // fails on a manifest it never stored, dropped it takes the chart it holds with + // it, and the chart beside it answers instead. + if listsManifests(manifest.MediaType) { + nested = append(nested, nestedIndex{desc: manifest, depth: 1}) + continue + } + if manifest.ArtifactType == artifactType { candidates = append(candidates, manifest) - case manifest.ArtifactType != "": + continue + } + if manifest.ArtifactType != "" { availableTypes = append(availableTypes, manifest.ArtifactType) - default: - undeclared = append(undeclared, manifest) } + unmatched = append(unmatched, manifest) } wantName := selectors[ocispec.AnnotationTitle] wantVersion := selectors[ocispec.AnnotationVersion] - // Second pass: entries that declare no type at all, matched on the config - // mediaType instead, which is how an index written before artifactType existed - // stays resolvable. It runs when the first pass found nothing, and also when - // no single entry it found announces the whole requested name and version: - // narrowing the candidates before either is checked is how a mixed index answers - // with the wrong chart. Entries carrying a platform are fetched here too, at one - // fetch per undeclared entry. That cost is not confined to failures: an index - // builder may set artifactType without copying the manifest annotations onto the - // descriptor, and then the declared entries announce nothing, so pulls that go on - // to succeed pay it as well. + // Second pass: the indexes an entry points at, and every entry whose declaration + // did not match, read for the config mediaType its manifest carries. An index + // written before artifactType existed declares nothing, and a builder that sets + // the field wrongly declares something else; both leave the config as the only + // place the artifact says what it is, so both are read here rather than only the + // first. It runs when the first pass found nothing, and also when no single entry + // it found announces the whole requested name and version: narrowing the + // candidates before either is checked is how a mixed index answers with the wrong + // chart. The cost is one fetch per entry that is not already a candidate, and it + // is not confined to failures: an index builder may set artifactType without + // copying the manifest annotations onto the descriptor, and then the declared + // entries announce nothing, so pulls that go on to succeed pay it as well. configCache := map[string]ocispec.Descriptor{} - var skipped []error + // Counted rather than taken from the length of the queue: an index may list the + // same manifest twice and the repeats are not read again, so the queue says how + // many entries were queued and this says how many were looked at. + read := 0 if len(candidates) == 0 || !announcesChart(candidates, wantName, wantVersion) { - for _, candidate := range undeclared { + // An index an entry points at is searched the same way the top one is, so a + // chart keeps its place wherever the publisher nested it. Entries already seen + // are not read twice, which is also what stops an index that lists itself. + seen := map[string]bool{indexDesc.Digest.String(): true} + for i := 0; i < len(nested); i++ { + entry := nested[i] + if seen[entry.desc.Digest.String()] { + continue + } + seen[entry.desc.Digest.String()] = true + if entry.depth > maxIndexDepth { + skipped = append(skipped, fmt.Errorf( + "%s: nested more than %d indexes deep and was not searched", entry.desc.Digest, maxIndexDepth)) + continue + } + if entry.desc.Size > maxEntryBytes { + skipped = append(skipped, fmt.Errorf( + "%s: entry declares %d bytes, more than a manifest is, and was not read", entry.desc.Digest, entry.desc.Size)) + continue + } + indexData, err := content.FetchAll(ctx, fetcher, entry.desc) + if err != nil { + skipped = append(skipped, fmt.Errorf("%s: %w", entry.desc.Digest, err)) + continue + } + var sub entryBody + if err := json.Unmarshal(indexData, &sub); err != nil { + skipped = append(skipped, fmt.Errorf("%s: %w", entry.desc.Digest, err)) + continue + } + if len(sub.Manifests) == 0 && sub.Config.MediaType != "" { + skipped = append(skipped, fmt.Errorf( + "%s: entry declares a list of manifests and holds a config instead, and was searched as neither", entry.desc.Digest)) + continue + } + for _, manifest := range sub.Manifests { + switch { + case listsManifests(manifest.MediaType): + nested = append(nested, nestedIndex{desc: manifest, depth: entry.depth + 1}) + case manifest.ArtifactType == artifactType: + candidates = append(candidates, manifest) + default: + if manifest.ArtifactType != "" { + availableTypes = append(availableTypes, manifest.ArtifactType) + } + unmatched = append(unmatched, manifest) + } + } + } + for _, candidate := range unmatched { + if seen[candidate.Digest.String()] { + continue + } + seen[candidate.Digest.String()] = true + if candidate.Size > maxEntryBytes { + skipped = append(skipped, fmt.Errorf( + "%s: entry declares %d bytes, more than a manifest is, and was not read", candidate.Digest, candidate.Size)) + continue + } manifestData, err := content.FetchAll(ctx, fetcher, candidate) if err != nil { skipped = append(skipped, fmt.Errorf("%s: %w", candidate.Digest, err)) continue } - var manifest ocispec.Manifest - if err := json.Unmarshal(manifestData, &manifest); err != nil { + var body entryBody + if err := json.Unmarshal(manifestData, &body); err != nil { skipped = append(skipped, fmt.Errorf("%s: %w", candidate.Digest, err)) continue } - if manifest.Config.MediaType == artifactType { + if body.Config.MediaType == "" { + skipped = append(skipped, fmt.Errorf( + "%s: entry declares a manifest and holds no config, and was read as neither", candidate.Digest)) + continue + } + read++ + if body.Config.MediaType == artifactType { candidates = append(candidates, candidate) - configCache[candidate.Digest.String()] = manifest.Config + configCache[candidate.Digest.String()] = body.Config } } } @@ -205,9 +334,13 @@ func resolveFromIndex(ctx context.Context, fetcher content.Fetcher, indexDesc oc "no manifest with artifactType %q found in image index; available types: %v; %d entries could not be read: %v", artifactType, availableTypes, len(skipped), skipped) } + readNote := fmt.Sprintf("the manifests of %d other entries were read and none declares a chart config", read) + if read == 1 { + readNote = "the manifest of the one other entry was read and it declares no chart config" + } return ocispec.Descriptor{}, fmt.Errorf( - "no manifest with artifactType %q found in image index; available types: %v; %d entries declared none", - artifactType, availableTypes, len(undeclared)) + "no manifest with artifactType %q found in image index; available types: %v; %s", + artifactType, availableTypes, readNote) case 1: // One candidate is taken without checking the name against it. Requiring a // match would break publishing a chart under a repository named differently @@ -278,7 +411,7 @@ func resolveFromIndex(ctx context.Context, fetcher content.Fetcher, indexDesc oc if len(skipped) > 0 && wantVersion != "" && named[0].version != wantVersion { if named[0].idErr != nil { return ocispec.Descriptor{}, fmt.Errorf( - "the version of the only chart named %q could not be read: %w; %d entries could not be read either: %v", + "the version of the only chart named %q could not be read: %w; %d entries could not be read: %v", wantName, named[0].idErr, len(skipped), skipped) } return ocispec.Descriptor{}, fmt.Errorf( @@ -505,7 +638,22 @@ func (c *GenericClient) PullGeneric(ref string, options GenericPullOptions) (*Ge if root.MediaType != ocispec.MediaTypeImageIndex { return root, nil } - return resolveFromIndex(ctx, src, root, options.ArtifactType, options.Selectors, options.ParseIdentity) + selected, err := resolveFromIndex(ctx, src, root, options.ArtifactType, options.Selectors, options.ParseIdentity) + if err != nil { + return ocispec.Descriptor{}, err + } + // The copy about to start filters on the same media types the caller + // allowed, and a root it will not store is dropped without an error of its + // own: the pull then fails on a blob nothing ever wrote, naming neither the + // entry nor why. Selection knows both, so it says so here instead. + if len(allowedMediaTypes) > 0 { + if i := sort.SearchStrings(allowedMediaTypes, selected.MediaType); i >= len(allowedMediaTypes) || allowedMediaTypes[i] != selected.MediaType { + return ocispec.Descriptor{}, fmt.Errorf( + "the chart selected from the image index is a %s, which this pull does not accept: %s", + selected.MediaType, selected.Digest) + } + } + return selected, nil } } diff --git a/pkg/registry/index_test.go b/pkg/registry/index_test.go index b23978400..edb7ba1a5 100644 --- a/pkg/registry/index_test.go +++ b/pkg/registry/index_test.go @@ -70,8 +70,17 @@ func TestPullFromImageIndex(t *testing.T) { chartManifestBytes, _ := json.Marshal(chartManifest) chartManifestDigest := digest.FromBytes(chartManifestBytes) - // Container manifest (we won't actually serve the blobs, just need valid structure) - containerManifestDigest := digest.Digest("sha256:ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff") + // The container manifest beside the chart, served like the registry serves it, + // since selection reads the config of an entry whose declaration did not match. + containerManifestBytes, _ := json.Marshal(ocispec.Manifest{ + MediaType: ocispec.MediaTypeImageManifest, + Config: ocispec.Descriptor{ + MediaType: ocispec.MediaTypeImageConfig, + Digest: digest.FromString("container-config"), + Size: 10, + }, + }) + containerManifestDigest := digest.FromBytes(containerManifestBytes) // Image Index containing both chart and container manifests imageIndex := ocispec.Index{ @@ -80,7 +89,7 @@ func TestPullFromImageIndex(t *testing.T) { { MediaType: ocispec.MediaTypeImageManifest, Digest: containerManifestDigest, - Size: 500, + Size: int64(len(containerManifestBytes)), ArtifactType: "application/vnd.oci.image.config.v1+json", Platform: &ocispec.Platform{ Architecture: "amd64", @@ -119,6 +128,13 @@ func TestPullFromImageIndex(t *testing.T) { w.WriteHeader(http.StatusOK) _, _ = w.Write(imageIndexBytes) + // Serve the container manifest by digest + case path == "/v2/testrepo/multichart/manifests/"+containerManifestDigest.String(): + w.Header().Set("Content-Type", ocispec.MediaTypeImageManifest) + w.Header().Set("Docker-Content-Digest", containerManifestDigest.String()) + w.WriteHeader(http.StatusOK) + _, _ = w.Write(containerManifestBytes) + // Serve chart manifest by digest case path == "/v2/testrepo/multichart/manifests/"+chartManifestDigest.String(): w.Header().Set("Content-Type", ocispec.MediaTypeImageManifest) @@ -441,6 +457,42 @@ func serveMultiChartIndex(indexBytes []byte, indexDigest digest.Digest, charts . })) } +type rawBlob struct { + mediaType string + data []byte +} + +// serveAlso answers for extra blobs by digest and delegates the rest to base. Index +// trees need it: everything a nested index lists has to be fetchable, or the pass +// over it is incomplete for a reason the fixture invented. +func serveAlso(base *httptest.Server, extras map[digest.Digest]rawBlob) *httptest.Server { + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + for d, blob := range extras { + if strings.Contains(r.URL.Path, d.Encoded()) { + w.Header().Set("Content-Type", blob.mediaType) + w.Header().Set("Docker-Content-Digest", d.String()) + _, _ = w.Write(blob.data) + return + } + } + base.Config.Handler.ServeHTTP(w, r) + })) +} + +// indexHolding builds an index over the given entries and returns it as a blob. +func indexHolding(entries ...ocispec.Descriptor) (ocispec.Descriptor, rawBlob) { + body, _ := json.Marshal(ocispec.Index{MediaType: ocispec.MediaTypeImageIndex, Manifests: entries}) + d := digest.FromBytes(body) + return ocispec.Descriptor{ + MediaType: ocispec.MediaTypeImageIndex, + Digest: d, + Size: int64(len(body)), + }, rawBlob{ + mediaType: ocispec.MediaTypeImageIndex, + data: body, + } +} + func TestPullFromImageIndexSelectsByName(t *testing.T) { alpha := newTestChart("alpha", "1.0.0") beta := newTestChart("beta", "1.0.0") @@ -496,6 +548,9 @@ func TestPullFromImageIndexNoChartWithRequestedName(t *testing.T) { assert.Contains(t, err.Error(), "none is named") assert.Contains(t, err.Error(), "alpha") assert.Contains(t, err.Error(), "beta") + // Selection runs inside the copy, so every message it produces reaches the caller + // wrapped in the copy's own error unless that wrapper is undone. + assert.NotContains(t, err.Error(), "MapRoot") } func TestPullFromImageIndexSelectsPlatformStampedChart(t *testing.T) { @@ -1175,3 +1230,574 @@ func TestPullFromImageIndexLegacySelectsByName(t *testing.T) { require.NoError(t, err) assertSelected(t, beta, result) } + +func TestPullFromImageIndexNestedIndexIsNotACandidate(t *testing.T) { + // An index entry may itself be an index, and nothing stops it from declaring the + // chart artifact type and the chart's identity. Taken as a candidate it would be + // handed to a copy that cannot root itself at an index; the chart it points at is + // what the pull has to come back with. + alpha := newTestChart("alpha", "1.0.0") + + innerDesc, innerBlob := indexHolding(alpha.indexDescriptor()) + innerDesc.ArtifactType = ChartArtifactType + innerDesc.Annotations = map[string]string{ + ocispec.AnnotationTitle: alpha.name, + ocispec.AnnotationVersion: alpha.version, + } + + index := ocispec.Index{ + MediaType: ocispec.MediaTypeImageIndex, + Manifests: []ocispec.Descriptor{innerDesc, alpha.indexDescriptor()}, + } + indexBytes, _ := json.Marshal(index) + indexDigest := digest.FromBytes(indexBytes) + + charts := serveMultiChartIndex(indexBytes, indexDigest, alpha) + defer charts.Close() + s := serveAlso(charts, map[digest.Digest]rawBlob{innerDesc.Digest: innerBlob}) + defer s.Close() + u, _ := url.Parse(s.URL) + host := "localhost:" + u.Port() + + client, err := NewClient(ClientOptPlainHTTP()) + require.NoError(t, err) + + result, err := client.Pull(host + "/testrepo/alpha:1.0.0") + require.NoError(t, err) + assertSelected(t, alpha, result) +} + +func TestPullFromImageIndexFindsRequestedChartInsideNestedIndex(t *testing.T) { + // The requested chart is reachable only through an entry that is an index. Left + // unsearched it is a chart that is not there, and the chart beside it answers in + // its place. + alpha := newTestChart("alpha", "1.0.0") + beta := newTestChart("beta", "1.0.0") + + innerDesc, innerBlob := indexHolding(alpha.indexDescriptor()) + + index := ocispec.Index{ + MediaType: ocispec.MediaTypeImageIndex, + Manifests: []ocispec.Descriptor{innerDesc, beta.indexDescriptor()}, + } + indexBytes, _ := json.Marshal(index) + indexDigest := digest.FromBytes(indexBytes) + + charts := serveMultiChartIndex(indexBytes, indexDigest, alpha, beta) + defer charts.Close() + s := serveAlso(charts, map[digest.Digest]rawBlob{innerDesc.Digest: innerBlob}) + defer s.Close() + u, _ := url.Parse(s.URL) + host := "localhost:" + u.Port() + + client, err := NewClient(ClientOptPlainHTTP()) + require.NoError(t, err) + + result, err := client.Pull(host + "/testrepo/alpha:1.0.0") + require.NoError(t, err) + assertSelected(t, alpha, result) +} + +func TestPullFromImageIndexFindsRequestedVersionInsideNestedIndex(t *testing.T) { + // Same search, one axis over: the requested version is the one nested away, and + // the version left in plain sight carries the requested name. + wanted := newTestChart("app", "2.0.0") + older := newTestChart("app", "1.0.0") + other := newTestChart("other", "1.0.0") + + innerDesc, innerBlob := indexHolding(wanted.indexDescriptor()) + + index := ocispec.Index{ + MediaType: ocispec.MediaTypeImageIndex, + Manifests: []ocispec.Descriptor{innerDesc, older.indexDescriptor(), other.indexDescriptor()}, + } + indexBytes, _ := json.Marshal(index) + indexDigest := digest.FromBytes(indexBytes) + + charts := serveMultiChartIndex(indexBytes, indexDigest, wanted, older, other) + defer charts.Close() + s := serveAlso(charts, map[digest.Digest]rawBlob{innerDesc.Digest: innerBlob}) + defer s.Close() + u, _ := url.Parse(s.URL) + host := "localhost:" + u.Port() + + client, err := NewClient(ClientOptPlainHTTP()) + require.NoError(t, err) + + result, err := client.Pull(host + "/testrepo/app:2.0.0") + require.NoError(t, err) + assertSelected(t, wanted, result) +} + +func TestPullFromImageIndexUnreadableNestedIndexIsNotAnAbsentChart(t *testing.T) { + // A nested index that cannot be fetched is the case the search cannot complete. + // The chart beside it is then not the only chart in the index, it is the only one + // that could be read, and answering with it would state a fact about the read. + beta := newTestChart("beta", "1.0.0") + + unreachable := ocispec.Descriptor{ + MediaType: ocispec.MediaTypeImageIndex, + Digest: digest.FromString("unreachable"), + Size: 2, + } + index := ocispec.Index{ + MediaType: ocispec.MediaTypeImageIndex, + Manifests: []ocispec.Descriptor{unreachable, beta.indexDescriptor()}, + } + indexBytes, _ := json.Marshal(index) + indexDigest := digest.FromBytes(indexBytes) + + s := serveMultiChartIndex(indexBytes, indexDigest, beta) + defer s.Close() + u, _ := url.Parse(s.URL) + host := "localhost:" + u.Port() + + client, err := NewClient(ClientOptPlainHTTP()) + require.NoError(t, err) + + _, err = client.Pull(host + "/testrepo/alpha:1.0.0") + require.Error(t, err) + assert.Contains(t, err.Error(), "could not be read") +} + +func TestPullFromImageIndexChartNamedUnlikeItsRepositoryBesideNestedIndex(t *testing.T) { + // Publishing a chart under a repository named after something else is legal, and + // the sole chart in an index answers whatever name was asked for. A multi-arch + // image sitting beside it is an index, and searching it must not turn the name + // this pull never knew into grounds for refusing the chart. + chart := newTestChart("testchart", "1.0.0") + + imageConfig := []byte(`{"architecture":"amd64","os":"linux"}`) + imageConfigDigest := digest.FromBytes(imageConfig) + imageManifest, _ := json.Marshal(ocispec.Manifest{ + MediaType: ocispec.MediaTypeImageManifest, + Config: ocispec.Descriptor{ + MediaType: ocispec.MediaTypeImageConfig, + Digest: imageConfigDigest, + Size: int64(len(imageConfig)), + }, + }) + imageDesc := ocispec.Descriptor{ + MediaType: ocispec.MediaTypeImageManifest, + Digest: digest.FromBytes(imageManifest), + Size: int64(len(imageManifest)), + Platform: &ocispec.Platform{OS: "linux", Architecture: "amd64"}, + } + innerDesc, innerBlob := indexHolding(imageDesc) + + index := ocispec.Index{ + MediaType: ocispec.MediaTypeImageIndex, + Manifests: []ocispec.Descriptor{chart.indexDescriptor(), innerDesc}, + } + indexBytes, _ := json.Marshal(index) + indexDigest := digest.FromBytes(indexBytes) + + charts := serveMultiChartIndex(indexBytes, indexDigest, chart) + defer charts.Close() + s := serveAlso(charts, map[digest.Digest]rawBlob{ + innerDesc.Digest: innerBlob, + imageDesc.Digest: {mediaType: ocispec.MediaTypeImageManifest, data: imageManifest}, + }) + defer s.Close() + u, _ := url.Parse(s.URL) + host := "localhost:" + u.Port() + + client, err := NewClient(ClientOptPlainHTTP()) + require.NoError(t, err) + + result, err := client.Pull(host + "/testrepo/multichart:1.0.0") + require.NoError(t, err) + assertSelected(t, chart, result) +} + +func TestPullFromImageIndexSelectsVersionCarryingBuildMetadata(t *testing.T) { + // A version with build metadata reaches the registry with the plus sign encoded + // as an underscore, while the chart's own annotation keeps the plus, so selection + // has to undo the encoding before the two can be compared. + withMetadata := newTestChart("app", "1.0.0+build") + plain := newTestChart("app", "2.0.0") + + index := ocispec.Index{ + MediaType: ocispec.MediaTypeImageIndex, + Manifests: []ocispec.Descriptor{plain.indexDescriptor(), withMetadata.indexDescriptor()}, + } + indexBytes, _ := json.Marshal(index) + indexDigest := digest.FromBytes(indexBytes) + + s := serveMultiChartIndex(indexBytes, indexDigest, plain, withMetadata) + defer s.Close() + u, _ := url.Parse(s.URL) + host := "localhost:" + u.Port() + + client, err := NewClient(ClientOptPlainHTTP()) + require.NoError(t, err) + + result, err := client.Pull(host + "/testrepo/app:1.0.0+build") + require.NoError(t, err) + assertSelected(t, withMetadata, result) +} + +// nestedChain wraps desc in levels indexes, each holding the one below, and returns +// the outermost of them together with every body the chain needs served. +func nestedChain(desc ocispec.Descriptor, levels int) (ocispec.Descriptor, map[digest.Digest]rawBlob) { + blobs := map[digest.Digest]rawBlob{} + for range levels { + wrapper, blob := indexHolding(desc) + blobs[wrapper.Digest] = blob + desc = wrapper + } + return desc, blobs +} + +func TestPullFromImageIndexStopsAtTheNestingLimit(t *testing.T) { + // The chart is reachable only by following indexes, so how deep the search goes + // is the only thing that decides whether it is found. At the limit it is; one + // index further down it is not, and the pull says which entry it left unread + // rather than reporting a chart that is not there. + alpha := newTestChart("alpha", "1.0.0") + + pull := func(t *testing.T, levels int) (*PullResult, error) { + t.Helper() + outermost, chain := nestedChain(alpha.indexDescriptor(), levels) + index := ocispec.Index{ + MediaType: ocispec.MediaTypeImageIndex, + Manifests: []ocispec.Descriptor{outermost}, + } + indexBytes, _ := json.Marshal(index) + indexDigest := digest.FromBytes(indexBytes) + + charts := serveMultiChartIndex(indexBytes, indexDigest, alpha) + defer charts.Close() + s := serveAlso(charts, chain) + defer s.Close() + u, _ := url.Parse(s.URL) + + client, err := NewClient(ClientOptPlainHTTP()) + require.NoError(t, err) + return client.Pull("localhost:" + u.Port() + "/testrepo/alpha:1.0.0") + } + + result, err := pull(t, maxIndexDepth) + require.NoError(t, err) + assertSelected(t, alpha, result) + + _, err = pull(t, maxIndexDepth+1) + require.Error(t, err) + assert.Contains(t, err.Error(), "indexes deep and was not searched") +} + +func TestPullFromImageIndexEntryDeclaringTheWrongTypeStillResolves(t *testing.T) { + // A builder that sets artifactType to something other than the chart type has + // broken the descriptor the same way one that omits it has, and in both cases the + // config the manifest carries is where the artifact says what it is. + alpha := newTestChart("alpha", "1.0.0") + desc := alpha.indexDescriptor() + desc.ArtifactType = ocispec.MediaTypeImageManifest + + index := ocispec.Index{ + MediaType: ocispec.MediaTypeImageIndex, + Manifests: []ocispec.Descriptor{desc}, + } + indexBytes, _ := json.Marshal(index) + indexDigest := digest.FromBytes(indexBytes) + + s := serveMultiChartIndex(indexBytes, indexDigest, alpha) + defer s.Close() + u, _ := url.Parse(s.URL) + host := "localhost:" + u.Port() + + client, err := NewClient(ClientOptPlainHTTP()) + require.NoError(t, err) + + result, err := client.Pull(host + "/testrepo/alpha:1.0.0") + require.NoError(t, err) + assertSelected(t, alpha, result) +} + +func TestPullFromImageIndexUnreadableNeighbourIsConfirmedAgainstOrNamed(t *testing.T) { + // An entry declaring a type of its own is read for the config its manifest + // carries, so a registry that will not serve it leaves the pass incomplete. What + // the sole chart is then worth depends on whether its identity can be confirmed + // against the reference: confirmed it answers, unconfirmed the entry that could + // not be read is named, since it is the one that might have held the request. + unreadable := ocispec.Descriptor{ + MediaType: ocispec.MediaTypeImageManifest, + Digest: digest.FromString("neighbour the registry will not serve"), + Size: 12, + ArtifactType: "application/vnd.oci.image.config.v1+json", + } + + pull := func(t *testing.T, repository string) (*PullResult, error) { + t.Helper() + alpha := newTestChart("alpha", "1.0.0") + index := ocispec.Index{ + MediaType: ocispec.MediaTypeImageIndex, + // Without annotations the declared entry announces no identity, so the + // entries that did not match are read and the neighbour is reached. + Manifests: []ocispec.Descriptor{alpha.declaredWithoutAnnotations(), unreadable}, + } + indexBytes, _ := json.Marshal(index) + indexDigest := digest.FromBytes(indexBytes) + + s := serveMultiChartIndex(indexBytes, indexDigest, alpha) + defer s.Close() + u, _ := url.Parse(s.URL) + + client, err := NewClient(ClientOptPlainHTTP()) + require.NoError(t, err) + return client.Pull("localhost:" + u.Port() + "/" + repository + ":1.0.0") + } + + t.Run("identity confirmed", func(t *testing.T) { + result, err := pull(t, "testrepo/alpha") + require.NoError(t, err) + assertSelected(t, newTestChart("alpha", "1.0.0"), result) + }) + + t.Run("identity not confirmed", func(t *testing.T) { + _, err := pull(t, "testrepo/somethingelse") + require.Error(t, err) + assert.Contains(t, err.Error(), unreadable.Digest.String()) + }) +} + +func TestPullFromImageIndexRepeatedEntryIsReadOnce(t *testing.T) { + // An index may list the same manifest twice, and the repeat is not read again. + // The count in the message says how many entries were looked at, so it has to + // come from the reading rather than from the length of the queue. + image, _ := json.Marshal(ocispec.Manifest{ + MediaType: ocispec.MediaTypeImageManifest, + Config: ocispec.Descriptor{ + MediaType: ocispec.MediaTypeImageConfig, + Digest: digest.FromString("image config"), + Size: 10, + }, + }) + imageDesc := ocispec.Descriptor{ + MediaType: ocispec.MediaTypeImageManifest, + Digest: digest.FromBytes(image), + Size: int64(len(image)), + } + + index := ocispec.Index{ + MediaType: ocispec.MediaTypeImageIndex, + Manifests: []ocispec.Descriptor{imageDesc, imageDesc}, + } + indexBytes, _ := json.Marshal(index) + indexDigest := digest.FromBytes(indexBytes) + + base := serveMultiChartIndex(indexBytes, indexDigest) + defer base.Close() + s := serveAlso(base, map[digest.Digest]rawBlob{ + imageDesc.Digest: {mediaType: ocispec.MediaTypeImageManifest, data: image}, + }) + defer s.Close() + u, _ := url.Parse(s.URL) + + client, err := NewClient(ClientOptPlainHTTP()) + require.NoError(t, err) + + _, err = client.Pull("localhost:" + u.Port() + "/testrepo/alpha:1.0.0") + require.Error(t, err) + assert.Contains(t, err.Error(), "the manifest of the one other entry was read") +} + +func TestPullFromImageIndexFindsChartInsideDockerManifestList(t *testing.T) { + // Docker Hub and older buildx write a manifest list where the OCI spec writes an + // index. It lists manifests the same way, so an entry carrying it is searched the + // same way; treated as an ordinary manifest it holds no config, and the chart + // inside it becomes a chart the index is said not to hold. + const dockerManifestList = "application/vnd.docker.distribution.manifest.list.v2+json" + alpha := newTestChart("alpha", "1.0.0") + + inner, _ := json.Marshal(ocispec.Index{ + MediaType: dockerManifestList, + Manifests: []ocispec.Descriptor{alpha.indexDescriptor()}, + }) + innerDigest := digest.FromBytes(inner) + + index := ocispec.Index{ + MediaType: ocispec.MediaTypeImageIndex, + Manifests: []ocispec.Descriptor{{ + MediaType: dockerManifestList, + Digest: innerDigest, + Size: int64(len(inner)), + }}, + } + indexBytes, _ := json.Marshal(index) + indexDigest := digest.FromBytes(indexBytes) + + base := serveMultiChartIndex(indexBytes, indexDigest, alpha) + defer base.Close() + s := serveAlso(base, map[digest.Digest]rawBlob{ + innerDigest: {mediaType: dockerManifestList, data: inner}, + }) + defer s.Close() + u, _ := url.Parse(s.URL) + + client, err := NewClient(ClientOptPlainHTTP()) + require.NoError(t, err) + + result, err := client.Pull("localhost:" + u.Port() + "/testrepo/alpha:1.0.0") + require.NoError(t, err) + assertSelected(t, alpha, result) +} + +func TestPullFromImageIndexOversizedEntryIsNotRead(t *testing.T) { + // The size on an index entry is the index's claim about content the registry + // serves. A manifest that large is not a manifest, so the entry is left unread + // and named rather than streamed into memory to be discarded. + alpha := newTestChart("alpha", "1.0.0") + + oversized := ocispec.Descriptor{ + MediaType: ocispec.MediaTypeImageManifest, + Digest: digest.FromString("oversized neighbour"), + Size: maxEntryBytes + 1, + } + index := ocispec.Index{ + MediaType: ocispec.MediaTypeImageIndex, + Manifests: []ocispec.Descriptor{alpha.declaredWithoutAnnotations(), oversized}, + } + indexBytes, _ := json.Marshal(index) + indexDigest := digest.FromBytes(indexBytes) + + // The oversized entry is served, so a pull that reads it succeeds in reading it; + // only the size gate keeps the body out. + served := make([]byte, 0) + s := serveAlso(serveMultiChartIndex(indexBytes, indexDigest, alpha), map[digest.Digest]rawBlob{ + oversized.Digest: {mediaType: ocispec.MediaTypeImageManifest, data: served}, + }) + defer s.Close() + u, _ := url.Parse(s.URL) + host := "localhost:" + u.Port() + + client, err := NewClient(ClientOptPlainHTTP()) + require.NoError(t, err) + + // Asked for by its own name, the chart still resolves: the unread entry makes the + // pass incomplete, and the identity of what is left is confirmed against the + // reference before it answers. + result, err := client.Pull(host + "/testrepo/alpha:1.0.0") + require.NoError(t, err) + assertSelected(t, alpha, result) + + // Asked for by another name, the entry that was not read is named, since it is + // the one that might have held the chart. + _, err = client.Pull(host + "/testrepo/somethingelse:1.0.0") + require.Error(t, err) + assert.Contains(t, err.Error(), "more than a manifest is") + assert.Contains(t, err.Error(), oversized.Digest.String()) +} + +func TestPullFromImageIndexEntryHoldingSomethingElseIsNotSilentlyDropped(t *testing.T) { + // An index body parses cleanly into a manifest and a manifest body parses cleanly + // into an index, because neither shape has a field the other rejects. An entry + // whose body does not hold what it declared is therefore read as neither, and + // saying so is what keeps the chart beside it from answering unchallenged. + beta := newTestChart("beta", "1.0.0") + foo := newTestChart("foo", "1.0.0") + + // An index body, listed under a manifest media type. + mislabelled, _ := json.Marshal(ocispec.Index{ + MediaType: ocispec.MediaTypeImageIndex, + Manifests: []ocispec.Descriptor{foo.indexDescriptor()}, + }) + mislabelledDesc := ocispec.Descriptor{ + MediaType: ocispec.MediaTypeImageManifest, + Digest: digest.FromBytes(mislabelled), + Size: int64(len(mislabelled)), + } + + index := ocispec.Index{ + MediaType: ocispec.MediaTypeImageIndex, + Manifests: []ocispec.Descriptor{mislabelledDesc, beta.declaredWithoutAnnotations()}, + } + indexBytes, _ := json.Marshal(index) + indexDigest := digest.FromBytes(indexBytes) + + s := serveAlso(serveMultiChartIndex(indexBytes, indexDigest, beta, foo), map[digest.Digest]rawBlob{ + mislabelledDesc.Digest: {mediaType: ocispec.MediaTypeImageManifest, data: mislabelled}, + }) + defer s.Close() + u, _ := url.Parse(s.URL) + host := "localhost:" + u.Port() + + client, err := NewClient(ClientOptPlainHTTP()) + require.NoError(t, err) + + _, err = client.Pull(host + "/testrepo/foo:1.0.0") + require.Error(t, err) + assert.Contains(t, err.Error(), mislabelledDesc.Digest.String()) +} + +func TestPullFromImageIndexChartWrittenToTheDockerSchema(t *testing.T) { + // A chart copied between registries by tooling that writes the Docker schema + // carries the Docker manifest media type while describing the same config and + // layers. Selecting it and then refusing to copy it fails the pull on a blob + // nothing stored, so the type the copy accepts has to cover what selection picks. + alpha := newTestChart("alpha", "1.0.0") + desc := alpha.indexDescriptor() + desc.MediaType = dockerManifestMediaType + + index := ocispec.Index{ + MediaType: ocispec.MediaTypeImageIndex, + Manifests: []ocispec.Descriptor{desc}, + } + indexBytes, _ := json.Marshal(index) + indexDigest := digest.FromBytes(indexBytes) + + // The registry answers for that manifest with the type the descriptor declares, + // which is what a registry holding a converted chart does. + base := serveMultiChartIndex(indexBytes, indexDigest, alpha) + defer base.Close() + s := serveAlso(base, map[digest.Digest]rawBlob{ + alpha.manifestDigest: {mediaType: dockerManifestMediaType, data: alpha.manifestBytes}, + }) + defer s.Close() + u, _ := url.Parse(s.URL) + + client, err := NewClient(ClientOptPlainHTTP()) + require.NoError(t, err) + + result, err := client.Pull("localhost:" + u.Port() + "/testrepo/alpha:1.0.0") + require.NoError(t, err) + assertSelected(t, alpha, result) +} + +func TestPullFromImageIndexChartTheCopyCannotStoreIsNamed(t *testing.T) { + // Selection matches on the artifact type and the config, neither of which says + // how the manifest itself is written. A chart written in a form this pull does + // not copy has to be named by selection, which knows what it picked and why, + // rather than left to surface as a blob the copy never stored. + alpha := newTestChart("alpha", "1.0.0") + desc := alpha.indexDescriptor() + const artifactManifest = "application/vnd.oci.artifact.manifest.v1+json" + desc.MediaType = artifactManifest + + index := ocispec.Index{ + MediaType: ocispec.MediaTypeImageIndex, + Manifests: []ocispec.Descriptor{desc}, + } + indexBytes, _ := json.Marshal(index) + indexDigest := digest.FromBytes(indexBytes) + + base := serveMultiChartIndex(indexBytes, indexDigest, alpha) + defer base.Close() + s := serveAlso(base, map[digest.Digest]rawBlob{ + alpha.manifestDigest: {mediaType: artifactManifest, data: alpha.manifestBytes}, + }) + defer s.Close() + u, _ := url.Parse(s.URL) + + client, err := NewClient(ClientOptPlainHTTP()) + require.NoError(t, err) + + _, err = client.Pull("localhost:" + u.Port() + "/testrepo/alpha:1.0.0") + require.Error(t, err) + // Named by selection, which knows the entry and the reason. Left to the copy, the + // same shape comes back as a count of descriptors that were not collected, which + // names neither the entry that was picked nor why nothing came of it. + assert.Contains(t, err.Error(), "which this pull does not accept") + assert.Contains(t, err.Error(), artifactManifest) + assert.NotContains(t, err.Error(), "not found") +}