Configure Envoy alpn_protocols based on service protocol #14356
Conversation
|
Hi @oulman! Thanks for this PR, it looks good, but we can work on adding more tests and verifying the intended behavior. |
|
Hi @malizz - Thanks! I've pushed a couple of changes.
|
oulman
changed the title
|
@oulman Question: you made the changes to listeners tlsContext only, but I’m wondering if we need to update the TLS contexts for other listeners (ingress listener for example) or pass through clusters too. |
|
Hi @malizz! I updated the context for the Ingress Gateway listener and added tests. Looking at pass-through clusters I'm a little confused on what actually needs to be done to support this. I found some issues (#1, #2) that make it sound like for Envoy <-> Envoy traffic ALPN isn't used in protocol negotiation. If mesh-to-mesh is OK as-is, do we need to enable ALPN negotiation for external upstream HTTP clusters? Thanks! |
| // Determine listener protocol type from configured service protocol. Don't hard fail on a config typo, | ||
| //The parse func returns default config if there is an error, so it's safe to continue. | ||
| cfg, _ := ParseProxyConfig(cfgSnap.Proxy.Config) | ||
|
|
There was a problem hiding this comment.
The default value for protocol in ParseProxyConfig function is tcp which is missing in getAlpnProtocols function. We can either add it to the switch cases or return an error if it's tcp.
There was a problem hiding this comment.
🤔 Right now calling getAlpnProtocols with tcp is returning an implicit nil because it's not matched. To confirm, we want to add an case statement for tcp but it should still return the empty alpnProtocols slice.
Description
This PR is largely based on @blake's work in #12170 but updated to use the TLSContext functions introduced in #13321. I'm not sure what the best path forward for configuring this from the Consul teams perspective but I wanted to raise a PR to start a conversation.
This fixes #11907, resolves #12106, and this internal feature request.
Testing & Reproduction steps
In my testing I verified that gRPC and HTTP2 services can negotiate h2 and http/1.1 and that services configured as HTTP will only negotiate http/1.1.
Links
Include any links here that might be helpful for people reviewing your PR (Tickets, GH issues, API docs, external benchmarks, tools docs, etc). If there are none, feel free to delete this section.
Please be mindful not to leak any customer or confidential information. HashiCorp employees may want to use our internal URL shortener to obfuscate links.
PR Checklist