feedback1 on annotation validation

[bpfoster]

What happened:

I specify an ingress with the annotation:

nginx.ingress.kubernetes.io/auth-tls-error-page: /error/cert-error.html

Previously, this worked and was accepted. As of today, I now receive an error:

admission webhook "validate.nginx.ingress.kubernetes.io" denied the request: annotation nginx.ingress.kubernetes.io/auth-tls-error-page contains invalid value

I am using the deployment resource at https://github.com/kubernetes/ingress-nginx/blob/main/deploy/static/provider/kind/deploy.yaml. Looking at the history, I see the controller was upgraded to 1.12.0-beta.0 yesterday. If I use the version of the file from prior to that commit (https://raw.githubusercontent.com/kubernetes/ingress-nginx/d70b849d253749431aebc13e789c5981ee6f4538/deploy/static/provider/kind/deploy.yaml) it works as expected, so it appears to be related to that change.

What you expected to happen:

I expect an absolute path to be acceptable. The nginx documentation even shows examples where the error_page is configured with an absolute path: https://nginx.org/en/docs/http/ngx_http_core_module.html#error_page

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

NGINX Ingress controller
  Release:       v1.12.0-beta.0
  Build:         80154a3694b52736c88de408811ebd1888712520
  Repository:    https://github.com/kubernetes/ingress-nginx
  nginx version: nginx/1.25.5

Kubernetes version (use kubectl version):

Client Version: version.Info{Major:"1", Minor:"26", GitVersion:"v1.26.11", GitCommit:"3cd242c51317aed8858119529ccab22079f523b1", GitTreeState:"clean", BuildDate:"2023-11-15T17:00:54Z", GoVersion:"go1.20.11", Compiler:"gc", Platform:"linux/amd64"}
Kustomize Version: v4.5.7
Server Version: version.Info{Major:"1", Minor:"26", GitVersion:"v1.26.15", GitCommit:"1649f592f1909b97aa3c2a0a8f968a3fd05a7b8b", GitTreeState:"clean", BuildDate:"2024-08-14T01:01:33Z", GoVersion:"go1.21.8", Compiler:"gc", Platform:"linux/amd64"}

Environment:

• Cloud provider or hardware configuration:

• OS (e.g. from /etc/os-release): RHEL 8.8

• Kernel (e.g. uname -a): Linux 4.18.0-477.67.1.el8_8.x86_64

• Install tools:

  • I am using kind v0.24.0 go1.22.6 linux/amd64

• Basic cluster related info:

  • kubectl version
  • kubectl get nodes -o wide

• How was the ingress-nginx-controller installed:

kubectl apply -f https://raw.githubusercontent.com/kubernetes/ingress-nginx/main/deploy/static/provider/kind/deploy.yaml

• Current State of the controller:

  • kubectl describe ingressclasses
  • 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 <appnamespace> 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

How to reproduce this issue:

Anything else we need to know:

[longwuyuan]

Your observation is accurate.
That beta ships with many breaking changes and one of them is related to the problem you are reporting (validation of annotations).

Please continue to use the v1.11 series.

/remove-kind bug
/kind support
/close

[k8s-ci-robot]

@longwuyuan: Closing this issue.

In response to this:

Your observation is accurate.
That beta ships with many breaking changes and one of them is related to the problem you are reporting (validation of annotations).

Please continue to use the v1.11 series.

/remove-kind bug
/kind support
/close

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.

[bpfoster]

Thank you. I am a little confused - are you saying that in 1.12 onward, absolute paths will not be supported? Or that this is a known issue in the beta that will be resolved?

[longwuyuan]

Sorry for the confusion. The flag --enable-annotation-validation gets enabled by default in this BETA. It is not enabled by default in earlier versions.

[longwuyuan]

More about the changes made here https://github.com/kubernetes/ingress-nginx/releases

[bpfoster]

Ah, I was wondering how the validation started being applied when it had been there for a while.

Do you then think the validation on this annotation could perhaps be less restrictive? I wonder if the regex used on this annotation could use some investigation:

ingress-nginx/internal/ingress/annotations/authtls/main.go

Line 45 in 0edf16f

redirectRegex = regexp.MustCompile(`^((https?://)?[A-Za-z0-9\-.]*(:\d+)?/[A-Za-z0-9\-.]*)?$`)

The regex exhibits some unexpected (to me at least) results in other cases too:

Value	Result
https://foobar	No match
https://foobar/	Match
http://foobar/foo/bar	No Match
foo/bar	Match
/foo	Match
/foo/bar	No Match

https://regex101.com/r/8qa1wO/2

I would expect all of these to be valid values.

[longwuyuan]

/reopen

Thank you for the feedback. The beta was released to get feedback.

@rikatz @tao12345666333 cc for feedback on annotation validation chars

[k8s-ci-robot]

@longwuyuan: Reopened this issue.

In response to this:

/reopen

Thank you for the feedback. The beta was released to get feedback.

@rikatz @tao12345666333 cc for feedback on annotation validation chars

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.

[longwuyuan]

/retitle feedback1 on annotation validation

[longwuyuan]

/kind feature
/remove-kind support
/triage accepted

[strongjz]

/assign @strongjz

[agatheblues]

Hello,

Like the OP I am applying the controller like so:

kubectl apply -f https://raw.githubusercontent.com/kubernetes/ingress-nginx/main/deploy/static/provider/kind/deploy.yaml

Instead of pinning a certain version (1.11) of this file, I thought I could turn off the --enable-annotation-validation by patching the args:

kubectl patch deployment/ingress-nginx-controller -n ingress-nginx --type='json' -p='[{ "op": "add", "path": "/spec/template/spec/containers/0/args/-", "value": "--enable-annotation-validation=false" }]'

But I still get a validation error after the patch:

admission webhook "validate.nginx.ingress.kubernetes.io" denied the request: annotation group ServerSnippet contains risky annotation based on ingress configuration

What am I doing wrong? Shouldn't the addition of the arg be enough to mute the error?

Thank you!

[longwuyuan]

check the related location like a flag in th epod or the configMap to be sure that the helm install configuration was a success.

annotation-validation and server-snippet are not the same config.

[agatheblues]

Thanks, in the end I chose to get rid of our ServerSnippet annotation as it was just used to setup gzip with all the defaults that the use-gzip annotation gives. For others with the same problem that means patching the controller's config map like so kubectl patch configmap/ingress-nginx-controller -n ingress-nginx -p '{"data": {"use-gzip": "true"}}' --type merge.

[strongjz]

https://regex101.com/r/xk9Zd1/1

^(?:https?://)?(?:\b\w+\b)?(?:/\w+)*(?:/)?$

Looks like will match all.

[strongjz]

Well interesting, that the e2e fails

https://github.com/kubernetes/ingress-nginx/actions/runs/11563866613/job/32187991098?pr=12249#step:5:383

[Gacko]

The original RegEx also checks for ports in the host part. No idea if this might break it.