tests: Add conformace tests for listenersets
What type of PR is this?
/kind test /area conformance-test
What this PR does / why we need it:
This PR contains an initial conformance test for listenersets. This aims to verify the following :
- Translation of a listenerset by an implementer
- Route inheritance of gateway-level routes by the listenerset
- Route isolation between gateways and listenersets based on parentRef and sectionName
- Gateway, ListenerSet, Route and Service in different namespaces
- ListenerSets not allowed on the gateway
- ListenerSets with protocol and hostname conflict
Run against kgateway
--- PASS: TestConformance/ListenerSetHostnameConflict (4.21s)
--- PASS: TestConformance/ListenerSetHostnameConflict/5_request_to_'hostname-conflict-listener-1.com/listenerset-2-route'_should_receive_one_of_[] (0.03s)
--- PASS: TestConformance/ListenerSetHostnameConflict/7_request_to_'hostname-conflict-listener-2.com/listenerset-2-route'_should_receive_one_of_[] (0.03s)
--- PASS: TestConformance/ListenerSetHostnameConflict/4_request_to_'hostname-conflict-listener-1.com/listenerset-1-route'_should_receive_one_of_[] (0.03s)
--- PASS: TestConformance/ListenerSetHostnameConflict/6_request_to_'hostname-conflict-listener-2.com/listenerset-1-route'_should_go_to_infra-backend-v2 (0.03s)
--- PASS: TestConformance/ListenerSetHostnameConflict/1_request_to_'listenerset-1-listener.com/listenerset-1-route'_should_go_to_infra-backend-v2 (0.03s)
--- PASS: TestConformance/ListenerSetHostnameConflict/3_request_to_'hostname-conflict-listener-1.com/gateway-route'_should_go_to_infra-backend-v1 (0.04s)
--- PASS: TestConformance/ListenerSetHostnameConflict/2_request_to_'listenerset-2-listener.com/listenerset-2-route'_should_go_to_infra-backend-v3 (0.04s)
--- PASS: TestConformance/ListenerSetHostnameConflict/0_request_to_'gateway-listener.com/gateway-route'_should_go_to_infra-backend-v1 (0.04s)
--- PASS: TestConformance/ListenerSetCrossNamespace (3.33s)
--- PASS: TestConformance/ListenerSetCrossNamespace/6_request_to_'listenerset-1-listener-1.com/gateway-listener-2-listener-route'_should_receive_one_of_[] (0.03s)
--- PASS: TestConformance/ListenerSetCrossNamespace/8_request_to_'gateway-listener-1.com/listenerset-1-route'_should_receive_one_of_[] (0.03s)
--- PASS: TestConformance/ListenerSetCrossNamespace/16_request_to_'listenerset-2-listener-1.com/listenerset-2-route'_should_receive_one_of_[] (0.04s)
--- PASS: TestConformance/ListenerSetCrossNamespace/9_request_to_'gateway-listener-2.com/listenerset-1-route'_should_receive_one_of_[] (0.04s)
--- PASS: TestConformance/ListenerSetCrossNamespace/4_request_to_'gateway-listener-1.com/gateway-listener-2-listener-route'_should_receive_one_of_[] (0.03s)
--- PASS: TestConformance/ListenerSetCrossNamespace/2_request_to_'listenerset-1-listener-1.com/gateway-route'_should_receive_one_of_[] (0.04s)
--- PASS: TestConformance/ListenerSetCrossNamespace/7_request_to_'listenerset-1-listener-2.com/gateway-listener-2-listener-route'_should_receive_one_of_[] (0.04s)
--- PASS: TestConformance/ListenerSetCrossNamespace/1_request_to_'gateway-listener-2.com/gateway-route'_should_go_to_infra-backend-v1 (0.04s)
--- PASS: TestConformance/ListenerSetCrossNamespace/5_request_to_'gateway-listener-2.com/gateway-listener-2-listener-route'_should_go_to_infra-backend-v1 (0.04s)
--- PASS: TestConformance/ListenerSetCrossNamespace/0_request_to_'gateway-listener-1.com/gateway-route'_should_go_to_infra-backend-v1 (0.05s)
--- PASS: TestConformance/ListenerSetCrossNamespace/12_request_to_'gateway-listener-1.com/listenerset-1-listener-2-listener-route'_should_receive_one_of_[] (0.01s)
--- PASS: TestConformance/ListenerSetCrossNamespace/14_request_to_'listenerset-1-listener-1.com/listenerset-1-listener-2-listener-route'_should_receive_one_of_[] (0.01s)
--- PASS: TestConformance/ListenerSetCrossNamespace/3_request_to_'listenerset-1-listener-2.com/gateway-route'_should_receive_one_of_[] (0.02s)
--- PASS: TestConformance/ListenerSetCrossNamespace/13_request_to_'gateway-listener-2.com/listenerset-1-listener-2-listener-route'_should_receive_one_of_[] (0.02s)
--- PASS: TestConformance/ListenerSetCrossNamespace/15_request_to_'listenerset-1-listener-2.com/listenerset-1-listener-2-listener-route'_should_go_to_infra-backend-v1 (0.02s)
--- PASS: TestConformance/ListenerSetCrossNamespace/11_request_to_'listenerset-1-listener-2.com/listenerset-1-route'_should_go_to_infra-backend-v1 (0.02s)
--- PASS: TestConformance/ListenerSetCrossNamespace/10_request_to_'listenerset-1-listener-1.com/listenerset-1-route'_should_go_to_infra-backend-v1 (0.02s)
--- PASS: TestConformance/ListenerSetNotAllowed (3.11s)
--- PASS: TestConformance/ListenerSetNotAllowed/2_request_to_'bar.com/route'_should_receive_one_of_[] (0.04s)
--- PASS: TestConformance/ListenerSetNotAllowed/1_request_to_'foo.com/route'_should_receive_one_of_[] (0.04s)
--- PASS: TestConformance/ListenerSetNotAllowed/0_request_to_'example.com/route'_should_go_to_infra-backend-v1 (0.05s)
--- PASS: TestConformance/ListenerSetProtocolConflict (4.21s)
--- PASS: TestConformance/ListenerSetProtocolConflict/4_request_to_'protocol-conflict-listener-1.com/listenerset-1-route'_should_receive_one_of_[] (0.03s)
--- PASS: TestConformance/ListenerSetProtocolConflict/5_request_to_'protocol-conflict-listener-1.com/listenerset-2-route'_should_receive_one_of_[] (0.03s)
--- PASS: TestConformance/ListenerSetProtocolConflict/3_request_to_'protocol-conflict-listener-1.com/gateway-route'_should_go_to_infra-backend-v1 (0.04s)
--- PASS: TestConformance/ListenerSetProtocolConflict/0_request_to_'gateway-listener.com/gateway-route'_should_go_to_infra-backend-v1 (0.04s)
--- PASS: TestConformance/ListenerSetProtocolConflict/7_request_to_'protocol-conflict-listener-2.com/listenerset-2-route'_should_receive_one_of_[] (0.04s)
--- PASS: TestConformance/ListenerSetProtocolConflict/6_request_to_'protocol-conflict-listener-2.com/listenerset-1-route'_should_go_to_infra-backend-v2 (0.04s)
--- PASS: TestConformance/ListenerSetProtocolConflict/1_request_to_'listenerset-1-listener.com/listenerset-1-route'_should_go_to_infra-backend-v2 (0.04s)
--- PASS: TestConformance/ListenerSetProtocolConflict/2_request_to_'listenerset-2-listener.com/listenerset-2-route'_should_go_to_infra-backend-v3 (0.04s)
Which issue(s) this PR fixes:
Fixes https://github.com/kubernetes-sigs/gateway-api/issues/3785
Does this PR introduce a user-facing change?:
Adding initial conformance tests for XListenerSets
Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all
[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: davidjumani Once this PR has been reviewed and has the lgtm label, please assign youngnick for approval. For more information see the Code Review Process.
The full list of commands accepted by this bot can be found here.
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
/test all
/retest
/cc @rikatz @dprotaso
/hold
Thanks for this PR @davidjumani, and I'm sorry that it shows how much stuff we missed when writing the original spec!
Nick, did we updated the spec and GEP with the missing items? Should we wait for #3978 before merging this one?
@davidjumani I have started for the easier part for me, on helpers, and will come back to this PR tomorrow for the rest
I appreciate the effort here! Thank you!
Nick, did we updated the spec and GEP with the missing items? Should we wait for #3978 before merging this one?
Yes, we should add these things to #3978 before merging this one.
I'll put some comments on there with the things I think we need to add.
Thanks @youngnick @rikatz I've addressed most of the comments and am waiting for the resolution of "HTTPRoutes attached to Gateway attach to all listeners, no matter the source" behaviour to address the rest
/hold
based on the comments in https://github.com/kubernetes-sigs/gateway-api/pull/3978 - there seems to be a misunderstanding of Route attachment semantics in this PR
see: https://github.com/kubernetes-sigs/gateway-api/pull/3978#issuecomment-3211055462
/assign
Thanks @youngnick @dprotaso @rikatz for your reviews! I've updated this to reflect the discussion in https://github.com/kubernetes-sigs/gateway-api/pull/3978
@youngnick @dprotaso @rikatz could you please have another look at this ? Thanks
Thanks @rikatz for the review! Wrt the open questions, can we get the conformance tests in right now? When we amend the GEP, the conformance tests can also be updated
@youngnick @dprotaso @rikatz Could you please have a look at this? Thanks
This is next on my queue, will be my first thing tomorrow (thursday)
I still have some questions, but overall lgtm and I would like to see these tests running with other implementations. I will try to trigger it tomorrow against Istio and kgateway to see how they behave (failing is fine, as this may not be implemented yet)
I have mapped some missing tests, but not for this PR:
- ListenerSet entry name unique on same set
- count of attached routes (maybe it is ther,e but I didn't spotted it)
- Port is now optional to allow for dynamic port assignment. If the port is unspecified or set to zero, the implementation will assign a unique port.
- When a ReferenceGrant is applied to a Gateway it MUST NOT be inherited by child ListenerSets. Thus a ListenerSet listener MUST NOT access secrets granted to the Gateway listeners
Thanks @rikatz I've updated the PR and description with the passing run on kgateway I'll also add the tests that have been suggested in a follow up
/retest
I have mapped some missing tests, but not for this PR:
Thanks @rikatz I've raised a PR with the list of conformance tests I believe would be sufficient to validate this feature Add conformance details to ListenerSets (GEP-1713)
/lgtm /hold cancel
The tests looks fine and reflect what we've been discussing. Some tests will need to be updated after GEP updates are merged, but this is a great start for implementations
/cc @robscott @shaneutt
/remove-label release-blocker
@rikatz: The label(s) /remove-label release-blocker cannot be applied. These labels are supported: api-review, tide/merge-method-merge, tide/merge-method-rebase, tide/merge-method-squash, team/katacoda, refactor, ci-short, ci-extended, ci-full. Is this label configured under labels -> additional_labels or labels -> restricted_labels in plugin.yaml?
In response to this:
/remove-label release-blocker
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.
New changes are detected. LGTM label has been removed.
I'll update these tests and the API to use AttachedListenerSets as a count once merged