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 <f@lex.la>
Assisted-By: Claude <noreply@anthropic.com>

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,

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
 )

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, &copyErr) && copyErr.Op == "MapRoot" {
+			return nil, copyErr.Err
+		}
 		return nil, err
 	}