Skip to content

Add mass repository grant and mass resource grant - #253

Merged
chrisghill merged 1 commit into
mainfrom
add-repository-and-resource-grant-commands
Sep 12, 2026
Merged

chrisghill merged 1 commit into
mainfrom
add-repository-and-resource-grant-commands

Conversation

@chrisghill

Copy link
Copy Markdown
Member

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.

@chrisghill

Copy link
Copy Markdown
Member Author

@claude review

@claude

claude Bot commented Sep 11, 2026 •

Copy link
Copy Markdown

Claude finished @chrisghill's task in 2m 7s —— View job


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.

@chrisghill
chrisghill merged commit 0715bc4 into main Sep 12, 2026
6 checks passed
@chrisghill
chrisghill deleted the add-repository-and-resource-grant-commands branch September 12, 2026 00:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants