You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Adds grant management for OCI repositories and resources: create, list, and delete under each. Grants are immutable server-side, so there is no update — changing one means deleting it and creating a replacement.
Commands
Repository grants match recipient projects and grant repo:pull. Resource grants match recipient environments and grant resource:export. Shared logic lives in internal/commands/grants, which serves both trees because ocirepos.Grant and resources.Grant are aliases of types.Grant.
These live under repository rather than bundle because nothing in the SDK or GraphQL schema scopes repo grants by artifact type — createRepoGrant takes any OCI repo id, and grants is a field on OciRepo itself. The server does currently reject resource-type repos ("resource-type repos are not grantable"), but that is a runtime restriction that may lift, not a reason to encode it in the command tree.
Recipient conditions
Conditions come from exactly one of three mutually exclusive sources:
--condition key=value, repeatable. Repeating a key unions its values into a closed set; key=* accepts any value as long as the attribute is set.
--conditions-file, taking the JSON form that grant list -o json emits under recipientConditions.
--all-projects / --all-environments for the organization-wide grant.
Requiring that last flag explicitly is deliberate: absence of conditions would otherwise make a typo silently produce the broadest possible grant. A conditions file holding null or {} is rejected for the same reason.
--attributes-style StringToString was not reusable here. pflag only comma-splits when an argument contains two or more =, so -c team=platform,data yields the single value "platform,data", and repeating a key overwrites rather than unions — a closed set is not expressible, and both attempts to write one fail silently.
Help text
Replaces every inline cobra Example field with a helpdoc, so examples live in docs/helpdocs alongside the rest of the CLI's prose. 32 of the 39 inline blocks duplicated a helpdoc that already had an ## Examples section; every example line was diffed against its helpdoc before removal, and the two that were genuinely missing (environment deploy --follow, concrete resource update examples) were merged in. Four commands gained a helpdoc they never had: bundle create, component update, instance destroy, resource delete. No Long: string literals remain in cmd/.
Lint
Disables revive's exported rule and staticcheck's ST1020/ST1021/ST1022. This module ships a binary, not a library, so a comment on an exported symbol is prose rather than generated API documentation — neither its presence nor its "Name ..." phrasing should be mandatory.
Testing
Unit tests cover condition parsing, action validation, grant-id validation, and rendering (97% statement coverage on the new package). Verified end to end against a live organization: all four condition shapes round-trip faithfully through the server (closed set as an array, wildcard as "*", org-wide as an omitted field), grant list -o json output feeds back into --conditions-file unchanged, and every error path behaves. All test data was cleaned up.
Known follow-ups, not addressed here
grant create on a missing source reports the server's message rather than the friendlier one grant list produces. The API returns code: "unknown" on every mutation validation message, so the SDK cannot classify it as ErrNotFound. Fix belongs in the API first, then the SDK; the CLI needs no change.
grant list -o json prints null rather than [] for an empty result. This matches every other list command in the CLI, so fixing it here alone would be inconsistent.
resource grant delete will delete a repository grant and vice versa — the server treats grants uniformly by id, as the SDK documents.
Review of PR #253 — mass repository grant / mass resource grant
Gather context (PR description, changed files)
Read new grants package (internal/commands/grants/*.go)
Read new commands (cmd/repository.go, cmd/resource.go)
Read helpdocs for grant commands
Check tests
Post review feedback
Summary
This is a clean, well-structured addition. The shared grants package (internal/commands/grants) does exactly what the PR description promises — condition parsing, action validation, ID validation, and rendering are all factored out so cmd/repository.go and cmd/resource.go stay thin and near-identical (the //nolint:dupl annotations on newRepositoryGrantCmd/newResourceGrantCmd are justified given the genuinely parallel-but-not-reusable command trees).
Specific things I verified and liked:
ResolveConditions mutual-exclusion logic (internal/commands/grants/conditions.go:19-40) is correct: exactly one of --condition, --conditions-file, --all-* is required, with clear errors otherwise. The wildcard/closed-set conflict detection in ParseConditions (lines 64-77) correctly rejects mixing key=* with key=value in either order, and correctly no-ops on a repeated key=*.
ParseConditionsFile rejecting null/{} (forcing explicit --all-projects/--all-environments) is a good defensive choice, and is well covered by TestParseConditionsFileRejectsWildcard.
FormatConditions sorts keys for stable table output and treats a nil vs. empty-slice condition value identically as the per-key wildcard — this matters for the JSON round-trip case, and TestRenderJSONConditionsRoundTripThroughFile in grants_test.go exercises exactly that path.
ValidateGrantID catching non-UUID input client-side (to avoid a raw GraphQL parse error reaching the user) is a nice touch, with a good error message pointing at the right grant list command per resource type.
Helpdocs are thorough and consistent with the actual flags/behavior in cmd/repository.go and cmd/resource.go — spot-checked several against the generated docs in docs/generated/, and they match.
Test coverage is solid: condition parsing edge cases (whitespace, embedded =/,, wildcard conflicts), action validation, grant ID validation, and both table/JSON rendering paths (including iteration-error propagation) are all covered.
Minor observations (non-blocking)
The three known follow-ups called out in the PR description (friendlier "not found" message on grant create, null vs [] for empty JSON lists, and grant delete not being type-checked) are reasonable to leave out of scope here — they're either blocked on the API/SDK or would be inconsistent with the rest of the CLI to fix in isolation.
Nothing else stood out as a correctness, security, or maintainability concern in the new grants package or the cmd/repository.go / cmd/resource.go wiring.
Note: I wasn't able to run go build/go vet/the test suite in this sandbox (bash tool calls beyond basic git/read commands required approval that wasn't available here), so this review is based on static reading of the diff and existing tests rather than a live build/test run. Given the PR description states unit tests pass at 97% coverage and were verified end-to-end against a live org, and the code reads correctly, I don't have reason to doubt it — but worth a final CI check before merge.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds grant management for OCI repositories and resources:
create,list, anddeleteunder each. Grants are immutable server-side, so there is noupdate— changing one means deleting it and creating a replacement.Commands
Repository grants match recipient projects and grant
repo:pull. Resource grants match recipient environments and grantresource:export. Shared logic lives ininternal/commands/grants, which serves both trees becauseocirepos.Grantandresources.Grantare aliases oftypes.Grant.These live under
repositoryrather thanbundlebecause nothing in the SDK or GraphQL schema scopes repo grants by artifact type —createRepoGranttakes any OCI repo id, andgrantsis a field onOciRepoitself. The server does currently reject resource-type repos ("resource-type repos are not grantable"), but that is a runtime restriction that may lift, not a reason to encode it in the command tree.Recipient conditions
Conditions come from exactly one of three mutually exclusive sources:
--condition key=value, repeatable. Repeating a key unions its values into a closed set;key=*accepts any value as long as the attribute is set.--conditions-file, taking the JSON form thatgrant list -o jsonemits underrecipientConditions.--all-projects/--all-environmentsfor the organization-wide grant.Requiring that last flag explicitly is deliberate: absence of conditions would otherwise make a typo silently produce the broadest possible grant. A conditions file holding
nullor{}is rejected for the same reason.--attributes-styleStringToStringwas not reusable here. pflag only comma-splits when an argument contains two or more=, so-c team=platform,datayields the single value"platform,data", and repeating a key overwrites rather than unions — a closed set is not expressible, and both attempts to write one fail silently.Help text
Replaces every inline cobra
Examplefield with a helpdoc, so examples live indocs/helpdocsalongside the rest of the CLI's prose. 32 of the 39 inline blocks duplicated a helpdoc that already had an## Examplessection; every example line was diffed against its helpdoc before removal, and the two that were genuinely missing (environment deploy --follow, concreteresource updateexamples) were merged in. Four commands gained a helpdoc they never had:bundle create,component update,instance destroy,resource delete. NoLong:string literals remain incmd/.Lint
Disables revive's
exportedrule and staticcheck'sST1020/ST1021/ST1022. This module ships a binary, not a library, so a comment on an exported symbol is prose rather than generated API documentation — neither its presence nor its "Name ..." phrasing should be mandatory.Testing
Unit tests cover condition parsing, action validation, grant-id validation, and rendering (97% statement coverage on the new package). Verified end to end against a live organization: all four condition shapes round-trip faithfully through the server (closed set as an array, wildcard as
"*", org-wide as an omitted field),grant list -o jsonoutput feeds back into--conditions-fileunchanged, and every error path behaves. All test data was cleaned up.Known follow-ups, not addressed here
grant createon a missing source reports the server's message rather than the friendlier onegrant listproduces. The API returnscode: "unknown"on every mutation validation message, so the SDK cannot classify it asErrNotFound. Fix belongs in the API first, then the SDK; the CLI needs no change.grant list -o jsonprintsnullrather than[]for an empty result. This matches every other list command in the CLI, so fixing it here alone would be inconsistent.resource grant deletewill delete a repository grant and vice versa — the server treats grants uniformly by id, as the SDK documents.