Skip to content
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

CFP: Gateway API - add ALPN negotiation to cilium-envoy for gRPC support #30794

Open
rauanmayemir opened this issue Feb 15, 2024 · 3 comments · May be fixed by #32486
Open

CFP: Gateway API - add ALPN negotiation to cilium-envoy for gRPC support #30794

rauanmayemir opened this issue Feb 15, 2024 · 3 comments · May be fixed by #32486
Labels
area/servicemesh GH issues or PRs regarding servicemesh feature/k8s-gateway-api kind/feature This introduces new functionality. sig/agent Cilium agent related.

Comments

@rauanmayemir
Copy link
Contributor

Cilium Feature Proposal

cilium-envoy is currently missing ALPN config section in the DownstreamTlsContext which results in envoy not exposing ALPN altogether.

The reason why we even need ALPN config is because as per gRPC protocol, clients must use HTTP2 and they must negotiate ALPN. While the requirement for HTTP2 is understandable, it's not clear to me why doesn't gRPC allow http2-prior-knowledge (HTTP/2 over a plain TCP connection). grpc/grpc-java#5167 grpc/grpc-java#7201 grpc/grpc-java#5543 This is reproducible throughout most of the platforms: I tried Java, Go, Python, PHP, and all of them fail sending gRPC tls client requests without ALPN.

Supporting ALPN is a very trivial patch to cilium-operator:

Index: operator/pkg/model/translation/envoy_listener.go
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/operator/pkg/model/translation/envoy_listener.go b/operator/pkg/model/translation/envoy_listener.go
--- a/operator/pkg/model/translation/envoy_listener.go	(revision 93d4f151799025dfe50b290e2c8f249773f13f9c)
+++ b/operator/pkg/model/translation/envoy_listener.go	(date 1706955589616)
@@ -277,6 +277,7 @@
 	downStreamContext := envoy_extensions_transport_sockets_tls_v3.DownstreamTlsContext{
 		CommonTlsContext: &envoy_extensions_transport_sockets_tls_v3.CommonTlsContext{
 			TlsCertificateSdsSecretConfigs: tlsSdsConfig,
+			AlpnProtocols:                  []string{"h2,http/1.1"},
 		},
 	}

It will unbreak gRPC support, but will break backends that do not serve dual HTTP1.1/HTTP2-H2C.

Since envoy is set up with useDownstreamProtocolConfig for connecting to upstream backends (i.e pods), it works fine for regular browser requests because without ALPN, browser will choose HTTP1.1 and receive the expected response.

However, if ALPN protocols setting is present (we will need h2,http/1.1), then browser will prefer HTTP2 over HTTP1.1, so requests to backends only serving HTTP1.1 will effectively fail with '502 protocol error'.

To address that, I have a suggestion to solve #30452 first.

@rauanmayemir rauanmayemir added the kind/feature This introduces new functionality. label Feb 15, 2024
@youngnick youngnick added area/servicemesh GH issues or PRs regarding servicemesh feature/k8s-gateway-api sig/agent Cilium agent related. labels Feb 16, 2024
@dhaugli
Copy link

dhaugli commented Mar 6, 2024

Would love this to be fixed, currently struggling getting Vault to function without this. Where we have to fall back to using Traefik to serve this.

@rauanmayemir
Copy link
Contributor Author

@dhaugli I'm planning to work on #30452 before we can bring this in.

@buroa
Copy link
Contributor

buroa commented Mar 26, 2024

Patiently waiting for this.

rauanmayemir added a commit to buhta/cilium that referenced this issue May 11, 2024
This feature is hidden behind `enable-gateway-api-alpn` flag on the operator gwapi cell. The implementation will change envoy listener configuration to expose ALPN suggesting both HTTP/2 and HTTP/1.1.

Fixes: cilium#30794

Signed-off-by: Rauan Mayemir <[email protected]>
@rauanmayemir rauanmayemir linked a pull request May 11, 2024 that will close this issue
rauanmayemir added a commit to buhta/cilium that referenced this issue May 11, 2024
This feature is hidden behind `enable-gateway-api-alpn` flag on the operator gwapi cell. The implementation will change envoy listener configuration to expose ALPN suggesting both HTTP/2 and HTTP/1.1.

Fixes: cilium#30794

Signed-off-by: Rauan Mayemir <[email protected]>
rauanmayemir added a commit to buhta/cilium that referenced this issue May 13, 2024
This feature is hidden behind `enable-gateway-api-alpn` flag on the operator gwapi cell. The implementation will change envoy listener configuration to expose ALPN suggesting both HTTP/2 and HTTP/1.1.

Fixes: cilium#30794

Signed-off-by: Rauan Mayemir <[email protected]>
rauanmayemir added a commit to buhta/cilium that referenced this issue May 13, 2024
This feature is hidden behind `enable-gateway-api-alpn` flag on the operator gwapi cell. The implementation will change envoy listener configuration to expose ALPN suggesting both HTTP/2 and HTTP/1.1.

Fixes: cilium#30794

Signed-off-by: Rauan Mayemir <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/servicemesh GH issues or PRs regarding servicemesh feature/k8s-gateway-api kind/feature This introduces new functionality. sig/agent Cilium agent related.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants