-
Notifications
You must be signed in to change notification settings - Fork 597
conformance: TLSRoute
with Terminate mode
#4137
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
base: main
Are you sure you want to change the base?
conformance: TLSRoute
with Terminate mode
#4137
Conversation
Welcome @phuhung273! |
Hi @phuhung273. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
/ok-to-test I'm not quite sure if this is specified explicitly (I don't see it mentioned in https://gateway-api.sigs.k8s.io/reference/spec/#listenertlsconfig or https://gateway-api.sigs.k8s.io/geps/gep-2907), but is |
Thanks for taking a look @mikemorris. I'm not sure about that, but can see we have a current
|
Yeah, this absolutely should have a new feature name, so that implementations can support as they are ready to. |
@phuhung273, thanks for getting us started! Also, while it's valid to use HTTP as the inner protocol, we should also end up testing bare TCP functions as well. |
Thank you also for taking a look @youngnick. Absolutely i will try this (although having no idea what youre saying currently 😅) Right now Im just trying to complete a simple case. This one seems useful https://github.com/projectcontour/contour/blob/main/internal/featuretests/v3/tlsroute_test.go, im trying to replicate the same. |
3e280d9
to
82b0822
Compare
TLSRoute
with Terminate modeTLSRoute
with Terminate mode
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: phuhung273 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 |
Verified with Contour, please see PR description for test output. Also added |
So, just clarifying: per our TLS Guide we have the following supports and cases:
I am wondering why we are considering a TLS = Terminate + TLSRoute here? Is this just an alternative to TLS = Terminate + TCPRoute? I think in this case it may be a bit misleading on which route I want / should use, if 2 do the same job. Also, we are explicitly saying on the TLSRoute GEP that we don't support TLSRoute termination (https://github.com/kubernetes-sigs/gateway-api/pull/4064/files#diff-7e6544694a096dc122ce2ef4d38e4a47bfe14b72d5ae3603af9c17f6ccf23339R33) so if we can first agree on the GEP on if we should or not, then move to Conformance I would appreciate for my own sanity 😅 Thanks! |
Ok can see this table in the guide Thanks @rikatz for the update. I will wait for GEP-2643 to finalize. But currently we don't have any conformance for TCPRoute in Terminate mode. So I can add one rite ? |
@rikatz TLSRoute support for attaching to Gateway listeners with It sounds like we may need to resolve some inconsistent documentation as mentioned in #1474? |
thanks Mike. I have missed those, or maybe and inconsistently left them behind. Will take a look on them, but I am wondering if it would be good/proper that we have all of this mapped on the GEP before moving with more conformance that may not reflect the final state of the GEP |
We've been somewhat inconsistent about this, but we generally haven't enforced substantial retroactive edits to older GEPs, instead allowing newer GEPs to supercede and prioritizing conformance tests and docs reflecting the current state while allowing older GEPs to stay as historical documents. |
yeah but in this case we don't have a TLSRoute GEP at all, and my plan is to have some covering all of the features/conformance that are already in place for TLSRoute |
Updated the GEP proposal to add TLSRoute termination: 23c275e |
If it's not covered in that summary table, and we have no GEP mentioning it, then we can't just change the docs and call it done. We can add it to the TLSRoute GEP as a new area, but it then needs to be reviewed and debated. Right now I don't see the use case for TLSRoute termination. |
What type of PR is this?
/kind test
/area conformance-test
What this PR does / why we need it:
This PR introduces basic same namespace conformance tests for
TLSRoute
with Terminate modeContour test
Which issue(s) this PR fixes:
Relates #3466
Does this PR introduce a user-facing change?: