From 916a70720efff874f67ba902cb027e0393429fb4 Mon Sep 17 00:00:00 2001 From: Nathan Cochran Date: Mon, 5 Oct 2026 15:35:48 -0400 Subject: [PATCH 1/2] Add --all flag to 'tiger config list' --- CLAUDE.md | 1 + docs/development.md | 3 + internal/cmd/auth_login.go | 2 +- internal/cmd/completion_helper.go | 4 +- internal/cmd/completion_helper_test.go | 9 +- internal/cmd/config_list.go | 9 +- internal/cmd/config_list_test.go | 405 +++++++++++++++---------- internal/config/config.go | 57 +++- internal/config/config_keys_test.go | 9 + 9 files changed, 316 insertions(+), 183 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 8355e70e..f707b82a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -102,6 +102,7 @@ Configuration is layered, with precedence **flags > `TIGER_*` env vars > config - **Pass what you read down the call chain.** Hand the `*config.Config` (plus client and project ID where needed) to the functions that need them rather than reloading. Don't pass the App into `internal/common` helpers — they take the specific values they use, which keeps them usable from both CLI and MCP. - **The App owns flag precedence.** `config.Load` binds the flags listed in `flagBindings` (`internal/config/config.go`) against the flag set it's given, so a flag bound for one command never leaks into another, and a command that doesn't define a flag simply skips it. - **Flags that override config values** must be added to `flagBindings` and declared *without* a bound variable (`cmd.Flags().String(...)`, or `Var(new(outputFlag), ...)` for validating flag types), so the only way to read the value is through the config. A variable in scope is an invitation to read the raw flag instead, silently bypassing the env var and config file. Flags that aren't config values stay plain local variables; one that only needs an env-var fallback can read the env var directly (see `--new-password` in `service_update_password.go`). +- **Internal config keys are private.** Keys in `privateDefaultValues` (`internal/config/config.go`) — the endpoint URLs — are settable like any other but hidden from `tiger config list`, wherever their value comes from, unless the hidden `--all` flag is given, and never offered by shell completion. Public keys go in `publicDefaultValues`. - **Config-derived state lives on the `Config` too.** Credential storage is a set of methods on it (`cfg.StoreCredentials`, `cfg.GetStoredCredentials`, `cfg.RemoveCredentials`), keyed off `cfg.ConfigDir`, and `cfg.Set`/`Unset`/`Reset` write the config file and then reload the struct **in place** — which is how a command's own config change (e.g. `tiger config set analytics false`) becomes visible to the rest of its invocation. `Set` also returns the value as stored, which isn't always its argument (`read_only` normalizes its legacy spellings) — echo the return value to the user, not the input. - **The active project lives in the stored credentials, not the config file.** Switching projects (`tiger project use`) therefore requires an OAuth login — an API key is scoped to one project — and any project change clears a stale default `service_id` (see `clearStaleDefaultService` in `project_helper.go`, which `auth login` calls too). - **MCP reloads per request.** The analytics middleware in `internal/mcp/server.go` calls `s.app.Load(ctx)` on every request, so config changes and logins/logouts take effect on the next tool call without restarting the server. Tool handlers read that state and must not load it again. diff --git a/docs/development.md b/docs/development.md index 228828d6..16dd2470 100644 --- a/docs/development.md +++ b/docs/development.md @@ -39,6 +39,9 @@ There are a handful of configuration options and environment variables that are - `console_url` (`TIGER_CONSOLE_URL`) - Tiger Cloud Console URL (default: https://console.cloud.tigerdata.com) - `gateway_url` (`TIGER_GATEWAY_URL`) - Tiger Cloud Gateway URL (default: https://console.cloud.tigerdata.com/api) - `docs_mcp_url` (`TIGER_DOCS_MCP_URL`) - Docs MCP server URL (default: https://mcp.tigerdata.com/docs) +- `releases_url` (`TIGER_RELEASES_URL`) - Release CDN used by `tiger upgrade` and the version check (default: https://cli.tigerdata.com) + +These are internal options: `tiger config list` and shell completion leave them out. Pass the hidden `--all` (`-a`) flag to `tiger config list` to include them. ## Running Tests diff --git a/internal/cmd/auth_login.go b/internal/cmd/auth_login.go index b086f249..e2e047de 100644 --- a/internal/cmd/auth_login.go +++ b/internal/cmd/auth_login.go @@ -235,7 +235,7 @@ func offerProdProtection(cmd *cobra.Command, cfg *config.Config) bool { return true } - stored, err := config.LoadForOutput(cfg.ConfigDir, false, true) + stored, err := config.LoadForOutput(cfg.ConfigDir, config.OutputOptions{NoDefaults: true}) if err != nil { // Can't tell whether it was ever answered, so don't ask — but do print the // bullet, which is the harmless half of the two. diff --git a/internal/cmd/completion_helper.go b/internal/cmd/completion_helper.go index d9cdcc58..570643e9 100644 --- a/internal/cmd/completion_helper.go +++ b/internal/cmd/completion_helper.go @@ -163,7 +163,7 @@ func configKeyCompletion(cmd *cobra.Command, args []string, toComplete string) ( if len(args) > 0 { return nil, cobra.ShellCompDirectiveNoFileComp } - return filterCompletionsByPrefix(config.ValidConfigOptions(), toComplete), cobra.ShellCompDirectiveNoFileComp + return filterCompletionsByPrefix(config.PublicConfigOptions(), toComplete), cobra.ShellCompDirectiveNoFileComp } // configKeyValueCompletion completes the and arguments of @@ -172,7 +172,7 @@ func configKeyValueCompletion(cmd *cobra.Command, args []string, toComplete stri switch len(args) { case 0: // Completing the key - return filterCompletionsByPrefix(config.ValidConfigOptions(), toComplete), cobra.ShellCompDirectiveNoFileComp + return filterCompletionsByPrefix(config.PublicConfigOptions(), toComplete), cobra.ShellCompDirectiveNoFileComp case 1: // Completing the value, based on the key already typed return filterCompletionsByPrefix(config.ValidConfigOptionValues(args[0]), toComplete), cobra.ShellCompDirectiveNoFileComp diff --git a/internal/cmd/completion_helper_test.go b/internal/cmd/completion_helper_test.go index 918b46ca..876ae68c 100644 --- a/internal/cmd/completion_helper_test.go +++ b/internal/cmd/completion_helper_test.go @@ -156,9 +156,10 @@ func TestCompletion(t *testing.T) { { // Config keys come straight from the registry, so this list is the // same one `tiger config list` renders (see TestConfigListCoversEveryKey). - name: "config set completes keys", + // Private keys are left out. + name: "config set completes public keys", args: []string{"__complete", "config", "set", ""}, - wantStdout: "analytics\napi_url\ncolor\nconsole_url\ndocs_mcp\ndocs_mcp_url\ngateway_url\nmcp_max_rows\noutput\npassword_storage\nread_only\nreleases_url\nservice_id\nversion_check\n" + noFileComp, + wantStdout: "analytics\ncolor\ndocs_mcp\nmcp_max_rows\noutput\npassword_storage\nread_only\nservice_id\nversion_check\n" + noFileComp, wantStderr: directive, checks: []checkFunc{checkNotLoaded}, }, @@ -178,9 +179,9 @@ func TestCompletion(t *testing.T) { checks: []checkFunc{checkNotLoaded}, }, { - name: "config unset completes keys", + name: "config unset completes public keys", args: []string{"__complete", "config", "unset", "docs"}, - wantStdout: "docs_mcp\ndocs_mcp_url\n" + noFileComp, + wantStdout: "docs_mcp\n" + noFileComp, wantStderr: directive, checks: []checkFunc{checkNotLoaded}, }, diff --git a/internal/cmd/config_list.go b/internal/cmd/config_list.go index 3840855e..2af1f3ae 100644 --- a/internal/cmd/config_list.go +++ b/internal/cmd/config_list.go @@ -15,6 +15,7 @@ import ( func buildConfigListCmd(app *common.App) *cobra.Command { var noDefaults bool var withEnv bool + var all bool cmd := &cobra.Command{ Use: "list", @@ -29,7 +30,11 @@ func buildConfigListCmd(app *common.App) *cobra.Command { // Values are re-read free of env and CLI flags (unless --with-env // is given), so `config list -o json` reports the configured // `output` value rather than the flag's. - cfgOut, err := config.LoadForOutput(cfg.ConfigDir, withEnv, noDefaults) + cfgOut, err := config.LoadForOutput(cfg.ConfigDir, config.OutputOptions{ + WithEnv: withEnv, + NoDefaults: noDefaults, + All: all, + }) if err != nil { return err } @@ -50,6 +55,8 @@ func buildConfigListCmd(app *common.App) *cobra.Command { registerFlagCompletion(cmd, "output", outputCompletion()) cmd.Flags().BoolVar(&noDefaults, "no-defaults", false, "do not show default values for unset fields") cmd.Flags().BoolVar(&withEnv, "with-env", false, "apply environment variable overrides") + cmd.Flags().BoolVarP(&all, "all", "a", false, "include internal config values") + markFlagHidden(cmd, "all") return cmd } diff --git a/internal/cmd/config_list_test.go b/internal/cmd/config_list_test.go index d3fcac09..4d211f86 100644 --- a/internal/cmd/config_list_test.go +++ b/internal/cmd/config_list_test.go @@ -12,61 +12,66 @@ import ( "github.com/timescale/tiger-cli/internal/config" ) -// Full `config list` output for an all-defaults config, per format. Cases -// whose output differs use their own literals. +// Full `config list` output for an all-defaults config, per format, without +// and with --all. Cases whose output differs use their own literals. const ( - configListDefaultsTable = `┌──────────────────┬───────────────────────────────────────────────────────────────┐ -│ PROPERTY │ VALUE │ -├──────────────────┼───────────────────────────────────────────────────────────────┤ -│ analytics │ true │ -│ api_url │ https://console.cloud.tigerdata.com/public/api/v1 │ -│ color │ true │ -│ console_url │ https://console.cloud.tigerdata.com │ -│ docs_mcp │ true │ -│ docs_mcp_url │ https://mcp.tigerdata.com/docs?disabled_skills=ghost-database │ -│ gateway_url │ https://console.cloud.tigerdata.com/api │ -│ mcp_max_rows │ 100 │ -│ output │ table │ -│ password_storage │ keyring │ -│ read_only │ off │ -│ releases_url │ https://cli.tigerdata.com │ -│ service_id │ │ -│ version_check │ true │ -└──────────────────┴───────────────────────────────────────────────────────────────┘ + configListDefaultsTable = `┌──────────────────┬─────────┐ +│ PROPERTY │ VALUE │ +├──────────────────┼─────────┤ +│ analytics │ true │ +│ color │ true │ +│ docs_mcp │ true │ +│ mcp_max_rows │ 100 │ +│ output │ table │ +│ password_storage │ keyring │ +│ read_only │ off │ +│ service_id │ │ +│ version_check │ true │ +└──────────────────┴─────────┘ ` configListDefaultsJSON = `{ "analytics": true, - "api_url": "https://console.cloud.tigerdata.com/public/api/v1", "color": true, - "console_url": "https://console.cloud.tigerdata.com", "docs_mcp": true, - "docs_mcp_url": "https://mcp.tigerdata.com/docs?disabled_skills=ghost-database", - "gateway_url": "https://console.cloud.tigerdata.com/api", "mcp_max_rows": 100, "output": "table", "password_storage": "keyring", "read_only": "off", - "releases_url": "https://cli.tigerdata.com", "service_id": "", "version_check": true } ` configListDefaultsYAML = `analytics: true -api_url: https://console.cloud.tigerdata.com/public/api/v1 color: true -console_url: https://console.cloud.tigerdata.com docs_mcp: true -docs_mcp_url: https://mcp.tigerdata.com/docs?disabled_skills=ghost-database -gateway_url: https://console.cloud.tigerdata.com/api mcp_max_rows: 100 output: table password_storage: keyring read_only: "off" -releases_url: https://cli.tigerdata.com service_id: "" version_check: true +` + + configListAllDefaultsTable = `┌──────────────────┬───────────────────────────────────────────────────────────────┐ +│ PROPERTY │ VALUE │ +├──────────────────┼───────────────────────────────────────────────────────────────┤ +│ analytics │ true │ +│ api_url │ https://console.cloud.tigerdata.com/public/api/v1 │ +│ color │ true │ +│ console_url │ https://console.cloud.tigerdata.com │ +│ docs_mcp │ true │ +│ docs_mcp_url │ https://mcp.tigerdata.com/docs?disabled_skills=ghost-database │ +│ gateway_url │ https://console.cloud.tigerdata.com/api │ +│ mcp_max_rows │ 100 │ +│ output │ table │ +│ password_storage │ keyring │ +│ read_only │ off │ +│ releases_url │ https://cli.tigerdata.com │ +│ service_id │ │ +│ version_check │ true │ +└──────────────────┴───────────────────────────────────────────────────────────────┘ ` ) @@ -74,7 +79,7 @@ func TestConfigListCmd(t *testing.T) { // A second config dir, pointed at by TIGER_CONFIG_DIR in one case to prove // the --config-dir flag takes precedence over the env var. envDir := t.TempDir() - writeConfigFile(t, envDir, map[string]any{"api_url": "https://env.api.com/v1"}) + writeConfigFile(t, envDir, map[string]any{"service_id": "env-dir-service"}) runCmdTests(t, []cmdTest{ { @@ -91,29 +96,23 @@ func TestConfigListCmd(t *testing.T) { name: "table output with configured values", args: []string{"config", "list"}, opts: []runOption{withConfig(map[string]any{ - "api_url": "https://test.api.com/v1", "service_id": "test-service", "analytics": false, "password_storage": "pgpass", })}, - wantStdout: `┌──────────────────┬───────────────────────────────────────────────────────────────┐ -│ PROPERTY │ VALUE │ -├──────────────────┼───────────────────────────────────────────────────────────────┤ -│ analytics │ false │ -│ api_url │ https://test.api.com/v1 │ -│ color │ true │ -│ console_url │ https://console.cloud.tigerdata.com │ -│ docs_mcp │ true │ -│ docs_mcp_url │ https://mcp.tigerdata.com/docs?disabled_skills=ghost-database │ -│ gateway_url │ https://console.cloud.tigerdata.com/api │ -│ mcp_max_rows │ 100 │ -│ output │ table │ -│ password_storage │ pgpass │ -│ read_only │ off │ -│ releases_url │ https://cli.tigerdata.com │ -│ service_id │ test-service │ -│ version_check │ true │ -└──────────────────┴───────────────────────────────────────────────────────────────┘ + wantStdout: `┌──────────────────┬──────────────┐ +│ PROPERTY │ VALUE │ +├──────────────────┼──────────────┤ +│ analytics │ false │ +│ color │ true │ +│ docs_mcp │ true │ +│ mcp_max_rows │ 100 │ +│ output │ table │ +│ password_storage │ pgpass │ +│ read_only │ off │ +│ service_id │ test-service │ +│ version_check │ true │ +└──────────────────┴──────────────┘ `, }, { @@ -121,22 +120,16 @@ func TestConfigListCmd(t *testing.T) { args: []string{"config", "list"}, opts: []runOption{withConfig(map[string]any{ "output": "json", - "api_url": "https://json.api.com/v1", "analytics": false, })}, wantStdout: `{ "analytics": false, - "api_url": "https://json.api.com/v1", "color": true, - "console_url": "https://console.cloud.tigerdata.com", "docs_mcp": true, - "docs_mcp_url": "https://mcp.tigerdata.com/docs?disabled_skills=ghost-database", - "gateway_url": "https://console.cloud.tigerdata.com/api", "mcp_max_rows": 100, "output": "json", "password_storage": "keyring", "read_only": "off", - "releases_url": "https://cli.tigerdata.com", "service_id": "", "version_check": true } @@ -160,22 +153,17 @@ func TestConfigListCmd(t *testing.T) { name: "yaml output from config file", args: []string{"config", "list"}, opts: []runOption{withConfig(map[string]any{ - "output": "yaml", - "api_url": "https://yaml.api.com/v1", + "output": "yaml", + "service_id": "yaml-service", })}, wantStdout: `analytics: true -api_url: https://yaml.api.com/v1 color: true -console_url: https://console.cloud.tigerdata.com docs_mcp: true -docs_mcp_url: https://mcp.tigerdata.com/docs?disabled_skills=ghost-database -gateway_url: https://console.cloud.tigerdata.com/api mcp_max_rows: 100 output: yaml password_storage: keyring read_only: "off" -releases_url: https://cli.tigerdata.com -service_id: "" +service_id: yaml-service version_check: true `, }, @@ -203,24 +191,19 @@ version_check: true name: "with-env applies env overrides", args: []string{"config", "list", "--with-env"}, opts: []runOption{withEnv("TIGER_SERVICE_ID", "env-service")}, - wantStdout: `┌──────────────────┬───────────────────────────────────────────────────────────────┐ -│ PROPERTY │ VALUE │ -├──────────────────┼───────────────────────────────────────────────────────────────┤ -│ analytics │ true │ -│ api_url │ https://console.cloud.tigerdata.com/public/api/v1 │ -│ color │ true │ -│ console_url │ https://console.cloud.tigerdata.com │ -│ docs_mcp │ true │ -│ docs_mcp_url │ https://mcp.tigerdata.com/docs?disabled_skills=ghost-database │ -│ gateway_url │ https://console.cloud.tigerdata.com/api │ -│ mcp_max_rows │ 100 │ -│ output │ table │ -│ password_storage │ keyring │ -│ read_only │ off │ -│ releases_url │ https://cli.tigerdata.com │ -│ service_id │ env-service │ -│ version_check │ true │ -└──────────────────┴───────────────────────────────────────────────────────────────┘ + wantStdout: `┌──────────────────┬─────────────┐ +│ PROPERTY │ VALUE │ +├──────────────────┼─────────────┤ +│ analytics │ true │ +│ color │ true │ +│ docs_mcp │ true │ +│ mcp_max_rows │ 100 │ +│ output │ table │ +│ password_storage │ keyring │ +│ read_only │ off │ +│ service_id │ env-service │ +│ version_check │ true │ +└──────────────────┴─────────────┘ `, }, { @@ -229,31 +212,115 @@ version_check: true opts: []runOption{withEnv("TIGER_SERVICE_ID", "env-service")}, wantStdout: configListDefaultsTable, }, + { + name: "private keys hidden even when set in config file", + args: []string{"config", "list", "--no-defaults"}, + opts: []runOption{withConfig(map[string]any{ + "api_url": "https://test.api.com/v1", + "console_url": "https://test.console.com", + "docs_mcp_url": "https://test.docs.com/mcp", + "gateway_url": "https://test.api.com/gw", + "releases_url": "https://test.releases.com", + "service_id": "test-service", + })}, + wantStdout: `┌────────────┬──────────────┐ +│ PROPERTY │ VALUE │ +├────────────┼──────────────┤ +│ service_id │ test-service │ +└────────────┴──────────────┘ +`, + }, + { + name: "private keys hidden even when set by env var", + args: []string{"config", "list", "--with-env"}, + opts: []runOption{withEnv("TIGER_API_URL", "https://env.api.com/v1")}, + wantStdout: configListDefaultsTable, + }, + { + name: "all includes private defaults", + args: []string{"config", "list", "--all"}, + wantStdout: configListAllDefaultsTable, + }, + { + name: "all shorthand", + args: []string{"config", "list", "-a"}, + wantStdout: configListAllDefaultsTable, + }, + { + name: "all includes private values from config file", + args: []string{"config", "list", "--all", "-o", "json"}, + opts: []runOption{withConfig(map[string]any{ + "api_url": "https://test.api.com/v1", + "console_url": "https://test.console.com", + "docs_mcp_url": "https://test.docs.com/mcp", + "gateway_url": "https://test.api.com/gw", + "releases_url": "https://test.releases.com", + })}, + wantStdout: `{ + "analytics": true, + "api_url": "https://test.api.com/v1", + "color": true, + "console_url": "https://test.console.com", + "docs_mcp": true, + "docs_mcp_url": "https://test.docs.com/mcp", + "gateway_url": "https://test.api.com/gw", + "mcp_max_rows": 100, + "output": "table", + "password_storage": "keyring", + "read_only": "off", + "releases_url": "https://test.releases.com", + "service_id": "", + "version_check": true +} +`, + }, + { + name: "all with no-defaults shows only configured values", + args: []string{"config", "list", "--all", "--no-defaults"}, + opts: []runOption{withConfig(map[string]any{ + "api_url": "https://test.api.com/v1", + "service_id": "test-service", + })}, + wantStdout: `┌────────────┬─────────────────────────┐ +│ PROPERTY │ VALUE │ +├────────────┼─────────────────────────┤ +│ api_url │ https://test.api.com/v1 │ +│ service_id │ test-service │ +└────────────┴─────────────────────────┘ +`, + }, + { + name: "all with with-env applies private env overrides", + args: []string{"config", "list", "--all", "--with-env", "-o", "yaml"}, + opts: []runOption{withEnv("TIGER_API_URL", "https://env.api.com/v1")}, + wantStdout: `analytics: true +api_url: https://env.api.com/v1 +color: true +console_url: https://console.cloud.tigerdata.com +docs_mcp: true +docs_mcp_url: https://mcp.tigerdata.com/docs?disabled_skills=ghost-database +gateway_url: https://console.cloud.tigerdata.com/api +mcp_max_rows: 100 +output: table +password_storage: keyring +read_only: "off" +releases_url: https://cli.tigerdata.com +service_id: "" +version_check: true +`, + }, { name: "config-dir flag overrides TIGER_CONFIG_DIR env var", - args: []string{"config", "list"}, + args: []string{"config", "list", "--no-defaults"}, opts: []runOption{ - withConfig(map[string]any{"api_url": "https://flag-test.api.com/v1"}), + withConfig(map[string]any{"service_id": "flag-dir-service"}), withEnv("TIGER_CONFIG_DIR", envDir), }, - wantStdout: `┌──────────────────┬───────────────────────────────────────────────────────────────┐ -│ PROPERTY │ VALUE │ -├──────────────────┼───────────────────────────────────────────────────────────────┤ -│ analytics │ true │ -│ api_url │ https://flag-test.api.com/v1 │ -│ color │ true │ -│ console_url │ https://console.cloud.tigerdata.com │ -│ docs_mcp │ true │ -│ docs_mcp_url │ https://mcp.tigerdata.com/docs?disabled_skills=ghost-database │ -│ gateway_url │ https://console.cloud.tigerdata.com/api │ -│ mcp_max_rows │ 100 │ -│ output │ table │ -│ password_storage │ keyring │ -│ read_only │ off │ -│ releases_url │ https://cli.tigerdata.com │ -│ service_id │ │ -│ version_check │ true │ -└──────────────────┴───────────────────────────────────────────────────────────────┘ + wantStdout: `┌────────────┬──────────────────┐ +│ PROPERTY │ VALUE │ +├────────────┼──────────────────┤ +│ service_id │ flag-dir-service │ +└────────────┴──────────────────┘ `, }, { @@ -263,24 +330,19 @@ version_check: true name: "legacy version_check_interval 0 disables version_check", args: []string{"config", "list"}, opts: []runOption{withConfig(map[string]any{"version_check_interval": 0})}, - wantStdout: `┌──────────────────┬───────────────────────────────────────────────────────────────┐ -│ PROPERTY │ VALUE │ -├──────────────────┼───────────────────────────────────────────────────────────────┤ -│ analytics │ true │ -│ api_url │ https://console.cloud.tigerdata.com/public/api/v1 │ -│ color │ true │ -│ console_url │ https://console.cloud.tigerdata.com │ -│ docs_mcp │ true │ -│ docs_mcp_url │ https://mcp.tigerdata.com/docs?disabled_skills=ghost-database │ -│ gateway_url │ https://console.cloud.tigerdata.com/api │ -│ mcp_max_rows │ 100 │ -│ output │ table │ -│ password_storage │ keyring │ -│ read_only │ off │ -│ releases_url │ https://cli.tigerdata.com │ -│ service_id │ │ -│ version_check │ false │ -└──────────────────┴───────────────────────────────────────────────────────────────┘ + wantStdout: `┌──────────────────┬─────────┐ +│ PROPERTY │ VALUE │ +├──────────────────┼─────────┤ +│ analytics │ true │ +│ color │ true │ +│ docs_mcp │ true │ +│ mcp_max_rows │ 100 │ +│ output │ table │ +│ password_storage │ keyring │ +│ read_only │ off │ +│ service_id │ │ +│ version_check │ false │ +└──────────────────┴─────────┘ `, }, { @@ -296,55 +358,66 @@ version_check: true }) } -// Every config key must be visible in every `config list` format. The table is -// rendered by a hand-written if-chain in outputTable and the other two by -// ConfigOutput's json/yaml tags, so a newly added key can silently fail to -// show up in one of them. The literal expectations above would still pass — -// they only assert the keys that were there when they were written — so assert -// the full key set against the registry instead. +// Every config key must be visible in every `config list` format: public keys +// by default, and every key with --all. The table is rendered by a +// hand-written if-chain in outputTable and the other two by ConfigOutput's +// json/yaml tags, so a newly added key can silently fail to show up in one of +// them. The literal expectations above would still pass — they only assert the +// keys that were there when they were written — so assert the full key set +// against the registry instead. func TestConfigListCoversEveryKey(t *testing.T) { - want := config.ValidConfigOptions() - slices.Sort(want) + for _, variant := range []struct { + name string + flags []string + want []string + }{ + {name: "public", want: config.PublicConfigOptions()}, + {name: "all", flags: []string{"--all"}, want: config.ValidConfigOptions()}, + } { + t.Run(variant.name, func(t *testing.T) { + args := append([]string{"config", "list"}, variant.flags...) - assertKeys := func(t *testing.T, got []string) { - t.Helper() - slices.Sort(got) - if !slices.Equal(want, got) { - t.Errorf("`config list` keys = %v, want %v", got, want) - } - } - - t.Run("table", func(t *testing.T) { - result := runCommand(t, []string{"config", "list"}, nil) - var got []string - for line := range strings.SplitSeq(result.stdout, "\n") { - cells := strings.Split(line, "\u2502") - // A data row is "│ key │ value │": empty, key, value, empty. - if len(cells) != 4 { - continue - } - if key := strings.TrimSpace(cells[1]); key != "" && key != "PROPERTY" { - got = append(got, key) + assertKeys := func(t *testing.T, got []string) { + t.Helper() + slices.Sort(got) + if !slices.Equal(variant.want, got) { + t.Errorf("`config list` keys = %v, want %v", got, variant.want) + } } - } - assertKeys(t, got) - }) - t.Run("json", func(t *testing.T) { - result := runCommand(t, []string{"config", "list", "-o", "json"}, nil) - var values map[string]any - if err := json.Unmarshal([]byte(result.stdout), &values); err != nil { - t.Fatalf("failed to parse json output: %v", err) - } - assertKeys(t, slices.Collect(maps.Keys(values))) - }) + t.Run("table", func(t *testing.T) { + result := runCommand(t, args, nil) + var got []string + for line := range strings.SplitSeq(result.stdout, "\n") { + cells := strings.Split(line, "\u2502") + // A data row is "│ key │ value │": empty, key, value, empty. + if len(cells) != 4 { + continue + } + if key := strings.TrimSpace(cells[1]); key != "" && key != "PROPERTY" { + got = append(got, key) + } + } + assertKeys(t, got) + }) - t.Run("yaml", func(t *testing.T) { - result := runCommand(t, []string{"config", "list", "-o", "yaml"}, nil) - var values map[string]any - if err := yaml.Unmarshal([]byte(result.stdout), &values); err != nil { - t.Fatalf("failed to parse yaml output: %v", err) - } - assertKeys(t, slices.Collect(maps.Keys(values))) - }) + t.Run("json", func(t *testing.T) { + result := runCommand(t, append(args, "-o", "json"), nil) + var values map[string]any + if err := json.Unmarshal([]byte(result.stdout), &values); err != nil { + t.Fatalf("failed to parse json output: %v", err) + } + assertKeys(t, slices.Collect(maps.Keys(values))) + }) + + t.Run("yaml", func(t *testing.T) { + result := runCommand(t, append(args, "-o", "yaml"), nil) + var values map[string]any + if err := yaml.Unmarshal([]byte(result.stdout), &values); err != nil { + t.Fatalf("failed to parse yaml output: %v", err) + } + assertKeys(t, slices.Collect(maps.Keys(values))) + }) + }) + } } diff --git a/internal/config/config.go b/internal/config/config.go index 77e9da48..515bbeb7 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -95,23 +95,37 @@ func parseReadOnlyMode(value string) (ReadOnlyMode, error) { return "", fmt.Errorf("invalid read_only value: %s (must be all, prod, or off)", value) } -var defaultValues = map[string]any{ +// publicDefaultValues holds the user-facing config keys, which `tiger config +// list` shows and shell completion offers. +var publicDefaultValues = map[string]any{ "analytics": DefaultAnalytics, - "api_url": DefaultAPIURL, "color": DefaultColor, - "console_url": DefaultConsoleURL, "docs_mcp": DefaultDocsMCP, - "docs_mcp_url": DefaultDocsMCPURL, - "gateway_url": DefaultGatewayURL, "mcp_max_rows": DefaultMCPMaxRows, "output": DefaultOutput, "password_storage": DefaultPasswordStorage, "read_only": DefaultReadOnly, - "releases_url": DefaultReleasesURL, "service_id": "", "version_check": DefaultVersionCheck, } +// privateDefaultValues holds internal config keys, mostly for pointing the CLI +// at other environments. They're settable like any other key, but hidden from +// `tiger config list` (unless --all is given) and from shell completion. +var privateDefaultValues = map[string]any{ + "api_url": DefaultAPIURL, + "console_url": DefaultConsoleURL, + "docs_mcp_url": DefaultDocsMCPURL, + "gateway_url": DefaultGatewayURL, + "releases_url": DefaultReleasesURL, +} + +var defaultValues = func() map[string]any { + m := maps.Clone(publicDefaultValues) + maps.Copy(m, privateDefaultValues) + return m +}() + // flagBindings maps CLI flag names to the config keys they override. Flags // missing from a caller's flag set are skipped, so command-local flags (e.g. // --output) bind only for the commands that define them. @@ -180,17 +194,24 @@ func Load(flags *pflag.FlagSet) (*Config, error) { return cfg, nil } +// OutputOptions controls which values LoadForOutput reports. +type OutputOptions struct { + WithEnv bool // apply TIGER_* env var overrides + NoDefaults bool // omit keys that aren't explicitly set + All bool // include private keys +} + // LoadForOutput loads config values for display purposes using a fresh viper // instance, independent of CLI flags. This keeps `tiger config list -o json` // from reporting the flag's format as the configured `output` value. -func LoadForOutput(configDir string, withEnv bool, noDefaults bool) (*ConfigOutput, error) { +func LoadForOutput(configDir string, opts OutputOptions) (*ConfigOutput, error) { v := viper.New() v.SetConfigFile(GetConfigFile(configDir)) - if withEnv { + if opts.WithEnv { applyEnvOverrides(v) } - if !noDefaults { + if !opts.NoDefaults { applyDefaults(v) } @@ -199,6 +220,18 @@ func LoadForOutput(configDir string, withEnv bool, noDefaults bool) (*ConfigOutp } migrateVersionCheck(v) + // Private keys are dropped wherever they came from (default, file, or env), + // so --all is the only way to see one. + if !opts.All { + public := viper.New() + for _, key := range v.AllKeys() { + if _, private := privateDefaultValues[key]; !private { + public.Set(key, v.Get(key)) + } + } + v = public + } + cfg := &ConfigOutput{} if err := v.Unmarshal(cfg); err != nil { return nil, fmt.Errorf("error unmarshaling config for output: %w", err) @@ -342,6 +375,12 @@ func ValidConfigOptions() []string { return slices.Sorted(maps.Keys(defaultValues)) } +// PublicConfigOptions returns the config keys offered by shell completion, +// sorted like ValidConfigOptions. Private keys are left out. +func PublicConfigOptions() []string { + return slices.Sorted(maps.Keys(publicDefaultValues)) +} + // ValidConfigOptionValues returns known completion values for a config key's // value, or nil if the key doesn't have a fixed set of values. func ValidConfigOptionValues(key string) []string { diff --git a/internal/config/config_keys_test.go b/internal/config/config_keys_test.go index f173d152..25dad243 100644 --- a/internal/config/config_keys_test.go +++ b/internal/config/config_keys_test.go @@ -60,6 +60,15 @@ func TestConfigKeyRegistriesAgree(t *testing.T) { } }) + // A key in both maps would be private or public depending on map order. + t.Run("public and private keys are disjoint", func(t *testing.T) { + for key := range privateDefaultValues { + if _, ok := publicDefaultValues[key]; ok { + t.Errorf("config key %q is both public and private", key) + } + } + }) + // Every flag that claims to override a config value must name a real one. t.Run("flagBindings target real keys", func(t *testing.T) { for flag, key := range flagBindings { From 913656e97acd160046f5081f4b78032b4db6736c Mon Sep 17 00:00:00 2001 From: Nathan Cochran Date: Mon, 5 Oct 2026 16:17:58 -0400 Subject: [PATCH 2/2] Fix '--with-env --no-default' behavior --- internal/cmd/config_list_test.go | 26 ++++++++++++++++++++++++++ internal/config/config.go | 20 ++++++++++++++++---- 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/internal/cmd/config_list_test.go b/internal/cmd/config_list_test.go index 4d211f86..51b5d43c 100644 --- a/internal/cmd/config_list_test.go +++ b/internal/cmd/config_list_test.go @@ -204,6 +204,21 @@ version_check: true │ service_id │ env-service │ │ version_check │ true │ └──────────────────┴─────────────┘ +`, + }, + { + name: "with-env and no-defaults shows a key set only by env var", + args: []string{"config", "list", "--with-env", "--no-defaults"}, + opts: []runOption{ + withConfig(map[string]any{"analytics": false}), + withEnv("TIGER_SERVICE_ID", "env-service"), + }, + wantStdout: `┌────────────┬─────────────┐ +│ PROPERTY │ VALUE │ +├────────────┼─────────────┤ +│ analytics │ false │ +│ service_id │ env-service │ +└────────────┴─────────────┘ `, }, { @@ -307,6 +322,17 @@ read_only: "off" releases_url: https://cli.tigerdata.com service_id: "" version_check: true +`, + }, + { + name: "all with with-env and no-defaults shows a private key set only by env var", + args: []string{"config", "list", "--all", "--with-env", "--no-defaults"}, + opts: []runOption{withEnv("TIGER_API_URL", "https://env.api.com/v1")}, + wantStdout: `┌──────────┬────────────────────────┐ +│ PROPERTY │ VALUE │ +├──────────┼────────────────────────┤ +│ api_url │ https://env.api.com/v1 │ +└──────────┴────────────────────────┘ `, }, { diff --git a/internal/config/config.go b/internal/config/config.go index 515bbeb7..09b5364c 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -209,7 +209,9 @@ func LoadForOutput(configDir string, opts OutputOptions) (*ConfigOutput, error) v.SetConfigFile(GetConfigFile(configDir)) if opts.WithEnv { - applyEnvOverrides(v) + if err := applyEnvOverrides(v); err != nil { + return nil, fmt.Errorf("failed to bind env vars: %w", err) + } } if !opts.NoDefaults { applyDefaults(v) @@ -253,7 +255,9 @@ func LoadForOutput(configDir string, opts OutputOptions) (*ConfigOutput, error) func (c *Config) reload() error { v := viper.New() v.SetConfigFile(c.GetConfigFile()) - applyEnvOverrides(v) + if err := applyEnvOverrides(v); err != nil { + return fmt.Errorf("failed to bind env vars: %w", err) + } applyDefaults(v) if err := bindFlags(v, c.flags); err != nil { @@ -440,9 +444,17 @@ func applyDefaults(v *viper.Viper) { } } -func applyEnvOverrides(v *viper.Viper) { +// applyEnvOverrides binds every config key to its TIGER_ env var. Binding +// explicitly, rather than via AutomaticEnv, makes viper aware of a key set only +// by env var, so it appears in `config list --with-env --no-defaults`. +func applyEnvOverrides(v *viper.Viper) error { v.SetEnvPrefix("TIGER") - v.AutomaticEnv() + for key := range defaultValues { + if err := v.BindEnv(key); err != nil { + return err + } + } + return nil } func readInConfig(v *viper.Viper) error {