Merge 40f45f2d6b into 2b128dcd27

pkg/registry/client.go

@@ -796,33 +796,54 @@ func (c *Client) Tags(ref string) ([]string, error) {
 	repository.PlainHTTP = c.plainHTTP
 	repository.Client = c.authorizer
 
-	var tagVersions []*semver.Version
-	err = repository.Tags(ctx, "", func(tags []string) error {
-		for _, tag := range tags {
-			// Change underscore (_) back to plus (+) for Helm
-			// See https://github.com/helm/helm/issues/10166
-			tagVersion, err := semver.StrictNewVersion(strings.ReplaceAll(tag, "_", "+"))
-			if err == nil {
-				tagVersions = append(tagVersions, tagVersion)
-			}
-		}
-
+	var tags []string
+	err = repository.Tags(ctx, "", func(repoTags []string) error {
+		tags = sortSemverTags(repoTags)
 		return nil
 	})
 	if err != nil {
 		return nil, err
 	}
 
-	// Sort the collection
-	sort.Sort(sort.Reverse(semver.Collection(tagVersions)))
+	return tags, nil
+}
+
+// sortSemverTags sorts the given tags by semantic version precedence (highest
+// first) and returns them preserving the original tag strings. The original
+// strings are kept because Helm's underscore convention (#10166) means a tag
+// like 1.1.17_meta is stored in the registry verbatim; normalizing it back to
+// 1.1.17+meta would produce a reference that does not exist in the registry.
+func sortSemverTags(tags []string) []string {
+	type parsedTag struct {
+		tag string
+		v   *semver.Version
+	}
+
+	var tagVersions []parsedTag
+	for _, tag := range tags {
+		// Change underscore (_) back to plus (+) for Helm
+		// See https://github.com/helm/helm/issues/10166
+		tagVersion, err := semver.StrictNewVersion(strings.ReplaceAll(tag, "_", "+"))
+		if err == nil {
+			tagVersions = append(tagVersions, parsedTag{tag: tag, v: tagVersion})
+		} else {
+			slog.Debug("Skipping tag that is not a valid semantic version", "tag", tag, "error", err)
+		}
+	}
+
+	// Sort the collection by semantic version precedence (highest first),
+	// while preserving the original tag string so it remains a valid OCI reference.
+	sort.Slice(tagVersions, func(i, j int) bool {
+		return tagVersions[i].v.GreaterThan(tagVersions[j].v)
+	})
 
-	tags := make([]string, len(tagVersions))
+	tagsOut := make([]string, len(tagVersions))
 
 	for iTv, tv := range tagVersions {
-		tags[iTv] = tv.String()
+		tagsOut[iTv] = tv.tag
 	}
 
-	return tags, nil
+	return tagsOut
 }
 
 // Resolve a reference to a descriptor.

pkg/registry/client_test.go

@@ -24,6 +24,7 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/Masterminds/semver/v3"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -153,3 +154,58 @@ func TestWarnIfHostHasPath(t *testing.T) {
 		})
 	}
 }
+
+// TestSortSemverTags verifies that sortSemverTags returns the original tag
+// strings (preserving Helm's underscore convention for build metadata, #10166)
+// while ordering them by semantic version precedence (highest first).
+func TestSortSemverTags(t *testing.T) {
+	tests := []struct {
+		name string
+		in   []string
+		want []string
+	}{
+		{
+			name: "underscore convention preserved",
+			in:   []string{"1.1.17_meta", "1.2.0", "1.1.16", "2.0.0", "1.1.17"},
+			want: []string{"2.0.0", "1.2.0", "1.1.17_meta", "1.1.17", "1.1.16"},
+		},
+		{
+			name: "plain semver tags unchanged",
+			in:   []string{"1.0.0", "2.1.0", "1.5.0", "3.0.0"},
+			want: []string{"3.0.0", "2.1.0", "1.5.0", "1.0.0"},
+		},
+		{
+			name: "pre-release ordering",
+			in:   []string{"1.0.0", "1.0.0-beta", "1.0.0-alpha", "1.0.1"},
+			want: []string{"1.0.1", "1.0.0", "1.0.0-beta", "1.0.0-alpha"},
+		},
+		{
+			name: "invalid tags dropped",
+			in:   []string{"1.0.0", "not-a-version", "2.0.0", "1.5.0"},
+			want: []string{"2.0.0", "1.5.0", "1.0.0"},
+		},
+		{
+			name: "mixed underscore and prerelease",
+			in:   []string{"1.2.0_rc1", "1.1.0", "1.2.0", "2.0.0_rc1"},
+			want: []string{"2.0.0_rc1", "1.2.0", "1.2.0_rc1", "1.1.0"},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := sortSemverTags(tt.in)
+			assert.Equal(t, tt.want, got)
+		})
+	}
+}
+
+// TestSortSemverTagsValidReferences ensures every returned tag is a valid OCI
+// reference string (i.e. it was preserved verbatim, not normalized).
+func TestSortSemverTagsValidReferences(t *testing.T) {
+	in := []string{"1.1.17_meta", "1.2.0", "1.1.17"}
+	for _, tag := range sortSemverTags(in) {
+		if _, err := semver.StrictNewVersion(strings.ReplaceAll(tag, "_", "+")); err != nil {
+			t.Errorf("returned tag %q is not a valid semantic version", tag)
+		}
+	}
+}