Skip to content

chore: Describe RBAC rules, remove unnecessary rules#717

Draft
NickLarsenNZ wants to merge 2 commits intomainfrom
chore/rbac-review
Draft

chore: Describe RBAC rules, remove unnecessary rules#717
NickLarsenNZ wants to merge 2 commits intomainfrom
chore/rbac-review

Conversation

@NickLarsenNZ
Copy link
Member

Part of stackabletech/issues#798

Note

This was initially generated by a coding assistant to see how well it can inspect code and review the RBAC rules. the changes will be properly checked before reviews are requested.

  • Document each rule
  • Check the docs make sense. Rewrite where necessary
  • Remove unnecessary permissions
  • Attach explanations to PR description
  • Run all tests
  • Split operator and product roles into separate files

Removed permissions

Resource Verbs removed Reason
pods all (create, delete, get, list, patch, update, watch) Operator never manages pods directly; StatefulSets create them
secrets all (create, delete, get, list, patch, update, watch) Operator only references secret names in Pod specs; never calls client.get::<Secret>() or cluster_resources.add(secret)
endpoints all (create, delete, get, list, patch, update, watch) Auto-created by Kubernetes when a Service is created; never managed directly
update (all resources) update Never calls client.update() or api.replace(); all writes use Server-Side Apply (apply_patch) which requires create + patch, not update
serviceaccounts update, watch Not in any .owns() or .watches() call in main.rs; update unused (SSA)
rolebindings update, watch Not in any .owns() or .watches() call in main.rs; update unused (SSA)
poddisruptionbudgets update, watch Not in any .owns() or .watches() call in main.rs; update unused (SSA)
listeners update, watch Not in any .owns() or .watches() call in main.rs; update unused (SSA)
jobs update, delete Jobs are applied via client.apply_patch() directly (not cluster_resources.add()), so they are not tracked in the orphan cleanup set — delete is never exercised; update unused (SSA)
supersetclusters patch Operator only patches the /status subresource, not the main resource
druidconnections patch Operator only patches the /status subresource, not the main resource
duplicate secrets entry secrets was listed twice in the same rule

Restructured rules

Change Reason
druidconnections/status moved to its own rule with only patch Was bundled with druidconnections (inheriting get, list, watch), but /status subresources do not support those verbs meaningfully; now consistent with how supersetclusters/status was already handled
Large combined rule for pods/configmaps/secrets/services/endpoints/serviceaccounts split into per-resource rules Each resource has different verb requirements; grouping them forced the least-privileged resources to receive the most-privileged verb set

Comments added

Every rule in the operator ClusterRole now has a comment explaining its purpose and which code paths require it.

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.

1 participant