Optimization: don't use regex location when avoidable

[Congelli501]

History & Current state

Since ingress-nginx 0.22, rewrite target requires the use of regex (#3174 (comment)) and since this change #3078 in version 1.0.0, every location of a server block uses regex if one ingress uses rewrite-target. I use version 1.9.4.

That can lead to inefficiencies, as regex based locations are slower and not indexed (regex are just tested in order).

Rewrite-target usage is quite common for webservice use cases, and the most common use-case is to strip the beginning of the path.
Eg, going from api.example.com/service-a/apiCall to /apiCall for the final service.
We don't need to use regex for that.

location /service-a {
...
   rewrite "(?i)/service-a/(.*)" /$1 break;
...

Would work just fine, without the need of a regex location.

For my use case, we have 400 paths on a location, so regex based routing is not optimal.

What do you want to happen?

Allow to strip the prefix of a path, without relying on a regex location for all locations.

Technical proposals

Proposal 1: via a dedicated option, like nginx.ingress.kubernetes.io/strip-prefix: boolean

That new nginx.ingress.kubernetes.io/strip-prefix: boolean option would work just like the pre-0.22 rewrite-target option: the location would be a fixed stirng prefix, and the rewrite would be automatically populated with rewrite "(?i)/<prefix>(.*)" /$1 break;

Pros:

• clearer option for what is seems to be the most common rewrite-target use-case
• no need to let the user escape regex chars, if the prefix contains dots for example

Cons:

• need for a new option
• cluster administrator intervention would be needed to benefit from this optimisation (need to switch to the new option)

Proposal 1-b: via a dedicated option, like nginx.ingress.kubernetes.io/rewrite-prefix: string

Same as proposal 1, but allow to change the prefix instead of just stripping it.

Pros:

• same as proposal 1
• more flexible

Cons:

• same as proposal 1
• harder to distinguish between rewrite-target and rewrite-prefix

Proposal 2: detect the srip prefix use case

For this second technical proposal, we could check if the prefix ends with (.*) and the rest of the prefix doesn't contain any regex special character.
If that's the case, we can use a string based prefix instead of the regex automatically.

Pros:

• no need for a new option
• optimization is transparent for cluster administrators

Cons:

• additional complexity in ingress-nginx
• any "complex" regex would break the optimisation, like the use of escaped regex char (like dots - \.) in the Prefix

Proposal 3: Add a nginx.ingress.kubernetes.io/rewrite-source: string option

Add a nginx.ingress.kubernetes.io/rewrite-source: string option, that would come along with the nginx.ingress.kubernetes.io/rewrite-target: string option.
That would allow to define the source and target of the rewrite:

location <prefix> {
   rewrite "(?i)/<rewrite-source>" <rewrite target dest> break;

The prefix option would always be considered as a plain string, unless the use-regex option is used.

To make that change non breaking, the new behaviour can be enabled only if nginx.ingress.kubernetes.io/rewrite-source: is defined. If not, consider the Prefix of the ingress as nginx.ingress.kubernetes.io/rewrite-source and force enable use-regex.
The usage of nginx.ingress.kubernetes.io/rewrite-target: string without nginx.ingress.kubernetes.io/rewrite-source: string might be deprecated.

Pros:

• the most flexible: allow complex regex rewrite without relying on regex prefix
• no need to let the user escape regex chars, if the prefix contains dots for example
• only consider the ingress prefix as a regex if the explicit use-regex option is set
• reduce complexity in ingress-nginx

Cons:

• need for a new option
• cluster administrator intervention would be needed to benefit from this optimisation (need to switch to the new option)

If this new feature is added, I would favor this latest proposal.

Is there currently another issue associated with this?

#8425
#10563 (not sure for this one)
#3078

Does it require a particular kubernetes version?

No

Sorry for the long post, I made it as clear as I can

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

[longwuyuan]

cc @rikatz @tao12345666333

[github-actions]

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.