diff --git a/internal/controller/ocirepository_controller.go b/internal/controller/ocirepository_controller.go index ebde8aa2d..c9600e402 100644 --- a/internal/controller/ocirepository_controller.go +++ b/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 diff --git a/internal/controller/ocirepository_controller_test.go b/internal/controller/ocirepository_controller_test.go index 0755ff8c7..7c4dc7a01 100644 --- a/internal/controller/ocirepository_controller_test.go +++ b/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,6 +2893,8 @@ 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()) @@ -2899,13 +2902,14 @@ func TestOCIRepository_getArtifactRef(t *testing.T) { 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", @@ -2913,7 +2917,7 @@ func TestOCIRepository_getArtifactRef(t *testing.T) { 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", @@ -2921,15 +2925,29 @@ func TestOCIRepository_getArtifactRef(t *testing.T) { 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) }) } } diff --git a/internal/controller/testdata/podinfo/podinfo-6.2.1.tar b/internal/controller/testdata/podinfo/podinfo-6.2.1.tar new file mode 100644 index 000000000..09616c2df Binary files /dev/null and b/internal/controller/testdata/podinfo/podinfo-6.2.1.tar differ diff --git a/internal/controller/testdata/podinfo/podinfo-6.2.1_ref.1234567.tar b/internal/controller/testdata/podinfo/podinfo-6.2.1_ref.1234567.tar new file mode 100644 index 000000000..09616c2df Binary files /dev/null and b/internal/controller/testdata/podinfo/podinfo-6.2.1_ref.1234567.tar differ