search

kubernetes / ingress-nginx

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

The admission webhook is not working as expect: It doesn't prevent to user from creating two ingress resources that have the same host/path combination. #8972

Open cha7ri opened 1 year ago

cha7ri commented 1 year ago

What happened: I did a quick investigation and the issue seems to be here this code check only the first host and return nil if it doesn't overlap. More details below:

The admission webhook is not working as expect: It doesn't prevent to user from creating two ingress resources that have the same host/path combination. Example:

What you expected to happen:

It shouldn't create both ingresses even if we we put cafe.example.com as a second host.

NGINX Ingress controller version (exec into the pod and run nginx-ingress-controller --version.):

-------------------------------------------------------------------------------
NGINX Ingress controller
  Release:       v1.1.0
  Build:         cacbee86b6ccc45bde8ffc184521bed3022e7dee
  Repository:    https://github.com/kubernetes/ingress-nginx
  nginx version: nginx/1.19.9

-------------------------------------------------------------------------------

Kubernetes version (use kubectl version):

  buildDate: "2022-06-15T14:22:29Z"
  compiler: gc
  gitCommit: f66044f4361b9f1f96f0053dd46cb7dce5e990a8
  gitTreeState: clean
  gitVersion: v1.24.2
  goVersion: go1.18.3
  major: "1"
  minor: "24"
  platform: darwin/amd64
kustomizeVersion: v4.5.4
serverVersion:
  buildDate: "2022-06-09T18:22:07Z"
  compiler: gc
  gitCommit: e1318dce57b3e319a2e3fecf343677d1c4d4aa75
  gitTreeState: clean
  gitVersion: v1.21.13-eks-84b4fe6
  goVersion: go1.16.15
  major: "1"
  minor: 21+
  platform: linux/amd64```

- **How was the ingress-nginx-controller installed**:
``` helm ls -A | grep -i ingress
ingress-nginx-public-le             ingress-nginx       49          2022-06-27 11:34:25.658567825 +0000 UTC deployed    ingress-nginx-4.0.12                    1.1.0

How to reproduce this issue:

Create a new cluster using kind

 kind create cluster --name nginx-test --image kindest/node:v1.21.12

Install ingress nginx controller

 kubectl apply -f https://raw.githubusercontent.com/kubernetes/ingress-nginx/controller-v1.3.0/deploy/static/provider/cloud/deploy.yaml

Make sure it's running

kubectl get pods --namespace=ingress-nginx
NAME                                        READY   STATUS      RESTARTS   AGE
ingress-nginx-admission-create-xvr5m        0/1     Completed   0          48m
ingress-nginx-admission-patch-lzp8c         0/1     Completed   1          48m
ingress-nginx-controller-55dcf56b68-2dj8w   1/1     Running     0          48m

Apply the first ingress

echo "apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  name: cafe-ingress
  annotations:
    kubernetes.io/ingress.class: "nginx"
spec:
  tls:
    - hosts:
        - cafe.example.com
      secretName: cafe-secret
  rules:
    - host: "cafe.example.com"
      http:
        paths:
          - path: /tea
            pathType: Prefix
            backend:
              service:
                name: tea-svc
                port:
                  number: 80
          - path: /coffee
            pathType: Prefix
            backend:
              service:
                name: coffee-svc
                port:
                  number: 80
" | kubectl apply -f -

Apply the second ingress

echo "apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  name: cafe-ingress-1
  annotations:
    kubernetes.io/ingress.class: "nginx"
spec:
  tls:
    - hosts:
        - cafe.example.com
      secretName: cafe-secret
  rules:
    - host: "dummy.example.com"
      http:
        paths:
          - path: /tea
            pathType: Prefix
            backend:
              service:
                name: tea-svc
                port:
                  number: 80
    - host: "cafe.example.com"
      http:
        paths:
          - path: /tea
            pathType: Prefix
            backend:
              service:
                name: tea-svc
                port:
                  number: 80
          - path: /coffee
            pathType: Prefix
            backend:
              service:
                name: coffee-svc
                port:
                  number: 80
" | kubectl apply -f -

Check that both ingresses are created

kubectl get ingress
NAME             CLASS    HOSTS                                ADDRESS   PORTS     AGE
cafe-ingress     <none>   cafe.example.com                               80, 443   26m
cafe-ingress-1   <none>   dummy.example.com,cafe.example.com             80, 443   26m

Check that nginx config has changed

POD_NAMESPACE=ingress-nginx
POD_NAME=$(kubectl get pods -n $POD_NAMESPACE -l app.kubernetes.io/name=ingress-nginx --field-selector=status.phase=Running -o jsonpath='{.items[0].metadata.name}')

kubectl exec -it $POD_NAME -n $POD_NAMESPACE -- cat /etc/nginx/nginx.conf | grep "start server"
    ## start server _
    ## start server cafe.example.com
    ## start server dummy.example.com

Edit the second ingress

echo "apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  name: cafe-ingress-1
  annotations:
    kubernetes.io/ingress.class: "nginx"
spec:
  tls:
    - hosts:
        - cafe.example.com
      secretName: cafe-secret
  rules:
    - host: "cafe.example.com"
      http:
        paths:
          - path: /tea
            pathType: Prefix
            backend:
              service:
                name: tea-svc
                port:
                  number: 80
    - host: "dummy.example.com"
      http:
        paths:
          - path: /tea
            pathType: Prefix
            backend:
              service:
                name: tea-svc
                port:
                  number: 80
          - path: /coffee
            pathType: Prefix
            backend:
              service:
                name: coffee-svc
                port:
                  number: 80
" | kubectl apply -f -

You should see the error below 

for: "STDIN": admission webhook "validate.nginx.ingress.kubernetes.io" denied the request: host "cafe.example.com" and path "/tea" is already defined in ingress default/cafe-ingress
k8s-ci-robot commented 1 year ago

@cha7ri: 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.
longwuyuan commented 1 year ago

You can post more accurate reproduce procedure and more real data from your cluster and the controller like nginx.conf and describe output etc

longwuyuan commented 1 year ago

You can also upgrade to the latest release of the controller before you try to gather data

longwuyuan commented 1 year ago

/remove-kind bug

cha7ri commented 1 year ago

@longwuyuan Sorry for not been specific about the reproduce procedure. I updated and tested it locally. please let me know if I have to provide more information.

longwuyuan commented 1 year ago

I think you are saying that the below config should be caught by webhook. Currently I am not sure why it is not catching that. Maybe there should be a check and a test for it. But I think it is a waste of time and resource to deal with the use case implied by your reproduce code. Explaining is below The bigger problem is something else which you don't want to comment on even after I pointed it out or you want to solve some problem that is not clear from the info in this issue.

I pointed out that the below does not seem to meet the ingress object spec ;

...
spec:
  tls:
    - hosts:
        - cafe.example.com
      secretName: cafe-secret
  rules:
    - host: "dummy.example.com"
...

The reason is this explain below ;

% k explain ingress.spec.tls
KIND:     Ingress
VERSION:  networking.k8s.io/v1

RESOURCE: tls <[]Object>

DESCRIPTION:
     TLS configuration. Currently the Ingress only supports a single TLS port,
     443. If multiple members of this list specify different hosts, they will be
     multiplexed on the same port according to the hostname specified through
     the SNI TLS extension, if the ingress controller fulfilling the ingress
     supports SNI.

     IngressTLS describes the transport layer security associated with an
     Ingress.

FIELDS:
   hosts        <[]string>
     Hosts are a list of hosts included in the TLS certificate. The values in
     this list must match the name/s used in the tlsSecret. Defaults to the
     wildcard host setting for the loadbalancer controller fulfilling this
     Ingress, if left unspecified.

   secretName   <string>
     SecretName is the name of the secret used to terminate TLS traffic on port
     443. Field is left optional to allow TLS routing based on SNI hostname
     alone. If the SNI host in a listener conflicts with the "Host" header field
     used by an IngressRule, the SNI host is used for termination and value of
     the Host header is used for routing.

[~] 
% k explain ingress.spec.rules.host
KIND:     Ingress
VERSION:  networking.k8s.io/v1

FIELD:    host <string>

DESCRIPTION:
     Host is the fully qualified domain name of a network host, as defined by
     RFC 3986. Note the following deviations from the "host" part of the URI as
     defined in RFC 3986: 1. IPs are not allowed. Currently an IngressRuleValue
     can only apply to the IP in the Spec of the parent Ingress.
     2. The `:` delimiter is not respected because ports are not allowed.
     Currently the port of an Ingress is implicitly :80 for http and :443 for
     https. Both these may change in the future. Incoming requests are matched
     against the host before the IngressRuleValue. If the host is unspecified,
     the Ingress routes all traffic based on the specified IngressRuleValue.

     Host can be "precise" which is a domain name without the terminating dot of
     a network host (e.g. "foo.bar.com") or "wildcard", which is a domain name
     prefixed with a single wildcard label (e.g. "*.foo.com"). The wildcard
     character '*' must appear by itself as the first DNS label and matches only
     a single label. You cannot have a wildcard label by itself (e.g. Host ==
     "*"). Requests will be matched against the Host field in the following way:
     1. If Host is precise, the request matches this rule if the http host
     header is equal to Host. 2. If Host is a wildcard, then the request matches
     this rule if the http host header is to equal to the suffix (removing the
     first label) of the wildcard rule.
dud225 commented 1 year ago

I confirm the bug, I'm facing the exact same issue you described. I think that the line you highlighted is the culprit, I'd suggest to open a PR to remove it.

tao12345666333 commented 1 year ago

I checked the actual generated rules, the nginx.conf actually generated in the two rules is the same (the part about cafe.example.com)

k8s-triage-robot commented 1 year ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

vaibhav2107 commented 6 months ago

/remove-lifecycle stale