Skip to content

feat(grants): add privilege enums for SPCS objects#40

Open
GClunies wants to merge 1 commit into
datacoves:mainfrom
GClunies:snowcap-34
Open

feat(grants): add privilege enums for SPCS objects#40
GClunies wants to merge 1 commit into
datacoves:mainfrom
GClunies:snowcap-34

Conversation

@GClunies

Copy link
Copy Markdown
Contributor

Summary

Snowcap can create SPCS objects but couldn't grant on them — PRIVS_FOR_RESOURCE_TYPE mapped COMPUTE_POOL, IMAGE_REPOSITORY, and SERVICE to None, so SPCS RBAC still required manual SQL and priv: ALL on these types silently expanded to an empty list (permanent drift). This adds the three privilege enums and wires them in; the generic on_<type> grant dispatch already handled everything else, so no changes were needed in grant.py, data_provider.py, identifiers.py, or blueprint.py.

Closes #34

Changes

  • Add ComputePoolPriv (ALL, MODIFY, MONITOR, OPERATE, OWNERSHIP, USAGE), ImageRepositoryPriv (ALL, OWNERSHIP, READ, WRITE), ServicePriv (ALL, MONITOR, OPERATE, OWNERSHIP, USAGE) and map them in PRIVS_FOR_RESOURCE_TYPE
  • Grants declarable via on_compute_pool:, on_image_repository:, on_service: in YAML/Python now get typed privileges and correct ALL expansion
  • Unit tests: per-enum tests, mapping tests, GrantedPrivilege.from_grant typed-path tests, ALL-expansion regression tests, and a completeness guard so a future grantable ResourceType mapped to None fails CI
  • Integration: single-cycle round-trip test (requires_snowflake) covering USAGE on a compute pool plus ALL on an image repository and service, asserting fetched privileges against hardcoded expected sets and a drift-free re-plan; cleanup registers the billable account-scoped pool first so an aborted cleanup can't leak it
  • Docs: SPCS grant examples in docs/resources/grant.md, grantable-privilege table in docs/snowflake-permissions.md

Note for reviewers: GRANT ALL excluding exactly OWNERSHIP is documented explicitly by Snowflake for compute pools; for image repositories and services the integration test asserts it, but it's worth a live spot-check before release.

Add ComputePoolPriv, ImageRepositoryPriv, and ServicePriv and wire them
into PRIVS_FOR_RESOURCE_TYPE, which previously mapped COMPUTE_POOL,
IMAGE_REPOSITORY, and SERVICE to None. That gap forced manual SQL for
SPCS RBAC: GrantedPrivilege.from_grant fell back to untyped strings and
priv: ALL silently expanded to an empty list, causing permanent drift.

The generic on_<type> grant dispatch already handles these types, so no
changes to grant.py, data_provider.py, identifiers.py, or blueprint.py
were needed. YAML grants via on_compute_pool/on_image_repository/
on_service now round-trip through fetch/plan without spurious drift.

Covered by per-enum unit tests, ALL-expansion regression tests, a
completeness guard so future grantable types can't silently map to
None, and a single-cycle live integration round-trip test. Docs updated
with SPCS grant examples and the grantable privilege lists.
@noel

noel commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Thanks @GClunies — cleanest PR in the batch: minimal, correctly scoped, and it ships a regression guard that pays dividends beyond the immediate fix. I checked out the branch and verified:

  • The enum values match Snowflake's documented SPCS privileges — Compute Pool (MODIFY/MONITOR/OPERATE/USAGE), Image Repository (READ/WRITE), Service (MONITOR/OPERATE/USAGE), each plus ALL/OWNERSHIP.
  • Mapping correctly flips all three from None to the new enums.
  • Ran test_privs / test_grant: 178 passed.

The test_resource_privs_maps_to_priv_enum completeness guard is the highlight — forcing any None mapping to be either given an enum or explicitly allowlisted prevents future regressions of exactly the priv: ALL→empty bug you're fixing here.

Requested before release

  1. Live GRANT ALL spot-check (you flagged this): confirm ALL excludes exactly OWNERSHIP for image repositories and services (documented for compute pools, inferred for the other two).

Follow-ups (out of scope, please file as issues)

  1. The allowlist parks a few genuinely-grantable types that carry the same latent priv: ALL→empty-drift bug — HYBRID_TABLE most clearly (grantable like a regular table), and RESOURCE_MONITOR. Worth issues so they aren't forgotten.
  2. When feat(authentication_policy): support PROGRAMMATIC_ACCESS_TOKEN and PAT_POLICY #41 (authentication policy) lands, remember to remove AUTHENTICATION_POLICY from RESOURCE_TYPES_WITHOUT_PRIV_ENUM.

Looks good

  • Surgical change, no touching of the generic grant paths.
  • Solid coverage: per-enum, mapping, from_grant typed-path, and ALL-expansion regression tests.

Nice work — approve after the live spot-check.

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.

feat(grants): add privilege enums for SPCS objects (COMPUTE POOL, IMAGE REPOSITORY, SERVICE)

2 participants