fix `ipallowlist` parser not handling `validation` type errors

[chengjoey]

What this PR does / why we need it:

fix ipallowlist parser not handling validation type errors

Types of changes

• [x] Bug fix (non-breaking change which fixes an issue)
• [ ] New feature (non-breaking change which adds functionality)
• [ ] CVE Report (Scanner found CVE and adding report)
• [ ] Breaking change (fix or feature that would cause existing functionality to change)
• [ ] Documentation only

Which issue/s this PR fixes

fixes #11967

How Has This Been Tested?

Checklist:

• [x] My change requires a change to the documentation.
• [ ] I have updated the documentation accordingly.
• [x] I've read the CONTRIBUTION guide
• [x] I have added unit and/or e2e tests to cover my changes.
• [x] All new and existing tests passed.

[k8s-ci-robot]

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. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[k8s-ci-robot]

Hi @chengjoey. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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-sigs/prow repository.

[netlify[bot]]

✅ Deploy Preview for kubernetes-ingress-nginx canceled.

Name	Link
🔨 Latest commit	f1360bc955457eddc956ad1fc9692ba1a914fb60
🔍 Latest deploy log	https://app.netlify.com/sites/kubernetes-ingress-nginx/deploys/67853c3cc285fb000817962a

[longwuyuan]

@chengjoey thanks for the contribution.

Can you kindly help and copy/paste visual data like ;

- run `make dev-env` in your laptop clone of your fork, in your branch
- kubectl create deploy httpd --image httpd:alpine --port 80
- kubectl expose deploy httpd --port 80
- kubectl create ing httpd --class nginx --rule httpd.local/"*"=httpd:80
- kubectl describe ing httpd
- curl  localhost -H "Host: httpd.local"
- podName=`kubectl -n ingress-nginx get pod | grep -i ingress-nginx-controller | awk '{print $1}'`
- kubectl -n ingress-nginx exec $podName -- cat /etc/nginx/nginx.conf | egrep -i "allow|deny"

[chengjoey]

@chengjoey thanks for the contribution.

Can you kindly help and copy/paste visual data like ;

- run `make dev-env` in your laptop clone of your fork, in your branch
- kubectl create deploy httpd --image httpd:alpine --port 80
- kubectl expose deploy httpd --port 80
- kubectl create ing httpd --class nginx --rule httpd.local/"*"=httpd:80
- kubectl describe ing httpd
- curl  localhost -H "Host: httpd.local"
- podName=`kubectl -n ingress-nginx get pod | grep -i ingress-nginx-controller | awk '{print $1}'`
- kubectl -n ingress-nginx exec $podName -- cat /etc/nginx/nginx.conf | egrep -i "allow|deny"

kubectl describe ing httpd

Name:             httpd
Labels:           <none>
Namespace:        default
Address:          
Ingress Class:    nginx
Default backend:  <default>
Rules:
  Host         Path  Backends
  ----         ----  --------
  httpd.local  
               /   httpd:80 (<none>)
Annotations:   <none>
Events:
  Type    Reason  Age   From                      Message
  ----    ------  ----  ----                      -------
  Normal  Sync    11s   nginx-ingress-controller  Scheduled for sync

curl localhost -H "Host: httpd.local"

<html><body><h1>It works!</h1></body></html>

kubectl -n ingress-nginx exec $podName -- cat /etc/nginx/nginx.conf | egrep -i "allow|deny"

        # allow configuring ssl session tickets
        # allow configuring custom ssl ciphers
                        # Allow websocket connections
                        allow 127.0.0.1;
                        allow ::1;
                        deny all;
                        # Allow websocket connections

@longwuyuan , could you please tell me why need do this, If this is necessary work, I think it should be added to GitHub CI

[longwuyuan]

@chengjoey sorry for the confusion.

• I copy/paste wrong or made a typo
• Please change that "kubectl create ing command like below to create ingress with the whitelist annotation

k create ing httpd --class nginx --rule httpd.local/"*"=httpd:80 --annotation nginx.ingress.kubernetes.io/whitelist-source-range='1.1.1.1 2.2.2.2'

• Then the grep from nginx.conf will show if that annotation made any changes to nginx.conf inside pod

• If you see the issue I have 3 pieces of data on this

  • The admission controller allows the rule

  • The nginx.conf has a comment explaining its a bad directive config

  $ kubectl -n ingress-nginx exec ingress-nginx-controller-6494fbd8fb-25r7f -- cat /etc/nginx/nginx.conf | grep -i 1.1.1.1
  # Location denied. Reason: "the annotation does not contain a valid IP address or network: invalid CIDR address: 1.1.1.1 2.2.2.2"
  

  • There is a relevant message in controller logs

  E0912 17:31:03.025233 7 annotations.go:216] "error reading Ingress annotation" err="the annotation does not contain a valid IP address or network: invalid CIDR address: 1.1.1.1 2.2.2.2" name="Allowlist" ingress="default/httpd"
  

• So requested you for similar data to make it visually clear what happens after your changes on these 3 aspects

• Thank you for your contribution

[longwuyuan]

I can/could clone your branch and check but I am hoping to get it visually explained here for all so that it adds value on the lines of why did this slip through and yet stop any real-damage by not getting inserted into nginx.conf. Is it possible that Nginx reconcile stopped the damage but admissionController needs a n rule or improvement.

[chengjoey]

• Please change that "kubectl create ing command like below to create ingress with the whitelist annotation

k create ing httpd --class nginx --rule httpd.local/"*"=httpd:80 --annotation nginx.ingress.kubernetes.io/whitelist-source-range='1.1.1.1 2.2.2.2'

k create ing httpd --class nginx --rule httpd.local/"*"=httpd:80 --annotation nginx.ingress.kubernetes.io/whitelist-source-range='1.1.1.1 2.2.2.2'

error: failed to create ingress: admission webhook "validate.nginx.ingress.kubernetes.io" denied the request: annotation nginx.ingress.kubernetes.io/whitelist-source-range contains invalid value

[chengjoey]

• Please change that "kubectl create ing command like below to create ingress with the whitelist annotation

k create ing httpd --class nginx --rule httpd.local/"*"=httpd:80 --annotation nginx.ingress.kubernetes.io/whitelist-source-range='1.1.1.1 2.2.2.2'

k create ing httpd --class nginx --rule httpd.local/"*"=httpd:80 --annotation nginx.ingress.kubernetes.io/whitelist-source-range='1.1.1.1 2.2.2.2'

error: failed to create ingress: admission webhook "validate.nginx.ingress.kubernetes.io" denied the request: annotation nginx.ingress.kubernetes.io/whitelist-source-range contains invalid value

@longwuyuan , This result should meet expectations? because we hope that adminssion-webhook can verify and return errors when creating ingress

[strongjz]

There no fix in here? this justs a test?

The issue in https://github.com/kubernetes/ingress-nginx/issues/11967

is caught by the admission controller?

[chengjoey]

There no fix in here? this justs a test?

The issue in #11967

is caught by the admission controller?

error: failed to create ingress: admission webhook "validate.nginx.ingress.kubernetes.io" denied the request: annotation nginx.ingress.kubernetes.io/whitelist-source-range contains invalid value

@strongjz , the invalid source range has been caughted by the admission controller after this pr fix

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: chengjoey Once this PR has been reviewed and has the lgtm label, please assign gacko for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

[strongjz]

/ok-to-test

[tvalchev2]

@strongjz When will this be merged/approved? We also really need this bugfix and are constantly running in such problems when using whitelist-source-range. Both whitelist-source-range IP Whitelisting and configuration-snippet Whitelisting go through the validation webhook, but only configuration-snippet Whitelisting goes through the Admission webhook, while whitelist-source-range slips through it even if it fails the validation.

This forces us to use configuration-snippet for whitelisting (I know - bad practice), until this PR has been approved and merged.