diff --git a/pkg/app/expiry_warning_test.go b/pkg/app/expiry_warning_test.go index 4b2240e0d..3dc80f3a7 100644 --- a/pkg/app/expiry_warning_test.go +++ b/pkg/app/expiry_warning_test.go @@ -18,9 +18,10 @@ import ( func expiringTokenData(out *bytes.Buffer) *global.Data { soon := time.Now().Add(3 * 24 * time.Hour).Format(time.RFC3339) return &global.Data{ - Output: out, - ErrLog: fsterr.Log, - Manifest: &manifest.Data{}, + Output: out, + ErrOutput: out, + ErrLog: fsterr.Log, + Manifest: &manifest.Data{}, Config: config.File{ Auth: config.Auth{ Default: "mytoken", @@ -52,8 +53,9 @@ func TestCheckTokenExpirationWarning(t *testing.T) { commandName: "service list", data: func(out *bytes.Buffer) *global.Data { return &global.Data{ - Output: out, - ErrLog: fsterr.Log, + Output: out, + ErrOutput: out, + ErrLog: fsterr.Log, Config: config.File{ Auth: config.Auth{ Default: "mytoken", @@ -76,8 +78,9 @@ func TestCheckTokenExpirationWarning(t *testing.T) { commandName: "service list", data: func(out *bytes.Buffer) *global.Data { return &global.Data{ - Output: out, - ErrLog: fsterr.Log, + Output: out, + ErrOutput: out, + ErrLog: fsterr.Log, Config: config.File{ Auth: config.Auth{ Default: "mytoken", @@ -99,9 +102,10 @@ func TestCheckTokenExpirationWarning(t *testing.T) { commandName: "service list", data: func(out *bytes.Buffer) *global.Data { return &global.Data{ - Output: out, - ErrLog: fsterr.Log, - Env: config.Environment{APIToken: "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIn0.fake"}, + Output: out, + ErrOutput: out, + ErrLog: fsterr.Log, + Env: config.Environment{APIToken: "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIn0.fake"}, Config: config.File{ Auth: config.Auth{ Default: "mytoken", @@ -123,9 +127,10 @@ func TestCheckTokenExpirationWarning(t *testing.T) { commandName: "service list", data: func(out *bytes.Buffer) *global.Data { return &global.Data{ - Output: out, - ErrLog: fsterr.Log, - Flags: global.Flags{Token: "some-raw-token"}, + Output: out, + ErrOutput: out, + ErrLog: fsterr.Log, + Flags: global.Flags{Token: "some-raw-token"}, Config: config.File{ Auth: config.Auth{ Default: "mytoken", @@ -147,8 +152,9 @@ func TestCheckTokenExpirationWarning(t *testing.T) { commandName: "service list", data: func(out *bytes.Buffer) *global.Data { return &global.Data{ - Output: out, - ErrLog: fsterr.Log, + Output: out, + ErrOutput: out, + ErrLog: fsterr.Log, Config: config.File{ Auth: config.Auth{ Default: "deleted-token", @@ -164,8 +170,9 @@ func TestCheckTokenExpirationWarning(t *testing.T) { commandName: "service list", data: func(out *bytes.Buffer) *global.Data { return &global.Data{ - Output: out, - ErrLog: fsterr.Log, + Output: out, + ErrOutput: out, + ErrLog: fsterr.Log, Config: config.File{ Auth: config.Auth{ Default: "mytoken", @@ -187,7 +194,8 @@ func TestCheckTokenExpirationWarning(t *testing.T) { commandName: "service list", data: func(out *bytes.Buffer) *global.Data { return &global.Data{ - Output: out, + Output: out, + ErrOutput: out, Config: config.File{ Auth: config.Auth{ Default: "mytoken", @@ -209,8 +217,9 @@ func TestCheckTokenExpirationWarning(t *testing.T) { commandName: "service list", data: func(out *bytes.Buffer) *global.Data { return &global.Data{ - Output: out, - ErrLog: fsterr.Log, + Output: out, + ErrOutput: out, + ErrLog: fsterr.Log, Config: config.File{ Auth: config.Auth{ Default: "sso-tok", @@ -338,11 +347,6 @@ func TestCheckTokenExpirationWarningSuppression(t *testing.T) { commandName: "service list", flags: global.Flags{Quiet: true}, }, - { - name: "suppressed with --json flag (sets Quiet)", - commandName: "service list", - flags: global.Flags{Quiet: true}, // --json sets Quiet=true in Exec - }, } originalEnv := os.Getenv("FASTLY_DISABLE_AUTH_COMMAND") @@ -365,6 +369,21 @@ func TestCheckTokenExpirationWarningSuppression(t *testing.T) { } } +// TestCheckTokenExpirationWarningShownForJSON verifies that --json mode still +// emits the warning (to stderr) rather than suppressing it entirely. +func TestCheckTokenExpirationWarningShownForJSON(t *testing.T) { + var buf bytes.Buffer + data := expiringTokenData(&buf) + data.Flags = global.Flags{JSON: true} + + checkTokenExpirationWarning(data, "service list") + + output := buf.String() + if !strings.Contains(output, "expires in") { + t.Errorf("expected expiry warning in --json mode (written to stderr), got: %s", output) + } +} + // TestCheckTokenExpirationWarningNotSuppressedForNonAuth ensures that commands // starting with "auth" as a prefix of another word (e.g. "authtoken") are not // incorrectly suppressed. diff --git a/pkg/app/run.go b/pkg/app/run.go index 2375c5500..58778dbdc 100644 --- a/pkg/app/run.go +++ b/pkg/app/run.go @@ -172,6 +172,7 @@ var Init = func(args []string, stdin io.Reader) (*global.Data, error) { ConfigPath: config.FilePath, Env: e, ErrLog: fsterr.Log, + ErrOutput: os.Stderr, ExecuteWasmTools: compute.ExecuteWasmTools, HTTPClient: httpClient, Manifest: &md, @@ -200,9 +201,11 @@ func Exec(data *global.Data) error { return err } - // Check for --json flag early and set quiet mode if found. + // Check for --json flag early. JSON mode suppresses stdout-bound noise + // (metadata notices, update checks) but still allows stderr warnings + // (token expiry, profile mismatch) so they don't corrupt JSON output. if slices.Contains(data.Args, "--json") { - data.Flags.Quiet = true + data.Flags.JSON = true } // We short-circuit the execution for specific cases: @@ -219,7 +222,7 @@ func Exec(data *global.Data) error { } metadataDisable, _ := strconv.ParseBool(data.Env.WasmMetadataDisable) - if !slices.Contains(data.Args, "--metadata-disable") && !metadataDisable && !data.Config.CLI.MetadataNoticeDisplayed && commandCollectsData(commandName) && !data.Flags.Quiet { + if !slices.Contains(data.Args, "--metadata-disable") && !metadataDisable && !data.Config.CLI.MetadataNoticeDisplayed && commandCollectsData(commandName) && !data.Flags.Quiet && !data.Flags.JSON { text.Important(data.Output, "The Fastly CLI is configured to collect data related to Wasm builds (e.g. compilation times, resource usage, and other non-identifying data). To learn more about what data is being collected, why, and how to disable it: https://www.fastly.com/documentation/reference/cli") text.Break(data.Output) data.Config.CLI.MetadataNoticeDisplayed = true @@ -230,7 +233,7 @@ func Exec(data *global.Data) error { time.Sleep(5 * time.Second) // this message is only displayed once so give the user a chance to see it before it possibly scrolls off screen } - if data.Flags.Quiet { + if data.Flags.Quiet || data.Flags.JSON { data.Manifest.File.SetQuiet(true) } @@ -287,9 +290,9 @@ func Exec(data *global.Data) error { if !data.Flags.Quiet && data.Flags.Token == "" && data.Env.APIToken == "" && data.Manifest != nil && data.Manifest.File.Profile != "" { if data.Config.GetAuthToken(data.Manifest.File.Profile) == nil { if defaultName, _ := data.Config.GetDefaultAuthToken(); defaultName != "" { - text.Warning(data.Output, "fastly.toml profile %q not found in auth config, using default token %q.\n", data.Manifest.File.Profile, defaultName) + text.Warning(data.ErrOutput, "fastly.toml profile %q not found in auth config, using default token %q.\n", data.Manifest.File.Profile, defaultName) } else { - text.Warning(data.Output, "fastly.toml profile %q not found in auth config and no default token is configured.\n", data.Manifest.File.Profile) + text.Warning(data.ErrOutput, "fastly.toml profile %q not found in auth config and no default token is configured.\n", data.Manifest.File.Profile) } } } @@ -316,7 +319,7 @@ func Exec(data *global.Data) error { displayToken(tokenSource, data) } if !data.Flags.Quiet { - checkConfigPermissions(tokenSource, data.Output) + checkConfigPermissions(tokenSource, data.ErrOutput) } data.APIClient, data.RTSClient, err = configureClients(token, apiEndpoint, data.APIClientFactory, data.Flags.Debug) @@ -328,7 +331,7 @@ func Exec(data *global.Data) error { checkTokenExpirationWarning(data, commandName) - f := checkForUpdates(data.Versioners.CLI, commandName, data.Flags.Quiet) + f := checkForUpdates(data.Versioners.CLI, commandName, data.Flags.Quiet || data.Flags.JSON) defer f(data.Output) return command.Exec(data.Input, data.Output) @@ -506,9 +509,10 @@ func checkAndRefreshAuthSSOToken(name string, at *config.AuthToken, data *global // This matches the set hidden by FASTLY_DISABLE_AUTH_COMMAND (pkg/env/env.go). var authRelatedCommands = []string{"auth", "auth-token", "sso", "profile", "whoami"} -// checkTokenExpirationWarning prints a warning if the active stored token is -// about to expire. Only fires for SourceAuth tokens; env/flag tokens are opaque. -// Suppressed for auth-related commands and when --quiet or --json is active. +// checkTokenExpirationWarning prints a warning to stderr if the active stored +// token is about to expire. Only fires for SourceAuth tokens; env/flag tokens +// are opaque. Suppressed for auth-related commands and when --quiet is active. +// In --json mode the warning still fires (written to stderr via data.ErrOutput). func checkTokenExpirationWarning(data *global.Data, commandName string) { if data.Flags.Quiet { return @@ -541,7 +545,7 @@ func checkTokenExpirationWarning(data *global.Data, commandName string) { summary := authcmd.ExpirationSummary(status, expires, time.Now()) remediation := authcmd.ExpirationRemediation(at.Type) - text.Warning(data.Output, "Your active token %s. %s\n", summary, remediation) + text.Warning(data.ErrOutput, "Your active token %s. %s\n", summary, remediation) } // isAuthRelatedCommand reports whether commandName belongs to an auth-related diff --git a/pkg/app/run_test.go b/pkg/app/run_test.go index e3e53cea9..cb9aadfd6 100644 --- a/pkg/app/run_test.go +++ b/pkg/app/run_test.go @@ -132,9 +132,7 @@ whoami } // TestExecQuietSuppressesExpiryWarning exercises the full Exec path to verify -// that --quiet suppresses the expiration warning end-to-end. (--json also sets -// Quiet=true at run.go:204, but config doesn't accept --json; the unit test -// TestCheckTokenExpirationWarningSuppression covers the Quiet flag directly.) +// that --quiet suppresses the expiration warning end-to-end. func TestExecQuietSuppressesExpiryWarning(t *testing.T) { var stdout bytes.Buffer @@ -178,6 +176,38 @@ func TestExecConfigShowsExpiryWarning(t *testing.T) { } } +// TestExecJSONLeavesStdoutCleanAndWritesWarningToStderr verifies that in +// --json mode, the expiry warning is written to stderr (not stdout) so it +// does not corrupt JSON output. Because the config command does not register +// --json as a flag, we simulate the effect by pre-setting Flags.JSON (which +// is what Exec does when it sees --json in the args). +func TestExecJSONLeavesStdoutCleanAndWritesWarningToStderr(t *testing.T) { + var ( + stdout bytes.Buffer + stderr bytes.Buffer + ) + + args := testutil.SplitArgs("config -l") + app.Init = func(_ []string, _ io.Reader) (*global.Data, error) { + data := testutil.MockGlobalData(args, &stdout) + data.ErrOutput = &stderr + data.Flags.JSON = true + data.Config.Auth.Tokens["user"].APITokenExpiresAt = time.Now().Add(3 * 24 * time.Hour).Format(time.RFC3339) + return data, nil + } + err := app.Run(args, nil) + if err != nil { + t.Fatalf("app.Run returned unexpected error: %v", err) + } + + if strings.Contains(stdout.String(), "expires in") { + t.Errorf("expected stdout free of expiry warning, got: %s", stdout.String()) + } + if !strings.Contains(stderr.String(), "expires in") { + t.Errorf("expected expiry warning on stderr, got: %s", stderr.String()) + } +} + // stripTrailingSpace removes any trailing spaces from the multiline str. func stripTrailingSpace(str string) string { buf := bytes.NewBuffer(nil) diff --git a/pkg/global/global.go b/pkg/global/global.go index 99f2d8cec..f8fd67759 100644 --- a/pkg/global/global.go +++ b/pkg/global/global.go @@ -62,7 +62,8 @@ type Data struct { // Env is all the data that is provided by the environment. Env config.Environment // ErrLog provides an interface for recording errors to disk. - ErrLog fsterr.LogInterface + ErrLog fsterr.LogInterface + ErrOutput io.Writer // ExecuteWasmTools is a function that executes the wasm-tools binary. ExecuteWasmTools func(bin string, args []string, global *Data) error // Flags are all the global CLI flags. @@ -204,6 +205,9 @@ type Flags struct { AutoYes bool // Debug enables the CLI's debug mode. Debug bool + // JSON indicates --json output was requested. Detected automatically by + // Exec. Unlike Quiet, JSON mode does not suppress stderr warnings. + JSON bool // NonInteractive auto-resolves all prompts. NonInteractive bool // Profile indicates the profile to use (consequently the 'token' used). diff --git a/pkg/testutil/args.go b/pkg/testutil/args.go index 21cd56723..a5e409135 100644 --- a/pkg/testutil/args.go +++ b/pkg/testutil/args.go @@ -124,6 +124,7 @@ func MockGlobalData(args []string, stdout io.Writer) *global.Data { ConfigPath: configPath, Env: config.Environment{}, ErrLog: errors.Log, + ErrOutput: stdout, ExecuteWasmTools: func(bin string, args []string, d *global.Data) error { fmt.Printf("bin: %s\n", bin) fmt.Printf("args: %#v\n", args)