From c3961501c32ff6955ef7b9921df437ef32a19d18 Mon Sep 17 00:00:00 2001 From: Olof Mattsson <14146601+omattsson@users.noreply.github.com> Date: Mon, 19 Oct 2026 12:05:23 +0200 Subject: [PATCH] fix(output): inline YAML of embedded structs, clone -o json, colors only on a terminal, chart rows Found in a live test of the new commands. - -o yaml put id, created_at, updated_at, deleted_at and version under "base:" (yaml.v3 does not inline an embedded struct without a tag). Embedded Base, StackTemplate and TemplateVersion now carry yaml:",inline", so YAML has the same top-level keys as JSON. Note: a script that reads .base.id from YAML must read .id now. - stack clone ignored -o json and -o yaml (it printed text). It now prints the instance in every format except table, as stack create does. - Status colors went to pipes and files. stackctl now prints colors only when stdout is a terminal and NO_COLOR is not set. A non-empty NO_COLOR also sets STACKCTL_NO_COLOR=1 for plugins. - The chart output (add-chart, update-chart) showed empty Name and Release Name rows: the server sends no such fields. It now shows the chart path, the deploy order and Required. Co-Authored-By: Claude Opus 5.5 --- .github/copilot-instructions.md | 2 +- CLAUDE.md | 3 +- EXTENDING.md | 2 +- README.md | 4 +- cli/cmd/definition.go | 13 ++-- cli/cmd/output_fixes_test.go | 106 ++++++++++++++++++++++++++++++++ cli/cmd/plugins.go | 3 + cli/cmd/plugins_test.go | 20 ++++++ cli/cmd/root.go | 18 +++++- cli/cmd/stack.go | 8 ++- cli/pkg/types/types.go | 22 +++---- cli/pkg/types/types_test.go | 45 ++++++++++++++ 12 files changed, 221 insertions(+), 25 deletions(-) create mode 100644 cli/cmd/output_fixes_test.go diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 5da7dba..36e86c4 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -84,7 +84,7 @@ When reviewing PRs for this project, check: ### Global Flags - `--output`, `-o` — table (default), json, yaml - `--quiet`, `-q` — IDs only, one per line -- `--no-color` — disable colored output +- `--no-color` — disable colored output (also off with `NO_COLOR` or when stdout is not a terminal) - `--api-url` — override API server URL - `--api-key` — override API key - `--insecure` — skip TLS verification (prints warning) diff --git a/CLAUDE.md b/CLAUDE.md index a1e3d47..003eec7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -101,7 +101,8 @@ cli/ # Go module root (github.com/omattsson/stackctl/cl **Global flags**: `--output`/`-o table|json|yaml`, `--quiet`/`-q`, `--api-url`, `--api-key`, `--no-color`, `--insecure`, `--debug` (or `STACKCTL_DEBUG=1`; logs HTTP requests and responses to stderr) **Output modes**: -- `table` (default): human-readable with colored status badges +- `table` (default): human-readable with colored status badges. Colors go only to a terminal: `--no-color`, `NO_COLOR` (non-empty) or a stdout that is not a terminal turn them off (`root.go`: `noColorEnv`, `isTerminalWriter`). +- Embedded structs in `pkg/types` (`Base`, `StackTemplate`, `TemplateVersion`) carry `yaml:",inline"`, so `-o yaml` has the same top-level keys as `-o json`. Add the tag to every new embedded struct. - `json`: machine-readable, full API response - `yaml`: machine-readable, full API response - `--quiet`: IDs only, one per line (pipeable to `xargs`) diff --git a/EXTENDING.md b/EXTENDING.md index 648d6ab..62dde18 100644 --- a/EXTENDING.md +++ b/EXTENDING.md @@ -121,7 +121,7 @@ that the user exported in the parent shell is kept. | `STACKCTL_INSECURE` | `--insecure` flag, parent shell, or current context `insecure: true` | `1` to skip TLS verification. stackctl exports `0` when the user sets `--insecure=false`. | | `STACKCTL_QUIET` | `--quiet` flag | `1` when the user requested quiet output (`0` for `--quiet=false`) | | `STACKCTL_OUTPUT` | `--output` flag | `table` / `json` / `yaml` / a registered custom format | -| `STACKCTL_NO_COLOR` | `--no-color` flag | `1` when the user disabled colored output (`0` for `--no-color=false`) | +| `STACKCTL_NO_COLOR` | `--no-color` flag, or a non-empty `NO_COLOR` | `1` when the user disabled colored output (`0` for `--no-color=false`). stackctl itself also prints no colors when its stdout is not a terminal; a plugin checks its own stdout the same way. | | `NO_COLOR` | `--no-color` flag, or parent shell | `1` with `--no-color` ([no-color.org](https://no-color.org) convention for the tools that the plugin runs) | | `STACKCTL_DEBUG` | `--debug` flag | `1` when the user requested HTTP debug output (`0` for `--debug=false`) | | `STACKCTL_CONFIG_DIR` | parent shell, or the config directory that stackctl uses | Directory of `config.yaml` and `tokens/`. A plugin that runs `stackctl` again uses the same files | diff --git a/README.md b/README.md index 2ec2467..5ff8ac1 100644 --- a/README.md +++ b/README.md @@ -658,7 +658,7 @@ These groups have no section above. Run `stackctl --help` for the flags. Most commands support multiple output formats via the `--output` flag: ```bash -# Table (default) — human-readable with colored status badges +# Table (default) — human-readable with colored status badges (colors only on a terminal) stackctl stack list # JSON — machine-readable, full API response @@ -679,7 +679,7 @@ stackctl stack list -q |------|-------|-------------| | `--output` | `-o` | Output format: `table`, `json`, `yaml` (default `table`) | | `--quiet` | `-q` | Output only IDs (one per line) | -| `--no-color` | | Disable colored output | +| `--no-color` | | Disable colored output. stackctl also prints no colors when `NO_COLOR` is set or when stdout is not a terminal (a pipe or a file). | | `--api-url` | | Override API server URL | | `--api-key` | | Override API key | | `--insecure` | | Skip TLS certificate verification | diff --git a/cli/cmd/definition.go b/cli/cmd/definition.go index b23eb3a..449a400 100644 --- a/cli/cmd/definition.go +++ b/cli/cmd/definition.go @@ -545,13 +545,18 @@ func printChartConfig(ch *types.ChartConfig) error { case output.FormatYAML: return printer.PrintYAML(ch) default: + // The server sends no "name" or "release_name" for a chart: the + // table shows the fields that it sends. fields := []output.KeyValue{ {Key: "ID", Value: ch.ID}, - {Key: "Name", Value: ch.Name}, {Key: "Chart", Value: ch.ChartName}, - {Key: "Repository", Value: ch.RepoURL}, - {Key: "Version", Value: ch.ChartVersion}, - {Key: "Release Name", Value: ch.ReleaseName}, + {Key: "Repository", Value: valueOrDash(ch.RepoURL)}, + {Key: "Chart Path", Value: valueOrDash(ch.ChartPath)}, + {Key: "Version", Value: valueOrDash(ch.ChartVersion)}, + {Key: "Deploy Order", Value: strconv.Itoa(ch.DeployOrder)}, + } + if ch.Required { + fields = append(fields, output.KeyValue{Key: "Required", Value: "yes"}) } return printer.PrintSingle(ch, fields) } diff --git a/cli/cmd/output_fixes_test.go b/cli/cmd/output_fixes_test.go new file mode 100644 index 0000000..6c52f36 --- /dev/null +++ b/cli/cmd/output_fixes_test.go @@ -0,0 +1,106 @@ +package cmd + +import ( + "bytes" + "encoding/json" + "net/http" + "os" + "testing" + + "github.com/omattsson/stackctl/cli/pkg/output" + "github.com/omattsson/stackctl/cli/pkg/types" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "gopkg.in/yaml.v3" +) + +func TestStackCloneCmd_OutputModes(t *testing.T) { + tests := []struct { + name string + format output.Format + quiet bool + check func(t *testing.T, out string) + }{ + {"json", output.FormatJSON, false, func(t *testing.T, out string) { + var inst types.StackInstance + require.NoError(t, json.Unmarshal([]byte(out), &inst), "stdout is JSON") + assert.Equal(t, "55", inst.ID) + assert.Equal(t, "my-stack-copy", inst.Name) + }}, + {"yaml", output.FormatYAML, false, func(t *testing.T, out string) { + var m map[string]any + require.NoError(t, yaml.Unmarshal([]byte(out), &m)) + assert.Equal(t, "55", m["id"], "id is a top-level key") + }}, + {"quiet", output.FormatTable, true, func(t *testing.T, out string) { + assert.Equal(t, "55\n", out) + }}, + {"table", output.FormatTable, false, func(t *testing.T, out string) { + assert.Contains(t, out, "Cloned stack 42") + assert.Contains(t, out, "new stack 55") + }}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, out := setupChannelCmd(t, stackCloneCmd, map[string]route{ + "POST /api/v1/stack-instances/42/clone": {http.StatusCreated, `{"id":"55","name":"my-stack-copy","status":"draft"}`}, + }) + printer.Format = tt.format + printer.Quiet = tt.quiet + require.NoError(t, stackCloneCmd.RunE(stackCloneCmd, []string{"42"})) + stdout, _ := out() + tt.check(t, stdout) + }) + } +} + +func TestPrintChartConfig_ShowsServerFields(t *testing.T) { + var buf bytes.Buffer + prev := printer + printer = output.NewPrinter("table", false, true) + printer.Writer = &buf + t.Cleanup(func() { printer = prev }) + + require.NoError(t, printChartConfig(&types.ChartConfig{ + Base: types.Base{ID: "c1"}, ChartName: "api", ChartPath: "/charts/api", ChartVersion: "1.2.0", DeployOrder: 3, Required: true, + })) + out := buf.String() + assert.Regexp(t, `Chart:\s+api`, out) + assert.Regexp(t, `Repository:\s+-`, out) + assert.Regexp(t, `Chart Path:\s+/charts/api`, out) + assert.Regexp(t, `Deploy Order:\s+3`, out) + assert.Regexp(t, `Required:\s+yes`, out) + assert.NotContains(t, out, "Release Name", "the server sends no release name") + assert.NotRegexp(t, `(?m)^Name:`, out, "the server sends no chart name field other than chart_name") + + buf.Reset() + require.NoError(t, printChartConfig(&types.ChartConfig{Base: types.Base{ID: "c2"}, ChartName: "db"})) + assert.Regexp(t, `Deploy Order:\s+0`, buf.String()) + assert.NotContains(t, buf.String(), "Required", "no Required row for an optional chart") +} + +// TestPrinterColor_OnlyOnATerminal: a pipe or a file gets no ANSI colors. +// Tests run without a terminal, so os.Stdout and a buffer are both not a +// terminal here. +func TestPrinterColor_OnlyOnATerminal(t *testing.T) { + assert.False(t, isTerminalWriter(&bytes.Buffer{})) + f, err := os.CreateTemp(t.TempDir(), "out") + require.NoError(t, err) + t.Cleanup(func() { _ = f.Close() }) + assert.False(t, isTerminalWriter(f), "a regular file is not a terminal") + + t.Setenv("NO_COLOR", "1") + assert.True(t, noColorEnv()) + t.Setenv("NO_COLOR", "") + assert.False(t, noColorEnv(), "an empty NO_COLOR does not count (no-color.org)") + + // PersistentPreRunE builds the printer: with a buffer as stdout the + // status text has no escape codes. + prev := printer + t.Cleanup(func() { printer = prev; versionCmd.SetOut(nil) }) + var buf bytes.Buffer + versionCmd.SetOut(&buf) + require.NoError(t, rootCmd.PersistentPreRunE(versionCmd, nil)) + assert.True(t, printer.NoColor) + assert.Equal(t, "running", printer.StatusColor("running")) +} diff --git a/cli/cmd/plugins.go b/cli/cmd/plugins.go index 697cf0b..adf4303 100644 --- a/cli/cmd/plugins.go +++ b/cli/cmd/plugins.go @@ -177,6 +177,9 @@ func pluginEnv(cmd *cobra.Command) []string { env = setEnv(env, "NO_COLOR", "1") } } + } else if noColorEnv() { + // NO_COLOR also turns off the colors of stackctl itself. + env = setEnv(env, "STACKCTL_NO_COLOR", "1") } if !hasEnv(env, "STACKCTL_CONFIG_DIR") { // The directory stackctl itself uses, so a plugin that reads the diff --git a/cli/cmd/plugins_test.go b/cli/cmd/plugins_test.go index 396c57d..2ff6e01 100644 --- a/cli/cmd/plugins_test.go +++ b/cli/cmd/plugins_test.go @@ -432,6 +432,26 @@ func TestPluginEnv_NoColorAndConfigDir(t *testing.T) { assert.Equal(t, "1", v) } +// TestPluginEnv_NoColorFromEnv: a non-empty NO_COLOR also sets +// STACKCTL_NO_COLOR, because stackctl itself then prints no colors. +func TestPluginEnv_NoColorFromEnv(t *testing.T) { + t.Setenv("STACKCTL_CONFIG_DIR", t.TempDir()) + t.Setenv("STACKCTL_NO_COLOR", "") + require.NoError(t, os.Unsetenv("STACKCTL_NO_COLOR")) + t.Setenv("NO_COLOR", "1") + + root := &cobra.Command{Use: "stackctl"} + root.PersistentFlags().Bool("no-color", false, "") + + v, _ := envValue(pluginEnv(root), "STACKCTL_NO_COLOR") + assert.Equal(t, "1", v) + + // An explicit --no-color=false wins over NO_COLOR. + require.NoError(t, root.PersistentFlags().Set("no-color", "false")) + v, _ = envValue(pluginEnv(root), "STACKCTL_NO_COLOR") + assert.Equal(t, "0", v) +} + func TestPluginEnv_ConfigDirFromDefault(t *testing.T) { xdg := t.TempDir() t.Setenv("STACKCTL_CONFIG_DIR", "") diff --git a/cli/cmd/root.go b/cli/cmd/root.go index 31c4562..821b166 100644 --- a/cli/cmd/root.go +++ b/cli/cmd/root.go @@ -13,6 +13,7 @@ import ( "github.com/omattsson/stackctl/cli/pkg/config" "github.com/omattsson/stackctl/cli/pkg/output" "github.com/spf13/cobra" + "golang.org/x/term" ) var ( @@ -45,8 +46,10 @@ Get started: SilenceUsage: true, SilenceErrors: true, PersistentPreRunE: func(cmd *cobra.Command, args []string) error { - // Initialize printer with Cobra's configured output writer - printer = output.NewPrinter(flagOutput, flagQuiet, flagNoColor) + // Initialize printer with Cobra's configured output writer. Colors + // only go to a terminal: not with --no-color, NO_COLOR + // (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() // Skip config loading for commands that should work without a config file @@ -261,6 +264,17 @@ func (e *configError) Error() string { return e.msg } +// noColorEnv reports whether NO_COLOR is set to a non-empty value. +func noColorEnv() bool { + return os.Getenv("NO_COLOR") != "" +} + +// isTerminalWriter reports whether w is a terminal. +func isTerminalWriter(w io.Writer) bool { + f, ok := w.(*os.File) + 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. func confirmAction(cmd *cobra.Command, message string) (bool, error) { diff --git a/cli/cmd/stack.go b/cli/cmd/stack.go index f6f5f8b..dc32480 100644 --- a/cli/cmd/stack.go +++ b/cli/cmd/stack.go @@ -1017,10 +1017,12 @@ Examples: } printServerWarning(cmd, instance.Warning) - if printer.Quiet { - fmt.Fprintln(printer.Writer, instance.ID) - } else { + // Table: a short message. Quiet, JSON, YAML and custom formats: + // the instance, as stack create prints it. + if printer.Format == output.FormatTable && !printer.Quiet { printer.PrintMessage("Cloned stack %s → new stack %s", id, instance.ID) + } else if err := printInstance(instance); err != nil { + return err } return checkStopApplied("stop-at", expiry.stopAt, instance) }, diff --git a/cli/pkg/types/types.go b/cli/pkg/types/types.go index 26134be..85cd130 100644 --- a/cli/pkg/types/types.go +++ b/cli/pkg/types/types.go @@ -27,7 +27,7 @@ type Base struct { // quota override that is not copied). "stack get" sets Warning to // ValuesDriftWarning when ValuesDrift is true and the server sends no text. type StackInstance struct { - Base + Base `yaml:",inline"` Name string `json:"name" yaml:"name"` StackDefinitionID string `json:"stack_definition_id" yaml:"stack_definition_id"` DefinitionName string `json:"definition_name,omitempty" yaml:"definition_name,omitempty"` @@ -231,7 +231,7 @@ const ValuesDriftWarning = "The stored overrides differ from the running values. // StackDefinition represents a stack definition with its chart configurations. type StackDefinition struct { - Base + Base `yaml:",inline"` Name string `json:"name" yaml:"name"` Description string `json:"description,omitempty" yaml:"description,omitempty"` DefaultBranch string `json:"default_branch" yaml:"default_branch"` @@ -282,7 +282,7 @@ type TemplateLinkRequest struct { // template has no release) and HasUnpublishedChanges. Older servers do not // send these fields; HasUnpublishedChanges is then nil. type StackTemplate struct { - Base + Base `yaml:",inline"` Name string `json:"name" yaml:"name"` Description string `json:"description,omitempty" yaml:"description,omitempty"` Category string `json:"category,omitempty" yaml:"category,omitempty"` @@ -311,7 +311,7 @@ type PublishTemplateRequest struct { // when the working copy equals the latest release (no new version). Older // servers return only the template; SnapshotCreated is then nil. type PublishTemplateResponse struct { - StackTemplate + StackTemplate `yaml:",inline"` SnapshotCreated *bool `json:"snapshot_created,omitempty" yaml:"snapshot_created,omitempty"` } @@ -320,7 +320,7 @@ type PublishTemplateResponse struct { // charts) and models.TemplateChartConfig (template charts) — LockedValues and // Required are populated only on template-chart responses. type ChartConfig struct { - Base + Base `yaml:",inline"` StackDefinitionID string `json:"stack_definition_id,omitempty" yaml:"stack_definition_id,omitempty"` Name string `json:"name" yaml:"name"` RepoURL string `json:"repository_url" yaml:"repository_url"` @@ -413,7 +413,7 @@ type UpdateClusterRequest struct { // - ServiceAccount — when true, this is a non-interactive identity used // by CI/automation; only admins can create these. type User struct { - Base + Base `yaml:",inline"` Username string `json:"username" yaml:"username"` DisplayName string `json:"display_name,omitempty" yaml:"display_name,omitempty"` Email string `json:"email,omitempty" yaml:"email,omitempty"` @@ -1219,7 +1219,7 @@ type NamespaceResourceUsage struct { // - PodLimit — 0 means "no pod-count limit"; positive integers cap namespace // pod count. type ClusterQuota struct { - Base + Base `yaml:",inline"` ClusterID string `json:"cluster_id" yaml:"cluster_id"` CPURequest string `json:"cpu_request,omitempty" yaml:"cpu_request,omitempty"` CPULimit string `json:"cpu_limit,omitempty" yaml:"cpu_limit,omitempty"` @@ -1405,7 +1405,7 @@ type SetQuotaOverrideRequest struct { // SharedValues represents cluster-level shared Helm values. type SharedValues struct { - Base + Base `yaml:",inline"` ClusterID string `json:"cluster_id" yaml:"cluster_id"` Name string `json:"name" yaml:"name"` Description string `json:"description,omitempty" yaml:"description,omitempty"` @@ -1658,8 +1658,8 @@ func (v TemplateVersion) CreatedByName() string { // TemplateVersionDetail is the full version response including the parsed snapshot. type TemplateVersionDetail struct { - TemplateVersion - Snapshot TemplateSnapshot `json:"snapshot" yaml:"snapshot"` + TemplateVersion `yaml:",inline"` + Snapshot TemplateSnapshot `json:"snapshot" yaml:"snapshot"` } // TemplateSnapshot is the state of a template captured at publish time. @@ -1741,7 +1741,7 @@ const TemplateVersionWorkingCopy = "working" // - LastRunAt — nil until the policy has been executed (manually or by the // scheduler) at least once. type CleanupPolicy struct { - Base + Base `yaml:",inline"` Name string `json:"name" yaml:"name"` ClusterID string `json:"cluster_id" yaml:"cluster_id"` Action string `json:"action" yaml:"action"` diff --git a/cli/pkg/types/types_test.go b/cli/pkg/types/types_test.go index f6d3bb9..54d6388 100644 --- a/cli/pkg/types/types_test.go +++ b/cli/pkg/types/types_test.go @@ -7,6 +7,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "gopkg.in/yaml.v3" ) func TestStackInstance_JSONRoundTrip(t *testing.T) { @@ -307,3 +308,47 @@ func TestUpdateTemplateRequest_JSON(t *testing.T) { require.NoError(t, err) assert.JSONEq(t, `{"name":"web","version":"1.1.0","default_branch":"main"}`, string(data)) } + +// TestYAML_EmbeddedStructsAreInline: -o yaml must put the fields of an +// embedded struct (Base, StackTemplate, TemplateVersion) at the top level, +// as JSON does, not under "base:". +func TestYAML_EmbeddedStructsAreInline(t *testing.T) { + t.Parallel() + base := Base{ID: "id-1"} + tests := []struct { + name string + v any + }{ + {"StackInstance", StackInstance{Base: base, Name: "s"}}, + {"StackDefinition", StackDefinition{Base: base}}, + {"StackTemplate", StackTemplate{Base: base}}, + {"PublishTemplateResponse", PublishTemplateResponse{StackTemplate: StackTemplate{Base: base}}}, + {"ChartConfig", ChartConfig{Base: base}}, + {"User", User{Base: base}}, + {"ClusterQuota", ClusterQuota{Base: base}}, + {"SharedValues", SharedValues{Base: base}}, + {"TemplateVersionDetail", TemplateVersionDetail{TemplateVersion: TemplateVersion{ID: "id-1"}}}, + {"CleanupPolicy", CleanupPolicy{Base: base}}, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + out, err := yaml.Marshal(tt.v) + require.NoError(t, err) + var m map[string]any + require.NoError(t, yaml.Unmarshal(out, &m)) + assert.Equal(t, "id-1", m["id"], "id is a top-level key:\n%s", out) + for _, nested := range []string{"base", "stacktemplate", "templateversion"} { + assert.NotContains(t, m, nested) + } + + // JSON keeps the same top-level id. + data, err := json.Marshal(tt.v) + require.NoError(t, err) + var j map[string]any + require.NoError(t, json.Unmarshal(data, &j)) + assert.Equal(t, "id-1", j["id"]) + }) + } +}