From b47ce5289a3ca9d85813a2085346d0a23d06bcce Mon Sep 17 00:00:00 2001 From: omattsson Date: Tue, 27 Oct 2026 21:15:06 +0100 Subject: [PATCH 1/4] feat(cmd): add agent-guide, an embedded guide for AI agents and scripts `stackctl agent-guide` prints a Markdown guide for agents and scripts: machine-readable output and where it is not JSON, -q IDs, exit codes, authentication without a prompt, confirmations (Aborted. exits 0, < /dev/null), --dry-run limits, waiting for long-running operations and checking the status after --follow, and common tasks. The guide is embedded in the binary, so it matches the installed version, and the command works without a config. `stackctl --help` points to it. TestAgentGuide_CommandsExist checks every stackctl invocation in the guide against the command tree and the flags of each command. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 4 +- README.md | 12 ++ cli/cmd/agent_guide.go | 46 ++++++++ cli/cmd/agent_guide.md | 117 ++++++++++++++++++++ cli/cmd/agent_guide_test.go | 214 ++++++++++++++++++++++++++++++++++++ cli/cmd/root.go | 6 +- 6 files changed, 396 insertions(+), 3 deletions(-) create mode 100644 cli/cmd/agent_guide.go create mode 100644 cli/cmd/agent_guide.md create mode 100644 cli/cmd/agent_guide_test.go diff --git a/CLAUDE.md b/CLAUDE.md index 37d9c34..5684c08 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -25,6 +25,7 @@ cli/ # Go module root (github.com/omattsson/stackctl/cl root.go # Root cobra command, global flags, config loading, flag > env > config (resolveAPIURL, resolveAPIKey) config.go # config set/get/list/use-context/current-context/delete-context version.go # Version info (build-time ldflags) + agent_guide.go, .md # agent-guide: embedded Markdown guide for agents and scripts ({{VERSION}} = build version); agent_guide_test.go checks every stackctl command and flag in it login.go # login, logout (--all: logout-all first), whoami, auth register token.go # Token helpers (save/load/delete session, fresh token for plugins) stack.go # stack list/get/create/update/deploy/stop/clean/delete/status/logs/clone/extend/values/compare/history/rollback/history-values/watch @@ -129,7 +130,7 @@ cli/ # Go module root (github.com/omattsson/stackctl/cl **Plugins** (`cmd/plugins.go`): for a plugin, `Execute` sets `TraverseChildren`, so the global flags before the plugin name are parsed by stackctl and not passed to the plugin; `pluginEnv` exports them (`STACKCTL_OUTPUT`, `STACKCTL_QUIET`, `STACKCTL_NO_COLOR` + `NO_COLOR`, `STACKCTL_DEBUG`, `STACKCTL_INSECURE`, `STACKCTL_CONFIG_DIR`, plus the context values). Arguments after the plugin name pass unchanged. Contract in `EXTENDING.md`. -**Config-free commands**: `version` and `completion` skip config file loading and work even if the config is missing or corrupted. +**Config-free commands**: `version`, `agent-guide` and `completion` skip config file loading and work even if the config is missing or corrupted. **Insecure mode**: When `--insecure` is active, a warning is printed to stderr. @@ -226,3 +227,4 @@ All API calls go to `/api/v1/*`. Key route groups: 5. Use `pkg/output` for formatting results 6. Add to parent command in `init()` 7. Write tests: flag parsing, success output, error handling +8. If agents or scripts need to know about it (output, prompts, exit codes, waiting), update `cmd/agent_guide.md` diff --git a/README.md b/README.md index 7d82199..ff25020 100644 --- a/README.md +++ b/README.md @@ -87,6 +87,18 @@ stackctl template quick-deploy 8c9d0e1f-2a3b-4c5d-9e6f-7a8b9c0d1e2f --name my-st stackctl stack list --mine ``` +## Use with AI agents and scripts + +`stackctl agent-guide` prints a Markdown guide for AI agents (Claude Code, Copilot, Codex, ...) and scripts: machine-readable output, exit codes, authentication without a prompt, `--yes`, waiting for deploys, and common tasks. The guide is part of the binary, so it matches the installed version, and it works without a config. `stackctl --help` points agents to it. + +To give an agent the guide up front, add a line to the instructions of your project (for example `CLAUDE.md` or `AGENTS.md`): + +```markdown +Before you use stackctl, run `stackctl agent-guide` and follow it. +``` + +Or save it next to them: `stackctl agent-guide > STACKCTL.md`. + ## Extending — add your own subcommands Drop an executable named `stackctl-` anywhere on your `$PATH` and it becomes `stackctl ` automatically. No plugin SDK, no recompile, no changes to stackctl itself. Same pattern as `git`, `kubectl`, and `gh`. diff --git a/cli/cmd/agent_guide.go b/cli/cmd/agent_guide.go new file mode 100644 index 0000000..fc16129 --- /dev/null +++ b/cli/cmd/agent_guide.go @@ -0,0 +1,46 @@ +package cmd + +import ( + _ "embed" + "fmt" + "strings" + + "github.com/spf13/cobra" +) + +// agentGuide is the guide for agents and scripts. {{VERSION}} is replaced +// with the build version, so the guide names the binary it came with. +// +//go:embed agent_guide.md +var agentGuide string + +var agentGuideCmd = &cobra.Command{ + Use: "agent-guide", + Short: "Print a guide for AI agents and scripts that use stackctl", + Long: `Print a Markdown guide for AI agents and scripts: machine-readable +output, exit codes, authentication without a prompt, confirmations, +long-running operations and common tasks. + +The guide is part of the binary, so it matches this version of stackctl. +It always prints Markdown (--output has no effect). This command works +without any configuration. + +Examples: + stackctl agent-guide + stackctl agent-guide > STACKCTL.md`, + Args: cobra.NoArgs, + SilenceUsage: true, + RunE: func(cmd *cobra.Command, args []string) error { + _, err := fmt.Fprint(cmd.OutOrStdout(), renderAgentGuide()) + return err + }, +} + +// renderAgentGuide returns the guide with the build version filled in. +func renderAgentGuide() string { + return strings.ReplaceAll(agentGuide, "{{VERSION}}", buildVersion) +} + +func init() { + rootCmd.AddCommand(agentGuideCmd) +} diff --git a/cli/cmd/agent_guide.md b/cli/cmd/agent_guide.md new file mode 100644 index 0000000..8aa62a0 --- /dev/null +++ b/cli/cmd/agent_guide.md @@ -0,0 +1,117 @@ +# stackctl guide for agents and scripts + +This guide matches stackctl {{VERSION}}. stackctl is the CLI of k8s-stack-manager: it creates, deploys and manages Helm-based stacks on Kubernetes clusters through the API of the server. It never talks to Kubernetes directly. + +Run `stackctl --help` for the flags of a command. This guide covers what the help does not: how to use stackctl without a terminal and without a person. + +## Rules + +1. **Read data with `-o json`.** List, get, create and other commands that return a resource then print only JSON on stdout; errors, warnings and notes go to stderr. The default table output is for people and can change. +2. **Actions print text, not JSON.** `stack deploy`, `stop`, `clean`, `delete` and every `--dry-run` print a plain message on stdout even with `-o json`; `--follow` streams the log to stdout. Read the result afterwards with `stackctl stack get -o json`. +3. **`-q` prints IDs, one per line**, for pipes and `$(...)`. On `stack create`, `clone` and `template quick-deploy` it is the stack ID; on `stack deploy`, `stop` and `clean` it is the ID of the deployment log. +4. **Exit code 0 means no error, 1 means an error.** The error text is on stderr. Do not parse table output to detect errors. Exit 0 does not always mean that the change happened: see rule 5 and "Long-running operations". +5. **Destructive commands ask for confirmation.** A command with a `--yes` flag (see its `--help`) reads the answer from stdin. Any answer other than `y` prints `Aborted.` and exits **0** without a change; a closed stdin fails with `use --yes to confirm`; an open stdin without input waits forever. Pass `--yes` only when the user asked for that change, and run commands with `< /dev/null` so a missing `--yes` fails instead of hanging. +6. **`--dry-run` does not check anything.** It prints the action and the name you gave and contacts no server: a misspelled name still "succeeds". To check the target, run `stackctl stack get -o json` first. +7. **Do not retry an operation that is still running.** See "Long-running operations". +8. **Never print or log secrets.** Do not echo `STACKCTL_API_KEY`, `STACKCTL_PASSWORD`, `STACKCTL_TOKEN` or the token files in the config directory. Prefer environment variables over `--api-key` and `--password`, which other processes can see. + +## Authentication + +Check the state before you start: + +```sh +stackctl config current-context # the active context (environment) +stackctl whoami # the authenticated user; exit 1 when not logged in +stackctl server info # server reachable? TTL limits; needs no credentials +``` + +Ways to authenticate without a prompt, in order of preference: + +- **API key:** set `STACKCTL_API_KEY` and `STACKCTL_API_URL`. No login and no token file. An API key wins over a stored login session. Servers before k8s-stack-manager v0.8.0 refuse an API key for `--follow` and `stack watch` (`This server does not accept API keys for live updates`): then poll `stack get` instead (see "Long-running operations"). +- **Username and password:** `STACKCTL_USERNAME=... STACKCTL_PASSWORD=... stackctl login`. stackctl stores a session and renews it by itself. +- `stackctl login --sso` opens a browser: do not use it unless a person is at the computer. + +Values resolve in this order: flag (`--api-url`, `--api-key`), environment variable, config file. `STACKCTL_CONFIG_DIR` selects another config directory, for example an empty one for an isolated run. `stackctl config use-context ` switches the environment for every later command: ask the user before you change it. + +`Not authenticated. Run 'stackctl login' first.` means the credentials are missing or the session ended. Ask the user for credentials; do not guess them. + +## Concepts + +- **Template:** a published blueprint of charts. `template quick-deploy` creates and deploys a stack from it in one step. +- **Definition:** the set of Helm charts and default values that a stack uses. +- **Stack (instance):** one deployment of a definition into its own namespace on a cluster, with an optional branch and an expiry (TTL). +- **Status:** `draft` (created, never deployed), `deploying`, `running`, `stopping`, `stopped`, `cleaning`, `error`, `failed`, `partial` (some charts failed). +- **stop** stops the stack and keeps it (a deploy starts it again); **clean** undeploys it and removes its namespace; **delete** removes the stack. +- **Overrides:** per-stack Helm values (`override`), per-chart branches (`override branch`) and quotas (`override quota`). + +## Common tasks + +Commands that take `` accept the stack name or the ID. + +```sh +# Find stacks +stackctl stack list --mine -o json # paged: {"data": [...], "total", "page", "pageSize"} +stackctl stack list --mine -q # IDs of all pages +stackctl stack get my-app -o json # one stack: id, name, status, expires_at, error_message, ... +stackctl dashboard -o json # failing and expiring stacks, recent deployments + +# Create and deploy +ID=$(stackctl stack create --definition my-def --name my-app --branch feature/x -q) +stackctl stack deploy "$ID" --follow +stackctl template quick-deploy --name my-app --ttl 480 -o json + +# Inspect +stackctl stack status my-app # pods +stackctl stack pods my-app # live pods and warning events +stackctl stack logs my-app # latest deployment log +stackctl stack values my-app # merged Helm values +stackctl stack preview my-app # values that the next deploy uses +stackctl stack history my-app + +# Change values, then deploy again +stackctl override set my-app my-chart --set image.tag=1.2.3 +stackctl override branch set my-app my-chart feature/x +stackctl stack deploy my-app --follow + +# Expiry +stackctl stack extend my-app --minutes 120 +stackctl stack update my-app --stop-at "2026-10-17 18:00" + +# Stop, clean up, delete (only when the user asked) +stackctl stack stop my-app --follow +stackctl stack clean my-app --yes --follow +stackctl stack delete my-app --yes +stackctl stack list --status stopped --mine -q | xargs -r -n 50 stackctl bulk delete --yes +``` + +Paged lists (`stack list`, `definition list`, `template list`) return one page as `{"data": [...], "total", "page", "pageSize"}` in JSON. When there are more items, stderr says `Showing X of TOTAL`; use `--page` and `--page-size` (at most 100), or `-q`, which prints the IDs of all pages. + +## Long-running operations + +`stack deploy`, `stop`, `clean` and `rollback` return when the server has accepted the operation; the work continues on the server. To wait for the result: + +- add `--follow` (`-f`): stackctl streams the log and exits when the stack reaches a final status; +- or run `stackctl stack watch --id [,...]`, which exits when every listed stack reaches a final status; +- or poll `stackctl stack get -o json` every few seconds until `status` is final. + +Final statuses: `running`, `stopped` and `draft` exit 0; `error`, `failed` and `partial` exit 1, with the server error on stderr. + +**Check the status after exit 0.** `--follow` also exits 0 when the server closes the connection normally (for example on a restart) before the operation ends. Read `status` from `stackctl stack get -o json` and confirm it is the one you expected (`running` after a deploy, `stopped` after a stop). + +When `--follow` or `watch --id` stops early for another reason (lost connection, session ended), it exits 1 and says that the operation continues on the server. **Do not start the operation again.** Run `stackctl stack watch --id `, or poll `stackctl stack get -o json`. + +## Errors + +- `Permission denied`: the user's role does not allow this. Tell the user; do not try other commands to get around it. +- `Resource not found` / `not found`: check the name or ID with a list command. +- `Conflict: ...`: the state does not allow the change (for example a name in use; the message lists free names). +- `Rate limited`: wait and try once more. +- `Server error`: report it; the server logs have the cause. +- `upgrade k8s-stack-manager to vX or later`: the server is too old for this feature. Tell the user. +- `Warning: ...` on stderr: a notice from the server (for example a deprecation). The command still worked. + +## More + +- `stackctl --help` lists all command groups, including plugins (`stackctl-` programs on `PATH`). +- `--debug` logs each HTTP request and the response status to stderr; credentials are masked. +- `stackctl version -o json` prints the version of stackctl. diff --git a/cli/cmd/agent_guide_test.go b/cli/cmd/agent_guide_test.go new file mode 100644 index 0000000..ae7c6c0 --- /dev/null +++ b/cli/cmd/agent_guide_test.go @@ -0,0 +1,214 @@ +package cmd + +import ( + "bytes" + "os" + "path/filepath" + "regexp" + "strings" + "testing" + + "github.com/spf13/cobra" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// The command tests use rootCmd and the build globals, so they cannot run +// in parallel. + +func TestAgentGuideCmd_PrintsGuideWithVersion(t *testing.T) { + restoreVersionInfo(t) + buildVersion = "1.2.3" + + var buf bytes.Buffer + agentGuideCmd.SetOut(&buf) + t.Cleanup(func() { agentGuideCmd.SetOut(nil) }) + + require.NoError(t, agentGuideCmd.RunE(agentGuideCmd, nil)) + + out := buf.String() + assert.True(t, strings.HasPrefix(out, "# stackctl guide for agents and scripts\n")) + assert.Contains(t, out, "This guide matches stackctl 1.2.3.") + assert.NotContains(t, out, "{{VERSION}}") +} + +func TestAgentGuideCmd_IgnoresOutputFormat(t *testing.T) { + // The guide is Markdown in every output mode. + for _, args := range [][]string{ + {"agent-guide"}, + {"agent-guide", "-o", "json"}, + {"agent-guide", "-o", "yaml"}, + {"agent-guide", "-q"}, + } { + t.Run(strings.Join(args, " "), func(t *testing.T) { + t.Setenv("STACKCTL_CONFIG_DIR", t.TempDir()) + t.Cleanup(func() { flagOutput, flagQuiet = "table", false }) + + var stdout bytes.Buffer + rootCmd.SetOut(&stdout) + rootCmd.SetArgs(args) + t.Cleanup(func() { rootCmd.SetOut(nil); rootCmd.SetArgs(nil) }) + + require.NoError(t, rootCmd.Execute()) + assert.Equal(t, renderAgentGuide(), stdout.String()) + }) + } +} + +func TestAgentGuideCmd_RejectsArgs(t *testing.T) { + assert.Error(t, agentGuideCmd.Args(agentGuideCmd, []string{"extra"})) +} + +func TestAgentGuideWithInvalidConfig(t *testing.T) { + // agent-guide must work with a corrupted config, like version. + tmpDir := t.TempDir() + t.Setenv("STACKCTL_CONFIG_DIR", tmpDir) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "config.yaml"), []byte(":\n- invalid\n"), 0600)) + + var stdout bytes.Buffer + rootCmd.SetOut(&stdout) + rootCmd.SetArgs([]string{"agent-guide"}) + t.Cleanup(func() { rootCmd.SetOut(nil); rootCmd.SetArgs(nil) }) + + require.NoError(t, rootCmd.Execute()) + assert.Contains(t, stdout.String(), "# stackctl guide for agents and scripts") +} + +func TestRootHelp_PointsToAgentGuide(t *testing.T) { + assert.Contains(t, rootCmd.Long, "stackctl agent-guide") +} + +// TestAgentGuide_CommandsExist keeps the guide in step with the CLI: every +// stackctl invocation in a code span or code block must name existing +// commands, and each of its flags must exist on that command. It reads +// rootCmd, so it does not run in parallel. +func TestAgentGuide_CommandsExist(t *testing.T) { + invocations := guideInvocations(agentGuide) + require.NotEmpty(t, invocations) + + for _, inv := range invocations { + t.Run(inv, func(t *testing.T) { + cmd, rest, err := walkCommand(rootCmd, strings.Fields(inv)[1:]) + require.NoError(t, err) + for _, tok := range rest { + name, ok := flagName(tok) + if !ok { + continue + } + assert.NotNil(t, lookupFlag(cmd, name), "%s has no flag %s", cmd.CommandPath(), tok) + } + }) + } +} + +func TestGuideInvocations(t *testing.T) { + t.Parallel() + + doc := "Run `stackctl stack get ` or `stackctl --help`.\n" + + "stackctl in prose is skipped, and so is `stackctl-`.\n" + + "```sh\n" + + "ID=$(stackctl stack create --name a -q)\n" + + "stackctl stack list -q | xargs -n 50 stackctl bulk delete --yes # note\n" + + "```\n" + // Code blocks come first, then the code spans of the prose. + assert.Equal(t, []string{ + "stackctl stack create --name a -q", + "stackctl stack list -q", + "stackctl bulk delete --yes", + "stackctl stack get ", + }, guideInvocations(doc)) +} + +var ( + codeBlockRe = regexp.MustCompile("(?s)```[a-z]*\n(.*?)```") + codeSpanRe = regexp.MustCompile("`([^`\n]+)`") + invocationRe = regexp.MustCompile(`(?:^|[\s(|;&])(stackctl(?: [^|;&#)\n]*)?)(?:$|[\s|;&#)])`) +) + +// guideInvocations returns the stackctl invocations in the code of a +// Markdown document, each cut at a shell operator, a comment or the end of +// a command substitution. Invocations with a placeholder command +// (stackctl --help) are left out. +func guideInvocations(doc string) []string { + var code []string + for _, m := range codeBlockRe.FindAllStringSubmatch(doc, -1) { + code = append(code, strings.Split(m[1], "\n")...) + } + prose := codeBlockRe.ReplaceAllString(doc, "") + for _, m := range codeSpanRe.FindAllStringSubmatch(prose, -1) { + code = append(code, m[1]) + } + + var out []string + for _, line := range code { + for _, m := range invocationRe.FindAllStringSubmatch(line, -1) { + inv := strings.TrimSpace(m[1]) + fields := strings.Fields(inv) + if len(fields) > 1 && strings.HasPrefix(fields[1], "<") { + continue + } + out = append(out, inv) + } + } + return out +} + +// walkCommand follows args through the subcommands of root. It stops at the +// first flag or at a command without subcommands (the rest are its +// arguments) and fails on an unknown subcommand. +func walkCommand(root *cobra.Command, args []string) (*cobra.Command, []string, error) { + cmd := root + for i, arg := range args { + if strings.HasPrefix(arg, "-") || !cmd.HasSubCommands() { + return cmd, args[i:], nil + } + next, _, err := cmd.Find([]string{arg}) + if err != nil || next == cmd { + return nil, nil, &unknownCommandError{path: cmd.CommandPath(), name: arg} + } + cmd = next + } + return cmd, nil, nil +} + +type unknownCommandError struct{ path, name string } + +func (e *unknownCommandError) Error() string { + return e.path + " has no subcommand " + e.name +} + +// flagName returns the flag name of a token: "image" for "--image=x", "q" +// for "-q". It returns false for a token that is not a flag. +func flagName(tok string) (string, bool) { + tok = strings.Trim(tok, `"'`) + if !strings.HasPrefix(tok, "-") || tok == "-" { + return "", false + } + name := strings.TrimLeft(tok, "-") + name, _, _ = strings.Cut(name, "=") + return name, name != "" +} + +// lookupFlag finds a local or inherited flag by name or shorthand. Cobra +// adds the help flag only when a command runs, so help and h always exist. +func lookupFlag(cmd *cobra.Command, name string) any { + if name == "help" || name == "h" { + return name + } + if len(name) == 1 { + if f := cmd.Flags().ShorthandLookup(name); f != nil { + return f + } + if f := cmd.InheritedFlags().ShorthandLookup(name); f != nil { + return f + } + return nil + } + if f := cmd.Flags().Lookup(name); f != nil { + return f + } + if f := cmd.InheritedFlags().Lookup(name); f != nil { + return f + } + return nil +} diff --git a/cli/cmd/root.go b/cli/cmd/root.go index 821b166..352f8a7 100644 --- a/cli/cmd/root.go +++ b/cli/cmd/root.go @@ -42,7 +42,9 @@ stacks across Kubernetes clusters. Get started: stackctl config use-context local stackctl config set api-url http://localhost:8081 - stackctl version`, + stackctl version + +AI agents and scripts: run 'stackctl agent-guide' first.`, SilenceUsage: true, SilenceErrors: true, PersistentPreRunE: func(cmd *cobra.Command, args []string) error { @@ -54,7 +56,7 @@ Get started: // Skip config loading for commands that should work without a config file name := cmd.Name() - if name == "version" || name == "completion" || (cmd.Parent() != nil && cmd.Parent().Name() == "completion") { + if name == "version" || name == "agent-guide" || name == "completion" || (cmd.Parent() != nil && cmd.Parent().Name() == "completion") { cfg = &config.Config{Contexts: map[string]*config.Context{}} return nil } From 9bc89ada6b987e4f241448d440c9a6294a9494c5 Mon Sep 17 00:00:00 2001 From: omattsson Date: Tue, 27 Oct 2026 21:18:29 +0100 Subject: [PATCH 2/4] fix(stack): --follow fails on a normal server close before a final status A normal WebSocket close (code 1000, for example on a server restart) ended `--follow` with status "unknown" and exit code 0, although the operation was still running. After such a close, --follow now reads the instance: a final status counts (error and partial still exit 1), else it exits 1 and says that the operation continues on the server. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 2 +- README.md | 2 +- cli/cmd/stack.go | 20 +++++++++++++++- cli/cmd/stack_follow_test.go | 44 ++++++++++++++++++++++++++++++++++++ 4 files changed, 65 insertions(+), 3 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 5684c08..12354f2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -190,7 +190,7 @@ All API calls go to `/api/v1/*`. Key route groups: - `PUT /:id/quota-overrides` replaces the whole override. `override quota set` does GET (only 404 "Instance quota override not found" = no override), applies the given flags (`""` clears a string field; pod_limit only via `--replace` or delete; `--pod-limit 0` = no limit) and PUTs; `--replace` skips the GET. An empty result is refused (points to `override quota delete`). GET and PUT are not atomic: a concurrent change is overwritten. Owners without admin/devops get 403 above the cluster quota (v0.7.0+); a value equal to the stored value passes. - `/api/v1/audit-logs` — list/export; `--action` and `--entity-type` are exact matches (`deploy`, `stack_instance`, …; v0.7.0+, see `auditFilterValuesHelp`). Older servers: HTTP method + last route segment (`create` / `deploy`) - `/api/v1/users` — list/delete/disable/enable/password (admin). `PUT /:id/role` `{"role"}` (k8s-stack-manager v0.8.0+, `user set-role `, user by ID or username via `resolveUserID`): response `id`, `old_role`, `new_role`, `changed`; 403 own role / caller not admin, 409 SSO user / last admin, 400 invalid role; a plain 404 (no route) maps to the v0.8.0 upgrade hint (`setRoleError`) -- `/ws` — the server closes a socket with code 1008 and reason `session revoked` or `token expired`. `Client.Watch` returns a `WatchStream`; `Err()` gives a `*WSClosedError` ("Connection closed by the server: . Log in again."). `stack watch` exits non-zero on it; for `token expired` it calls `Client.RenewSession` (forced renewal, false without refresh token) and reconnects, at most once per 30 s (`stackWatchMinReconnect`), then with `--id` GETs each pending instance (`watchCatchUp`) so a terminal status in the gap counts. Ctrl-C during renewal or dial exits 0. Another connection loss: `--id` with pending instances exits non-zero ("connection lost before all instances reached a terminal status"), else a stderr note and exit 0. `StreamDeploymentLogs` returns the same `WSClosedError`; `--follow` (`followLogsCtx`, op deploy/stop/clean/rollback, "" for `stack logs -f`) uses the same `renewAfterClose`; it reads the instance after the first connect (the server sets deploying/stopping/cleaning before the 202; not for `logs -f`) and after each reconnect (`StreamOptions.AfterSubscribe`). When it cannot follow to the end it says that the operation continues on the server and points to `stack watch --id` / `stack status` (never "run again"); errors name the operation ("rollback failed (status error): ..."). Terminal statuses live in `client.IsTerminalStatus` (running, stopped, draft, error, partial, failed) and `client.IsFailedStatus` (error, partial, failed → exit 1) +- `/ws` — the server closes a socket with code 1008 and reason `session revoked` or `token expired`. `Client.Watch` returns a `WatchStream`; `Err()` gives a `*WSClosedError` ("Connection closed by the server: . Log in again."). `stack watch` exits non-zero on it; for `token expired` it calls `Client.RenewSession` (forced renewal, false without refresh token) and reconnects, at most once per 30 s (`stackWatchMinReconnect`), then with `--id` GETs each pending instance (`watchCatchUp`) so a terminal status in the gap counts. Ctrl-C during renewal or dial exits 0. Another connection loss: `--id` with pending instances exits non-zero ("connection lost before all instances reached a terminal status"), else a stderr note and exit 0. `StreamDeploymentLogs` returns the same `WSClosedError`; `--follow` (`followLogsCtx`, op deploy/stop/clean/rollback, "" for `stack logs -f`) uses the same `renewAfterClose`; it reads the instance after the first connect (the server sets deploying/stopping/cleaning before the 202; not for `logs -f`) and after each reconnect (`StreamOptions.AfterSubscribe`). A normal close (code 1000, stream status `unknown`) reads the instance (`followEndError`): a final status counts, else it fails like a lost connection. When it cannot follow to the end it says that the operation continues on the server and points to `stack watch --id` / `stack status` (never "run again"); errors name the operation ("rollback failed (status error): ..."). Terminal statuses live in `client.IsTerminalStatus` (running, stopped, draft, error, partial, failed) and `client.IsFailedStatus` (error, partial, failed → exit 1) - `/api/v1/admin/orphaned-namespaces` — `orphaned list [--details]`; `DELETE /:ns?confirm=` is needed for a namespace without the label managed-by=k8s-stack-manager (409 without it; `orphaned delete --confirm`). `test/contract/path_contract_test.go` checks every `/api/v1` literal of `pkg/client` and `cmd` against the vendored swagger - `POST /api/v1/auth/logout-all` — `logout --all`: uses the session token of the context when a token file exists (also when an API key is set), else the API key; a 401 gets one forced `RenewSession` and one retry. On failure the command stops and keeps the token file; on success the normal logout follows. - `GET /api/v1/dashboard` — `dashboard`: covers all users; `failing_instances` has status error only, `expiring_soon` the next hour. For the role `user`, `clusters` has ID and name only (no health): the table hides clusters without `health_status`. diff --git a/README.md b/README.md index ff25020..8124dc2 100644 --- a/README.md +++ b/README.md @@ -320,7 +320,7 @@ stackctl stack rollback my-app --target-log List commands (`stack list`, `definition list`, `template list`) show one page: 25 items by default, `--page-size` up to 100. When the server has more items, the command writes `Showing X of TOTAL. Use --page/--page-size to see more.` to stderr, so `-q` and `-o json` output stays clean. `-q` without `--page` prints the IDs of all pages, so `stackctl stack list --mine -q | xargs ...` gets every ID. Older servers return every match of `--mine` and name queries on one page. When a name matches more than one stack or definition, the error lists the first page and the number of matches. -`stack deploy`, `stop`, `clean` and `rollback` with `--follow` (`-f`), and `stack watch --id`, wait for a terminal status: `running`, `stopped` or `draft` (exit code 0), or `error` or `partial` (exit code 1). `partial` means that some charts deployed and others failed; the message names the operation, the status and the server error, for example `deploy partially failed (status partial): some charts did not deploy: ...`. `--follow` reads the instance after it connects, so an operation that ended before the connection counts. It handles a server close like `stack watch`: after `token expired` it renews the session, connects again and reads the instance, so a final status during the reconnect counts. When `--follow` cannot follow the operation to the end (session revoked, SSO login without refresh token, failed renewal, lost connection), it exits with code 1 and says that the operation continues on the server. Do not start the operation again: run `stackctl stack watch --id ` or `stackctl stack status `. +`stack deploy`, `stop`, `clean` and `rollback` with `--follow` (`-f`), and `stack watch --id`, wait for a terminal status: `running`, `stopped` or `draft` (exit code 0), or `error` or `partial` (exit code 1). `partial` means that some charts deployed and others failed; the message names the operation, the status and the server error, for example `deploy partially failed (status partial): some charts did not deploy: ...`. `--follow` reads the instance after it connects, so an operation that ended before the connection counts. It handles a server close like `stack watch`: after `token expired` it renews the session, connects again and reads the instance, so a final status during the reconnect counts. When `--follow` cannot follow the operation to the end (session revoked, SSO login without refresh token, failed renewal, lost connection, or a normal close by the server, for example on a restart, while the status of the instance is not final), it exits with code 1 and says that the operation continues on the server. Do not start the operation again: run `stackctl stack watch --id ` or `stackctl stack status `. `--follow` and `stack watch` work with an API key on newer servers. An older server accepts only a login session for live updates and refuses an API key: stackctl then prints `This server does not accept API keys for live updates; use a login session or upgrade the server.` diff --git a/cli/cmd/stack.go b/cli/cmd/stack.go index dc32480..0711279 100644 --- a/cli/cmd/stack.go +++ b/cli/cmd/stack.go @@ -49,7 +49,8 @@ func followLogs(c *client.Client, instanceID, op string) error { // it on shutdown. op is the operation that the command started (deploy, // stop, clean, rollback), or followOpLogs. Returns nil when: // - the operation reaches a terminal non-error status (running, -// stopped, draft, "unknown" on graceful server close); +// stopped, draft); after a normal server close it reads the instance +// for the status; // - ctx is cancelled before any other error surfaces (Ctrl-C path). // // For an operation, it reads the instance right after the first connect: @@ -101,8 +102,25 @@ func followLogsCtx(ctx context.Context, c *client.Client, instanceID, op string, return followStoppedError(followCloseError(closeErr, outcome, rerr), instanceID, op) } } + return followEndError(ctx, c, instanceID, op, result, warn) + } +} + +// followEndError returns the error for the end of a stream. A normal close +// (code 1000, for example a server restart) ends the stream without a +// final status: then it reads the instance and fails when the status is not +// final, so the exit code never claims a result that stackctl did not see. +func followEndError(ctx context.Context, c *client.Client, instanceID, op string, result *types.StreamResult, warn io.Writer) error { + if result == nil || client.IsTerminalStatus(result.Status) { return followResultError(instanceID, op, result) } + if final := followCatchUp(ctx, c, instanceID, warn); final != nil { + return followResultError(instanceID, op, final) + } + if ctx.Err() != nil { + return nil + } + return followStoppedError(errors.New("the server closed the connection before the operation ended"), instanceID, op) } // followCloseError returns the message for a server close that ends diff --git a/cli/cmd/stack_follow_test.go b/cli/cmd/stack_follow_test.go index e06f69d..54bd57e 100644 --- a/cli/cmd/stack_follow_test.go +++ b/cli/cmd/stack_follow_test.go @@ -376,3 +376,47 @@ func TestStackWatchCmd_PartialIsFailedTerminal(t *testing.T) { } assert.Contains(t, out.String(), "42 partial") } + +// TestFollowLogsCtx_NormalCloseReadsInstance: a normal close (code 1000) +// ends the stream without a final status. --follow reads the instance and +// exits 0 only for a final status that is not a failure. +func TestFollowLogsCtx_NormalCloseReadsInstance(t *testing.T) { + tests := []struct { + name string + op string + status string + wantErr string + }{ + {"deploy ended running", followOpDeploy, "running", ""}, + {"deploy ended error", followOpDeploy, "error", "deploy failed (status error): image pull failed"}, + {"deploy still deploying", followOpDeploy, "deploying", "the server closed the connection before the operation ended\nThe deploy continues on the server. Do not start it again."}, + {"logs still deploying", followOpLogs, "deploying", "The operation continues on the server"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var getCalls atomic.Int32 + // The first read (after the subscribe) sees the operation in + // progress; the read after the close sees tt.status. + server, conns := watchCloseServer(t, func(_ int32, _ *http.Request, conn *websocket.Conn) { + readSubscribe(conn) + writeWatchStatus(t, conn, "42", "deploying") + _ = conn.WriteControl(websocket.CloseMessage, + websocket.FormatCloseMessage(websocket.CloseNormalClosure, ""), + time.Now().Add(time.Second)) + }, map[string]http.HandlerFunc{ + "/api/v1/stack-instances/42": instanceRoute(&getCalls, "deploying", tt.status), + }) + defer server.Close() + _ = setupStackTestCmd(t, server.URL) + + _, err := runFollow(t, tt.op) + if tt.wantErr == "" { + require.NoError(t, err) + } else { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + } + assert.Equal(t, int32(1), conns.Load(), "no reconnect") + }) + } +} From d9d250fa5b2f2f319f829db75fd818161e93da44 Mon Sep 17 00:00:00 2001 From: omattsson Date: Tue, 27 Oct 2026 21:21:11 +0100 Subject: [PATCH 3/4] fix(cmd): a declined confirmation exits with code 1 A declined confirmation prompt printed "Aborted." to stdout and exited 0, so a script could not tell a declined delete from a done one. It now returns errAborted: exit code 1 and "aborted: not confirmed, nothing changed" on stderr. This covers every command with a confirmation (confirmAction), including bulk, override, chart and action commands. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 2 +- README.md | 8 ++++++++ cli/cmd/agent_guide.md | 4 ++-- cli/cmd/apikey.go | 3 +-- cli/cmd/apikey_test.go | 5 ++--- cli/cmd/bulk.go | 9 +++------ cli/cmd/bulk_test.go | 15 ++++++--------- cli/cmd/chart_changes.go | 9 +++------ cli/cmd/chart_changes_test.go | 34 ++++++++++++++++++++++----------- cli/cmd/cleanup_policy_test.go | 5 ++--- cli/cmd/cluster.go | 3 +-- cli/cmd/cluster_test.go | 15 ++++++--------- cli/cmd/definition_test.go | 5 ++--- cli/cmd/notification_channel.go | 3 +-- cli/cmd/orphaned.go | 3 +-- cli/cmd/orphaned_test.go | 5 ++--- cli/cmd/override.go | 10 +++------- cli/cmd/override_test.go | 15 ++++++--------- cli/cmd/root.go | 8 ++++++-- cli/cmd/stack.go | 6 ++---- cli/cmd/stack_action.go | 3 +-- cli/cmd/stack_action_test.go | 2 +- cli/cmd/stack_test.go | 10 ++++------ cli/cmd/template_test.go | 5 ++--- cli/test/e2e/cli_e2e_test.go | 11 +++++++---- 25 files changed, 96 insertions(+), 102 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 12354f2..bc3b4b7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -108,7 +108,7 @@ cli/ # Go module root (github.com/omattsson/stackctl/cl - `yaml`: machine-readable, full API response - `--quiet`: IDs only, one per line (pipeable to `xargs`) -**Destructive operations**: Commands that delete or clean resources must prompt for confirmation. `--yes` flag skips the prompt. +**Destructive operations**: Commands that delete or clean resources must prompt for confirmation (`confirmAction`). `--yes` flag skips the prompt. A declined prompt returns `errAborted` (exit 1, "aborted: not confirmed, nothing changed" on stderr), so scripts can tell that nothing changed. ## HTTP Client Pattern diff --git a/README.md b/README.md index 8124dc2..5c4af14 100644 --- a/README.md +++ b/README.md @@ -687,6 +687,14 @@ stackctl stack list -q `definition export` and `audit log export` do not use `-o`. They use `--output-file` and, for the audit export, `--format` (`json` or `csv`). +## Confirmations + +Commands that delete, clean, roll back or replace data ask for confirmation. `--yes` (`-y`) skips the question. Without `--yes`: + +- `y` runs the command. +- Any other answer stops it with exit code 1 and `aborted: not confirmed, nothing changed` on stderr. +- A closed stdin (no answer, for example `< /dev/null`) fails with `use --yes to confirm`. + ## Global Flags | Flag | Short | Description | diff --git a/cli/cmd/agent_guide.md b/cli/cmd/agent_guide.md index 8aa62a0..932d7ce 100644 --- a/cli/cmd/agent_guide.md +++ b/cli/cmd/agent_guide.md @@ -9,8 +9,8 @@ Run `stackctl --help` for the flags of a command. This guide covers wh 1. **Read data with `-o json`.** List, get, create and other commands that return a resource then print only JSON on stdout; errors, warnings and notes go to stderr. The default table output is for people and can change. 2. **Actions print text, not JSON.** `stack deploy`, `stop`, `clean`, `delete` and every `--dry-run` print a plain message on stdout even with `-o json`; `--follow` streams the log to stdout. Read the result afterwards with `stackctl stack get -o json`. 3. **`-q` prints IDs, one per line**, for pipes and `$(...)`. On `stack create`, `clone` and `template quick-deploy` it is the stack ID; on `stack deploy`, `stop` and `clean` it is the ID of the deployment log. -4. **Exit code 0 means no error, 1 means an error.** The error text is on stderr. Do not parse table output to detect errors. Exit 0 does not always mean that the change happened: see rule 5 and "Long-running operations". -5. **Destructive commands ask for confirmation.** A command with a `--yes` flag (see its `--help`) reads the answer from stdin. Any answer other than `y` prints `Aborted.` and exits **0** without a change; a closed stdin fails with `use --yes to confirm`; an open stdin without input waits forever. Pass `--yes` only when the user asked for that change, and run commands with `< /dev/null` so a missing `--yes` fails instead of hanging. +4. **Exit code 0 means no error, 1 means an error.** The error text is on stderr. Do not parse table output to detect errors. For deploy, stop, clean and rollback, exit 0 means that the server accepted the operation, not that it ended: see "Long-running operations". +5. **Destructive commands ask for confirmation.** A command with a `--yes` flag (see its `--help`) reads the answer from stdin. Any answer other than `y` exits 1 with `aborted: not confirmed, nothing changed`; a closed stdin fails with `use --yes to confirm`; an open stdin without input waits forever. Pass `--yes` only when the user asked for that change, and run commands with `< /dev/null` so a missing `--yes` fails instead of hanging. 6. **`--dry-run` does not check anything.** It prints the action and the name you gave and contacts no server: a misspelled name still "succeeds". To check the target, run `stackctl stack get -o json` first. 7. **Do not retry an operation that is still running.** See "Long-running operations". 8. **Never print or log secrets.** Do not echo `STACKCTL_API_KEY`, `STACKCTL_PASSWORD`, `STACKCTL_TOKEN` or the token files in the config directory. Prefer environment variables over `--api-key` and `--password`, which other processes can see. diff --git a/cli/cmd/apikey.go b/cli/cmd/apikey.go index 5f4a3fb..eb41549 100644 --- a/cli/cmd/apikey.go +++ b/cli/cmd/apikey.go @@ -217,8 +217,7 @@ Examples: return err } if !confirmed { - printer.PrintMessage("Aborted.") - return nil + return errAborted } c, err := newClient() diff --git a/cli/cmd/apikey_test.go b/cli/cmd/apikey_test.go index d0d2ed2..d1aa5eb 100644 --- a/cli/cmd/apikey_test.go +++ b/cli/cmd/apikey_test.go @@ -403,7 +403,7 @@ func TestAPIKeyRevokeCmd_InteractiveDecline(t *testing.T) { })) defer server.Close() - buf := setupStackTestCmd(t, server.URL) + _ = setupStackTestCmd(t, server.URL) apikeyRevokeCmd.SetIn(strings.NewReader("n\n")) apikeyRevokeCmd.SetErr(&bytes.Buffer{}) t.Cleanup(func() { @@ -411,8 +411,7 @@ func TestAPIKeyRevokeCmd_InteractiveDecline(t *testing.T) { apikeyRevokeCmd.SetErr(nil) }) - require.NoError(t, apikeyRevokeCmd.RunE(apikeyRevokeCmd, []string{"k1"})) - assert.Contains(t, buf.String(), "Aborted") + require.ErrorIs(t, apikeyRevokeCmd.RunE(apikeyRevokeCmd, []string{"k1"}), errAborted) } func TestAPIKeyRevokeCmd_QuietOutput(t *testing.T) { diff --git a/cli/cmd/bulk.go b/cli/cmd/bulk.go index 88bcc32..a233761 100644 --- a/cli/cmd/bulk.go +++ b/cli/cmd/bulk.go @@ -140,8 +140,7 @@ Examples: return err } if !confirmed { - printer.PrintMessage("Aborted.") - return nil + return errAborted } resp, err := c.BulkClean(ids) @@ -199,8 +198,7 @@ Examples: return err } if !confirmed { - printer.PrintMessage("Aborted.") - return nil + return errAborted } resp, err := c.BulkDelete(ids) @@ -324,8 +322,7 @@ Examples: return err } if !confirmed { - printer.PrintMessage("Aborted.") - return nil + return errAborted } resp, err := c.BulkDeleteTemplates(ids) diff --git a/cli/cmd/bulk_test.go b/cli/cmd/bulk_test.go index f8447fa..9eb2784 100644 --- a/cli/cmd/bulk_test.go +++ b/cli/cmd/bulk_test.go @@ -180,7 +180,7 @@ func TestBulkCleanCmd_Declined(t *testing.T) { })) defer server.Close() - buf := setupBulkTestCmd(t, server.URL) + _ = setupBulkTestCmd(t, server.URL) bulkCleanCmd.Flags().Set("ids", "1,2,3") bulkCleanCmd.Flags().Set("yes", "false") @@ -195,8 +195,7 @@ func TestBulkCleanCmd_Declined(t *testing.T) { bulkCleanCmd.SetErr(&bytes.Buffer{}) err := bulkCleanCmd.RunE(bulkCleanCmd, []string{}) - require.NoError(t, err) - assert.Contains(t, buf.String(), "Aborted") + require.ErrorIs(t, err, errAborted) } func TestBulkCleanCmd_WithYesFlag(t *testing.T) { @@ -265,7 +264,7 @@ func TestBulkDeleteCmd_Declined(t *testing.T) { })) defer server.Close() - buf := setupBulkTestCmd(t, server.URL) + _ = setupBulkTestCmd(t, server.URL) bulkDeleteCmd.Flags().Set("ids", "1,2,3") bulkDeleteCmd.Flags().Set("yes", "false") @@ -280,8 +279,7 @@ func TestBulkDeleteCmd_Declined(t *testing.T) { bulkDeleteCmd.SetErr(&bytes.Buffer{}) err := bulkDeleteCmd.RunE(bulkDeleteCmd, []string{}) - require.NoError(t, err) - assert.Contains(t, buf.String(), "Aborted") + require.ErrorIs(t, err, errAborted) } func TestBulkDeleteCmd_WithYesFlag(t *testing.T) { @@ -1109,7 +1107,7 @@ func TestBulkTemplateDeleteCmd_Declined(t *testing.T) { })) defer server.Close() - buf := setupBulkTestCmd(t, server.URL) + _ = setupBulkTestCmd(t, server.URL) bulkTemplateDeleteCmd.Flags().Set("ids", "1,2,3") bulkTemplateDeleteCmd.Flags().Set("yes", "false") @@ -1124,8 +1122,7 @@ func TestBulkTemplateDeleteCmd_Declined(t *testing.T) { bulkTemplateDeleteCmd.SetErr(&bytes.Buffer{}) err := bulkTemplateDeleteCmd.RunE(bulkTemplateDeleteCmd, []string{}) - require.NoError(t, err) - assert.Contains(t, buf.String(), "Aborted") + require.ErrorIs(t, err, errAborted) } func TestBulkTemplateDeleteCmd_WithYesFlag(t *testing.T) { diff --git a/cli/cmd/chart_changes.go b/cli/cmd/chart_changes.go index 4149e59..743b34b 100644 --- a/cli/cmd/chart_changes.go +++ b/cli/cmd/chart_changes.go @@ -173,8 +173,7 @@ Examples: return err } if !ok { - printer.PrintMessage("Aborted.") - return nil + return errAborted } if err := c.DeleteDefinitionChart(defID, chartID); err != nil { return err @@ -371,8 +370,7 @@ Examples: return err } if !ok { - printer.PrintMessage("Aborted.") - return nil + return errAborted } } @@ -564,8 +562,7 @@ Examples: return err } if !ok { - printer.PrintMessage("Aborted.") - return nil + return errAborted } if err := c.DeleteTemplateChart(templateID, chartID); err != nil { return err diff --git a/cli/cmd/chart_changes_test.go b/cli/cmd/chart_changes_test.go index 01dc0d8..e2ed2e0 100644 --- a/cli/cmd/chart_changes_test.go +++ b/cli/cmd/chart_changes_test.go @@ -128,10 +128,11 @@ func TestDefinitionRemoveChartCmd(t *testing.T) { stdin string wantDelete bool wantOut string + wantErr error }{ - {"yes flag", true, "", true, "Removed chart c2 from definition 7\n"}, - {"confirmed", false, "y\n", true, "Removed chart c2 from definition 7\n"}, - {"declined", false, "n\n", false, "Aborted.\n"}, + {"yes flag", true, "", true, "Removed chart c2 from definition 7\n", nil}, + {"confirmed", false, "y\n", true, "Removed chart c2 from definition 7\n", nil}, + {"declined", false, "n\n", false, "", errAborted}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -146,7 +147,12 @@ func TestDefinitionRemoveChartCmd(t *testing.T) { definitionRemoveChartCmd.SetIn(strings.NewReader(tt.stdin)) } - require.NoError(t, definitionRemoveChartCmd.RunE(definitionRemoveChartCmd, []string{"7", "worker"})) + err := definitionRemoveChartCmd.RunE(definitionRemoveChartCmd, []string{"7", "worker"}) + if tt.wantErr != nil { + require.ErrorIs(t, err, tt.wantErr) + } else { + require.NoError(t, err) + } assert.Equal(t, tt.wantDelete, containsString(rs.requests, "DELETE /api/v1/stack-definitions/7/charts/c2")) assert.Equal(t, tt.wantOut, buf.String()) if !tt.yes { @@ -228,12 +234,13 @@ func TestDefinitionUpgradeCmd(t *testing.T) { wantPost bool wantOut string wantStderr string + wantErr error }{ - {"yes", upgradeAvailable, true, "", true, "Upgraded definition 7 to version 1.1.0\n", ""}, + {"yes", upgradeAvailable, true, "", true, "Upgraded definition 7 to version 1.1.0\n", "", nil}, {"confirmed", upgradeAvailable, false, "y\n", true, "Upgraded definition 7 to version 1.1.0\n", - "Upgrade definition 7 to version 1.1.0? The default values of the template charts replace the values of the definition. (y/n): "}, - {"declined", upgradeAvailable, false, "n\n", false, "Aborted.\n", ""}, - {"nothing to do", noUpgrade, true, "", false, "No upgrade: the definition has template version 1.1.0, and the template has no newer published version.\n", ""}, + "Upgrade definition 7 to version 1.1.0? The default values of the template charts replace the values of the definition. (y/n): ", nil}, + {"declined", upgradeAvailable, false, "n\n", false, "", "", errAborted}, + {"nothing to do", noUpgrade, true, "", false, "No upgrade: the definition has template version 1.1.0, and the template has no newer published version.\n", "", nil}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -249,7 +256,12 @@ func TestDefinitionUpgradeCmd(t *testing.T) { definitionUpgradeCmd.SetIn(strings.NewReader(tt.stdin)) } - require.NoError(t, definitionUpgradeCmd.RunE(definitionUpgradeCmd, []string{"7"})) + err := definitionUpgradeCmd.RunE(definitionUpgradeCmd, []string{"7"}) + if tt.wantErr != nil { + require.ErrorIs(t, err, tt.wantErr) + } else { + require.NoError(t, err) + } assert.Equal(t, tt.wantPost, containsString(rs.requests, "POST /api/v1/stack-definitions/7/upgrade")) assert.Equal(t, tt.wantOut, buf.String()) assert.Contains(t, stderr.String(), tt.wantStderr) @@ -443,8 +455,8 @@ func TestChartCmds_OutputModes(t *testing.T) { rs := &recordServer{routes: map[string]route{"GET /api/v1/templates/9": {body: `{"id":"9","charts":[{"id":"t2","chart_name":"worker"}]}`}}} buf, _ := setupChartCmd(t, templateRemoveChartCmd, rs) templateRemoveChartCmd.SetIn(strings.NewReader("n\n")) - require.NoError(t, templateRemoveChartCmd.RunE(templateRemoveChartCmd, []string{"9", "worker"})) - assert.Equal(t, "Aborted.\n", buf.String()) + require.ErrorIs(t, templateRemoveChartCmd.RunE(templateRemoveChartCmd, []string{"9", "worker"}), errAborted) + assert.Empty(t, buf.String()) assert.NotContains(t, strings.Join(rs.requests, ","), "DELETE") }) } diff --git a/cli/cmd/cleanup_policy_test.go b/cli/cmd/cleanup_policy_test.go index 8fae1f0..b43c4bc 100644 --- a/cli/cmd/cleanup_policy_test.go +++ b/cli/cmd/cleanup_policy_test.go @@ -722,7 +722,7 @@ func TestCleanupPolicyDeleteCmd_InteractiveDecline(t *testing.T) { })) defer server.Close() - buf := setupStackTestCmd(t, server.URL) + _ = setupStackTestCmd(t, server.URL) cleanupPolicyDeleteCmd.SetIn(strings.NewReader("n\n")) cleanupPolicyDeleteCmd.SetErr(&bytes.Buffer{}) @@ -731,8 +731,7 @@ func TestCleanupPolicyDeleteCmd_InteractiveDecline(t *testing.T) { cleanupPolicyDeleteCmd.SetErr(nil) }) - require.NoError(t, cleanupPolicyDeleteCmd.RunE(cleanupPolicyDeleteCmd, []string{"1"})) - assert.Contains(t, buf.String(), "Aborted") + require.ErrorIs(t, cleanupPolicyDeleteCmd.RunE(cleanupPolicyDeleteCmd, []string{"1"}), errAborted) } // ---------- run ---------- diff --git a/cli/cmd/cluster.go b/cli/cmd/cluster.go index 0f754c7..d3c014a 100644 --- a/cli/cmd/cluster.go +++ b/cli/cmd/cluster.go @@ -513,8 +513,7 @@ Examples: return err } if !confirmed { - printer.PrintMessage(msgAborted) - return nil + return errAborted } c, err := newClient() diff --git a/cli/cmd/cluster_test.go b/cli/cmd/cluster_test.go index 230e0d4..7e56f9e 100644 --- a/cli/cmd/cluster_test.go +++ b/cli/cmd/cluster_test.go @@ -776,7 +776,7 @@ func TestClusterSharedValuesDeleteCmd_Declined(t *testing.T) { })) defer server.Close() - buf := setupClusterTestCmd(t, server.URL) + _ = setupClusterTestCmd(t, server.URL) clusterSharedValuesDeleteCmd.Flags().Set("yes", "false") t.Cleanup(func() { @@ -789,8 +789,7 @@ func TestClusterSharedValuesDeleteCmd_Declined(t *testing.T) { clusterSharedValuesDeleteCmd.SetErr(&bytes.Buffer{}) err := clusterSharedValuesDeleteCmd.RunE(clusterSharedValuesDeleteCmd, []string{"1", "5"}) - require.NoError(t, err) - assert.Contains(t, buf.String(), "Aborted") + require.ErrorIs(t, err, errAborted) } func TestClusterSharedValuesDeleteCmd_QuietOutput(t *testing.T) { @@ -1201,7 +1200,7 @@ func TestClusterDeleteCmd_Declined(t *testing.T) { })) defer server.Close() - buf := setupClusterTestCmd(t, server.URL) + _ = setupClusterTestCmd(t, server.URL) clusterDeleteCmd.Flags().Set("yes", "false") t.Cleanup(func() { clusterDeleteCmd.Flags().Set("yes", "false") @@ -1212,8 +1211,7 @@ func TestClusterDeleteCmd_Declined(t *testing.T) { clusterDeleteCmd.SetErr(&bytes.Buffer{}) err := clusterDeleteCmd.RunE(clusterDeleteCmd, []string{"1"}) - require.NoError(t, err) - assert.Contains(t, buf.String(), "Aborted") + require.ErrorIs(t, err, errAborted) } func TestClusterDeleteCmd_DryRun(t *testing.T) { @@ -2242,7 +2240,7 @@ func TestClusterQuotaDeleteCmd_Declined(t *testing.T) { })) defer server.Close() - buf := setupClusterTestCmd(t, server.URL) + _ = setupClusterTestCmd(t, server.URL) clusterQuotaDeleteCmd.Flags().Set("yes", "false") t.Cleanup(func() { resetFlag(t, clusterQuotaDeleteCmd.Flags(), "yes", "false") @@ -2253,8 +2251,7 @@ func TestClusterQuotaDeleteCmd_Declined(t *testing.T) { clusterQuotaDeleteCmd.SetErr(&bytes.Buffer{}) err := clusterQuotaDeleteCmd.RunE(clusterQuotaDeleteCmd, []string{"1"}) - require.NoError(t, err) - assert.Contains(t, buf.String(), "Aborted") + require.ErrorIs(t, err, errAborted) } func TestClusterQuotaDeleteCmd_Forbidden(t *testing.T) { diff --git a/cli/cmd/definition_test.go b/cli/cmd/definition_test.go index 62fbd0b..04b5191 100644 --- a/cli/cmd/definition_test.go +++ b/cli/cmd/definition_test.go @@ -547,7 +547,7 @@ func TestDefinitionDeleteCmd_Declined(t *testing.T) { })) defer server.Close() - buf := setupStackTestCmd(t, server.URL) + _ = setupStackTestCmd(t, server.URL) definitionDeleteCmd.Flags().Set("yes", "false") t.Cleanup(func() { @@ -560,8 +560,7 @@ func TestDefinitionDeleteCmd_Declined(t *testing.T) { definitionDeleteCmd.SetErr(&bytes.Buffer{}) err := definitionDeleteCmd.RunE(definitionDeleteCmd, []string{"5"}) - require.NoError(t, err) - assert.Contains(t, buf.String(), "Aborted") + require.ErrorIs(t, err, errAborted) } func TestDefinitionDeleteCmd_NotFound(t *testing.T) { diff --git a/cli/cmd/notification_channel.go b/cli/cmd/notification_channel.go index 9942d02..07cc16c 100644 --- a/cli/cmd/notification_channel.go +++ b/cli/cmd/notification_channel.go @@ -563,8 +563,7 @@ Examples: return err } if !ok { - printer.PrintMessage("Aborted.") - return nil + return errAborted } if err := c.DeleteNotificationChannel(id); err != nil { return err diff --git a/cli/cmd/orphaned.go b/cli/cmd/orphaned.go index fe26f7d..bf2f30f 100644 --- a/cli/cmd/orphaned.go +++ b/cli/cmd/orphaned.go @@ -137,8 +137,7 @@ Examples: return err } if !confirmed { - printer.PrintMessage("Aborted.") - return nil + return errAborted } c, err := newClient() diff --git a/cli/cmd/orphaned_test.go b/cli/cmd/orphaned_test.go index ebd49c4..548f283 100644 --- a/cli/cmd/orphaned_test.go +++ b/cli/cmd/orphaned_test.go @@ -175,7 +175,7 @@ func TestOrphanedDeleteCmd_Declined(t *testing.T) { })) defer server.Close() - buf := setupStackTestCmd(t, server.URL) + _ = setupStackTestCmd(t, server.URL) orphanedDeleteCmd.Flags().Set("yes", "false") t.Cleanup(func() { @@ -188,8 +188,7 @@ func TestOrphanedDeleteCmd_Declined(t *testing.T) { orphanedDeleteCmd.SetErr(&bytes.Buffer{}) err := orphanedDeleteCmd.RunE(orphanedDeleteCmd, []string{"stack-old-app"}) - require.NoError(t, err) - assert.Contains(t, buf.String(), "Aborted") + require.ErrorIs(t, err, errAborted) } func TestOrphanedDeleteCmd_NotFound(t *testing.T) { diff --git a/cli/cmd/override.go b/cli/cmd/override.go index 995582b..93a20ed 100644 --- a/cli/cmd/override.go +++ b/cli/cmd/override.go @@ -19,7 +19,6 @@ import ( ) const ( - msgAborted = "Aborted." flagDescSkipConfirm = "Skip confirmation prompt" ) @@ -255,8 +254,7 @@ Examples: return err } if !confirmed { - printer.PrintMessage(msgAborted) - return nil + return errAborted } } @@ -960,8 +958,7 @@ Examples: return err } if !confirmed { - printer.PrintMessage(msgAborted) - return nil + return errAborted } if err := c.DeleteQuotaOverride(instanceID); err != nil { @@ -1002,8 +999,7 @@ func deleteChartOverride(cmd *cobra.Command, args []string, kind string, deleteF return err } if !confirmed { - printer.PrintMessage(msgAborted) - return nil + return errAborted } if err := deleteFn(c, instanceID, chartID); err != nil { diff --git a/cli/cmd/override_test.go b/cli/cmd/override_test.go index 82c300c..cff7181 100644 --- a/cli/cmd/override_test.go +++ b/cli/cmd/override_test.go @@ -557,7 +557,7 @@ func TestOverrideDeleteCmd_ConfirmDecline(t *testing.T) { })) defer server.Close() - buf := setupStackTestCmd(t, server.URL) + _ = setupStackTestCmd(t, server.URL) overrideDeleteCmd.Flags().Set("yes", "false") t.Cleanup(func() { @@ -570,8 +570,7 @@ func TestOverrideDeleteCmd_ConfirmDecline(t *testing.T) { overrideDeleteCmd.SetErr(&bytes.Buffer{}) err := overrideDeleteCmd.RunE(overrideDeleteCmd, []string{"42", "1"}) - require.NoError(t, err) - assert.Contains(t, buf.String(), "Aborted") + require.ErrorIs(t, err, errAborted) } func TestOverrideDeleteCmd_QuietOutput(t *testing.T) { @@ -911,7 +910,7 @@ func TestOverrideBranchDeleteCmd_ConfirmDecline(t *testing.T) { })) defer server.Close() - buf := setupStackTestCmd(t, server.URL) + _ = setupStackTestCmd(t, server.URL) overrideBranchDeleteCmd.Flags().Set("yes", "false") t.Cleanup(func() { @@ -924,8 +923,7 @@ func TestOverrideBranchDeleteCmd_ConfirmDecline(t *testing.T) { overrideBranchDeleteCmd.SetErr(&bytes.Buffer{}) err := overrideBranchDeleteCmd.RunE(overrideBranchDeleteCmd, []string{"42", "1"}) - require.NoError(t, err) - assert.Contains(t, buf.String(), "Aborted") + require.ErrorIs(t, err, errAborted) } func TestOverrideBranchDeleteCmd_QuietOutput(t *testing.T) { @@ -1454,7 +1452,7 @@ func TestOverrideQuotaDeleteCmd_ConfirmDecline(t *testing.T) { })) defer server.Close() - buf := setupStackTestCmd(t, server.URL) + _ = setupStackTestCmd(t, server.URL) overrideQuotaDeleteCmd.Flags().Set("yes", "false") t.Cleanup(func() { @@ -1467,8 +1465,7 @@ func TestOverrideQuotaDeleteCmd_ConfirmDecline(t *testing.T) { overrideQuotaDeleteCmd.SetErr(&bytes.Buffer{}) err := overrideQuotaDeleteCmd.RunE(overrideQuotaDeleteCmd, []string{"42"}) - require.NoError(t, err) - assert.Contains(t, buf.String(), "Aborted") + require.ErrorIs(t, err, errAborted) } func TestOverrideQuotaDeleteCmd_QuietOutput(t *testing.T) { diff --git a/cli/cmd/root.go b/cli/cmd/root.go index 352f8a7..57c57b7 100644 --- a/cli/cmd/root.go +++ b/cli/cmd/root.go @@ -3,6 +3,7 @@ package cmd import ( "bufio" "crypto/tls" + "errors" "fmt" "io" "net/http" @@ -279,6 +280,10 @@ func isTerminalWriter(w io.Writer) bool { // confirmAction prompts the user for confirmation unless --yes is set. // Returns true if the action should proceed, false if aborted. +// errAborted is returned when the user declines a confirmation prompt: the +// command exits with code 1, so a script can tell that nothing changed. +var errAborted = errors.New("aborted: not confirmed, nothing changed") + func confirmAction(cmd *cobra.Command, message string) (bool, error) { yes, _ := cmd.Flags().GetBool("yes") if yes { @@ -320,8 +325,7 @@ func deleteByID(cmd *cobra.Command, args []string, promptFmt string, resolveFn f return err } if !confirmed { - printer.PrintMessage("Aborted.") - return nil + return errAborted } if err := deleteFn(c, id); err != nil { diff --git a/cli/cmd/stack.go b/cli/cmd/stack.go index 0711279..c8a8990 100644 --- a/cli/cmd/stack.go +++ b/cli/cmd/stack.go @@ -729,8 +729,7 @@ Examples: return err } if !confirmed { - printer.PrintMessage("Aborted.") - return nil + return errAborted } resp, err := c.CleanStack(id) @@ -1676,8 +1675,7 @@ Examples: return err } if !confirmed { - printer.PrintMessage("Aborted.") - return nil + return errAborted } targetLog, _ := cmd.Flags().GetString("target-log") diff --git a/cli/cmd/stack_action.go b/cli/cmd/stack_action.go index 1273a34..159094f 100644 --- a/cli/cmd/stack_action.go +++ b/cli/cmd/stack_action.go @@ -209,8 +209,7 @@ Examples: return err } if !ok { - fmt.Fprintln(cmd.ErrOrStderr(), "Aborted.") - return nil + return errAborted } } diff --git a/cli/cmd/stack_action_test.go b/cli/cmd/stack_action_test.go index cdf3534..9330acc 100644 --- a/cli/cmd/stack_action_test.go +++ b/cli/cmd/stack_action_test.go @@ -233,7 +233,7 @@ func TestStackActionRunCmd_Confirmation(t *testing.T) { wantErr string }{ {"yes", "y\n", true, ""}, - {"no", "n\n", false, ""}, + {"no", "n\n", false, "aborted: not confirmed"}, {"no answer", "", false, "use --yes to confirm"}, } for _, tt := range tests { diff --git a/cli/cmd/stack_test.go b/cli/cmd/stack_test.go index c21ca5c..9447157 100644 --- a/cli/cmd/stack_test.go +++ b/cli/cmd/stack_test.go @@ -531,7 +531,7 @@ func TestStackCleanCmd_Declined(t *testing.T) { })) defer server.Close() - buf := setupStackTestCmd(t, server.URL) + _ = setupStackTestCmd(t, server.URL) stackCleanCmd.Flags().Set("yes", "false") t.Cleanup(func() { @@ -544,8 +544,7 @@ func TestStackCleanCmd_Declined(t *testing.T) { stackCleanCmd.SetErr(&bytes.Buffer{}) err := stackCleanCmd.RunE(stackCleanCmd, []string{"42"}) - require.NoError(t, err) - assert.Contains(t, buf.String(), "Aborted") + require.ErrorIs(t, err, errAborted) } func TestStackCleanCmd_WithYesFlag(t *testing.T) { @@ -605,7 +604,7 @@ func TestStackDeleteCmd_Declined(t *testing.T) { })) defer server.Close() - buf := setupStackTestCmd(t, server.URL) + _ = setupStackTestCmd(t, server.URL) stackDeleteCmd.Flags().Set("yes", "false") t.Cleanup(func() { @@ -618,8 +617,7 @@ func TestStackDeleteCmd_Declined(t *testing.T) { stackDeleteCmd.SetErr(&bytes.Buffer{}) err := stackDeleteCmd.RunE(stackDeleteCmd, []string{"42"}) - require.NoError(t, err) - assert.Contains(t, buf.String(), "Aborted") + require.ErrorIs(t, err, errAborted) } func TestStackDeleteCmd_WithYesFlag(t *testing.T) { diff --git a/cli/cmd/template_test.go b/cli/cmd/template_test.go index bd5333f..c3abd49 100644 --- a/cli/cmd/template_test.go +++ b/cli/cmd/template_test.go @@ -597,7 +597,7 @@ func TestTemplateDeleteCmd_Declined(t *testing.T) { })) defer server.Close() - buf := setupStackTestCmd(t, server.URL) + _ = setupStackTestCmd(t, server.URL) templateDeleteCmd.Flags().Set("yes", "false") t.Cleanup(func() { @@ -610,8 +610,7 @@ func TestTemplateDeleteCmd_Declined(t *testing.T) { templateDeleteCmd.SetErr(&bytes.Buffer{}) err := templateDeleteCmd.RunE(templateDeleteCmd, []string{"10"}) - require.NoError(t, err) - assert.Contains(t, buf.String(), "Aborted") + require.ErrorIs(t, err, errAborted) } func TestTemplateDeleteCmd_NotFound(t *testing.T) { diff --git a/cli/test/e2e/cli_e2e_test.go b/cli/test/e2e/cli_e2e_test.go index 734c226..be1a634 100644 --- a/cli/test/e2e/cli_e2e_test.go +++ b/cli/test/e2e/cli_e2e_test.go @@ -1751,10 +1751,13 @@ func TestE2E_BulkTemplateDelete_NoConfirmation(t *testing.T) { dir := t.TempDir() setupE2EStackContext(t, dir, server.URL) - // Without --yes, provide "n" on stdin — should abort - stdout, _, err := runStackctlWithStdin(t, dir, "n\n", "bulk", "template", "delete", "--ids", "1,2") - require.NoError(t, err) - assert.Contains(t, stdout, "Aborted") + // Without --yes, "n" on stdin declines: exit code 1, nothing changed. + stdout, stderr, err := runStackctlWithStdin(t, dir, "n\n", "bulk", "template", "delete", "--ids", "1,2") + var exitErr *exec.ExitError + require.ErrorAs(t, err, &exitErr) + assert.Equal(t, 1, exitErr.ExitCode()) + assert.Contains(t, stderr, "aborted: not confirmed, nothing changed") + assert.NotContains(t, stdout, "Deleted") } // ---------- Cluster E2E Mock Server ---------- From 772df3a9818ae9284dc95d17bd819bb019da4951 Mon Sep 17 00:00:00 2001 From: omattsson Date: Tue, 27 Oct 2026 21:41:24 +0100 Subject: [PATCH 4/4] fix(output): stdout holds only data with -o json, -o yaml and -q Messages for people (PrintMessage: "Deleted ...", --dry-run text, "Deploying stack ...") went to stdout in every mode, so `-o json | jq` and `$(... -q)` got text. Printer.MachineReadable() (-o json|yaml, -q) now sends them to Printer.ErrWriter (stderr); the table format and custom formats keep them on stdout. - stack deploy, stop, clean and rollback share printOperation: -o json|yaml prints the 202 response (log_id, message), -q the log ID. With --follow in those modes the log goes to stderr and stdout gets the instance after the operation (-q: the log ID). `stack logs -f` prints the log as without --follow. followHelp describes it. - cluster shared-values list and config list print [] for an empty list with -o json|yaml instead of a message. - confirmAction returns errAborted itself (callers only pass the error on), and the skip-config check uses an annotation (skipsConfig) instead of command names. - agent-guide: rules for the new output, login needs a context, bulk commands exit 0 with failed items, draft after a clean, quick-deploy and bulk are asynchronous, SIGINT exits 0, no fixed --stop-at date. The guide test now also checks positional argument counts and quoted arguments. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 5 +- README.md | 2 + cli/cmd/agent_guide.go | 1 + cli/cmd/agent_guide.md | 29 ++- cli/cmd/agent_guide_test.go | 143 ++++++++++---- cli/cmd/apikey.go | 6 +- cli/cmd/bulk.go | 18 +- cli/cmd/chart_changes.go | 20 +- cli/cmd/cluster.go | 11 +- cli/cmd/completion.go | 1 + cli/cmd/config.go | 4 +- cli/cmd/notification_channel.go | 6 +- cli/cmd/orphaned.go | 6 +- cli/cmd/override.go | 18 +- cli/cmd/root.go | 43 +++-- cli/cmd/root_test.go | 10 + cli/cmd/stack.go | 164 ++++++++-------- cli/cmd/stack_action.go | 6 +- cli/cmd/stack_operation_output_test.go | 252 +++++++++++++++++++++++++ cli/cmd/version.go | 1 + cli/pkg/output/output.go | 23 ++- cli/pkg/output/output_test.go | 39 ++++ 22 files changed, 591 insertions(+), 217 deletions(-) create mode 100644 cli/cmd/stack_operation_output_test.go diff --git a/CLAUDE.md b/CLAUDE.md index bc3b4b7..d2f84a1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -107,8 +107,9 @@ cli/ # Go module root (github.com/omattsson/stackctl/cl - `json`: machine-readable, full API response - `yaml`: machine-readable, full API response - `--quiet`: IDs only, one per line (pipeable to `xargs`) +- `printer.PrintMessage` (text for people) writes to `printer.ErrWriter` (stderr) when `printer.MachineReadable()` (`-o json|yaml` or `-q`; not the table or a custom format), so stdout holds only data. An empty list prints `[]` in those modes, not a message. deploy/stop/clean/rollback end in `printOperation` (202 response for json/yaml, log ID for `-q`); with `--follow` in those modes `followOperation` sends the log to stderr (`followLogWriter`) and prints the instance (GET) or the log ID; `stack logs -f` prints the log as without `--follow` -**Destructive operations**: Commands that delete or clean resources must prompt for confirmation (`confirmAction`). `--yes` flag skips the prompt. A declined prompt returns `errAborted` (exit 1, "aborted: not confirmed, nothing changed" on stderr), so scripts can tell that nothing changed. +**Destructive operations**: Commands that delete or clean resources must prompt for confirmation: `if err := confirmAction(cmd, msg); err != nil { return err }`. `--yes` flag skips the prompt. A declined prompt returns `errAborted` (exit 1, "aborted: not confirmed, nothing changed" on stderr), so scripts can tell that nothing changed. ## HTTP Client Pattern @@ -130,7 +131,7 @@ cli/ # Go module root (github.com/omattsson/stackctl/cl **Plugins** (`cmd/plugins.go`): for a plugin, `Execute` sets `TraverseChildren`, so the global flags before the plugin name are parsed by stackctl and not passed to the plugin; `pluginEnv` exports them (`STACKCTL_OUTPUT`, `STACKCTL_QUIET`, `STACKCTL_NO_COLOR` + `NO_COLOR`, `STACKCTL_DEBUG`, `STACKCTL_INSECURE`, `STACKCTL_CONFIG_DIR`, plus the context values). Arguments after the plugin name pass unchanged. Contract in `EXTENDING.md`. -**Config-free commands**: `version`, `agent-guide` and `completion` skip config file loading and work even if the config is missing or corrupted. +**Config-free commands**: `version`, `agent-guide` and `completion` skip config file loading and work even if the config is missing or corrupted. Mark such a command (and its subcommands) with the annotation `skipConfigAnnotation` (`skipsConfig` in `root.go`). **Insecure mode**: When `--insecure` is active, a warning is printed to stderr. diff --git a/README.md b/README.md index 5c4af14..e93ebdb 100644 --- a/README.md +++ b/README.md @@ -685,6 +685,8 @@ stackctl stack list -o yaml stackctl stack list -q ``` +With `-o json`, `-o yaml` and `-q`, stdout holds only data. Messages for people (for example `Deleted stack ...`, `--dry-run` text) go to stderr. `stack deploy`, `stop`, `clean` and `rollback` print the server response (`log_id`, `message`), and `-q` prints the log ID. With `--follow` the deployment log goes to stderr and stdout gets the stack after the operation (`-q`: the log ID); `stack logs -f` prints the log as without `--follow`. An empty list prints `[]`. Custom output formats keep the messages on stdout. + `definition export` and `audit log export` do not use `-o`. They use `--output-file` and, for the audit export, `--format` (`json` or `csv`). ## Confirmations diff --git a/cli/cmd/agent_guide.go b/cli/cmd/agent_guide.go index fc16129..e83374d 100644 --- a/cli/cmd/agent_guide.go +++ b/cli/cmd/agent_guide.go @@ -28,6 +28,7 @@ without any configuration. Examples: stackctl agent-guide stackctl agent-guide > STACKCTL.md`, + Annotations: map[string]string{skipConfigAnnotation: "true"}, Args: cobra.NoArgs, SilenceUsage: true, RunE: func(cmd *cobra.Command, args []string) error { diff --git a/cli/cmd/agent_guide.md b/cli/cmd/agent_guide.md index 932d7ce..a171803 100644 --- a/cli/cmd/agent_guide.md +++ b/cli/cmd/agent_guide.md @@ -6,14 +6,13 @@ Run `stackctl --help` for the flags of a command. This guide covers wh ## Rules -1. **Read data with `-o json`.** List, get, create and other commands that return a resource then print only JSON on stdout; errors, warnings and notes go to stderr. The default table output is for people and can change. -2. **Actions print text, not JSON.** `stack deploy`, `stop`, `clean`, `delete` and every `--dry-run` print a plain message on stdout even with `-o json`; `--follow` streams the log to stdout. Read the result afterwards with `stackctl stack get -o json`. -3. **`-q` prints IDs, one per line**, for pipes and `$(...)`. On `stack create`, `clone` and `template quick-deploy` it is the stack ID; on `stack deploy`, `stop` and `clean` it is the ID of the deployment log. -4. **Exit code 0 means no error, 1 means an error.** The error text is on stderr. Do not parse table output to detect errors. For deploy, stop, clean and rollback, exit 0 means that the server accepted the operation, not that it ended: see "Long-running operations". -5. **Destructive commands ask for confirmation.** A command with a `--yes` flag (see its `--help`) reads the answer from stdin. Any answer other than `y` exits 1 with `aborted: not confirmed, nothing changed`; a closed stdin fails with `use --yes to confirm`; an open stdin without input waits forever. Pass `--yes` only when the user asked for that change, and run commands with `< /dev/null` so a missing `--yes` fails instead of hanging. -6. **`--dry-run` does not check anything.** It prints the action and the name you gave and contacts no server: a misspelled name still "succeeds". To check the target, run `stackctl stack get -o json` first. -7. **Do not retry an operation that is still running.** See "Long-running operations". -8. **Never print or log secrets.** Do not echo `STACKCTL_API_KEY`, `STACKCTL_PASSWORD`, `STACKCTL_TOKEN` or the token files in the config directory. Prefer environment variables over `--api-key` and `--password`, which other processes can see. +1. **Read with `-o json`.** stdout then holds only JSON: the resource, the list or the server response. Messages, warnings, notes and the deployment log of `--follow` go to stderr. A command without data (for example `stack delete` or `--dry-run`) leaves stdout empty. The default table output is for people and can change. +2. **`-q` prints IDs, one per line**, for pipes and `$(...)`. On `stack create`, `clone` and `template quick-deploy` it is the stack ID; on `stack deploy`, `stop`, `clean` and `rollback` it is the ID of the deployment log. +3. **Exit code 0 means no error, 1 means an error.** The error text is on stderr. Do not parse table output to detect errors. Exceptions: for operations that run on the server, exit 0 means that the server accepted the operation, not that it ended (see "Long-running operations"); `bulk` commands exit 0 even when some items fail (check `failed` and `results` in `-o json`); a plugin (`stackctl `) returns its own exit code. +4. **Destructive commands ask for confirmation.** A command with a `--yes` flag (see its `--help`) reads the answer from stdin. Any answer other than `y` exits 1 with `aborted: not confirmed, nothing changed`; a closed stdin fails with `use --yes to confirm`; an open stdin without input waits forever. Pass `--yes` only when the user asked for that change, and run commands with `< /dev/null` so a missing `--yes` fails instead of hanging. +5. **`--dry-run` does not check anything.** It prints the action and the name you gave and contacts no server: a misspelled name still "succeeds". To check the target, run `stackctl stack get -o json` first. +6. **Do not retry an operation that is still running.** See "Long-running operations". +7. **Never print or log secrets.** Do not echo `STACKCTL_API_KEY`, `STACKCTL_PASSWORD`, `STACKCTL_TOKEN` or the token files in the config directory. Prefer environment variables over `--api-key` and `--password`, which other processes can see. ## Authentication @@ -28,7 +27,7 @@ stackctl server info # server reachable? TTL limits; needs no crede Ways to authenticate without a prompt, in order of preference: - **API key:** set `STACKCTL_API_KEY` and `STACKCTL_API_URL`. No login and no token file. An API key wins over a stored login session. Servers before k8s-stack-manager v0.8.0 refuse an API key for `--follow` and `stack watch` (`This server does not accept API keys for live updates`): then poll `stack get` instead (see "Long-running operations"). -- **Username and password:** `STACKCTL_USERNAME=... STACKCTL_PASSWORD=... stackctl login`. stackctl stores a session and renews it by itself. +- **Username and password:** `STACKCTL_USERNAME=... STACKCTL_PASSWORD=... stackctl login`. stackctl stores the session in the current context and renews it by itself. It needs a context: in a new config directory run `stackctl config use-context ` and `stackctl config set api-url ` first. - `stackctl login --sso` opens a browser: do not use it unless a person is at the computer. Values resolve in this order: flag (`--api-url`, `--api-key`), environment variable, config file. `STACKCTL_CONFIG_DIR` selects another config directory, for example an empty one for an isolated run. `stackctl config use-context ` switches the environment for every later command: ask the user before you change it. @@ -40,7 +39,7 @@ Values resolve in this order: flag (`--api-url`, `--api-key`), environment varia - **Template:** a published blueprint of charts. `template quick-deploy` creates and deploys a stack from it in one step. - **Definition:** the set of Helm charts and default values that a stack uses. - **Stack (instance):** one deployment of a definition into its own namespace on a cluster, with an optional branch and an expiry (TTL). -- **Status:** `draft` (created, never deployed), `deploying`, `running`, `stopping`, `stopped`, `cleaning`, `error`, `failed`, `partial` (some charts failed). +- **Status:** `draft` (not deployed: new, or after a clean), `deploying`, `running`, `stopping`, `stopped`, `cleaning`, `error`, `failed`, `partial` (some charts failed). - **stop** stops the stack and keeps it (a deploy starts it again); **clean** undeploys it and removes its namespace; **delete** removes the stack. - **Overrides:** per-stack Helm values (`override`), per-chart branches (`override branch`) and quotas (`override quota`). @@ -57,7 +56,7 @@ stackctl dashboard -o json # failing and expiring stacks, recent # Create and deploy ID=$(stackctl stack create --definition my-def --name my-app --branch feature/x -q) -stackctl stack deploy "$ID" --follow +stackctl stack deploy "$ID" --follow -o json # log on stderr, the stack after the deploy on stdout stackctl template quick-deploy --name my-app --ttl 480 -o json # Inspect @@ -75,7 +74,7 @@ stackctl stack deploy my-app --follow # Expiry stackctl stack extend my-app --minutes 120 -stackctl stack update my-app --stop-at "2026-10-17 18:00" +stackctl stack update my-app --stop-at "" # a future local time; needs k8s-stack-manager v0.12.0+ # Stop, clean up, delete (only when the user asked) stackctl stack stop my-app --follow @@ -88,15 +87,15 @@ Paged lists (`stack list`, `definition list`, `template list`) return one page a ## Long-running operations -`stack deploy`, `stop`, `clean` and `rollback` return when the server has accepted the operation; the work continues on the server. To wait for the result: +`stack deploy`, `stop`, `clean`, `rollback`, `template quick-deploy` and the `bulk` commands return when the server has accepted the operation; the work continues on the server (a `bulk` result `success` means that the operation started). To wait for the result: -- add `--follow` (`-f`): stackctl streams the log and exits when the stack reaches a final status; +- add `--follow` (`-f`, only `stack deploy`, `stop`, `clean` and `rollback`): stackctl streams the log and exits when the stack reaches a final status. With `-o json` the log goes to stderr and stdout gets the stack after the operation (also after a failed one); - or run `stackctl stack watch --id [,...]`, which exits when every listed stack reaches a final status; - or poll `stackctl stack get -o json` every few seconds until `status` is final. Final statuses: `running`, `stopped` and `draft` exit 0; `error`, `failed` and `partial` exit 1, with the server error on stderr. -**Check the status after exit 0.** `--follow` also exits 0 when the server closes the connection normally (for example on a restart) before the operation ends. Read `status` from `stackctl stack get -o json` and confirm it is the one you expected (`running` after a deploy, `stopped` after a stop). +**Check that the final status is the expected one.** `--follow` exits 0 for any final status that is not a failure, and also when it is interrupted (SIGINT), which does not stop the operation. Expect `running` after a deploy or rollback, `stopped` after a stop and `draft` after a clean. When `--follow` or `watch --id` stops early for another reason (lost connection, session ended), it exits 1 and says that the operation continues on the server. **Do not start the operation again.** Run `stackctl stack watch --id `, or poll `stackctl stack get -o json`. diff --git a/cli/cmd/agent_guide_test.go b/cli/cmd/agent_guide_test.go index ae7c6c0..4d72631 100644 --- a/cli/cmd/agent_guide_test.go +++ b/cli/cmd/agent_guide_test.go @@ -2,6 +2,7 @@ package cmd import ( "bytes" + "fmt" "os" "path/filepath" "regexp" @@ -9,6 +10,7 @@ import ( "testing" "github.com/spf13/cobra" + "github.com/spf13/pflag" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -42,7 +44,12 @@ func TestAgentGuideCmd_IgnoresOutputFormat(t *testing.T) { } { t.Run(strings.Join(args, " "), func(t *testing.T) { t.Setenv("STACKCTL_CONFIG_DIR", t.TempDir()) - t.Cleanup(func() { flagOutput, flagQuiet = "table", false }) + t.Cleanup(func() { + flagOutput, flagQuiet = "table", false + // pluginEnv reads Changed: do not leak it into later tests. + rootCmd.PersistentFlags().Lookup("output").Changed = false + rootCmd.PersistentFlags().Lookup("quiet").Changed = false + }) var stdout bytes.Buffer rootCmd.SetOut(&stdout) @@ -80,23 +87,53 @@ func TestRootHelp_PointsToAgentGuide(t *testing.T) { // TestAgentGuide_CommandsExist keeps the guide in step with the CLI: every // stackctl invocation in a code span or code block must name existing -// commands, and each of its flags must exist on that command. It reads -// rootCmd, so it does not run in parallel. +// commands, each of its flags must exist on that command, and the command +// must accept the number of positional arguments. Commands in prose without +// the stackctl prefix are not checked. It reads rootCmd, so it does not run +// in parallel. func TestAgentGuide_CommandsExist(t *testing.T) { invocations := guideInvocations(agentGuide) require.NotEmpty(t, invocations) for _, inv := range invocations { t.Run(inv, func(t *testing.T) { - cmd, rest, err := walkCommand(rootCmd, strings.Fields(inv)[1:]) + cmd, rest, err := walkCommand(rootCmd, shellWords(inv)[1:]) + require.NoError(t, err) + args, err := positionalArgs(cmd, rest) require.NoError(t, err) - for _, tok := range rest { - name, ok := flagName(tok) - if !ok { - continue - } - assert.NotNil(t, lookupFlag(cmd, name), "%s has no flag %s", cmd.CommandPath(), tok) + assert.NoError(t, cmd.ValidateArgs(args), "arguments of %s", cmd.CommandPath()) + }) + } +} + +func TestShellWords(t *testing.T) { + t.Parallel() + assert.Equal(t, + []string{"stackctl", "stack", "update", "x", "--stop-at", "", "$ID"}, + shellWords(`stackctl stack update x --stop-at "" "$ID"`)) +} + +func TestPositionalArgs(t *testing.T) { + tests := []struct { + name string + cmd *cobra.Command + args []string + want []string + wantErr string + }{ + {"value flag takes the next word", stackCreateCmd, []string{"x", "--definition", "d", "--branch=b"}, []string{"x"}, ""}, + {"bool and global flags", stackDeleteCmd, []string{"--dry-run", "x", "-o", "json", "-q", "--help"}, []string{"x"}, ""}, + {"unknown flag", stackCreateCmd, []string{"--nope"}, nil, "stackctl stack create has no flag --nope"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := positionalArgs(tt.cmd, tt.args) + if tt.wantErr != "" { + require.EqualError(t, err, tt.wantErr) + return } + require.NoError(t, err) + assert.Equal(t, tt.want, got) }) } } @@ -177,38 +214,72 @@ func (e *unknownCommandError) Error() string { return e.path + " has no subcommand " + e.name } -// flagName returns the flag name of a token: "image" for "--image=x", "q" -// for "-q". It returns false for a token that is not a flag. -func flagName(tok string) (string, bool) { - tok = strings.Trim(tok, `"'`) - if !strings.HasPrefix(tok, "-") || tok == "-" { - return "", false +// shellWords splits a command line at spaces; double quotes keep words +// together and are removed. +func shellWords(line string) []string { + var words []string + var word strings.Builder + inWord, quoted := false, false + for _, r := range line { + switch { + case r == '"': + quoted = !quoted + inWord = true + case r == ' ' && !quoted: + if inWord { + words = append(words, word.String()) + word.Reset() + inWord = false + } + default: + word.WriteRune(r) + inWord = true + } } - name := strings.TrimLeft(tok, "-") - name, _, _ = strings.Cut(name, "=") - return name, name != "" + if inWord { + words = append(words, word.String()) + } + return words } -// lookupFlag finds a local or inherited flag by name or shorthand. Cobra -// adds the help flag only when a command runs, so help and h always exist. -func lookupFlag(cmd *cobra.Command, name string) any { - if name == "help" || name == "h" { - return name - } - if len(name) == 1 { - if f := cmd.Flags().ShorthandLookup(name); f != nil { - return f +// positionalArgs returns the positional arguments of cmd in args. A flag +// that is not boolean takes the next word as its value unless it has +// "=value". It fails on a flag that cmd does not have. +func positionalArgs(cmd *cobra.Command, args []string) ([]string, error) { + var positional []string + for i := 0; i < len(args); i++ { + arg := args[i] + if !strings.HasPrefix(arg, "-") || arg == "-" { + positional = append(positional, arg) + continue } - if f := cmd.InheritedFlags().ShorthandLookup(name); f != nil { - return f + name, _, hasValue := strings.Cut(strings.TrimLeft(arg, "-"), "=") + if name == "help" || name == "h" { + // Cobra adds the help flag only when a command runs. + continue + } + f := lookupFlag(cmd, name) + if f == nil { + return nil, fmt.Errorf("%s has no flag %s", cmd.CommandPath(), arg) + } + if f.Value.Type() != "bool" && !hasValue { + i++ } - return nil - } - if f := cmd.Flags().Lookup(name); f != nil { - return f } - if f := cmd.InheritedFlags().Lookup(name); f != nil { - return f + return positional, nil +} + +// lookupFlag finds a local or inherited flag by name, or by shorthand for a +// one-letter name. +func lookupFlag(cmd *cobra.Command, name string) *pflag.Flag { + for _, fs := range []*pflag.FlagSet{cmd.Flags(), cmd.InheritedFlags()} { + if len(name) == 1 { + if f := fs.ShorthandLookup(name); f != nil { + return f + } + } else if f := fs.Lookup(name); f != nil { + return f + } } return nil } diff --git a/cli/cmd/apikey.go b/cli/cmd/apikey.go index eb41549..f5b1819 100644 --- a/cli/cmd/apikey.go +++ b/cli/cmd/apikey.go @@ -212,13 +212,9 @@ Examples: return nil } - confirmed, err := confirmAction(cmd, fmt.Sprintf("This will revoke API key %s. Continue? (y/n): ", keyID)) - if err != nil { + if err := confirmAction(cmd, fmt.Sprintf("This will revoke API key %s. Continue? (y/n): ", keyID)); err != nil { return err } - if !confirmed { - return errAborted - } c, err := newClient() if err != nil { diff --git a/cli/cmd/bulk.go b/cli/cmd/bulk.go index a233761..74d2cba 100644 --- a/cli/cmd/bulk.go +++ b/cli/cmd/bulk.go @@ -135,13 +135,9 @@ Examples: return err } - confirmed, err := confirmAction(cmd, fmt.Sprintf("This will clean %d stack instances. Continue? (y/n): ", len(ids))) - if err != nil { + if err := confirmAction(cmd, fmt.Sprintf("This will clean %d stack instances. Continue? (y/n): ", len(ids))); err != nil { return err } - if !confirmed { - return errAborted - } resp, err := c.BulkClean(ids) if err != nil { @@ -193,13 +189,9 @@ Examples: return err } - confirmed, err := confirmAction(cmd, fmt.Sprintf("This will permanently delete %d stack instances. Continue? (y/n): ", len(ids))) - if err != nil { + if err := confirmAction(cmd, fmt.Sprintf("This will permanently delete %d stack instances. Continue? (y/n): ", len(ids))); err != nil { return err } - if !confirmed { - return errAborted - } resp, err := c.BulkDelete(ids) if err != nil { @@ -317,13 +309,9 @@ Examples: return err } - confirmed, err := confirmAction(cmd, fmt.Sprintf("This will permanently delete %d template(s). Continue? (y/n): ", len(ids))) - if err != nil { + if err := confirmAction(cmd, fmt.Sprintf("This will permanently delete %d template(s). Continue? (y/n): ", len(ids))); err != nil { return err } - if !confirmed { - return errAborted - } resp, err := c.BulkDeleteTemplates(ids) if err != nil { diff --git a/cli/cmd/chart_changes.go b/cli/cmd/chart_changes.go index 743b34b..f5ea829 100644 --- a/cli/cmd/chart_changes.go +++ b/cli/cmd/chart_changes.go @@ -168,13 +168,9 @@ Examples: if err != nil { return err } - ok, err := confirmAction(cmd, fmt.Sprintf("Remove chart %s from definition %s? (y/n): ", args[1], args[0])) - if err != nil { + if err := confirmAction(cmd, fmt.Sprintf("Remove chart %s from definition %s? (y/n): ", args[1], args[0])); err != nil { return err } - if !ok { - return errAborted - } if err := c.DeleteDefinitionChart(defID, chartID); err != nil { return err } @@ -364,14 +360,10 @@ Examples: if err := printUpgradeCheckTo(cmd, c, defID, check); err != nil { return err } - ok, err := confirmAction(cmd, fmt.Sprintf("Upgrade definition %s to version %s? The default values of the template charts replace the values of the definition. (y/n): ", - args[0], client.SanitizeServerText(check.LatestVersion))) - if err != nil { + if err := confirmAction(cmd, fmt.Sprintf("Upgrade definition %s to version %s? The default values of the template charts replace the values of the definition. (y/n): ", + args[0], client.SanitizeServerText(check.LatestVersion))); err != nil { return err } - if !ok { - return errAborted - } } def, err := c.UpgradeDefinition(defID) @@ -557,13 +549,9 @@ Examples: if err != nil { return err } - ok, err := confirmAction(cmd, fmt.Sprintf("Remove chart %s from template %s? (y/n): ", args[1], args[0])) - if err != nil { + if err := confirmAction(cmd, fmt.Sprintf("Remove chart %s from template %s? (y/n): ", args[1], args[0])); err != nil { return err } - if !ok { - return errAborted - } if err := c.DeleteTemplateChart(templateID, chartID); err != nil { return err } diff --git a/cli/cmd/cluster.go b/cli/cmd/cluster.go index d3c014a..dc9f716 100644 --- a/cli/cmd/cluster.go +++ b/cli/cmd/cluster.go @@ -222,10 +222,13 @@ Examples: return nil } - if len(svList) == 0 { + if len(svList) == 0 && !printer.MachineReadable() { printer.PrintMessage("No shared values found for cluster %s", id) return nil } + if svList == nil { + svList = []types.SharedValues{} // -o json: [], not null + } switch printer.Format { case output.FormatJSON: @@ -508,13 +511,9 @@ Examples: return nil } - confirmed, err := confirmAction(cmd, fmt.Sprintf("This will delete shared values %s from cluster %s. Continue? (y/n): ", svID, clusterID)) - if err != nil { + if err := confirmAction(cmd, fmt.Sprintf("This will delete shared values %s from cluster %s. Continue? (y/n): ", svID, clusterID)); err != nil { return err } - if !confirmed { - return errAborted - } c, err := newClient() if err != nil { diff --git a/cli/cmd/completion.go b/cli/cmd/completion.go index 2b542b8..5192b9a 100644 --- a/cli/cmd/completion.go +++ b/cli/cmd/completion.go @@ -24,6 +24,7 @@ Fish: PowerShell: stackctl completion powershell | Out-String | Invoke-Expression`, + Annotations: map[string]string{skipConfigAnnotation: "true"}, Args: cobra.ExactArgs(1), ValidArgs: []string{"bash", "zsh", "fish", "powershell"}, SilenceUsage: true, diff --git a/cli/cmd/config.go b/cli/cmd/config.go index ce8ac2a..2ac5520 100644 --- a/cli/cmd/config.go +++ b/cli/cmd/config.go @@ -93,7 +93,7 @@ Examples: stackctl config list -o json`, SilenceUsage: true, RunE: func(cmd *cobra.Command, args []string) error { - if len(cfg.Contexts) == 0 { + if len(cfg.Contexts) == 0 && !printer.MachineReadable() { printer.PrintMessage("No contexts configured. Run 'stackctl config use-context ' to create one.") return nil } @@ -112,7 +112,7 @@ Examples: APIKey string `json:"api-key,omitempty" yaml:"api-key,omitempty"` Insecure bool `json:"insecure,omitempty" yaml:"insecure,omitempty"` } - var structured []contextOutput + structured := []contextOutput{} // -o json: [], not null headers := []string{"", "CONTEXT", "API URL", "API KEY", "INSECURE"} var rows [][]string for _, name := range names { diff --git a/cli/cmd/notification_channel.go b/cli/cmd/notification_channel.go index 07cc16c..47a8c94 100644 --- a/cli/cmd/notification_channel.go +++ b/cli/cmd/notification_channel.go @@ -558,13 +558,9 @@ Examples: if err != nil { return err } - ok, err := confirmAction(cmd, fmt.Sprintf("Delete notification channel %s? (y/n): ", args[0])) - if err != nil { + if err := confirmAction(cmd, fmt.Sprintf("Delete notification channel %s? (y/n): ", args[0])); err != nil { return err } - if !ok { - return errAborted - } if err := c.DeleteNotificationChannel(id); err != nil { return err } diff --git a/cli/cmd/orphaned.go b/cli/cmd/orphaned.go index bf2f30f..b4803a4 100644 --- a/cli/cmd/orphaned.go +++ b/cli/cmd/orphaned.go @@ -132,13 +132,9 @@ Examples: return nil } - confirmed, err := confirmAction(cmd, fmt.Sprintf("This will delete orphaned namespace %q. Continue? (y/n): ", namespace)) - if err != nil { + if err := confirmAction(cmd, fmt.Sprintf("This will delete orphaned namespace %q. Continue? (y/n): ", namespace)); err != nil { return err } - if !confirmed { - return errAborted - } c, err := newClient() if err != nil { diff --git a/cli/cmd/override.go b/cli/cmd/override.go index 93a20ed..f168893 100644 --- a/cli/cmd/override.go +++ b/cli/cmd/override.go @@ -249,13 +249,9 @@ Examples: } if len(values) == 0 && existing != nil { - confirmed, err := confirmAction(cmd, fmt.Sprintf("The new values are empty. This will remove the value override of chart %s on instance %s. Continue? (y/n): ", args[1], instanceID)) - if err != nil { + if err := confirmAction(cmd, fmt.Sprintf("The new values are empty. This will remove the value override of chart %s on instance %s. Continue? (y/n): ", args[1], instanceID)); err != nil { return err } - if !confirmed { - return errAborted - } } return writeValueOverride(c, instanceID, chartID, args[1], existing, current, values) @@ -953,13 +949,9 @@ Examples: return err } - confirmed, err := confirmAction(cmd, fmt.Sprintf("This will delete the quota override for instance %s. Continue? (y/n): ", instanceID)) - if err != nil { + if err := confirmAction(cmd, fmt.Sprintf("This will delete the quota override for instance %s. Continue? (y/n): ", instanceID)); err != nil { return err } - if !confirmed { - return errAborted - } if err := c.DeleteQuotaOverride(instanceID); err != nil { return err @@ -994,13 +986,9 @@ func deleteChartOverride(cmd *cobra.Command, args []string, kind string, deleteF return err } - confirmed, err := confirmAction(cmd, fmt.Sprintf("This will delete the %s override for chart %s on instance %s. Continue? (y/n): ", kind, args[1], instanceID)) - if err != nil { + if err := confirmAction(cmd, fmt.Sprintf("This will delete the %s override for chart %s on instance %s. Continue? (y/n): ", kind, args[1], instanceID)); err != nil { return err } - if !confirmed { - return errAborted - } if err := deleteFn(c, instanceID, chartID); err != nil { return overrideError(err, kind, args[1], args[0]) diff --git a/cli/cmd/root.go b/cli/cmd/root.go index 57c57b7..c966ad9 100644 --- a/cli/cmd/root.go +++ b/cli/cmd/root.go @@ -54,10 +54,10 @@ AI agents and scripts: run 'stackctl agent-guide' first.`, // (https://no-color.org) or a stdout that is a pipe or a file. printer = output.NewPrinter(flagOutput, flagQuiet, flagNoColor || noColorEnv() || !isTerminalWriter(cmd.OutOrStdout())) printer.Writer = cmd.OutOrStdout() + printer.ErrWriter = cmd.ErrOrStderr() // Skip config loading for commands that should work without a config file - name := cmd.Name() - if name == "version" || name == "agent-guide" || name == "completion" || (cmd.Parent() != nil && cmd.Parent().Name() == "completion") { + if skipsConfig(cmd) { cfg = &config.Config{Contexts: map[string]*config.Context{}} return nil } @@ -278,16 +278,33 @@ func isTerminalWriter(w io.Writer) bool { return ok && term.IsTerminal(int(f.Fd())) } -// confirmAction prompts the user for confirmation unless --yes is set. -// Returns true if the action should proceed, false if aborted. +// skipConfigAnnotation marks a Cobra command that works without a config +// file (version, agent-guide, completion). It also applies to the +// subcommands of a marked command. +const skipConfigAnnotation = "stackctl.skip-config" + +// skipsConfig reports whether cmd or one of its parents has +// skipConfigAnnotation. +func skipsConfig(cmd *cobra.Command) bool { + for c := cmd; c != nil; c = c.Parent() { + if c.Annotations[skipConfigAnnotation] == "true" { + return true + } + } + return false +} + // errAborted is returned when the user declines a confirmation prompt: the // command exits with code 1, so a script can tell that nothing changed. var errAborted = errors.New("aborted: not confirmed, nothing changed") -func confirmAction(cmd *cobra.Command, message string) (bool, error) { +// confirmAction prompts the user for confirmation unless --yes is set. +// It returns nil if the action should proceed and errAborted if the user +// declined, so callers only pass the error on. +func confirmAction(cmd *cobra.Command, message string) error { yes, _ := cmd.Flags().GetBool("yes") if yes { - return true, nil + return nil } fmt.Fprint(cmd.ErrOrStderr(), message) reader := bufio.NewReader(cmd.InOrStdin()) @@ -295,14 +312,14 @@ func confirmAction(cmd *cobra.Command, message string) (bool, error) { if err != nil && (err != io.EOF || answer == "") { if err == io.EOF { // stdin closed without an answer: a non-interactive shell. - return false, fmt.Errorf("no answer to the confirmation prompt (non-interactive shell?): use --yes to confirm") + return fmt.Errorf("no answer to the confirmation prompt (non-interactive shell?): use --yes to confirm") } - return false, fmt.Errorf("reading confirmation: %w (use --yes to skip the prompt)", err) + return fmt.Errorf("reading confirmation: %w (use --yes to skip the prompt)", err) } if strings.TrimSpace(strings.ToLower(answer)) != "y" { - return false, nil + return errAborted } - return true, nil + return nil } func deleteByID(cmd *cobra.Command, args []string, promptFmt string, resolveFn func(*client.Client, string) (string, error), deleteFn func(*client.Client, string) error, successFmt string) error { @@ -320,13 +337,9 @@ func deleteByID(cmd *cobra.Command, args []string, promptFmt string, resolveFn f return err } - confirmed, err := confirmAction(cmd, fmt.Sprintf(promptFmt, id)) - if err != nil { + if err := confirmAction(cmd, fmt.Sprintf(promptFmt, id)); err != nil { return err } - if !confirmed { - return errAborted - } if err := deleteFn(c, id); err != nil { return err diff --git a/cli/cmd/root_test.go b/cli/cmd/root_test.go index 476acb0..0b15ebd 100644 --- a/cli/cmd/root_test.go +++ b/cli/cmd/root_test.go @@ -6,6 +6,7 @@ import ( "path/filepath" "testing" + "github.com/spf13/cobra" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -98,3 +99,12 @@ func TestInsecureWarningFromConfig(t *testing.T) { assert.NoError(t, err) assert.Contains(t, stderr.String(), "WARNING: TLS certificate verification is disabled") } + +func TestSkipsConfig(t *testing.T) { + for _, c := range []*cobra.Command{versionCmd, agentGuideCmd, completionCmd} { + assert.True(t, skipsConfig(c), c.Name()) + } + for _, c := range []*cobra.Command{rootCmd, stackCmd, stackGetCmd, configListCmd} { + assert.False(t, skipsConfig(c), c.Name()) + } +} diff --git a/cli/cmd/stack.go b/cli/cmd/stack.go index c8a8990..d837d21 100644 --- a/cli/cmd/stack.go +++ b/cli/cmd/stack.go @@ -36,12 +36,73 @@ const ( // (error, partial). // // Installs an os.Interrupt signal handler for Ctrl-C; writes log lines to -// os.Stdout and warnings to os.Stderr. Test code should call followLogsCtx -// directly to inject a context + writers. -func followLogs(c *client.Client, instanceID, op string) error { +// out and warnings to warn. Test code should call followLogsCtx directly to +// inject a context. +func followLogs(c *client.Client, instanceID, op string, out, warn io.Writer) error { ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt) defer stop() - return followLogsCtx(ctx, c, instanceID, op, os.Stdout, os.Stderr) + return followLogsCtx(ctx, c, instanceID, op, out, warn) +} + +// followLogWriter returns the writer for the log lines of --follow: stdout +// in the table format; stderr in machine-readable modes (-o json|yaml, -q), +// where stdout holds only data. +func followLogWriter(cmd *cobra.Command) io.Writer { + if printer.MachineReadable() { + return cmd.ErrOrStderr() + } + return cmd.OutOrStdout() +} + +// printOperation ends deploy, stop, clean and rollback once the server has +// accepted the operation (HTTP 202). With --follow it waits for the final +// status (followOperation). Else -q prints the log ID, -o json|yaml prints +// resp and the table format prints message. +func printOperation(cmd *cobra.Command, c *client.Client, id, op, logID string, resp any, message string) error { + if follow, _ := cmd.Flags().GetBool("follow"); follow { + return followOperation(cmd, c, id, op, logID) + } + if printer.Quiet { + fmt.Fprintln(printer.Writer, logID) + return nil + } + switch printer.Format { + case output.FormatJSON: + return printer.PrintJSON(resp) + case output.FormatYAML: + return printer.PrintYAML(resp) + default: + printer.PrintMessage("%s", message) + return nil + } +} + +// followOperation follows an operation to its final status. The table +// format streams the log to stdout. In machine-readable modes the log goes +// to stderr, so stdout holds only data: -q prints the log ID, other formats +// print the instance as read after the operation (also after a failed +// one; the exit code shows the result). +func followOperation(cmd *cobra.Command, c *client.Client, id, op, logID string) error { + followErr := followLogs(c, id, op, followLogWriter(cmd), cmd.ErrOrStderr()) + if !printer.MachineReadable() { + return followErr + } + if printer.Quiet { + fmt.Fprintln(printer.Writer, logID) + return followErr + } + inst, err := c.GetStack(id) + if err != nil { + if followErr != nil { + fmt.Fprintf(cmd.ErrOrStderr(), "Warning: could not read stack %s: %v\n", id, err) + return followErr + } + return fmt.Errorf("reading stack %s after the %s: %w", id, op, err) + } + if err := printInstance(inst); err != nil { + return err + } + return followErr } // followLogsCtx is the testable core of followLogs. It does NOT install a @@ -263,8 +324,9 @@ const flagPageSize = "page-size" // part of the help of every command with --follow that waits for a status. const followHelp = `With --follow, the exit code shows the result of the operation: - running, stopped or draft: exit code 0. - - error: exit code 1. The operation failed. + - error or failed: exit code 1. The operation failed. - partial: exit code 1. Some charts deployed and others failed. + - Ctrl-C (SIGINT): exit code 0; the operation continues on the server. The command reads the instance status after it connects, so an operation that ends before the connection is complete counts. When the access token expires during a long operation (server close @@ -272,13 +334,17 @@ When the access token expires during a long operation (server close the instance status, so a status change during the reconnect is not lost. The command stops with exit code 1 when it cannot follow the operation to the end: the server ends the session ("session revoked"), the login has no -refresh token (SSO login), the renewal fails, or the connection is lost. -The operation continues on the server. Do not start it again. Run +refresh token (SSO login), the renewal fails, the connection is lost, or +the server closes the connection normally (for example on a restart) while +the status is not final. The operation continues on the server. Do not start it again. Run 'stackctl stack watch --id ' or 'stackctl stack status ' to see the result (after 'stackctl login' if necessary). --follow works with an API key from k8s-stack-manager v0.8.0. An older server refuses an API key for live updates: use a login session, or run -the command without --follow and use 'stackctl stack status '.` +the command without --follow and use 'stackctl stack status '. +Without --follow, -o json|yaml prints the response of the server (log_id, +message) and -q the log ID. With --follow in these modes the log goes to +stderr; stdout gets the instance after the operation (-q: the log ID).` var stackCmd = &cobra.Command{ Use: "stack", @@ -629,18 +695,8 @@ Examples: return err } - follow, _ := cmd.Flags().GetBool("follow") - if follow { - return followLogs(c, id, followOpDeploy) - } - - if printer.Quiet { - fmt.Fprintln(printer.Writer, resp.LogID) - return nil - } - - printer.PrintMessage("Deploying stack %s... (log ID: %s)", id, resp.LogID) - return nil + return printOperation(cmd, c, id, followOpDeploy, resp.LogID, resp, + fmt.Sprintf("Deploying stack %s... (log ID: %s)", id, resp.LogID)) }, } @@ -678,18 +734,8 @@ Examples: return err } - follow, _ := cmd.Flags().GetBool("follow") - if follow { - return followLogs(c, id, followOpStop) - } - - if printer.Quiet { - fmt.Fprintln(printer.Writer, resp.LogID) - return nil - } - - printer.PrintMessage("Stopping stack %s... (log ID: %s)", id, resp.LogID) - return nil + return printOperation(cmd, c, id, followOpStop, resp.LogID, resp, + fmt.Sprintf("Stopping stack %s... (log ID: %s)", id, resp.LogID)) }, } @@ -724,31 +770,17 @@ Examples: return err } - confirmed, err := confirmAction(cmd, fmt.Sprintf("This will undeploy and remove the namespace for stack %s. Continue? (y/n): ", id)) - if err != nil { + if err := confirmAction(cmd, fmt.Sprintf("This will undeploy and remove the namespace for stack %s. Continue? (y/n): ", id)); err != nil { return err } - if !confirmed { - return errAborted - } resp, err := c.CleanStack(id) if err != nil { return err } - follow, _ := cmd.Flags().GetBool("follow") - if follow { - return followLogs(c, id, followOpClean) - } - - if printer.Quiet { - fmt.Fprintln(printer.Writer, resp.LogID) - return nil - } - - printer.PrintMessage("Cleaning stack %s... (log ID: %s)", id, resp.LogID) - return nil + return printOperation(cmd, c, id, followOpClean, resp.LogID, resp, + fmt.Sprintf("Cleaning stack %s... (log ID: %s)", id, resp.LogID)) }, } @@ -952,7 +984,12 @@ Examples: follow, _ := cmd.Flags().GetBool("follow") if follow { - return followLogs(c, id, followOpLogs) + followErr := followLogs(c, id, followOpLogs, followLogWriter(cmd), cmd.ErrOrStderr()) + if followErr != nil || !printer.MachineReadable() { + return followErr + } + // -o json|yaml and -q: the stream went to stderr; stdout + // gets the log as without --follow. } log, err := c.GetStackLogs(id) @@ -1670,13 +1707,9 @@ Examples: return err } - confirmed, err := confirmAction(cmd, fmt.Sprintf("This will rollback stack %s. Continue? (y/n): ", id)) - if err != nil { + if err := confirmAction(cmd, fmt.Sprintf("This will rollback stack %s. Continue? (y/n): ", id)); err != nil { return err } - if !confirmed { - return errAborted - } targetLog, _ := cmd.Flags().GetString("target-log") req := &types.RollbackRequest{TargetLogID: targetLog} @@ -1691,25 +1724,8 @@ Examples: } printServerWarning(cmd, warning) - follow, _ := cmd.Flags().GetBool("follow") - if follow { - return followLogs(c, id, followOpRollback) - } - - if printer.Quiet { - fmt.Fprintln(printer.Writer, resp.LogID) - return nil - } - - switch printer.Format { - case output.FormatJSON: - return printer.PrintJSON(resp) - case output.FormatYAML: - return printer.PrintYAML(resp) - default: - printer.PrintMessage("Rollback started for stack %s (log ID: %s)", id, resp.LogID) - return nil - } + return printOperation(cmd, c, id, followOpRollback, resp.LogID, resp, + fmt.Sprintf("Rollback started for stack %s (log ID: %s)", id, resp.LogID)) }, } diff --git a/cli/cmd/stack_action.go b/cli/cmd/stack_action.go index 159094f..2ab0225 100644 --- a/cli/cmd/stack_action.go +++ b/cli/cmd/stack_action.go @@ -204,13 +204,9 @@ Examples: } if action.Confirm != "" { - ok, err := confirmAction(cmd, client.SanitizeServerText(action.Confirm)+" (y/n): ") - if err != nil { + if err := confirmAction(cmd, client.SanitizeServerText(action.Confirm)+" (y/n): "); err != nil { return err } - if !ok { - return errAborted - } } res, err := c.RunStackAction(id, action.Name, params) diff --git a/cli/cmd/stack_operation_output_test.go b/cli/cmd/stack_operation_output_test.go new file mode 100644 index 0000000..b387d5c --- /dev/null +++ b/cli/cmd/stack_operation_output_test.go @@ -0,0 +1,252 @@ +package cmd + +import ( + "bytes" + "encoding/json" + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" + "time" + + "github.com/gorilla/websocket" + "github.com/omattsson/stackctl/cli/pkg/output" + "github.com/omattsson/stackctl/cli/pkg/types" + "github.com/spf13/cobra" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "gopkg.in/yaml.v3" +) + +// Tests in this file mutate package globals (printer, cfg) and command +// flags, so they do not run in parallel. + +// setOutputMode sets the printer of setupStackTestCmd to format and quiet +// and returns the buffer that gets the messages (stderr). +func setOutputMode(t *testing.T, stdout *bytes.Buffer, format string, quiet bool) *bytes.Buffer { + t.Helper() + var stderr bytes.Buffer + printer = output.NewPrinter(format, quiet, true) + printer.Writer = stdout + printer.ErrWriter = &stderr + return &stderr +} + +func setFlag(t *testing.T, cmd *cobra.Command, name, value string) { + t.Helper() + prev := cmd.Flags().Lookup(name).Value.String() + require.NoError(t, cmd.Flags().Set(name, value)) + t.Cleanup(func() { _ = cmd.Flags().Set(name, prev) }) +} + +// TestStackOperationCmds_OutputModes: deploy, stop, clean and rollback +// print a message in the table format, the 202 response with -o json|yaml +// and the log ID with -q. stdout never mixes text into JSON or IDs. +func TestStackOperationCmds_OutputModes(t *testing.T) { + cmds := []struct { + cmd *cobra.Command + op string + want string + }{ + {stackDeployCmd, "deploy", "Deploying stack 42... (log ID: log-1)\n"}, + {stackStopCmd, "stop", "Stopping stack 42... (log ID: log-1)\n"}, + {stackCleanCmd, "clean", "Cleaning stack 42... (log ID: log-1)\n"}, + {stackRollbackCmd, "rollback", "Rollback started for stack 42 (log ID: log-1)\n"}, + } + for _, c := range cmds { + for _, mode := range []string{"table", "json", "yaml", "quiet"} { + t.Run(c.op+"/"+mode, func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // assert, not require: this runs outside the test goroutine. + if !assert.Equal(t, "/api/v1/stack-instances/42/"+c.op, r.URL.Path) { + w.WriteHeader(http.StatusNotFound) + return + } + w.WriteHeader(http.StatusAccepted) + _ = json.NewEncoder(w).Encode(types.DeployResponse{LogID: "log-1", Message: "started"}) + })) + defer server.Close() + buf := setupStackTestCmd(t, server.URL) + format := mode + if mode == "quiet" { + format = "table" + } + stderr := setOutputMode(t, buf, format, mode == "quiet") + if c.cmd.Flags().Lookup("yes") != nil { + setFlag(t, c.cmd, "yes", "true") + } + + require.NoError(t, c.cmd.RunE(c.cmd, []string{"42"})) + + var got types.DeployResponse + switch mode { + case "table": + assert.Equal(t, c.want, buf.String()) + case "json": + require.NoError(t, json.Unmarshal(buf.Bytes(), &got)) + assert.Equal(t, types.DeployResponse{LogID: "log-1", Message: "started"}, got) + case "yaml": + require.NoError(t, yaml.Unmarshal(buf.Bytes(), &got)) + assert.Equal(t, types.DeployResponse{LogID: "log-1", Message: "started"}, got) + case "quiet": + assert.Equal(t, "log-1\n", buf.String()) + } + assert.Empty(t, stderr.String()) + }) + } + } +} + +// TestStackDryRun_MachineOutputOnStderr: the --dry-run message is for +// people, so with -o json and -q it goes to stderr and stdout stays empty. +func TestStackDryRun_MachineOutputOnStderr(t *testing.T) { + for _, mode := range []string{"json", "quiet"} { + t.Run(mode, func(t *testing.T) { + var calls atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { + calls.Add(1) + })) + defer server.Close() + buf := setupStackTestCmd(t, server.URL) + format := map[string]string{"json": "json", "quiet": "table"}[mode] + stderr := setOutputMode(t, buf, format, mode == "quiet") + setFlag(t, stackDeleteCmd, "dry-run", "true") + + require.NoError(t, stackDeleteCmd.RunE(stackDeleteCmd, []string{"my-app"})) + assert.Empty(t, buf.String()) + assert.Equal(t, "Would delete my-app\n", stderr.String()) + assert.Zero(t, calls.Load(), "dry-run sends no request") + }) + } +} + +// TestStackDeployCmd_FollowMachineOutput: with --follow and -o json the log +// goes to stderr and stdout gets the instance after the deploy, also after +// a failed one; with -q stdout gets the log ID. +func TestStackDeployCmd_FollowMachineOutput(t *testing.T) { + tests := []struct { + name string + format string + quiet bool + final string + wantErr string + }{ + {"json running", "json", false, "running", ""}, + {"json error", "json", false, "error", "deploy failed (status error)"}, + {"yaml running", "yaml", false, "running", ""}, + {"quiet running", "table", true, "running", ""}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var getCalls atomic.Int32 + server, _ := watchCloseServer(t, func(_ int32, _ *http.Request, conn *websocket.Conn) { + readSubscribe(conn) + payload, _ := json.Marshal(types.WSDeploymentLog{InstanceID: "42", Line: "helm upgrade api"}) + msg, _ := json.Marshal(types.WSMessage{Type: "deployment.log", Data: payload}) + _ = conn.WriteMessage(websocket.TextMessage, msg) + writeWatchStatus(t, conn, "42", tt.final) + _, _, _ = conn.ReadMessage() + }, map[string]http.HandlerFunc{ + "/api/v1/stack-instances/42/deploy": func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusAccepted) + _ = json.NewEncoder(w).Encode(types.DeployResponse{LogID: "log-1"}) + }, + "/api/v1/stack-instances/42": instanceRoute(&getCalls, "deploying", tt.final), + }) + defer server.Close() + buf := setupStackTestCmd(t, server.URL) + _ = setOutputMode(t, buf, tt.format, tt.quiet) + var cmdErr bytes.Buffer + stackDeployCmd.SetErr(&cmdErr) + t.Cleanup(func() { stackDeployCmd.SetErr(nil) }) + setFlag(t, stackDeployCmd, "follow", "true") + + done := make(chan error, 1) + go func() { done <- stackDeployCmd.RunE(stackDeployCmd, []string{"42"}) }() + var err error + select { + case err = <-done: + case <-time.After(5 * time.Second): + t.Fatal("stack deploy -f did not exit") + } + + if tt.wantErr == "" { + require.NoError(t, err) + } else { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + } + assert.Contains(t, cmdErr.String(), "helm upgrade api", "the log goes to stderr") + assert.NotContains(t, buf.String(), "helm upgrade api") + + var inst types.StackInstance + switch { + case tt.quiet: + assert.Equal(t, "log-1\n", buf.String()) + case tt.format == "json": + require.NoError(t, json.Unmarshal(buf.Bytes(), &inst)) + assert.Equal(t, tt.final, inst.Status) + default: + require.NoError(t, yaml.Unmarshal(buf.Bytes(), &inst)) + assert.Equal(t, tt.final, inst.Status) + } + }) + } +} + +// TestStackLogsCmd_FollowMachineOutput: `stack logs -f -o json` streams the +// log to stderr and prints the log as JSON on stdout, as without --follow. +func TestStackLogsCmd_FollowMachineOutput(t *testing.T) { + server, _ := watchCloseServer(t, func(_ int32, _ *http.Request, conn *websocket.Conn) { + readSubscribe(conn) + payload, _ := json.Marshal(types.WSDeploymentLog{InstanceID: "42", Line: "helm upgrade api"}) + msg, _ := json.Marshal(types.WSMessage{Type: "deployment.log", Data: payload}) + _ = conn.WriteMessage(websocket.TextMessage, msg) + writeWatchStatus(t, conn, "42", "running") + _, _, _ = conn.ReadMessage() + }, map[string]http.HandlerFunc{ + "/api/v1/stack-instances/42/deploy-log": func(w http.ResponseWriter, _ *http.Request) { + _ = json.NewEncoder(w).Encode(types.DeploymentLogResult{Data: []types.DeploymentLog{{ID: "log-1", Status: "success"}}}) + }, + }) + defer server.Close() + buf := setupStackTestCmd(t, server.URL) + _ = setOutputMode(t, buf, "json", false) + var cmdErr bytes.Buffer + stackLogsCmd.SetErr(&cmdErr) + t.Cleanup(func() { stackLogsCmd.SetErr(nil) }) + setFlag(t, stackLogsCmd, "follow", "true") + + require.NoError(t, stackLogsCmd.RunE(stackLogsCmd, []string{"42"})) + + assert.Contains(t, cmdErr.String(), "helm upgrade api") + var got types.DeploymentLog + require.NoError(t, json.Unmarshal(buf.Bytes(), &got), buf.String()) + assert.Equal(t, "log-1", got.ID) +} + +// TestEmptyLists_MachineOutputIsEmptyArray: an empty list prints [] with +// -o json (not a message, not null), so `jq length` gives 0. +func TestEmptyLists_MachineOutputIsEmptyArray(t *testing.T) { + t.Run("cluster shared-values list", func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte("[]")) + })) + defer server.Close() + buf := setupStackTestCmd(t, server.URL) + stderr := setOutputMode(t, buf, "json", false) + + require.NoError(t, clusterSharedValuesListCmd.RunE(clusterSharedValuesListCmd, []string{"c1"})) + assert.JSONEq(t, "[]", buf.String()) + assert.Empty(t, stderr.String()) + }) + t.Run("config list", func(t *testing.T) { + buf := setupStackTestCmd(t, "http://unused") + cfg.Contexts = nil + stderr := setOutputMode(t, buf, "json", false) + + require.NoError(t, configListCmd.RunE(configListCmd, nil)) + assert.JSONEq(t, "[]", buf.String()) + assert.Empty(t, stderr.String()) + }) +} diff --git a/cli/cmd/version.go b/cli/cmd/version.go index 378f02f..96c1003 100644 --- a/cli/cmd/version.go +++ b/cli/cmd/version.go @@ -33,6 +33,7 @@ that stackctl is installed correctly. Examples: stackctl version stackctl version -o json`, + Annotations: map[string]string{skipConfigAnnotation: "true"}, SilenceUsage: true, RunE: func(cmd *cobra.Command, args []string) error { versionInfo := map[string]string{ diff --git a/cli/pkg/output/output.go b/cli/pkg/output/output.go index ef52c0c..eceb4a8 100644 --- a/cli/pkg/output/output.go +++ b/cli/pkg/output/output.go @@ -114,6 +114,9 @@ type Printer struct { Quiet bool NoColor bool Writer io.Writer + // ErrWriter gets the messages of PrintMessage in machine-readable + // modes (os.Stderr when nil). + ErrWriter io.Writer } // NewPrinter creates a new Printer with the given format. @@ -276,8 +279,26 @@ type KeyValue struct { } // PrintMessage prints a plain text message (used for confirmations, status updates). +// +// In machine-readable modes (MachineReadable) it writes to ErrWriter, so +// stdout holds only data: `-o json` output stays valid JSON and `-q` output +// stays a list of IDs. Else it writes to Writer. func (p *Printer) PrintMessage(format string, args ...interface{}) { - fmt.Fprintf(p.Writer, format+"\n", args...) + w := p.Writer + if p.MachineReadable() { + w = p.ErrWriter + if w == nil { + w = os.Stderr + } + } + fmt.Fprintf(w, format+"\n", args...) +} + +// MachineReadable reports whether stdout is for programs: -o json, -o yaml +// or -q. The table format and custom formats are not: a custom format can +// print messages as its data. +func (p *Printer) MachineReadable() bool { + return p.Quiet || p.Format == FormatJSON || p.Format == FormatYAML } // PrintError prints an error message to stderr. diff --git a/cli/pkg/output/output_test.go b/cli/pkg/output/output_test.go index c0f1146..75f98a5 100644 --- a/cli/pkg/output/output_test.go +++ b/cli/pkg/output/output_test.go @@ -274,6 +274,45 @@ func TestPrintMessage(t *testing.T) { assert.Equal(t, "Hello world, count: 42\n", buf.String()) } +// TestPrintMessage_MachineModesUseErrWriter: messages for people go to +// ErrWriter with JSON, YAML or quiet mode, so stdout holds only data. The +// table format and custom formats keep them on Writer. +func TestPrintMessage_MachineModesUseErrWriter(t *testing.T) { + t.Parallel() + tests := []struct { + name string + format Format + quiet bool + wantOut bool + }{ + {"zero printer", "", false, true}, + {"table", FormatTable, false, true}, + {"table quiet", FormatTable, true, false}, + {"json", FormatJSON, false, false}, + {"yaml", FormatYAML, false, false}, + {"custom", Format("csv"), false, true}, + {"custom quiet", Format("csv"), true, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + var out, errOut bytes.Buffer + p := &Printer{Format: tt.format, Quiet: tt.quiet, Writer: &out, ErrWriter: &errOut} + + p.PrintMessage("Deleted %s", "x") + assert.Equal(t, !tt.wantOut, p.MachineReadable()) + + if tt.wantOut { + assert.Equal(t, "Deleted x\n", out.String()) + assert.Empty(t, errOut.String()) + } else { + assert.Empty(t, out.String()) + assert.Equal(t, "Deleted x\n", errOut.String()) + } + }) + } +} + // --- Resource-specific formatter tests --- // Helper to create a realistic StackInstance for tests.