msb-public
/
helm
mirror of https://github.com/helm/helm
1
0
Code Issues Projects Releases Wiki Activity

fix: parse SOURCE_DATE_EPOCH once per Save, error on invalid value

Browse Source 
- sourceDateEpoch() now returns (time.Time, bool, error) and errors
  when SOURCE_DATE_EPOCH is set but cannot be parsed as an integer,
  rather than silently falling back to non-reproducible behavior
- Save() calls sourceDateEpoch() once and passes the resolved *time.Time
  down through writeTarContents() and writeToTar(), eliminating
  repeated env reads and preventing mixed timestamps within a single
  archive
- Add TestSourceDateEpochInvalid to both v2 and v3 test suites

Addresses Copilot review comments on PR #32060.

Signed-off-by: Kunal Jain <qlapon@gmail.com>
pull/32060/head
Kunal Jain 6 months ago committed by Kunal Jain
parent f7591abe2a
commit 773b9d6c04
4 changed files with 86 additions and 34 deletions
Show Stats Download Patch File Download Diff File

44
internal/chart/v3/util/save.go
Unescape View File

@ -126,6 +126,15 @@ func Save(c *chart.Chart, outDir string) (string, error) {
return "", fmt.Errorf("is not a directory: %s", dir) return "", fmt.Errorf("is not a directory: %s", dir)
} }
epoch, hasEpoch, err := sourceDateEpoch()
if err != nil {
return "", err
}
var epochPtr *time.Time
if hasEpoch {
epochPtr = &epoch
}
f, err := os.Create(filename) f, err := os.Create(filename)
if err != nil { if err != nil {
return "", err return "", err
@ -148,14 +157,14 @@ func Save(c *chart.Chart, outDir string) (string, error) {
} }
}() }()
if err := writeTarContents(twriter, c, ""); err != nil { if err := writeTarContents(twriter, c, "", epochPtr); err != nil {
rollback = true rollback = true
return filename, err return filename, err
} }
return filename, nil return filename, nil
} }
func writeTarContents(out *tar.Writer, c *chart.Chart, prefix string) error { func writeTarContents(out *tar.Writer, c *chart.Chart, prefix string, epoch *time.Time) error {
err := validateName(c.Name()) err := validateName(c.Name())
if err != nil { if err != nil {
return err return err
@ -167,7 +176,7 @@ func writeTarContents(out *tar.Writer, c *chart.Chart, prefix string) error {
if err != nil { if err != nil {
return err return err
} }
if err := writeToTar(out, filepath.Join(base, ChartfileName), cdata, c.ModTime); err != nil { if err := writeToTar(out, filepath.Join(base, ChartfileName), cdata, c.ModTime, epoch); err != nil {
return err return err
} }
@ -177,7 +186,7 @@ func writeTarContents(out *tar.Writer, c *chart.Chart, prefix string) error {
if err != nil { if err != nil {
return err return err
} }
if err := writeToTar(out, filepath.Join(base, "Chart.lock"), ldata, c.Lock.Generated); err != nil { if err := writeToTar(out, filepath.Join(base, "Chart.lock"), ldata, c.Lock.Generated, epoch); err != nil {
return err return err
} }
} }
@ -185,7 +194,7 @@ func writeTarContents(out *tar.Writer, c *chart.Chart, prefix string) error {
// Save values.yaml // Save values.yaml
for _, f := range c.Raw { for _, f := range c.Raw {
if f.Name == ValuesfileName { if f.Name == ValuesfileName {
if err := writeToTar(out, filepath.Join(base, ValuesfileName), f.Data, f.ModTime); err != nil { if err := writeToTar(out, filepath.Join(base, ValuesfileName), f.Data, f.ModTime, epoch); err != nil {
return err return err
} }
} }
@ -196,7 +205,7 @@ func writeTarContents(out *tar.Writer, c *chart.Chart, prefix string) error {
if !json.Valid(c.Schema) { if !json.Valid(c.Schema) {
return errors.New("invalid JSON in " + SchemafileName) return errors.New("invalid JSON in " + SchemafileName)
} }
if err := writeToTar(out, filepath.Join(base, SchemafileName), c.Schema, c.SchemaModTime); err != nil { if err := writeToTar(out, filepath.Join(base, SchemafileName), c.Schema, c.SchemaModTime, epoch); err != nil {
return err return err
} }
} }
@ -204,7 +213,7 @@ func writeTarContents(out *tar.Writer, c *chart.Chart, prefix string) error {
// Save templates // Save templates
for _, f := range c.Templates { for _, f := range c.Templates {
n := filepath.Join(base, f.Name) n := filepath.Join(base, f.Name)
if err := writeToTar(out, n, f.Data, f.ModTime); err != nil { if err := writeToTar(out, n, f.Data, f.ModTime, epoch); err != nil {
return err return err
} }
} }
@ -212,14 +221,14 @@ func writeTarContents(out *tar.Writer, c *chart.Chart, prefix string) error {
// Save files // Save files
for _, f := range c.Files { for _, f := range c.Files {
n := filepath.Join(base, f.Name) n := filepath.Join(base, f.Name)
if err := writeToTar(out, n, f.Data, f.ModTime); err != nil { if err := writeToTar(out, n, f.Data, f.ModTime, epoch); err != nil {
return err return err
} }
} }
// Save dependencies // Save dependencies
for _, dep := range c.Dependencies() { for _, dep := range c.Dependencies() {
if err := writeTarContents(out, dep, filepath.Join(base, ChartsDir)); err != nil { if err := writeTarContents(out, dep, filepath.Join(base, ChartsDir), epoch); err != nil {
return err return err
} }
} }
@ -227,7 +236,7 @@ func writeTarContents(out *tar.Writer, c *chart.Chart, prefix string) error {
} }
// writeToTar writes a single file to a tar archive. // writeToTar writes a single file to a tar archive.
func writeToTar(out *tar.Writer, name string, body []byte, modTime time.Time) error { func writeToTar(out *tar.Writer, name string, body []byte, modTime time.Time, epoch *time.Time) error {
// TODO: Do we need to create dummy parent directory names if none exist? // TODO: Do we need to create dummy parent directory names if none exist?
h := &tar.Header{ h := &tar.Header{
Name: filepath.ToSlash(name), Name: filepath.ToSlash(name),
@ -235,8 +244,8 @@ func writeToTar(out *tar.Writer, name string, body []byte, modTime time.Time) er
Size: int64(len(body)), Size: int64(len(body)),
ModTime: modTime, ModTime: modTime,
} }
if epoch, ok := sourceDateEpoch(); ok { if epoch != nil {
h.ModTime = epoch h.ModTime = *epoch
} else if h.ModTime.IsZero() { } else if h.ModTime.IsZero() {
h.ModTime = time.Now() h.ModTime = time.Now()
} }
@ -247,18 +256,19 @@ func writeToTar(out *tar.Writer, name string, body []byte, modTime time.Time) er
return err return err
} }
// sourceDateEpoch returns the time specified by SOURCE_DATE_EPOCH env var, if set. // sourceDateEpoch parses the SOURCE_DATE_EPOCH environment variable.
// SOURCE_DATE_EPOCH is a Unix timestamp used to ensure reproducible builds. // SOURCE_DATE_EPOCH is a Unix timestamp used to ensure reproducible builds.
func sourceDateEpoch() (time.Time, bool) { // It returns an error if the variable is set but cannot be parsed.
func sourceDateEpoch() (time.Time, bool, error) {
s, ok := os.LookupEnv("SOURCE_DATE_EPOCH") s, ok := os.LookupEnv("SOURCE_DATE_EPOCH")
if !ok { if !ok {
return time.Time{}, false return time.Time{}, false, nil
} }
secs, err := strconv.ParseInt(s, 10, 64) secs, err := strconv.ParseInt(s, 10, 64)
if err != nil { if err != nil {
return time.Time{}, false return time.Time{}, false, fmt.Errorf("invalid SOURCE_DATE_EPOCH %q: %w", s, err)
} }
return time.Unix(secs, 0), true return time.Unix(secs, 0), true, nil
} }
// If the name has directory name has characters which would change the location // If the name has directory name has characters which would change the location

16
internal/chart/v3/util/save_test.go
Unescape View File

@ -410,3 +410,19 @@ func sha256Sum(filePath string) (string, error) {
return hex.EncodeToString(h.Sum(nil)), nil return hex.EncodeToString(h.Sum(nil)), nil
} }
func TestSourceDateEpochInvalid(t *testing.T) {
t.Setenv("SOURCE_DATE_EPOCH", "not-a-number")
c := &chart.Chart{
Metadata: &chart.Metadata{
APIVersion: chart.APIVersionV3,
Name: "ahab",
Version: "1.2.3",
},
}
_, err := Save(c, t.TempDir())
if err == nil {
t.Fatal("expected error for invalid SOURCE_DATE_EPOCH, got nil")
}
}

44
pkg/chart/v2/util/save.go
Unescape View File

@ -126,6 +126,15 @@ func Save(c *chart.Chart, outDir string) (string, error) {
return "", fmt.Errorf("is not a directory: %s", dir) return "", fmt.Errorf("is not a directory: %s", dir)
} }
epoch, hasEpoch, err := sourceDateEpoch()
if err != nil {
return "", err
}
var epochPtr *time.Time
if hasEpoch {
epochPtr = &epoch
}
f, err := os.Create(filename) f, err := os.Create(filename)
if err != nil { if err != nil {
return "", err return "", err
@ -148,14 +157,14 @@ func Save(c *chart.Chart, outDir string) (string, error) {
} }
}() }()
if err := writeTarContents(twriter, c, ""); err != nil { if err := writeTarContents(twriter, c, "", epochPtr); err != nil {
rollback = true rollback = true
return filename, err return filename, err
} }
return filename, nil return filename, nil
} }
func writeTarContents(out *tar.Writer, c *chart.Chart, prefix string) error { func writeTarContents(out *tar.Writer, c *chart.Chart, prefix string, epoch *time.Time) error {
err := validateName(c.Name()) err := validateName(c.Name())
if err != nil { if err != nil {
return err return err
@ -176,7 +185,7 @@ func writeTarContents(out *tar.Writer, c *chart.Chart, prefix string) error {
if err != nil { if err != nil {
return err return err
} }
if err := writeToTar(out, filepath.Join(base, ChartfileName), cdata, c.ModTime); err != nil { if err := writeToTar(out, filepath.Join(base, ChartfileName), cdata, c.ModTime, epoch); err != nil {
return err return err
} }
@ -188,7 +197,7 @@ func writeTarContents(out *tar.Writer, c *chart.Chart, prefix string) error {
if err != nil { if err != nil {
return err return err
} }
if err := writeToTar(out, filepath.Join(base, "Chart.lock"), ldata, c.Lock.Generated); err != nil { if err := writeToTar(out, filepath.Join(base, "Chart.lock"), ldata, c.Lock.Generated, epoch); err != nil {
return err return err
} }
} }
@ -197,7 +206,7 @@ func writeTarContents(out *tar.Writer, c *chart.Chart, prefix string) error {
// Save values.yaml // Save values.yaml
for _, f := range c.Raw { for _, f := range c.Raw {
if f.Name == ValuesfileName { if f.Name == ValuesfileName {
if err := writeToTar(out, filepath.Join(base, ValuesfileName), f.Data, f.ModTime); err != nil { if err := writeToTar(out, filepath.Join(base, ValuesfileName), f.Data, f.ModTime, epoch); err != nil {
return err return err
} }
} }
@ -208,7 +217,7 @@ func writeTarContents(out *tar.Writer, c *chart.Chart, prefix string) error {
if !json.Valid(c.Schema) { if !json.Valid(c.Schema) {
return errors.New("invalid JSON in " + SchemafileName) return errors.New("invalid JSON in " + SchemafileName)
} }
if err := writeToTar(out, filepath.Join(base, SchemafileName), c.Schema, c.SchemaModTime); err != nil { if err := writeToTar(out, filepath.Join(base, SchemafileName), c.Schema, c.SchemaModTime, epoch); err != nil {
return err return err
} }
} }
@ -216,7 +225,7 @@ func writeTarContents(out *tar.Writer, c *chart.Chart, prefix string) error {
// Save templates // Save templates
for _, f := range c.Templates { for _, f := range c.Templates {
n := filepath.Join(base, f.Name) n := filepath.Join(base, f.Name)
if err := writeToTar(out, n, f.Data, f.ModTime); err != nil { if err := writeToTar(out, n, f.Data, f.ModTime, epoch); err != nil {
return err return err
} }
} }
@ -224,14 +233,14 @@ func writeTarContents(out *tar.Writer, c *chart.Chart, prefix string) error {
// Save files // Save files
for _, f := range c.Files { for _, f := range c.Files {
n := filepath.Join(base, f.Name) n := filepath.Join(base, f.Name)
if err := writeToTar(out, n, f.Data, f.ModTime); err != nil { if err := writeToTar(out, n, f.Data, f.ModTime, epoch); err != nil {
return err return err
} }
} }
// Save dependencies // Save dependencies
for _, dep := range c.Dependencies() { for _, dep := range c.Dependencies() {
if err := writeTarContents(out, dep, filepath.Join(base, ChartsDir)); err != nil { if err := writeTarContents(out, dep, filepath.Join(base, ChartsDir), epoch); err != nil {
return err return err
} }
} }
@ -239,7 +248,7 @@ func writeTarContents(out *tar.Writer, c *chart.Chart, prefix string) error {
} }
// writeToTar writes a single file to a tar archive. // writeToTar writes a single file to a tar archive.
func writeToTar(out *tar.Writer, name string, body []byte, modTime time.Time) error { func writeToTar(out *tar.Writer, name string, body []byte, modTime time.Time, epoch *time.Time) error {
// TODO: Do we need to create dummy parent directory names if none exist? // TODO: Do we need to create dummy parent directory names if none exist?
h := &tar.Header{ h := &tar.Header{
Name: filepath.ToSlash(name), Name: filepath.ToSlash(name),
@ -247,8 +256,8 @@ func writeToTar(out *tar.Writer, name string, body []byte, modTime time.Time) er
Size: int64(len(body)), Size: int64(len(body)),
ModTime: modTime, ModTime: modTime,
} }
if epoch, ok := sourceDateEpoch(); ok { if epoch != nil {
h.ModTime = epoch h.ModTime = *epoch
} else if h.ModTime.IsZero() { } else if h.ModTime.IsZero() {
h.ModTime = time.Now() h.ModTime = time.Now()
} }
@ -259,18 +268,19 @@ func writeToTar(out *tar.Writer, name string, body []byte, modTime time.Time) er
return err return err
} }
// sourceDateEpoch returns the time specified by SOURCE_DATE_EPOCH env var, if set. // sourceDateEpoch parses the SOURCE_DATE_EPOCH environment variable.
// SOURCE_DATE_EPOCH is a Unix timestamp used to ensure reproducible builds. // SOURCE_DATE_EPOCH is a Unix timestamp used to ensure reproducible builds.
func sourceDateEpoch() (time.Time, bool) { // It returns an error if the variable is set but cannot be parsed.
func sourceDateEpoch() (time.Time, bool, error) {
s, ok := os.LookupEnv("SOURCE_DATE_EPOCH") s, ok := os.LookupEnv("SOURCE_DATE_EPOCH")
if !ok { if !ok {
return time.Time{}, false return time.Time{}, false, nil
} }
secs, err := strconv.ParseInt(s, 10, 64) secs, err := strconv.ParseInt(s, 10, 64)
if err != nil { if err != nil {
return time.Time{}, false return time.Time{}, false, fmt.Errorf("invalid SOURCE_DATE_EPOCH %q: %w", s, err)
} }
return time.Unix(secs, 0), true return time.Unix(secs, 0), true, nil
} }
// If the name has directory name has characters which would change the location // If the name has directory name has characters which would change the location

16
pkg/chart/v2/util/save_test.go
Unescape View File

@ -414,3 +414,19 @@ func sha256Sum(filePath string) (string, error) {
return hex.EncodeToString(h.Sum(nil)), nil return hex.EncodeToString(h.Sum(nil)), nil
} }
func TestSourceDateEpochInvalid(t *testing.T) {
t.Setenv("SOURCE_DATE_EPOCH", "not-a-number")
c := &chart.Chart{
Metadata: &chart.Metadata{
APIVersion: chart.APIVersionV2,
Name: "ahab",
Version: "1.2.3",
},
}
_, err := Save(c, t.TempDir())
if err == nil {
t.Fatal("expected error for invalid SOURCE_DATE_EPOCH, got nil")
}
}

Write Preview
Loading…
Cancel
Save