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..9355fb474 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" @@ -56,11 +55,3 @@ func TestNewPushWithPlainHTTP(t *testing.T) { assert.NotNil(t, client) 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) -} diff --git a/pkg/cmd/push.go b/pkg/cmd/push.go index 94c5732ff..defe416c6 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,52 @@ 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 { + 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 +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 +117,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( o.certFile, o.keyFile, o.caFile, o.insecureSkipTLSVerify, o.plainHTTP, o.username, o.password, + &suppressSummaryWriter{w: cmd.ErrOrStderr()}, ) if err != nil { @@ -83,15 +136,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 +161,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..114a4e38d 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,80 @@ 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) + } +} diff --git a/pkg/cmd/root.go b/pkg/cmd/root.go index 04ba91c1f..ca51e6b91 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 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, ) (*registry.Client, error) { + out := io.Writer(os.Stderr) + if len(w) > 0 && w[0] != nil { + 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{ 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..a5df97c26 100644 --- a/pkg/pusher/pusher.go +++ b/pkg/pusher/pusher.go @@ -70,9 +70,14 @@ 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 - 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..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...)