feat(env): add link / unlink / links commands for env variables#2120
feat(env): add link / unlink / links commands for env variables#2120jlaneve wants to merge 4 commits into
Conversation
Adds three new subcommands under 'astro env variable': link <key> --deployment-id <dep> [--value <override>] [--exclude] unlink <key> --deployment-id <dep> [--exclude] links <key> link/unlink default to the workspace object's Links list (with optional per-link override value). With --exclude they target excludeLinks instead, useful for opting specific deployments out of an --auto-link var. links shows the consolidated state (workspace value, autoLink flag, all links with their overrides, all excludes). The CLI uses the platform's dedicated POST .../exclude-linking endpoint for adding excludes; everything else goes through PATCH on the env-object, echoing autoLinkDeployments back to dodge AINF-1792 (the platform clears that field when an update body omits it). Verified end-to-end against a real workspace + deployment: - link/unlink with and without override - override visible at deployment scope - exclude removes from auto-link resolved set - explicit link with override coexists with auto-link, override wins - every PATCH path preserves autoLinkDeployments=true Variables only this PR; airflow-variable / connection / metrics-export linking will follow once shape is validated.
Coverage Report for CI Build 6Coverage increased (+0.1%) to 39.834%Details
Uncovered Changes
Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
…ted overrides Two issues found in adversarial smoke testing: 1. ListVarLinks hardcoded includeSecrets=false on its underlying GET, so per-link override values for secret vars were always redacted, even when the user passed --include-secrets. Plumb the flag through. 2. The table renderer showed "-" both for "no override" and for "override redacted because var is secret and --include-secrets is off" - the user couldn't tell which. Now renders "(hidden, use --include-secrets)" for the redacted case so the ambiguity is surfaced. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Empty or non-CUID --deployment-id values were being passed straight to the platform, which responds with an opaque "Internal server error" instead of a useful message. Add validateDeploymentID to LinkVar/UnlinkVar/ExcludeVar/ UnexcludeVar so the CLI rejects with: - "--deployment-id cannot be empty" for empty - "%q is not a valid deployment ID (expected a CUID)" for malformed Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces the duplicate-link rejection with upsert semantics. The platform's PATCH on links/excludeLinks merges per-entry rather than fully replacing the array, so absent fields are preserved (Option A). This shapes the contract: link <key> # ensure linked; preserves any existing override link <key> --value v # ensure linked with override v (replaces existing) link <key> --exclude # ensure excluded; idempotent no-op if already Removing an existing override requires unlink + relink (two steps). Filed AINF-1809 upstream so the API can support explicit-clear in a future PATCH. unlink and unlink --exclude keep their reject-on-not-present behavior since removal of something that isn't there is usually a typo. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
neel-astro
left a comment
There was a problem hiding this comment.
Mostly looks good to me, apart from a comment around command structure
| if depID == "" { | ||
| return errors.New("--deployment-id cannot be empty") | ||
| } | ||
| if !organization.IsCUID(depID) { |
There was a problem hiding this comment.
Should we move IsCUID to utils instead of referencing cross-border from the organization package
| report.WorkspaceValue = current.EnvironmentVariable.Value | ||
| report.IsSecret = current.EnvironmentVariable.IsSecret | ||
| } | ||
| if current.Links != nil { |
There was a problem hiding this comment.
I still do not understand why codegen creates pointers to an array :), not a comment for the logic, just a frustration :)
|
|
||
| func newEnvVarLinksCmd(out io.Writer) *cobra.Command { | ||
| cmd := &cobra.Command{ | ||
| Use: "links <id-or-key>", |
There was a problem hiding this comment.
We have been using the format of <resource> <action>, keeping the resource as singular throughout the CLI commands with parent resource ID passed as flags instead of args (ex: astro deployment team list --deployment-id or astro deployment token organization-token list --deployment-id), so I think a more apt version would be astro env var link list --variable-id or astro env var link list --variable-key
There was a problem hiding this comment.
similar comment for link and unlink command, where the variable id or key would be a parent resource identifier, so pass that on as a flags instead of args
Summary
Adds explicit per-deployment linking — including per-link value overrides — for workspace-scoped env variables.
Today, the only way to attach a workspace var to specific deployments via the CLI is
--auto-link(all-or-nothing). To get "shared default with per-deployment differences" you have to use the UI. This PR closes that gap.Variables only this PR; airflow-variable / connection / metrics-export linking will follow in focused PRs once this shape is validated.
Mental model
The platform stores overrides on the link itself, not as separate objects:
When a deployment resolves the linked var, the override (if present) replaces the workspace default — but only for that deployment. The workspace value and other deployments are unaffected.
New commands
--valueand--excludeare mutually exclusive (excludes don't carry override values).Upsert semantics
linkis idempotent and partial-update:--valueis passed).--valueis passed.--valueis a no-op for an existing link's override (the platform's PATCH preserves omitted fields).unlinkthenlinkwithout--value. Two-step, explicit. (Tracked upstream as AINF-1809 — the API doesn't currently support "explicit clear" in a single PATCH.)link --excludeis also idempotent (excluding an already-excluded deployment is a no-op success).unlinkandunlink --excludekeep reject-on-not-present, since removal of something that isn't there is usually a typo.Example workflows
1. Dev / staging / prod from one workspace var
2. Rotate the override
linkis upsert, so just re-run with the new value:3. Shared-by-default, opt out one deployment
4. Auto-link with one specific override
Keep auto-link on so every deployment gets the workspace default, but give one deployment a different value.
This is one of the cases the platform investigation explicitly confirmed: explicit
LinksandautoLinkDeployments=truecoexist, and the explicit link's override wins for that deployment.5. Auditing
6. Cleanup
Wire-level details
For reviewers who want to dig in:
linkandunlinkPATCH/environment-objects/{id}with the modifiedLinksarray. The platform doesn't expose an incremental "add one link" endpoint, so we GET-modify-PATCH.link --excludeuses the dedicatedPOST /environment-objects/{id}/exclude-linkingendpoint (single-shot, incremental).unlink --excludePATCHes with the trimmedexcludeLinksarray (no incremental endpoint exists for removal).autoLinkDeploymentsback. The platform clears that field when an update body omits it. Stress-tested with 6 mixed link/unlink/exclude/unexclude operations on anautoLinkDeployments=truevar; the flag survived all 6.environmentVariable: { value: ... }even when only mutating links. The CLI re-sends the existing workspace value so this is a no-op for the value while satisfying the type-discriminator check.Validation surface
--deployment-id ""--deployment-id "garbage"--valueand--excludelinkon already-linked deploymentlink --excludeon already-excluded deploymentunlink/unlink --excludeon not-presentRelated upstream issues
scopeEntityId. Worked around with CLI pre-validation.autoLinkDeploymentscleared when omitted from PATCH. Mitigated by echoing the field on every PATCH.Verified end-to-end
Real workspace + multiple real deployments, every operation cleaned up after.
linkwithout overridelink --value <override>, override visible at deployment scope--value(upsert replaces override)--value(no-op for existing override)link --value ""(empty-string override)$, quotes, backslasheslinkto two deployments, distinct overrides, isolation between themlink --excludeon already-excluded deploymentlink --value→ override wins at deployment--include-secretsreveals itautoLinkDeployments=true--valueWhat is not in this PR
airflow-variable / connection / metrics-export link— follow-up; each has its own override-flag set.--from-file— could chain naturally later (sibling PR feat(env): add --from-file bulk import for env and airflow variables #2119 adds bulk import for create/update; same pattern would extend here).update-linkverb —linkis upsert, so this is unnecessary.astro env variable links(no key) to audit every workspace var at once.Test plan
go test ./...greengo vet ./...cleangolangci-lint run ./...cleancloud/env/link_test.go(link/unlink/exclude/unexclude/list/upsert-override/upsert-preserves-other-links/idempotent-exclude/scope-validation/deployment-id-validation)autoLinkDeployments=truevar, flag survives all 6🤖 Generated with Claude Code