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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New ingress-controller should not mandate the cluster level permission on IngressClass #7510

Closed
yong-jie-gong opened this issue Aug 20, 2021 · 17 comments · Fixed by #7578
Closed
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Milestone

Comments

@yong-jie-gong
Copy link
Contributor

yong-jie-gong commented Aug 20, 2021

NGINX Ingress controller version: v1.0.0-beta.1

Kubernetes version (use kubectl version): 1.21.0

Environment:

  • Cloud provider or hardware configuration:

  • OS (e.g. from /etc/os-release): CentOS Linux release 7.6.1810 (Core)

  • Kernel (e.g. uname -a): Linux shc-sma-cd56.hpeswlab.net 3.10.0-957.5.1.el7.x86_64 Basic structure  #1 SMP Fri Feb 1 14:54:57 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux

  • Install tools:

    • Please mention how/where was clsuter created like kubeadm/kops/minikube/kind etc.
  • Basic cluster related info:

    • kubectl version
    • kubectl get nodes -o wide
  • How was the ingress-nginx-controller installed:

    • If helm was used then please show output of helm ls -A
    • If helm was used then please show output of helm -n <ingresscontrollernamepspace> get values <helmreleasename>
    • If helm was not used, then please explain how the ingress-nginx-controller was installed or copy/paste the command used to install the controller below
    • if you have more than one instance of the ingress-nginx-controller installed in the same cluster, please provide details for all the instances
  • Current State of the controller:

    • kubectl -n <ingresscontrollernamespace> get all -A -o wide
    • kubectl -n <ingresscontrollernamespace> describe po <ingresscontrollerpodname>
    • kubectl -n <ingresscontrollernamespace> describe svc <ingresscontrollerservicename>
  • Current state of ingress object, if applicable:

    • kubectl -n <appnnamespace> get all,ing -o wide
    • kubectl -n <appnamespace> describe ing <ingressname>
    • If applicable, then, your complete and exact curl/grpcurl command (redacted if required) and the reponse to the curl/grpcurl command with the -v flag
  • Others:

    • Any other related information like ;
      • copy/paste of the snippet (if applicable)
      • kubectl describe ... of any custom configmap(s) created and in use
      • Any other related information that may help

What happened:

I deploy my ingress-controller in kubernetes 1.21 with namespaced permissions only. with old ingress-controller, everythings is fine. but once upgrade to latest ingress controller 1.0.0, ingress-controller cannot start any more because ingress-controller mandate the cluster level permission on "IngressClass". without this permisison, ingress-controller even fail to start while it is fine in old version. is there someone know it is one bug or intended?

With annotation based ingress-controller, my application can easily deploy in shared k8s environment since namespace permission is good enough. but with new approach, i must ask the k8s administrator to create the cluster level object "IngressClass". this udpate change completely application deployment flavor which force k8s kubernete administrator to create the cluster level resource "IngressClass" for every application deployed in k8s cluster which don't have cluster level permission

Error message:

E0819 08:46:59.449156      20 reflector.go:138] k8s.io/[email protected]/tools/cache/reflector.go:167: Failed to watch *v1.IngressClass: failed to list *v1.IngressClass: ingressclasses.networking.k8s.io is forbidden: User "system:serviceaccount:test1-rw15y:demo-nginx-ingress" cannot list resource "ingressclasses" in API group "networking.k8s.io" at the cluster scope
E0819 08:47:06.075761      20 reflector.go:138] k8s.io/[email protected]/tools/cache/reflector.go:167: Failed to watch *v1.IngressClass: failed to list *v1.IngressClass: ingressclasses.networking.k8s.io is forbidden: User "system:serviceaccount:itsma-rw15y:itom-nginx-ingress" cannot list resource "ingressclasses" in API group "networking.k8s.io" at the cluster scope
I0819 08:47:06.667186      20 healthz.go:244] nginx-ingress-controller check failed: healthz
[-]nginx-ingress-controller failed: reading /tmp/nginx.pid: open /tmp/nginx.pid: no such file or directory

What you expected to happen:

IngressController should start as normal even if ServiceAccount don't have permsison on class level object "IngressClass"

How to reproduce it:

Anything else we need to know:

/kind bug

@yong-jie-gong yong-jie-gong added the kind/bug Categorizes issue or PR as related to a bug. label Aug 20, 2021
@k8s-ci-robot k8s-ci-robot added needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority labels Aug 20, 2021
@longwuyuan
Copy link
Contributor

/remove-kind bug

  • There will be more documentation related to controller v1.X.X when it is released as GA.
  • That documentation will explain the need for a ingressClass.
  • You can use controller v1.X.X with a flag to the controller startup args like --watch-without-ingress-class
  • Old versions of the ingress related api gets deprecated in K8s v1.22 so for we are releasing v1.X.X of the controller with changes.
  • For K8s v1.21 you can continue to use controller v0.X.X
  • The current GA release of controller v0.X.X is v0.48.1 .
  • v1.X.X of the ingress-nginx controller is in beta and ingressClass is documented here https://kubernetes.io/docs/concepts/services-networking/ingress/#ingress-class

Please close the issue and come discuss in kubernetes.slack.com in the ingress-nginx-users channel

@k8s-ci-robot k8s-ci-robot added needs-kind Indicates a PR lacks a `kind/foo` label and requires one. and removed kind/bug Categorizes issue or PR as related to a bug. labels Aug 21, 2021
@akshitgrover
Copy link
Contributor

akshitgrover commented Aug 21, 2021

@longwuyuan
This was discussed in #ingress-nginx-dev channel before @yongjie-gong filed the issue
@rikatz asked to file an issue

Also, I think the issue here is not regarding the use of IngressClass but the hard requirement of Cluster level permissions required for it. Basically, There can be a use case where we need to enable the ingress controller to implement Ingress objects only on a few namespaces and not on all namespaces.

If we decide to fix this issue, I'd like to take this up
Thank you

@rikatz
Copy link
Contributor

rikatz commented Aug 21, 2021

Yeah we need to fix this, at least for v1.0.1.

My suggestion right now is try to fetch the cluster scoped ingress class, if it fails per access denied do not fetch them to store and warn users about current permission not supporting ingress class

We should also start looking at how to support namespaced ingress classes

@rikatz
Copy link
Contributor

rikatz commented Aug 21, 2021

/milestone v1.0.1

@rikatz rikatz added this to the v1.0.1 milestone Aug 21, 2021
@longwuyuan
Copy link
Contributor

#7519

@akshitgrover
Copy link
Contributor

akshitgrover commented Aug 21, 2021

@rikatz
I'd like to take this up
As I understand, We need to do away with the hard requirement of Get permissions for IngressClass objects, If permissions are not present we'll just issue a soft warning in the logs

And for v1.0.1 we'll be supporting Namespaced IngressClasses, Being tracked in #7519 as @longwuyuan tagged in the previous comment. Right?

@longwuyuan
Copy link
Contributor

/kind feature

@k8s-ci-robot k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. and removed needs-kind Indicates a PR lacks a `kind/foo` label and requires one. labels Aug 22, 2021
@rikatz
Copy link
Contributor

rikatz commented Aug 22, 2021

@akshitgrover go for it :)

For now, we should not panic when ingress class is forbidden, but instead disable it

Ping me in slack if you need some help!

@rikatz
Copy link
Contributor

rikatz commented Aug 22, 2021

/kind bug
/remove-kind feature

I think this is a bug, as we don't support namespaced ingress class yet, and we should not stop users of using the ingress-nginx if they don't have cluster wide permission for ingress class :)

@k8s-ci-robot k8s-ci-robot added kind/bug Categorizes issue or PR as related to a bug. and removed kind/feature Categorizes issue or PR as related to a new feature. labels Aug 22, 2021
@akshitgrover
Copy link
Contributor

Sure thing
I'll assign it to myself
/assign

@akshitgrover
Copy link
Contributor

I guess @yongjie-gong has already raised a PR
/unassign
/assign @yongjie-gong

@iamNoah1
Copy link
Contributor

/priority important-soon
/triage accepted

@k8s-ci-robot k8s-ci-robot added priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-priority needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Sep 22, 2021
@rikatz rikatz modified the milestones: v1.0.2, v1.0.3 Sep 26, 2021
@rikatz rikatz modified the milestones: v1.0.3, v1.0.4 Oct 10, 2021
@hickey
Copy link

hickey commented Nov 15, 2021

This seems to still be an issue.

Currently using 1.0.4 installed from the ingress-nginx 4.0.6 helm chart. If rbac.scope is set to true during the helm chart installation, the service account is not able to list IngressClasses and none of the intended Ingresses are processed (log file shows that they have been ignored). RBAC Role (not ClusterRole) gets set correctly with the list, get, watch verbs applied for IngressClasses in both the "" and networking.k8s.io API groups.

Once I change rbac.scope to false, the ingress controller is able to read the IngressClasses and everything works properly.

From the events above, it looks like this PR made it into the 1.0.4 image, but it appears that more testing needs to be done to insure the changes are correct. @rikatz can you confirm that the 1.0.4 image received the changes from this PR? (correction: this is just the issue and #7519 indicates that namespaced IngressClasses may not possible.)

@mmuric-sigsci
Copy link

I'm still getting the same error when working with v.1.0.1. - for the versions v.0.x.x, everything works fine.
Can you please follow up with the progress on this case?

@iamNoah1
Copy link
Contributor

@mmuric-sigsci is it an option for you to upgrade to the latest version?!

@dmakeienko
Copy link

Encountered a similar issue on nginx 1.1.1 with helm chart 4.0.15

@bmv126
Copy link

bmv126 commented Mar 2, 2022

When you use rbac.scope=true (namespace scoped) installation of ingress-nginx, the only way to make controller detect the ingress resource is to use the watchIngressWithoutClass: true when installing ingress-nginx.

Another option is to use the deprecated annotation in ingress resource as mentioned in #8136.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants