helm: clusterrole to add objectbucketclaims to user facing roles #12329

jouve · 2023-06-05T18:02:04Z

Description of your changes:

create rbac to view/edit objectbucketclaim and aggregate to user facing roles

view/edit/admin clusterroles are bound to a user & namespace, to manage said namespace, and are now able to create a bucket with this change.

example for cert-manager or elastic-operator

Which issue is resolved by this Pull Request:
Resolves #

Checklist:

Commit Message Formatting: Commit titles and messages follow guidelines in the developer guide.
Skip Tests for Docs: If this is only a documentation change, add the label skip-ci on the PR.
Reviewed the developer guide on Submitting a Pull Request
Pending release notes updated with breaking and/or notable changes for the next minor release.
Documentation has been updated, if necessary.
Unit tests have been added, if necessary.
Integration tests have been added, if necessary.

travisn

The OBCs already have rbac defined in the rook-ceph-object-bucket clusterrole. Do we just need to make the labels on the clusterrole configurable? The new clusterroles seem duplicative of the existing clusterrole.

jouve · 2023-06-05T21:28:34Z

this clusterrole grants too many things as it is "Used for provisioning ObjectBuckets (OBs) in response to ObjectBucketClaims (OBCs) ", (ie. it is used by the operator to implement an obc into an ob+secret)

the idea of those "user facing" clusterrole (view,edit,admin) are to be granted to a user to manage his application in a namespace (with a RoleBinding), so only namespaced resources [1], to consume those resources.

in "plain" words, as a user limited to a namespace, I need a bucket (obc) and attach it to my application (using the AK/SK in the secret created with the ob).

The parallel of pv/pvc/storageclass in the default "view/edit" clusterrole (kubectl get clusterrole view) :

  apiGroups:
  - ""
  resources:
  ...
  - persistentvolumeclaims
  - persistentvolumeclaims/status
  verbs:
  - get
  - list
  - watch

with only persistentvolumeclaims and not persistentvolume or storageclass.

definition of edit/admin/view:

Allows admin access, intended to be granted within a namespace using a RoleBinding.
If used in a RoleBinding, allows read/write access to most resources in a namespace, including the ability to create roles and role bindings within the namespace.
Allows read/write access to most objects in a namespace.
This role does not allow viewing or modifying roles or role bindings.
Allows read-only access to see most objects in a namespace. It does not allow viewing roles or role bindings.

[1]: I did not include resources like cephblockpools, while they are namespaces, I don't consider a "standard" user (ie. not a cluster admin) would deploy those anyway

jouve · 2023-06-09T15:52:27Z

regarding the Integration test CephUpgradeSuite,
as the change in this MR is disabled by default,
I think it's safe to say that the failure an unrelated

BlaineEXE · 2023-06-09T18:06:04Z

@jouve could you provide more context about the specifics of what these changes intend? I don't doubt that many of our RBAC resources could be improved a little, but I'm concerned that these changes might be specifically targeted toward users in your cluster and not something that would be helpful for all Rook users.

jouve · 2023-06-10T10:39:31Z

Let's assume 2 kinds of users in a cluster:

admin : operates the cluster, has access to everything, equivalent of root in linux
standard (for lack of a better word): operates applications in one (or many) namespace, (like non-root users in linux).

k8s defines roles, that it calls "user facing roles" to support this model : https://kubernetes.io/docs/reference/access-authn-authz/rbac/#user-facing-roles

cluster-admin, bound to system:masters group by default with a clusterrolebinding, it's grant access to every operation on every resource in every namespace
admin, intended to be bound with a rolebinding (so namespaced), it allow the "standard" user to manage resources (so namespaced kind) in his namespace: pod (pod,cm,secret,persistentvolumeclaims, ...), apps (deploy,sts,...), etc

k8s also completes "admin" with 2 other roles:

view: read-only role
edit: like admin, except it can't grant role/rolebinding

To make this easier to use with custom resources, k8s does embed a controller so that clusterrole with the annotation rbac.authorization.k8s.io/aggregate-to-{admin/edit/view}: "true" are "merged" with the admin/edit/view defined above.

that way, the admin only need to grant admin/edit/view to "standard" users once, and fix admin/edit/view roles for new custom resources (using the mechanism described above).

=> this model is provided by k8s, not "my" model.

objectbucketclaim being the object storage equivalent of pvc (pvc is in the admin/edit/view) , I think it's safe to add have the feature to add it to the 3 roles admin/view/edit (disabled by default in this PR for backward compatibility).

for example: a "standard" (with role admin in his namespace) user deploy nextcloud (a file hosting app, like google drive) in his namespace, nextclound can use filesystem storage or object storage as a backend:

the user creates a pvc, mount it in the pod, configure the app for local storage
the user creates a obc, inject the secret in the pod, configure the app for object storage

github-actions · 2023-07-10T20:02:08Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in two weeks if no further activity occurs. Thank you for your contributions.

github-actions · 2023-07-25T20:02:13Z

This pull request has been automatically closed due to inactivity. Please re-open if these changes are still required.

parth-gr · 2023-07-26T06:37:59Z

@jouve any updates

jouve · 2023-07-26T07:47:25Z

thanks for reopening :)

what would be needed to progress ?

parth-gr · 2023-07-26T07:50:47Z

Is there changes needed to add or you are waiting for the reviews

jouve · 2023-07-26T12:02:44Z

I replied to @travisn and @BlaineEXE. Idk if they were convinced ?!

github-actions · 2023-08-26T20:02:05Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in two weeks if no further activity occurs. Thank you for your contributions.

travisn · 2023-09-15T17:08:30Z

deploy/charts/rook-ceph/values.yaml

@@ -55,6 +55,10 @@ logLevel: INFO
 # -- If true, create & use RBAC resources
 rbacEnable: true

+rbacAggregate:
+  # -- If true, create a ClusterRole aggregated to [user facing roles](https://kubernetes.io/docs/reference/access-authn-authz/rbac/#user-facing-roles)
+  enabled: false


The roles are currently very specific to OBCs. Perhaps this setting should be more specific to OBCs, and if any other RBAC is aggregated in the future, they would have separate settings. What about this?

rbacAggregate: enableOBCs: true

BlaineEXE · 2023-09-15T17:55:08Z

deploy/charts/rook-ceph/templates/aggregate-roles.yaml

@@ -0,0 +1,35 @@
+{{- if .Values.rbacAggregate.enabled }}
+apiVersion: rbac.authorization.k8s.io/v1


Are these roles automatically added to any k8s user in the view/edit group, or are they only enabled for users after more admin control? I wonder if it could be easier to include these as default roles created by Rook, which would not add complexity to the helm chart. As long as doing so wouldn't accidentally give users extra permissions, maybe we should consider that.

the roles are added to the cluster roles view / edit / admin respectively thanks to the ClusterRole aggregation.

view/edit/admin are default roles of the ClusterRole aggregator.

then a "human" admin binds those role to users in a namespace (using rolebinding), so it's disabled by default to ensure backward compatibility.

travisn

Please squash the commits and also add a description paragraph as described in the commit structure. Otherwise, LGTM

add view & edit ClusterRole for obc. the roles are added to the default user facing roles view / edit / admin respectively using k8s ClusterRole aggregation. feature disabled for backward compatibility and controlled with rbacAggregate.enableOBCs. Signed-off-by: Cyril Jouve <jv.cyril@gmail.com>

helm: clusterrole to add objectbucketclaims to user facing roles (backport #12329)

travisn reviewed Jun 5, 2023

View reviewed changes

github-actions bot added the stale Labeled by the stale bot label Jul 10, 2023

github-actions bot closed this Jul 25, 2023

parth-gr reopened this Jul 26, 2023

jouve force-pushed the aggrole branch from 90c88ac to 91aac04 Compare July 26, 2023 07:44

github-actions bot removed the stale Labeled by the stale bot label Jul 26, 2023

github-actions bot added the stale Labeled by the stale bot label Aug 26, 2023

jouve force-pushed the aggrole branch from 91aac04 to deb1315 Compare September 9, 2023 19:19

github-actions bot removed the stale Labeled by the stale bot label Sep 9, 2023

jouve changed the title ~~helm: clusterrole to aggregated to user facing roles~~ helm: clusterrole to add objectbucketclaims to user facing roles Sep 10, 2023

jouve force-pushed the aggrole branch from deb1315 to 92baf67 Compare September 10, 2023 13:02

travisn reviewed Sep 15, 2023

View reviewed changes

BlaineEXE reviewed Sep 15, 2023

View reviewed changes

travisn requested changes Sep 18, 2023

View reviewed changes

jouve force-pushed the aggrole branch from fd7976e to 45d897b Compare September 19, 2023 08:19

travisn approved these changes Sep 19, 2023

View reviewed changes

travisn merged commit 0db7108 into rook:master Sep 19, 2023
50 checks passed

travisn added the backport-release-1.12 label Sep 19, 2023

mergify bot mentioned this pull request Sep 19, 2023

helm: clusterrole to add objectbucketclaims to user facing roles (backport #12329) #12928

Merged

mergify bot added a commit that referenced this pull request Sep 19, 2023

Merge pull request #12928 from rook/mergify/bp/release-1.12/pr-12329

c83e99a

helm: clusterrole to add objectbucketclaims to user facing roles (backport #12329)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

helm: clusterrole to add objectbucketclaims to user facing roles #12329

helm: clusterrole to add objectbucketclaims to user facing roles #12329

jouve commented Jun 5, 2023 •

edited

travisn left a comment

jouve commented Jun 5, 2023 •

edited

jouve commented Jun 9, 2023

BlaineEXE commented Jun 9, 2023

jouve commented Jun 10, 2023

github-actions bot commented Jul 10, 2023

github-actions bot commented Jul 25, 2023

parth-gr commented Jul 26, 2023

jouve commented Jul 26, 2023

parth-gr commented Jul 26, 2023

jouve commented Jul 26, 2023

github-actions bot commented Aug 26, 2023

travisn Sep 15, 2023

jouve Sep 16, 2023

BlaineEXE Sep 15, 2023

jouve Sep 16, 2023

travisn left a comment

		@@ -0,0 +1,35 @@
		{{- if .Values.rbacAggregate.enabled }}
		apiVersion: rbac.authorization.k8s.io/v1

helm: clusterrole to add objectbucketclaims to user facing roles #12329

helm: clusterrole to add objectbucketclaims to user facing roles #12329

Conversation

jouve commented Jun 5, 2023 • edited

travisn left a comment

Choose a reason for hiding this comment

jouve commented Jun 5, 2023 • edited

jouve commented Jun 9, 2023

BlaineEXE commented Jun 9, 2023

jouve commented Jun 10, 2023

github-actions bot commented Jul 10, 2023

github-actions bot commented Jul 25, 2023

parth-gr commented Jul 26, 2023

jouve commented Jul 26, 2023

parth-gr commented Jul 26, 2023

jouve commented Jul 26, 2023

github-actions bot commented Aug 26, 2023

travisn Sep 15, 2023

Choose a reason for hiding this comment

jouve Sep 16, 2023

Choose a reason for hiding this comment

BlaineEXE Sep 15, 2023

Choose a reason for hiding this comment

jouve Sep 16, 2023

Choose a reason for hiding this comment

travisn left a comment

Choose a reason for hiding this comment

jouve commented Jun 5, 2023 •

edited

jouve commented Jun 5, 2023 •

edited