From 40f45f2d6b0c7fdece317f268cfc018534f4b763 Mon Sep 17 00:00:00 2001 From: waterWang Date: Thu, 27 Aug 2026 16:39:13 +0000 Subject: [PATCH] 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 --- pkg/registry/client.go | 53 ++++++++++++++++++++++++----------- pkg/registry/client_test.go | 56 +++++++++++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+), 16 deletions(-) diff --git a/pkg/registry/client.go b/pkg/registry/client.go index 4c7b9a6c8..b55ca3773 100644 --- a/pkg/registry/client.go +++ b/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. diff --git a/pkg/registry/client_test.go b/pkg/registry/client_test.go index 21604ce7e..dbcbfe282 100644 --- a/pkg/registry/client_test.go +++ b/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) + } + } +}