feat: add library variable set list and view commands - #697
Conversation
Adds `octopus library-variable-set` with `list` and `view`. `view` fetches the set and its variables in one command and groups values that share a name, so a variable with several scoped values reads as one entry instead of several, in table, basic and json output. Read-only for now; create/update/delete of sets and variables is not included. Refs #346 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
list -f json emitted ID/VariableSetID; every other list command and view -f json use Id/VariableSetId. view -f json emitted "Variables": null when --filter matched nothing, and [] otherwise. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Covers what MockHttpServer can't: real ScopeValues resolution, the server returning no value for a sensitive variable, script module exclusion, and the two json fixes above. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
NickJosevski
left a comment
There was a problem hiding this comment.
One correctness issue found (verified against the portal's route definitions); everything else looked good.
| } | ||
| } | ||
|
|
||
| webUrl := util.GenerateWebURL(opts.Host, set.SpaceID, fmt.Sprintf("library/variablesets/%s", set.GetID())) |
There was a problem hiding this comment.
The portal has no library/variablesets/{id} route, so this URL (printed in basic output, returned as WebUrl in JSON, and opened by --web) lands on the portal's not-found page. The variable set detail page lives at library/variables/{id}.
I verified this against the portal bundle of a current server: variableSetPage is defined with the route template /variables/{variableSetId} nested under /library, the legacy v3 redirects also use library/variables/..., and the string variablesets does not appear in any route template.
| webUrl := util.GenerateWebURL(opts.Host, set.SpaceID, fmt.Sprintf("library/variablesets/%s", set.GetID())) | |
| webUrl := util.GenerateWebURL(opts.Host, set.SpaceID, fmt.Sprintf("library/variables/%s", set.GetID())) |
The expected URLs in view_test.go (the basic and json cases) and the WebUrl assertion in test/integration/libraryvariableset_test.go encode the same wrong path and will need updating with it.
The portal has no `library/variablesets/{id}` route; the variable set
detail page lives at `library/variables/{id}`. Verified against a current
server's portal bundle: the route segment is
`library:createRouteSegment(routeTemplate`/library`, { ...
variableSetPage: createPageRoute(routeTemplate`/variables/{variableSetId}`) })`,
the legacy v3 redirect for a variable set is `/library/variables/new`, and
the deep-link map sends `libraryvariablesets` ids to `variableSetPage`.
The string `variablesets` appears in no route template.
Updates the basic/json expectations in view_test.go and the WebUrl
assertion in the integration test with it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
formatForBasic returned early on an empty group list, so `-f basic` dropped the "View this library variable set in Octopus Deploy" line for a set with no variables (or when --filter matched none) while every other basic view in the CLI always prints it. Adds a test for the filtered case. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Refs #346
What this adds
A new top-level
octopus library-variable-setcommand (aliaseslibrary-variable-sets,lvs) with two read-only subcommands.library-variable-set list— lists the variable sets in the space, sorted by name, with--filter/-qfor substring matching. Script modules share thelibraryvariablesetsendpoint and are excluded (the existingsharedVariable.GetAllLibraryVariableSetshelper already filters onContentType == "Variables").library-variable-set view [<name> | <id>]— this is the part aimed at the issue. It resolves the set, fetches its variables, and groups every value stored under the same variable name into one entry, with the unscoped value first because it is the fallback the scoped ones override.--filter/-qnarrows to variables whose name contains a string;--web/-wopens the set in the browser. Prompts for the set in interactive mode, errors in--no-prompt.This addresses both pain points named in the issue:
/libraryvariablesets/and/variables/doesn't make much sense" — one command does both requests and stitches the result together.Scope IDs are resolved to display names via the variable set's
ScopeValues, falling back to the raw ID rather than erroring, so one unresolvable scope value can't break the whole view (this differs deliberately frompkg/cmd/project/variables/shared.ToScopeValues, which returns an error).Table output (
-f table, the default) repeats the variable name only on the first row of each group:-f jsonemits the set's own fields plusVariablesas an array of{Name, Values[]}, each value carryingId,Value,IsSensitive,IsScoped, aScopeobject using the API's own property names (Environment,Role,Machine,TenantTag, …) with resolved names, and a flatScopeSummarystring.-f basicprints a grouped text block. Sensitive values are masked as***.This also lights up the previously-unused
annotations.IsLibrarygroup, so the root help now has aLIBRARY COMMANDSsection.Deliberately left out
/api/{space}/variables/variableset-…is a read-modify-write of the whole variable set with optimistic concurrency onVersion; doing it safely (and prompting for scopes) is a much bigger surface than a first slice should carry, and it deserves its own design discussion — see the open questions below.list, andviewwon't resolve one.viewreports onlyTemplateCount.pkg/question/selectors— resolution lives inpkg/cmd/libraryvariableset/shared, mirroringpkg/cmd/channel/shared.ResolveChannel. Worth promoting if a second command needs it.Follow-up surface, roughly in the order I'd do it:
create/deleteof a set (cheap, the SDK hasAdd/Update/DeleteByID);variable create/update/deletewithin a set, reusingpkg/cmd/project/variables/sharedscope flags; script-module support; templates.Test evidence
go build ./...— clean.go test ./pkg/...— pass, exit 0, no failures. New tests:pkg/cmd/libraryvariableset/shared/shared_test.go— grouping (including case-insensitive names and unscoped-first ordering), scope ID→name resolution, fallback to the raw ID for unknown scope values, tolerance of a missingScopeValueslookup, sensitive masking.pkg/cmd/libraryvariableset/list/list_test.go— table/json/basic, name filter, script modules excluded.pkg/cmd/libraryvariableset/view/view_test.go— required-in-automation-mode error, unknown-set error, interactive prompt, grouped table, basic, json.Test support changes:
NewLibraryVariableSet/NewVariableSetForLibraryVariableSetfixtures, and theLibraryVariables+Variableslinks added totestutil.NewRootResource()(they were missing, so the SDK couldn't build those service paths under test).Open questions / options
1. Command noun. I went with
library-variable-set.library-variable-set(chosen) — matches the API resource and the Octopus UI's "Library › Variable Sets", and matches the existing kebab-case multi-word convention (worker-pool,project-group,build-information). Downside: long to type, hence thelvsalias.variable-set— shorter, but ambiguous with project variable sets and withoctopus project variable.library variable-set— alibraryparent for futurelibrary certificate,library script-module,library step-templateetc. Arguably the tidiest long-term shape, but it commits us to alibrarynamespace now, and none of the sibling commands exist.Recommendation: keep
library-variable-set. If we later want alibraryparent we can add it and alias.2. Should variable editing live in the CLI at all?
Versionfor optimistic concurrency; a CLI that silently clobbers a concurrent edit would be worse than no CLI. It also needs sensitive-value handling (the server never returns them, so a naive round-trip would blank them).set-valuefor an existing variable identified by name + scope, which is the common case and can be done as a targeted read-modify-write with the version echoed back.Recommendation: ship read-only now; do the middle ground next, with explicit conflict detection on
Version, before attempting general create/delete. I'd like a call on whether we're comfortable with the CLI writing variable sets at all before I build it.3. How should scoping be displayed and filtered? Currently: names resolved from
ScopeValues, joined asEnvironment: Production, Test; Role: web-server, with(unscoped)for the empty scope, and the only filter is on variable name.Open sub-questions:
viewgain scope filters (--environment-scope,--role-scope, …) matching the flag namespkg/cmd/project/variables/sharedalready uses for writes? Consistent, but it's seven more flags on a read command and the semantics of "matching" a partial scope are not obvious (exact set match? contains?).Scopewith names,ScopeIdswith raw IDs) is more useful for tooling but noisier.grep/awk.-f jsonis the answer for machines, so I chose readability — but it is a judgement call.Recommendation: leave the filters as name-only for now; add
ScopeIdsto the JSON if anyone is actually scripting against it. Happy to change the table blanking if reviewers prefer repetition.🤖 Generated with Claude Code