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.

Sign up for GitHub

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

Update RBAC permissions for controller and webhook #1035

Merged
merged 1 commit into from
Jan 14, 2022
Merged

Update RBAC permissions for controller and webhook #1035

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions charts/karpenter/templates/controller/rbac.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ rules:
- apiGroups: [""]
resources: ["events"]
verbs: ["create"]
- apiGroups: ["coordination.k8s.io"]
resources: ["leases"]
verbs: ["create", "get", "patch", "update", "watch"]
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
Expand All @@ -55,23 +58,23 @@ metadata:
{{- include "karpenter.labels" . | indent 4 }}
rules:
- apiGroups: ["karpenter.sh"]
resources: ["provisioners", "provisioners/status"]
resources: ["provisioners"]
verbs: ["get", "list", "watch"]
- apiGroups: ["karpenter.sh"]
resources: ["provisioners/status"]
verbs: ["create", "delete", "patch", "get", "list", "watch"]
- apiGroups: [""]
resources: ["persistentvolumes", "persistentvolumeclaims"]
verbs: ["get", "list", "watch", "update"]
- apiGroups: ["storage.k8s.io"]
resources: ["storageclasses"]
verbs: ["get", "list", "watch"]
- apiGroups: ["coordination.k8s.io"]
resources: ["leases"]
verbs: ["create", "get", "patch", "update", "watch"]
- apiGroups: [""]
resources: ["nodes", "pods"]
verbs: ["get", "list", "watch", "patch", "delete"]
- apiGroups: [""]
resources: ["configmaps"]
verbs: ["get", "list", "watch", "update"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that knative/pkg's configmap controller might need this to update configmaps. We should be able to restrict it to the karpenter namespace.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I see, this is the controller's rbac.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you have in mind as it relates to approving the running workflow?

No rush, just curious to see the workflow run to completion and getting acquainted to the contribution process for this project.

Either way - this should be the first contribution of many since I plan to consistently contribute to this project going forward.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either way - this should be the first contribution of many since I plan to consistently contribute to this project going forward.

Awesome!

Unfortunately, RBAC is not included in the current testing workflow. We run make battletest as part of CI, which stands up a local APIServer/ETCD using controller-runtime's testenv package. In order to test this, you should run make apply (see https://karpenter.sh/docs/development-guide/) and do some basic scaling and provisioner CRUD.

We definitely need to improve this -- I'll cut a ticket.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Were you able to test this manually?

verbs: ["get", "list", "watch"]
- apiGroups: [""]
resources: ["nodes"]
verbs: ["create"]
Expand Down
4 changes: 4 additions & 0 deletions charts/karpenter/templates/webhook/rbac.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,11 @@ rules:
verbs: ["get", "list", "watch"]
- apiGroups: [""]
resources: ["secrets"]
resourceNames: ["karpenter-webhook-cert"]
verbs: ["get", "list", "watch", "update"]
- apiGroups: [""]
resources: ["secrets"]
verbs: ["get", "list", "watch"]
- apiGroups: ["coordination.k8s.io"]
resources: ["leases"]
verbs: ["get", "watch", "create", "update"]
Expand Down