Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
verdel opened this issue Dec 1, 2023 · 2 comments
Labels
kind/feature Categorizes issue or PR as related to a new feature. needs-priority needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.

Comments

@verdel
Copy link

verdel commented Dec 1, 2023

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?

@verdel verdel added the kind/feature Categorizes issue or PR as related to a new feature. label Dec 1, 2023
@k8s-ci-robot
Copy link
Contributor

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/test-infra repository.

@k8s-ci-robot k8s-ci-robot added needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority labels Dec 1, 2023
@verdel
Copy link
Author

verdel commented Dec 1, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. needs-priority needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.
Projects
Development

No branches or pull requests

2 participants