rewrite-target annotation validation does not allow = and ? characters

[melnikovn]

What happened:

Annotation validation rule is way too strict for rewrite-target not allowing characters like = and ? that are valid url characters:

W0221 10:03:30.532045       7 validators.go:221] validation error on ingress env-test/test: annotation rewrite-target contains invalid value /$1?=$2

The annotation works fine when not validated, so I guess it should also pass the test.

This is relevant for current master, I've checked the rules there: https://github.com/sauterp/ingress-nginx/blob/main/internal/ingress/annotations/rewrite/main.go#L43 https://github.com/sauterp/ingress-nginx/blob/main/internal/ingress/annotations/parser/validators.go#L74

but tested on v1.9.4.

NGINX Ingress controller version: v1.9.4

Kubernetes version: v1.26.12-eks-5e0fdde

How to reproduce this issue:

Ingress object with the following annotation fails:

apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  annotations:
    nginx.ingress.kubernetes.io/rewrite-target: /$1?x=$2
    nginx.ingress.kubernetes.io/use-regex: "true"
  name: test
spec:
  ingressClassName: nginx
  rules:
  - host: test.com
    http:
      paths:
      - backend:
          service:
            name: service-name
            port:
              number: 8080
        path: /test/(test)/(.*)$
        pathType: Prefix

[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](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]

/remove-kind bug

I suspect you have to go simpler on the target as its coming coming from a regexpgroup. If you begin with a regexpgroup as input and then go on to use more regexp, then it may be desired but not practical for the vast majority of users or for practical for implementing regexp in the derived value of rewrite-target

[github-actions[bot]]

This is stale, but we won't close it automatically, just bare in mind the maintainers may be busy with other tasks and will reach your issue ASAP. If you have any question or request to prioritize this, please reach #ingress-nginx-dev on Kubernetes Slack.

[whale2-spread]

/remove-kind bug

I suspect you have to go simpler on the target as its coming coming from a regexpgroup. If you begin with a regexpgroup as input and then go on to use more regexp, then it may be desired but not practical for the vast majority of users or for practical for implementing regexp in the derived value of rewrite-target

I'm sorry, but it's really hard to make sense of the above. Are you suggesting there is another way to implement such a rewrite? Rewriting part of the path as a parameter seems to be an obvious thing people might want to do.

[longwuyuan]

Your expectation is fair. Not contending them.

I think I was trying to state that in my opinion, rewrite is not a K8S KEP Ingress API feature. Rewrite directive is inherent obviously in Nginx.

Since it helps so much, rewrite has been implemented in this controller. But if the implementation is a threat resulting from symbols/characters misuse in value, then either the validations have to be written afresh, only and only for the rewrite annotation (or other such annotations where its fair to expect these characters/symbols). Or those characters/symbols have to be disallowed from being misused into increasing risk.

[longwuyuan]

@whale2-spread we discussed this in today's community meeting. @Gacko has merged a PR to allow characters in the v1.10. and v1.11.x of the controller.

Can you please try with the recent releases and update behaviour.

[Gacko]

This change has not been released, yet, and it only added , to the allowed characters for URLs. Apart from that the mentioned characters should be allowed for URLs in more recent version.

So altogether I'd ask you to re-produce this issue on a more recent controller version, at best v1.11.x, as we are not supporting v1.9.x anymore.