-
Notifications
You must be signed in to change notification settings - Fork 339
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
Generate inbound/outbound for HTTP/2 #998
Conversation
Signed-off-by: Nikolay Nikolaev <nikolay.nikolaev@konghq.com>
Signed-off-by: Nikolay Nikolaev <nikolay.nikolaev@konghq.com>
Signed-off-by: Nikolay Nikolaev <nikolay.nikolaev@konghq.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know this is a matter of taste, I've simply put here my suggestions
@@ -53,8 +53,10 @@ func (g InboundProxyGenerator) Generate(ctx xds_context.Context, proxy *model.Pr | |||
filterChainBuilder := func() *envoy_listeners.FilterChainBuilder { | |||
filterChainBuilder := envoy_listeners.NewFilterChainBuilder() | |||
switch protocol { | |||
// configuration for HTTP case | |||
case mesh_core.ProtocolHTTP: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd still prefer :)
case mesh_core.ProtocolHTTP: | |
case mesh_core.ProtocolHTTP, mesh_core.ProtocolHTTP2: |
fallthrough | ||
case mesh_core.ProtocolHTTP2: | ||
fallthrough | ||
case mesh_core.ProtocolGRPC: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and
case mesh_core.ProtocolGRPC: | |
case mesh_core.ProtocolHTTP, mesh_core.ProtocolHTTP2, mesh_core.ProtocolGRPC: |
case mesh_core.ProtocolHTTP: | ||
fallthrough | ||
case mesh_core.ProtocolHTTP2: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
case mesh_core.ProtocolHTTP2: | |
case mesh_core.ProtocolHTTP, mesh_core.ProtocolHTTP2: |
@nickolaev #991 is also fixed by this PR i think. |
Double tested with HTTP/2 and gRPC server client |
Signed-off-by: Nikolay Nikolaev <nikolay.nikolaev@konghq.com>
Signed-off-by: Nikolay Nikolaev <nikolay.nikolaev@konghq.com>
* fix(kuma-cp) generate RDS for HTTP, HTTP/2 and gRPC * fix(kuma-cp) generate inbound also for HTTP/2 * test(kuma-cp) fix xds/generator test Signed-off-by: Nikolay Nikolaev <nikolay.nikolaev@konghq.com>
Summary
As noted in #997, we have gaps in handling HTTP/2 and gRPC when generating the XDS configuration Snapshots. This PR proposes a fix for this on all the relevant places, plus adds/fixes the relevant tests.
Issues resolved
Fix #997 #991
Documentation