conformance: test cases for HTTPS connection coalescing #4364
Conversation
Signed-off-by: Norwin Schnyder <norwin.schnyder+github@gmail.com>
k8s-ci-robot
added
release-note
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: snorwin The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
k8s-ci-robot
added
size/S
|
/cc @robscott |
|
/retest |
| {host: "example.org", serverName: "second-example.org", statusCode: 421}, | ||
| {host: "second-example.org", serverName: "example.org", statusCode: 421}, |
There was a problem hiding this comment.
I think the relevant part of the spec is here:
gateway-api/apis/v1/gateway_types.go
Line 394 in f95fe2b
We're actually using RFC 2119 interpretation of keywords here, so SHOULD is a recommendation, not a requirement. With that said, I think we could justify an extended test (separate feature name) to cover this.
There was a problem hiding this comment.
Shouldn't we enforce this behaviour only for reused connections? If Cx is estabilishing a new connection per request then misdirected does not make sense.
@snorwin can you verify that connection is being reused in this test? I was looking into RoundTripper and I can see that DisableKeepAlives is set.
| {host: "unknown-example.org", serverName: "unknown-example.org", statusCode: 404}, | ||
| {host: "unknown-example.org", serverName: "second-example.org", statusCode: 404}, |
There was a problem hiding this comment.
Since these are both a MUST, I don't think they need any qualifiers and can be a part of a core conformance test. Hopefully these don't trip up any existing implementations.
4 participants
What type of PR is this?
/area conformance-test
What this PR does / why we need it:
In #3630, the Gateway API’s interaction with connection coalescing was improved as part of GEP-3567. However, these changes are not yet reflected in the conformance tests.
Which issue(s) this PR fixes:
N/A
Does this PR introduce a user-facing change?: