Repository navigation
Preset icons sized by glyph, and environments secrets in the CLI #7838
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| # Docs site | ||
|
|
||
| Everything under `content/` is published at docs.superset.sh, so it documents | ||
| only what a customer can run today. | ||
|
|
||
| - **CLI reference (`content/docs/cli/cli-reference.mdx`) lists public commands | ||
| only.** A command or group with `audience: "internal"` in its `command.ts` or | ||
| `meta.ts` under `packages/cli/src/commands/` does not appear here, however | ||
| useful it is to us; the desktop's help and `superset --help` are where an | ||
| internal command is discovered. Check the audience before adding an entry, | ||
| and remove the entry in the same change that makes a command internal. | ||
| - Features behind a flag or an allowlist (cloud workspaces and everything under | ||
| `environments`, for example) stay out until they ship to everyone. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| @AGENTS.md |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| import { CLIError, positional, string } from "@superset/cli-framework"; | ||
| import { command } from "../../../../lib/command"; | ||
| import { resolveEnvironment } from "../../../../lib/environments"; | ||
|
|
||
| export default command({ | ||
| description: "Delete a variable from an environment", | ||
| args: [positional("name").required().desc("Variable name")], | ||
| options: { | ||
| environment: string().desc( | ||
| "Environment (id or name); required when the organization has several", | ||
| ), | ||
| }, | ||
| run: async ({ ctx, args, options }) => { | ||
| const organizationId = ctx.config.organizationId; | ||
| if (!organizationId) { | ||
| throw new CLIError("No active organization", "Run: superset auth login"); | ||
| } | ||
| const environment = await resolveEnvironment( | ||
| ctx.api, | ||
| organizationId, | ||
| options.environment, | ||
| ); | ||
| const key = args.name as string; | ||
| await ctx.api.environment.secrets.remove.mutate({ | ||
| environmentId: environment.id, | ||
| key, | ||
| }); | ||
| return { | ||
| data: { environment: environment.name, key }, | ||
| message: `Deleted ${key} from ${environment.name}`, | ||
| }; | ||
| }, | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| import { CLIError, string, table } from "@superset/cli-framework"; | ||
| import { command } from "../../../../lib/command"; | ||
| import { resolveEnvironment } from "../../../../lib/environments"; | ||
|
|
||
| export default command({ | ||
| description: "List an environment's variables; values are never shown", | ||
| options: { | ||
| environment: string().desc( | ||
| "Environment (id or name); required when the organization has several", | ||
| ), | ||
| }, | ||
| display: (data) => | ||
| table( | ||
| (data as { secrets: Record<string, unknown>[] }).secrets ?? [], | ||
| ["key", "visibility", "updatedAt"], | ||
| ["NAME", "VISIBILITY", "UPDATED"], | ||
| [48, 10, 24], | ||
| ), | ||
| run: async ({ ctx, options }) => { | ||
| const organizationId = ctx.config.organizationId; | ||
| if (!organizationId) { | ||
| throw new CLIError("No active organization", "Run: superset auth login"); | ||
| } | ||
| const environment = await resolveEnvironment( | ||
| ctx.api, | ||
| organizationId, | ||
| options.environment, | ||
| ); | ||
| const rows = await ctx.api.environment.secrets.list.query({ | ||
| environmentId: environment.id, | ||
| }); | ||
| return { | ||
| data: { | ||
| environment: { id: environment.id, name: environment.name }, | ||
| secrets: rows.map((row) => ({ | ||
| key: row.key, | ||
| visibility: row.sensitive ? "secret" : "visible", | ||
| updatedAt: row.updatedAt.toISOString(), | ||
| })), | ||
| }, | ||
| }; | ||
| }, | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| export default { | ||
| sandbox: false, | ||
| description: "Manage the variables an environment gives its cloud workspaces", | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,88 @@ | ||
| import { readFileSync } from "node:fs"; | ||
| import { boolean, CLIError, positional, string } from "@superset/cli-framework"; | ||
| import { parseEnvContent } from "@superset/shared/env-file"; | ||
| import { command } from "../../../../lib/command"; | ||
| import { resolveEnvironment } from "../../../../lib/environments"; | ||
|
|
||
| async function readStdin(): Promise<string> { | ||
| const chunks: Buffer[] = []; | ||
| for await (const chunk of process.stdin) { | ||
| chunks.push(typeof chunk === "string" ? Buffer.from(chunk) : chunk); | ||
| } | ||
| return Buffer.concat(chunks).toString("utf-8"); | ||
| } | ||
|
|
||
| export default command({ | ||
| description: | ||
| "Set a variable on an environment, from --body, stdin, or a .env file", | ||
| args: [ | ||
| positional("name").desc("Variable name; omit when passing --env-file"), | ||
| ], | ||
| options: { | ||
| body: string().alias("b").desc("The value; read from stdin when omitted"), | ||
| envFile: string() | ||
| .alias("f") | ||
| .desc("Set every variable in a .env file instead of one name"), | ||
| visible: boolean().desc( | ||
| "Store it readable in settings instead of as a secret", | ||
| ), | ||
| environment: string().desc( | ||
| "Environment (id or name); required when the organization has several", | ||
| ), | ||
| }, | ||
| run: async ({ ctx, args, options }) => { | ||
| const organizationId = ctx.config.organizationId; | ||
| if (!organizationId) { | ||
| throw new CLIError("No active organization", "Run: superset auth login"); | ||
| } | ||
| const name = args.name as string | undefined; | ||
| if (options.envFile && (name || options.body)) { | ||
| throw new CLIError("Pass either --env-file or a name with a value"); | ||
| } | ||
| if (!options.envFile && !name) { | ||
| throw new CLIError( | ||
| "Provide a variable name, or --env-file to set several", | ||
| ); | ||
| } | ||
|
|
||
| let entries: Array<{ key: string; value: string }>; | ||
| if (options.envFile) { | ||
| entries = parseEnvContent(readFileSync(options.envFile, "utf-8")); | ||
| if (entries.length === 0) { | ||
| throw new CLIError(`No variables found in ${options.envFile}`); | ||
| } | ||
| } else { | ||
| const value = | ||
| options.body ?? (process.stdin.isTTY ? undefined : await readStdin()); | ||
| if (value === undefined) { | ||
| throw new CLIError( | ||
| "No value given", | ||
| `Pass --body, or pipe it: printf '%s' "$VALUE" | superset environments secrets set ${name}`, | ||
| ); | ||
| } | ||
| entries = [{ key: name as string, value }]; | ||
| } | ||
|
|
||
| const environment = await resolveEnvironment( | ||
| ctx.api, | ||
| organizationId, | ||
| options.environment, | ||
| ); | ||
| for (const entry of entries) { | ||
| await ctx.api.environment.secrets.set.mutate({ | ||
| environmentId: environment.id, | ||
| key: entry.key, | ||
| value: entry.value, | ||
| sensitive: !options.visible, | ||
| }); | ||
| } | ||
| const keys = entries.map((entry) => entry.key); | ||
| return { | ||
| data: { environment: environment.name, keys }, | ||
| message: | ||
| keys.length === 1 | ||
| ? `Set ${keys[0]} on ${environment.name}` | ||
| : `Set ${keys.length} variables on ${environment.name}: ${keys.join(", ")}`, | ||
| }; | ||
| }, | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| export { resolveEnvironment } from "./resolveEnvironment"; |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,46 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { CLIError } from "@superset/cli-framework"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import type { ApiClient } from "../api-client"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export interface ResolvedEnvironment { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| id: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| name: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * The environment a command acts on, by id or name. Nothing requested is | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * only unambiguous when the organization has one environment. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export async function resolveEnvironment( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| api: ApiClient, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| organizationId: string, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| requested: string | undefined, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ): Promise<ResolvedEnvironment> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const environments = await api.environment.list.query({ organizationId }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (environments.length === 0) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw new CLIError( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "No environments in this organization", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "Create one in Settings → Environments", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [only] = environments; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (requested === undefined) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (only && environments.length === 1) return only; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw new CLIError( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "Several environments in this organization", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| `Pass --environment with one of: ${environments.map((environment) => environment.name).join(", ")}`, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const wanted = requested.trim().toLowerCase(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const selected = environments.find( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| (environment) => | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| environment.id.toLowerCase() === wanted || | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| environment.name.toLowerCase() === wanted, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!selected) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+28
to
+39
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: sed -n '1,100p' packages/cli/src/lib/environments/resolveEnvironment.ts
rg -n 'environments\.list|environment.*name.*unique|unique.*environment.*name' packages/cli packages/api packages/shared 2>/dev/null | head -80Repository: superset-sh/superset Length of output: 1560 🏁 Script executed: set -u
printf '%s\n' '--- changed file and diff ---'
git diff --no-ext-diff --unified=80 c61f8f1aa3aa5bda331ce03967572f3996f86b0a 54afee28717edf30732323850e23e848054eb4a0 -- packages/cli/src/lib/environments/resolveEnvironment.ts
printf '%s\n' '--- environment list definitions and schemas ---'
rg -n -S --glob '!node_modules' --glob '!dist' 'environment\.list|list\s*:\s*.*environment|listEnvironments|Environment.*(Schema|Input|Model)|environment[s]?[^[:alnum:]]' packages/cli packages/api packages/shared apps 2>/dev/null | head -240
printf '%s\n' '--- resolver callers and set/delete paths ---'
rg -n -S --glob '!node_modules' --glob '!dist' 'resolveEnvironment|environment\.(set|delete)|deleteEnvironment|setEnvironment|environmentId' packages/cli 2>/dev/null | head -240
printf '%s\n' '--- environment fixtures and constraints ---'
rg -n -S --glob '!node_modules' --glob '!dist' 'environments|environment_id|environmentId|organizationId' packages/cli packages/api packages/shared apps 2>/dev/null | grep -Ei 'test|spec|fixture|seed|schema|migration|unique|index|name' | head -240Repository: superset-sh/superset Length of output: 41173 🤖 get_repo_knowledge executed:
Length of output: 27862 🏁 Script executed: set -u
printf '%s\n' '--- exact environment router/procedure symbols ---'
rg -n -S --glob '!node_modules' --glob '!dist' --glob '*.{ts,tsx,sql}' '(environmentRouter|environment\.list|environmentList|listEnvironments|createEnvironment|updateEnvironment|deleteEnvironment|secrets\.set|secrets\.remove)' apps/api packages/trpc packages/cli | head -260
printf '%s\n' '--- candidate environment files ---'
git ls-files | rg -i '(^|/)(environment|environments)(/|\.|$)|environment.*(router|schema|migration)|schema.*environment|migration.*environment' | head -200
printf '%s\n' '--- precise CLI callers ---'
for f in packages/cli/src/commands/environments/secrets/{list,set,delete}/command.ts packages/cli/src/lib/environments/index.ts packages/cli/src/lib/api-client.ts; do
if test -f "$f"; then
echo "--- $f"
cat -n "$f"
fi
done
printf '%s\n' '--- uniqueness and environment row fields ---'
rg -n -S --glob '!node_modules' --glob '!dist' --glob '*.{ts,tsx,sql,json}' '(environment[^[:alnum:]_]*(name|id)|name[^[:alnum:]_]*environment|organization_id|organizationId)' apps/api packages/trpc packages/local-db packages/cli | grep -Ei 'environment|unique|index|constraint|create|list|select|where' | head -300Repository: superset-sh/superset Length of output: 37092 🏁 Script executed: set -u
printf '%s\n' '--- environment router ---'
cat -n packages/trpc/src/router/environment/environment.ts
printf '%s\n' '--- environment resolver and secrets router ---'
cat -n packages/trpc/src/router/environment/resolve-environment.ts
cat -n packages/trpc/src/router/environment/secrets/secrets.ts
printf '%s\n' '--- imported schema declarations ---'
sed -n '1,90p' packages/trpc/src/router/environment/environment.ts | rg -n '^(.*import|.*from)'
rg -n -S --glob '*.{ts,tsx,sql}' 'export .*environments|environments\s*=|pgTable\(.*environment|sqliteTable\(.*environment|environment.*name' packages/trpc packages/local-db | head -220
printf '%s\n' '--- environment tests/fixtures ---'
git ls-files | rg -i 'environment.*(test|spec|fixture|seed)|((test|spec|fixture|seed).*)environment' | head -160Repository: superset-sh/superset Length of output: 26127 🏁 Script executed: set -u
printf '%s\n' '--- database schema package and environment table ---'
git ls-files | rg '(^|/)packages/(db|database)/|(^|/)db/.*schema|schema\.ts$' | head -240
rg -n -S --glob '*.{ts,tsx,sql}' 'environments\s*=|environmentScopeValues|created_by_user_id|createdByUserId|archived_at|archivedAt' packages apps | grep -v 'node_modules\|dist' | head -260
printf '%s\n' '--- environment migrations and seeds ---'
git ls-files | rg -i '(migration|drizzle|seed|fixture).*environment|environment.*(migration|drizzle|seed|fixture)' | head -220
printf '%s\n' '--- environment selector precedent ---'
cat -n packages/shared/src/cloud-environments.ts
cat -n packages/cli/src/lib/cloud-workspaces/resolveEnvironment.tsRepository: superset-sh/superset Length of output: 41845 🏁 Script executed: set -u
printf '%s\n' '--- environments table schema ---'
sed -n '630,700p' packages/db/src/schema/schema.ts
printf '%s\n' '--- environment enum and migration ---'
sed -n '65,80p' packages/db/src/schema/enums.ts
cat -n packages/db/drizzle/0104_environments.sql
printf '%s\n' '--- repository migration additions ---'
cat -n packages/db/drizzle/0115_cloud_environments_repositories.sql | sed -n '1,80p'Repository: superset-sh/superset Length of output: 10344 Reject ambiguous environment selectors before set/delete.
Suggested fix const wanted = requested.trim().toLowerCase();
- const selected = environments.find(
+ const matches = environments.filter(
(environment) =>
environment.id.toLowerCase() === wanted ||
environment.name.toLowerCase() === wanted,
);
+ if (matches.length > 1) {
+ throw new CLIError(
+ "Several environments match this selector",
+ `Pass --environment with one of: ${matches.map((environment) => environment.name).join(", ")}`,
+ );
+ }
+ const [selected] = matches;
if (!selected) {📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw new CLIError( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| `No environment "${requested}" in this organization`, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| `Environments: ${environments.map((environment) => environment.name).join(", ")}`, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return selected; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| // Parser grammar from dotenv (BSD-2-Clause), which cannot be imported in the | ||
| // renderer: its module root pulls in fs, path, os and crypto. | ||
| const LINE = | ||
| /(?:^|^)\s*(?:export\s+)?([\w.-]+)(?:\s*=\s*?|:\s+?)(\s*'(?:\\'|[^'])*'|\s*"(?:\\"|[^"])*"|\s*`(?:\\`|[^`])*`|[^#\r\n]+)?\s*(?:#.*)?(?:$|$)/gm; | ||
|
|
||
| export interface EnvEntry { | ||
| key: string; | ||
| value: string; | ||
| } | ||
|
|
||
| export function parseEnvContent(content: string): EnvEntry[] { | ||
| const entries: EnvEntry[] = []; | ||
| const normalised = content.replace(/\r\n?/gm, "\n"); | ||
|
|
||
| LINE.lastIndex = 0; | ||
| let match = LINE.exec(normalised); | ||
| while (match !== null) { | ||
| const [, key, rawValue] = match; | ||
| match = LINE.exec(normalised); | ||
| if (!key) continue; | ||
| let value = (rawValue ?? "").trim(); | ||
| const quote = value[0]; | ||
|
|
||
| value = value.replace(/^(['"`])([\s\S]*)\1$/gm, "$2"); | ||
| if (quote === '"') { | ||
| value = value.replace(/\\n/g, "\n").replace(/\\r/g, "\r"); | ||
| } | ||
|
|
||
| entries.push({ key, value }); | ||
| } | ||
|
|
||
| return entries; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Reject empty stdin: a run with no input sets the secret to an empty string.
The command reads stdin whenever
process.stdin.isTTYis falsy. That covers CI jobs, agent shells, and cron, not only real pipes. In those contexts stdin is often empty or/dev/null.readStdin()then returns"". Thevalue === undefinedcheck does not catch"". The command callssecrets.set.mutatewith an empty value and reportsSet FOO on …. An existing secret gets overwritten with nothing, and the output looks like a success.If stdin returns no data, throw the same "No value given" error. Leave the
--body ""path unchanged, so a user can still set an empty value on purpose.🐛 Proposed fix
📝 Committable suggestion
🤖 Prompt for AI Agents