gateway-api icon indicating copy to clipboard operation
gateway-api copied to clipboard

tests: Add conformace tests for listenersets

Open davidjumani opened this issue 5 months ago • 26 comments

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

davidjumani avatar Jun 30 '25 15:06 davidjumani

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

k8s-ci-robot avatar Jun 30 '25 15:06 k8s-ci-robot

[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.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot avatar Jun 30 '25 15:06 k8s-ci-robot

/test all

davidjumani avatar Jul 01 '25 01:07 davidjumani

/retest

davidjumani avatar Aug 16 '25 09:08 davidjumani

/cc @rikatz @dprotaso

robscott avatar Aug 18 '25 22:08 robscott

/hold

youngnick avatar Aug 20 '25 06:08 youngnick

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?

rikatz avatar Aug 20 '25 15:08 rikatz

@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!

rikatz avatar Aug 20 '25 21:08 rikatz

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.

youngnick avatar Aug 21 '25 00:08 youngnick

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

davidjumani avatar Aug 21 '25 10:08 davidjumani

/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

dprotaso avatar Aug 21 '25 15:08 dprotaso

/assign

rikatz avatar Aug 21 '25 19:08 rikatz

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

davidjumani avatar Aug 27 '25 18:08 davidjumani

@youngnick @dprotaso @rikatz could you please have another look at this ? Thanks

davidjumani avatar Sep 11 '25 15:09 davidjumani

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

davidjumani avatar Sep 29 '25 15:09 davidjumani

@youngnick @dprotaso @rikatz Could you please have a look at this? Thanks

davidjumani avatar Oct 15 '25 14:10 davidjumani

This is next on my queue, will be my first thing tomorrow (thursday)

rikatz avatar Oct 22 '25 20:10 rikatz

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

rikatz avatar Oct 23 '25 19:10 rikatz

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

davidjumani avatar Oct 27 '25 14:10 davidjumani

/retest

davidjumani avatar Oct 27 '25 17:10 davidjumani

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)

davidjumani avatar Oct 28 '25 15:10 davidjumani

/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

rikatz avatar Dec 02 '25 19:12 rikatz

/remove-label release-blocker

rikatz avatar Dec 02 '25 19:12 rikatz

@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.

k8s-ci-robot avatar Dec 02 '25 19:12 k8s-ci-robot

New changes are detected. LGTM label has been removed.

k8s-ci-robot avatar Dec 04 '25 22:12 k8s-ci-robot

I'll update these tests and the API to use AttachedListenerSets as a count once merged

davidjumani avatar Dec 15 '25 19:12 davidjumani