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.

cc @rikatz @tao12345666333 @strongjz @Gacko

Because, IIRC, we also have a check for multiple Ingress resources with the same hostname and do not allow them. I'm not 100% sure about this, but if we do have it, then this would mean you can't define another Ingress resource without use-regex anyway.

Maybe I didn't understand correctly, but we do have multiple Ingress resources with the same hostname and it works completely fine.

But even if it's the case. Why turn Exact and Prefix into a regex matcher instead of leaving them as is.

While being "expected" behavior it's definitely confusing

#10618

A small example to illustrate.

If you define two ingresses using the same host header and the one ingress is ImplementationSpecific and has the annotation
nginx.ingress.kubernetes.io/use-regex: "true" and the other is an Exact path type, like

---
apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  name: frontend-exact
  namespace: echoserver
spec:
  ingressClassName: nginx
  rules:
  - host: example.com
    http:
      paths:
      - path: /foo
        pathType: Exact
        backend:
          service:
            name: echoserver-exact
            port:
              name: http
---
apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  name: frontend-regex
  namespace: echoserver
  annotations:
    nginx.ingress.kubernetes.io/use-regex: "true"
spec:
  ingressClassName: nginx
  rules:
  - host: example.com
    http:
      paths:
      - path: /[a-z0-9_.-]{3,40}
        pathType: ImplementationSpecific
        backend:
          service:
            name: echoserver-regex
            port:
              name: http

Then the default nginx.tmpl rendering will create a config that looks like

        ## start server example.com
        server {
                server_name example.com ;

                location ~* "^/[a-z0-9_.-]{3,40}" {

                        set $namespace      "echoserver";
                        set $ingress_name   "frontend-regex";
                        …
                }

                location ~* "^/foo" {

                        set $namespace      "echoserver";
                        set $ingress_name   "frontend-exact";
                        …
                }
                ….
        }
        ## end server example.com

And both of the location directives have converted into a regex match - this means that the Exact Pathtype /foo does not have higher priority than the regex path "^/[a-z0-9_.-]{3,40}"

But with the patch we are applying to the nginx.tmpl we get

        ## start server example.com
        server {
                server_name example.com ;

                location ~* "^/[a-z0-9_.-]{3,40}" {

                        set $namespace      "echoserver";
                        set $ingress_name   "frontend-regex";
                        …
                }

                location = /foo {

                        set $namespace      "echoserver";
                        set $ingress_name   "frontend-exact";
                        …
                }
                ….
        }
        ## end server example.com

Notice that location = /foo is no longer a regex match and hence will have higher priority than the location ~* "^/[a-z0-9_.-]{3,40}"

And we/I just cannot understand why this would not be the default behaviour (or maybe have an option for it)

ANY annotation from an ingress, applies to the entire ingress and the impact of the annotation is not limited to just one single path, configured in the ingress. Or so I had assumed AFAIK. I did not think that only the use-regex annotation worked like that. I think annotations are implemented to apply to all paths in the ingress. I am not a developer so it will take a long time to read and understand the code.

The server-block that all that annotations apply to are the server from the ingress.spec.rules.host field.

The location blocks inside the server block are from the ingress.spec.rules.http.paths.path field.

When multiple ingress resources have the same value for the ingress.spec.rules.host field, they are merged in the nginx.conf file, under the same server block.

Following above 3 assumptions, what you are experiencing seems consistent with expected behavior.

I guess you are expecting that this use-regex annotation should do something exceptional and not apply one annotation to ALL the paths, and not apply to all the locations blocks of the server. Because it is messing with the priority for matching the rule to the incoming request headers.

Your expectation seems fair. If I am not wrong some other people have also reported this, in other issues.

And yet there are lots of users who have not reported this problem so I guess they do not have the same use case and/or same conflict of interest with prioritization of the location blocks.

I too will wait for comments and thoughts on this. It will be interesting to learn, how one annotation should NOT apply to all the paths.

/assign

While we use a template to work-around this behavior it'd be better to change the actual go code for it to work in a "more expected way". #10618 mentions the specific place where the search for "any use-regex for host" is happening to make all the paths in the server declaration regex based.

If you want I can try to prepare a PR that would make it behave the way that would make more sense (IMO).

There're two possibilities:

1. Change all paths declared in an ingress to regex if it has the use-regex annotation which IMO would still be strange as it would change Exact paths to use regex based location block. (I'm not an expert in nginx but maybe it makes sense if there's a rewrite)
2. Change only ImplementationSpecific paths to regex based location block which would be the most sane behavior. Especially looking at this table (it doesn't have ImplementationSpecific but the way Exact and Prefix work makes me expect that ImplementationSpecific is more like Prefix rather than having priority over Exact).

Edit:

Also one point in favor of implementing 2 is that it would make the most sense for ImplementationSpecific routes to deviate from the behavior described in the Ingress API documentation. Currently this annotation turns Exact routes into Prefix routes on the whole host which is technically a deviation from the API specification.

Without being a developer, I can just opine that it will not be easy to change the go code behavior, if that go code is generic code path for all annotations. If You want the generic go code to behave differently, just for the use-regex annotation, then its not going to be an improvement.

But all that is assumption so we just wait for other comments.

it will not be easy to change the go code behavior, if that go code is generic code path for all annotations

It's actually not. It's a special case only for the use-regex and rewrite-target annotations which can be seen in the enforceRegexModifier function.

@arkberezkin thank you for that info. It helps a lot I think. We have to wiat for comments from others.

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.

This appears to still be a problem, and I'm happy to submit an PR to address it.

The buildLocation function in controller/template/template.go currently looks like this:

func buildLocation(input interface{}, enforceRegex bool) string {
	location, ok := input.(*ingress.Location)
	if !ok {
		klog.Errorf("expected an '*ingress.Location' type but %T was returned", input)
		return slash
	}

	path := location.Path
	if enforceRegex {
		return fmt.Sprintf(`~* "^%s"`, path)
	}

	if location.PathType != nil && *location.PathType == networkingv1.PathTypeExact {
		return fmt.Sprintf(`= %s`, path)
	}

	return path
}

Notice that the enforceRegex condition is checked first, before checking if the location is exact.

It seems like switching the order of these two checks, so the check for Exact happens first, would address this problem, without changing any of the regex logic.

Is there some reason why these checks happen in the order they do today?

@dacohen We’ve also encountered a similar issue. Although we can work around it using regex, we still hope that this behavior can be fixed directly. I think your method is feasible.