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

conformance: Revisit HTTPRouteHTTPSListener test 404 case #3044

Closed
sunjayBhatia opened this issue May 1, 2024 · 4 comments
Closed

conformance: Revisit HTTPRouteHTTPSListener test 404 case #3044

sunjayBhatia opened this issue May 1, 2024 · 4 comments
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@sunjayBhatia
Copy link
Member

sunjayBhatia commented May 1, 2024

What happened:

While validating v1.1.0-rc1 with Contour, running into this test failing: #2824

The request to unknown-example.com never succeeds to get an HTTP response (the test expects a 404), since there is no Listener/certificate configured that will allow the gateway to terminate the TLS connection

What you expected to happen:

Should we expect this test request to return a 404 or should we instead expect a TLS connection error since there is no explicit configuration for a Listener TLS certificate that matches the request?

How to reproduce it (as minimally and precisely as possible):

Run the conformance tests for v1.1.0-rc1 against Contour, using the branch in this PR: projectcontour/contour#6398

Anything else we need to know?:

We can likely work around this with fallback certificate configured in contour

I did approve the original PR that added that test but overlooked the 404 case for the request with the unknown hostname since it was added later in the PR's lifecycle, I think we might want to revisit this before releasing v1.1.0

@sunjayBhatia sunjayBhatia added the kind/bug Categorizes issue or PR as related to a bug. label May 1, 2024
@robscott
Copy link
Member

robscott commented May 2, 2024

Thanks for catching this @sunjayBhatia! @dprotaso is out, but if you or anyone has time to remove this specific test case before RC2 tomorrow, I think it would be good to pull it out for the time being and revisit after v1.1.

sunjayBhatia added a commit to sunjayBhatia/gateway-api that referenced this issue May 2, 2024
@sunjayBhatia
Copy link
Member Author

#3045 PR to remove the questionable case for now

@sunjayBhatia
Copy link
Member Author

rethinking my own thinking here: #3045 (comment)

this might just be fine and something we need to fix in contour

@sunjayBhatia
Copy link
Member Author

I'm going to close this for now since I think this is probably correct actually, sorry for the confusion

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

No branches or pull requests

2 participants