diff --git a/go.mod b/go.mod index 6a691efac..c34a80649 100644 --- a/go.mod +++ b/go.mod @@ -32,7 +32,7 @@ require ( github.com/santhosh-tekuri/jsonschema/v6 v6.0.3 github.com/spf13/cobra v1.10.2 github.com/spf13/pflag v1.0.10 - github.com/stretchr/testify v1.12.0 + github.com/stretchr/testify v1.12.1 github.com/tetratelabs/wazero v1.12.0 go.yaml.in/yaml/v3 v3.0.5 golang.org/x/crypto v0.55.0 diff --git a/go.sum b/go.sum index 54d14492a..bfac195f1 100644 --- a/go.sum +++ b/go.sum @@ -307,8 +307,8 @@ github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXf github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= -github.com/stretchr/testify v1.12.0 h1:K6Mr6jO9JICuend/5xzTM03ydSV3vdNRYAdPSukj8uI= -github.com/stretchr/testify v1.12.0/go.mod h1:bOYBZb5qJ00vPzWfIqBUZPaxK8jWiXc6d3ErP4Ca9Gw= +github.com/stretchr/testify v1.12.1 h1:EuwCh5fleGS7H32xRwO3wRGT7DxrDhLAT6FF8MpWDWE= +github.com/stretchr/testify v1.12.1/go.mod h1:MDEgiDPPsNp5cuIrHPPCyornHKgEVbtFUmoNlxoYthg= github.com/tetratelabs/wabin v0.0.0-20230304001439-f6f874872834 h1:ZF+QBjOI+tILZjBaFj3HgFonKXUcwgJ4djLb6i42S3Q= github.com/tetratelabs/wabin v0.0.0-20230304001439-f6f874872834/go.mod h1:m9ymHTgNSEjuxvw8E7WWe4Pl4hZQHXONY8wE6dMLaRk= github.com/tetratelabs/wazero v1.12.0 h1:DuWcpNu/FzgEXgGBDp8J1Spc+CWOvvtvVyjKlaZopYU= diff --git a/internal/chart/v3/loader/load.go b/internal/chart/v3/loader/load.go index 48f346ccf..0ff040eae 100644 --- a/internal/chart/v3/loader/load.go +++ b/internal/chart/v3/loader/load.go @@ -183,8 +183,20 @@ func LoadFiles(files []*archive.BufferedFile) (*chart.Chart, error) { // The reader is expected to contain one or more YAML documents, the values of which are merged. // And the values can be either a chart's default values or user-supplied values. func LoadValues(data io.Reader) (map[string]any, error) { + // Read fully first. YAMLReader/LineReader can drop a final unterminated + // line when its length is an exact multiple of bufio.Reader's default + // buffer (4096). Appending a trailing newline avoids that case. + // See https://github.com/helm/helm/issues/32506 + b, err := io.ReadAll(data) + if err != nil { + return nil, err + } + if len(b) > 0 && b[len(b)-1] != '\n' { + b = append(b, '\n') + } + values := map[string]any{} - reader := utilyaml.NewYAMLReader(bufio.NewReader(data)) + reader := utilyaml.NewYAMLReader(bufio.NewReader(bytes.NewReader(b))) for { currentMap := map[string]any{} raw, err := reader.Read() diff --git a/internal/chart/v3/loader/load_test.go b/internal/chart/v3/loader/load_test.go index dd5fdc8db..e34364f39 100644 --- a/internal/chart/v3/loader/load_test.go +++ b/internal/chart/v3/loader/load_test.go @@ -21,6 +21,7 @@ import ( "bytes" "compress/gzip" "errors" + "fmt" "io" "log" "os" @@ -419,6 +420,33 @@ foo: } } +func TestLoadValuesEOFBoundary(t *testing.T) { + // Reproduces #32506: a single logical line whose length is a multiple of + // bufio's default buffer (4096) and has no trailing newline used to be + // dropped entirely by YAMLReader, yielding empty values. + // Also cover 8192 (2x buffer) so we do not only hit the single-buffer case. + for _, size := range []int{4096, 8192} { + t.Run(fmt.Sprintf("size_%d", size), func(t *testing.T) { + prefix := []byte(`{"foo":"`) + suffix := []byte(`"}`) + pad := size - len(prefix) - len(suffix) + data := make([]byte, 0, size) + data = append(data, prefix...) + data = append(data, bytes.Repeat([]byte("x"), pad)...) + data = append(data, suffix...) + if len(data) != size { + t.Fatalf("test setup: want data length %d, got %d", size, len(data)) + } + + values, err := LoadValues(bytes.NewReader(data)) + require.NoError(t, err) + assert.Equal(t, map[string]any{ + "foo": string(bytes.Repeat([]byte("x"), pad)), + }, values) + }) + } +} + func TestMergeValuesV3(t *testing.T) { nestedMap := map[string]any{ "foo": "bar", diff --git a/pkg/action/package_test.go b/pkg/action/package_test.go index 421f34b33..2b359fa52 100644 --- a/pkg/action/package_test.go +++ b/pkg/action/package_test.go @@ -17,9 +17,14 @@ limitations under the License. package action import ( + "archive/tar" + "compress/gzip" + "errors" + "io" "os" "path" "testing" + "time" "github.com/Masterminds/semver/v3" "github.com/stretchr/testify/assert" @@ -144,3 +149,60 @@ func TestRun(t *testing.T) { require.Equal(t, "empty-0.1.0.tgz", filename) require.NoError(t, os.Remove(filename)) } + +// TestRunWithSourceDateEpochStampsLockGenerated verifies that packaging a chart +// that has a Chart.lock stamps both the tar entry modtime and the marshaled +// generated: field in Chart.lock to the given epoch. +// +// This guards against the normalization regression where a caller supplying a +// local-timezone or sub-second time.Time would produce a non-reproducible +// generated: value even when the same SOURCE_DATE_EPOCH is used on different +// machines. +func TestRunWithSourceDateEpochStampsLockGenerated(t *testing.T) { + // Use a non-local, non-UTC timezone and sub-second precision to confirm + // normalization: without UTC().Truncate(time.Second) the generated: field + // would contain a timezone offset or fractional seconds. + loc := time.FixedZone("UTC+3", 3*60*60) + rawEpoch := time.Unix(1700000000, 123456789).In(loc) + epoch := rawEpoch.UTC().Truncate(time.Second) + + client := NewPackage() + client.SourceDateEpoch = &rawEpoch + + filename, err := client.Run("testdata/charts/chart-with-lock", nil) + require.NoError(t, err) + t.Cleanup(func() { os.Remove(filename) }) + + f, err := os.Open(filename) + require.NoError(t, err) + defer f.Close() + + gr, err := gzip.NewReader(f) + require.NoError(t, err) + defer gr.Close() + + const wantPath = "chart-with-lock/Chart.lock" + found := false + tr := tar.NewReader(gr) + for { + hdr, err := tr.Next() + if errors.Is(err, io.EOF) { + break + } + require.NoError(t, err) + if hdr.Name != wantPath { + continue + } + found = true + require.True(t, epoch.Equal(hdr.ModTime), + "Chart.lock tar modtime: got %v, want %v", hdr.ModTime, epoch) + + raw, err := io.ReadAll(tr) + require.NoError(t, err) + wantGenerated := epoch.Format(time.RFC3339) + require.Contains(t, string(raw), wantGenerated, + "Chart.lock generated: field should contain normalized UTC timestamp") + break + } + require.True(t, found, "expected archive to contain %q entry", wantPath) +} diff --git a/pkg/action/testdata/charts/chart-with-lock/Chart.lock b/pkg/action/testdata/charts/chart-with-lock/Chart.lock new file mode 100644 index 000000000..824df0a40 --- /dev/null +++ b/pkg/action/testdata/charts/chart-with-lock/Chart.lock @@ -0,0 +1,3 @@ +dependencies: [] +digest: sha256:0000000000000000000000000000000000000000000000000000000000000 +generated: "2016-01-01T00:00:00Z" diff --git a/pkg/action/testdata/charts/chart-with-lock/Chart.yaml b/pkg/action/testdata/charts/chart-with-lock/Chart.yaml new file mode 100644 index 000000000..2cb1b3557 --- /dev/null +++ b/pkg/action/testdata/charts/chart-with-lock/Chart.yaml @@ -0,0 +1,4 @@ +apiVersion: v2 +name: chart-with-lock +version: 0.1.0 +description: Test chart with a Chart.lock, used to verify SourceDateEpoch stamping diff --git a/pkg/chart/v2/chart.go b/pkg/chart/v2/chart.go index 9772754ce..ccfe84f51 100644 --- a/pkg/chart/v2/chart.go +++ b/pkg/chart/v2/chart.go @@ -177,34 +177,41 @@ func (ch *Chart) CRDObjects() []CRD { return crds } -// StampModTimes sets timestamps on the chart (and dependencies) to epoch. -// This is used for reproducible builds via SOURCE_DATE_EPOCH. -func (ch *Chart) StampModTimes(epoch time.Time) { - ch.ModTime = epoch +// StampModTimes sets timestamps on the chart (and dependencies) to t, +// normalized to UTC and truncated to whole seconds. +// +// Normalization is required because Chart.lock's generated: field is written +// by yaml.Marshal from Lock.Generated. Without UTC/truncate, a caller +// supplying a local-zone or sub-second time.Time produces a generated: value +// with a timezone offset or fractional seconds, making the lock file content +// non-reproducible across machines even when the same SOURCE_DATE_EPOCH is used. +func (ch *Chart) StampModTimes(t time.Time) { + t = t.UTC().Truncate(time.Second) + ch.ModTime = t if len(ch.Schema) > 0 { - ch.SchemaModTime = epoch + ch.SchemaModTime = t } if ch.Lock != nil { - ch.Lock.Generated = epoch + ch.Lock.Generated = t } for _, f := range ch.Raw { if f != nil { - f.ModTime = epoch + f.ModTime = t } } for _, f := range ch.Templates { if f != nil { - f.ModTime = epoch + f.ModTime = t } } for _, f := range ch.Files { if f != nil { - f.ModTime = epoch + f.ModTime = t } } for _, dep := range ch.Dependencies() { - dep.StampModTimes(epoch) + dep.StampModTimes(t) } } diff --git a/pkg/chart/v2/loader/load.go b/pkg/chart/v2/loader/load.go index 28115d062..d7b125b9b 100644 --- a/pkg/chart/v2/loader/load.go +++ b/pkg/chart/v2/loader/load.go @@ -210,8 +210,20 @@ func LoadFiles(files []*archive.BufferedFile) (*chart.Chart, error) { // The reader is expected to contain one or more YAML documents, the values of which are merged. // And the values can be either a chart's default values or user-supplied values. func LoadValues(data io.Reader) (map[string]any, error) { + // Read fully first. YAMLReader/LineReader can drop a final unterminated + // line when its length is an exact multiple of bufio.Reader's default + // buffer (4096). Appending a trailing newline avoids that case. + // See https://github.com/helm/helm/issues/32506 + b, err := io.ReadAll(data) + if err != nil { + return nil, err + } + if len(b) > 0 && b[len(b)-1] != '\n' { + b = append(b, '\n') + } + values := map[string]any{} - reader := utilyaml.NewYAMLReader(bufio.NewReader(data)) + reader := utilyaml.NewYAMLReader(bufio.NewReader(bytes.NewReader(b))) for { currentMap := map[string]any{} raw, err := reader.Read() diff --git a/pkg/chart/v2/loader/load_test.go b/pkg/chart/v2/loader/load_test.go index 8bf254321..ae1da1130 100644 --- a/pkg/chart/v2/loader/load_test.go +++ b/pkg/chart/v2/loader/load_test.go @@ -21,6 +21,7 @@ import ( "bytes" "compress/gzip" "errors" + "fmt" "io" "log" "os" @@ -463,6 +464,33 @@ foo: } } +func TestLoadValuesEOFBoundary(t *testing.T) { + // Reproduces #32506: a single logical line whose length is a multiple of + // bufio's default buffer (4096) and has no trailing newline used to be + // dropped entirely by YAMLReader, yielding empty values. + // Also cover 8192 (2x buffer) so we do not only hit the single-buffer case. + for _, size := range []int{4096, 8192} { + t.Run(fmt.Sprintf("size_%d", size), func(t *testing.T) { + prefix := []byte(`{"foo":"`) + suffix := []byte(`"}`) + pad := size - len(prefix) - len(suffix) + data := make([]byte, 0, size) + data = append(data, prefix...) + data = append(data, bytes.Repeat([]byte("x"), pad)...) + data = append(data, suffix...) + if len(data) != size { + t.Fatalf("test setup: want data length %d, got %d", size, len(data)) + } + + values, err := LoadValues(bytes.NewReader(data)) + require.NoError(t, err) + assert.Equal(t, map[string]any{ + "foo": string(bytes.Repeat([]byte("x"), pad)), + }, values) + }) + } +} + func TestMergeValuesV2(t *testing.T) { nestedMap := map[string]any{ "foo": "bar",