Skip to content

Commit f33d278

Browse files
JusticeleegJusticeleeg
authored
fix: cli stdout warnings corrupt redirected output (#710)
* prompts * Revert "prompts" This reverts commit 890004e. * fix(cli): route cobra-emitted text to stderr Cobra's auto-emitted output (deprecation warnings, usage-on-error) was going to stdout, corrupting redirected structured output such as `cluster kubeconfig --stdout > kubeconfig.yaml`. Align with cobra's default by setting Out to stderr. Refs sc-136974 * fix(cluster kubeconfig): route diagnostic prints to stderr Move the kubeconfig success, backup-removal failure, and context-updated messages from stdout to stderr so they no longer corrupt the YAML payload when output is redirected. Also switch the --stdout data path from Println to Print to avoid an extra trailing newline. Refs sc-136974 * fix(release lint): keep auto-discovery messages out of -o json output The auto-discovery branch printed informational text to r.w (stdout) regardless of output format, breaking JSON parsers when running `release lint -o json` outside a configured project. Gate the discovery messages on table format, and have the empty-result early return emit a valid JSON payload so consumers always receive parseable output. Refs sc-136974 * fix(logger): inspect configured writer for TTY detection Logger TTY checks were hardcoded to os.Stdout.Fd(), so spinners fired even when the logger had been pointed at a non-TTY writer. Replace each call site with an isTTY helper that type-asserts the configured writer. Switch the two `release download` callers from os.Stdout to os.Stderr — the command's data path writes files to disk, so progress messages belong on stderr. Refs sc-136974 * fix(app rm): suppress spinners when -o json is requested `app rm` shares its logger writer with structured output, so the fetch/delete spinners leaked into the JSON payload. Gate the spinner calls on `outputFormat == \"table\"` to keep the JSON output clean. Refs sc-136974 * fix(logger): preserve spinner UX when writer wraps stdout The previous patch broke spinners for any logger built from a tabwriter wrapping stdout: the type assertion to *os.File failed, so isTTY() always returned false. Capture the stdout TTY state once in Execute() and let callers thread it through with SetIsTerminal so wrapped writers still spin when stdout is a real terminal. Refs sc-136974 * fix(cli): keep --help on stdout while routing diagnostics to stderr Setting cobra Out to stderr also redirected --help, breaking `replicated --help | grep ...` and `--help > help.txt`. Restore the CLI convention by overriding the root HelpFunc to write to stdout while leaving deprecation warnings and usage-on-error on stderr. Refs sc-136974
1 parent a36c04a commit f33d278

11 files changed

Lines changed: 98 additions & 36 deletions

‎cli/cmd/app_hostname_ls.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ replicated app hostname ls --app myapp --output json`,
7979
func (r *runners) listAppHostnames(ctx context.Context, outputFormat string) error {
8080
// Only show spinners for table output
8181
showSpinners := outputFormat == "table"
82-
log := logger.NewLogger(r.w)
82+
log := logger.NewLogger(r.w).SetIsTerminal(r.stdoutIsTTY)
8383

8484
// Resolve app ID from slug or ID
8585
appSlugOrID := r.appSlug

‎cli/cmd/app_rm.go‎

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -54,15 +54,22 @@ replicated app delete "Custom App" --output json`,
5454
}
5555

5656
func (r *runners) deleteApp(ctx context.Context, cmd *cobra.Command, appName string, opts deleteAppOpts, outputFormat string) error {
57-
log := logger.NewLogger(r.w)
57+
log := logger.NewLogger(r.w).SetIsTerminal(r.stdoutIsTTY)
58+
showSpinners := outputFormat == "table"
5859

59-
log.ActionWithSpinner("Fetching App")
60+
if showSpinners {
61+
log.ActionWithSpinner("Fetching App")
62+
}
6063
app, err := r.kotsAPI.GetApp(ctx, appName, true)
6164
if err != nil {
62-
log.FinishSpinnerWithError()
65+
if showSpinners {
66+
log.FinishSpinnerWithError()
67+
}
6368
return errors.Wrap(err, "list apps")
6469
}
65-
log.FinishSpinner()
70+
if showSpinners {
71+
log.FinishSpinner()
72+
}
6673

6774
apps := []types.AppAndChannels{
6875
{
@@ -86,13 +93,19 @@ func (r *runners) deleteApp(ctx context.Context, cmd *cobra.Command, appName str
8693
}
8794
}
8895

89-
log.ActionWithSpinner("Deleting App")
96+
if showSpinners {
97+
log.ActionWithSpinner("Deleting App")
98+
}
9099
err = r.kotsAPI.DeleteKOTSApp(ctx, app.ID)
91100
if err != nil {
92-
log.FinishSpinnerWithError()
101+
if showSpinners {
102+
log.FinishSpinnerWithError()
103+
}
93104
return errors.Wrap(err, "delete app")
94105
}
95-
log.FinishSpinner()
106+
if showSpinners {
107+
log.FinishSpinner()
108+
}
96109

97110
return nil
98111
}

‎cli/cmd/cluster_kubeconfig.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ func (r *runners) kubeconfigCluster(_ *cobra.Command, args []string) error {
108108
}
109109

110110
if r.args.kubeconfigStdout {
111-
fmt.Println(string(kubeconfig))
111+
fmt.Print(string(kubeconfig))
112112
return nil
113113
}
114114

@@ -123,7 +123,7 @@ func (r *runners) kubeconfigCluster(_ *cobra.Command, args []string) error {
123123
return errors.Wrap(err, "write kubeconfig")
124124
}
125125

126-
fmt.Printf("kubeconfig written to %s\n", r.args.kubeconfigPath)
126+
fmt.Fprintf(os.Stderr, "kubeconfig written to %s\n", r.args.kubeconfigPath)
127127
return nil
128128
}
129129

@@ -180,7 +180,7 @@ func (r *runners) kubeconfigCluster(_ *cobra.Command, args []string) error {
180180
for _, backupPath := range backupPaths {
181181
err := os.Remove(backupPath)
182182
if err != nil {
183-
fmt.Printf("failed to remove backup kubeconfig: %s\n", err.Error())
183+
fmt.Fprintf(os.Stderr, "failed to remove backup kubeconfig: %s\n", err.Error())
184184
}
185185
}
186186
}()
@@ -215,7 +215,7 @@ func (r *runners) kubeconfigCluster(_ *cobra.Command, args []string) error {
215215
return errors.Wrap(err, "write kubeconfig")
216216
}
217217

218-
fmt.Printf(" ✓ Updated kubernetes context '%s' in '%s'\n", mergedConfig.CurrentContext, kubeconfigPaths[0])
218+
fmt.Fprintf(os.Stderr, " ✓ Updated kubernetes context '%s' in '%s'\n", mergedConfig.CurrentContext, kubeconfigPaths[0])
219219

220220
return nil
221221
}

‎cli/cmd/cluster_prepare.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ func (r *runners) prepareCluster(_ *cobra.Command, args []string) error {
154154
return errors.New("no app specified")
155155
}
156156

157-
log := logger.NewLogger(r.w)
157+
log := logger.NewLogger(r.w).SetIsTerminal(r.stdoutIsTTY)
158158

159159
release, err := prepareRelease(r, log)
160160
if err != nil {

‎cli/cmd/installer_create.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ func (r *runners) installerCreate(_ *cobra.Command, _ []string) error {
5858
return errors.Errorf("Installer specs are only supported for KOTS applications, app %q has type %q", r.appID, r.appType)
5959
}
6060

61-
log := logger.NewLogger(r.w)
61+
log := logger.NewLogger(r.w).SetIsTerminal(r.stdoutIsTTY)
6262
if r.args.createInstallerAutoDefaults {
6363
log.ActionWithSpinner("Reading Environment")
6464
err := r.setKOTSDefaultInstallerParams()

‎cli/cmd/lint.go‎

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -147,8 +147,12 @@ func (r *runners) runLint(cmd *cobra.Command, args []string) error {
147147
autoDiscoveryMode := len(config.Charts) == 0 && len(config.Preflights) == 0 && len(config.Manifests) == 0
148148

149149
if autoDiscoveryMode {
150-
fmt.Fprintf(r.w, "No .replicated config found. Auto-discovering lintable resources in current directory...\n\n")
151-
r.w.Flush()
150+
showAutoDiscoveryMessages := r.outputFormat == "table"
151+
152+
if showAutoDiscoveryMessages {
153+
fmt.Fprintf(r.w, "No .replicated config found. Auto-discovering lintable resources in current directory...\n\n")
154+
r.w.Flush()
155+
}
152156

153157
// Auto-discover Helm charts (for counting and display)
154158
chartPaths, err := lint2.DiscoverChartPaths(filepath.Join(".", "**"))
@@ -190,18 +194,28 @@ func (r *runners) runLint(cmd *cobra.Command, args []string) error {
190194
}
191195

192196
// Print what was discovered
193-
fmt.Fprintf(r.w, "Discovered resources:\n")
194-
fmt.Fprintf(r.w, " - %d Helm chart(s)\n", len(chartPaths))
195-
fmt.Fprintf(r.w, " - %d Preflight spec(s)\n", len(preflightPaths))
196-
fmt.Fprintf(r.w, " - %d Support Bundle spec(s)\n", len(sbPaths))
197-
fmt.Fprintf(r.w, " - %d HelmChart manifest(s)\n\n", len(helmChartPaths))
198-
r.w.Flush()
197+
if showAutoDiscoveryMessages {
198+
fmt.Fprintf(r.w, "Discovered resources:\n")
199+
fmt.Fprintf(r.w, " - %d Helm chart(s)\n", len(chartPaths))
200+
fmt.Fprintf(r.w, " - %d Preflight spec(s)\n", len(preflightPaths))
201+
fmt.Fprintf(r.w, " - %d Support Bundle spec(s)\n", len(sbPaths))
202+
fmt.Fprintf(r.w, " - %d HelmChart manifest(s)\n\n", len(helmChartPaths))
203+
r.w.Flush()
204+
}
199205

200206
// If nothing was found and EC linting is not enabled, exit early.
201207
// EC linting runs after this block, so don't bail out when it's enabled.
202208
if len(chartPaths) == 0 && len(preflightPaths) == 0 && len(sbPaths) == 0 && !config.ReplLint.Linters.EmbeddedCluster.IsEnabled() {
203-
fmt.Fprintf(r.w, "No lintable resources found in current directory.\n")
204-
r.w.Flush()
209+
if showAutoDiscoveryMessages {
210+
fmt.Fprintf(r.w, "No lintable resources found in current directory.\n")
211+
r.w.Flush()
212+
}
213+
if r.outputFormat == "json" {
214+
output.Summary = r.calculateOverallSummary(output)
215+
if err := print.LintResults(r.outputFormat, r.w, output); err != nil {
216+
return errors.Wrap(err, "failed to print JSON output to stdout")
217+
}
218+
}
205219
return nil
206220
}
207221
}

‎cli/cmd/release_create.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,7 @@ func (r *runners) releaseCreate(cmd *cobra.Command, args []string) (err error) {
230230
printIfError(cmd, err)
231231
}()
232232

233-
log := logger.NewLogger(r.w)
233+
log := logger.NewLogger(r.w).SetIsTerminal(r.stdoutIsTTY)
234234
if r.outputFormat == "json" {
235235
// suppress log lines for machine-readable output
236236
log.Silence()

‎cli/cmd/release_download.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ func (r *runners) releaseDownload(command *cobra.Command, args []string) error {
105105
return r.releaseInspect(command, args)
106106
}
107107

108-
log := logger.NewLogger(os.Stdout)
108+
log := logger.NewLogger(os.Stderr)
109109

110110
// Determine sequence to download
111111
var seq int64
@@ -280,7 +280,7 @@ func (r *runners) downloadReleaseArchive(seq int64, dest string) error {
280280
}
281281
defer os.RemoveAll(tempDir)
282282

283-
log := logger.NewLogger(os.Stdout)
283+
log := logger.NewLogger(os.Stderr)
284284
if err := kotsrelease.Save(tempDir, release, log); err != nil {
285285
return errors.Wrap(err, "save release to temp dir")
286286
}

‎cli/cmd/root.go‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"text/tabwriter"
1010

1111
"github.com/Masterminds/sprig/v3"
12+
"github.com/mattn/go-isatty"
1213
"github.com/pkg/errors"
1314
"github.com/replicatedhq/replicated/client"
1415
replicatedcache "github.com/replicatedhq/replicated/pkg/cache"
@@ -104,20 +105,31 @@ Use "{{.CommandPath}} [command] --help" for more information about a command.{{e
104105
func Execute(rootCmd *cobra.Command, stdin io.Reader, stdout io.Writer, stderr io.Writer) error {
105106
w := tabwriter.NewWriter(stdout, minWidth, tabWidth, padding, padChar, tabwriter.TabIndent)
106107

108+
stdoutIsTTY := false
109+
if f, ok := stdout.(*os.File); ok {
110+
stdoutIsTTY = isatty.IsTerminal(f.Fd())
111+
}
112+
107113
// get api client and app ID after flags are parsed
108114
runCmds := &runners{
109-
rootCmd: rootCmd,
110-
stdin: stdin,
111-
w: w,
115+
rootCmd: rootCmd,
116+
stdin: stdin,
117+
w: w,
118+
stdoutIsTTY: stdoutIsTTY,
112119
}
113120
if runCmds.rootCmd == nil {
114121
runCmds.rootCmd = GetRootCmd()
115122
}
116123
if stderr != nil {
117124
runCmds.rootCmd.SetErr(stderr)
125+
runCmds.rootCmd.SetOut(stderr)
118126
}
119127
if stdout != nil {
120-
runCmds.rootCmd.SetOut(stdout)
128+
defaultHelpFunc := runCmds.rootCmd.HelpFunc()
129+
runCmds.rootCmd.SetHelpFunc(func(cmd *cobra.Command, args []string) {
130+
cmd.SetOut(stdout)
131+
defaultHelpFunc(cmd, args)
132+
})
121133
}
122134

123135
// Setup PersistentPreRun to handle --debug flag

‎cli/cmd/runner.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ type runners struct {
2424
stdin io.Reader
2525
outputFormat string
2626
w *tabwriter.Writer
27+
stdoutIsTTY bool
2728

2829
rootCmd *cobra.Command
2930
args runnerArgs

0 commit comments

Comments
 (0)