projectcontour / contour

Contour is a Kubernetes ingress controller using Envoy proxy.
https://projectcontour.io
Apache License 2.0
3.68k stars 669 forks source link

Allow ingress routes for hostnames without a TLD defined #755

Closed josebiro closed 5 years ago

josebiro commented 5 years ago

Describe the solution you'd like I'd like to be able to create an ingress route with a simple hostname, (instead of|as well as) a fully qualified domain name.

For example, assigning an ingress route on any of the following names:

g
grafana
grafana.example.com

This is useful for services on a local LAN with a search domain configured because Envoy won't respond for services if the fully qualified name doesn't match exactly, and in a search path lookup, the TLD is not appended to the request.

Anything else you would like to add: The following CRD validator seems to work as I'd like and preserves the current validation logic:

fqdn:
  type: string
  pattern: ^([a-zA-Z0-9]{1,})$|^([a-zA-Z0-9]+(-[a-zA-Z0-9]+)*\.)+[a-z]{2,}$

It also manages to not crash the API server, which is ... um... better than previous experiments.

Environment:

davecheney commented 5 years ago

Thank you for your request. I believe I understand what you are asking for.

The concern I have with this approach is not the complexity of regex, that's the callers decision, but the differences between regex implementations.

For example, few know that the ingress.http.path spec is actually a regex; and its scoped to be a javascript style regex. Envoy also supports a pattern matcher but it is a different regex implementation for which 90% of the time the differences are not important, until they are because it does something strange.

I don't want to expose regexes where I don't have to because that will also mean specifying a regex dialect, and that ties us to Envoy's pattern matcher very closely and really paints us into a corner if Envoy ever decide to change their regex engine -- heck, maybe they fix a bug and we have users who using the previous behaviour.

Can we come up with some other simple pattern language that will suffice? Would *.yourcomain.com work for you?

josebiro commented 5 years ago

Understood and acknowledged. I had to brush up on the various implementation differences to come up with the suggestion, so I immediately started to share the concern.

I'm not sure I totally understand your suggestion, but I think you're recommending something like {myhost}.mydomain.int as the value of fqdn, and use {myhost} as the hostname for an additional envoy 'cluster'+ config?

If so, the config would be simpler. And even if it's opt in behavior, it would be easier to maintain. So SGTM.

+ ??? I forget envoy's terms

davecheney commented 5 years ago

What it sounds like you're asking for is the alias feature we removed from the ingressroute document just before 0.6.

In a sentence, aliases let you add extra host names for a single ingress route root. This used to be an extra field on the vhost spec, but was removed in favor of using delegation, https://github.com/heptio/contour/blob/master/docs/ingressroute.md#virtualhost-aliases

On 14 October 2018 at 12:55, Joseph Bironas notifications@github.com wrote:

Understood and acknowledged. I had to brush up on the various implementation differences to come up with the suggestion, so I immediately started to share the concern.

I'm not sure I totally understand your suggestion, but I think you're recommending something like {myhost}.mydomain.int as the value of fqdn, and use {myhost} as the hostname for an additional envoy 'cluster'* config?

If so, the config would be simpler. And even if it's opt in behavior, it would be easier to maintain. So SGTM.

  • ??? I forget envoy's terms

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/heptio/contour/issues/755#issuecomment-429589739, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAcA-HvvfVRxH-GORQtPiIaHhDhtBE4ks5ukpmJgaJpZM4Xa1bm .

josebiro commented 5 years ago

Taking a brief look through the PRs that removed aliases -- on the surface it looks like they were using a pattern similar to the fqdn validator. That would limit their usefulness for the "hostname without a TLD" use case.

davecheney commented 5 years ago

I'm sorry, I think I misunderstood your original request. My apologies.

What I believe you are asking for is a modification to the crd validation rules. This seems reasonable.

On 14 October 2018 at 15:31, Joseph Bironas notifications@github.com wrote:

Taking a brief look through the PRs that removed aliases -- on the surface it looks like they were using a pattern similar to the fqdn validator. That would limit their usefulness for the "hostname without a TLD" use case.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/heptio/contour/issues/755#issuecomment-429595768, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAcAypEsqfBxiAUmfQOg-_GuTCt4zsjks5ukr4PgaJpZM4Xa1bm .

davecheney commented 5 years ago

/cc @youngnick I think this is a similar issue to #1117. If you can look at adjusting the validation to allow a single [a-z]+ without a trailing period that should be fine.

@josebiro as a workaround you can edit the CRD validation to remove the the regex, we’ll take whatever you publish to the FQDN field and pass to Envoy, it’s your responsibility that that hostname makes sense in your environment.