-
Notifications
You must be signed in to change notification settings - Fork 591
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
GRPCRoute
conformance
#5705
Comments
Let me pick up this one. |
Update: We encountered errors in some conformance test cases. This happens because when the KIC translator processes the GRPCRoute resource, it sets the default protocol to kubernetes-ingress-controller/internal/dataplane/translator/translate_grpcroute.go Lines 56 to 59 in 5701912
More background context here:
When we implemented this feature, the Kong Gateway version used was 3.5, which is based on OpenResty 1.21.4.2. Since version 3.6, it is based on 1.25.3.1 which added support for "Allow h2c and HTTP/1.1 support on the same listening socket" We have set this default value grpcs in KIC#5283 because we don't want to break users existing configurations.
Why we need to change the default GRPCRoute's protocol
|
Decision reached with @tao12345666333 :
|
What's the actual conformance test that fails? Is it actually failing because the upstream traffic is sent over grpcs instead of grpc. or just because the gate is disabled? If conformance is deploying an application that only supports plaintext, we should probably actually try to change that upstream, since it should be controlled by the Service, not any of the GWAPI resources, and shouldn't be a test requirement as such. None of https://gateway-api.sigs.k8s.io/guides/grpc-routing/ appear to mention any default, and https://gateway-api.sigs.k8s.io/geps/gep-1016/#http2-cleartext mentions that this is more for testing, so it probably shouldn't be a default. AFAICT the actual tests only care that we reach the correct backend, and don't see which protocol we use to do so. The Deployments created by https://github.com/kubernetes-sigs/gateway-api/blob/main/conformance/base/manifests.yaml do indeed not support TLS based on basic testing, but we shouldn't take that as a de facto part of the standard, it's just a shortcoming of the test environment. Those Services arguably should indicate their protocol explicitly, though there's apparently no standard or de facto standard value for gRPC in https://gateway-api.sigs.k8s.io/api-types/backendtlspolicy/ is another option, but I don't think we have any support for it yet (and the test manifests don't currently include it). @shaneutt do you know anything about this? The tests apparently impose a hidden requirement that's arguably at odds with typical production gRPC expectations, where using encrypted HTTP/2 as the underlying transport is the norm. |
I'm unavailable and wont be able to dig in for the next few days. I did however share this thread with other Gateway API folks and we're agreed to hold on releasing |
Interim conclusion:
I will complete this goal through the following steps to avoid generating a huge PR and facilitate review.
|
since #5776 hasn't been merged, I will reopen this issue. |
the docs PR Kong/docs.konghq.com#7501 has been merged. I think we can close this one. |
Is there an existing issue for this?
Problem Statement
Gateway API v1.1 will be released soon with a new set of GRPC conformance tests, and the graduation of the
GRPCRoute
API. Since we are implementing theGRPCRoute
, we want to run the newGRPCRoute
tests and claim conformance with it.Acceptance Criteria
GRPCRoute
conformance tests are successfully run.The text was updated successfully, but these errors were encountered: