fix: route registry client output to stdout instead of stderr#32056
fix: route registry client output to stdout instead of stderr#32056
Conversation
Commands like 'helm registry login', 'helm push', and 'helm pull' were
writing success messages ("Login Succeeded", "Pushed:", "Pulled:",
"Digest:") to stderr instead of stdout. The root cause was that
newDefaultRegistryClient and newRegistryClientWithTLS hard-coded
os.Stderr as the registry client writer, ignoring the out io.Writer
that main() passes as os.Stdout.
Thread out io.Writer through newRegistryClient, newDefaultRegistryClient,
and newRegistryClientWithTLS, and update all call sites in pkg/cmd.
Fixes helm#13464
Signed-off-by: Terry Howe <terrylhowe@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR updates Helm’s CLI wiring so OCI registry client “success” messages (e.g., Login Succeeded, Pulled:, Pushed:, Digest:) are written to the out io.Writer passed into command construction (typically stdout), rather than being hard-coded to stderr.
Changes:
- Thread
out io.Writerthrough the registry client factory functions inpkg/cmd/root.go. - Update multiple
pkg/cmd/*commands to passoutintonewRegistryClient(...). - Update
show’s helper to acceptoutand propagate it to the registry client creation.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkg/cmd/root.go | Adds out io.Writer plumbed into registry client constructors and sets actionConfig.RegistryClient using it. |
| pkg/cmd/template.go | Passes out into newRegistryClient(...) for OCI chart operations during helm template. |
| pkg/cmd/show.go | Passes out into newRegistryClient(...) for OCI chart operations during helm show *. |
| pkg/cmd/upgrade.go | Passes out into newRegistryClient(...) for OCI chart operations during helm upgrade. |
| pkg/cmd/install.go | Passes out into newRegistryClient(...) for OCI chart operations during helm install. |
| pkg/cmd/pull.go | Passes out into newRegistryClient(...) for OCI pulls. |
| pkg/cmd/push.go | Passes out into newRegistryClient(...) for OCI pushes. |
| pkg/cmd/package.go | Passes out into newRegistryClient(...) when packaging with OCI-related chart paths/options. |
| pkg/cmd/dependency_build.go | Passes out into newRegistryClient(...) for OCI dependency fetching. |
| pkg/cmd/dependency_update.go | Passes out into newRegistryClient(...) for OCI dependency fetching. |
Comments suppressed due to low confidence (4)
pkg/cmd/template.go:93
- Passing
out(stdout) to the registry client will cause OCI pull status lines (e.g., "Pulled:" / "Digest:") emitted byregistry.Clientto be written to the same stream as the rendered manifests, which breakshelm templateoutput for piping/automation. Consider routing registry client output tocmd.ErrOrStderr()(or another stderr writer) here so the manifest output onoutstays clean.
registryClient, err := newRegistryClient(out, client.CertFile, client.KeyFile, client.CaFile,
client.InsecureSkipTLSVerify, client.PlainHTTP, client.Username, client.Password)
if err != nil {
return fmt.Errorf("missing registry client: %w", err)
}
client.SetRegistryClient(registryClient)
pkg/cmd/install.go:151
helm installsupports machine-readable output modes via--output(outfmt.Write(out, ...)). If the chart is an OCI reference, the registry client will write status lines ("Pulled:" / "Digest:") to its writer; usingouthere can corrupt JSON/YAML output. Consider sending registry client output tocmd.ErrOrStderr()(or another stderr writer) instead ofoutso formatted output remains valid.
registryClient, err := newRegistryClient(out, client.CertFile, client.KeyFile, client.CaFile,
client.InsecureSkipTLSVerify, client.PlainHTTP, client.Username, client.Password)
if err != nil {
return fmt.Errorf("missing registry client: %w", err)
}
client.SetRegistryClient(registryClient)
pkg/cmd/upgrade.go:113
helm upgradesupports formatted outputs (table/json/yaml viaoutfmt). For OCI charts, the registry client writes status lines ("Pulled:" / "Digest:") to its configured writer; passingouthere can break machine-readable output by mixing in those lines. Consider routing registry client output tocmd.ErrOrStderr()(or another stderr writer) instead ofout.
registryClient, err := newRegistryClient(out, client.CertFile, client.KeyFile, client.CaFile,
client.InsecureSkipTLSVerify, client.PlainHTTP, client.Username, client.Password)
if err != nil {
return fmt.Errorf("missing registry client: %w", err)
}
client.SetRegistryClient(registryClient)
pkg/cmd/root.go:264
- This change alters user-visible stream routing by wiring the registry client’s writer to the provided
out. There doesn’t appear to be cmd-level test coverage asserting that registry success messages (e.g., "Login Succeeded" / "Pulled:" / "Pushed:") land on the intended stream; current helpers generally merge stdout/stderr into one buffer. Adding a focused test that keeps stdout/stderr separate would help prevent regressions in output redirection behavior.
registryClient, err := newDefaultRegistryClient(out, false, "", "")
if err != nil {
return nil, err
}
actionConfig.RegistryClient = registryClient
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func addRegistryClient(out io.Writer, client *action.Show) error { | ||
| registryClient, err := newRegistryClient(out, client.CertFile, client.KeyFile, client.CaFile, |
There was a problem hiding this comment.
helm show writes chart contents to out (often consumed programmatically), but the registry client also writes OCI pull status lines ("Pulled:" / "Digest:") to its writer. Passing out here will interleave those status lines with the chart output. Consider using stderr (e.g., cmd.ErrOrStderr() from each subcommand) for the registry client writer, and keep out only for the command’s primary output.
| func addRegistryClient(out io.Writer, client *action.Show) error { | |
| registryClient, err := newRegistryClient(out, client.CertFile, client.KeyFile, client.CaFile, | |
| func addRegistryClient(errOut io.Writer, client *action.Show) error { | |
| registryClient, err := newRegistryClient(errOut, client.CertFile, client.KeyFile, client.CaFile, |
Documentation UpdateI've prepared a documentation update for this change: helm/helm-www#2079 - Adds upgrade guidance noting that scripts capturing registry command output (like This helps users migrating to Helm 4 understand they may need to update scripts that rely on the previous stderr behavior. 🤖 Generated by Promptless |
scottrigby
left a comment
There was a problem hiding this comment.
This lgtm. Function signature changes are all internal. non-error CLI output moving from stderr to stdout is expected, so this is a bug fix.
Summary
helm registry login,helm push, andhelm pullwere writing success messages (Login Succeeded,Pushed:,Pulled:,Digest:) to stderr instead of stdoutnewDefaultRegistryClientandnewRegistryClientWithTLSinpkg/cmd/root.gohard-codedos.Stderras the registry client writer, ignoring theout io.Writer(os.Stdout) passed frommain()out io.Writerthrough the three registry client factory functions and update all call sites inpkg/cmd/Debug/warning output via
slogremains on stderr. Only user-facing success messages move to stdout.Test plan
helm registry login <host> -u user -p pass > stdout.log 2> stderr.log— verifyLogin Succeededappears instdout.loghelm push chart.tgz oci://... > stdout.log 2> stderr.log— verifyPushed:andDigest:appear instdout.loghelm pull oci://... > stdout.log 2> stderr.log— verifyPulled:andDigest:appear instdout.loggo test ./pkg/cmd/...Closes: #13464
🤖 Generated with Claude Code