kubernetes-sigs / gateway-api

Repository for the next iteration of composite service (e.g. Ingress) and load balancing APIs.
https://gateway-api.sigs.k8s.io
Apache License 2.0
1.76k stars 456 forks source link

conformance: Add test for HTTPS Listener/Route with empty hostnames #2040

Open sunjayBhatia opened 1 year ago

sunjayBhatia commented 1 year ago

What would you like to be added: An explicit test for TLS/HTTPS requests to a Gateway/Route combination that does not specify any hostnames

Why this is needed: Inadvertently while adding this test we have added this test, but we should have an explicit one for documentation's sake and so it is not conflated with other features.

We could maybe modify the Routes used in this test to specify a hostname as well so we don't conflate the functionality

Seems like same-namespace-with-https-listener is one of the few Gateways in the conformance suite that does not specify any hostnames

JSee98 commented 1 year ago

Hi, I wanted to pick this issue, seems a good first issue. I had a question (excuse me if it is too dumb of a question). When we say that we want test that does not specify any hostnames, where exactly are we expecting hostname to be undefined, as I checked it is defined for redirect requests.

JSee98 commented 1 year ago

/assign

JSee98 commented 1 year ago

So, after digging around a bit I understand that we want a gateway where a hostname isn't defined for the listeners? Then we want to add tests where we specifically add hostname within the routes. Is my understanding of the issue correct?

JSee98 commented 1 year ago

/help

JSee98 commented 1 year ago

/remove-help

sunjayBhatia commented 1 year ago

So, after digging around a bit I understand that we want a gateway where a hostname isn't defined for the listeners? Then we want to add tests where we specifically add hostname within the routes. Is my understanding of the issue correct?

We want to define a test that has no hostname matches on Gateway Listeners or Routes to make sure that implementations can successfully route requests to arbitrary hosts (HTTP and HTTPS request should work)

JSee98 commented 1 year ago

So, after digging around a bit I understand that we want a gateway where a hostname isn't defined for the listeners? Then we want to add tests where we specifically add hostname within the routes. Is my understanding of the issue correct?

We want to define a test that has no hostname matches on Gateway Listeners or Routes to make sure that implementations can successfully route requests to arbitrary hosts (HTTP and HTTPS request should work)

Sure, will get a test out for this. Thanks for clarifying.

youngnick commented 1 year ago

@sunjayBhatia, I can't remember, do we specify that this should act as a fallback, that is, that Listeners that do match a hostname should take precedence over ones that don't? (I hope we do!)

sunjayBhatia commented 1 year ago

Looks like we have this language https://github.com/kubernetes-sigs/gateway-api/blob/4738c6c6e981d4a0e38c8c6120dc0198e4128351/apis/v1beta1/gateway_types.go#L96-L102

But would be great to codify that in a conformance test if we don't have it already

that is sort of covered here but not quite explicit: https://github.com/kubernetes-sigs/gateway-api/blob/4738c6c6e981d4a0e38c8c6120dc0198e4128351/conformance/tests/httproute-hostname-intersection.go

cc @JSee98 it would be great if the test you add is able to fulfill the linked spec description above and show the route rule matching precedence for more specific hostnames over wildcards

JSee98 commented 1 year ago

@sunjayBhatia Sure, I'll create two tests then. One for the original requirement and the other for the wildcard vs perfect match of hostname.

shaneutt commented 1 year ago

Just checking in: how are we doing on this one @JSee98, @sunjayBhatia?

sunjayBhatia commented 1 year ago

@JSee98 are you still working on this? If not we can find someone else who is interested or I can take it

JSee98 commented 1 year ago

@shaneutt @sunjayBhatia Hi guys, really sorry for the delayed response. I have been extremely occupied with work lately. I can raise a PR for my changes by the end of the week if that works?? I had created the tests initially, however, had some issues setting up the environment locally. So not a lot left I guess.

sunjayBhatia commented 1 year ago

@JSee98 that would be great 👍🏽 Let us know if you need any other support, and thank you for contributing!

JSee98 commented 1 year ago

Hi @sunjayBhatia, I'm having a little issue to setup the environment to run my tests. I followed the developer setup mentioned here https://github.com/kubernetes/community/blob/master/contributors/devel/README.md#setting-up-your-dev-environment-coding-and-debugging Using this doc I tried to get a docker container running with the necessary services to run the conformance test, however, wasn't able to. This is what I've tried:

I'd really appreciate if you can help me out here

shaneutt commented 1 year ago

Well overall it seems reasonable to do this:

/triage accepted

However it doesn't seem obvious that this needs to be done in a hurry, if someone has a different perspective please share it.

/priority backlog

k8s-triage-robot commented 1 month ago

This issue has not been updated in over 1 year, and should be re-triaged.

You can:

For more details on the triage process, see https://www.kubernetes.dev/docs/guide/issue-triage/

/remove-triage accepted