fix(registry): preserve original tag strings in Tags (#32582)

Sort by semver precedence but return the original tag as stored in the
registry, not the normalized form produced by semver.Version.String().
This is particularly important for tags using Helm's underscore convention
(#10166): a tag like 1.1.17_meta was being returned as 1.1.17+meta, which
is not a valid OCI reference and cannot be pulled.

Signed-off-by: waterWang <waterWang@users.noreply.github.com>

pkg/registry/client.go

@@ -791,33 +791,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)
+		}
+	}
+}