-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
fix ingress rbac roles #806
Conversation
Changes Unknown when pulling c96758a on puja108:patch-1 into ** on kubernetes:master**. |
There are two changes here - one for The other is @aledbf, @weitzj @liggitt - correct me if I'm wrong here, but if we just need So instead of adding Of course, if I'm wrong and we need @puja108 - thanks! |
@@ -69,6 +70,7 @@ rules: | |||
- secrets | |||
verbs: | |||
- get | |||
- update |
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.
you only need access to update a single config map, the one used for the leader election, so that should be in a separate rule, with a resourceNames
stanza listing the particular name of the config map you need to update
I just swept the code under the nginx package and didn't see any calls to get a node... am I missing that? It's not clear to me why the ingress controller would need that |
@liggitt here https://github.com/kubernetes/ingress/blob/master/core/pkg/ingress/controller/controller.go#L295 |
Thanks for the input! As both comments above show (and I was thinking of that, too), we might want to make the roles tighter. I will move the update cm to a separate rule @liggitt thanks for the tip. As for the get nodes, I'll check #798 if it is tight enough and otherwise include the new rule in this PR. |
added the changes, will also update readme now |
This looks good to me. Please squash your commits so it can get merged. |
Using gcr.io/google_containers/nginx-ingress-controller:0.9.0-beta.7 the nginx-controller needs to handle leader-election via configmaps. To perform the leader-election the nginx-controller needs to have the appropiate RBAC permissions. Previously to this fix, the following errors occured: - cannot get configmaps in the namespace "NAMESPACE_PLACEHOLDER". (get configmaps ingress-controller-leader-nginx) - initially creating leader election record: User "system:serviceaccount:NAMESPACE_PLACEHOLDER" cannot create configmaps in the namespace "NAMESPACE_PLACEHOLDER". (post configmaps) fix ingress rbac roles There was 2 things that the current IC (0.9 beta7) needs. The ClusterRole was missing `get nodes`: ``` RBAC DENY: user "system:serviceaccount:kube-system:nginx-ingress-controller" groups [system:serviceaccounts system:serviceaccounts:kube-system system:authenticated] cannot "get" resource "nodes" named "xxx" cluster-wide ``` The Role was missing `update configmaps`: ```RBAC DENY: user "system:serviceaccount:kube-system:nginx-ingress-controller" groups [system:serviceaccounts system:serviceaccounts:kube-system system:authenticated] cannot "update" resource "configmaps" named "ingress-controller-leader-nginx" in namespace "kube-system"``` removed update configmap because of kubernetes#798 rebased on master, moved get nodes to own rule added get nodes to cluster permissions
sqashed |
@puja108 thanks! |
There was 2 things that the current IC (0.9 beta7) needs. (Note that in the pasted api server logs my SA is called
nginx-ingress-controller
and the IC is running inkube-system
)The ClusterRole was missing
get nodes
:The Role was missing
update configmaps
:RBAC DENY: user "system:serviceaccount:kube-system:nginx-ingress-controller" groups [system:serviceaccounts system:serviceaccounts:kube-system system:authenticated] cannot "update" resource "configmaps" named "ingress-controller-leader-nginx" in namespace "kube-system"