Adding read perms for pods and services to DNS01 ClusterRole #7823
Conversation
Signed-off-by: alnoor <alihamzanoor99@gmail.com>
|
@ali-hamza-noor: The label(s) In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
|
/ok-to-test |
Signed-off-by: alihamzanoor <alihamzanoor99@gmail.com>
Signed-off-by: alnoor <alihamzanoor99@gmail.com>
| @@ -308,6 +308,9 @@ rules: | |||
| - apiGroups: [""] | |||
| resources: ["events"] | |||
| verbs: ["create", "patch"] | |||
| - apiGroups: [ "" ] | |||
| resources: [ "pods", "services" ] | |||
| verbs: [ "get", "list", "watch"] | |||
There was a problem hiding this comment.
I think there are some other permissions missing still:
- apiGroups: [""]
resources: ["pods", "services"]
verbs: ["get", "list", "watch"]
- apiGroups: ["networking.k8s.io"]
resources: ["ingresses"]
verbs: ["get", "list", "watch"]
- apiGroups: [ "gateway.networking.k8s.io" ]
resources: [ "httproutes" ]
verbs: ["get", "list", "watch"]
Anyway, I think it would be better to revert this in v1.18 and include a fixed version in v1.19 instead.
Ideally, we could eg. stop these informers.
There was a problem hiding this comment.
Yeah sure. With regards to additional perms for ingresses and httproutes, I think we agreed on not needing those in one of the stand-ups before. Let's talk more on this in the stand-up. I do like the idea of stopping informers.
|
@ali-hamza-noor We discussed this at stand up this morning and I agree with @inteon that we should revert this feature and try again in 1.19. Let's try and figure out a better UX for this feature; something like global.disableHTTP01 and figure out how to disable the Pod informer. That would have an added benefit of saving some memory in the controller because it would no longer need to cache all the Pods in the cluster. |
|
PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
Pull Request Motivation
The DNS01 role needs the read perms as there was a bug (#7827) reported whenever the users were disabling the HTTP01 role
TODO @ali-hamza-noor : add reference to #7666
Kind
/kind bug
Release Note