From 4c6f62b3f53be659601dfc2b7ec09b047cb13cb0 Mon Sep 17 00:00:00 2001 From: Jojin Date: Sat, 25 Jul 2026 15:42:56 +0530 Subject: [PATCH] feat(push): add -o/--output flag (table|json|yaml) to helm push Adds the standard -o/--output flag to 'helm push' so the pushed reference and manifest digest can be consumed programmatically (e.g. to sign the chart with cosign). The push result is plumbed from the OCI registry client to the CLI through a new, purely additive functional option (pusher.WithPushResultHandler / action.WithPushResultHandler), so no public Go SDK signatures change (HIP-0004). The default table output is byte-for-byte identical to the previous behavior; for json/yaml the registry client's human-readable summary is suppressed so stdout contains only the structured result. Closes #11735 Signed-off-by: Jojin --- pkg/action/push.go | 10 ++++ pkg/action/push_test.go | 9 ++++ pkg/cmd/push.go | 69 ++++++++++++++++++++++++-- pkg/cmd/push_test.go | 93 ++++++++++++++++++++++++++++++++++++ pkg/pusher/ocipusher.go | 10 +++- pkg/pusher/ocipusher_test.go | 10 ++++ pkg/pusher/pusher.go | 9 ++++ 7 files changed, 203 insertions(+), 7 deletions(-) diff --git a/pkg/action/push.go b/pkg/action/push.go index 0c7148f65..ead388539 100644 --- a/pkg/action/push.go +++ b/pkg/action/push.go @@ -38,6 +38,7 @@ type Push struct { insecureSkipTLSVerify bool plainHTTP bool out io.Writer + pushResultHandler func(*registry.PushResult) } // PushOpt is a type of function that sets options for a push action. @@ -80,6 +81,14 @@ func WithPushOptWriter(out io.Writer) PushOpt { } } +// WithPushResultHandler sets a handler on the push configuration object which +// is called with the result of a successful push. +func WithPushResultHandler(handler func(result *registry.PushResult)) PushOpt { + return func(p *Push) { + p.pushResultHandler = handler + } +} + // NewPushWithOpts creates a new push, with configuration options. func NewPushWithOpts(opts ...PushOpt) *Push { p := &Push{} @@ -100,6 +109,7 @@ func (p *Push) Run(chartRef string, remote string) (string, error) { pusher.WithTLSClientConfig(p.certFile, p.keyFile, p.caFile), pusher.WithInsecureSkipTLSVerify(p.insecureSkipTLSVerify), pusher.WithPlainHTTP(p.plainHTTP), + pusher.WithPushResultHandler(p.pushResultHandler), }, } diff --git a/pkg/action/push_test.go b/pkg/action/push_test.go index 125799252..84ef1769f 100644 --- a/pkg/action/push_test.go +++ b/pkg/action/push_test.go @@ -21,6 +21,8 @@ import ( "testing" "github.com/stretchr/testify/assert" + + "helm.sh/helm/v4/pkg/registry" ) func TestNewPushWithPushConfig(t *testing.T) { @@ -64,3 +66,10 @@ func TestNewPushWithPushOptWriter(t *testing.T) { assert.NotNil(t, client) assert.Equal(t, buf, client.out) } + +func TestNewPushWithPushResultHandler(t *testing.T) { + client := NewPushWithOpts(WithPushResultHandler(func(_ *registry.PushResult) {})) + + assert.NotNil(t, client) + assert.NotNil(t, client.pushResultHandler) +} diff --git a/pkg/cmd/push.go b/pkg/cmd/push.go index f32ce92be..f3b128185 100644 --- a/pkg/cmd/push.go +++ b/pkg/cmd/push.go @@ -23,8 +23,10 @@ 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" + "helm.sh/helm/v4/pkg/registry" ) const pushDesc = ` @@ -46,6 +48,7 @@ type registryPushOptions struct { func newPushCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { o := ®istryPushOptions{} + var outfmt output.Format cmd := &cobra.Command{ Use: "push [chart] [remote]", @@ -70,8 +73,15 @@ func newPushCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { return noMoreArgsComp() }, RunE: func(_ *cobra.Command, args []string) error { + // The registry client writes a human-readable push summary + // directly to its writer. Suppress it for the machine-readable + // output formats so that only the structured result is written. + registryClientOut := out + if outfmt != output.Table { + registryClientOut = io.Discard + } registryClient, err := newRegistryClient( - out, o.certFile, o.keyFile, o.caFile, o.insecureSkipTLSVerify, o.plainHTTP, o.username, o.password, + registryClientOut, o.certFile, o.keyFile, o.caFile, o.insecureSkipTLSVerify, o.plainHTTP, o.username, o.password, ) if err != nil { @@ -80,18 +90,28 @@ func newPushCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { cfg.RegistryClient = registryClient chartRef := args[0] remote := args[1] + var result *registry.PushResult 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.WithPushOptWriter(out), + action.WithPushResultHandler(func(r *registry.PushResult) { + result = r + })) client.Settings = settings - output, err := client.Run(chartRef, remote) + uploadOutput, err := client.Run(chartRef, remote) if err != nil { return err } - fmt.Fprint(out, output) - return nil + if outfmt == output.Table { + fmt.Fprint(out, uploadOutput) + return nil + } + if result == nil { + return fmt.Errorf("no push result available to write as %s", outfmt) + } + return outfmt.Write(out, newPushWriter(result)) }, } @@ -104,5 +124,44 @@ 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, &outfmt) + return cmd } + +// pushResult is the structure written by the push command for the +// machine-readable output formats. +type pushResult struct { + Ref string `json:"ref"` + Digest string `json:"digest"` +} + +type pushWriter struct { + result pushResult +} + +func newPushWriter(result *registry.PushResult) *pushWriter { + w := &pushWriter{result: pushResult{Ref: result.Ref}} + if result.Manifest != nil { + w.result.Digest = result.Manifest.Digest + } + return w +} + +// WriteTable mirrors the push summary the registry client writes for the +// default table output format. +func (w *pushWriter) WriteTable(out io.Writer) error { + if _, err := fmt.Fprintf(out, "Pushed: %s\n", w.result.Ref); err != nil { + return err + } + _, err := fmt.Fprintf(out, "Digest: %s\n", w.result.Digest) + return err +} + +func (w *pushWriter) WriteJSON(out io.Writer) error { + return output.EncodeJSON(out, w.result) +} + +func (w *pushWriter) WriteYAML(out io.Writer) error { + return output.EncodeYAML(out, w.result) +} diff --git a/pkg/cmd/push_test.go b/pkg/cmd/push_test.go index 80d08b48f..c144c7bcb 100644 --- a/pkg/cmd/push_test.go +++ b/pkg/cmd/push_test.go @@ -17,9 +17,102 @@ limitations under the License. package cmd import ( + "encoding/json" + "fmt" + "path/filepath" "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "sigs.k8s.io/yaml" + + "helm.sh/helm/v4/pkg/repo/v1/repotest" ) +func TestPushCmd(t *testing.T) { + srv := repotest.NewTempServer( + t, + repotest.WithChartSourceGlob("testdata/testcharts/*.tgz*"), + ) + defer srv.Stop() + + ociSrv, err := repotest.NewOCIServer(t, srv.Root()) + require.NoError(t, err) + ociSrv.Run(t) + + ref := ociSrv.RegistryURL + "/u/ocitestuser/compressedchart:0.1.0" + digestPattern := `^sha256:[0-9a-f]{64}$` + + tests := []struct { + name string + format string + check func(t *testing.T, out string) + }{ + { + name: "push with default table output", + check: func(t *testing.T, out string) { + t.Helper() + assert.Contains(t, out, fmt.Sprintf("Pushed: %s\n", ref)) + assert.Regexp(t, `Digest: sha256:[0-9a-f]{64}\n`, out) + }, + }, + { + name: "push with table output", + format: "table", + check: func(t *testing.T, out string) { + t.Helper() + assert.Contains(t, out, fmt.Sprintf("Pushed: %s\n", ref)) + assert.Regexp(t, `Digest: sha256:[0-9a-f]{64}\n`, out) + }, + }, + { + name: "push with json output", + format: "json", + check: func(t *testing.T, out string) { + t.Helper() + result := map[string]string{} + require.NoError(t, json.Unmarshal([]byte(out), &result), "expected pure JSON output, got %q", out) + assert.Equal(t, ref, result["ref"]) + assert.Regexp(t, digestPattern, result["digest"]) + }, + }, + { + name: "push with yaml output", + format: "yaml", + check: func(t *testing.T, out string) { + t.Helper() + result := map[string]string{} + require.NoError(t, yaml.Unmarshal([]byte(out), &result), "expected pure YAML output, got %q", out) + assert.Equal(t, ref, result["ref"]) + assert.Regexp(t, digestPattern, result["digest"]) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cmd := fmt.Sprintf("push testdata/testcharts/compressedchart-0.1.0.tgz oci://%s/u/ocitestuser --registry-config %s --plain-http", + ociSrv.RegistryURL, + filepath.Join(srv.Root(), "config.json"), + ) + if tt.format != "" { + cmd += " --output " + tt.format + } + _, out, err := executeActionCommand(cmd) + require.NoError(t, err) + tt.check(t, out) + }) + } +} + +func TestPushOutputCompletion(t *testing.T) { + runTestCmd(t, []cmdTestCase{{ + name: "completion for output flag of push", + cmd: "__complete push --output ''", + golden: "output/output-comp.txt", + }}) +} + func TestPushFileCompletion(t *testing.T) { checkFileCompletion(t, "push", true) checkFileCompletion(t, "push package.tgz", false) diff --git a/pkg/pusher/ocipusher.go b/pkg/pusher/ocipusher.go index 2a12e09b4..d9e4cd92d 100644 --- a/pkg/pusher/ocipusher.go +++ b/pkg/pusher/ocipusher.go @@ -93,8 +93,14 @@ 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 + result, err := client.Push(chartBytes, ref, pushOpts...) + if err != nil { + return err + } + if pusher.opts.pushResultHandler != nil { + pusher.opts.pushResultHandler(result) + } + return nil } // NewOCIPusher constructs a valid OCI client as a Pusher diff --git a/pkg/pusher/ocipusher_test.go b/pkg/pusher/ocipusher_test.go index ceb61d442..64b0343fe 100644 --- a/pkg/pusher/ocipusher_test.go +++ b/pkg/pusher/ocipusher_test.go @@ -70,6 +70,16 @@ func TestNewOCIPusher(t *testing.T) { op, ok = p.(*OCIPusher) require.True(t, ok, "expected NewOCIPusher to produce an *OCIPusher") assert.Equal(t, registryClient, op.opts.registryClient, "Expected NewOCIPusher to contain %p as RegistryClient, got %p", registryClient, op.opts.registryClient) + + // Test if setting pushResultHandler is being passed to the ops + p, err = NewOCIPusher( + WithPushResultHandler(func(_ *registry.PushResult) {}), + ) + require.NoError(t, err) + + op, ok = p.(*OCIPusher) + require.True(t, ok, "expected NewOCIPusher to produce an *OCIPusher") + assert.NotNil(t, op.opts.pushResultHandler, "Expected NewOCIPusher to contain a push result handler") } func TestOCIPusher_Push_ErrorHandling(t *testing.T) { diff --git a/pkg/pusher/pusher.go b/pkg/pusher/pusher.go index 8ce78b011..0fca6e073 100644 --- a/pkg/pusher/pusher.go +++ b/pkg/pusher/pusher.go @@ -34,6 +34,7 @@ type options struct { caFile string insecureSkipTLSVerify bool plainHTTP bool + pushResultHandler func(*registry.PushResult) } // Option allows specifying various settings configurable by the user for overriding the defaults @@ -69,6 +70,14 @@ func WithPlainHTTP(plainHTTP bool) Option { } } +// WithPushResultHandler sets a handler which is called with the result of a +// successful push. Pushers which do not produce a push result ignore it. +func WithPushResultHandler(handler func(result *registry.PushResult)) Option { + return func(opts *options) { + opts.pushResultHandler = handler + } +} + // Pusher is an interface to support upload to the specified URL. type Pusher interface { // Push file content by url string