Add per-resource permissions support to cluster_policies - #6304
Open
Sankalp-Mittal wants to merge 5 commits into
Open
Add per-resource permissions support to cluster_policies#6304Sankalp-Mittal wants to merge 5 commits into
Sankalp-Mittal wants to merge 5 commits into
Conversation
Cluster policies support a single ACL level, CAN_USE (SDK compute.ClusterPolicyPermissionLevel). Wire up the per-resource `permissions` field so a policy can grant CAN_USE to users, groups, and service principals via the direct engine's generic permissions applier: - Declare the ClusterPolicyPermission type and add the Permissions field. - Register cluster_policies.permissions in the direct engine and map the permissions-API object type to /cluster-policies/. - Regenerate the bundle JSON schema and fill in the field descriptions. cluster_policies is intentionally left out of the top-level levelsMap (it stays in unsupportedResources): the bundle-wide permissions block is expressed in CAN_MANAGE/CAN_VIEW/CAN_RUN, none of which a cluster policy can accept, so only the explicit per-resource form is supported. Co-authored-by: Isaac
Add basic and out_of_band_change acceptance tests for the cluster_policies `permissions` field, filtering permission requests with print_requests.py. The basic test surfaced that FixPermissions injects the current user as CAN_MANAGE onto every resource with a permissions field. Cluster policies only accept CAN_USE, so the real backend would reject that. Add cluster_policies to fix_permissions.go's ignoredResources (like secret_scopes) so only the user-specified CAN_USE grants are sent. Co-authored-by: Isaac
Add a Cloud=true test that deploys a policy with CAN_USE (accepted) and then asserts a direct CAN_MANAGE set-permissions call is rejected. Model the rejection in the testserver: the permissions API only accepts CAN_USE for cluster policies, so a non-CAN_USE level returns 400. This keeps the local run and the cloud run in agreement. The backend error text differs between the fake and a real workspace, so it is routed to a LOG file and musterr asserts only that the request fails. Co-authored-by: Isaac
Runs generate-check output: the refschema field list, the level enum (CAN_USE only), and the required-field entry for the new cluster_policies permissions block. Co-authored-by: Isaac
Collaborator
Integration test reportCommit: 504c16d
9 interesting tests: 3 SKIP, 3 KNOWN, 2 RECOVERED, 1 flaky
Top 8 slowest tests (at least 2 minutes):
|
TestInvariantConfigsCoverage requires every resource with a permissions field to have an invariant config exercising it. Attach a CAN_USE grant to the cluster_policies invariant config. Co-authored-by: Isaac
Sankalp-Mittal
marked this pull request as ready for review
August 18, 2026 13:45
Contributor
Approval status: pending
|
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Stacked on #6225 (cluster policy resource). Adds the per-resource
permissionsfield tocluster_policiesso a policy can grant access via the direct engine's generic permissions applier.Cluster policies support a single ACL level —
CAN_USE(SDKcompute.ClusterPolicyPermissionLevel), meaning "may create clusters governed by this policy." There is noCAN_MANAGE/CAN_VIEW/CAN_RUN; policy administration is an admin-only function.Changes
ClusterPolicyPermission, add thePermissionsfield, registercluster_policies.permissionsin the direct engine, and map the permissions-API object type to/cluster-policies/.FixPermissions: addcluster_policiestoignoredResourcesso the deploying user is not auto-injected asCAN_MANAGE/IS_OWNER— cluster policies can't accept it, and the real backend would reject the deploy (same treatment assecret_scopes).CAN_USElevel on a cluster policy returns400, so local and cloud runs agree.levelenum =CAN_USEonly,levelrequired).Design note
cluster_policiesis intentionally not added to the top-levellevelsMap; it stays inunsupportedResources. The bundle-widepermissions:block is expressed inCAN_MANAGE/CAN_VIEW/CAN_RUN(and auto-grants the deployerCAN_MANAGE), none of which a cluster policy can accept. Supporting only the explicit per-resource form avoids silently dropping those grants. Two current-user injection paths had to be handled for this: the top-levellevelsMapand theFixPermissionsmutator.Testing
Acceptance tests under
acceptance/bundle/resources/cluster_policies/permissions/:basic/— deploy applies theCAN_USEgrants, redeploy updates the ACL; requests asserted withprint_requests.py.out_of_band_change/— an out-of-band ACL change is detected bybundle planand reconciled on redeploy.levels/(Cloud=true) —CAN_USEdeploys successfully; a directCAN_MANAGEset-permissionscall is rejected (asserted withmusterr; the backend error differs between the fake and cloud, so it is routed to aLOGfile rather than pinned).Plus the
cluster_policies.permissionsCRUD roundtrip fixture indresources/all_test.go. Unit + acceptance suites pass locally.This pull request and its description were written by Isaac.