Merge 97acf9d093 into d5465e930e

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)
 }

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")
 }

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
 }

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())
+	}
+}

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

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),
 	)

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

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...)