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

http: alpn upstream #13922

Merged
merged 29 commits into from
Dec 15, 2020
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 11 additions & 8 deletions api/envoy/config/cluster/v3/cluster.proto
Original file line number Diff line number Diff line change
Expand Up @@ -135,9 +135,12 @@ message Cluster {
}

enum ClusterProtocolSelection {
// Cluster can only operate on one of the possible upstream protocols (HTTP1.1, HTTP2).
// If :ref:`http2_protocol_options <envoy_api_field_config.cluster.v3.Cluster.http2_protocol_options>` are
// present, HTTP2 will be used, otherwise HTTP1.1 will be used.
// If both :ref:`http2_protocol_options <envoy_api_field_config.cluster.v3.Cluster.http2_protocol_options>`
// and :ref:`http_protocol_options <envoy_api_field_config.cluster.v3.Cluster.http_protocol_options>` are
// configured, Envoy will attempt to do ALPN negotiation for TLS connections, failing
// over to HTTP/1.1 if ALPN negotiation fails.
// If only one protocol option is present it will be used as the hard-coded
// protocol. If neither is present, HTTP/1.1 will be used.
alyssawilk marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Somewhat orthogonal, while talking config though, I was of the mind to refuse to allow the new config unless TLS was explicitly configured - you can't do ALPN without TLS and given the ALPN pool "needs" to fail over to HTTP/1 I think it'd be easy to accidentally configure ALPN, forget TLS, and get locked into HTTP/1

We can't require TLS though, because there's other ALPN (say ALTS ALPN), which we use internally.
I was thinking we could make transport sockets register if they do ALPN, and reject config which enables H1/H2 without ALPN. That doesn't extend well to HTTP/3 which AFIK requires TLS/ALTS but doesn't actually do ALPN.
Worst case we could just comment a warning, and increment a stat of ALPN fails, but I'm wondering if you have ideas to make borked configs more obvious here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my comment above. This is what I was going to suggest and I think we should do this.

USE_CONFIGURED_PROTOCOL = 0;

// Use HTTP1.1 or HTTP2, depending on which one is used on the downstream connection.
Expand Down Expand Up @@ -767,14 +770,14 @@ message Cluster {

// HTTP protocol options that are applied only to upstream HTTP connections.
// These options apply to all HTTP versions.
core.v3.UpstreamHttpProtocolOptions upstream_http_protocol_options = 46;
core.v3.UpstreamHttpProtocolOptions upstream_http_protocol_options = 46 [deprecated = true];

// Additional options when handling HTTP requests upstream. These options will be applicable to
// both HTTP1 and HTTP2 requests.
core.v3.HttpProtocolOptions common_http_protocol_options = 29;
core.v3.HttpProtocolOptions common_http_protocol_options = 29 [deprecated = true];

// Additional options when handling HTTP1 requests.
core.v3.Http1ProtocolOptions http_protocol_options = 13;
core.v3.Http1ProtocolOptions http_protocol_options = 13 [deprecated = true];

// Even if default HTTP2 protocol options are desired, this field must be
// set so that Envoy will assume that the upstream supports HTTP/2 when
Expand All @@ -783,7 +786,7 @@ message Cluster {
// with ALPN, `http2_protocol_options` must be specified. As an aside this allows HTTP/2
// connections to happen over plain text.
core.v3.Http2ProtocolOptions http2_protocol_options = 14
[(udpa.annotations.security).configure_for_untrusted_upstream = true];
[deprecated = true, (udpa.annotations.security).configure_for_untrusted_upstream = true];

// The extension_protocol_options field is used to provide extension-specific protocol options
// for upstream connections. The key should match the extension filter name, such as
Expand Down Expand Up @@ -913,7 +916,7 @@ message Cluster {
core.v3.Metadata metadata = 25;

// Determines how Envoy selects the protocol used to speak to upstream hosts.
ClusterProtocolSelection protocol_selection = 26;
ClusterProtocolSelection protocol_selection = 26 [deprecated = true];

// Optional options for upstream connections.
UpstreamConnectionOptions upstream_connection_options = 30;
Expand Down
39 changes: 10 additions & 29 deletions api/envoy/config/cluster/v4alpha/cluster.proto

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -834,3 +834,50 @@ message RequestIDExtension {
// Request ID extension specific configuration.
google.protobuf.Any typed_config = 1;
}

// If this is used, the cluster will only operate on one of the possible upstream protocols (HTTP/1.1, HTTP/2).
// If :ref:`http2_protocol_options <envoy_api_field_config.cluster.v3.Cluster.http2_protocol_options>` are
// present, HTTP2 will be used, otherwise HTTP1.1 will be used.
message ExplicitHttpConfig {
oneof protocol_config {
config.core.v3.Http1ProtocolOptions http_protocol_options = 1;

config.core.v3.Http1ProtocolOptions http2_protocol_options = 2;
}
}

// If this is used, the cluster can use either of the configured protocols, and
// will use whichecer protocol was used by the downstream connection.
message UseDownstreamHttpConfig {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So one more question for the @envoyproxy/api-shepherds

Do we want this in the HCM?
Right now we've got this thing pattern of upstream getting a per-network-filter config, which is why I've stuck this there, but realistically this creates hard deps from the Upstream config to the HCM config, which I'm not a big fan of. This IMO is really the intersection of HTTP and upstream, not the intersection of HCM and upstream, but I can't think of a better place to put it so baring advice I guess I'll go with this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO it's fine to have this here, but I think if you want to put this in a new proto closer to the upstream configs that would be fine also?

config.core.v3.Http1ProtocolOptions http_protocol_options = 1;

config.core.v3.Http1ProtocolOptions http2_protocol_options = 2;
}

// If this is used, Envoy will negotiate ALPN to determine if HTTP/1 or HTTP/2 should be used.
message AlpnHttpConfig {
alyssawilk marked this conversation as resolved.
Show resolved Hide resolved
config.core.v3.Http1ProtocolOptions http_protocol_options = 3;

config.core.v3.Http1ProtocolOptions http2_protocol_options = 4;
}

// HttpProtocolOptions specifies Http upstream protocol options. This object
// is used in
// :ref:`typed_extension_protocol_options<envoy_api_field_config.cluster.v3.Cluster.typed_extension_protocol_options>`,
// // keyed by the name `envoy.filters.network.http_connection_manager`.
//
// This controls what protocol should be used for upstream.
// [#next-free-field: 6]
message HttpProtocolOptions {
config.core.v3.HttpProtocolOptions common_http_protocol_options = 1;

config.core.v3.UpstreamHttpProtocolOptions upstream_http_protocol_options = 2;

oneof upstream_protocol_options {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this one can have a required validation.

ExplicitHttpConfig explicit_http_config = 3;

UseDownstreamHttpConfig use_downstream_protocol_config = 4;

AlpnHttpConfig alpn_config = 5;
}
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions docs/root/version_history/current.rst
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ New Features
* hds: added support for delta updates in the :ref:`HealthCheckSpecifier <envoy_v3_api_msg_service.health.v3.HealthCheckSpecifier>`, making only the Endpoints and Health Checkers that changed be reconstructed on receiving a new message, rather than the entire HDS.
* health_check: added option to use :ref:`no_traffic_healthy_interval <envoy_v3_api_field_config.core.v3.HealthCheck.no_traffic_healthy_interval>` which allows a different no traffic interval when the host is healthy.
* http: added frame flood and abuse checks to the upstream HTTP/2 codec. This check is off by default and can be enabled by setting the `envoy.reloadable_features.upstream_http2_flood_checks` runtime key to true.
* http: alpn is now supported upstream, configurable by setting both :ref:`HTTP/1 options <envoy_v3_api_msg_config.core.v3.HttpProtocolOptions>` and :ref:`HTTP/2 options <envoy_v3_api_msg_config.core.v3.Http2ProtocolOptions>` for a given cluster.
* jwt_authn: added support for :ref:`per-route config <envoy_v3_api_msg_extensions.filters.http.jwt_authn.v3.PerRouteConfig>`.
* listener: added an optional :ref:`default filter chain <envoy_v3_api_field_config.listener.v3.Listener.default_filter_chain>`. If this field is supplied, and none of the :ref:`filter_chains <envoy_v3_api_field_config.listener.v3.Listener.filter_chains>` matches, this default filter chain is used to serve the connection.
* lua: added `downstreamDirectRemoteAddress()` and `downstreamLocalAddress()` APIs to :ref:`streamInfo() <config_http_filters_lua_stream_info_wrapper>`.
Expand Down
19 changes: 11 additions & 8 deletions generated_api_shadow/envoy/config/cluster/v3/cluster.proto

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

24 changes: 15 additions & 9 deletions generated_api_shadow/envoy/config/cluster/v4alpha/cluster.proto

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading