diff --git a/internal/chart/v3/loader/directory.go b/internal/chart/v3/loader/directory.go index b49874f76..413e7b34d 100644 --- a/internal/chart/v3/loader/directory.go +++ b/internal/chart/v3/loader/directory.go @@ -23,6 +23,8 @@ import ( "path/filepath" "strings" + "k8s.io/apimachinery/pkg/api/resource" + chart "helm.sh/helm/v4/internal/chart/v3" "helm.sh/helm/v4/internal/sympath" "helm.sh/helm/v4/pkg/chart/loader/archive" @@ -123,7 +125,8 @@ func LoadDirWithOptions(dir string, opts archive.Options) (*chart.Chart, error) } if fi.Size() > opts.MaxDecompressedFileSize { - return fmt.Errorf("chart file %q is larger than the maximum file size %d", fi.Name(), opts.MaxDecompressedFileSize) + maxSize := resource.NewQuantity(opts.MaxDecompressedFileSize, resource.BinarySI) + return fmt.Errorf("chart file %q is larger than the maximum file size %s", fi.Name(), maxSize.String()) } data, err := os.ReadFile(name) diff --git a/pkg/chart/loader/archive/archive.go b/pkg/chart/loader/archive/archive.go index 032cc7675..d7682e623 100644 --- a/pkg/chart/loader/archive/archive.go +++ b/pkg/chart/loader/archive/archive.go @@ -30,6 +30,8 @@ import ( "regexp" "strings" "time" + + "k8s.io/apimachinery/pkg/api/resource" ) var drivePathPattern = regexp.MustCompile(`^[a-zA-Z]:/`) @@ -139,11 +141,13 @@ func LoadArchiveFilesWithOptions(in io.Reader, opts Options) ([]*BufferedFile, e } if hd.Size > remainingSize { - return nil, fmt.Errorf("decompressed chart is larger than the maximum size %d bytes", opts.MaxDecompressedChartSize) + maxSize := resource.NewQuantity(opts.MaxDecompressedChartSize, resource.BinarySI) + return nil, fmt.Errorf("decompressed chart is larger than the maximum size %s", maxSize.String()) } if hd.Size > opts.MaxDecompressedFileSize { - return nil, fmt.Errorf("decompressed chart file %q is larger than the maximum file size %d bytes", hd.Name, opts.MaxDecompressedFileSize) + maxSize := resource.NewQuantity(opts.MaxDecompressedFileSize, resource.BinarySI) + return nil, fmt.Errorf("decompressed chart file %q is larger than the maximum file size %s", hd.Name, maxSize.String()) } limitedReader := io.LimitReader(tr, remainingSize) @@ -159,7 +163,8 @@ func LoadArchiveFilesWithOptions(in io.Reader, opts Options) ([]*BufferedFile, e // is the one that goes over the limit. It assumes the Size stored in the tar header // is correct, something many applications do. if bytesWritten < hd.Size || remainingSize <= 0 { - return nil, fmt.Errorf("decompressed chart is larger than the maximum size %d bytes", opts.MaxDecompressedChartSize) + maxSize := resource.NewQuantity(opts.MaxDecompressedChartSize, resource.BinarySI) + return nil, fmt.Errorf("decompressed chart is larger than the maximum size %s", maxSize.String()) } data := bytes.TrimPrefix(b.Bytes(), utf8bom) diff --git a/pkg/chart/v2/loader/directory.go b/pkg/chart/v2/loader/directory.go index 9a68efd61..468ad5a6d 100644 --- a/pkg/chart/v2/loader/directory.go +++ b/pkg/chart/v2/loader/directory.go @@ -23,6 +23,8 @@ import ( "path/filepath" "strings" + "k8s.io/apimachinery/pkg/api/resource" + "helm.sh/helm/v4/internal/sympath" "helm.sh/helm/v4/pkg/chart/loader/archive" chart "helm.sh/helm/v4/pkg/chart/v2" @@ -106,7 +108,8 @@ func LoadDirWithOptions(dir string, opts archive.Options) (*chart.Chart, error) } if fi.Size() > opts.MaxDecompressedFileSize { - return fmt.Errorf("chart file %q is larger than the maximum file size %d", fi.Name(), opts.MaxDecompressedFileSize) + maxSize := resource.NewQuantity(opts.MaxDecompressedFileSize, resource.BinarySI) + return fmt.Errorf("chart file %q is larger than the maximum file size %s", fi.Name(), maxSize.String()) } data, err := os.ReadFile(name) diff --git a/pkg/cli/environment.go b/pkg/cli/environment.go index 627a5b6f0..606780c5f 100644 --- a/pkg/cli/environment.go +++ b/pkg/cli/environment.go @@ -222,6 +222,31 @@ func envFloat32Or(name string, def float32) float32 { return float32(ret) } +// parseQuantityOrInt64 parses a string as either a Kubernetes Quantity or plain int64. +// Returns the parsed value and an error if parsing fails. +func parseQuantityOrInt64(s string) (int64, error) { + s = strings.TrimSpace(s) + + // Try parsing as Kubernetes Quantity first + if q, err := resource.ParseQuantity(s); err == nil { + if v, ok := q.AsInt64(); ok { + return v, nil + } + f := q.AsApproximateFloat64() + if f > 0 && f < float64(^uint64(0)>>1) { + return int64(f), nil + } + return 0, fmt.Errorf("quantity %q is too large to fit in int64", s) + } + + // Fallback to plain int64 + v, err := strconv.ParseInt(s, 10, 64) + if err != nil { + return 0, fmt.Errorf("invalid value %q (expected int or k8s Quantity like 512Mi)", s) + } + return v, nil +} + // Tries to parse as a k8s Quantity first, falls back to plain int64 parsing. func envInt64OrQuantityBytes(name string, def int64) int64 { if name == "" { @@ -232,26 +257,46 @@ func envInt64OrQuantityBytes(name string, def int64) int64 { return def } - envVal = strings.TrimSpace(envVal) - - if q, err := resource.ParseQuantity(envVal); err == nil { - if v, ok := q.AsInt64(); ok { - return v - } - f := q.AsApproximateFloat64() - if f > 0 && f < float64(^uint64(0)>>1) { - return int64(f) - } - slog.Warn("Environment variable %s is too large to fit in int64: %q", name, envVal) + v, err := parseQuantityOrInt64(envVal) + if err != nil { + defQuantity := resource.NewQuantity(def, resource.BinarySI) + slog.Warn(err.Error() + fmt.Sprintf(": using default %s", defQuantity.String())) return def } + return v +} - if v, err := strconv.ParseInt(envVal, 10, 64); err == nil { - return v +// QuantityBytesValue is a custom flag type that accepts both plain int64 and k8s Quantity formats +type QuantityBytesValue struct { + value *int64 +} + +// NewQuantityBytesValue creates a new QuantityBytesValue flag with a pointer to an int64 +func NewQuantityBytesValue(p *int64) *QuantityBytesValue { + return &QuantityBytesValue{value: p} +} + +// Set parses the input string as either a Kubernetes Quantity or plain int64 +func (q *QuantityBytesValue) Set(s string) error { + v, err := parseQuantityOrInt64(s) + if err != nil { + return err + } + *q.value = v + return nil +} + +// String returns the string representation of the value +func (q *QuantityBytesValue) String() string { + if q.value == nil { + return "0" } + return strconv.FormatInt(*q.value, 10) +} - slog.Warn("Environment variable %s has invalid value %q (expected int or k8s Quantity like 512Mi): using default %d", name, envVal, def) - return def +// Type returns the type name for help messages +func (q *QuantityBytesValue) Type() string { + return "quantity" } func envCSV(name string) (ls []string) { diff --git a/pkg/cli/environment_test.go b/pkg/cli/environment_test.go index 3d8b39b2f..18cb8bc59 100644 --- a/pkg/cli/environment_test.go +++ b/pkg/cli/environment_test.go @@ -372,6 +372,68 @@ func TestUserAgentHeaderInK8sRESTClientConfig(t *testing.T) { } } +func TestQuantityBytesValue(t *testing.T) { + tests := []struct { + name string + input string + expected int64 + expectError bool + }{ + { + name: "plain int64", + input: "12345", + expected: 12345, + }, + { + name: "quantity Mi", + input: "256Mi", + expected: 256 * 1024 * 1024, + }, + { + name: "quantity Gi", + input: "1Gi", + expected: 1 * 1024 * 1024 * 1024, + }, + { + name: "quantity with whitespace", + input: " 512Mi ", + expected: 512 * 1024 * 1024, + }, + { + name: "invalid value", + input: "not-a-number", + expectError: true, + }, + { + name: "lowercase suffix rejected", + input: "1gi", + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var val int64 + qv := NewQuantityBytesValue(&val) + + err := qv.Set(tt.input) + + if tt.expectError { + if err == nil { + t.Errorf("expected error but got none") + } + } else { + if err != nil { + t.Errorf("unexpected error: %v", err) + } + if val != tt.expected { + t.Errorf("expected %d, got %d", tt.expected, val) + } + } + }) + } +} + func resetEnv() func() { origEnv := os.Environ() diff --git a/pkg/cmd/install.go b/pkg/cmd/install.go index 149b86fdf..0516a7a26 100644 --- a/pkg/cmd/install.go +++ b/pkg/cmd/install.go @@ -34,6 +34,7 @@ import ( "helm.sh/helm/v4/pkg/chart" "helm.sh/helm/v4/pkg/chart/loader" "helm.sh/helm/v4/pkg/chart/loader/archive" + "helm.sh/helm/v4/pkg/cli" "helm.sh/helm/v4/pkg/cli/output" "helm.sh/helm/v4/pkg/cli/values" "helm.sh/helm/v4/pkg/cmd/require" @@ -132,6 +133,8 @@ charts in a repository, use 'helm search'. func newInstallCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { client := action.NewInstall(cfg) + client.MaxChartSize = settings.MaxChartSize + client.MaxChartFileSize = settings.MaxChartFileSize valueOpts := &values.Options{} var outfmt output.Format @@ -180,8 +183,8 @@ func newInstallCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { addDryRunFlag(cmd) bindOutputFlag(cmd, &outfmt) bindPostRenderFlag(cmd, &client.PostRenderer, settings) - f.Int64Var(&client.MaxChartSize, "max-chart-size", settings.MaxChartSize, "maximum size in bytes for a decompressed chart (default is 100mb)") - f.Int64Var(&client.MaxChartFileSize, "max-file-size", settings.MaxChartFileSize, "maximum size in bytes for a single file in a chart (default is 5mb)") + f.Var(cli.NewQuantityBytesValue(&client.MaxChartSize), "max-chart-size", "maximum size for a decompressed chart (e.g., 500Ki, 5Mi)") + f.Var(cli.NewQuantityBytesValue(&client.MaxChartFileSize), "max-file-size", "maximum size for a single file in a chart (e.g., 5Mi, 10Mi)") return cmd } diff --git a/pkg/cmd/pull.go b/pkg/cmd/pull.go index c24941a2d..01d7e24cf 100644 --- a/pkg/cmd/pull.go +++ b/pkg/cmd/pull.go @@ -25,6 +25,7 @@ import ( "github.com/spf13/cobra" "helm.sh/helm/v4/pkg/action" + "helm.sh/helm/v4/pkg/cli" "helm.sh/helm/v4/pkg/cmd/require" ) @@ -45,6 +46,9 @@ result in an error, and the chart will not be saved locally. func newPullCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { client := action.NewPull(action.WithConfig(cfg)) + // Initialize from environment settings so they serve as defaults for the flags + client.MaxChartSize = settings.MaxChartSize + client.MaxChartFileSize = settings.MaxChartFileSize cmd := &cobra.Command{ Use: "pull [chart URL | repo/chartname] [...]", @@ -89,8 +93,8 @@ func newPullCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { f.BoolVar(&client.VerifyLater, "prov", false, "fetch the provenance file, but don't perform verification") f.StringVar(&client.UntarDir, "untardir", ".", "if untar is specified, this flag specifies the name of the directory into which the chart is expanded") f.StringVarP(&client.DestDir, "destination", "d", ".", "location to write the chart. If this and untardir are specified, untardir is appended to this") - f.Int64Var(&client.MaxChartSize, "max-chart-size", settings.MaxChartSize, "maximum size in bytes for a decompressed chart (default is 100mb)") - f.Int64Var(&client.MaxChartFileSize, "max-file-size", settings.MaxChartFileSize, "maximum size in bytes for a single file in a chart (default is 5mb)") + f.Var(cli.NewQuantityBytesValue(&client.MaxChartSize), "max-chart-size", "maximum size for a decompressed chart (e.g., 100Mi, 1Gi; default is 100Mi)") + f.Var(cli.NewQuantityBytesValue(&client.MaxChartFileSize), "max-file-size", "maximum size for a single file in a chart (e.g., 5Mi, 10Mi; default is 5Mi)") addChartPathOptionsFlags(f, &client.ChartPathOptions) err := cmd.RegisterFlagCompletionFunc("version", func(_ *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { diff --git a/pkg/cmd/pull_test.go b/pkg/cmd/pull_test.go index 024e1aac7..eae095e2b 100644 --- a/pkg/cmd/pull_test.go +++ b/pkg/cmd/pull_test.go @@ -207,14 +207,15 @@ func TestPullCmd(t *testing.T) { { name: "Fail fetching OCI chart with version mismatch", args: fmt.Sprintf("oci://%s/u/ocitestuser/oci-dependent-chart:0.2.0 --version 0.1.0", ociSrv.RegistryURL), - wantErrorMsg: "chart reference and version mismatch: 0.1.0 is not 0.2.0", wantError: true, + wantErrorMsg: "chart reference and version mismatch: 0.1.0 is not 0.2.0", + failExpect: "chart reference and version mismatch", }, { name: "Fail because of small max chart size", - args: "test/test1 --max-chart-size=90", + args: "test/signtest --untar --max-chart-size=1Ki", wantError: true, - wantErrorMsg: "decompressed chart is larger than the maximum size 90 bytes", + wantErrorMsg: "decompressed chart is larger than the maximum size 1Ki", }, } diff --git a/pkg/cmd/testdata/output/install-with-restricted-chart-size.txt b/pkg/cmd/testdata/output/install-with-restricted-chart-size.txt index 75e30d611..565547228 100644 --- a/pkg/cmd/testdata/output/install-with-restricted-chart-size.txt +++ b/pkg/cmd/testdata/output/install-with-restricted-chart-size.txt @@ -1 +1 @@ -Error: INSTALLATION FAILED: unable to load chart archive: decompressed chart is larger than the maximum size 42 bytes +Error: INSTALLATION FAILED: unable to load chart archive: decompressed chart is larger than the maximum size 42 diff --git a/pkg/cmd/testdata/output/upgrade-failed-max-chart-size.txt b/pkg/cmd/testdata/output/upgrade-failed-max-chart-size.txt index 1ea17076f..5bf16467a 100644 --- a/pkg/cmd/testdata/output/upgrade-failed-max-chart-size.txt +++ b/pkg/cmd/testdata/output/upgrade-failed-max-chart-size.txt @@ -1 +1 @@ -Error: unable to load chart archive: decompressed chart is larger than the maximum size 52 bytes +Error: unable to load chart archive: decompressed chart is larger than the maximum size 52 diff --git a/pkg/cmd/upgrade.go b/pkg/cmd/upgrade.go index efa366785..ca4757b92 100644 --- a/pkg/cmd/upgrade.go +++ b/pkg/cmd/upgrade.go @@ -33,6 +33,7 @@ import ( ci "helm.sh/helm/v4/pkg/chart" "helm.sh/helm/v4/pkg/chart/loader" "helm.sh/helm/v4/pkg/chart/loader/archive" + "helm.sh/helm/v4/pkg/cli" "helm.sh/helm/v4/pkg/cli/output" "helm.sh/helm/v4/pkg/cli/values" "helm.sh/helm/v4/pkg/cmd/require" @@ -84,6 +85,9 @@ which can contain sensitive values. To hide Kubernetes Secrets use the func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { client := action.NewUpgrade(cfg) + // Initialize from environment settings so they serve as defaults for the flags + client.MaxChartSize = settings.MaxChartSize + client.MaxChartFileSize = settings.MaxChartFileSize valueOpts := &values.Options{} var outfmt output.Format var createNamespace bool @@ -306,8 +310,8 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { f.BoolVar(&client.DependencyUpdate, "dependency-update", false, "update dependencies if they are missing before installing the chart") f.BoolVar(&client.EnableDNS, "enable-dns", false, "enable DNS lookups when rendering templates") f.BoolVar(&client.TakeOwnership, "take-ownership", false, "if set, upgrade will ignore the check for helm annotations and take ownership of the existing resources") - f.Int64Var(&client.MaxChartSize, "max-chart-size", settings.MaxChartSize, "maximum size in bytes for a decompressed chart (default is 100mb)") - f.Int64Var(&client.MaxChartFileSize, "max-file-size", settings.MaxChartFileSize, "maximum size in bytes for a single file in a chart (default is 5mb)") + f.Var(cli.NewQuantityBytesValue(&client.MaxChartSize), "max-chart-size", "maximum size for a decompressed chart (e.g., 100Mi, 1Gi; default is 100Mi)") + f.Var(cli.NewQuantityBytesValue(&client.MaxChartFileSize), "max-file-size", "maximum size for a single file in a chart (e.g., 5Mi, 10Mi; default is 5Mi)") addDryRunFlag(cmd) addChartPathOptionsFlags(f, &client.ChartPathOptions) addValueOptionsFlags(f, valueOpts)