-
Notifications
You must be signed in to change notification settings - Fork 111
fine grained rbac #7962
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’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 2.14_stage
Are you sure you want to change the base?
fine grained rbac #7962
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: swopebe The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
release_notes/acm_whats_new.adoc
Outdated
@@ -51,6 +51,8 @@ See link:../console/console_intro.adoc#web-console[Web console] for more informa | |||
|
|||
For new features that are related specifically to {mce-short}, see link:../clusters/release_notes/mce_whats_new.adoc#whats-new-mce[What's new for Cluster lifecycle with {mce-short}] in the _Cluster_ section of the documentation. | |||
|
|||
- Grant more specific permissions for virtual machines with fine-grained role base access for you virtual machines. As a cluster administrator, you can manage and control permissions at the namespace level on managed clusters rather than at the cluster level. See link:../secure_clusters/fine_grain_rbac.adoc/#fine-grain-rbac[Managing fine-grained role-based access control] for information. |
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.
This is the release note.
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.
need to just add TP status here
…nternal doc that is close to what is in the demo, prepared steps based on demo.
…nternal doc that is close to what is in the demo, prepared steps based on demo.
…nternal doc that is close to what is in the demo, prepared steps based on demo.
From the End to End testing google doc, this pre-req part is missing. This is mandatory, or else the kubevirt roles will not be present on the hub cluster. Copying directly from the google doc:
oc get policy -n open-cluster-management-global-set
oc label managedclusters local-cluster environment=virtualization
oc edit policy -n open-cluster-management-global-set policy-virt-clusterroles CHANGE THIS: Note: this will get changed back to inform automatically after some time. This is expected, and we only need it set to enforce for a short time period just so the clusterroles can be added to the hub cluster. |
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.
Added multiple changes and considerations. Let me know if there is anything I can do to further assist!
secure_cluster/fine_grain_rbac.adoc
Outdated
. In your `MultiClusterHub` custom resource `spec.overrides.components` field, set `search` to `enabled` to retrieve a list of managed clusters namespaces that can represent virtual machines that are used for access control. | ||
. Create the `ClusterRoles` resource. If you installed operators that create cluster roles for you, you already have this requirement. | ||
. On the hub cluster `ClusterRole` resource, add the `rbac.open-cluster-management.io/filter: vm-clusterroles` label so that you see cluster roles in the console when you create or edit cluster permissions. | ||
. Ensure you have virtual machines by creating or migrating them on the hub cluster, which gives the managed cluster namespaces. |
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.
Not sure what this means: "which gives the managed cluster namespaces.". Maybe we can remove this line, unless I am missing something.
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.
I was taking content from the issue and watching multiple demos, so at some point it likely was mentioned. Not sure. I can remove it.
secure_cluster/fine_grain_rbac.adoc
Outdated
|
||
. As a cluster administrator, click *AccessControlManagement* > *Add Access Control* in the console. | ||
. *Optional:* Click the *YAML* option on to see the metadata that you enter populate in the *Access Control YAML* window. | ||
. In the _Basic information_ window, add the information for your cluster permission, such as the cluster and the user or group. Choose the cluster or virtual machine with the specific namespace that requires permission. |
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.
. In the _Basic information_ window, add the information for your cluster permission, such as the cluster and the user or group. Choose the cluster or virtual machine with the specific namespace that requires permission. | |
. In the _Basic information_ window, add the information for your cluster permission, such as the cluster and the user or group. Choose the cluster or specific virtual machine namespaces that the user requires permission for. |
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.
Will work on this line, ideally not to end with the word for, but keep the same meaning.
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.
In formal technical doc, we do not end the sentence with a preposition, so see how that works. The line previously did say something similar, though.
secure_cluster/fine_grain_rbac.adoc
Outdated
. As a cluster administrator, click *AccessControlManagement* > *Add Access Control* in the console. | ||
. *Optional:* Click the *YAML* option on to see the metadata that you enter populate in the *Access Control YAML* window. | ||
. In the _Basic information_ window, add the information for your cluster permission, such as the cluster and the user or group. Choose the cluster or virtual machine with the specific namespace that requires permission. | ||
. Add the `Role Bindings` information, such as the namespaces in the cluster or virtual machine, users or groups, and roles, such as `kubevirt.io:view` for fine-grained role-based access control. You can choose a combination of `RoleBindings`. |
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.
. Add the `Role Bindings` information, such as the namespaces in the cluster or virtual machine, users or groups, and roles, such as `kubevirt.io:view` for fine-grained role-based access control. You can choose a combination of `RoleBindings`. | |
. Add the `Role Bindings` information, such as the namespaces in the cluster, users or groups, and roles, such as `kubevirt.io:view` for fine-grained role-based access control. You can choose a combination of `RoleBindings`. |
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.
Users choose namespaces, but not virtual machines
secure_cluster/fine_grain_rbac.adoc
Outdated
. Check for a `Ready` status in the console, though for the Technology Preview, you might not see a valid status for all possible `ClusterPermissions` combinations. | ||
. Click *Edit access control* to edit the `Role Bindings` and `Cluster Role Binding`. | ||
. *Optional:* Click *Export YAML* to use the resources | ||
//why would they do this? |
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.
Just to answer your question, admins can export yaml and use it as a template for creating k8s resources through git ops or cli.
secure_cluster/fine_grain_rbac.adoc
Outdated
//why would they do this? | ||
. You can delete the `ClusterPermissions` resource when you are ready. | ||
|
||
See the following example of the `ClusterRole` resource, with roles for {ocp-virt-short} and the `ClusterRoleBinding`resource: |
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.
This should be worded differently. It is not really meant to be shown as an example, but these need to be applied on the hub cluster as workarounds to bugs and limitations. These need to be applied before the user that is getting permissions assigned to them logs into the hub, or else their user experience will be poor.
See the notes I added in from the End to End Testing doc. Customer needs to change cluster names and subjects based on the users/groups they are adding for RBAC through ClusterPermission.
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.
We have tasks and concepts in the RBAC folder. We don't document known issues or workarounds within those procedures.
This content was taken from the template and the demos, but the content at the bottom in the google doc was not in the template, so I didn't see it.
This doc will look something like this formally:
https://docs.redhat.com/en/documentation/red_hat_advanced_cluster_management_for_kubernetes/2.13/html/multicluster_engine_operator_with_red_hat_advanced_cluster_management/mce-acm-integration#discover-hosted-acm
(this is why we provide links in the doc issue template so that everyone knows how we format the doc)
Notes are specific, steps are imperative for all users, if a workaround goes into the main doc, it goes in as a step bc it is for all readers.
If this YAML is a workaround, that needs to be identified in a separate issue perhaps for troubleshooting. This issue is for the initial documentation procedure.
Limitations, known issues, etc...those are release notes. T-shooting is also an option. We go in to this here in the process doc:
https://docs.google.com/document/d/1YTqpZRH54Bnn4WJ2nZmjaCoiRtqmrc2w6DdQxe_yLZ8/edit?tab=t.0#heading=h.9fvyr2rdriby
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.
Sorry for the confusion. I called this "workaround" because to me that is what it is. However in reality, it is a setup requirement. The yaml I provided needs to be applied for the feature to work correctly.
secure_cluster/fine_grain_rbac.adoc
Outdated
= Implementing fine-grained role-based access control (Technology Preview*) | ||
|
||
{acm} supports fine-grained role-based access control (RBAC). As a cluster administrator, you can manage and control permissions at the namespace level on managed clusters, rather than at the cluster level. Grant permissions to a namespace without granting permission to the entire managed cluster, or virtual machine, to further secure your clusters. | ||
|
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.
Suggestion - rephrasing the last sentence to something like -
Grant permission to virtual machines based on namespaces they belong in a managed cluster. Cannot grant permissions to individual virtual machines, but can grant permissions to all virtual machines in cluster
(yes... we can still do this...)
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.
Please check my comments, I'm concern about https://issues.redhat.com/browse/ACM-21610 since we expect the ticket's PR to be merged by the end of the day 🤞 and to be included on 2.14.0
Co-authored-by: Matt Short <[email protected]>
Co-authored-by: Matt Short <[email protected]>
Co-authored-by: Enrique Mingorance Cano <[email protected]>
Co-authored-by: Enrique Mingorance Cano <[email protected]>
Co-authored-by: Enrique Mingorance Cano <[email protected]>
Co-authored-by: Enrique Mingorance Cano <[email protected]>
Co-authored-by: Enrique Mingorance Cano <[email protected]>
Co-authored-by: Matt Short <[email protected]>
The end-to-end testing doc is a bit hard for me to follow--We didn't get this info in our template, though. That is where I pulled prereqs and much of the content, from the information in the template. From the testing doc, if there is something that needs to be documented, it's likely not in this first draft and needs to be added to the PR. See that there are different prereqs in the issue template: We cannot have a mile long list of prereqs--at that point we may want to consider reorganizing. |
Here are the final steps to enable this feature. These have been revised a lot from what we have currently, so please use these as the new default steps. Lot's of previous steps have been removed because they are no longer needed. I will put CLI steps first and UI steps second, and then we can review them in our next meeting. CLI:
(change the remediationAction farthest to the bottom that has the least indentation, there are 2 and only the bottom one needs to be changed)
OPTIONAL: To enable Grafana for a specific user and cluster (and if observability is enabled)
UI:
(change the remediationAction farthest to the bottom that has the least indentation, there are 2 and only the bottom one needs to be changed)
OPTIONAL: To enable Grafana for a specific user and cluster (and if observability is enabled)
|
Here are CLI instructions for adding access control:
|
. Review and click *Create permission*. | ||
. Check for a `Ready` status in the console, though for the Technology Preview, you might not see a valid status for all possible `ClusterPermissions` combinations. | ||
. Click *Edit permission* to edit the `Role bindings` and `Cluster role binding`. | ||
. *Optional:* Click *Export YAML* to use the resources for GitOps or in the terminal. |
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.
Add ClusterPermission explanation to this
Yes, but I could not move forward with the conflicts in the information I had and still create proper documentation, so we needed clarity and a clearer draft. This content needs to look like the rest of the ACM content. After a couple of meetings, we have the steps ironed out. In the doc process, you can close your issues, since the doc merge is always after the code merges due to workload. This is why the doc team needs their own issues for the release work. You are free to close your issue knowing that we are working the feature doc. |
+ | ||
*Note:* Run `oc get mch -A` to get the name and namespace of the `MultiClusterHub` resource if you do not use the `open-cluster-management` namespace. | ||
|
||
. Label your `local-cluster` with `environment=virtualization`. Run the following command: |
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.
Why does the user do this?
---- | ||
remediationAction: enforce | ||
---- | ||
//I think we should actually break this out a bit to show them. |
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.
It may actually be better to break this out a bit to show them in the YAML--I didn't see an example of how we say this in the doc and we don't want to use directional language.
resources: ["managedclusters"] | ||
verbs: ["get"] | ||
resourceNames: ["cluster01", "cluster02", "cluster03"] <2> | ||
--- |
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.
We originally had ClusterRole and ClusterRoleBinding in two different steps, before the meetings and such. We also have this separated in the console process, but all together in this process.
---- | ||
oc apply -f user-observability-grafana-access.yml | ||
---- | ||
|
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.
The CLI procedure is ready for tech review, see comments.^^^
No description provided.