test(conformance):Add BackendTLSPolicy conformance test for GRPCRoute#4882
test(conformance):Add BackendTLSPolicy conformance test for GRPCRoute#4882Thealisyed wants to merge 1 commit into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Thealisyed 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 |
|
Hi @Thealisyed. 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 Tip We noticed you've done this a few times! Consider joining the org to skip this step and gain Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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. |
Add BackendTLSPolicy conformance test for GRPCRoute Add conformance tests verifying that BackendTLSPolicy works correctly with GRPCRoute, covering valid TLS configuration, hostname mismatch and CA certificate mismatch scenarios. Also adds MakeRequestAndExpectEventuallyConsistentFailure to the gRPC test utilities and registers BackendTLSPolicy features in the GATEWAY-GRPC conformance profile. Ai tool was used to assist Signed-off-by: Ali Syed <thealisyed@gmail.com>
51b323d to
3c1d305
Compare
|
Istio results: |
| spec: | ||
| containers: | ||
| - name: grpc-tls-backend | ||
| image: registry.k8s.io/gateway-api/echo-basic:v1.5.1 |
There was a problem hiding this comment.
Would this need to be updated when new releases are cut ?
There was a problem hiding this comment.
yes, we may want to sync all of them soon
| @@ -0,0 +1,199 @@ | |||
| apiVersion: apps/v1 | |||
There was a problem hiding this comment.
Instead of creating a new deployment, why not just update the ones existing on conformance/base/manifests.yaml to also set a TLS endpoint for GRPC? You can even pick just one and use it
There was a problem hiding this comment.
but grps is served on the same port as https? 8443, I guess we need a different port
There was a problem hiding this comment.
Ack, will add TLS to the existing grpc-infra-backend-v1 in base/manifests.yaml on port 8443
|
@Thealisyed I am not getting it passing on Istio, maybe we need to investigate why better and if this is related to the test or not. The negative mismatched tests not passing is fine, as something is erroring. But I wanted to have at least one scenario where this passes. Maybe test with other implementations as well (envoy gateway, or kgateway) and see if this is a problem of implementation or a problem of Istio? :) |
| Features: []features.FeatureName{ | ||
| features.SupportGateway, | ||
| features.SupportGRPCRoute, | ||
| features.SupportBackendTLSPolicy, |
There was a problem hiding this comment.
I think we should use different tag for this SupportBckendTLSPolicyForGRPC?
There was a problem hiding this comment.
Our test requires both SupportGRPCRoute and SupporttBackendTLSPolicy so only implementations that claim both would run it. If we implement the per route type feature then it would be introducing a new pattern and be problematic maybe?
There was a problem hiding this comment.
@kl52752 this is based on the Union feature concept. When an implementation claims to support GRPCRoute and to support BackendTLSPolicy they must support both.
| @@ -0,0 +1,199 @@ | |||
| apiVersion: apps/v1 | |||
There was a problem hiding this comment.
but grps is served on the same port as https? 8443, I guess we need a different port
/kind test
/area conformance-test
What this PR does / why we need it:
Add BackendTLSPolicy conformance test for GRPCRoute
Add conformance tests verifying that BackendTLSPolicy works correctly with GRPCRoute, covering valid TLS configuration, hostname mismatch and CA certificate mismatch scenarios.
Also adds MakeRequestAndExpectEventuallyConsistentFailure to the gRPC test utilities and registers BackendTLSPolicy features in the GATEWAY-GRPC conformance profile.
Which issue(s) this PR fixes:
Fixes #4754
Does this PR introduce a user-facing change?: