From f90a29f3a89ee5ce3a86b28298949ffbe3508f58 Mon Sep 17 00:00:00 2001 From: Jarno Rajahalme Date: Fri, 3 Nov 2017 16:14:48 -0700 Subject: [PATCH 01/11] cds: Add "auto_http2" option. Currently clusters can not open both HTTP1.1 and HTTP2 upstream connections at the same time. When the new cluster option "auto_http2" is set to "true", the cluster must open an HTTP2 upstream connection if the downstream connection is HTTP2, and an HTTP1.1 upstream connection if the downstream connection is HTTP1.1. This option is to have no effect if there is no corresponding downstream connection. This functionality removes the need to operate multiple clusters and routing rules for them when the backends accept both HTTP1.1 and HTTP2 connections, and when the choice of the HTTP protocol is significant, as with gRPC. Signed-off-by: Jarno Rajahalme --- api/cds.proto | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/api/cds.proto b/api/cds.proto index 86a2906cf..2226051ff 100644 --- a/api/cds.proto +++ b/api/cds.proto @@ -417,6 +417,11 @@ message Cluster { // For instance, if the metadata is intended for the Router filter, the filter // name should be specified as *envoy.router*. Metadata metadata = 25; + + // Cluster can operate both HTTP1.1 and HTTP2 connections and the protocol + // is chosen based on the downstream protocol, if any. In case there is no + // downstream connection, this option will be ignored. + bool auto_http2 = 26; } // An extensible structure containing the address Envoy should bind to when From 8b90cc7027a6934b263dfe0973dd5040a75ec877 Mon Sep 17 00:00:00 2001 From: Jarno Rajahalme Date: Fri, 5 Jan 2018 17:27:27 -0800 Subject: [PATCH 02/11] cds: Fix spacing. Remove over-enthusiastic spaces. Signed-off-by: Jarno Rajahalme --- api/cds.proto | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/cds.proto b/api/cds.proto index 2226051ff..3bd4f4980 100644 --- a/api/cds.proto +++ b/api/cds.proto @@ -419,7 +419,7 @@ message Cluster { Metadata metadata = 25; // Cluster can operate both HTTP1.1 and HTTP2 connections and the protocol - // is chosen based on the downstream protocol, if any. In case there is no + // is chosen based on the downstream protocol, if any. In case there is no // downstream connection, this option will be ignored. bool auto_http2 = 26; } From 935aee0e39c758f981cfa53845b6ee40e86e8d28 Mon Sep 17 00:00:00 2001 From: Jarno Rajahalme Date: Mon, 8 Jan 2018 10:23:19 -0800 Subject: [PATCH 03/11] cds: Allow multiple protocol options be present. Remove the oneof qualifier from protocol options. If multiple options are present, then the cluster can use multiple upstream protocols. If both HTTP1.1 and HTTP2 options are present, then the cluster will choose the upstream protocol based on the downstream connection. Signed-off-by: Jarno Rajahalme --- api/cds.proto | 39 ++++++++++++++++----------------------- 1 file changed, 16 insertions(+), 23 deletions(-) diff --git a/api/cds.proto b/api/cds.proto index 3bd4f4980..57f2415a0 100644 --- a/api/cds.proto +++ b/api/cds.proto @@ -161,24 +161,22 @@ message Cluster { // verification. UpstreamTlsContext tls_context = 11; - oneof protocol_options { - // [#not-implemented-hide:] - TcpProtocolOptions tcp_protocol_options = 12; - - // Additional options when handling HTTP1 requests. - Http1ProtocolOptions http_protocol_options = 13; - - // 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 - // making new HTTP connection pool connections. Currently, Envoy only - // supports prior knowledge for upstream connections. Even if TLS is used - // with ALPN, `http2_protocol_options` must be specified. As an aside this allows HTTP/2 - // connections to happen over plain text. - Http2ProtocolOptions http2_protocol_options = 14; - - // [#not-implemented-hide:] - GrpcProtocolOptions grpc_protocol_options = 15; - } + // [#not-implemented-hide:] + TcpProtocolOptions tcp_protocol_options = 12; + + // Additional options when handling HTTP1 requests. + Http1ProtocolOptions http_protocol_options = 13; + + // 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 + // making new HTTP connection pool connections. Currently, Envoy only + // supports prior knowledge for upstream connections. Even if TLS is used + // with ALPN, `http2_protocol_options` must be specified. As an aside this allows HTTP/2 + // connections to happen over plain text. + Http2ProtocolOptions http2_protocol_options = 14; + + // [#not-implemented-hide:] + GrpcProtocolOptions grpc_protocol_options = 15; // If the DNS refresh rate is specified and the cluster type is either // :ref:`STRICT_DNS`, @@ -417,11 +415,6 @@ message Cluster { // For instance, if the metadata is intended for the Router filter, the filter // name should be specified as *envoy.router*. Metadata metadata = 25; - - // Cluster can operate both HTTP1.1 and HTTP2 connections and the protocol - // is chosen based on the downstream protocol, if any. In case there is no - // downstream connection, this option will be ignored. - bool auto_http2 = 26; } // An extensible structure containing the address Envoy should bind to when From 3ae475637e1fc3502abba879a8f16015e6891e86 Mon Sep 17 00:00:00 2001 From: Jarno Rajahalme Date: Mon, 8 Jan 2018 11:53:54 -0800 Subject: [PATCH 04/11] cds: Add protocol_selection option. Add a new enum ClusterProtocolSelection and a corresponding 'protocol_selection' option. Default value enforces backwards compatible behavior of exclusively using HTTP/1 or HTTP/2 depending on the protocol options present. Signed-off-by: Jarno Rajahalme --- api/cds.proto | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/api/cds.proto b/api/cds.proto index 57f2415a0..54aad6d32 100644 --- a/api/cds.proto +++ b/api/cds.proto @@ -415,6 +415,13 @@ message Cluster { // For instance, if the metadata is intended for the Router filter, the filter // name should be specified as *envoy.router*. Metadata metadata = 25; + + enum ClusterProtocolSelection { + // Cluster can only operate on one of the possible upstream protocols (HTTP1.1, HTTP2). + // If http2_protocol_options are present, HTTP2 will be used, otherwise HTTP1.1 will be used. + EXCLUSIVE_AS_CONFIGURED = 0; + } + ClusterProtocolSelection protocol_selection = 26; } // An extensible structure containing the address Envoy should bind to when From d2a491c2b38334407c4e4a3b6706dfae2488f2c8 Mon Sep 17 00:00:00 2001 From: Jarno Rajahalme Date: Mon, 8 Jan 2018 11:58:07 -0800 Subject: [PATCH 05/11] cds: Add 'USE_DOWNSTREAM_PROTO' enum value to 'ClusterProtocolSelection'. Use HTTP1.1 or HTTP2, depending on which one is used on the downstream connection. Signed-off-by: Jarno Rajahalme --- api/cds.proto | 2 ++ 1 file changed, 2 insertions(+) diff --git a/api/cds.proto b/api/cds.proto index 54aad6d32..9d8d72d8e 100644 --- a/api/cds.proto +++ b/api/cds.proto @@ -420,6 +420,8 @@ message Cluster { // Cluster can only operate on one of the possible upstream protocols (HTTP1.1, HTTP2). // If http2_protocol_options are present, HTTP2 will be used, otherwise HTTP1.1 will be used. EXCLUSIVE_AS_CONFIGURED = 0; + // Use HTTP1.1 or HTTP2, depending on which one is used on the downstream connection. + USE_DOWNSTREAM_PROTO = 1; } ClusterProtocolSelection protocol_selection = 26; } From cc5c6f6e8781b2cddf43786182fa456fe3d99994 Mon Sep 17 00:00:00 2001 From: Jarno Rajahalme Date: Mon, 8 Jan 2018 13:24:15 -0800 Subject: [PATCH 06/11] cds: Spell out PROCOCOL. Better avoid 'proto' when not meaning protobufs. Suggested-by: Harvey Tuch Signed-off-by: Jarno Rajahalme --- api/cds.proto | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/cds.proto b/api/cds.proto index 9d8d72d8e..896bbe714 100644 --- a/api/cds.proto +++ b/api/cds.proto @@ -421,7 +421,7 @@ message Cluster { // If http2_protocol_options are present, HTTP2 will be used, otherwise HTTP1.1 will be used. EXCLUSIVE_AS_CONFIGURED = 0; // Use HTTP1.1 or HTTP2, depending on which one is used on the downstream connection. - USE_DOWNSTREAM_PROTO = 1; + USE_DOWNSTREAM_PROTOCOL = 1; } ClusterProtocolSelection protocol_selection = 26; } From 5f59143263e9c1b70abb3cc8b208a73273318dc5 Mon Sep 17 00:00:00 2001 From: Jarno Rajahalme Date: Mon, 8 Jan 2018 13:31:31 -0800 Subject: [PATCH 07/11] cds: Drop unimplemented TCP and gRPC protocol options. Now that the protocol options are no longer mutually exclusive, HTTP2 options can also be used for gRPC. We can add these back in when needed. The old code points remain unused. Signed-off-by: Jarno Rajahalme --- api/cds.proto | 6 ------ 1 file changed, 6 deletions(-) diff --git a/api/cds.proto b/api/cds.proto index 896bbe714..445b7a1ea 100644 --- a/api/cds.proto +++ b/api/cds.proto @@ -161,9 +161,6 @@ message Cluster { // verification. UpstreamTlsContext tls_context = 11; - // [#not-implemented-hide:] - TcpProtocolOptions tcp_protocol_options = 12; - // Additional options when handling HTTP1 requests. Http1ProtocolOptions http_protocol_options = 13; @@ -175,9 +172,6 @@ message Cluster { // connections to happen over plain text. Http2ProtocolOptions http2_protocol_options = 14; - // [#not-implemented-hide:] - GrpcProtocolOptions grpc_protocol_options = 15; - // If the DNS refresh rate is specified and the cluster type is either // :ref:`STRICT_DNS`, // or :ref:`LOGICAL_DNS`, From a4fa3e95bb8d0c16de6e8cb9aae8adb61ed1bc99 Mon Sep 17 00:00:00 2001 From: Jarno Rajahalme Date: Tue, 9 Jan 2018 13:16:46 -0800 Subject: [PATCH 08/11] cds: Link comment to http2_protocol_options. Requested-by: Matt Klein Signed-off-by: Jarno Rajahalme --- api/cds.proto | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/api/cds.proto b/api/cds.proto index 445b7a1ea..9637e26f7 100644 --- a/api/cds.proto +++ b/api/cds.proto @@ -412,7 +412,8 @@ message Cluster { enum ClusterProtocolSelection { // Cluster can only operate on one of the possible upstream protocols (HTTP1.1, HTTP2). - // If http2_protocol_options are present, HTTP2 will be used, otherwise HTTP1.1 will be used. + // If :ref:`http2_protocol_options ` are + // present, HTTP2 will be used, otherwise HTTP1.1 will be used. EXCLUSIVE_AS_CONFIGURED = 0; // Use HTTP1.1 or HTTP2, depending on which one is used on the downstream connection. USE_DOWNSTREAM_PROTOCOL = 1; From 1c5328b782f3642d01119d4ea1d137004805461a Mon Sep 17 00:00:00 2001 From: Jarno Rajahalme Date: Tue, 9 Jan 2018 13:19:39 -0800 Subject: [PATCH 09/11] cds: Add comments for deleted fields. Requested-by: Matt Klein Signed-off-by: Jarno Rajahalme --- api/cds.proto | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/api/cds.proto b/api/cds.proto index 9637e26f7..49a9365a3 100644 --- a/api/cds.proto +++ b/api/cds.proto @@ -161,6 +161,8 @@ message Cluster { // verification. UpstreamTlsContext tls_context = 11; + // deleted = 12; + // Additional options when handling HTTP1 requests. Http1ProtocolOptions http_protocol_options = 13; @@ -172,6 +174,8 @@ message Cluster { // connections to happen over plain text. Http2ProtocolOptions http2_protocol_options = 14; + // deleted = 15; + // If the DNS refresh rate is specified and the cluster type is either // :ref:`STRICT_DNS`, // or :ref:`LOGICAL_DNS`, From fe29d04c21cbc268fcf7f068e6a2417973f3a964 Mon Sep 17 00:00:00 2001 From: Jarno Rajahalme Date: Tue, 9 Jan 2018 13:26:37 -0800 Subject: [PATCH 10/11] cds: Fix spacing. Signed-off-by: Jarno Rajahalme --- api/cds.proto | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/cds.proto b/api/cds.proto index 49a9365a3..3c8fe865b 100644 --- a/api/cds.proto +++ b/api/cds.proto @@ -162,7 +162,7 @@ message Cluster { UpstreamTlsContext tls_context = 11; // deleted = 12; - + // Additional options when handling HTTP1 requests. Http1ProtocolOptions http_protocol_options = 13; @@ -175,7 +175,7 @@ message Cluster { Http2ProtocolOptions http2_protocol_options = 14; // deleted = 15; - + // If the DNS refresh rate is specified and the cluster type is either // :ref:`STRICT_DNS`, // or :ref:`LOGICAL_DNS`, From 2df30180e73f1de43da8916304bc7c0a909fba66 Mon Sep 17 00:00:00 2001 From: Jarno Rajahalme Date: Wed, 10 Jan 2018 10:13:37 -0800 Subject: [PATCH 11/11] cds: Review fixes. Requested-by: Hong Zhang Signed-off-by: Jarno Rajahalme --- api/cds.proto | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/api/cds.proto b/api/cds.proto index 3c8fe865b..a80137fdf 100644 --- a/api/cds.proto +++ b/api/cds.proto @@ -161,7 +161,7 @@ message Cluster { // verification. UpstreamTlsContext tls_context = 11; - // deleted = 12; + reserved 12; // Additional options when handling HTTP1 requests. Http1ProtocolOptions http_protocol_options = 13; @@ -174,7 +174,7 @@ message Cluster { // connections to happen over plain text. Http2ProtocolOptions http2_protocol_options = 14; - // deleted = 15; + reserved 15; // If the DNS refresh rate is specified and the cluster type is either // :ref:`STRICT_DNS`, @@ -418,7 +418,7 @@ message Cluster { // Cluster can only operate on one of the possible upstream protocols (HTTP1.1, HTTP2). // If :ref:`http2_protocol_options ` are // present, HTTP2 will be used, otherwise HTTP1.1 will be used. - EXCLUSIVE_AS_CONFIGURED = 0; + USE_CONFIGURED_PROTOCOL = 0; // Use HTTP1.1 or HTTP2, depending on which one is used on the downstream connection. USE_DOWNSTREAM_PROTOCOL = 1; }