validating webhook should ignore ingresses with a different ingressclass #7546
Comments
k8s-ci-robot
added
the
needs-triage
|
@lazybetrayer: This issue is currently awaiting triage. If Ingress contributors determines this is a relevant issue, they will accept it by applying the The 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/test-infra repository. |
|
What is the use case to create 2 ingress resources with identical rule?
Thanks,
; Long
…On Thu, 26 Aug, 2021, 8:33 AM Kubernetes Prow Robot, < ***@***.***> wrote:
@lazybetrayer <https://github.com/lazybetrayer>: This issue is currently
awaiting triage.
If Ingress contributors determines this is a relevant issue, they will
accept it by applying the triage/accepted label and provide further
guidance.
The triage/accepted label can be added by org members by writing /triage
accepted in a comment.
Instructions for interacting with me using PR comments are available here
<https://git.k8s.io/community/contributors/guide/pull-requests.md>. If
you have questions or suggestions related to my behavior, please file an
issue against the kubernetes/test-infra
<https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:>
repository.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#7546 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABGZVWQWDGIPTFPXHESHJN3T6WVHFANCNFSM5C2MNKZQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email>
.
|
|
/remove-kind bug |
k8s-ci-robot
added
needs-kind
|
One use case as described here would be different ingress controller for internal vs. external traffic. We do this for instance to enforce mTLS auth for external traffic while letting internal talk to the ingress without auth while serving up the exact same content at the exact same path. I'm sure other use cases can be thought of. This has now stopped for us and forced me to pin the chart version to 3.33.0 (last working for us, haven't tested anything after that) and I could not re-instate the previous behaviour following any documentation I could find (I tried this but it's quite hard to understand in the first place). Any advice on how to keep using multiple ingress controllers for the same host+path combination would be highly appreciated. I would also argue that this should remain a bug as previously working behaviour has been broken. Thanks, /kind bug |
k8s-ci-robot
added
kind/bug
|
Thanks. To me myself one aspect is still unclear. I was asking about the
ingress.spec host value that is a fqdn. Like api1.mydomain.com and path /.
Can you elaborate why internal and external ingress will both configure
same api1.mydomain.com and /.
Thanks,
; Long
…On Fri, 27 Aug, 2021, 4:17 AM Nico Engelen, ***@***.***> wrote:
One use case as described here
<https://kubernetes.github.io/ingress-nginx/user-guide/multiple-ingress/#multiple-ingress-nginx-controllers>
would be different ingress controller for internal vs. external traffic. We
do this for instance to enforce mTLS auth for external traffic while
letting internal talk to the ingress without auth while serving up the
exact same content at the exact same path. I'm sure other use cases can be
thought of.
This has now stopped for us and forced me to pin the chart version to
3.33.0 (last working for us, haven't tested anything after that) and I
could not re-instate the previous behaviour following any documentation I
could find (I tried this
<https://kubernetes.github.io/ingress-nginx/#i-have-more-than-one-controller-running-in-my-cluster-and-i-want-to-use-the-new-spec>
but it's quite hard to understand in the first place).
Any advice on how to keep using multiple ingress controllers for the same
host+path combination would be highly appreciated. I would also argue that
this should remain a bug as previously working behaviour has been broken.
Thanks,
Nico
/kind bug
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#7546 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABGZVWSEF2NUEATUODF4NBDT6277VANCNFSM5C2MNKZQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
In my company, a same domain can be mapped to different IPs, one for internal access and one for external access. |
|
Ok thanks. So just fyi, 2 controllers in one cluster are a longtime
supporter and functioning feature so this is not a bug. Both controllers
need to be configured with different ingressClass and the ingress objects
need to be configured with a ingressClassName.
Thanks,
; Long
…On Fri, 27 Aug, 2021, 7:54 AM Wang Zhen, ***@***.***> wrote:
Thanks. To me myself one aspect is still unclear. I was asking about the
ingress.spec host value that is a fqdn. Like api1.mydomain.com and path
/. Can you elaborate why internal and external ingress will both configure
same api1.mydomain.com and /. Thanks, ; Long
… <#m_6536236180590232772_>
On Fri, 27 Aug, 2021, 4:17 AM Nico Engelen, *@*.***> wrote: One use case
as described here
https://kubernetes.github.io/ingress-nginx/user-guide/multiple-ingress/#multiple-ingress-nginx-controllers
would be different ingress controller for internal vs. external traffic. We
do this for instance to enforce mTLS auth for external traffic while
letting internal talk to the ingress without auth while serving up the
exact same content at the exact same path. I'm sure other use cases can be
thought of. This has now stopped for us and forced me to pin the chart
version to 3.33.0 (last working for us, haven't tested anything after that)
and I could not re-instate the previous behaviour following any
documentation I could find (I tried this
https://kubernetes.github.io/ingress-nginx/#i-have-more-than-one-controller-running-in-my-cluster-and-i-want-to-use-the-new-spec
but it's quite hard to understand in the first place). Any advice on how to
keep using multiple ingress controllers for the same host+path combination
would be highly appreciated. I would also argue that this should remain a
bug as previously working behaviour has been broken. Thanks, Nico /kind bug
— You are receiving this because you commented. Reply to this email
directly, view it on GitHub <#7546 (comment)
<#7546 (comment)>>,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/ABGZVWSEF2NUEATUODF4NBDT6277VANCNFSM5C2MNKZQ
. Triage notifications on the go with GitHub Mobile for iOS
https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675
or Android
https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub
.
In my company, a same domain can be mapped to different IPs, one for
internal access and one for external access.
To support this, we deploy two ingress controllers.
Sometimes we create 2 ingress resources with identical rule but different
ingress class, since they are used for different IPs.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#7546 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABGZVWWTTIQW5OCIDS4HSZTT63ZOJANCNFSM5C2MNKZQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
|
Please show the output of ;
Kubectl describe ingressclasses
Kubectl get ing ingressname -o yaml
Thanks,
; Long
…On Fri, 27 Aug, 2021, 8:46 AM Yuan, ***@***.***> wrote:
Ok thanks. So just fyi, 2 controllers in one cluster are a longtime
supporter and functioning feature so this is not a bug. Both controllers
need to be configured with different ingressClass and the ingress objects
need to be configured with a ingressClassName.
Thanks,
; Long
On Fri, 27 Aug, 2021, 7:54 AM Wang Zhen, ***@***.***> wrote:
> Thanks. To me myself one aspect is still unclear. I was asking about the
> ingress.spec host value that is a fqdn. Like api1.mydomain.com and path
> /. Can you elaborate why internal and external ingress will both configure
> same api1.mydomain.com and /. Thanks, ; Long
> … <#m_4995458809013350320_m_6536236180590232772_>
> On Fri, 27 Aug, 2021, 4:17 AM Nico Engelen, *@*.***> wrote: One use case
> as described here
> https://kubernetes.github.io/ingress-nginx/user-guide/multiple-ingress/#multiple-ingress-nginx-controllers
> would be different ingress controller for internal vs. external traffic. We
> do this for instance to enforce mTLS auth for external traffic while
> letting internal talk to the ingress without auth while serving up the
> exact same content at the exact same path. I'm sure other use cases can be
> thought of. This has now stopped for us and forced me to pin the chart
> version to 3.33.0 (last working for us, haven't tested anything after that)
> and I could not re-instate the previous behaviour following any
> documentation I could find (I tried this
> https://kubernetes.github.io/ingress-nginx/#i-have-more-than-one-controller-running-in-my-cluster-and-i-want-to-use-the-new-spec
> but it's quite hard to understand in the first place). Any advice on how to
> keep using multiple ingress controllers for the same host+path combination
> would be highly appreciated. I would also argue that this should remain a
> bug as previously working behaviour has been broken. Thanks, Nico /kind bug
> — You are receiving this because you commented. Reply to this email
> directly, view it on GitHub <#7546 (comment)
> <#7546 (comment)>>,
> or unsubscribe
> https://github.com/notifications/unsubscribe-auth/ABGZVWSEF2NUEATUODF4NBDT6277VANCNFSM5C2MNKZQ
> . Triage notifications on the go with GitHub Mobile for iOS
> https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675
> or Android
> https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub
> .
>
> In my company, a same domain can be mapped to different IPs, one for
> internal access and one for external access.
> To support this, we deploy two ingress controllers.
> Sometimes we create 2 ingress resources with identical rule but different
> ingress class, since they are used for different IPs.
>
> —
> You are receiving this because you commented.
> Reply to this email directly, view it on GitHub
> <#7546 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/ABGZVWWTTIQW5OCIDS4HSZTT63ZOJANCNFSM5C2MNKZQ>
> .
> Triage notifications on the go with GitHub Mobile for iOS
> <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
> or Android
> <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
>
>
|
|
You can see this issue #7538 closed as recently as yesterday, where a user is successfully using 2 controllers in one cluster. I will change this back to kind support for now. If our triaging results in data that proves a bug of some sort, then we can set the bug label again. /remove-kind bug |
k8s-ci-robot
added
kind/support
Let me clarify, we run 2 controllers in one cluster successfully. Both controllers are configured with different ingressClass and the ingress objects are configured with a correct ingressClassName. There are no problems with most ingress resources. ingress classes: apiVersion: networking.k8s.io/v1
kind: IngressClass
metadata:
name: external
spec:
controller: k8s.io/ingress-nginx-external
---
apiVersion: networking.k8s.io/v1
kind: IngressClass
metadata:
name: internal
spec:
controller: k8s.io/ingress-nginx-internalcontroller are running with If we create below resources, apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
name: test1
spec:
ingressClassName: external
rules:
- host: www.example.com
http:
paths:
- backend:
service:
name: test
port:
number: 1111
path: /
pathType: Prefix
---
apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
name: test2
spec:
ingressClassName: internal
rules:
- host: www.example.com
http:
paths:
- backend:
service:
name: test
port:
number: 1111
path: /
pathType: Prefix |
|
Thank you. This manifest coupled with the initial message clarifies the problem. We will have to check why that code to validate was changed. Can you also please confirm that both the manifests in previous message were used to create ingress resources in the same namespace. If yes, it seems you are asking for a feature. can I summarise this as ;
Also, one more request, can you please post the below data to this issue ; |
|
/assign |
|
Hi We are also affected by this issue. I wanted to share our use-case, so that it's clear why this is useful. We are using nginx ingress as "main" ingress to the k8s cluster. Some applications need a more advanced ingress-controllar than what nginx offers and for this we use Skipper. In this case nginx forwards traffic to the Skipper service. Skipper must also be configured using the k8s ingress resources. Therefore you end up with two ingress resources, with the same host and path (and possible in the same namespace) but a different ingress-class. Thanks for taking care of this issue! |
|
I also affected by this issue. One way to avoid error on ingress creating/updating is to disable validating hook and remove it from cluster. |
|
We've got another use case where it's important that Ingress objects are only validated by the controller for the matching ingressClass. controller:
config:
http-snippet: |
proxy_cache_path /tmp/nginx-cache levels=1:2 keys_zone=static-cache:2m max_size=100m inactive=1d use_temp_path=off;Then in the Ingress objects itself we refer to this cache: annotations:
nginx.ingress.kubernetes.io/configuration-snippet: |
proxy_cache static-cache;
...Our second ingress controller is not configured for caching, but still tries to validate the Ingress objects, leading to the Ingress object being rejected by the ingress controller that will never actually handle this Ingress, because it has a different (Ignore the So it definitely looks like a bug in ingress-nginx to me. |
|
Having the same issue in our dev and production clusters. We use a split-horizon DNS where we want all ingresses (hosts and paths) accessible over a VPN but only some ingresses accessible to the public. To achieve this, we've got the same setup as @lazybetrayer - two ingress controllers ( The simplest way to reproduce the issue is: $ helm create nginx
$ helm install nginx ./nginx -n default
$ kubectl apply -f - <<EOF
apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
name: nginx-internal
namespace: default
spec:
ingressClassName: nginx-internal
rules:
- host: nginx.example.com
http:
paths:
- backend:
service:
name: nginx
port:
number: 80
path: /
pathType: ImplementationSpecific
---
apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
name: nginx-external
namespace: default
spec:
ingressClassName: nginx-external
rules:
- host: nginx.example.com
http:
paths:
- backend:
service:
name: nginx
port:
number: 80
path: /
pathType: ImplementationSpecific
EOFWhich then returns the error: ### $ kubectl get ingressclasses -o yaml
apiVersion: v1
items:
- apiVersion: networking.k8s.io/v1
kind: IngressClass
metadata:
annotations:
meta.helm.sh/release-name: nginx-external-ingress-controller
meta.helm.sh/release-namespace: kube-system
creationTimestamp: "2021-10-01T09:11:05Z"
generation: 1
labels:
app.kubernetes.io/component: controller
app.kubernetes.io/instance: nginx-external-ingress-controller
app.kubernetes.io/managed-by: Helm
app.kubernetes.io/name: ingress-nginx
app.kubernetes.io/version: 1.0.3
helm.sh/chart: ingress-nginx-4.0.5
name: nginx-external
resourceVersion: "226814148"
selfLink: /apis/networking.k8s.io/v1/ingressclasses/nginx-external
uid: d3afa791-8fa7-473f-a2ae-c398397e6f4a
spec:
controller: k8s.io/nginx-external
- apiVersion: networking.k8s.io/v1
kind: IngressClass
metadata:
annotations:
ingressclass.kubernetes.io/is-default-class: "true"
meta.helm.sh/release-name: nginx-internal-ingress-controller
meta.helm.sh/release-namespace: kube-system
creationTimestamp: "2021-10-01T09:11:06Z"
generation: 1
labels:
app.kubernetes.io/component: controller
app.kubernetes.io/instance: nginx-internal-ingress-controller
app.kubernetes.io/managed-by: Helm
app.kubernetes.io/name: ingress-nginx
app.kubernetes.io/version: 1.0.3
helm.sh/chart: ingress-nginx-4.0.5
name: nginx-internal
resourceVersion: "226814176"
selfLink: /apis/networking.k8s.io/v1/ingressclasses/nginx-internal
uid: 449d39ee-2990-4d55-abbc-9b750795b919
spec:
controller: k8s.io/nginx-internal
kind: List
metadata:
resourceVersion: ""
selfLink: "" |
|
@rikatz Could you maybe have a look at this issue, also considering the discussion about your changes here: https://github.com/kubernetes/ingress-nginx/pull/7341/files/f5c4dc299c2622dcc0e8ff038dac9cc4b0f4fcbb#diff-4198ec010671801881244a8052177f31bcbc682c99fbd7391bceb136025c0568 Can we have this issue classified as a bug again, please? :-) |
|
/kind bug |
|
@rikatz This was merged but it wasn't released as part of 4.0.17. Any chance we can have it released? |
|
We can make a release |
|
@tao12345666333 do you wanna start the release process? |
|
Yes, I'll handle it |
This version got fix for this: kubernetes/ingress-nginx#7546
This version got fix for this: kubernetes/ingress-nginx#7546
This version got fix for this: kubernetes/ingress-nginx#7546
This version got fix for this: kubernetes/ingress-nginx#7546 Condition to enable external-dns annotation for svc
This version got fix for this: kubernetes/ingress-nginx#7546 Condition to enable external-dns annotation for svc
This version got fix for this: kubernetes/ingress-nginx#7546
|
tested it with chart version 4.0.18, the controller with validating webhooks did still process the one it shouldn't 😢 my setup: two controllers with ingress-classes configured: nginx and nginx-private nginx-controller - admission webhook enabled created ingress for class nginx-private, logs showed that nginx-controller was rejecting it, nginx-private-controller accepting. however on nginx-controller the log stated that admission-webhook will accept this ing-object. then the ingress was stuck no LB assigned. workaround: disabled admission webhooks on both controllers it seems to work now. |
|
It works for me. I will double check if I have both admission webhooks enabled just in case. |
|
Can confirm that our setup (#7546 (comment)) after upgrading to 1.1.2 (Chart v4.0.18) and enabling admission webhooks, is working exactly as expected. Unless we use the deprecated |
|
the annotation kubernetes.io/ingress.class is for legacy reason enabled on the controllers. as soon as all ingresses are migrated gonna remove the deprecated annotation feature and check it again, thx! update 01-06-22: we had both ingress-controllers running in the same namespace, only the controller which was started first was able to bind the loadbalancer to the ingress-object we did some testing and after separating the controllers into their own namespaces (nginx-private, nginx-public), everything works like a charm. update 08-06-22: |
@mkreidenweis-schulmngr Do you know if there is a workaround for this? I'm seeing this issue just now where I have created a second controller to bypass our CDN so we can use mTLS directly. I'm relying on http headers set by nginx in a http-snippet but getting the same error as you've described: My configmap is setup in the same namespace as the 2nd contriller: It seems this issue persists. Am I missing some configuration to get around it? |
|
@jseparovic is it possible for you to provide a step by step instructions process that someone can use on their minikube clsuter or a kind cluster, and reproduce this problem. |
|
@longwuyuan Yep no worries. I'll update this comment once I've put something together |
|
@longwuyuan I cannot seem to reproduce this another cluster, and since the original cluster was reconfigured to not require the configmap on the secondary controller. |
thanks for updating |
|
I am observing this issue or something else masquerading as it: Deleting the internal ingress allows for the external ingress to be modified and then internal ingress is created and it does not have a problem. The code runs in a deployment pipeline. Here are the ingress definitions: The nginx version: Please, let me know what else I can provide to help you to help me. |
|
@MarkKharitonov did you follow these steps already? if so, and they resolve the issue, it would be great if you could PR some docs or at least open a docs issue
|
|
@afirth - Are these steps relevant if the ingresses live in different namespaces, as is my case? It was my understanding from reading that comment that having ingresses in different namespaces eliminates all the issues naturally. Have I missed anything? |
|
This is false alarm. I have found the root cause. It is actually educational. We copied the nginx images to our internal image repo in Azure and never refreshed it while we did refresh the HELM chart deployed. This is a lesson for us to improve this rather broken process. |
and others
NGINX Ingress controller version: v1.0.0
Kubernetes version (use
kubectl version): v1.20.9Environment: Bare Metal
What happened:
Before v1.0.0, there's a check to skip validating ingress with a different ingressclass:
ingress-nginx/internal/ingress/controller/controller.go
Line 224 in f3c5069
In v1.0.0, this code is removed. With multiple ingress controllers, both will validate the same ingress.
If we create two ingresses with same host and path but different ingressclasses, the second one will be rejected.
What you expected to happen:
skip validating ingress with a different ingressclass
/kind bug
The text was updated successfully, but these errors were encountered: