kubernetes / ingress-nginx

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

Annotation validation and set_by_lua_block snippet for auth-realm annotation value #10713

Open verdel opened 11 months ago

verdel commented 11 months ago

We use the following structure in the Ingress manifests to customize the enabling/disabling of Basic authentication for different path in the requests:

- apiVersion: networking.k8s.io/v1
  kind: Ingress
  metadata:
    annotations:
      nginx.ingress.kubernetes.io/auth-realm: $basic_auth_realm
      nginx.ingress.kubernetes.io/auth-secret: basic-secret
      nginx.ingress.kubernetes.io/auth-type: basic
      nginx.ingress.kubernetes.io/configuration-snippet: |
        set_by_lua_block $basic_auth_realm {
          if ngx.var.uri == "/manifest.webmanifest" then return "off" end
          if ngx.var.uri == "/manifest.json" then return "off" end
          return "Authentication Required"
        }

After enabling the --enable-annotation-validation option for the Ingress Controller, it is not allowed to use the $ symbol in the value of the nginx.ingress.kubernetes.io/auth-realm annotation.

Here is a link to the annotation validation block.

We can use only basic chars and space.

So, we can no longer customize the use of Basic authentication due to the impossibility of using the Lua snippet name as a value for the annotation.

How risky is it in terms of validation to add the $ symbol to the allowed characters for the annotation value?

k8s-ci-robot commented 11 months 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/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
verdel commented 11 months ago

The same validation issue exists for the annotations nginx.ingress.kubernetes.io/permanent-redirect and nginx.ingress.kubernetes.io/temporal-redirect, where we cannot use the nginx embedded variable $request_uri because of this validation functions.