fix: allow spec compliant, but unusually ordered HELM Chart OCI Artifacts in HELM OCI Artifact downloader (#1470)

<!-- markdownlint-disable MD041 -->
#### What this PR does / why we need it

This allows arbitrarily ordered, but spec
(https://github.com/helm/community/blob/main/hips/hip-0006.md) compliant
HELM Charts in the Downloader when interpreting HELM Chart Artifact
Types.

#### Which issue(s) this PR fixes
<!--
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
-->
This will especially fix resource downloads which previously assumed
layer 0, which is incorrect. Instead we have to find the first and only
occurrence of `application/vnd.cncf.helm.chart.content.v1.tar+gzip`.
Similarly, finding provenance data should not be hardcoded to layer 1,
but to the first and only occurrence of
`application/vnd.cncf.helm.chart.provenance.v1.prov`

Fix #1469

---------

Co-authored-by: Frederic Wilhelm <frederic.wilhelm@sap.com>

Author: jakobmoellerdev

api/ocm/extensions/download/handlers/helm/download_test.go

@@ -4,6 +4,7 @@ import (
 	. "github.com/mandelsoft/goutils/testutils"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+
 	. "ocm.software/ocm/api/helper/builder"
 
 	"github.com/mandelsoft/filepath/pkg/filepath"
@@ -31,6 +32,7 @@ const (
 
 	ArtifactType       = "NotHelmChart"
 	SpecialOCIResource = "specialhelm"
+	UnusualOCIResource = "unusualhelm"
 )
 
 var _ = Describe("upload", func() {
@@ -48,6 +50,9 @@ var _ = Describe("upload", func() {
 					env.Resource(SpecialOCIResource, Version, ArtifactType, v1.LocalRelation, func() {
 						env.BlobFromFile(artifactset.MediaType(artdesc.MediaTypeImageManifest), filepath.Join("/testdata/test-chart-oci-artifact.tgz"))
 					})
+					env.Resource(UnusualOCIResource, Version, resourcetypes.HELM_CHART, v1.LocalRelation, func() {
+						env.BlobFromFile(artifactset.MediaType(artdesc.MediaTypeImageManifest), filepath.Join("/testdata/unusual-ordered-helm-chart.tgz"))
+					})
 				})
 			})
 		})
@@ -73,4 +78,14 @@ var _ = Describe("upload", func() {
 		MustBeSuccessful(tarutils.ExtractArchiveToFs(env.FileSystem(), path, env.FileSystem()))
 		Expect(Must(vfs.DirExists(env.FileSystem(), "/test-chart"))).To(BeTrue())
 	})
+
+	It("successfully download unusual artifacts with non-defacto helm chart order", func() {
+		MustBeSuccessful(download.RegisterHandlerByName(env, helm.PATH, nil, download.ForArtifactType(ArtifactType)))
+
+		repo := Must(ctf.Open(env.OCMContext(), accessobj.ACC_READONLY, CTFPath, 0o777, env))
+		cv := Must(repo.LookupComponentVersion(Component, Version))
+		path := Must(download.DownloadResource(env.OCMContext(), Must(cv.SelectResources(selectors.Identity(v1.Identity{"name": UnusualOCIResource})))[0], "/resource", download.WithFileSystem(env.FileSystem())))
+		MustBeSuccessful(tarutils.ExtractArchiveToFs(env.FileSystem(), path, env.FileSystem()))
+		Expect(Must(vfs.FileExists(env.FileSystem(), "/postgresql/Chart.yaml"))).To(BeTrue())
+	})
 })

api/ocm/extensions/download/handlers/helm/handler.go

@@ -1,12 +1,14 @@
 package helm
 
 import (
+	"errors"
+	"fmt"
 	"io"
 	"strings"
 
-	"github.com/mandelsoft/goutils/errors"
 	"github.com/mandelsoft/goutils/finalizer"
 	"github.com/mandelsoft/vfs/pkg/vfs"
+	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 	helmregistry "helm.sh/helm/v3/pkg/registry"
 
 	"ocm.software/ocm/api/oci"
@@ -24,6 +26,16 @@ import (
 
 const TYPE = resourcetypes.HELM_CHART
 
+var (
+	// ErrNoMatchingLayer denotes that no layer matching the requested media type was found,,
+	// which is invalid as per HELM OCI specification for [helmregistry.ChartLayerMediaType]
+	// but valid for [helmregistry.ProvLayerMediaType].
+	ErrNoMatchingLayer = errors.New("no matching layer found")
+	// ErrMultipleMatchingLayers denotes that multiple layers matching the requested media type were found,
+	// which is invalid as per HELM OCI specification.
+	ErrMultipleMatchingLayers = errors.New("multiple matching layers found")
+)
+
 type Handler struct{}
 
 func init() {
@@ -101,41 +113,72 @@ func (h Handler) Download(p common.Printer, racc cpi.ResourceAccess, path string
 	return h.fromOCIArtifact(p, meth, path, fs)
 }
 
+// download downloads the chart and optional provenance file from an  oci.ArtifactAccess.
+// the format of the artifact is expected to match the official HELM Reference Specification
+//
+// see https://github.com/helm/community/blob/dd5fe7878e293c573cc22db5d36558709c7b8a43/hips/hip-0006.md
 func download(p common.Printer, art oci.ArtifactAccess, path string, fs vfs.FileSystem) (chart, prov string, err error) {
 	var finalize finalizer.Finalizer
 	defer finalize.FinalizeWithErrorPropagation(&err)
 
 	m := art.ManifestAccess()
 	if m == nil {
-		return "", "", errors.Newf("artifact is no image manifest")
+		return "", "", errors.New("artifact is no image manifest")
 	}
-	if len(m.GetDescriptor().Layers) < 1 {
-		return "", "", errors.Newf("no layers found")
+	desc := m.GetDescriptor()
+
+	chartDesc, err := findLayer(desc.Layers, helmregistry.ChartLayerMediaType)
+	if err != nil {
+		return "", "", fmt.Errorf("no valid chart layer found: %w", err)
 	}
 
-	blob, err := m.GetBlob(m.GetDescriptor().Layers[0].Digest)
+	chartBlob, err := m.GetBlob(chartDesc.Digest)
 	if err != nil {
-		return "", "", err
+		return "", "", fmt.Errorf("no valid chart blob found: %w", err)
 	}
-	finalize.Close(blob)
+	finalize.Close(chartBlob)
 
 	chart = AssureArchiveSuffix(path)
-	err = write(p, blob, chart, fs)
-	if err != nil {
+	if err := write(p, chartBlob, chart, fs); err != nil {
 		return "", "", err
 	}
-	if len(m.GetDescriptor().Layers) > 1 {
-		prov = chart[:len(chart)-3] + "prov"
-		blob, err := m.GetBlob(m.GetDescriptor().Layers[1].Digest)
+
+	// Optional provenance layer, if present, add it separately
+	if provDesc, err := findLayer(desc.Layers, helmregistry.ProvLayerMediaType); err == nil {
+		provBlob, err := m.GetBlob(provDesc.Digest)
 		if err != nil {
 			return "", "", err
 		}
-		err = write(p, blob, path, fs)
-		if err != nil {
+		finalize.Close(provBlob)
+
+		prov = chart[:len(chart)-3] + "prov"
+		if err := write(p, provBlob, path, fs); err != nil {
 			return "", "", err
 		}
+	} else if !errors.Is(err, ErrNoMatchingLayer) { // Ignore if no provenance layer is found, because its optional.
+		return "", "", err
+	}
+
+	return chart, prov, nil
+}
+
+func findLayer(layers []ocispec.Descriptor, mediaType string) (*ocispec.Descriptor, error) {
+	var candidates []*ocispec.Descriptor
+
+	for _, l := range layers {
+		if mime.BaseType(l.MediaType) == mime.BaseType(mediaType) {
+			candidates = append(candidates, &l)
+		}
+	}
+
+	switch {
+	case len(candidates) > 1:
+		return nil, fmt.Errorf("%w: %s", ErrMultipleMatchingLayers, mediaType)
+	case len(candidates) == 0:
+		return nil, fmt.Errorf("%w: %s", ErrNoMatchingLayer, mediaType)
+	default:
+		return candidates[0], nil
 	}
-	return chart, prov, err
 }
 
 func write(p common.Printer, blob blobaccess.DataReader, path string, fs vfs.FileSystem) (err error) {