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

Enable GatewayHTTPListenerIsolation conformance test #3352

Closed
arkodg opened this issue May 8, 2024 · 9 comments · Fixed by #4000
Closed

Enable GatewayHTTPListenerIsolation conformance test #3352

arkodg opened this issue May 8, 2024 · 9 comments · Fixed by #4000
Assignees
Labels
area/conformance Gateway API Conformance Related Issues area/testing
Milestone

Comments

@arkodg
Copy link
Contributor

arkodg commented May 8, 2024

Description:

Describe the issue.

Details of the test are in kubernetes-sigs/gateway-api#3047

Remove from https://github.com/envoyproxy/gateway/blob/0457ee7417288159eaa4422ee243b808a1696604/test/conformance/experimental_conformance_test.go#L33C3-L33C47 once fixed

[optional Relevant Links:]

Any extra documentation required to understand the issue.

@arkodg arkodg added triage help wanted Extra attention is needed area/conformance Gateway API Conformance Related Issues and removed triage labels May 8, 2024
@arkodg arkodg added this to the v1.1.0-rc1 milestone May 8, 2024
@arkodg arkodg modified the milestones: v1.1.0-rc1, v1.1.0 May 31, 2024
@levikobi
Copy link
Contributor

levikobi commented Jun 5, 2024

Hi @arkodg, I would like to try taking on this one if that's ok.
If there's anything important/helpful I should know about before diving in, please let me know

@shawnh2
Copy link
Contributor

shawnh2 commented Jun 5, 2024

Thanks @levikobi for picking this one up!

@shawnh2 shawnh2 added area/testing and removed help wanted Extra attention is needed labels Jun 5, 2024
@arkodg
Copy link
Contributor Author

arkodg commented Jun 5, 2024

thanks for picking this one up @levikobi ! feel free to comment on the issue here with what you are seeing fail in the test, and can link you to the relevant code that may need to be revisited

Copy link

github-actions bot commented Jul 6, 2024

This issue has been automatically marked as stale because it has not had activity in the last 30 days.

@github-actions github-actions bot added the stale label Jul 6, 2024
@arkodg arkodg removed the stale label Jul 18, 2024
@arkodg
Copy link
Contributor Author

arkodg commented Jul 18, 2024

hey @levikobi stil working on this one ? its would be great if we could get this into v1.1

@levikobi
Copy link
Contributor

Hi @arkodg, sorry for the delay. I have been pretty busy lately.
Yes, I would like to work on this one. I'll keep you updated throughout this week.
When is v1.1 supposed to come out?

@levikobi
Copy link
Contributor

levikobi commented Jul 29, 2024

Hi @arkodg
I run GatewayHTTPListenerIsolation conformance test, and the following cases fail, as their requests receive a 404 response:

  • 0_request_to_'bar.com/empty-hostname'_should_go_to_infra-backend-v1
  • 5_request_to_'bar.example.com/wildcard-example-com'_should_go_to_infra-backend-v1
  • 10_request_to_'bar.foo.example.com/wildcard-foo-example-com'_should_go_to_infra-backend-v1
  • 15_request_to_'abc.foo.example.com/abc-foo-example-com'_should_go_to_infra-backend-v1

The other cases are passing since they expect a 404 response.

I looked at the proxy instance and saw this log:

[2024-07-28 05:01:00.536][1][warning][config] [source/extensions/config_subscription/grpc/delta_subscription_state.cc:269] delta config for type.googleapis.com/envoy.config.route.v3.RouteConfiguration rejected: Only unique values for domains are permitted. Duplicate entry of domain *.example.com in route gateway-conformance-infra/http-listener-isolation-with-hostname-intersection/empty-hostname
[2024-07-28 05:01:00.536][1][warning][config] [source/extensions/config_subscription/grpc/grpc_subscription_impl.cc:138] gRPC config for type.googleapis.com/envoy.config.route.v3.RouteConfiguration rejected: Only unique values for domains are permitted. Duplicate entry of domain *.example.com in route gateway-conformance-infra/http-listener-isolation-with-hostname-intersection/empty-hostname

To my understanding, and please correct me if I'm wrong - the fix should be at the translation logic at internal/gatewayapi/translator.go?

@arkodg
Copy link
Contributor Author

arkodg commented Jul 29, 2024

thanks for digging into this @levikobi
relevant logic lives in

hosts := computeHosts(GetHostnames(route), listener.Hostname)
and
// 1:1 between IR HTTPRoute Hostname and xDS VirtualHost.

you can even convert the filing sub tests into unit tests by adding input test files in https://github.com/envoyproxy/gateway/tree/main/internal/gatewayapi/testdata and running make testdata and looking at the o/p file to decipher if its what we expect or not

@levikobi
Copy link
Contributor

levikobi commented Aug 5, 2024

Thanks for the help @arkodg!
I managed to make the conformance tests pass and created a PR. However, It's still a draft since I want to add some more tests around the new code.
Please feel free to take a look though

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/conformance Gateway API Conformance Related Issues area/testing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants