From 97acf9d093d17a7337b9cc6ba5fd23a28dd9b7c1 Mon Sep 17 00:00:00 2001 From: Ilya Kiselev Date: Fri, 19 Jun 2026 23:32:50 +0300 Subject: [PATCH] feat(push): add --output json/yaml/table flag to helm push Add support for the --output flag to 'helm push' command for machine-readable output formats (JSON and YAML), enabling programmatic consumption of push results and integration with tools like cosign for artifact signing. Signed-off-by: Ilya Kiselev --- pkg/action/push.go | 30 +++++------ pkg/action/push_test.go | 11 ++-- pkg/cmd/push.go | 77 ++++++++++++++++++++++++--- pkg/cmd/push_test.go | 95 ++++++++++++++++++++++++++++++++++ pkg/pusher/ocipusher.go | 21 ++++---- pkg/pusher/ocipusher_test.go | 6 +-- pkg/pusher/pusher.go | 11 +++- pkg/uploader/chart_uploader.go | 11 ++-- 8 files changed, 211 insertions(+), 51 deletions(-) diff --git a/pkg/action/push.go b/pkg/action/push.go index 0c7148f65..b60da305f 100644 --- a/pkg/action/push.go +++ b/pkg/action/push.go @@ -17,8 +17,8 @@ limitations under the License. package action import ( + "errors" "io" - "strings" "helm.sh/helm/v4/pkg/cli" "helm.sh/helm/v4/pkg/pusher" @@ -37,7 +37,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. @@ -73,13 +72,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{} @@ -89,12 +81,15 @@ func NewPushWithOpts(opts ...PushOpt) *Push { return p } -// Run executes 'helm push' against the given chart archive. -func (p *Push) Run(chartRef string, remote string) (string, error) { - var out strings.Builder - +// 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: &out, + Out: io.Discard, Pushers: pusher.All(p.Settings), Options: []pusher.Option{ pusher.WithTLSClientConfig(p.certFile, p.keyFile, p.caFile), @@ -104,9 +99,12 @@ func (p *Push) Run(chartRef string, remote string) (string, error) { } if registry.IsOCI(remote) { - // Don't use the default registry client if tls options are set. + if p.cfg == nil { + return nil, errors.New("missing action configuration: use WithPushConfig when constructing Push") + } + // Use the configured registry client for OCI remotes. 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/action/push_test.go b/pkg/action/push_test.go index 125799252..c069c48aa 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,8 @@ func TestNewPushWithPlainHTTP(t *testing.T) { assert.True(t, 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) +func TestPushRunNilConfigOCI(t *testing.T) { + p := NewPushWithOpts() + _, err := p.Run("testdata/testcharts/subchart-1.tgz", "oci://localhost:5000/test") + assert.EqualError(t, err, "missing action configuration: use WithPushConfig when constructing Push") } diff --git a/pkg/cmd/push.go b/pkg/cmd/push.go index f32ce92be..0219a7fac 100644 --- a/pkg/cmd/push.go +++ b/pkg/cmd/push.go @@ -19,10 +19,12 @@ package cmd import ( "fmt" "io" + "strings" "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 +44,56 @@ 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 +} + +// 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. +func (w *pushWriter) WriteTable(out io.Writer) error { + if _, err := fmt.Fprintf(out, "Pushed: %s\n", w.result.Ref); err != nil { + return err + } + if _, err := fmt.Fprintf(out, "Digest: %s\n", w.result.Digest); err != nil { + return err + } + 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 { @@ -69,9 +121,14 @@ func newPushCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { } return noMoreArgsComp() }, - RunE: func(_ *cobra.Command, args []string) error { + 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( - out, o.certFile, o.keyFile, o.caFile, o.insecureSkipTLSVerify, o.plainHTTP, o.username, o.password, + &suppressSummaryWriter{w: cmd.ErrOrStderr()}, + o.certFile, o.keyFile, o.caFile, o.insecureSkipTLSVerify, o.plainHTTP, o.username, o.password, ) if err != nil { @@ -83,15 +140,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 +165,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..525824e2f 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,96 @@ func TestPushFileCompletion(t *testing.T) { checkFileCompletion(t, "push package.tgz", false) 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", + Digest: "sha256:abc123", + }} + var buf bytes.Buffer + if err := w.WriteTable(&buf); err != nil { + t.Fatal(err) + } + got := buf.String() + 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) + } + if !strings.Contains(got, "sha256:abc123") { + t.Errorf("table output missing Digest value, 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) + } +} + +func TestSuppressSummaryWriter(t *testing.T) { + var buf bytes.Buffer + w := &suppressSummaryWriter{w: &buf} + + w.Write([]byte("Pushed: oci://example.com/chart:1.0.0\n")) + w.Write([]byte("Digest: sha256:abc123\n")) + if buf.Len() != 0 { + t.Errorf("expected summary lines to be suppressed, got: %q", buf.String()) + } + + w.Write([]byte("WARNING: chart name contains underscores\n")) + if !strings.Contains(buf.String(), "WARNING:") { + t.Errorf("expected non-summary output to be forwarded, got: %q", buf.String()) + } +} 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..8d5d43eeb 100644 --- a/pkg/pusher/pusher.go +++ b/pkg/pusher/pusher.go @@ -70,9 +70,16 @@ func WithPlainHTTP(plainHTTP bool) Option { } // Pusher is an interface to support upload to the specified URL. +// Currently only OCI (oci://) registries are supported; the returned +// *registry.PushResult contains OCI-specific manifest metadata (ref, digest). +// +// Note: the Push method signature was updated in Helm v4 to return +// *registry.PushResult alongside the error, enabling callers to obtain +// structured push metadata without parsing text output. +// This is an intentional breaking change in the v4 major release. type Pusher interface { - // Push file content by url string - Push(chartRef, url string, options ...Option) error + // Push uploads the chart at chartRef to url and returns the push result. + 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..9c41fa14a 100644 --- a/pkg/uploader/chart_uploader.go +++ b/pkg/uploader/chart_uploader.go @@ -36,20 +36,21 @@ type ChartUploader struct { RegistryClient *registry.Client } -// UploadTo uploads a chart. Depending on the settings, it may also upload a provenance file. -func (c *ChartUploader) UploadTo(ref, remote string) error { +// UploadTo uploads a chart to the remote URL and returns the push result containing +// the ref and manifest digest. Depending on the settings, it may also upload a provenance file. +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...)