Merge eb0820b319 into 8a9993eff0

internal/chart/v3/loader/load.go

@@ -72,7 +72,6 @@ func Load(name string) (*chart.Chart, error) {
 func LoadFiles(files []*archive.BufferedFile) (*chart.Chart, error) {
 	c := new(chart.Chart)
 	subcharts := make(map[string][]*archive.BufferedFile)
-	var subChartsKeys []string
 
 	// do not rely on assumed ordering of files in the chart and crash
 	// if Chart.yaml was not coming early enough to initialize metadata
@@ -124,9 +123,6 @@ func LoadFiles(files []*archive.BufferedFile) (*chart.Chart, error) {
 
 			fname := strings.TrimPrefix(f.Name, "charts/")
 			cname, _, _ := strings.Cut(fname, "/")
-			if slices.Index(subChartsKeys, cname) == -1 {
-				subChartsKeys = append(subChartsKeys, cname)
-			}
 			subcharts[cname] = append(subcharts[cname], &archive.BufferedFile{Name: fname, ModTime: f.ModTime, Data: f.Data})
 		default:
 			c.Files = append(c.Files, &common.File{Name: f.Name, ModTime: f.ModTime, Data: f.Data})
@@ -141,7 +137,9 @@ func LoadFiles(files []*archive.BufferedFile) (*chart.Chart, error) {
 		return c, err
 	}
 
-	for n, files := range subcharts {
+    // Iterate in sorted key order, not random Go map iteration order, for reproducible tarballs when saving
+	for _, n := range slices.Sorted(maps.Keys(subcharts)) {
+		files := subcharts[n]
 		var sc *chart.Chart
 		var err error
 		switch {

internal/chart/v3/loader/load_test.go

@@ -289,6 +289,36 @@ icon: https://example.com/64x64.png
 	assert.Empty(t, text.String(), "Expected no message to Stderr, got %s", text.String())
 }
 
+func TestLoadFilesSubchartOrderIsDeterministic(t *testing.T) {
+	modTime := time.Now()
+	files := []*archive.BufferedFile{
+		{
+			Name:    "Chart.yaml",
+			ModTime: modTime,
+			Data:    []byte("apiVersion: v3\nname: frobnitz\nversion: \"1.2.3\"\n"),
+		},
+	}
+	for _, name := range []string{"delta", "bravo", "echo", "alpine", "charlie"} {
+		files = append(files, &archive.BufferedFile{
+			Name:    "charts/" + name + "/Chart.yaml",
+			ModTime: modTime,
+			Data:    []byte("apiVersion: v3\nname: " + name + "\nversion: \"0.1.0\"\n"),
+		})
+	}
+	want := []string{"alpine", "bravo", "charlie", "delta", "echo"}
+
+	for i := range 20 {
+		c, err := LoadFiles(files)
+		require.NoError(t, err, "Expected good files to be loaded")
+
+		got := make([]string, 0, len(c.Dependencies()))
+		for _, dep := range c.Dependencies() {
+			got = append(got, dep.Name())
+		}
+		require.Equal(t, want, got, "subchart order must not depend on map iteration order (load %d)", i)
+	}
+}
+
 // Packaging the chart on a Windows machine will produce an
 // archive that has \\ as delimiters. Test that we support these archives
 func TestLoadFileBackslash(t *testing.T) {

pkg/chart/v2/loader/load.go

@@ -26,6 +26,7 @@ import (
 	"maps"
 	"os"
 	"path/filepath"
+	"slices"
 	"strings"
 
 	utilyaml "k8s.io/apimachinery/pkg/util/yaml"
@@ -169,7 +170,9 @@ func LoadFiles(files []*archive.BufferedFile) (*chart.Chart, error) {
 		return c, err
 	}
 
-	for n, files := range subcharts {
+    // Iterate in sorted key order, not random Go map iteration order, for reproducible tarballs when saving
+	for _, n := range slices.Sorted(maps.Keys(subcharts)) {
+		files := subcharts[n]
 		var sc *chart.Chart
 		var err error
 		switch {

pkg/chart/v2/loader/load_test.go

@@ -331,6 +331,36 @@ icon: https://example.com/64x64.png
 	assert.Empty(t, text.String(), "Expected no message to Stderr, got %s", text.String())
 }
 
+func TestLoadFilesSubchartOrderIsDeterministic(t *testing.T) {
+	modTime := time.Now()
+	files := []*archive.BufferedFile{
+		{
+			Name:    "Chart.yaml",
+			ModTime: modTime,
+			Data:    []byte("apiVersion: v1\nname: frobnitz\nversion: \"1.2.3\"\n"),
+		},
+	}
+	for _, name := range []string{"delta", "bravo", "echo", "alpine", "charlie"} {
+		files = append(files, &archive.BufferedFile{
+			Name:    "charts/" + name + "/Chart.yaml",
+			ModTime: modTime,
+			Data:    []byte("apiVersion: v1\nname: " + name + "\nversion: \"0.1.0\"\n"),
+		})
+	}
+	want := []string{"alpine", "bravo", "charlie", "delta", "echo"}
+
+	for i := range 20 {
+		c, err := LoadFiles(files)
+		require.NoError(t, err, "Expected good files to be loaded")
+
+		got := make([]string, 0, len(c.Dependencies()))
+		for _, dep := range c.Dependencies() {
+			got = append(got, dep.Name())
+		}
+		require.Equal(t, want, got, "subchart order must not depend on map iteration order (load %d)", i)
+	}
+}
+
 // Packaging the chart on a Windows machine will produce an
 // archive that has \\ as delimiters. Test that we support these archives
 func TestLoadFileBackslash(t *testing.T) {

pkg/chart/v2/util/save_test.go

@@ -34,6 +34,7 @@ import (
 	"time"
 
 	"helm.sh/helm/v4/pkg/chart/common"
+	"helm.sh/helm/v4/pkg/chart/loader/archive"
 	chart "helm.sh/helm/v4/pkg/chart/v2"
 	"helm.sh/helm/v4/pkg/chart/v2/loader"
 
@@ -367,6 +368,46 @@ func TestRepeatableSave(t *testing.T) {
 	}
 }
 
+func TestRepeatableSaveWithSubcharts(t *testing.T) {
+	modTime := time.Now()
+	files := []*archive.BufferedFile{
+		{
+			Name:    "Chart.yaml",
+			ModTime: modTime,
+			Data:    []byte("apiVersion: v1\nname: frobnitz\nversion: \"1.2.3\"\n"),
+		},
+	}
+	for _, name := range []string{"delta", "bravo", "echo", "alpine", "charlie"} {
+		files = append(files, &archive.BufferedFile{
+			Name:    "charts/" + name + "/Chart.yaml",
+			ModTime: modTime,
+			Data:    []byte("apiVersion: v1\nname: " + name + "\nversion: \"0.1.0\"\n"),
+		})
+	}
+
+	epoch := time.Unix(0, 0).UTC()
+	save := func() string {
+		t.Helper()
+		c, err := loader.LoadFiles(files)
+		require.NoError(t, err, "Failed to load files")
+		require.Len(t, c.Dependencies(), 5, "Expected all subcharts to load")
+
+		// Pin timestamps, as SOURCE_DATE_EPOCH does, so entry order is the only thing left moving.
+		c.StampModTimes(epoch)
+
+		where, err := Save(c, t.TempDir())
+		require.NoError(t, err, "Failed to save")
+		sum, err := sha256Sum(where)
+		require.NoError(t, err, "Failed to check shasum")
+		return sum
+	}
+
+	want := save()
+	for i := range 5 {
+		require.Equal(t, want, save(), "Save() is not repeatable across loads (iteration %d)", i)
+	}
+}
+
 func sha256Sum(filePath string) (string, error) {
 	f, err := os.Open(filePath)
 	if err != nil {