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