kubernetes / ingress-nginx

Ingress-NGINX Controller for Kubernetes
https://kubernetes.github.io/ingress-nginx/
Apache License 2.0
17.21k stars 8.2k forks source link

Admission Webhook fined-grained request filtering (matchConditions) #11628

Closed maximumG closed 2 weeks ago

maximumG commented 1 month ago

What do you want to happen?

When running multiple instances of ingress-nginx with different IngressClass (e.g : external, internal) in the same cluster, the admission webhook configuration cannot currently condition calling one or the other webhook based on the IngressClass.

One workaround today is to use the webhook ObjectSelector condition (see here) and to label Ingresses accordingly (external / internal).

apiVersion: admissionregistration.k8s.io/v1
kind: ValidatingWebhookConfiguration
metadata:
  name: ingress-nginx-external-admission
[...]
webhooks:
  - objectSelector:
      my-custom-label: external # or internal

Since Kubernetes v1.30, admission webhook can leverage the matchConditions statement (see here). This new feature allows for fine-grained request filtering based on the IngressClassName field

apiVersion: admissionregistration.k8s.io/v1
kind: ValidatingWebhookConfiguration
metadata:
  name: ingress-nginx-external-admission
[...]
webhooks:
  - objectSelector: {} # not using objectSelector anymore
    matchConditions:
      - expression: object.spec.ingressClassName == "external" # or internal

:information_source: It would be interesting to add support for matchCondition in the ingress-nginx helm chart

Is there currently another issue associated with this?

N/A

Does it require a particular kubernetes version?

Kubernetes v1.30 in stable (beta since v1.28)

k8s-ci-robot commented 1 month ago

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-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
longwuyuan commented 1 month ago

Your issue description does not provide any info that has basic relevance.

This link https://kubernetes.github.io/ingress-nginx/faq/#how-can-i-easily-install-multiple-instances-of-the-ingress-nginx-controller-in-the-same-cluster suggests how to use multiple instances of the controller so any problems on that is likely going to cause multiple reports about using more than one controller in same cluster.

You can answer all the questions asked in the template of a new bug report by editing this issue description.

If you actually copy/paste data from a kind cluster that is a proof of

annot currently condition calling one or the other webhook based on the IngressClass.

because i can install 2 instances of the controller on one single kind cluster and not reproduce the problem you state

strongjz commented 1 month ago

/area helm /assign @Gacko

github-actions[bot] commented 2 weeks ago

This is stale, but we won't close it automatically, just bare in mind the maintainers may be busy with other tasks and will reach your issue ASAP. If you have any question or request to prioritize this, please reach #ingress-nginx-dev on Kubernetes Slack.

Gacko commented 2 weeks ago

Hey,

one issue here might be, that your suggestion and the possibilities opposed by matchConditions do not comply to the Ingress API. Ingress Controllers must lookup IngressClass resources by their controller value. Only Ingress resources for which the controller found IngressClass resources may be reconciled.

If someone would now manually add IngressClass resources, which is totally allowed, they could be taken into account by the Ingress Controller via their controller value while Ingress resources assigned to them might be ignored by the webhook. Also there's no need to actually use the chart. One can also just use the controller image.

The current implementation of simply skipping non-matching Ingress resources in the controller might not be perfect, but it probably also isn't for other webhooks in a cluster and at least ensures all assigned IngressClass resources are always being considered.

After all this is more of an issue of the Ingress API that hasn't changed in years and will be superseded by Gateway API. I therefore ask for your understanding and will close this issue now.

Regards Marco

maximumG commented 2 weeks ago

Thanks @Gacko for your reply. Your answer totally make sense when I read it twice :smiley: . Many thanks for looking into it.