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) + } + } +}