-
Notifications
You must be signed in to change notification settings - Fork 43
馃摑 Support cluster-wide permissions grants in AddonTemplate #176
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
bhperry
wants to merge
1
commit into
open-cluster-management-io:main
Choose a base branch
from
bhperry:addon-template-clusterrolebinding
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
127 changes: 127 additions & 0 deletions
127
enhancements/sig-architecture/233-addon-template-clusterrolebinding/README.md
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,127 @@ | ||
| # Addon Template ClusterRoleBinding | ||
|
|
||
| ## Release Signoff Checklist | ||
|
|
||
| - [ ] Enhancement is `provisional` | ||
| - [ ] Design details are appropriately documented from clear requirements | ||
| - [ ] Test plan is defined | ||
| - [ ] Graduation criteria for dev preview, tech preview, GA | ||
| - [ ] User-facing documentation is created in [website](https://github.com/open-cluster-management-io/open-cluster-management-io.github.io/) | ||
|
|
||
| ## Summary | ||
| Extend AddonTemplate kubeClient registration to support ClusterRoleBindings for addon agent service accounts. A new hubPermission type called "AllNamespaces" will be added along with an associated AllNamespacesBindingConfig which references a ClusterRole in the hub. The addon manager will create a ClusterRoleBinding for each ManagedClusterAddon that binds the ClusterRole to the subject associated with the spoke addon agent. | ||
|
|
||
| ## Motivation | ||
| Currently hubPermissions only supports creating a RoleBinding for CurrentCluster or SingleNamespace. This means that spoke addon agents can't be granted permissions on non-namespaced resources/custom-resources without a hub addon controller that manages the ClusterRoleBinding for ManagedClusterAddons. | ||
|
|
||
| My use-case is for a non-namespaced CRD that describes shared state for the fleet which needs to be distributed out to spoke clusters. The only way for me to do this right now is to add a custom controller in the addon's hub manager that watches ManagedClusterAddons and creates the ClusterRoleBindings itself. | ||
|
|
||
| ## Proposal | ||
| - Add "AllNamespaces" hub permission type, which references a single ClusterRole in the hub | ||
| - Addon Manager will manage ClusterRoleBindings for spoke agent addons with AllNamespaces permissions | ||
| - Each ManagedClusterAddon has its own ClusterRoleBinding named `open-cluster-management:<addon-name>:clusterrole:<cluster-name>:agent` | ||
|
|
||
| ### Design Details | ||
|
|
||
| ### Permission change | ||
|
|
||
| Add `clusterrolebindings` to the `open-cluster-management:{{ .ClusterManagerName }}-addon-manager:controller` ClusterRole | ||
|
|
||
| #### API change | ||
|
|
||
| Add new HubPermissionsBindingType | ||
|
|
||
| ```go | ||
|
|
||
| const ( | ||
| ... | ||
|
|
||
| // HubPermissionsBindingAllNamespaces means the addon agent will have access to resources in any namespace, | ||
| // or to non-namespaced resources on the hub cluster. | ||
| HubPermissionsBindingAllNamespaces HubPermissionsBindingType = "AllNamespaces" | ||
| ) | ||
| ``` | ||
|
|
||
| Add config for AllNamespaces permissions | ||
|
|
||
| ```go | ||
| type HubPermissionConfig struct { | ||
| // Type of the permissions setting. It defines how to bind the roleRef on the hub cluster. It can be: | ||
| // - CurrentCluster: Bind the roleRef to the namespace with the same name as the managedCluster. | ||
| // - SingleNamespace: Bind the roleRef to the namespace specified by SingleNamespaceBindingConfig. | ||
| // - AllNamespaces: Bind the clusterRoleRef to the subject for the managedCluster. | ||
| // | ||
| // +kubebuilder:validation:Required | ||
| // +kubebuilder:validation:Enum:=CurrentCluster;SingleNamespace;AllNamespaces | ||
| Type HubPermissionsBindingType `json:"type"` | ||
|
|
||
| ... | ||
|
|
||
| // AllNamespaces contains the configuration of AllNamespaces type binding. | ||
| // It is required when the type is AllNamespaces | ||
| AllNamespaces *AllNamespacesBindingConfig `json:"allNamespaces,omitempty"` | ||
|
bhperry marked this conversation as resolved.
|
||
| } | ||
|
|
||
| type AllNamespacesBindingConfig struct { | ||
| // ClusterRoleName is the name of the clusterrole the addon agent is bound. A clusterrolebinding | ||
| // will be created referring to this cluster role with subjects for each cluster namespace. | ||
| // The user must make sure the clusterrole exists on the hub cluster. | ||
| // +kubebuilder:validation:Required | ||
| ClusterRoleName string `json:"clusterRoleName"` | ||
| } | ||
|
bhperry marked this conversation as resolved.
|
||
| ``` | ||
|
|
||
| #### hub implementation | ||
| Update `createKubeClientPermissions` in `pkg/addon/templateagent/registration.go` to handle the additional hub permission case `addonapiv1alpha1.HubPermissionsBindingAllNamespaces`. This will call a new method `createPermissionClusterRoleBinding` which is nearly the same as the existing `createPermissionRoleBinding` except that it creates a ClusterRoleBinding instead of a RoleBinding. | ||
|
|
||
| #### examples | ||
|
|
||
| ```yaml | ||
| apiVersion: addon.open-cluster-management.io/v1beta1 | ||
| kind: AddOnTemplate | ||
| metadata: | ||
| name: my-agent | ||
| spec: | ||
| addonName: my-agent | ||
| agentSpec: | ||
| workload: | ||
| manifests: | ||
| - apiVersion: apps/v1 | ||
| kind: Deployment | ||
| metadata: | ||
| name: my-agent-addon | ||
| namespace: open-cluster-management-agent-addon | ||
| spec: | ||
| replicas: 1 | ||
| selector: | ||
| matchLabels: | ||
| addon-agent: my-agent | ||
| template: | ||
| metadata: | ||
| labels: | ||
| addon-agent: my-agent | ||
| spec: | ||
| containers: | ||
| - name: addon-agent | ||
| image: registry.example.com/my-agent:latest | ||
| registration: | ||
| - type: KubeClient | ||
| kubeClient: | ||
| hubPermissions: | ||
| - type: AllNamespaces | ||
| allNamespaces: | ||
| clusterRoleName: my-agent-hub-permissions | ||
| ``` | ||
|
|
||
| ### Test Plan | ||
| - Verify existing AddonTemplates with hub permissions continue to work as before | ||
| - Add AllNamespaces binding for a hub ClusterRole, verify ClusterRoleBinding is created for each ManagedClusterAddon | ||
|
|
||
| ### Graduation Criteria | ||
| N/A | ||
|
|
||
| ### Upgrade Strategy | ||
| Upgrade AddonTemplate CRD. Existing AddonTemplates will not need to be changed. | ||
|
|
||
| ### Version Skew Strategy | ||
| The AllNamespaces hubPermissions type is added on top of existing types. No migration required. | ||
13 changes: 13 additions & 0 deletions
13
enhancements/sig-architecture/233-addon-template-clusterrolebinding/metadata.yaml
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| title: addon-template-clusterrolebinding | ||
| authors: | ||
| - "@bhperry" | ||
| reviewers: | ||
| - "@qiujian16" | ||
| - "@haoqing0110" | ||
| - "@zhujian7" | ||
| approvers: | ||
| - "@qiujian16" | ||
| - "@haoqing0110" | ||
| creation-date: 2026-03-25 | ||
| last-updated: 2026-03-25 | ||
| status: provisional |
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can multiple clusterroles be configured? How to prevent the name collision?
We may need to validate that AllNamespaces typed registration can only have 1?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually I recently noticed there is the same problem in general with hubPermissions. There is a single rolebinding name used. If you try to set multiple entries it causes an error.