Support Helm semver encoding in OCI repositories

internal/controller/ocirepository_controller.go

@@ -40,6 +40,7 @@ import (
 	"github.com/google/go-containerregistry/pkg/v1/remote"
 	"github.com/notaryproject/notation-go/verifier/trustpolicy"
 	"github.com/sigstore/cosign/v3/pkg/cosign"
+	"helm.sh/helm/v4/pkg/registry"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
@@ -871,7 +872,7 @@ func (r *OCIRepositoryReconciler) getArtifactRef(obj *sourcev1.OCIRepository, op
 		}
 
 		if obj.Spec.Reference.SemVer != "" {
-			return r.getTagBySemver(repo, obj.Spec.Reference.SemVer, filterTags(obj.Spec.Reference.SemverFilter), options)
+			return r.getTagBySemver(repo, obj.Spec.Reference.SemVer, filterTags(obj.Spec.Reference.SemverFilter), obj.GetLayerMediaType(), options)
 		}
 
 		if obj.Spec.Reference.Tag != "" {
@@ -884,7 +885,7 @@ func (r *OCIRepositoryReconciler) getArtifactRef(obj *sourcev1.OCIRepository, op
 
 // getTagBySemver call the remote container registry, fetches all the tags from the repository,
 // and returns the latest tag according to the semver expression.
-func (r *OCIRepositoryReconciler) getTagBySemver(repo name.Repository, exp string, filter filterFunc, options []remote.Option) (name.Reference, error) {
+func (r *OCIRepositoryReconciler) getTagBySemver(repo name.Repository, exp string, filter filterFunc, mediaType string, options []remote.Option) (name.Reference, error) {
 	tags, err := remote.List(repo, options...)
 	if err != nil {
 		return nil, err
@@ -901,8 +902,9 @@ func (r *OCIRepositoryReconciler) getTagBySemver(repo name.Repository, exp strin
 	}
 
 	var matchingVersions []*semver.Version
-	for _, t := range validTags {
-		v, err := version.ParseVersion(t)
+	for _, ociTag := range validTags {
+		semVerTag := convertOCIToSemVerTag(ociTag, mediaType)
+		v, err := version.ParseVersion(semVerTag)
 		if err != nil {
 			continue
 		}
@@ -916,8 +918,42 @@ func (r *OCIRepositoryReconciler) getTagBySemver(repo name.Repository, exp strin
 		return nil, fmt.Errorf("no match found for semver: %s", exp)
 	}
 
+	// Find the latest SemVer.
 	sort.Sort(sort.Reverse(semver.Collection(matchingVersions)))
-	return repo.Tag(matchingVersions[0].Original()), nil
+	semVerTag := matchingVersions[0].Original()
+
+	// Convert the latest SemVer to an OCI tag and return the reference.
+	ociTag := convertSemVerToOCITag(semVerTag, mediaType)
+	return repo.Tag(ociTag), nil
+}
+
+// convertSemVerToOCITag converts a SemVer tag to an OCI tag
+// according to rules defined by the media type.
+//
+// For OCI Helm charts, the conversion is mapping `+` to `_`,
+// because `+` is not permitted in OCI tags, while `_` is not
+// permitted in SemVer. Each character not being permitted in
+// one of the two sides establishes a perfect bijection between,
+// which then makes the mapping implemented by Helm (and honored
+// here) completely safe.
+func convertSemVerToOCITag(semVer, mediaType string) string {
+	if mediaType == registry.ChartLayerMediaType {
+		return strings.ReplaceAll(semVer, "+", "_")
+	}
+	return semVer
+}
+
+// convertOCIToSemVerTag converts an OCI tag to a SemVer tag
+// according to rules defined by the media type.
+//
+// For OCI Helm charts, the conversion is mapping `_` to `+`,
+// see the comment above on convertSemVerToOCITag for the
+// mapping in the opposite direction and rationale.
+func convertOCIToSemVerTag(ociTag, mediaType string) string {
+	if mediaType == registry.ChartLayerMediaType {
+		return strings.ReplaceAll(ociTag, "_", "+")
+	}
+	return ociTag
 }
 
 // keychain generates the credential keychain based on the resource

internal/controller/ocirepository_controller_test.go

@@ -43,6 +43,7 @@ import (
 	"github.com/notaryproject/notation-go/signer"
 	"github.com/notaryproject/notation-go/verifier/trustpolicy"
 	. "github.com/onsi/gomega"
+	"github.com/onsi/gomega/types"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 	coptions "github.com/sigstore/cosign/v3/cmd/cosign/cli/options"
 	"github.com/sigstore/cosign/v3/cmd/cosign/cli/sign"
@@ -2892,44 +2893,61 @@ func TestOCIRepository_getArtifactRef(t *testing.T) {
 		"6.1.5",
 		"6.1.6-rc.1",
 		"6.1.6",
+		"6.2.1_ref.1234567", // Version 6.2.1+ref.1234567, encoded as a tag
+		"6.2.1",             // Version 6.2.1, same precedence as 6.2.1, per semver rule 10
 	)
 	g.Expect(err).ToNot(HaveOccurred())
 
 	tests := []struct {
 		name      string
 		url       string
 		reference *sourcev1.OCIRepositoryRef
+		selector  *sourcev1.OCILayerSelector
 		wantErr   bool
-		want      string
+		want      types.GomegaMatcher
 	}{
 		{
 			name: "valid url with no reference",
 			url:  "oci://ghcr.io/stefanprodan/charts",
-			want: "ghcr.io/stefanprodan/charts:latest",
+			want: Equal("ghcr.io/stefanprodan/charts:latest"),
 		},
 		{
 			name: "valid url with tag reference",
 			url:  "oci://ghcr.io/stefanprodan/charts",
 			reference: &sourcev1.OCIRepositoryRef{
 				Tag: "6.1.6",
 			},
-			want: "ghcr.io/stefanprodan/charts:6.1.6",
+			want: Equal("ghcr.io/stefanprodan/charts:6.1.6"),
 		},
 		{
 			name: "valid url with digest reference",
 			url:  "oci://ghcr.io/stefanprodan/charts",
 			reference: &sourcev1.OCIRepositoryRef{
 				Digest: imgs["6.1.6"].digest.String(),
 			},
-			want: "ghcr.io/stefanprodan/charts@" + imgs["6.1.6"].digest.String(),
+			want: Equal("ghcr.io/stefanprodan/charts@" + imgs["6.1.6"].digest.String()),
 		},
 		{
 			name: "valid url with semver reference",
 			url:  fmt.Sprintf("oci://%s/podinfo", server.registryHost),
 			reference: &sourcev1.OCIRepositoryRef{
-				SemVer: ">= 6.1.6",
+				SemVer: "~6.1.x",
+			},
+			want: Equal(server.registryHost + "/podinfo:6.1.6"),
+		},
+		{
+			name: "valid url with semver reference and build identifier",
+			url:  fmt.Sprintf("oci://%s/podinfo", server.registryHost),
+			reference: &sourcev1.OCIRepositoryRef{
+				SemVer: ">= 6.2.0",
 			},
-			want: server.registryHost + "/podinfo:6.1.6",
+			selector: &sourcev1.OCILayerSelector{
+				MediaType: "application/vnd.cncf.helm.chart.content.v1.tar+gzip",
+			},
+			// Build info does not have a defined sort order in SemVer, so these
+			// two are equivalently new.
+			want: Or(Equal(server.registryHost+"/podinfo:6.2.1_ref.1234567"),
+				Equal(server.registryHost+"/podinfo:6.2.1")),
 		},
 		{
 			name:    "invalid url without oci prefix",
@@ -2943,7 +2961,7 @@ func TestOCIRepository_getArtifactRef(t *testing.T) {
 				SemVer:       ">= 6.1.x-0",
 				SemverFilter: ".*-rc.*",
 			},
-			want: server.registryHost + "/podinfo:6.1.6-rc.1",
+			want: Equal(server.registryHost + "/podinfo:6.1.6-rc.1"),
 		},
 		{
 			name: "valid url with semver filter and unexisting version",
@@ -2984,6 +3002,9 @@ func TestOCIRepository_getArtifactRef(t *testing.T) {
 			if tt.reference != nil {
 				obj.Spec.Reference = tt.reference
 			}
+			if tt.selector != nil {
+				obj.Spec.LayerSelector = tt.selector
+			}
 
 			opts := makeRemoteOptions(ctx, makeTransport(true), authn.DefaultKeychain, nil)
 			got, err := r.getArtifactRef(obj, opts)
@@ -2992,7 +3013,7 @@ func TestOCIRepository_getArtifactRef(t *testing.T) {
 				return
 			}
 			g.Expect(err).ToNot(HaveOccurred())
-			g.Expect(got.String()).To(Equal(tt.want))
+			g.Expect(got.String()).To(tt.want)
 		})
 	}
 }