Merge pull request #251 from nicholasSUSE/fix-rc-ordering

Improve version ordering, fixing RCs

Author: nicholasSUSE

main.go

@@ -390,7 +390,7 @@ func main() {
 	newChartFlag := cli.BoolFlag{
 		Name: "new-chart",
 		Usage: `Usage:
-			-new-chart=<false or true>
+			--new-chart=<false or true>
 		`,
 		Required:    false,
 		Destination: &NewChart,
@@ -1149,6 +1149,7 @@ func chartBump(c *cli.Context) {
 	logger.Log(ctx, slog.LevelInfo, "", slog.String("branch", Branch))
 	logger.Log(ctx, slog.LevelInfo, "", slog.String("overrideVersion", OverrideVersion))
 	logger.Log(ctx, slog.LevelInfo, "", slog.Bool("multi-RC", MultiRC))
+	logger.Log(ctx, slog.LevelInfo, "", slog.Bool("new-chart", NewChart))
 
 	if CurrentPackage == "" || Branch == "" || OverrideVersion == "" {
 		logger.Fatal(ctx, fmt.Sprintf("must provide values for CurrentPackage[%s], Branch[%s], and OverrideVersion[%s]",

pkg/helm/helm.go

@@ -6,7 +6,11 @@ import (
 	"fmt"
 	"log/slog"
 	"os"
+	"sort"
+	"strconv"
+	"strings"
 
+	"github.com/Masterminds/semver"
 	"github.com/go-git/go-billy/v5"
 	"github.com/rancher/charts-build-scripts/pkg/filesystem"
 	"github.com/rancher/charts-build-scripts/pkg/logger"
@@ -41,8 +45,8 @@ func CreateOrUpdateHelmIndex(ctx context.Context, rootFs billy.Filesystem) error
 	}
 
 	// Sort entries to ensure consistent ordering
-	helmIndexFile.SortEntries()
-	newHelmIndexFile.SortEntries()
+	SortVersions(helmIndexFile)
+	SortVersions(newHelmIndexFile)
 
 	// Update index
 	helmIndexFile, upToDate := UpdateIndex(ctx, helmIndexFile, newHelmIndexFile)
@@ -128,3 +132,85 @@ func OpenIndexYaml(ctx context.Context, rootFs billy.Filesystem) (*helmRepo.Inde
 
 	return helmRepo.LoadIndexFile(helmIndexFilePath)
 }
+
+// SortVersions sorts chart versions with custom RC handling
+func SortVersions(index *helmRepo.IndexFile) {
+	for _, versions := range index.Entries {
+		sort.Slice(versions, func(i, j int) bool {
+			return compareVersions(versions[i].Version, versions[j].Version)
+		})
+	}
+}
+
+// compareVersions compares two version strings for sorting
+// Returns true if versionA should come before versionB (descending order)
+func compareVersions(versionA, versionB string) bool {
+	// Parse both versions
+	baseA, rcA, isRCA := parseVersionWithRC(versionA)
+	baseB, rcB, isRCB := parseVersionWithRC(versionB)
+
+	// Parse base versions using semver
+	semverA, errA := semver.NewVersion(baseA)
+	semverB, errB := semver.NewVersion(baseB)
+
+	if errA != nil {
+		return false // push invalid to end
+	}
+	if errB != nil {
+		return true // push invalid to end
+	}
+
+	// If base versions are different, use semver comparison (descending)
+	if !semverA.Equal(semverB) {
+		return semverA.GreaterThan(semverB)
+	}
+
+	// Same base version - handle RC logic
+	// Stable (non-RC) should come first
+	if !isRCA && isRCB {
+		return true // A is stable, B is RC - A comes first
+	}
+	if isRCA && !isRCB {
+		return false // A is RC, B is stable - B comes first
+	}
+
+	// Both are RCs - higher RC number comes first (descending)
+	if isRCA && isRCB {
+		return rcA > rcB
+	}
+
+	// Both are stable with same base version - they're equal
+	return false
+}
+
+// parseVersionWithRC extracts the base version and RC number from a version string
+// Example: "108.0.0+up0.9.0-rc.1" returns ("108.0.0+up0.9.0", 1, true)
+func parseVersionWithRC(version string) (baseVersion string, rcNumber int, isRC bool) {
+	// Split by '+' to separate version from build metadata
+	parts := strings.Split(version, "+")
+	if len(parts) != 2 {
+		return version, 0, false
+	}
+
+	baseVersionNum := parts[0]
+	buildMetadata := parts[1]
+
+	// Check if build metadata contains RC
+	if !strings.Contains(buildMetadata, "-rc.") {
+		return version, 0, false
+	}
+
+	// Extract RC number
+	rcParts := strings.Split(buildMetadata, "-rc.")
+	if len(rcParts) != 2 {
+		return version, 0, false
+	}
+
+	rcNum, err := strconv.Atoi(rcParts[1])
+	if err != nil {
+		return version, 0, false
+	}
+
+	// Return base version with the non-RC part of build metadata
+	return baseVersionNum + "+" + rcParts[0], rcNum, true
+}

pkg/helm/helm_test.go

@@ -0,0 +1,97 @@
+package helm
+
+import (
+	"testing"
+
+	helmRepo "helm.sh/helm/v3/pkg/repo"
+	"helm.sh/helm/v3/pkg/chart"
+)
+
+func TestSortVersions(t *testing.T) {
+	tests := []struct {
+		name     string
+		input    helmRepo.ChartVersions
+		expected []string // expected order of versions
+	}{
+		{
+			name: "stable versions only - should sort descending",
+			input: helmRepo.ChartVersions{
+				{Metadata: &chart.Metadata{Version: "108.0.0+up0.9.0"}},
+				{Metadata: &chart.Metadata{Version: "108.0.2+up0.9.2"}},
+				{Metadata: &chart.Metadata{Version: "108.0.1+up0.9.1"}},
+			},
+			expected: []string{
+				"108.0.2+up0.9.2",
+				"108.0.1+up0.9.1",
+				"108.0.0+up0.9.0",
+			},
+		},
+		{
+			name: "stable + RCs with same base - stable first, then RCs descending",
+			input: helmRepo.ChartVersions{
+				{Metadata: &chart.Metadata{Version: "108.0.0+up0.9.0-rc.1"}},
+				{Metadata: &chart.Metadata{Version: "108.0.0+up0.9.0"}},
+				{Metadata: &chart.Metadata{Version: "108.0.0+up0.9.0-rc.2"}},
+			},
+			expected: []string{
+				"108.0.0+up0.9.0",
+				"108.0.0+up0.9.0-rc.2",
+				"108.0.0+up0.9.0-rc.1",
+			},
+		},
+		{
+			name: "RCs only - should sort descending by RC number",
+			input: helmRepo.ChartVersions{
+				{Metadata: &chart.Metadata{Version: "108.0.0+up0.9.0-rc.1"}},
+				{Metadata: &chart.Metadata{Version: "108.0.0+up0.9.0-rc.3"}},
+				{Metadata: &chart.Metadata{Version: "108.0.0+up0.9.0-rc.2"}},
+			},
+			expected: []string{
+				"108.0.0+up0.9.0-rc.3",
+				"108.0.0+up0.9.0-rc.2",
+				"108.0.0+up0.9.0-rc.1",
+			},
+		},
+		{
+			name: "mixed base versions with RCs - sort by semver first",
+			input: helmRepo.ChartVersions{
+				{Metadata: &chart.Metadata{Version: "108.0.0+up0.9.0-rc.1"}},
+				{Metadata: &chart.Metadata{Version: "109.0.0+up0.10.0"}},
+				{Metadata: &chart.Metadata{Version: "108.0.0+up0.9.0"}},
+				{Metadata: &chart.Metadata{Version: "109.0.0+up0.10.0-rc.1"}},
+			},
+			expected: []string{
+				"109.0.0+up0.10.0",
+				"109.0.0+up0.10.0-rc.1",
+				"108.0.0+up0.9.0",
+				"108.0.0+up0.9.0-rc.1",
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Create mock IndexFile
+			index := &helmRepo.IndexFile{
+				Entries: map[string]helmRepo.ChartVersions{
+					"test-chart": tt.input,
+				},
+			}
+
+			// Run the sort function
+			SortVersions(index)
+
+			// Check the order
+			sorted := index.Entries["test-chart"]
+			if len(sorted) != len(tt.expected) {
+				t.Fatalf("expected %d versions, got %d", len(tt.expected), len(sorted))
+			}
+
+			for i, expectedVersion := range tt.expected {
+				if sorted[i].Version != expectedVersion {
+					t.Errorf("at index %d: expected %s, got %s", i, expectedVersion, sorted[i].Version)
+				}
+			}
+		})
+	}
+}

pkg/lifecycle/parser.go

@@ -3,10 +3,8 @@ package lifecycle
 import (
 	"context"
 	"fmt"
-	"sort"
 	"strings"
 
-	"github.com/Masterminds/semver"
 	"github.com/go-git/go-billy/v5"
 	helmRepo "helm.sh/helm/v3/pkg/repo"
 )
@@ -103,19 +101,3 @@ func (ld *Dependencies) populateAssetsVersionsPath(ctx context.Context) error {
 	// Now fileNames slice contains the names of all files in the directories
 	return nil
 }
-
-// sortAssetsVersions will convert to semver and
-// sort the assets for each key in the assetsVersionsMap
-func (ld *Dependencies) sortAssetsVersions() {
-	// Iterate over the map and sort the assets for each key
-	for k, assets := range ld.AssetsVersionsMap {
-		sort.Slice(assets, func(i, j int) bool {
-			vi, _ := semver.NewVersion(assets[i].Version)
-			vj, _ := semver.NewVersion(assets[j].Version)
-			return vi.LessThan(vj)
-		})
-		ld.AssetsVersionsMap[k] = assets
-	}
-
-	return
-}

pkg/lifecycle/parser_test.go

@@ -106,34 +106,3 @@ func Test_populateAssetsVersionsPath(t *testing.T) {
 		assert.Error(t, err, "populateAssetsVersionsPath should have returned an error")
 	})
 }
-
-func Test_sortAssetsVersions(t *testing.T) {
-	// Arrange
-	dependency := &Dependencies{
-		AssetsVersionsMap: map[string][]Asset{
-			"chart1": {
-				{Version: "1.0.0"},
-				{Version: "0.1.0"},
-				{Version: "0.0.1"},
-			},
-			"chart2": {
-				{Version: "2.0.0"},
-				{Version: "1.1.0"},
-				{Version: "1.0.1"},
-			},
-		},
-	}
-
-	// Act
-	dependency.sortAssetsVersions()
-
-	// Assertions
-	assert.Equal(t, len(dependency.AssetsVersionsMap), 2, "Expected 2 charts in the assetsVersionsMap")
-	assert.Equal(t, dependency.AssetsVersionsMap["chart1"][0].Version, "0.0.1", "Expected 0.0.1")
-	assert.Equal(t, dependency.AssetsVersionsMap["chart1"][1].Version, "0.1.0", "Expected 0.1.0")
-	assert.Equal(t, dependency.AssetsVersionsMap["chart1"][2].Version, "1.0.0", "Expected 1.0.0")
-
-	assert.Equal(t, dependency.AssetsVersionsMap["chart2"][0].Version, "1.0.1", "Expected 1.0.1")
-	assert.Equal(t, dependency.AssetsVersionsMap["chart2"][1].Version, "1.1.0", "Expected 1.1.0")
-	assert.Equal(t, dependency.AssetsVersionsMap["chart2"][2].Version, "2.0.0", "Expected 2.0.0")
-}