helm: use namespaced Role for manager-role when restrictWatchNamespaces is set#450
helm: use namespaced Role for manager-role when restrictWatchNamespaces is set#450aldredb wants to merge 5 commits into
Conversation
…es is set When restrictWatchNamespaces is configured, the manager-role is now emitted as a namespace-scoped Role (one per watched namespace) instead of a ClusterRole. The RoleBinding references kind: Role accordingly. The two inherently cluster-scoped rules (namespaces get, subjectaccessreviews create) remain in the separate manager-cluster-role ClusterRole, which is unchanged. This eliminates the broad ClusterRole that customers with strict RBAC policies object to, while preserving identical effective permissions within the watched namespaces. Default behavior (empty restrictWatchNamespaces) is unchanged.
jaypipes
added
the
rbac
The namespace field already differentiates, so the release namespace prefix is redundant for namespace-scoped resources. Addresses review feedback.
|
|
|
@jaypipes I addressed your comments |
jaypipes
left a comment
jaypipes
left a comment
There was a problem hiding this comment.
@aldredb OK, so there's an issue uncovered by the excellent @Shivs11.
We have this hack/sync-rbac-rules.py script that is intended to keep the different RBAC rules that are specified with kubebuilder code markers in sync with the helm/temporal-worker-controller/templates/rbac.yaml file's contents. As far as we can tell, that script will be broken by this PR -- or the script may end up just overwriting the changes manually introduced in this PR.
So, I'm going to put a hold on merging this particular PR until we can figure out a way forward on this. I suspect we may want to have this PR update the hack/sync-rbac-roles.py script to generate the Role (not ClusterRole) rules entries. For now, though, unfortunately this PR isn't going to make it into the v1.8.1 (chart 0.28.0) release series, sorry!
The Helm template now has two marker pairs: - GENERATED RULES BEGIN/END — full rules for the ClusterRole (default mode) - GENERATED RULES (NAMESPACED) BEGIN/END — rules minus cluster-scoped entries (namespaces, subjectaccessreviews) for the namespaced Role The sync script parses config/rbac/role.yaml, filters out cluster-scoped rules for the namespaced block, and updates both marker pairs. Addresses review feedback about hack/sync-rbac-rules.py compatibility.
Add support for the GENERATED RULES (NAMESPACED) BEGIN/END marker pair. The script now updates both blocks: full rules for the ClusterRole, and filtered rules (excluding namespaces and subjectaccessreviews) for the namespaced Role. No external dependencies — stdlib only.
Tested with Kindly need your review. Hope this PR can still make it to the 0.28.0 chart release |
3 participants
Summary
When
rbac.restrictWatchNamespacesis configured, the manager-role is currently emitted as aClusterRole— only the binding is namespace-scoped (viaRoleBinding). Organizations with strictRBAC policies push back on ClusterRoles even when they're only bound via RoleBinding, because
ClusterRoles are visible cluster-wide and can be bound by anyone with bind permissions.
This PR changes the chart so that when
restrictWatchNamespacesis set:Role(one per watched namespace) instead of aClusterRoleRoleBindingreferenceskind: Roleinstead ofkind: ClusterRolenamespaces get,subjectaccessreviews create) remain in the separatemanager-cluster-roleClusterRole, which is unchangedDefault behavior (empty
restrictWatchNamespaces) is unchanged — stillClusterRole+ClusterRoleBinding.What was verified
helm templaterenders correctly for both default and restricted modeshelm lintpassesrestrictWatchNamespacesproduces one Role + one RoleBinding per namespacenamespaces getandsubjectaccessreviews createexcluded from the namespaced Role (both covered bymanager-cluster-role)