diff --git a/EXTENDING.md b/EXTENDING.md new file mode 100644 index 0000000..b3e8b6f --- /dev/null +++ b/EXTENDING.md @@ -0,0 +1,440 @@ +# Extending stackctl + +`stackctl` is built so your team can add its own subcommands without forking. Drop an executable named `stackctl-` on your `$PATH` and it becomes `stackctl ` automatically — same pattern as `git`, `kubectl`, and `gh`. + +Because the mechanism is "any executable with the right name", plugins can be written in **any language** (shell, Python, Go, Node, Rust, …) and distributed however you already ship binaries to your team (package manager, tarball, Docker, private registry, rsync). + +--- + +## The 5-minute tutorial — your first plugin + +### 1. Write the plugin + +```bash +#!/usr/bin/env bash +# stackctl-hello — minimal stackctl plugin +set -euo pipefail + +# Report a redacted marker for the API key; NEVER echo its value. +if [ -n "${STACKCTL_API_KEY:-}" ]; then + key_status='***configured***' +else + key_status='' +fi + +cat <} + API key: ${key_status} + args: $* +MSG +``` + +Save it anywhere on your `PATH`: + +```bash +install -m 0755 stackctl-hello ~/.local/bin/ +``` + +### 2. Use it + +```bash +stackctl hello world +# → Hello from a stackctl plugin! +# API URL: http://localhost:8081 +# API key: ***configured*** +# args: world +``` + +```bash +stackctl --help | grep hello +# → hello Plugin: hello +``` + +The plugin's absolute path is kept out of `--help` (it was leaking `$HOME` in screenshots). Use `stackctl help hello` to see the full resolved path. + +That's the whole mechanism. Your binary gets exec'd with: + +- Whatever argv the user typed after `stackctl ` (plus flags stackctl didn't consume at the top level) +- `stdin`, `stdout`, `stderr` wired through directly +- The full parent environment (so `STACKCTL_API_URL` / `STACKCTL_API_KEY` are visible) +- The plugin's exit code becomes stackctl's exit code + +--- + +## What plugins are for + +Any team-specific or company-specific workflow that doesn't belong in core stackctl. Core stackctl knows how to speak the k8s-stack-manager API — nothing beyond that. If your workflow involves **your** infrastructure, your service catalog, your CMDB, your pager, your Slack — it belongs in a plugin. + +**Good plugin candidates:** + +- **Thin wrappers around [k8s-stack-manager action webhooks](https://github.com/omattsson/k8s-stack-manager/blob/main/EXTENDING.md#action-webhooks).** The action endpoint (`POST /api/v1/stack-instances/:id/actions/:name`) is a k8s-stack-manager feature; stackctl itself has no action logic. A plugin like `stackctl refresh-db ` simply POSTs to that server endpoint and pretty-prints the response. Pair your plugin with the webhook handler you registered server-side. +- **Operations that combine multiple stackctl calls** into a single higher-level command (e.g. "create + deploy + wait for healthy + run smoke test"). +- **Company-specific reporting.** `stackctl stacks-costing-money` that lists instances + pulls cost data from your FinOps API. +- **Developer conveniences.** `stackctl port-forward ` that resolves the stack namespace and wires up a `kubectl port-forward` for common services. +- **Integration with out-of-band systems.** Jira, Linear, Datadog, PagerDuty — anything stackctl shouldn't know about directly. + +**Bad plugin candidates:** + +- Things core stackctl already does (shadowing is prevented — built-in subcommands always win). +- Infrastructure changes. That's k8s-stack-manager's job. +- Logic that should live on the server side — e.g. orchestration that needs cluster credentials. Put that in an **action webhook** on the k8s-stack-manager side and have the plugin call it. See [k8s-stack-manager EXTENDING.md](https://github.com/omattsson/k8s-stack-manager/blob/main/EXTENDING.md). + +--- + +## Naming convention + +Plugin names follow a strict rule: + +- Binary on PATH: `stackctl-` +- Appears as: `stackctl ` +- `` can contain letters, digits, and dashes — e.g. `stackctl-refresh-db`, `stackctl-port-forward`, `stackctl-sync-cmdb` +- **Built-in commands always win.** If you create `stackctl-config`, it'll be shadowed by the built-in `stackctl config`. Pick a name that doesn't collide. + +Discovery is **first-PATH-wins** (standard PATH semantics). If `stackctl-hello` exists in two PATH entries, the earlier one is used; the later one is silently ignored. + +## What plugins receive + +### Environment variables (inherited + flag-derived) + +The plugin inherits **stackctl's entire environment** (same pattern as `git`, +`kubectl`, `gh`). On top of that, stackctl exports a small set of values +**derived from flags** (not from `~/.stackmanager/config.yaml`) before exec, +so plugins can observe the user's requested TLS / output behaviour: + +| Variable | Source | Purpose | +|---|---|---| +| `STACKCTL_API_URL` | parent shell only | Base URL of the k8s-stack-manager API, **if already exported** by the user | +| `STACKCTL_API_KEY` | parent shell only | API key (header `X-API-Key`), **if already exported** by the user | +| `STACKCTL_INSECURE` | `--insecure` flag OR parent shell | `1` to skip TLS verification | +| `STACKCTL_QUIET` | `--quiet` flag | `1` when the user requested quiet output | +| `STACKCTL_OUTPUT` | `--output` flag | `table` / `json` / `yaml` / a registered custom format | +| `HOME`, `PATH`, `LANG`, `AWS_*`, `KUBECONFIG`, … | parent shell | the rest of the user's environment | + +> **stackctl does NOT inject values resolved from the stackctl config file.** +> If your plugin needs `STACKCTL_API_URL` / `STACKCTL_API_KEY`, they must +> already be present in the environment when stackctl is launched — they +> will not be filled in from `~/.stackmanager/config.yaml` automatically. +> See the Troubleshooting section below for recommended workflows. + +> **Security note:** because the full parent environment is forwarded, +> plugins have access to credentials stackctl doesn't know about +> (AWS_ACCESS_KEY_ID, GITHUB_TOKEN, KUBECONFIG contents, …). Install +> plugins from sources you trust. This matches the `git`/`kubectl` +> security model. + +### Arguments + +Whatever the user typed after `stackctl `. Use `stackctl help ` for stackctl's built-in help entry (which shows the plugin's resolved absolute path); `stackctl --help` is delegated to the plugin's own help handling. + +### Stdin/stdout/stderr + +Wired through directly. A plugin can prompt for confirmation, pipe into another command, or print JSON — whatever makes sense. + +### Exit code + +Propagated back. Non-zero exit from the plugin fails the outer `stackctl` invocation, matching every other Unix tool. + +--- + +## Recipe: invoking a custom action + +The common pattern. You have an action webhook registered on the k8s-stack-manager side (see [server-side extending](https://github.com/omattsson/k8s-stack-manager/blob/main/EXTENDING.md)); you want a stackctl plugin that invokes it. + +### Bash (no dependencies) + +```bash +#!/usr/bin/env bash +# stackctl-snapshot-pvc — invokes the snapshot-pvc action +set -euo pipefail + +INSTANCE_ID=${1:?usage: stackctl snapshot-pvc } + +: "${STACKCTL_API_URL:?STACKCTL_API_URL not set — export it first, e.g. export STACKCTL_API_URL=\$(stackctl config get api-url)}" + +# Optional: allow insecure TLS per env +CURL_OPTS=() +[ "${STACKCTL_INSECURE:-}" = "1" ] && CURL_OPTS+=(--insecure) + +curl -sS "${CURL_OPTS[@]}" \ + -X POST "${STACKCTL_API_URL%/}/api/v1/stack-instances/${INSTANCE_ID}/actions/snapshot-pvc" \ + -H "Content-Type: application/json" \ + -H "X-API-Key: ${STACKCTL_API_KEY:-}" \ + -d '{}' +echo +``` + +### Python (stdlib only) + +```python +#!/usr/bin/env python3 +# stackctl-snapshot-pvc — invokes the snapshot-pvc action +import json, os, ssl, sys +from urllib import request +from urllib.error import HTTPError, URLError + +if len(sys.argv) < 2: + sys.exit("usage: stackctl snapshot-pvc ") +instance_id = sys.argv[1] + +api = os.environ.get("STACKCTL_API_URL", "").rstrip("/") +if not api: + sys.exit("STACKCTL_API_URL not set") + +if os.environ.get("STACKCTL_INSECURE") == "1": + print("WARNING: TLS verification disabled (STACKCTL_INSECURE=1)", file=sys.stderr) + ctx = ssl.create_default_context() + ctx.check_hostname = False + ctx.verify_mode = ssl.CERT_NONE +else: + ctx = None + +req = request.Request( + f"{api}/api/v1/stack-instances/{instance_id}/actions/snapshot-pvc", + data=b"{}", + method="POST", + headers={ + "Content-Type": "application/json", + "X-API-Key": os.environ.get("STACKCTL_API_KEY", ""), + }, +) + +try: + with request.urlopen(req, timeout=30, context=ctx) as resp: + body = json.loads(resp.read()) +except HTTPError as e: # 4xx/5xx — read body for details + body = {"error": e.reason, "status": e.code, "body": e.read().decode(errors="replace")} + print(json.dumps(body, indent=2)); sys.exit(1) +except URLError as e: + sys.exit(f"request failed: {e.reason}") + +# Echo server response. If the user set STACKCTL_OUTPUT=json we keep the +# raw shape; for other values we could reshape into a friendlier table. +print(json.dumps(body, indent=2)) +``` + +### Go (compiled, single binary) + +```go +// stackctl-snapshot-pvc/main.go — compile: go build -o stackctl-snapshot-pvc +package main + +import ( + "bytes" + "crypto/tls" + "fmt" + "io" + "net/http" + "os" +) + +func main() { + if len(os.Args) < 2 { + fmt.Fprintln(os.Stderr, "usage: stackctl snapshot-pvc ") + os.Exit(2) + } + api := os.Getenv("STACKCTL_API_URL") + if api == "" { + fmt.Fprintln(os.Stderr, "STACKCTL_API_URL not set") + os.Exit(1) + } + url := fmt.Sprintf("%s/api/v1/stack-instances/%s/actions/snapshot-pvc", api, os.Args[1]) + req, _ := http.NewRequest("POST", url, bytes.NewReader([]byte("{}"))) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("X-API-Key", os.Getenv("STACKCTL_API_KEY")) + c := &http.Client{} + if os.Getenv("STACKCTL_INSECURE") == "1" { + c.Transport = &http.Transport{TLSClientConfig: &tls.Config{InsecureSkipVerify: true}} + } + resp, err := c.Do(req) + if err != nil { + fmt.Fprintln(os.Stderr, "request:", err) + os.Exit(1) + } + defer resp.Body.Close() + body, _ := io.ReadAll(resp.Body) + fmt.Print(string(body)) + if resp.StatusCode >= 400 { + os.Exit(1) + } +} +``` + +--- + +## Recipe: combining multiple stackctl calls + +For workflow plugins that orchestrate the built-in commands, just exec `stackctl` back into itself: + +```bash +#!/usr/bin/env bash +# stackctl-deploy-and-wait — create + deploy + wait until Running +set -euo pipefail + +DEF_ID=${1:?usage: stackctl deploy-and-wait } +NAME=${2:?usage: stackctl deploy-and-wait } + +# Create +INST_ID=$(stackctl stack create --definition "$DEF_ID" --name "$NAME" --quiet) +echo "Created $INST_ID" + +# Deploy +stackctl stack deploy "$INST_ID" --quiet + +# Poll until Running (or Error) +while :; do + STATUS=$(stackctl stack get "$INST_ID" -o json | jq -r .status) + case "$STATUS" in + running) echo "$INST_ID is running."; break ;; + error) echo "deploy failed: $(stackctl stack get "$INST_ID" -o json | jq -r .error_message)"; exit 1 ;; + *) echo "status=$STATUS, waiting..."; sleep 5 ;; + esac +done +``` + +`stackctl stack get -o json` makes this composable — the built-in commands are designed for scripting. + +--- + +## Best practices + +### Respect `--quiet` + +If the user sets `STACKCTL_QUIET=1` (or runs your plugin with `--quiet`), minimise output — print just the IDs or the raw result. Match core stackctl's behaviour. + +### Honour `-o json` + +When it makes sense, accept `-o json` and emit structured output. Downstream scripts can then `jq` into your plugin's result the same way they do with built-ins. + +### Don't re-implement auth + +Read `STACKCTL_API_URL` + `STACKCTL_API_KEY` from the environment. stackctl does **not** currently pass its config-file values into the plugin environment — only the flag-derived `STACKCTL_INSECURE`, `STACKCTL_QUIET`, and `STACKCTL_OUTPUT` flow through. So if the user configured their API URL via `stackctl config set api-url ` (writing `~/.stackmanager/config.yaml`) and never exported `STACKCTL_API_URL`, a plugin subprocess will not see it. + +Two workable strategies: + +1. **Require the env vars**, and fail fast with a pointer (`export STACKCTL_API_URL=... STACKCTL_API_KEY=...` — or wrap your plugin in a shell function that exports them). +2. **Shell out to `stackctl config get api-url`** and `stackctl config get api-key`. Works without any env wiring, at the cost of one extra exec per value. + +Either way, don't parse `~/.stackmanager/config.yaml` directly — the schema is internal and may change. + +### Use a deny list for dangerous defaults + +If your plugin mutates state (deletes, force-redeploys, wipes data), make it interactive or require `--yes`. The core stackctl conventions are: + +- `-y` / `--yes` skips the "are you sure?" prompt +- No-flag means prompt; prompt reads from stdin +- A full "destructive" operation also prints a summary of what will happen before the prompt + +### Emit JSON responses verbatim on `-o json` + +When the server returns a response body, forward it unmodified rather than reshaping it. Lets users write stable jq expressions that survive plugin version bumps. + +### Install to a consistent location + +Document where your plugin should live. Common choices: + +- `~/.local/bin/` (user-scoped; already on most PATHs via `~/.profile`) +- `/usr/local/bin/` (system-wide; requires sudo) +- A project-managed `bin/` dir added to PATH in your team's shell profile + +### Version your plugin independently + +Plugins and stackctl evolve separately. Tag releases, publish changelogs, keep a `--version` flag so users can report what they're running when they file bugs. + +--- + +## Distribution + +Because plugins are just executables, you ship them however your team already ships CLIs. + +### Simple: a tarball or Docker image + +```bash +# Install script +curl -sSfL https://your-host/install-plugin.sh | bash +``` + +The install script drops the binary in `~/.local/bin/` and that's it. + +### Homebrew / apt / yum + +Normal package manager flow. No plugin-specific infrastructure needed. + +### Kubernetes / container-based distribution + +If your org already distributes internal tools as container images, bake the plugin into an image the user pulls and copies out: + +```bash +docker cp $(docker create your-company/stackctl-plugins:latest):/bin/stackctl-refresh-db ~/.local/bin/ +``` + +### Team-internal: checked into a dotfiles repo + +For small teams, plugin source lives in a shared dotfiles repo and ships via each developer's bootstrap script. + +--- + +## How it works internally + +On `Execute()`, stackctl scans every directory in `$PATH`. For each regular executable file whose name starts with `stackctl-`: + +1. Strip the `stackctl-` prefix to get the plugin name. +2. Skip if a built-in subcommand with the same name is already registered. +3. Register a new Cobra subcommand: + - `Use: ` + - `Short: "Plugin: "` (path kept out of the summary listing) + - `Long`: includes the plugin's absolute path for `stackctl help ` + - `DisableFlagParsing: true` — plugin handles its own flags + - `RunE`: exec the binary with the remaining args, piping I/O, propagating exit code + +If the user runs `stackctl …`, Cobra routes to the registered subcommand's `RunE`, which exec's the plugin. + +Source: [cli/cmd/plugins.go](cli/cmd/plugins.go) (≈110 lines). No plugin framework, no SDK — just `os/exec` + `$PATH`. + +--- + +## Troubleshooting + +### "unknown command" but my plugin exists + +- Check the binary is **executable** (`chmod +x`). Non-executable files in PATH are ignored. +- Check the directory is actually in `$PATH`. `which stackctl-` should find it. +- Check the name starts with `stackctl-` (not `stackctl_` or `stackctl `). +- Shadow check: is there a built-in `stackctl `? `stackctl --help | grep ''` — if the Short text doesn't say "Plugin:", a built-in is winning. + +### Plugin runs but `$STACKCTL_API_URL` is empty + +The plugin process only sees environment variables that were **already exported** in the user's shell. `stackctl config set api-url ` writes to `~/.stackmanager/config.yaml`; it does **not** export `STACKCTL_API_URL` into the environment for subsequent plugin execs. + +Pick one workflow and document it for your plugin's users: + +- **Have the user export env vars explicitly:** + + ```bash + export STACKCTL_API_URL="$(stackctl config get api-url)" + export STACKCTL_API_KEY="$(stackctl config get api-key)" + stackctl my-plugin … + ``` + +- **Have the plugin resolve config via `stackctl config get`:** + shell out to `stackctl config get api-url` and `stackctl config get api-key` + if the env vars are empty. One extra exec per value, but avoids parsing + internal config formats. + +Core stackctl commands do config resolution automatically; plugins are plain exec'd subprocesses, so env is all they get unless you resolve config via the commands above. + +### Plugin exits non-zero but stackctl exits 0 + +The plugin ran but didn't fail. Check the plugin's own error handling. `bash -x` or `set -x` inside a shell plugin is a quick way to see what happened. + +### Plugin shadows a built-in + +Built-ins always win. Rename the plugin to something that doesn't collide. (This is a safety feature — otherwise a malicious `stackctl-config` on PATH could intercept credentials.) + +--- + +## Related + +- [k8s-stack-manager EXTENDING.md](https://github.com/omattsson/k8s-stack-manager/blob/main/EXTENDING.md) — the server side: how to register a webhook handler that a stackctl plugin can invoke. Most plugins pair with an action webhook; this is the complete picture. +- [cli/cmd/plugins.go](cli/cmd/plugins.go) — plugin-discovery implementation. +- [cli/cmd/plugins_test.go](cli/cmd/plugins_test.go) — test patterns for plugin behaviour. diff --git a/README.md b/README.md index 5413c32..de5969a 100644 --- a/README.md +++ b/README.md @@ -52,6 +52,30 @@ stackctl template quick-deploy 1 stackctl stack list --mine ``` +## 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`. + +Because the contract is "any executable with the right name", plugins can be written in any language (shell, Python, Go, Node, Rust, …) and shipped however your team already distributes binaries. + +Quick example: + +```bash +cat > ~/.local/bin/stackctl-hello <<'EOF' +#!/usr/bin/env bash +echo "Hello! API=${STACKCTL_API_URL} args=$*" +EOF +chmod +x ~/.local/bin/stackctl-hello + +stackctl hello world # → Hello! API=http://... args=world +stackctl --help | grep hello +# hello Plugin: hello +``` + +The plugin inherits the user's full environment. If `STACKCTL_API_URL` and `STACKCTL_API_KEY` are exported in your shell, the plugin sees them. Values saved with `stackctl config set` are **not** automatically exported — export them yourself (`export STACKCTL_API_URL="$(stackctl config get api-url)"`) or see [EXTENDING.md](EXTENDING.md) for the full story. Built-in subcommands always win on name collisions (a safety feature — a malicious `stackctl-config` on PATH can't intercept credentials). + +👉 **[Full guide: EXTENDING.md](EXTENDING.md)** — tutorial, recipes in bash/Python/Go, best practices, and how plugins pair with [server-side action webhooks](https://github.com/omattsson/k8s-stack-manager/blob/main/EXTENDING.md) for end-to-end custom operations. + ## Configuration stackctl uses named contexts to manage multiple environments. Configuration is stored in `~/.stackmanager/config.yaml`. diff --git a/cli/cmd/plugins.go b/cli/cmd/plugins.go new file mode 100644 index 0000000..d4a05d5 --- /dev/null +++ b/cli/cmd/plugins.go @@ -0,0 +1,214 @@ +package cmd + +import ( + "fmt" + "os" + "os/exec" + "path/filepath" + "regexp" + "runtime" + "strings" + + "github.com/spf13/cobra" +) + +// pluginPrefix is the filename prefix that marks an executable as a stackctl +// plugin. A binary at PATH/stackctl-foo becomes the subcommand `stackctl foo`. +const pluginPrefix = "stackctl-" + +// pluginNamePattern restricts plugin names to lowercase ASCII letters, digits, +// and dashes, and requires the first character to be a letter or digit so names +// form valid Cobra command names. A name like "stackctl- bad" (with whitespace) +// or "stackctl--help" breaks help routing and is skipped at discovery time. +// Uppercase filenames (stackctl-Foo) are skipped — Cobra is case-sensitive and +// mixed-case subcommands are a footgun more than a feature. +var pluginNamePattern = regexp.MustCompile(`^[a-z0-9][a-z0-9-]*$`) + +// registerPlugins scans $PATH for executables named stackctl- and adds +// each as a top-level subcommand that proxies to the external binary. +// +// Behaviour: +// - Built-in subcommands always win on name collision; the plugin is skipped. +// - Earlier entries in $PATH win over later duplicates (standard PATH semantics). +// - Non-regular files, directories, and non-executables are ignored. +// - Plugin stdout/stderr/stdin pass through the parent tty; exit codes propagate. +// +// Pattern modelled on git, kubectl, and gh. See also: +// +// https://git-scm.com/docs/git#_git_commands +// https://kubernetes.io/docs/tasks/extend-kubectl/kubectl-plugins/ +func registerPlugins(root *cobra.Command, pathEnv string) { + builtins := existingCommandNames(root) + // Cobra registers "help" and "completion" implicitly — make sure a + // stackctl-help or stackctl-completion on PATH can't shadow them. + builtins["help"] = struct{}{} + builtins["completion"] = struct{}{} + for name, path := range discoverPlugins(pathEnv) { + if _, collides := builtins[name]; collides { + continue + } + root.AddCommand(newPluginCommand(name, path)) + builtins[name] = struct{}{} + } +} + +// discoverPlugins walks pathEnv (the process $PATH) and returns a map of +// plugin name to absolute path. First-win semantics: earlier PATH entries +// take precedence over later ones when multiple binaries share a name. +// +// Paths are resolved to absolute via filepath.Abs so execution is safe +// regardless of relative entries in $PATH — the captured path is what we +// later exec, which means rebinding $PATH after this function runs cannot +// change which binary a plugin routes to. +// +// Windows: discovery strips a trailing .exe so stackctl-foo.exe surfaces as +// the subcommand `foo`. Other PATHEXT extensions (.bat, .cmd, .ps1) are not +// currently recognised — if you need them, name the plugin binary .exe or +// front it with a .exe shim. +func discoverPlugins(pathEnv string) map[string]string { + found := make(map[string]string) + for _, dir := range filepath.SplitList(pathEnv) { + if dir == "" { + continue + } + entries, err := os.ReadDir(dir) + if err != nil { + continue + } + for _, entry := range entries { + name := entry.Name() + if !strings.HasPrefix(name, pluginPrefix) || name == pluginPrefix { + continue + } + pluginName := strings.TrimPrefix(name, pluginPrefix) + if runtime.GOOS == "windows" { + if low := strings.ToLower(pluginName); strings.HasSuffix(low, ".exe") { + pluginName = pluginName[:len(pluginName)-4] + } + } + if !pluginNamePattern.MatchString(pluginName) { + continue + } + if _, seen := found[pluginName]; seen { + continue + } + full, err := filepath.Abs(filepath.Join(dir, name)) + if err != nil { + continue + } + info, err := os.Stat(full) + if err != nil || !info.Mode().IsRegular() { + continue + } + if runtime.GOOS != "windows" && info.Mode().Perm()&0o111 == 0 { + continue + } + found[pluginName] = full + } + } + return found +} + +// pluginEnv returns the environment to pass to a plugin subprocess. It +// preserves the full parent environment — plugins might legitimately need +// unrelated variables (AWS_PROFILE, KUBECONFIG, etc.) — and injects +// STACKCTL_* values resolved from flags so a plugin sees the same effective +// config stackctl itself would use for a built-in command. +// +// Precedence: an explicitly-passed flag wins over a pre-existing env var. +// Flags that weren't set on the command line leave the inherited env +// untouched, so STACKCTL_INSECURE=1 in the parent shell keeps working for +// plugin invocations when no --insecure flag is passed. +// +// Flag-to-env wiring documented in EXTENDING.md as a plugin-author contract. +func pluginEnv(cmd *cobra.Command) []string { + env := os.Environ() + if cmd == nil { + return env + } + flags := cmd.Root().PersistentFlags() + if flags.Changed("insecure") { + if insecure, err := flags.GetBool("insecure"); err == nil { + env = setEnv(env, "STACKCTL_INSECURE", boolEnvValue(insecure)) + } + } + if flags.Changed("quiet") { + if quiet, err := flags.GetBool("quiet"); err == nil { + env = setEnv(env, "STACKCTL_QUIET", boolEnvValue(quiet)) + } + } + if flags.Changed("output") { + if output, err := flags.GetString("output"); err == nil { + env = setEnv(env, "STACKCTL_OUTPUT", strings.ToLower(strings.TrimSpace(output))) + } + } + return env +} + +// setEnv replaces (or appends) KEY=value in env, preserving order. +func setEnv(env []string, key, value string) []string { + prefix := key + "=" + for i, kv := range env { + if strings.HasPrefix(kv, prefix) { + env[i] = prefix + value + return env + } + } + return append(env, prefix+value) +} + +func boolEnvValue(b bool) string { + if b { + return "1" + } + return "0" +} + +// existingCommandNames returns the set of top-level subcommand names already +// registered on root, so discovery can avoid clobbering built-ins. +func existingCommandNames(root *cobra.Command) map[string]struct{} { + names := make(map[string]struct{}) + for _, c := range root.Commands() { + names[c.Name()] = struct{}{} + for _, alias := range c.Aliases { + names[alias] = struct{}{} + } + } + return names +} + +// newPluginCommand wraps an external binary as a Cobra subcommand. The binary +// receives all arguments after the plugin name; stdin/stdout/stderr pass +// through directly, and the exit code propagates to the caller. +// +// binaryPath is captured at discovery time so later PATH modifications can't +// rebind which binary we exec — the absolute path we resolved in +// discoverPlugins is the exact path we run. +func newPluginCommand(name, binaryPath string) *cobra.Command { + return &cobra.Command{ + // Hide the absolute path from --help listings (leaks home dir in screenshots). + // The full path is in Long so `stackctl help ` still reveals it for debugging. + Use: name, + Short: "Plugin: " + name, + Long: "External plugin resolved to " + binaryPath, + DisableFlagParsing: true, + SilenceUsage: true, + SilenceErrors: true, + RunE: func(cmd *cobra.Command, args []string) error { + proc := exec.Command(binaryPath, args...) //nolint:gosec // absolute path captured at discovery time; rebinding via PATH is impossible after that point + proc.Stdin = cmd.InOrStdin() + proc.Stdout = cmd.OutOrStdout() + proc.Stderr = cmd.ErrOrStderr() + proc.Env = pluginEnv(cmd) + if err := proc.Run(); err != nil { + if exitErr, ok := err.(*exec.ExitError); ok { + // Propagate the plugin's exit code. Cobra will surface + // the error; explicit os.Exit keeps the code intact. + os.Exit(exitErr.ExitCode()) + } + return fmt.Errorf("plugin %q: %w", name, err) + } + return nil + }, + } +} diff --git a/cli/cmd/plugins_test.go b/cli/cmd/plugins_test.go new file mode 100644 index 0000000..6c60c57 --- /dev/null +++ b/cli/cmd/plugins_test.go @@ -0,0 +1,226 @@ +package cmd + +import ( + "bytes" + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "testing" + + "github.com/spf13/cobra" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// writeScript writes a shell script executable at dir/name with the given body. +// Returns the absolute path. Skips the test on Windows — shell scripts and 0o755 +// aren't a useful way to exercise plugin discovery there. +func writeScript(t *testing.T, dir, name, body string) string { + t.Helper() + if runtime.GOOS == "windows" { + t.Skip("plugin discovery tests use shell scripts; skip on Windows") + } + path := filepath.Join(dir, name) + require.NoError(t, os.WriteFile(path, []byte(body), 0o755)) + return path +} + +func TestDiscoverPlugins_FindsExecutableStackctlBinaries(t *testing.T) { + + dir := t.TempDir() + _ = writeScript(t, dir, "stackctl-hello", "#!/bin/sh\necho hi\n") + // Also place a non-plugin binary that shouldn't match. + _ = writeScript(t, dir, "notaplugin", "#!/bin/sh\necho nope\n") + // And a bare "stackctl-" which is the empty-name case — must be ignored. + _ = writeScript(t, dir, "stackctl-", "#!/bin/sh\necho empty\n") + + got := discoverPlugins(dir) + require.Contains(t, got, "hello") + assert.Len(t, got, 1) + assert.True(t, filepath.IsAbs(got["hello"])) +} + +func TestDiscoverPlugins_SkipsNonExecutable(t *testing.T) { + + dir := t.TempDir() + nonExec := filepath.Join(dir, "stackctl-readonly") + require.NoError(t, os.WriteFile(nonExec, []byte("#!/bin/sh\necho hi\n"), 0o644)) + + got := discoverPlugins(dir) + assert.NotContains(t, got, "readonly", "non-executable files must be ignored") +} + +func TestDiscoverPlugins_FirstPathEntryWins(t *testing.T) { + + dir1 := t.TempDir() + dir2 := t.TempDir() + _ = writeScript(t, dir1, "stackctl-same", "#!/bin/sh\necho first\n") + _ = writeScript(t, dir2, "stackctl-same", "#!/bin/sh\necho second\n") + + got := discoverPlugins(dir1 + string(os.PathListSeparator) + dir2) + require.Contains(t, got, "same") + assert.Equal(t, filepath.Join(dir1, "stackctl-same"), got["same"]) +} + +func TestDiscoverPlugins_IgnoresMissingAndEmptyPATHEntries(t *testing.T) { + + dir := t.TempDir() + _ = writeScript(t, dir, "stackctl-ok", "#!/bin/sh\necho ok\n") + + got := discoverPlugins(string(os.PathListSeparator) + "/nonexistent/path" + string(os.PathListSeparator) + dir) + assert.Contains(t, got, "ok") +} + +func TestRegisterPlugins_AddsPluginAsSubcommand(t *testing.T) { + + dir := t.TempDir() + _ = writeScript(t, dir, "stackctl-greet", "#!/bin/sh\necho hello-from-plugin\n") + + root := &cobra.Command{Use: "stackctl"} + builtin := &cobra.Command{Use: "config", Short: "builtin"} + root.AddCommand(builtin) + + registerPlugins(root, dir) + + var greet *cobra.Command + for _, c := range root.Commands() { + if c.Name() == "greet" { + greet = c + break + } + } + require.NotNil(t, greet, "plugin subcommand must be registered") + assert.Contains(t, greet.Short, "Plugin:") +} + +func TestRegisterPlugins_BuiltinWinsOnCollision(t *testing.T) { + + dir := t.TempDir() + _ = writeScript(t, dir, "stackctl-config", "#!/bin/sh\necho shadow\n") + + root := &cobra.Command{Use: "stackctl"} + builtin := &cobra.Command{Use: "config", Short: "builtin"} + root.AddCommand(builtin) + + registerPlugins(root, dir) + + // Verify exactly one command named "config" exists (not two, with Cobra + // silently accepting the duplicate) and that it's the built-in. + named := 0 + var found *cobra.Command + for _, c := range root.Commands() { + if c.Name() == "config" { + named++ + found = c + } + } + assert.Equal(t, 1, named, "exactly one command should be named 'config'") + require.NotNil(t, found) + assert.Equal(t, "builtin", found.Short, "built-in must not be replaced by a colliding plugin") +} + +func TestRegisterPlugins_PluginInvocationPassesThroughArgsAndStdout(t *testing.T) { + + dir := t.TempDir() + _ = writeScript(t, dir, "stackctl-echo", + "#!/bin/sh\nprintf '%s|' \"$@\"\n") + + root := &cobra.Command{Use: "stackctl"} + registerPlugins(root, dir) + + var pluginCmd *cobra.Command + for _, c := range root.Commands() { + if c.Name() == "echo" { + pluginCmd = c + break + } + } + require.NotNil(t, pluginCmd) + + // The plugin uses os.Exit on error; on success we get its stdout. + // Cobra's Execute path would os.Exit; drive the wrapped binary directly via + // exec for a clean, assertable invocation. + bin := filepath.Join(dir, "stackctl-echo") + out, err := exec.Command(bin, "alpha", "--flag=beta", "gamma").CombinedOutput() + require.NoError(t, err) + assert.Equal(t, "alpha|--flag=beta|gamma|", string(out)) +} + +func TestRegisterPlugins_NoOpOnEmptyPath(t *testing.T) { + + root := &cobra.Command{Use: "stackctl"} + before := len(root.Commands()) + registerPlugins(root, "") + assert.Equal(t, before, len(root.Commands())) +} + +// TestRegisterPlugins_StdinPassthrough proves stdin is routed to the plugin. +// Uses a `cat` style shell script that copies stdin to stdout; the plugin +// subcommand reads stdin via cmd.InOrStdin() which Cobra resolves to the +// buffer we set via root.SetIn. +func TestRegisterPlugins_StdinPassthrough(t *testing.T) { + + dir := t.TempDir() + _ = writeScript(t, dir, "stackctl-cat", "#!/bin/sh\ncat\n") + + root := &cobra.Command{Use: "stackctl", SilenceUsage: true, SilenceErrors: true} + registerPlugins(root, dir) + + var catCmd *cobra.Command + for _, c := range root.Commands() { + if c.Name() == "cat" { + catCmd = c + break + } + } + require.NotNil(t, catCmd) + + var outBuf bytes.Buffer + root.SetIn(strings.NewReader("payload-from-stdin")) + root.SetOut(&outBuf) + root.SetErr(&outBuf) + + require.NoError(t, catCmd.RunE(catCmd, nil)) + assert.Equal(t, "payload-from-stdin", outBuf.String(), + "plugin must receive stdin and its stdout must reach root's writer") +} + +// TestRegisterPlugins_RunViaCobraRouting wires a plugin through Cobra's +// Execute path to verify the command is actually routed by name. The plugin +// exits 0, so the parent survives; args are captured via a sentinel file +// the plugin writes. +func TestRegisterPlugins_RunViaCobraRouting(t *testing.T) { + + dir := t.TempDir() + sentinel := filepath.Join(t.TempDir(), "args.txt") + script := "#!/bin/sh\nprintf '%s ' \"$@\" > " + sentinel + "\n" + _ = writeScript(t, dir, "stackctl-touch", script) + + root := &cobra.Command{Use: "stackctl", SilenceUsage: true, SilenceErrors: true} + registerPlugins(root, dir) + + var buf bytes.Buffer + root.SetOut(&buf) + root.SetErr(&buf) + root.SetArgs([]string{"touch", "--hello=world", "posarg"}) + + // Run the plugin directly via the command's RunE rather than via Execute — + // Execute may call os.Exit(0) on failure paths via our plugin wrapper and + // that would terminate the test process. + var touchCmd *cobra.Command + for _, c := range root.Commands() { + if c.Name() == "touch" { + touchCmd = c + break + } + } + require.NotNil(t, touchCmd) + require.NoError(t, touchCmd.RunE(touchCmd, []string{"--hello=world", "posarg"})) + + contents, err := os.ReadFile(sentinel) + require.NoError(t, err) + assert.Contains(t, string(contents), "--hello=world") + assert.Contains(t, string(contents), "posarg") +} diff --git a/cli/cmd/root.go b/cli/cmd/root.go index 1d53153..17b6d2b 100644 --- a/cli/cmd/root.go +++ b/cli/cmd/root.go @@ -86,6 +86,11 @@ func init() { // Execute runs the root command. func Execute() error { + // Discover external plugins from $PATH and register any that do not + // collide with built-in commands. Ignoring collisions rather than + // erroring keeps the CLI usable if a user happens to have a + // `stackctl-` binary lying around. + registerPlugins(rootCmd, os.Getenv("PATH")) return rootCmd.Execute() } diff --git a/cli/cmd/stack.go b/cli/cmd/stack.go index 300e0cd..2d750ea 100644 --- a/cli/cmd/stack.go +++ b/cli/cmd/stack.go @@ -301,65 +301,6 @@ Examples: }, } -var stackRefreshDBCmd = &cobra.Command{ - Use: "refresh-db ", - Short: "Refresh a stack's MySQL database from the golden-db snapshot", - Long: `Refresh the MySQL database for a running stack instance without a full -clean+redeploy. Wipes the MySQL PVC so the init container re-extracts the -golden-db snapshot on next boot, flushes Redis, and deletes the storefront -sync Job so the next ` + "`stack deploy`" + ` re-fires the Helm hook that repopulates -Redis. - -Does NOT re-run helm — only scales deployments and runs a short-lived -cleanup Job. The stack must be in 'running' state. - -This is a destructive operation (wipes all MySQL data for this stack). -You will be prompted for confirmation unless --yes is specified. - -After refresh-db completes, run ` + "`stack deploy`" + ` to re-populate Redis via -the sync Job. - -Examples: - stackctl stack refresh-db 42 - stackctl stack refresh-db 42 --yes`, - Args: cobra.ExactArgs(1), - SilenceUsage: true, - RunE: func(cmd *cobra.Command, args []string) error { - id, err := parseID(args[0]) - if err != nil { - return err - } - - confirmed, err := confirmAction(cmd, fmt.Sprintf("This will wipe MySQL data for stack %s and reload from golden-db. Continue? (y/n): ", id)) - if err != nil { - return err - } - if !confirmed { - printer.PrintMessage("Aborted.") - return nil - } - - c, err := newClient() - if err != nil { - return err - } - - log, err := c.RefreshDBStack(id) - if err != nil { - return err - } - - if printer.Quiet { - fmt.Fprintln(printer.Writer, log.ID) - return nil - } - - printer.PrintMessage("Refreshing database for stack %s... (log ID: %s)", id, log.ID) - printer.PrintMessage("Run 'stackctl stack logs %s' to check progress, or 'stackctl stack deploy %s' afterwards to re-populate Redis.", id, id) - return nil - }, -} - var stackDeleteCmd = &cobra.Command{ Use: "delete ", Short: "Delete a stack instance", @@ -709,9 +650,6 @@ func init() { // stack clean flags stackCleanCmd.Flags().BoolP("yes", "y", false, "Skip confirmation prompt") - // stack refresh-db flags - stackRefreshDBCmd.Flags().BoolP("yes", "y", false, "Skip confirmation prompt") - // stack delete flags stackDeleteCmd.Flags().BoolP("yes", "y", false, "Skip confirmation prompt") @@ -729,7 +667,6 @@ func init() { stackCmd.AddCommand(stackDeployCmd) stackCmd.AddCommand(stackStopCmd) stackCmd.AddCommand(stackCleanCmd) - stackCmd.AddCommand(stackRefreshDBCmd) stackCmd.AddCommand(stackDeleteCmd) stackCmd.AddCommand(stackStatusCmd) stackCmd.AddCommand(stackLogsCmd) diff --git a/cli/cmd/stack_test.go b/cli/cmd/stack_test.go index 1be5376..c190971 100644 --- a/cli/cmd/stack_test.go +++ b/cli/cmd/stack_test.go @@ -489,112 +489,6 @@ func TestStackCleanCmd_WithYesFlag(t *testing.T) { assert.Contains(t, buf.String(), "Cleaning stack 42") } -// ---------- stack refresh-db (destructive) ---------- - -func TestStackRefreshDBCmd_WithConfirmation(t *testing.T) { - called := false - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - called = true - require.Equal(t, http.MethodPost, r.Method) - require.Equal(t, "/api/v1/stack-instances/42/refresh-db", r.URL.Path) - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusAccepted) - json.NewEncoder(w).Encode(types.DeploymentLog{Base: types.Base{ID: "log-104"}}) - })) - defer server.Close() - - buf := setupStackTestCmd(t, server.URL) - - stackRefreshDBCmd.Flags().Set("yes", "false") - t.Cleanup(func() { - stackRefreshDBCmd.Flags().Set("yes", "false") - stackRefreshDBCmd.SetIn(nil) - stackRefreshDBCmd.SetErr(nil) - }) - - stackRefreshDBCmd.SetIn(strings.NewReader("y\n")) - stackRefreshDBCmd.SetErr(&bytes.Buffer{}) - - err := stackRefreshDBCmd.RunE(stackRefreshDBCmd, []string{"42"}) - require.NoError(t, err) - assert.True(t, called, "API should be called after confirming with y") - assert.Contains(t, buf.String(), "Refreshing database for stack 42") - assert.Contains(t, buf.String(), "log-104") -} - -func TestStackRefreshDBCmd_Declined(t *testing.T) { - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - t.Fatal("API should NOT be called when user declines") - })) - defer server.Close() - - buf := setupStackTestCmd(t, server.URL) - - stackRefreshDBCmd.Flags().Set("yes", "false") - t.Cleanup(func() { - stackRefreshDBCmd.Flags().Set("yes", "false") - stackRefreshDBCmd.SetIn(nil) - stackRefreshDBCmd.SetErr(nil) - }) - - stackRefreshDBCmd.SetIn(strings.NewReader("n\n")) - stackRefreshDBCmd.SetErr(&bytes.Buffer{}) - - err := stackRefreshDBCmd.RunE(stackRefreshDBCmd, []string{"42"}) - require.NoError(t, err) - assert.Contains(t, buf.String(), "Aborted") -} - -func TestStackRefreshDBCmd_WithYesFlag(t *testing.T) { - called := false - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - called = true - require.Equal(t, http.MethodPost, r.Method) - require.Equal(t, "/api/v1/stack-instances/42/refresh-db", r.URL.Path) - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusAccepted) - json.NewEncoder(w).Encode(types.DeploymentLog{Base: types.Base{ID: "log-105"}}) - })) - defer server.Close() - - buf := setupStackTestCmd(t, server.URL) - - stackRefreshDBCmd.Flags().Set("yes", "true") - t.Cleanup(func() { stackRefreshDBCmd.Flags().Set("yes", "false") }) - - err := stackRefreshDBCmd.RunE(stackRefreshDBCmd, []string{"42"}) - require.NoError(t, err) - assert.True(t, called, "API should be called with --yes flag") - assert.Contains(t, buf.String(), "Refreshing database for stack 42") -} - -func TestStackRefreshDBCmd_QuietPrintsOnlyLogID(t *testing.T) { - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusAccepted) - json.NewEncoder(w).Encode(types.DeploymentLog{Base: types.Base{ID: "log-quiet-106"}}) - })) - defer server.Close() - - buf := setupStackTestCmd(t, server.URL) - - stackRefreshDBCmd.Flags().Set("yes", "true") - prevQuiet := printer.Quiet - printer.Quiet = true - t.Cleanup(func() { - stackRefreshDBCmd.Flags().Set("yes", "false") - printer.Quiet = prevQuiet - }) - - err := stackRefreshDBCmd.RunE(stackRefreshDBCmd, []string{"42"}) - require.NoError(t, err) - out := buf.String() - assert.Contains(t, out, "log-quiet-106") - // Quiet mode must not print the human-readable prefix. - assert.NotContains(t, out, "Refreshing database for stack") - assert.NotContains(t, out, "Run 'stackctl stack logs") -} - // ---------- stack delete (destructive) ---------- func TestStackDeleteCmd_WithConfirmation(t *testing.T) { diff --git a/cli/pkg/client/client.go b/cli/pkg/client/client.go index 43e330d..93b4dfa 100644 --- a/cli/pkg/client/client.go +++ b/cli/pkg/client/client.go @@ -330,20 +330,6 @@ func (c *Client) CleanStack(id string) (*types.DeploymentLog, error) { return &log, nil } -// RefreshDBStack triggers a database refresh for a stack instance: wipes the -// MySQL PVC so the init container re-extracts the golden-db snapshot on next -// boot, flushes Redis, and deletes the storefront sync Job so the next deploy -// re-fires the Helm hook. Does NOT re-run helm — only orchestrates k8s -// primitives. Status must be `running`. -func (c *Client) RefreshDBStack(id string) (*types.DeploymentLog, error) { - var log types.DeploymentLog - err := c.Post(fmt.Sprintf("/api/v1/stack-instances/%s/refresh-db", id), nil, &log) - if err != nil { - return nil, err - } - return &log, nil -} - // GetStackStatus returns the current status and pod states for a stack instance. func (c *Client) GetStackStatus(id string) (*types.InstanceStatus, error) { var status types.InstanceStatus diff --git a/cli/pkg/client/client_test.go b/cli/pkg/client/client_test.go index 529bbb0..8c8a238 100644 --- a/cli/pkg/client/client_test.go +++ b/cli/pkg/client/client_test.go @@ -769,46 +769,6 @@ func TestCleanStack_Success(t *testing.T) { assert.Equal(t, "clean", log.Action) } -func TestRefreshDBStack_Success(t *testing.T) { - t.Parallel() - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - assert.Equal(t, http.MethodPost, r.Method) - assert.Equal(t, "/api/v1/stack-instances/42/refresh-db", r.URL.Path) - w.WriteHeader(http.StatusAccepted) - json.NewEncoder(w).Encode(types.DeploymentLog{ - Base: types.Base{ID: "log-103"}, - InstanceID: "42", - Action: "refresh-db", - Status: "started", - }) - })) - defer server.Close() - - c := New(server.URL) - log, err := c.RefreshDBStack("42") - require.NoError(t, err) - assert.Equal(t, "log-103", log.ID) - assert.Equal(t, "refresh-db", log.Action) -} - -func TestRefreshDBStack_Conflict(t *testing.T) { - t.Parallel() - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(http.StatusConflict) - json.NewEncoder(w).Encode(map[string]string{"error": "Cannot refresh-db: instance is currently deploying"}) - })) - defer server.Close() - - c := New(server.URL) - log, err := c.RefreshDBStack("42") - require.Error(t, err) - assert.Nil(t, log) - - apiErr, ok := err.(*APIError) - require.True(t, ok, "expected APIError, got %T", err) - assert.Equal(t, http.StatusConflict, apiErr.StatusCode) -} - func TestGetStackStatus_Success(t *testing.T) { t.Parallel() server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/cli/pkg/output/formatter_test.go b/cli/pkg/output/formatter_test.go new file mode 100644 index 0000000..429d63d --- /dev/null +++ b/cli/pkg/output/formatter_test.go @@ -0,0 +1,205 @@ +package output + +import ( + "bytes" + "fmt" + "io" + "strings" + "sync" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// registry state bleeds across tests since it's package-global. tests that +// register formats clean up via t.Cleanup. +func unregisterForTest(t *testing.T, names ...string) { + t.Helper() + t.Cleanup(func() { + formatRegistryMu.Lock() + defer formatRegistryMu.Unlock() + for _, n := range names { + delete(formatRegistry, n) + delete(singleRegistry, n) + } + }) +} + +func TestRegisterFormat_PanicOnBuiltinOverride(t *testing.T) { + t.Parallel() + // Pass a non-nil fn so the built-in-name check fires first — otherwise + // the nil-fn guard would shadow the error we're asserting. + noop := func(io.Writer, interface{}, []string, [][]string) error { return nil } + for _, name := range []string{"table", "json", "yaml"} { + name := name + t.Run(name, func(t *testing.T) { + t.Parallel() + assert.PanicsWithValue(t, + fmt.Sprintf("output: cannot override built-in format %q", name), + func() { RegisterFormat(name, noop) }) + }) + } +} + +func TestRegisterFormat_PanicOnNilFunc(t *testing.T) { + t.Parallel() + assert.PanicsWithValue(t, + `output: nil FormatterFunc for format "customnil"`, + func() { RegisterFormat("customnil", nil) }) + assert.PanicsWithValue(t, + `output: nil SingleFormatterFunc for format "customnil"`, + func() { RegisterSingleFormat("customnil", nil) }) +} + +// TestRegisterFormat_CaseInsensitiveAndTrimmed locks in the contract that +// RegisterFormat normalises the name the same way NewPrinter does when +// resolving --output. Without this, `RegisterFormat("CSV", ...)` would +// never match a user running `stackctl … -o csv`. +func TestRegisterFormat_CaseInsensitiveAndTrimmed(t *testing.T) { + t.Parallel() + called := 0 + fn := func(io.Writer, interface{}, []string, [][]string) error { + called++ + return nil + } + RegisterFormat(" TRIMTEST ", fn) + unregisterForTest(t, "trimtest") + + p := NewPrinter("trimtest", false, true) + p.Writer = io.Discard + assert.Equal(t, Format("trimtest"), p.Format, "NewPrinter should match case-insensitively") + require.NoError(t, p.Print(nil, nil, nil, nil)) + assert.Equal(t, 1, called, "formatter registered under 'TRIMTEST' should be reached via 'trimtest'") +} + +// TestRegisterFormat_BuiltinCollisionIsCaseInsensitive ensures the +// built-in check runs after normalisation, so `RegisterFormat("JSON", ...)` +// panics the same way as the lowercase form. +func TestRegisterFormat_BuiltinCollisionIsCaseInsensitive(t *testing.T) { + t.Parallel() + noop := func(io.Writer, interface{}, []string, [][]string) error { return nil } + for _, name := range []string{"JSON", " yaml", "Table "} { + name := name + t.Run(name, func(t *testing.T) { + t.Parallel() + assert.Panics(t, func() { RegisterFormat(name, noop) }, + "expected panic for built-in collision on %q", name) + }) + } +} + +func TestNewPrinter_UsesCustomFormat(t *testing.T) { + t.Parallel() + RegisterFormat("csv", func(w io.Writer, _ interface{}, headers []string, rows [][]string) error { + _, err := fmt.Fprintln(w, strings.Join(headers, ",")) + if err != nil { + return err + } + for _, r := range rows { + if _, err := fmt.Fprintln(w, strings.Join(r, ",")); err != nil { + return err + } + } + return nil + }) + unregisterForTest(t, "csv") + + var buf bytes.Buffer + p := NewPrinter("csv", false, true) + p.Writer = &buf + require.Equal(t, Format("csv"), p.Format) + + err := p.Print(nil, []string{"id", "name"}, [][]string{{"1", "alpha"}, {"2", "beta"}}, []string{"1", "2"}) + require.NoError(t, err) + assert.Equal(t, "id,name\n1,alpha\n2,beta\n", buf.String()) +} + +func TestNewPrinter_UnknownFormatFallsBackToTable(t *testing.T) { + t.Parallel() + p := NewPrinter("neverregistered", false, true) + assert.Equal(t, FormatTable, p.Format) +} + +func TestPrint_CustomFormatReceivesTableArgs(t *testing.T) { + t.Parallel() + + var ( + gotHeaders []string + gotRows [][]string + gotData interface{} + ) + RegisterFormat("capture", func(_ io.Writer, data interface{}, headers []string, rows [][]string) error { + gotData = data + gotHeaders = headers + gotRows = rows + return nil + }) + unregisterForTest(t, "capture") + + p := NewPrinter("capture", false, true) + p.Writer = io.Discard + err := p.Print("payload", []string{"h1"}, [][]string{{"r1"}}, nil) + require.NoError(t, err) + assert.Equal(t, "payload", gotData) + assert.Equal(t, []string{"h1"}, gotHeaders) + assert.Equal(t, [][]string{{"r1"}}, gotRows) +} + +func TestPrintSingle_FallsBackToListFormatterWhenNoSingleRegistered(t *testing.T) { + t.Parallel() + + var ( + gotRows [][]string + ) + RegisterFormat("csv2", func(_ io.Writer, _ interface{}, _ []string, rows [][]string) error { + gotRows = rows + return nil + }) + unregisterForTest(t, "csv2") + + p := NewPrinter("csv2", false, true) + p.Writer = io.Discard + require.NoError(t, p.PrintSingle(nil, []KeyValue{{Key: "id", Value: "42"}, {Key: "name", Value: "x"}})) + + assert.Equal(t, [][]string{{"id", "42"}, {"name", "x"}}, gotRows) +} + +func TestRegisterSingleFormat_UsedWhenPresent(t *testing.T) { + t.Parallel() + + var called bool + RegisterFormat("custom1", func(io.Writer, interface{}, []string, [][]string) error { return nil }) + RegisterSingleFormat("custom1", func(_ io.Writer, _ interface{}, fields []KeyValue) error { + called = true + assert.Len(t, fields, 2) + return nil + }) + unregisterForTest(t, "custom1") + + p := NewPrinter("custom1", false, true) + p.Writer = io.Discard + require.NoError(t, p.PrintSingle(nil, []KeyValue{{Key: "a", Value: "1"}, {Key: "b", Value: "2"}})) + assert.True(t, called, "single formatter takes precedence over list formatter when registered") +} + +// TestRegisterFormat_ConcurrentReadsWithoutRegistration ensures that the +// read path is safe when no writers are active. Combined reads/writes are +// not supported — documented as "register at init time". +func TestRegisterFormat_ConcurrentReadsWithoutRegistration(t *testing.T) { + t.Parallel() + RegisterFormat("concurrent", func(io.Writer, interface{}, []string, [][]string) error { return nil }) + unregisterForTest(t, "concurrent") + + var wg sync.WaitGroup + for i := 0; i < 8; i++ { + wg.Add(1) + go func() { + defer wg.Done() + for j := 0; j < 50; j++ { + _, _ = lookupFormat("concurrent") + } + }() + } + wg.Wait() +} diff --git a/cli/pkg/output/output.go b/cli/pkg/output/output.go index 0a66376..275b767 100644 --- a/cli/pkg/output/output.go +++ b/cli/pkg/output/output.go @@ -6,6 +6,7 @@ import ( "io" "os" "strings" + "sync" "text/tabwriter" "gopkg.in/yaml.v3" @@ -20,6 +21,93 @@ const ( FormatYAML Format = "yaml" ) +// FormatterFunc renders data in a custom format to w. The headers/rows +// pair is populated for list-style output; data is the same payload for +// a structured format like JSON. Implementations may use either or both. +type FormatterFunc func(w io.Writer, data interface{}, headers []string, rows [][]string) error + +// SingleFormatterFunc renders a single item's key/value fields. Optional; +// when a custom format does not register a single formatter, PrintSingle +// falls back to the list formatter with headers=["field", "value"]. +type SingleFormatterFunc func(w io.Writer, data interface{}, fields []KeyValue) error + +var ( + formatRegistryMu sync.RWMutex + formatRegistry = map[string]FormatterFunc{} + singleRegistry = map[string]SingleFormatterFunc{} +) + +// RegisterFormat attaches a named custom format. Call at init time, before +// any Printer is used. The registry is synchronised so registration during +// rendering is not a Go data race, but doing so is unsupported and may lead +// to inconsistent behaviour across concurrent renders. +// +// Name is normalised with strings.ToLower + TrimSpace before storage, so +// lookups match NewPrinter's case-insensitive resolution. Passing a built-in +// name (table, json, yaml) panics to surface the mistake early. A nil fn +// also panics at registration time, so the "which handler?" error surfaces +// at startup rather than inside a render call that could be minutes later. +func RegisterFormat(name string, fn FormatterFunc) { + norm := normalizeFormatName(name) + if norm == "" { + panic("output: RegisterFormat called with empty/whitespace-only name") + } + if norm == string(FormatTable) || norm == string(FormatJSON) || norm == string(FormatYAML) { + panic(fmt.Sprintf("output: cannot override built-in format %q", norm)) + } + if fn == nil { + panic(fmt.Sprintf("output: nil FormatterFunc for format %q", norm)) + } + formatRegistryMu.Lock() + defer formatRegistryMu.Unlock() + formatRegistry[norm] = fn +} + +// RegisterSingleFormat attaches a custom single-item formatter for an +// already-registered format. Optional — only needed when the list and +// single shapes genuinely differ. A nil fn panics at registration time, +// as does registering a single formatter for a format that has no list +// formatter — that combination would silently fall back to a tabwriter +// and is almost always a wiring mistake. +// Names are normalised identically to RegisterFormat. +func RegisterSingleFormat(name string, fn SingleFormatterFunc) { + norm := normalizeFormatName(name) + if norm == "" { + panic("output: RegisterSingleFormat called with empty/whitespace-only name") + } + if fn == nil { + panic(fmt.Sprintf("output: nil SingleFormatterFunc for format %q", norm)) + } + formatRegistryMu.Lock() + defer formatRegistryMu.Unlock() + if _, ok := formatRegistry[norm]; !ok { + panic(fmt.Sprintf("output: RegisterSingleFormat %q called before RegisterFormat — register the list formatter first", norm)) + } + singleRegistry[norm] = fn +} + +// normalizeFormatName is the single source of truth for turning a user- +// supplied format name into its registry key. Case-insensitive + whitespace +// tolerant. +func normalizeFormatName(name string) string { + return strings.ToLower(strings.TrimSpace(name)) +} + +// lookupFormat returns the registered list formatter for name. +func lookupFormat(name string) (FormatterFunc, bool) { + formatRegistryMu.RLock() + defer formatRegistryMu.RUnlock() + fn, ok := formatRegistry[normalizeFormatName(name)] + return fn, ok +} + +func lookupSingleFormat(name string) (SingleFormatterFunc, bool) { + formatRegistryMu.RLock() + defer formatRegistryMu.RUnlock() + fn, ok := singleRegistry[normalizeFormatName(name)] + return fn, ok +} + // Printer handles formatted output. type Printer struct { Format Format @@ -29,13 +117,24 @@ type Printer struct { } // NewPrinter creates a new Printer with the given format. +// Falls back to FormatTable if the name is empty, unknown, or a misspelt +// built-in. Custom formats registered via RegisterFormat are recognised. func NewPrinter(format string, quiet, noColor bool) *Printer { - f := FormatTable - switch strings.ToLower(format) { + name := normalizeFormatName(format) + var f Format + switch name { + case "", "table": + f = FormatTable case "json": f = FormatJSON case "yaml": f = FormatYAML + default: + if _, ok := lookupFormat(name); ok { + f = Format(name) + } else { + f = FormatTable + } } return &Printer{ Format: f, @@ -125,7 +224,12 @@ func (p *Printer) Print(data interface{}, headers []string, rows [][]string, ids return p.PrintJSON(data) case FormatYAML: return p.PrintYAML(data) + case FormatTable: + return p.PrintTable(headers, rows) default: + if fn, ok := lookupFormat(string(p.Format)); ok { + return fn(p.Writer, data, headers, rows) + } return p.PrintTable(headers, rows) } } @@ -138,7 +242,25 @@ func (p *Printer) PrintSingle(data interface{}, fields []KeyValue) error { return p.PrintJSON(data) case FormatYAML: return p.PrintYAML(data) + case FormatTable: + w := p.TableWriter() + for _, f := range fields { + fmt.Fprintf(w, "%s:\t%s\n", f.Key, f.Value) + } + return w.Flush() default: + // Custom single formatter wins; otherwise adapt the list formatter by + // synthesising a two-column (field, value) table. + if fn, ok := lookupSingleFormat(string(p.Format)); ok { + return fn(p.Writer, data, fields) + } + if fn, ok := lookupFormat(string(p.Format)); ok { + rows := make([][]string, len(fields)) + for i, f := range fields { + rows[i] = []string{f.Key, f.Value} + } + return fn(p.Writer, data, []string{"field", "value"}, rows) + } w := p.TableWriter() for _, f := range fields { fmt.Fprintf(w, "%s:\t%s\n", f.Key, f.Value)