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