Skip to content
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

feat: Remove ClusterRole #169

Merged
merged 2 commits into from
Jun 27, 2023
Merged

Conversation

ChandonPierre
Copy link
Contributor

@ChandonPierre ChandonPierre commented Jun 7, 2023

Some managed k8s offerings do not allow installing cluster scoped resources

This PR allows for an entirely namespace scoped installation (if .Values.serviceAccount.clusterRoleBinding is not provided it defaults to true)

~~ We do this with kustomize today, would be nice to have support in the chart 馃槃 ~~

So after some testing with removing the clusterrole, I can indeed do k auth can-i list customresourcedefinitions --as system:serviceaccount:authentik:authentik successfully, meaning that the role still gives access to read CRDs

Does not appear to be required to list CRDs 馃槃

@ChandonPierre ChandonPierre requested a review from a team as a code owner June 7, 2023 01:29
@BeryJu
Copy link
Member

BeryJu commented Jun 7, 2023

The clusterrole is required for the authentik kubernetes outpost integration to fully work, without it authentik can't detect what is installed in your cluster and if authentik needs to create any special traefik/etc config (also I'm curious what kind of K8s provider doesn't allow for ClusterRoles)

@ChandonPierre
Copy link
Contributor Author

The clusterrole is required for the authentik kubernetes outpost integration to fully work, without it authentik can't detect what is installed in your cluster and if authentik needs to create any special traefik/etc config

Using the namespace scoped Role + the authentik sa token, I'm still able to GET CRDs:

- apiGroups:
- apiextensions.k8s.io
resources:
- customresourcedefinitions
verbs:
- list

For a fully namespace scoped installation, I'm not sure I understand what the ClusterRole is needed for since it seems to provide the same RBAC

rules:
- apiGroups:
- apiextensions.k8s.io
resources:
- customresourcedefinitions
verbs:
- list

(also I'm curious what kind of K8s provider doesn't allow for ClusterRoles)

Currently the CoreWeave Kubernetes offering does not allow for ClusterRoles (though this will be changed in the future)

@BeryJu
Copy link
Member

BeryJu commented Jun 8, 2023

So after some testing with removing the clusterrole, I can indeed do k auth can-i list customresourcedefinitions --as system:serviceaccount:authentik:authentik successfully, meaning that the role still gives access to read CRDs; I'm trying to find this behaviour mentioned in the K8s docs as those mention that a ClusterRole is used to access non-namespaced resources such as the CR definition (https://kubernetes.io/docs/reference/access-authn-authz/rbac/)

With this we could basically remove the ClusterRole completely, since the only reason for its existence was to read the CRDs

@ChandonPierre
Copy link
Contributor Author

With this we could basically remove the ClusterRole completely, since the only reason for its existence was to read the CRDs

Even better 馃槂

@ChandonPierre ChandonPierre changed the title feat: make ClusterRole optional feat: Remove ClusterRole Jun 8, 2023
Revert "feat: make ClusterRole optional"

This reverts commit a95a41b.
@BeryJu BeryJu merged commit 2c53130 into goauthentik:main Jun 27, 2023
1 check passed
@ChandonPierre ChandonPierre mentioned this pull request Jul 3, 2023
BeryJu added a commit that referenced this pull request Jul 13, 2023
BeryJu added a commit that referenced this pull request Jul 13, 2023
* Revert "feat: Remove ClusterRole (#169)"

This reverts commit 2c53130.

* add toggle for clusterrole

* bump version
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.

None yet

2 participants