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

Standardise or document Ingress path expressions across implementations #555

Closed
thockin opened this issue Apr 4, 2017 · 34 comments
Closed
Labels
area/ingress lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.

Comments

@thockin
Copy link
Member

thockin commented Apr 4, 2017

From @wstrange on May 2, 2016 21:12

Path expressions in the Ingress are provider specific - so a path expression (say /foo/) that works in one provider does not work the same in another.

For example:
path: /foo/

Seems to result in different behaviour on GKE vs. the nginx controller.

If it is not realistic to standardise paths, the providers should document the interpretation of the path expression ( example: does /foo/ match /foo/bar?)

Copied from original issue: kubernetes-retired/contrib#885

@thockin
Copy link
Member Author

thockin commented Apr 4, 2017

From @bprashanth on May 2, 2016 21:48

If you can point out the exact difference and expected behavior, we might be able to get the nginx controller to do what we want. It's obviously a lot harder to change the cloud lb formatting rules.

@thockin
Copy link
Member Author

thockin commented Apr 4, 2017

From @wstrange on May 2, 2016 22:31

Are AWS and GCE the same?

If so - I'd suggest that is the behaviour we want.

@thockin
Copy link
Member Author

thockin commented Apr 4, 2017

From @bprashanth on May 2, 2016 22:46

What is "this"? we need to handle this on a case by case basis, we don't have bw to figure out every part of the spec each of these implement differently :)

@thockin
Copy link
Member Author

thockin commented Apr 4, 2017

From @wstrange on May 2, 2016 22:56

I have no experience with AWS - so I can not help here

I appreciate the bandwidth issue, and don't have any expectations of a quick fix :-)

For now, I would suggest aligning nginx with "whatever GKE does"

If I get more time I will look at the differences between the two. For now, I just added foo/ and foo/*, and that seems to work on both

@thockin
Copy link
Member Author

thockin commented Apr 4, 2017

From @bprashanth on May 2, 2016 23:4

Hmm, IIUC in this case nginx is actually more lenient and accepts both foo/ and foo/* to do the same thing (i.e proxy to /foo/bar or whatever, haven't tried it yet)? If so, we should try to get "foo/" to match exactly "foo/"
@aledbf

@thockin
Copy link
Member Author

thockin commented Apr 4, 2017

From @aledbf on May 3, 2016 0:13

@bprashanth the current behavior in the nginx controller is to not use exact match. Please check this example from the nginx docs (configuration B is how it works now).
If we can build a table with multiple examples and the expected outcome I think is possible to modify the configuration in nginx without much effort

@thockin
Copy link
Member Author

thockin commented Apr 4, 2017

From @wstrange on May 3, 2016 19:16

To further clarify:
For the nginx controller, an ingress path: /foo gets translated to nginx.conf location /foo { }

This is a "prefix" match. That is, it will match /foo, /foo/bar, /foo/bar/xx, ..

On GCE, I believe two path rules need to be setup to handle this case:
/foo
/foo/*

I can't recollect if the GCE ingress controller does this or not (may get time to test later this week). If not- setting up those two rules would seem to make the implementations align.

I believe I previously had a trailing / on my path:
path: /foo/

Which caused some of my prior confusion.

@thockin
Copy link
Member Author

thockin commented Apr 4, 2017

From @wstrange on May 11, 2016 17:48

Update: GCE ingress controller does not add a wildcard to a path expression.

To get the same behaviour as Nginx, you need two path expressions in the ingress definition:
path: /foo
path: /foo/*

Which is equivalent to:
path: /foo

on Nginx.

This is unfortunate that the same ingress definition can not be used on both controllers.

@thockin
Copy link
Member Author

thockin commented Apr 4, 2017

From @bprashanth on May 11, 2016 18:8

yeah so /foo is exactly foo and if you want anything more, you need foo/*. I think this conflicts with the nginx defaults as pointed out in kubernetes-retired/contrib#885 (comment)

@thockin
Copy link
Member Author

thockin commented Apr 4, 2017

From @gjcarneiro on February 13, 2017 15:26

I've been wondering the opposite: how to have an exact match for a path with the nginx controller. Any idea?

@thockin
Copy link
Member Author

thockin commented Apr 4, 2017

From @gjcarneiro on February 14, 2017 17:53

My use case is that I have a web application pod with 3 containers:

  1. /static should go to the frontend (nginx static files) container
  2. / should go to the landing page container
  3. everything else should go to the backend container

The problem is that for 2 I need to match exactly the path /, but not backend-y things like /login, while for 3 I need to match "every thing else", which has been / up until now.

I would love to have GCE-style ingress path rules in the contrib nginx controller.

@munnerz
Copy link
Member

munnerz commented Jul 5, 2017

This is currently a blocker for kube-lego standardising it's implementation of ingress controllers, meaning we have to maintain dedicated support for ingress controllers in kube-lego itself.

It'd be ideal if this could be standardised rather than documented - this could be something that the ingress controller itself can translate if required. That way, applications can build against a well defined set of APIs for Ingress, just like with other Kubernetes resources.

Out-of-tree implementations of ingress make perfect sense, but sacrificing portability makes it an awkward resource type to work with.

@thockin
Copy link
Member Author

thockin commented Jul 6, 2017

The problem with standardizing is that it's just a string, and it's non-standard. I'm far from an expert in all the implementations, so I am VERY open to ideas on how to make this more useful, especially while it is still pre-GA.

@csbell

@wstrange
Copy link

wstrange commented Jul 6, 2017

Having path consistently between gke, nginx and traefik would go a long way to making this easier.
Based on https://docs.traefik.io/user-guide/kubernetes/ it looks like Traefik aligns with nginx

/foo means "any path that starts with /foo"

@gjcarneiro
Copy link

But the GCE version is more precise and more useful. What if I want to match just the path /foo, while directing requests for subpaths inside /foo to another backend?

As I stated before, I have such a use case. In our scenario, the exact path / means the web site's landing page, which is handled by pod different from the general web app backend.

@thockin
Copy link
Member Author

thockin commented Jul 7, 2017 via email

@aledbf
Copy link
Member

aledbf commented Jul 7, 2017

The problem with standardizing is that it's just a string, and it's non-standard

Why not use the gclb url-map as the standard and delegate the transformation to the ingress implementations?

@aledbf
Copy link
Member

aledbf commented Jul 7, 2017

That leaves us with a truly lowest common denominator, and ... is that useful to anyone?

No, but for me (as a user) it's really annoying to have to change the path when a change in the ingress controller implementation is required (like minikube to gce)

@thockin
Copy link
Member Author

thockin commented Jul 7, 2017 via email

@aledbf
Copy link
Member

aledbf commented Jul 7, 2017

Would it be better or worse to use a different resource kind?

Better from a developer perspective because we can get rid of the annotations and use real types.
I'm not sure from the user experience because we still need to create/change the ingress rules.

@thockin
Copy link
Member Author

thockin commented Jul 7, 2017 via email

@esbie
Copy link

esbie commented Jul 7, 2017

Even if not all ingress controllers conform to the standard, at least newly made controllers could be implemented to that common spec. And I'm happy to move at least the linkerd ingress controller to whatever the standard will be.

@thockin
Copy link
Member Author

thockin commented Jul 8, 2017 via email

@unixwitch
Copy link
Contributor

an annotation that says "interpret paths strings as nginx"

Might I suggest specifying that path is a simple string prefix match (either Traefik or GCE/GKE style - I think GCE/GKE is more useful personally), and adding a pathRegex field for regular expression matching, which controllers could optionally implement (and document whether they do or not)?

This seems to support all reasonable use-cases: most applications can simply use the lowest common denominator, which is path prefix matching, and work on all ingress controllers. Applications that require complex path matching can use pathRegex and document that they require an ingress controller that supports it.

As an ingress controller implementer, my concern is that users should be able to switch from other controllers to mine, and vice versa, without having to rewrite all their ingress rules. (This is especially important for applications like kube-lego that create their own ingress rules.) Adding more things to non-standard annotations works against that.

@thockin
Copy link
Member Author

thockin commented Jul 9, 2017 via email

@unixwitch
Copy link
Contributor

We can't have a second field that only some controllers implement - that forces users to know which controller is in play

But it doesn't, it only requires them to know whether the controller supports pathRegex or not.

Compare this with PVCs: a user can request a ReadWriteMany PV on a cluster that doesn't support it, and they won't get any storage. This is surely confusing to some users. Yet we don't have numerous implementation-specific resource types like GCEPersistentVolumeClaim, CephFSPersistentVolumeClaim, etc., and require the user to know that CephFS supports readwritemany while GCE doesn't. Instead, the cluster administrator can use any PV provisioner that supports the ReadWriteMany access mode and it will work with any PVC that requests that access mode.

Likewise with Ingress, the cluster administrator could provision any Ingress controller they like, as long as it supports pathRegex, and it will work with any Ingress that requires regex path matching without needing to change any Ingress rules.

users will use that field on platforms that do not support it, and it will fail with no indication of why

I would actually like to see a way for the controller to indicate error status to the user, which could be displayed in kubectl get ing or kubectl describe ing. For example, my Ingress controller supports access control to restrict what domain(s) each namespace can use, but I have no way to indicate to the user that they aren't allowed to use a particular domain and that's why their Ingress isn't working.

@thockin
Copy link
Member Author

thockin commented Jul 9, 2017 via email

@zimbatm
Copy link

zimbatm commented Sep 18, 2017

My understanding of the Ingress resource role was to find a common denominator between controllers. Otherwise how can a helm chart be developed that works across environments?

In that case it's easier to ignore Ingress altogether and use a Service and do routing and load-balancing internally.

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

Prevent issues from auto-closing with an /lifecycle frozen comment.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or @fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 6, 2018
@thockin
Copy link
Member Author

thockin commented Jan 6, 2018

/lifecycle frozen
/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jan 6, 2018
@bryanlarsen
Copy link

Has this been documented anywhere yet? Another piece of inconsistency is match order: I believe that nginx is longest match while others are first match.

@andrewwippler
Copy link

I have stumbled across this issue in my search for how the path is translated by the ingress controller - if any regex was added to the path string before it was consumed by the ingress. After reading this thread, I understand it is not manipulated in any way.

What got me confused about this path were how the docs described it:

Path is an extended POSIX regex as defined by IEEE Std 1003.1, (i.e this
follows the egrep/unix syntax, not the perl syntax) matched against the
path of an incoming request. Currently it can contain characters disallowed
from the conventional "path" part of a URL as defined by RFC 3986. Paths
must begin with a '/'. If unspecified, the path defaults to a catch all
sending traffic to the backend.

To me, this sounded like the string was being manipulated by the ingress controller before being passed into the ingress.

While there are inconsistencies between ingress controllers, we as administrators define the ingress class we want to use in the kubernetes.io/ingress.class annotation. If one should change the ingress class, one should also assume the paths might not work exactly the same and may have to be refactored; however, if using the same ingress class, the same path definition should behave in the same manner (when the file is applied to a different cluster).

To me, this issue sounds like a definition issue rather than needing to standardize all ingress controllers. Would we have this discussion if the path were defined as: the string passed into the corresponding ingress expression to determine where to route the request? That definition tells me with a GCE ingress I might have to specify /foo/* to match sub paths. Of course with a definition like that, I would also want to pass in a path with one of nginx's location regex modifiers: path: "= /exactmatch" or path: "^~ /path" (see: http://nginx.org/en/docs/http/ngx_http_core_module.html#location)

@thockin
Copy link
Member Author

thockin commented May 16, 2018 via email

@aledbf
Copy link
Member

aledbf commented May 25, 2019

Closing. This is out of the scope of Ingress V1 kubernetes/enhancements#1025

@aledbf aledbf closed this as completed May 25, 2019
denniseffing added a commit to codecentric/habitcentric that referenced this issue Jan 18, 2022
The Nginx Ingress load balancer of Minikube uses "prefix match" to match
path expressions.
The GKE Ingress load balancer uses "exact match".
This configuration works on GKE and Minikube.

See kubernetes/ingress-nginx#555 for more
information.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ingress lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.
Projects
None yet
Development

No branches or pull requests