From eb0820b31964e3a3a47e4d3ab67a5946e7210c51 Mon Sep 17 00:00:00 2001 From: "samuel.wright" Date: Sun, 20 Sep 2026 10:49:54 +0200 Subject: [PATCH] fix: reproducible chart archive builds with multiple subcharts Subcharts are currently loaded in a random order, leading to randomly ordered subcharts in the tarball when running `helm package`. This means that charts with multiple subcharts cannot be reliably reproduced. This was nearly fixed in #31323, which added the `subChartsKeys` slice but only for a dedup check. I've added tests that show that this solution works, which fail when run against `main`. Signed-off-by: samuel.wright --- internal/chart/v3/loader/load.go | 8 ++---- internal/chart/v3/loader/load_test.go | 30 ++++++++++++++++++++ pkg/chart/v2/loader/load.go | 5 +++- pkg/chart/v2/loader/load_test.go | 30 ++++++++++++++++++++ pkg/chart/v2/util/save_test.go | 41 +++++++++++++++++++++++++++ 5 files changed, 108 insertions(+), 6 deletions(-) diff --git a/internal/chart/v3/loader/load.go b/internal/chart/v3/loader/load.go index 72343d7b6..5e7c09327 100644 --- a/internal/chart/v3/loader/load.go +++ b/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 { diff --git a/internal/chart/v3/loader/load_test.go b/internal/chart/v3/loader/load_test.go index 2163cf4e2..57b9422e2 100644 --- a/internal/chart/v3/loader/load_test.go +++ b/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) { diff --git a/pkg/chart/v2/loader/load.go b/pkg/chart/v2/loader/load.go index fc57190ec..946ee1047 100644 --- a/pkg/chart/v2/loader/load.go +++ b/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 { diff --git a/pkg/chart/v2/loader/load_test.go b/pkg/chart/v2/loader/load_test.go index 5af88b341..bf6bf5159 100644 --- a/pkg/chart/v2/loader/load_test.go +++ b/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) { diff --git a/pkg/chart/v2/util/save_test.go b/pkg/chart/v2/util/save_test.go index 6599addb2..beeae8118 100644 --- a/pkg/chart/v2/util/save_test.go +++ b/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 {