From 0b9946ae61958d56be40cf82b47e36dc3919ec0c Mon Sep 17 00:00:00 2001 From: Ilya Kiselev Date: Sun, 5 Apr 2026 20:01:22 +0300 Subject: [PATCH 01/10] feat(push): add --output json/yaml/table flag to helm push Add support for --output flag to 'helm push' command for machine-readable output formats (JSON and YAML). This enables programmatic consumption of push results and integration with tools like cosign for artifact signing. Changes: - Modified Pusher interface to return (*registry.PushResult, error) - Updated OCIPusher.Push() and push() to return PushResult - Updated action.Push.Run() to return (*registry.PushResult, error) - Added output formatting to push command (table/json/yaml) - Created pushResult struct with Ref and Digest fields - Implemented pushWriter with WriteTable/WriteJSON/WriteYAML methods - Updated test fixtures to handle new return signature The default table format maintains backward compatibility with existing plain-text output style. Fixes #11735 Signed-off-by: Ilya Kiselev --- pkg/action/push.go | 9 ++--- pkg/cmd/push.go | 45 ++++++++++++++++++++++--- pkg/cmd/push_test.go | 60 ++++++++++++++++++++++++++++++++++ pkg/pusher/ocipusher.go | 21 ++++++------ pkg/pusher/ocipusher_test.go | 6 ++-- pkg/pusher/pusher.go | 4 +-- pkg/uploader/chart_uploader.go | 8 ++--- 7 files changed, 122 insertions(+), 31 deletions(-) diff --git a/pkg/action/push.go b/pkg/action/push.go index 0c7148f65..856a1993a 100644 --- a/pkg/action/push.go +++ b/pkg/action/push.go @@ -18,7 +18,6 @@ package action import ( "io" - "strings" "helm.sh/helm/v4/pkg/cli" "helm.sh/helm/v4/pkg/pusher" @@ -90,11 +89,9 @@ func NewPushWithOpts(opts ...PushOpt) *Push { } // Run executes 'helm push' against the given chart archive. -func (p *Push) Run(chartRef string, remote string) (string, error) { - var out strings.Builder - +func (p *Push) Run(chartRef string, remote string) (*registry.PushResult, error) { c := uploader.ChartUploader{ - Out: &out, + Out: io.Discard, Pushers: pusher.All(p.Settings), Options: []pusher.Option{ pusher.WithTLSClientConfig(p.certFile, p.keyFile, p.caFile), @@ -108,5 +105,5 @@ func (p *Push) Run(chartRef string, remote string) (string, error) { c.Options = append(c.Options, pusher.WithRegistryClient(p.cfg.RegistryClient)) } - return out.String(), c.UploadTo(chartRef, remote) + return c.UploadTo(chartRef, remote) } diff --git a/pkg/cmd/push.go b/pkg/cmd/push.go index 94c5732ff..a6f0168a4 100644 --- a/pkg/cmd/push.go +++ b/pkg/cmd/push.go @@ -23,6 +23,7 @@ import ( "github.com/spf13/cobra" "helm.sh/helm/v4/pkg/action" + "helm.sh/helm/v4/pkg/cli/output" "helm.sh/helm/v4/pkg/cmd/require" "helm.sh/helm/v4/pkg/pusher" ) @@ -42,6 +43,34 @@ type registryPushOptions struct { plainHTTP bool password string username string + outfmt output.Format +} + +// pushResult represents the result of a helm push operation +type pushResult struct { + Ref string `json:"ref"` + Digest string `json:"digest"` +} + +// pushWriter implements the output.Writer interface for push results +type pushWriter struct { + result pushResult +} + +// WriteTable is a no-op for push: the registry client already prints +// "Pushed:" and "Digest:" to stderr, so we avoid duplicating that output. +func (w *pushWriter) WriteTable(_ io.Writer) error { + return nil +} + +// WriteJSON writes the push result in JSON format +func (w *pushWriter) WriteJSON(out io.Writer) error { + return output.EncodeJSON(out, w.result) +} + +// WriteYAML writes the push result in YAML format +func (w *pushWriter) WriteYAML(out io.Writer) error { + return output.EncodeYAML(out, w.result) } func newPushCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { @@ -83,15 +112,19 @@ func newPushCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { client := action.NewPushWithOpts(action.WithPushConfig(cfg), action.WithTLSClientConfig(o.certFile, o.keyFile, o.caFile), action.WithInsecureSkipTLSVerify(o.insecureSkipTLSVerify), - action.WithPlainHTTP(o.plainHTTP), - action.WithPushOptWriter(out)) + action.WithPlainHTTP(o.plainHTTP)) client.Settings = settings - output, err := client.Run(chartRef, remote) + result, err := client.Run(chartRef, remote) if err != nil { return err } - fmt.Fprint(out, output) - return nil + writer := &pushWriter{ + result: pushResult{ + Ref: result.Ref, + Digest: result.Manifest.Digest, + }, + } + return o.outfmt.Write(out, writer) }, } @@ -104,5 +137,7 @@ func newPushCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { f.StringVar(&o.username, "username", "", "chart repository username where to locate the requested chart") f.StringVar(&o.password, "password", "", "chart repository password where to locate the requested chart") + bindOutputFlag(cmd, &o.outfmt) + return cmd } diff --git a/pkg/cmd/push_test.go b/pkg/cmd/push_test.go index 80d08b48f..964337ddd 100644 --- a/pkg/cmd/push_test.go +++ b/pkg/cmd/push_test.go @@ -17,6 +17,8 @@ limitations under the License. package cmd import ( + "bytes" + "strings" "testing" ) @@ -25,3 +27,61 @@ func TestPushFileCompletion(t *testing.T) { checkFileCompletion(t, "push package.tgz", false) checkFileCompletion(t, "push package.tgz oci://localhost:5000", false) } + +func TestPushWriterTable(t *testing.T) { + // WriteTable is intentionally a no-op: the registry client already prints + // push details to stderr, so we avoid duplicating that output. + w := &pushWriter{result: pushResult{ + Ref: "oci://example.com/charts/mychart:1.0.0", + Digest: "sha256:abc123", + }} + var buf bytes.Buffer + if err := w.WriteTable(&buf); err != nil { + t.Fatal(err) + } + if got := buf.String(); got != "" { + t.Errorf("table output should be empty (registry client writes to stderr), got: %q", got) + } +} + +func TestPushWriterJSON(t *testing.T) { + w := &pushWriter{result: pushResult{ + Ref: "oci://example.com/charts/mychart:1.0.0", + Digest: "sha256:abc123", + }} + var buf bytes.Buffer + if err := w.WriteJSON(&buf); err != nil { + t.Fatal(err) + } + got := buf.String() + if !strings.Contains(got, `"ref"`) || !strings.Contains(got, `"digest"`) { + t.Errorf("JSON output missing fields, got: %q", got) + } + if !strings.Contains(got, "oci://example.com/charts/mychart:1.0.0") { + t.Errorf("JSON output missing Ref value, got: %q", got) + } + if !strings.Contains(got, "sha256:abc123") { + t.Errorf("JSON output missing Digest value, got: %q", got) + } +} + +func TestPushWriterYAML(t *testing.T) { + w := &pushWriter{result: pushResult{ + Ref: "oci://example.com/charts/mychart:1.0.0", + Digest: "sha256:abc123", + }} + var buf bytes.Buffer + if err := w.WriteYAML(&buf); err != nil { + t.Fatal(err) + } + got := buf.String() + if !strings.Contains(got, "ref:") || !strings.Contains(got, "digest:") { + t.Errorf("YAML output missing fields, got: %q", got) + } + if !strings.Contains(got, "oci://example.com/charts/mychart:1.0.0") { + t.Errorf("YAML output missing Ref value, got: %q", got) + } + if !strings.Contains(got, "sha256:abc123") { + t.Errorf("YAML output missing Digest value, got: %q", got) + } +} diff --git a/pkg/pusher/ocipusher.go b/pkg/pusher/ocipusher.go index 2a12e09b4..f205946e7 100644 --- a/pkg/pusher/ocipusher.go +++ b/pkg/pusher/ocipusher.go @@ -37,42 +37,42 @@ type OCIPusher struct { } // Push performs a Push from repo.Pusher. -func (pusher *OCIPusher) Push(chartRef, href string, options ...Option) error { +func (pusher *OCIPusher) Push(chartRef, href string, options ...Option) (*registry.PushResult, error) { for _, opt := range options { opt(&pusher.opts) } return pusher.push(chartRef, href) } -func (pusher *OCIPusher) push(chartRef, href string) error { +func (pusher *OCIPusher) push(chartRef, href string) (*registry.PushResult, error) { stat, err := os.Stat(chartRef) if err != nil { if errors.Is(err, fs.ErrNotExist) { - return fmt.Errorf("%s: no such file", chartRef) + return nil, fmt.Errorf("%s: no such file", chartRef) } - return err + return nil, err } if stat.IsDir() { - return errors.New("cannot push directory, must provide chart archive (.tgz)") + return nil, errors.New("cannot push directory, must provide chart archive (.tgz)") } meta, err := loader.Load(chartRef) if err != nil { - return err + return nil, err } client := pusher.opts.registryClient if client == nil { c, err := pusher.newRegistryClient() if err != nil { - return err + return nil, err } client = c } chartBytes, err := os.ReadFile(chartRef) if err != nil { - return err + return nil, err } var pushOpts []registry.PushOption @@ -80,7 +80,7 @@ func (pusher *OCIPusher) push(chartRef, href string) error { if _, err := os.Stat(provRef); err == nil { provBytes, err := os.ReadFile(provRef) if err != nil { - return err + return nil, err } pushOpts = append(pushOpts, registry.PushOptProvData(provBytes)) } @@ -93,8 +93,7 @@ func (pusher *OCIPusher) push(chartRef, href string) error { chartArchiveFileCreatedTime := stat.ModTime() pushOpts = append(pushOpts, registry.PushOptCreationTime(chartArchiveFileCreatedTime.Format(time.RFC3339))) - _, err = client.Push(chartBytes, ref, pushOpts...) - return err + return client.Push(chartBytes, ref, pushOpts...) } // NewOCIPusher constructs a valid OCI client as a Pusher diff --git a/pkg/pusher/ocipusher_test.go b/pkg/pusher/ocipusher_test.go index b7d362681..a8cc83878 100644 --- a/pkg/pusher/ocipusher_test.go +++ b/pkg/pusher/ocipusher_test.go @@ -134,7 +134,7 @@ func TestOCIPusher_Push_ErrorHandling(t *testing.T) { chartRef = tt.setupFunc() } - err = pusher.Push(chartRef, "oci://localhost:5000/test") + _, err = pusher.Push(chartRef, "oci://localhost:5000/test") if err == nil { t.Fatal("Expected error but got none") } @@ -375,7 +375,7 @@ func TestOCIPusher_Push_ChartOperations(t *testing.T) { t.Fatal(err) } - err = pusher.Push(chartRef, tt.href) + _, err = pusher.Push(chartRef, tt.href) if tt.expectError { if err == nil { @@ -407,7 +407,7 @@ func TestOCIPusher_Push_MultipleOptions(t *testing.T) { } // Test that multiple options are applied correctly - err = pusher.Push(chartPath, "oci://localhost:5000/test", + _, err = pusher.Push(chartPath, "oci://localhost:5000/test", WithPlainHTTP(true), WithInsecureSkipTLSVerify(true), ) diff --git a/pkg/pusher/pusher.go b/pkg/pusher/pusher.go index 8ce78b011..adaa26d06 100644 --- a/pkg/pusher/pusher.go +++ b/pkg/pusher/pusher.go @@ -71,8 +71,8 @@ func WithPlainHTTP(plainHTTP bool) Option { // Pusher is an interface to support upload to the specified URL. type Pusher interface { - // Push file content by url string - Push(chartRef, url string, options ...Option) error + // Push file content by url string, returning the push result and any error + Push(chartRef, url string, options ...Option) (*registry.PushResult, error) } // Constructor is the function for every pusher which creates a specific instance diff --git a/pkg/uploader/chart_uploader.go b/pkg/uploader/chart_uploader.go index b3d612e38..91aaa19bc 100644 --- a/pkg/uploader/chart_uploader.go +++ b/pkg/uploader/chart_uploader.go @@ -37,19 +37,19 @@ type ChartUploader struct { } // UploadTo uploads a chart. Depending on the settings, it may also upload a provenance file. -func (c *ChartUploader) UploadTo(ref, remote string) error { +func (c *ChartUploader) UploadTo(ref, remote string) (*registry.PushResult, error) { u, err := url.Parse(remote) if err != nil { - return fmt.Errorf("invalid chart URL format: %s", remote) + return nil, fmt.Errorf("invalid chart URL format: %s", remote) } if u.Scheme == "" { - return fmt.Errorf("scheme prefix missing from remote (e.g. \"%s://\")", registry.OCIScheme) + return nil, fmt.Errorf("scheme prefix missing from remote (e.g. \"%s://\")", registry.OCIScheme) } p, err := c.Pushers.ByScheme(u.Scheme) if err != nil { - return err + return nil, err } return p.Push(ref, u.String(), c.Options...) From 5a093e9d8e9ff4a10e4d7411b942b5b75478fbfd Mon Sep 17 00:00:00 2001 From: Ilya Kiselev Date: Sun, 5 Apr 2026 21:22:15 +0300 Subject: [PATCH 02/10] action/push: remove unused WithPushOptWriter and out field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cmd/push: implement WriteTable to emit ref and digest The WithPushOptWriter option and the out field on Push were never wired to any meaningful output path — ChartUploader.Out is not read by UploadTo() and the registry client manages its own writer. Remove them to avoid dead API surface. WriteTable now writes a tab-aligned REF/DIGEST result to the command's stdout stream. The registry client continues to write its own progress output to stderr, so there is no duplication. Signed-off-by: Mentigen Signed-off-by: Ilya Kiselev --- pkg/action/push.go | 8 -------- pkg/cmd/push.go | 13 +++++++++---- pkg/cmd/push_test.go | 13 +++++++++---- 3 files changed, 18 insertions(+), 16 deletions(-) diff --git a/pkg/action/push.go b/pkg/action/push.go index 856a1993a..285005f86 100644 --- a/pkg/action/push.go +++ b/pkg/action/push.go @@ -36,7 +36,6 @@ type Push struct { caFile string insecureSkipTLSVerify bool plainHTTP bool - out io.Writer } // PushOpt is a type of function that sets options for a push action. @@ -72,13 +71,6 @@ func WithPlainHTTP(plainHTTP bool) PushOpt { } } -// WithPushOptWriter sets the registryOut field on the push configuration object. -func WithPushOptWriter(out io.Writer) PushOpt { - return func(p *Push) { - p.out = out - } -} - // NewPushWithOpts creates a new push, with configuration options. func NewPushWithOpts(opts ...PushOpt) *Push { p := &Push{} diff --git a/pkg/cmd/push.go b/pkg/cmd/push.go index a6f0168a4..1d135fa64 100644 --- a/pkg/cmd/push.go +++ b/pkg/cmd/push.go @@ -19,6 +19,7 @@ package cmd import ( "fmt" "io" + "text/tabwriter" "github.com/spf13/cobra" @@ -57,10 +58,14 @@ type pushWriter struct { result pushResult } -// WriteTable is a no-op for push: the registry client already prints -// "Pushed:" and "Digest:" to stderr, so we avoid duplicating that output. -func (w *pushWriter) WriteTable(_ io.Writer) error { - return nil +// WriteTable writes a minimal human-readable push result to the provided output. +// The registry client prints progress to stderr; this writes the structured +// result (ref + digest) to the command's stdout stream. +func (w *pushWriter) WriteTable(out io.Writer) error { + tw := tabwriter.NewWriter(out, 0, 0, 2, ' ', 0) + fmt.Fprintf(tw, "REF\t%s\n", w.result.Ref) + fmt.Fprintf(tw, "DIGEST\t%s\n", w.result.Digest) + return tw.Flush() } // WriteJSON writes the push result in JSON format diff --git a/pkg/cmd/push_test.go b/pkg/cmd/push_test.go index 964337ddd..177cb88c6 100644 --- a/pkg/cmd/push_test.go +++ b/pkg/cmd/push_test.go @@ -29,8 +29,6 @@ func TestPushFileCompletion(t *testing.T) { } func TestPushWriterTable(t *testing.T) { - // WriteTable is intentionally a no-op: the registry client already prints - // push details to stderr, so we avoid duplicating that output. w := &pushWriter{result: pushResult{ Ref: "oci://example.com/charts/mychart:1.0.0", Digest: "sha256:abc123", @@ -39,8 +37,15 @@ func TestPushWriterTable(t *testing.T) { if err := w.WriteTable(&buf); err != nil { t.Fatal(err) } - if got := buf.String(); got != "" { - t.Errorf("table output should be empty (registry client writes to stderr), got: %q", got) + got := buf.String() + if !strings.Contains(got, "REF") || !strings.Contains(got, "DIGEST") { + t.Errorf("table output missing headers, got: %q", got) + } + if !strings.Contains(got, "oci://example.com/charts/mychart:1.0.0") { + t.Errorf("table output missing Ref value, got: %q", got) + } + if !strings.Contains(got, "sha256:abc123") { + t.Errorf("table output missing Digest value, got: %q", got) } } From 0fa2b9a350ead8f5e1e24478716fc9f3bcea4167 Mon Sep 17 00:00:00 2001 From: Ilya Kiselev Date: Sun, 5 Apr 2026 21:31:36 +0300 Subject: [PATCH 03/10] action/push: remove test for deleted WithPushOptWriter option Signed-off-by: Ilya Kiselev --- pkg/action/push_test.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/pkg/action/push_test.go b/pkg/action/push_test.go index 35c6f3efc..ba28e2cc2 100644 --- a/pkg/action/push_test.go +++ b/pkg/action/push_test.go @@ -17,7 +17,6 @@ limitations under the License. package action import ( - "bytes" "testing" "github.com/stretchr/testify/assert" @@ -57,10 +56,3 @@ func TestNewPushWithPlainHTTP(t *testing.T) { assert.Equal(t, true, client.plainHTTP) } -func TestNewPushWithPushOptWriter(t *testing.T) { - buf := new(bytes.Buffer) - client := NewPushWithOpts(WithPushOptWriter(buf)) - - assert.NotNil(t, client) - assert.Equal(t, buf, client.out) -} From be039a06bef4b44e5e05c7bc9dd80acfa8dbed2b Mon Sep 17 00:00:00 2001 From: Ilya Kiselev Date: Sun, 5 Apr 2026 21:40:04 +0300 Subject: [PATCH 04/10] action/push: fix trailing blank line in push_test.go Signed-off-by: Ilya Kiselev --- pkg/action/push_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/action/push_test.go b/pkg/action/push_test.go index ba28e2cc2..627ee2ed5 100644 --- a/pkg/action/push_test.go +++ b/pkg/action/push_test.go @@ -55,4 +55,3 @@ func TestNewPushWithPlainHTTP(t *testing.T) { assert.NotNil(t, client) assert.Equal(t, true, client.plainHTTP) } - From d3083fce1ccf73932ef47fe22cb6785002a276e4 Mon Sep 17 00:00:00 2001 From: Ilya Kiselev Date: Sun, 5 Apr 2026 21:52:37 +0300 Subject: [PATCH 05/10] cmd/push: align WriteTable format and add output flag test Address Copilot review feedback: - WriteTable now uses "Pushed:"/"Digest:" labels consistent with the registry client's built-in output (pkg/registry/client.go:746-747), so the default --output table experience is familiar to existing users - Add TestPushOutputFlagCompletion to verify the --output flag is properly registered and offers json/yaml/table completions - Document that Pusher.Push and action.Push.Run signature changes are intentional breaking changes in the Helm v4 major release Signed-off-by: Ilya Kiselev --- pkg/action/push.go | 7 ++++++- pkg/cmd/push.go | 14 ++++++-------- pkg/cmd/push_test.go | 18 ++++++++++++++++-- pkg/pusher/pusher.go | 7 ++++++- 4 files changed, 34 insertions(+), 12 deletions(-) diff --git a/pkg/action/push.go b/pkg/action/push.go index 285005f86..5eb6110b8 100644 --- a/pkg/action/push.go +++ b/pkg/action/push.go @@ -80,7 +80,12 @@ func NewPushWithOpts(opts ...PushOpt) *Push { return p } -// Run executes 'helm push' against the given chart archive. +// Run executes 'helm push' against the given chart archive and returns the +// structured push result containing the ref and manifest digest. +// +// Note: the return type changed from (string, error) to (*registry.PushResult, error) +// in Helm v4 as an intentional breaking change, enabling structured access to +// push metadata without text parsing. func (p *Push) Run(chartRef string, remote string) (*registry.PushResult, error) { c := uploader.ChartUploader{ Out: io.Discard, diff --git a/pkg/cmd/push.go b/pkg/cmd/push.go index 1d135fa64..56d882595 100644 --- a/pkg/cmd/push.go +++ b/pkg/cmd/push.go @@ -19,7 +19,6 @@ package cmd import ( "fmt" "io" - "text/tabwriter" "github.com/spf13/cobra" @@ -58,14 +57,13 @@ type pushWriter struct { result pushResult } -// WriteTable writes a minimal human-readable push result to the provided output. -// The registry client prints progress to stderr; this writes the structured -// result (ref + digest) to the command's stdout stream. +// WriteTable writes the push result in human-readable form, using the same +// "Pushed:"/"Digest:" labels as the registry client's built-in output so that +// the default (--output table) experience is consistent and familiar. func (w *pushWriter) WriteTable(out io.Writer) error { - tw := tabwriter.NewWriter(out, 0, 0, 2, ' ', 0) - fmt.Fprintf(tw, "REF\t%s\n", w.result.Ref) - fmt.Fprintf(tw, "DIGEST\t%s\n", w.result.Digest) - return tw.Flush() + fmt.Fprintf(out, "Pushed: %s\n", w.result.Ref) + fmt.Fprintf(out, "Digest: %s\n", w.result.Digest) + return nil } // WriteJSON writes the push result in JSON format diff --git a/pkg/cmd/push_test.go b/pkg/cmd/push_test.go index 177cb88c6..114a4e38d 100644 --- a/pkg/cmd/push_test.go +++ b/pkg/cmd/push_test.go @@ -28,6 +28,20 @@ func TestPushFileCompletion(t *testing.T) { checkFileCompletion(t, "push package.tgz oci://localhost:5000", false) } +// TestPushOutputFlagCompletion verifies that the --output flag is registered +// on the push command and that its shell completion offers table/json/yaml. +func TestPushOutputFlagCompletion(t *testing.T) { + _, out, err := executeActionCommandC(storageFixture(), "__complete push --output ''") + if err != nil { + t.Fatal(err) + } + for _, want := range []string{"json", "yaml", "table"} { + if !strings.Contains(out, want) { + t.Errorf("output flag completion missing %q, got: %q", want, out) + } + } +} + func TestPushWriterTable(t *testing.T) { w := &pushWriter{result: pushResult{ Ref: "oci://example.com/charts/mychart:1.0.0", @@ -38,8 +52,8 @@ func TestPushWriterTable(t *testing.T) { t.Fatal(err) } got := buf.String() - if !strings.Contains(got, "REF") || !strings.Contains(got, "DIGEST") { - t.Errorf("table output missing headers, got: %q", got) + if !strings.Contains(got, "Pushed:") || !strings.Contains(got, "Digest:") { + t.Errorf("table output missing Pushed:/Digest: labels, got: %q", got) } if !strings.Contains(got, "oci://example.com/charts/mychart:1.0.0") { t.Errorf("table output missing Ref value, got: %q", got) diff --git a/pkg/pusher/pusher.go b/pkg/pusher/pusher.go index adaa26d06..a5df97c26 100644 --- a/pkg/pusher/pusher.go +++ b/pkg/pusher/pusher.go @@ -70,8 +70,13 @@ func WithPlainHTTP(plainHTTP bool) Option { } // Pusher is an interface to support upload to the specified URL. +// +// Note: the Push method signature was updated in Helm v4 to return +// *registry.PushResult alongside the error, enabling callers to obtain +// structured push metadata (ref, digest) without parsing text output. +// This is an intentional breaking change in the v4 major release. type Pusher interface { - // Push file content by url string, returning the push result and any error + // Push uploads the chart at chartRef to url and returns the push result. Push(chartRef, url string, options ...Option) (*registry.PushResult, error) } From 7e92ee69a5e71c660c2649af2e13a3bc237bac15 Mon Sep 17 00:00:00 2001 From: Ilya Kiselev Date: Sun, 5 Apr 2026 22:02:14 +0300 Subject: [PATCH 06/10] cmd: allow overriding registry client writer; suppress in helm push Add an optional io.Writer parameter to newRegistryClient (and the internal newDefaultRegistryClient / newRegistryClientWithTLS helpers) so callers can control where registry client output goes. All existing callers are unaffected (default remains os.Stderr). For helm push, pass io.Discard so that the registry client's built-in "Pushed:"/"Digest:" lines are suppressed. The --output writer (WriteTable / WriteJSON / WriteYAML) is the single source of truth for push result output, preventing duplication on the terminal. Signed-off-by: Ilya Kiselev --- pkg/cmd/push.go | 4 ++++ pkg/cmd/root.go | 22 +++++++++++++++------- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/pkg/cmd/push.go b/pkg/cmd/push.go index 56d882595..71e79a7c7 100644 --- a/pkg/cmd/push.go +++ b/pkg/cmd/push.go @@ -102,8 +102,12 @@ func newPushCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { return noMoreArgsComp() }, RunE: func(_ *cobra.Command, args []string) error { + // Suppress the registry client's built-in "Pushed:"/"Digest:" lines; + // the --output writer (WriteTable/WriteJSON/WriteYAML) is the single + // source of structured push output for this command. registryClient, err := newRegistryClient( o.certFile, o.keyFile, o.caFile, o.insecureSkipTLSVerify, o.plainHTTP, o.username, o.password, + io.Discard, ) if err != nil { diff --git a/pkg/cmd/root.go b/pkg/cmd/root.go index 04ba91c1f..02d004420 100644 --- a/pkg/cmd/root.go +++ b/pkg/cmd/root.go @@ -257,7 +257,7 @@ func newRootCmdWithConfig(actionConfig *action.Configuration, out io.Writer, arg log.Fatal(err) } - registryClient, err := newDefaultRegistryClient(false, "", "") + registryClient, err := newDefaultRegistryClient(false, "", "", os.Stderr) if err != nil { return nil, err } @@ -402,28 +402,36 @@ func checkForExpiredRepos(repofile string) { } +// newRegistryClient creates a registry client. The optional w parameter +// overrides where the registry client writes its output (default: os.Stderr). +// Pass io.Discard to suppress the client's built-in push/pull summary lines. func newRegistryClient( certFile, keyFile, caFile string, insecureSkipTLSVerify, plainHTTP bool, username, password string, + w ...io.Writer, ) (*registry.Client, error) { + out := io.Writer(os.Stderr) + if len(w) > 0 { + out = w[0] + } if certFile != "" && keyFile != "" || caFile != "" || insecureSkipTLSVerify { - registryClient, err := newRegistryClientWithTLS(certFile, keyFile, caFile, insecureSkipTLSVerify, username, password) + registryClient, err := newRegistryClientWithTLS(certFile, keyFile, caFile, insecureSkipTLSVerify, username, password, out) if err != nil { return nil, err } return registryClient, nil } - registryClient, err := newDefaultRegistryClient(plainHTTP, username, password) + registryClient, err := newDefaultRegistryClient(plainHTTP, username, password, out) if err != nil { return nil, err } return registryClient, nil } -func newDefaultRegistryClient(plainHTTP bool, username, password string) (*registry.Client, error) { +func newDefaultRegistryClient(plainHTTP bool, username, password string, out io.Writer) (*registry.Client, error) { opts := []registry.ClientOption{ registry.ClientOptDebug(settings.Debug), registry.ClientOptEnableCache(true), - registry.ClientOptWriter(os.Stderr), + registry.ClientOptWriter(out), registry.ClientOptCredentialsFile(settings.RegistryConfig), registry.ClientOptBasicAuth(username, password), } @@ -440,7 +448,7 @@ func newDefaultRegistryClient(plainHTTP bool, username, password string) (*regis } func newRegistryClientWithTLS( - certFile, keyFile, caFile string, insecureSkipTLSVerify bool, username, password string, + certFile, keyFile, caFile string, insecureSkipTLSVerify bool, username, password string, out io.Writer, ) (*registry.Client, error) { tlsConf, err := tlsutil.NewTLSConfig( tlsutil.WithInsecureSkipVerify(insecureSkipTLSVerify), @@ -456,7 +464,7 @@ func newRegistryClientWithTLS( registryClient, err := registry.NewClient( registry.ClientOptDebug(settings.Debug), registry.ClientOptEnableCache(true), - registry.ClientOptWriter(os.Stderr), + registry.ClientOptWriter(out), registry.ClientOptCredentialsFile(settings.RegistryConfig), registry.ClientOptHTTPClient(&http.Client{ Transport: &http.Transport{ From 6f584a40e0abdad24c5d2b5b366eeacb70b232ee Mon Sep 17 00:00:00 2001 From: Ilya Kiselev Date: Mon, 6 Apr 2026 16:13:38 +0300 Subject: [PATCH 08/10] cmd/action: address Copilot review feedback - root.go: guard against nil writer in newRegistryClient variadic parameter to prevent panic on explicit nil pass - push.go (cmd): replace io.Discard with suppressSummaryWriter that forwards warnings/errors to stderr while silently dropping the registry client's built-in "Pushed:"/"Digest:" summary lines - push.go (action): return a clear error when Run() is called on an OCI remote without WithPushConfig, preventing nil-pointer panic on p.cfg.RegistryClient dereference Signed-off-by: Ilya Kislitsyn Signed-off-by: Ilya Kiselev --- pkg/action/push.go | 4 ++++ pkg/cmd/push.go | 25 +++++++++++++++++++++---- pkg/cmd/root.go | 2 +- 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/pkg/action/push.go b/pkg/action/push.go index 5eb6110b8..4c61856eb 100644 --- a/pkg/action/push.go +++ b/pkg/action/push.go @@ -17,6 +17,7 @@ limitations under the License. package action import ( + "fmt" "io" "helm.sh/helm/v4/pkg/cli" @@ -98,6 +99,9 @@ func (p *Push) Run(chartRef string, remote string) (*registry.PushResult, error) } if registry.IsOCI(remote) { + if p.cfg == nil { + return nil, fmt.Errorf("missing action configuration: use WithPushConfig when constructing Push") + } // Don't use the default registry client if tls options are set. c.Options = append(c.Options, pusher.WithRegistryClient(p.cfg.RegistryClient)) } diff --git a/pkg/cmd/push.go b/pkg/cmd/push.go index 71e79a7c7..defe416c6 100644 --- a/pkg/cmd/push.go +++ b/pkg/cmd/push.go @@ -19,6 +19,7 @@ package cmd import ( "fmt" "io" + "strings" "github.com/spf13/cobra" @@ -57,6 +58,21 @@ type pushWriter struct { result pushResult } +// suppressSummaryWriter forwards all writes to the underlying writer, silently +// dropping lines that match the registry client's built-in "Pushed:"/"Digest:" +// summary output. Warnings, errors, and other output are forwarded intact. +type suppressSummaryWriter struct { + w io.Writer +} + +func (s *suppressSummaryWriter) Write(p []byte) (int, error) { + line := string(p) + if strings.HasPrefix(line, "Pushed: ") || strings.HasPrefix(line, "Digest: ") { + return len(p), nil + } + return s.w.Write(p) +} + // WriteTable writes the push result in human-readable form, using the same // "Pushed:"/"Digest:" labels as the registry client's built-in output so that // the default (--output table) experience is consistent and familiar. @@ -101,13 +117,14 @@ func newPushCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { } return noMoreArgsComp() }, - RunE: func(_ *cobra.Command, args []string) error { - // Suppress the registry client's built-in "Pushed:"/"Digest:" lines; - // the --output writer (WriteTable/WriteJSON/WriteYAML) is the single + RunE: func(cmd *cobra.Command, args []string) error { + // Suppress the registry client's built-in "Pushed:"/"Digest:" summary + // lines while forwarding all other output (warnings, etc.) to stderr. + // The --output writer (WriteTable/WriteJSON/WriteYAML) is the single // source of structured push output for this command. registryClient, err := newRegistryClient( o.certFile, o.keyFile, o.caFile, o.insecureSkipTLSVerify, o.plainHTTP, o.username, o.password, - io.Discard, + &suppressSummaryWriter{w: cmd.ErrOrStderr()}, ) if err != nil { diff --git a/pkg/cmd/root.go b/pkg/cmd/root.go index 02d004420..bbc2739f3 100644 --- a/pkg/cmd/root.go +++ b/pkg/cmd/root.go @@ -410,7 +410,7 @@ func newRegistryClient( w ...io.Writer, ) (*registry.Client, error) { out := io.Writer(os.Stderr) - if len(w) > 0 { + if len(w) > 0 && w[0] != nil { out = w[0] } if certFile != "" && keyFile != "" || caFile != "" || insecureSkipTLSVerify { From 2ce8dbd3a99c6bb6afcb9599d0d784983707a2bb Mon Sep 17 00:00:00 2001 From: Ilya Kiselev Date: Mon, 6 Apr 2026 16:17:53 +0300 Subject: [PATCH 09/10] action: use errors.New instead of fmt.Errorf for static string Fixes revive lint: unnecessary-format. Signed-off-by: Ilya Kislitsyn Signed-off-by: Ilya Kiselev --- pkg/action/push.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/action/push.go b/pkg/action/push.go index 4c61856eb..53343af1b 100644 --- a/pkg/action/push.go +++ b/pkg/action/push.go @@ -17,7 +17,7 @@ limitations under the License. package action import ( - "fmt" + "errors" "io" "helm.sh/helm/v4/pkg/cli" @@ -100,7 +100,7 @@ func (p *Push) Run(chartRef string, remote string) (*registry.PushResult, error) if registry.IsOCI(remote) { if p.cfg == nil { - return nil, fmt.Errorf("missing action configuration: use WithPushConfig when constructing Push") + return nil, errors.New("missing action configuration: use WithPushConfig when constructing Push") } // Don't use the default registry client if tls options are set. c.Options = append(c.Options, pusher.WithRegistryClient(p.cfg.RegistryClient)) From a763b06f11f0596ef3dd7a25785a891322de19c9 Mon Sep 17 00:00:00 2001 From: Ilya Kiselev Date: Tue, 28 Apr 2026 16:30:21 +0300 Subject: [PATCH 10/10] fix(push): correct inaccurate code comments Signed-off-by: Ilya Kiselev --- pkg/action/push.go | 2 +- pkg/cmd/root.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/action/push.go b/pkg/action/push.go index 53343af1b..b60da305f 100644 --- a/pkg/action/push.go +++ b/pkg/action/push.go @@ -102,7 +102,7 @@ func (p *Push) Run(chartRef string, remote string) (*registry.PushResult, error) if p.cfg == nil { return nil, errors.New("missing action configuration: use WithPushConfig when constructing Push") } - // Don't use the default registry client if tls options are set. + // Use the configured registry client for OCI remotes. c.Options = append(c.Options, pusher.WithRegistryClient(p.cfg.RegistryClient)) } diff --git a/pkg/cmd/root.go b/pkg/cmd/root.go index bbc2739f3..ca51e6b91 100644 --- a/pkg/cmd/root.go +++ b/pkg/cmd/root.go @@ -404,7 +404,7 @@ func checkForExpiredRepos(repofile string) { // newRegistryClient creates a registry client. The optional w parameter // overrides where the registry client writes its output (default: os.Stderr). -// Pass io.Discard to suppress the client's built-in push/pull summary lines. +// Pass a filtering writer to control registry client output; pass io.Discard to suppress all output. func newRegistryClient( certFile, keyFile, caFile string, insecureSkipTLSVerify, plainHTTP bool, username, password string, w ...io.Writer,