From 9c80b61e832180f28c482e2ade0e2c673bfec2d1 Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Tue, 26 Jan 2021 15:45:57 -0500 Subject: [PATCH 1/2] http3: adding upstream API hooks Signed-off-by: Alyssa Wilk --- api/envoy/config/core/v3/protocol.proto | 5 ++ api/envoy/config/core/v4alpha/protocol.proto | 8 +++ .../http/v3/http_protocol_options.proto | 8 ++- .../http/v4alpha/http_protocol_options.proto | 8 ++- generated_api_shadow/BUILD | 1 + .../envoy/config/core/v3/protocol.proto | 5 ++ .../envoy/config/core/v4alpha/protocol.proto | 8 +++ .../http/v3/http_protocol_options.proto | 8 ++- .../http/v4alpha/http_protocol_options.proto | 8 ++- include/envoy/upstream/upstream.h | 2 + source/common/http/utility.cc | 19 ++++++- source/common/http/utility.h | 12 +++++ .../common/upstream/cluster_manager_impl.cc | 6 +-- source/common/upstream/upstream_impl.cc | 8 +++ source/extensions/upstreams/http/config.cc | 22 +++++++- source/extensions/upstreams/http/config.h | 2 + test/common/http/utility_test.cc | 11 ++++ test/common/upstream/upstream_impl_test.cc | 50 +++++++++++++++++++ 18 files changed, 181 insertions(+), 10 deletions(-) diff --git a/api/envoy/config/core/v3/protocol.proto b/api/envoy/config/core/v3/protocol.proto index 80d971c1466b..359d59e0f29b 100644 --- a/api/envoy/config/core/v3/protocol.proto +++ b/api/envoy/config/core/v3/protocol.proto @@ -389,3 +389,8 @@ message GrpcProtocolOptions { Http2ProtocolOptions http2_protocol_options = 1; } + +// [#not-implemented-hide:] +message Http3ProtocolOptions { + Http2ProtocolOptions http2_protocol_options = 1; +} diff --git a/api/envoy/config/core/v4alpha/protocol.proto b/api/envoy/config/core/v4alpha/protocol.proto index 60f0b3210805..09bcd838dbb7 100644 --- a/api/envoy/config/core/v4alpha/protocol.proto +++ b/api/envoy/config/core/v4alpha/protocol.proto @@ -382,3 +382,11 @@ message GrpcProtocolOptions { Http2ProtocolOptions http2_protocol_options = 1; } + +// [#not-implemented-hide:] +message Http3ProtocolOptions { + option (udpa.annotations.versioning).previous_message_type = + "envoy.config.core.v3.Http3ProtocolOptions"; + + Http2ProtocolOptions http2_protocol_options = 1; +} diff --git a/api/envoy/extensions/upstreams/http/v3/http_protocol_options.proto b/api/envoy/extensions/upstreams/http/v3/http_protocol_options.proto index e7cf42df2387..00cac9d27336 100644 --- a/api/envoy/extensions/upstreams/http/v3/http_protocol_options.proto +++ b/api/envoy/extensions/upstreams/http/v3/http_protocol_options.proto @@ -58,7 +58,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // .... [further cluster config] // [#next-free-field: 6] message HttpProtocolOptions { - // If this is used, the cluster will only operate on one of the possible upstream protocols (HTTP/1.1, HTTP/2). + // If this is used, the cluster will only operate on one of the possible upstream protocols. // Note that HTTP/2 should generally be used for upstream clusters doing gRPC. message ExplicitHttpConfig { oneof protocol_config { @@ -67,6 +67,9 @@ message HttpProtocolOptions { config.core.v3.Http1ProtocolOptions http_protocol_options = 1; config.core.v3.Http2ProtocolOptions http2_protocol_options = 2; + + // [#not-implemented-hide:] + config.core.v3.Http3ProtocolOptions http3_protocol_options = 3; } } @@ -76,6 +79,9 @@ message HttpProtocolOptions { config.core.v3.Http1ProtocolOptions http_protocol_options = 1; config.core.v3.Http2ProtocolOptions http2_protocol_options = 2; + + // [#not-implemented-hide:] + config.core.v3.Http3ProtocolOptions http3_protocol_options = 3; } // If this is used, the cluster can use either HTTP/1 or HTTP/2, and will use whichever diff --git a/api/envoy/extensions/upstreams/http/v4alpha/http_protocol_options.proto b/api/envoy/extensions/upstreams/http/v4alpha/http_protocol_options.proto index 277ceb9aa989..e3cf4476983a 100644 --- a/api/envoy/extensions/upstreams/http/v4alpha/http_protocol_options.proto +++ b/api/envoy/extensions/upstreams/http/v4alpha/http_protocol_options.proto @@ -62,7 +62,7 @@ message HttpProtocolOptions { option (udpa.annotations.versioning).previous_message_type = "envoy.extensions.upstreams.http.v3.HttpProtocolOptions"; - // If this is used, the cluster will only operate on one of the possible upstream protocols (HTTP/1.1, HTTP/2). + // If this is used, the cluster will only operate on one of the possible upstream protocols. // Note that HTTP/2 should generally be used for upstream clusters doing gRPC. message ExplicitHttpConfig { option (udpa.annotations.versioning).previous_message_type = @@ -74,6 +74,9 @@ message HttpProtocolOptions { config.core.v4alpha.Http1ProtocolOptions http_protocol_options = 1; config.core.v4alpha.Http2ProtocolOptions http2_protocol_options = 2; + + // [#not-implemented-hide:] + config.core.v4alpha.Http3ProtocolOptions http3_protocol_options = 3; } } @@ -86,6 +89,9 @@ message HttpProtocolOptions { config.core.v4alpha.Http1ProtocolOptions http_protocol_options = 1; config.core.v4alpha.Http2ProtocolOptions http2_protocol_options = 2; + + // [#not-implemented-hide:] + config.core.v4alpha.Http3ProtocolOptions http3_protocol_options = 3; } // If this is used, the cluster can use either HTTP/1 or HTTP/2, and will use whichever diff --git a/generated_api_shadow/BUILD b/generated_api_shadow/BUILD index 5b4131922cd0..effde23bad70 100644 --- a/generated_api_shadow/BUILD +++ b/generated_api_shadow/BUILD @@ -166,6 +166,7 @@ proto_library( "//envoy/extensions/common/tap/v3:pkg", "//envoy/extensions/compression/gzip/compressor/v3:pkg", "//envoy/extensions/compression/gzip/decompressor/v3:pkg", + "//envoy/extensions/filters/common/dependency/v3:pkg", "//envoy/extensions/filters/common/fault/v3:pkg", "//envoy/extensions/filters/common/matcher/action/v3:pkg", "//envoy/extensions/filters/http/adaptive_concurrency/v3:pkg", diff --git a/generated_api_shadow/envoy/config/core/v3/protocol.proto b/generated_api_shadow/envoy/config/core/v3/protocol.proto index 80d971c1466b..359d59e0f29b 100644 --- a/generated_api_shadow/envoy/config/core/v3/protocol.proto +++ b/generated_api_shadow/envoy/config/core/v3/protocol.proto @@ -389,3 +389,8 @@ message GrpcProtocolOptions { Http2ProtocolOptions http2_protocol_options = 1; } + +// [#not-implemented-hide:] +message Http3ProtocolOptions { + Http2ProtocolOptions http2_protocol_options = 1; +} diff --git a/generated_api_shadow/envoy/config/core/v4alpha/protocol.proto b/generated_api_shadow/envoy/config/core/v4alpha/protocol.proto index c9fc21d4cfa4..269b59ad13ed 100644 --- a/generated_api_shadow/envoy/config/core/v4alpha/protocol.proto +++ b/generated_api_shadow/envoy/config/core/v4alpha/protocol.proto @@ -392,3 +392,11 @@ message GrpcProtocolOptions { Http2ProtocolOptions http2_protocol_options = 1; } + +// [#not-implemented-hide:] +message Http3ProtocolOptions { + option (udpa.annotations.versioning).previous_message_type = + "envoy.config.core.v3.Http3ProtocolOptions"; + + Http2ProtocolOptions http2_protocol_options = 1; +} diff --git a/generated_api_shadow/envoy/extensions/upstreams/http/v3/http_protocol_options.proto b/generated_api_shadow/envoy/extensions/upstreams/http/v3/http_protocol_options.proto index e7cf42df2387..00cac9d27336 100644 --- a/generated_api_shadow/envoy/extensions/upstreams/http/v3/http_protocol_options.proto +++ b/generated_api_shadow/envoy/extensions/upstreams/http/v3/http_protocol_options.proto @@ -58,7 +58,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // .... [further cluster config] // [#next-free-field: 6] message HttpProtocolOptions { - // If this is used, the cluster will only operate on one of the possible upstream protocols (HTTP/1.1, HTTP/2). + // If this is used, the cluster will only operate on one of the possible upstream protocols. // Note that HTTP/2 should generally be used for upstream clusters doing gRPC. message ExplicitHttpConfig { oneof protocol_config { @@ -67,6 +67,9 @@ message HttpProtocolOptions { config.core.v3.Http1ProtocolOptions http_protocol_options = 1; config.core.v3.Http2ProtocolOptions http2_protocol_options = 2; + + // [#not-implemented-hide:] + config.core.v3.Http3ProtocolOptions http3_protocol_options = 3; } } @@ -76,6 +79,9 @@ message HttpProtocolOptions { config.core.v3.Http1ProtocolOptions http_protocol_options = 1; config.core.v3.Http2ProtocolOptions http2_protocol_options = 2; + + // [#not-implemented-hide:] + config.core.v3.Http3ProtocolOptions http3_protocol_options = 3; } // If this is used, the cluster can use either HTTP/1 or HTTP/2, and will use whichever diff --git a/generated_api_shadow/envoy/extensions/upstreams/http/v4alpha/http_protocol_options.proto b/generated_api_shadow/envoy/extensions/upstreams/http/v4alpha/http_protocol_options.proto index 277ceb9aa989..e3cf4476983a 100644 --- a/generated_api_shadow/envoy/extensions/upstreams/http/v4alpha/http_protocol_options.proto +++ b/generated_api_shadow/envoy/extensions/upstreams/http/v4alpha/http_protocol_options.proto @@ -62,7 +62,7 @@ message HttpProtocolOptions { option (udpa.annotations.versioning).previous_message_type = "envoy.extensions.upstreams.http.v3.HttpProtocolOptions"; - // If this is used, the cluster will only operate on one of the possible upstream protocols (HTTP/1.1, HTTP/2). + // If this is used, the cluster will only operate on one of the possible upstream protocols. // Note that HTTP/2 should generally be used for upstream clusters doing gRPC. message ExplicitHttpConfig { option (udpa.annotations.versioning).previous_message_type = @@ -74,6 +74,9 @@ message HttpProtocolOptions { config.core.v4alpha.Http1ProtocolOptions http_protocol_options = 1; config.core.v4alpha.Http2ProtocolOptions http2_protocol_options = 2; + + // [#not-implemented-hide:] + config.core.v4alpha.Http3ProtocolOptions http3_protocol_options = 3; } } @@ -86,6 +89,9 @@ message HttpProtocolOptions { config.core.v4alpha.Http1ProtocolOptions http_protocol_options = 1; config.core.v4alpha.Http2ProtocolOptions http2_protocol_options = 2; + + // [#not-implemented-hide:] + config.core.v4alpha.Http3ProtocolOptions http3_protocol_options = 3; } // If this is used, the cluster can use either HTTP/1 or HTTP/2, and will use whichever diff --git a/include/envoy/upstream/upstream.h b/include/envoy/upstream/upstream.h index 74582b1c41a6..041579e22ddb 100644 --- a/include/envoy/upstream/upstream.h +++ b/include/envoy/upstream/upstream.h @@ -720,6 +720,8 @@ class ClusterInfo { // If USE_ALPN and HTTP2 are true, the upstream protocol will be negotiated using ALPN. // If ALPN is attempted but not supported by the upstream HTTP/1.1 is used. static const uint64_t USE_ALPN = 0x8; + // Whether the upstream supports HTTP3. This is used when creating connection pools. + static const uint64_t HTTP3 = 0x10; }; virtual ~ClusterInfo() = default; diff --git a/source/common/http/utility.cc b/source/common/http/utility.cc index 86660479c683..e05d2eaa1473 100644 --- a/source/common/http/utility.cc +++ b/source/common/http/utility.cc @@ -162,6 +162,13 @@ initializeAndValidateOptions(const envoy::config::core::v3::Http2ProtocolOptions envoy::config::core::v3::Http2ProtocolOptions initializeAndValidateOptions(const envoy::config::core::v3::Http2ProtocolOptions& options) { envoy::config::core::v3::Http2ProtocolOptions options_clone(options); + initializeAndValidateOptions(options, options_clone); + return options_clone; +} + +void initializeAndValidateOptions(const envoy::config::core::v3::Http2ProtocolOptions& options, + envoy::config::core::v3::Http2ProtocolOptions& options_clone) { + // This will throw an exception when a custom parameter and a named parameter collide. validateCustomSettingsParameters(options); @@ -217,13 +224,21 @@ initializeAndValidateOptions(const envoy::config::core::v3::Http2ProtocolOptions options_clone.mutable_max_inbound_window_update_frames_per_data_frame_sent()->set_value( OptionsLimits::DEFAULT_MAX_INBOUND_WINDOW_UPDATE_FRAMES_PER_DATA_FRAME_SENT); } - - return options_clone; } } // namespace Utility } // namespace Http2 +namespace Http3 { +namespace Utility { + +void initializeAndValidateOptions(envoy::config::core::v3::Http3ProtocolOptions& options) { + Http2::Utility::initializeAndValidateOptions(options.http2_protocol_options(), + *options.mutable_http2_protocol_options()); +} + +} // namespace Utility +} // namespace Http3 namespace Http { static const char kDefaultPath[] = "/"; diff --git a/source/common/http/utility.h b/source/common/http/utility.h index d677b097d86c..27f1e0cf1d77 100644 --- a/source/common/http/utility.h +++ b/source/common/http/utility.h @@ -115,6 +115,9 @@ struct OptionsLimits { envoy::config::core::v3::Http2ProtocolOptions initializeAndValidateOptions(const envoy::config::core::v3::Http2ProtocolOptions& options); +void initializeAndValidateOptions(const envoy::config::core::v3::Http2ProtocolOptions& from_options, + envoy::config::core::v3::Http2ProtocolOptions& to_options); + envoy::config::core::v3::Http2ProtocolOptions initializeAndValidateOptions(const envoy::config::core::v3::Http2ProtocolOptions& options, bool hcm_stream_error_set, @@ -122,6 +125,15 @@ initializeAndValidateOptions(const envoy::config::core::v3::Http2ProtocolOptions } // namespace Utility } // namespace Http2 +namespace Http3 { +namespace Utility { + +/* Does in-place initialization and validation of http3 options. */ +void initializeAndValidateOptions(envoy::config::core::v3::Http3ProtocolOptions& options); + +} // namespace Utility +} // namespace Http3 + namespace Http { namespace Utility { diff --git a/source/common/upstream/cluster_manager_impl.cc b/source/common/upstream/cluster_manager_impl.cc index 35ac22b48195..e9af3f815dd9 100644 --- a/source/common/upstream/cluster_manager_impl.cc +++ b/source/common/upstream/cluster_manager_impl.cc @@ -1490,9 +1490,9 @@ Http::ConnectionPool::InstancePtr ProdClusterManagerFactory::allocateConnPool( const Network::ConnectionSocket::OptionsSharedPtr& options, const Network::TransportSocketOptionsSharedPtr& transport_socket_options, ClusterConnectivityState& state) { - if (protocols.size() == 2 && - ((protocols[0] == Http::Protocol::Http2 && protocols[1] == Http::Protocol::Http11) || - (protocols[1] == Http::Protocol::Http2 && protocols[0] == Http::Protocol::Http11))) { + if (protocols.size() == 2) { + ASSERT((protocols[0] == Http::Protocol::Http2 && protocols[1] == Http::Protocol::Http11) || + (protocols[1] == Http::Protocol::Http2 && protocols[0] == Http::Protocol::Http11)); return std::make_unique(dispatcher, api_.randomGenerator(), host, priority, options, transport_socket_options, state); diff --git a/source/common/upstream/upstream_impl.cc b/source/common/upstream/upstream_impl.cc index c4badf0f937a..8e2a6fe43e3b 100644 --- a/source/common/upstream/upstream_impl.cc +++ b/source/common/upstream/upstream_impl.cc @@ -895,8 +895,12 @@ ClusterInfoImpl::upstreamHttpProtocol(absl::optional downstream_ features_ & Upstream::ClusterInfo::Features::USE_DOWNSTREAM_PROTOCOL) { return {downstream_protocol.value()}; } else if (features_ & Upstream::ClusterInfo::Features::USE_ALPN) { + ASSERT(!(features_ & Upstream::ClusterInfo::Features::HTTP3)); return {Http::Protocol::Http2, Http::Protocol::Http11}; } else { + if (features_ & Upstream::ClusterInfo::Features::HTTP3) { + return {Http::Protocol::Http3}; + } return {(features_ & Upstream::ClusterInfo::Features::HTTP2) ? Http::Protocol::Http2 : Http::Protocol::Http11}; } @@ -929,6 +933,10 @@ ClusterImplBase::ClusterImplBase( fmt::format("ALPN configured for cluster {} which has a non-ALPN transport socket: {}", cluster.name(), cluster.DebugString())); } + if ((info_->features() & ClusterInfoImpl::Features::HTTP3)) { + throw EnvoyException( + fmt::format("HTTP3 not yet supported: {}", cluster.name(), cluster.DebugString())); + } // Create the default (empty) priority set before registering callbacks to // avoid getting an update the first time it is accessed. diff --git a/source/extensions/upstreams/http/config.cc b/source/extensions/upstreams/http/config.cc index 2d8cbd447c4f..aa894181ef93 100644 --- a/source/extensions/upstreams/http/config.cc +++ b/source/extensions/upstreams/http/config.cc @@ -41,6 +41,19 @@ getHttp2Options(const envoy::extensions::upstreams::http::v3::HttpProtocolOption return options.explicit_http_config().http2_protocol_options(); } +absl::optional +getHttp3Options(const envoy::extensions::upstreams::http::v3::HttpProtocolOptions& options) { + if (options.has_use_downstream_protocol_config() && + options.use_downstream_protocol_config().has_http3_protocol_options()) { + return options.use_downstream_protocol_config().http3_protocol_options(); + } + if (options.has_explicit_http_config() && + options.explicit_http_config().has_http3_protocol_options()) { + return options.explicit_http_config().http3_protocol_options(); + } + return {}; +} + } // namespace uint64_t ProtocolOptionsConfigImpl::parseFeatures(const envoy::config::cluster::v3::Cluster& config, @@ -50,13 +63,15 @@ uint64_t ProtocolOptionsConfigImpl::parseFeatures(const envoy::config::cluster:: if (options.use_http2_) { features |= Upstream::ClusterInfo::Features::HTTP2; } + if (options.use_http3_) { + features |= Upstream::ClusterInfo::Features::HTTP3; + } if (options.use_downstream_protocol_) { features |= Upstream::ClusterInfo::Features::USE_DOWNSTREAM_PROTOCOL; } if (options.use_alpn_) { features |= Upstream::ClusterInfo::Features::USE_ALPN; } - if (config.close_connections_on_host_health_failure()) { features |= Upstream::ClusterInfo::Features::CLOSE_CONNECTIONS_ON_HOST_HEALTH_FAILURE; } @@ -67,12 +82,17 @@ ProtocolOptionsConfigImpl::ProtocolOptionsConfigImpl( const envoy::extensions::upstreams::http::v3::HttpProtocolOptions& options) : http1_settings_(Envoy::Http::Utility::parseHttp1Settings(getHttpOptions(options))), http2_options_(Http2::Utility::initializeAndValidateOptions(getHttp2Options(options))), + http3_options_(getHttp3Options(options)), common_http_protocol_options_(options.common_http_protocol_options()), upstream_http_protocol_options_( options.has_upstream_http_protocol_options() ? absl::make_optional( options.upstream_http_protocol_options()) : absl::nullopt) { + if (http3_options_.has_value()) { + use_http3_ = true; + Http3::Utility::initializeAndValidateOptions(http3_options_.value()); + } if (options.has_explicit_http_config() && options.explicit_http_config().has_http2_protocol_options()) { use_http2_ = true; diff --git a/source/extensions/upstreams/http/config.h b/source/extensions/upstreams/http/config.h index 6d84667b8caa..bd620bc15761 100644 --- a/source/extensions/upstreams/http/config.h +++ b/source/extensions/upstreams/http/config.h @@ -41,12 +41,14 @@ class ProtocolOptionsConfigImpl : public Upstream::ProtocolOptionsConfig { const Envoy::Http::Http1Settings http1_settings_; const envoy::config::core::v3::Http2ProtocolOptions http2_options_; + absl::optional http3_options_{}; const envoy::config::core::v3::HttpProtocolOptions common_http_protocol_options_; const absl::optional upstream_http_protocol_options_; bool use_downstream_protocol_{}; bool use_http2_{}; + bool use_http3_{}; bool use_alpn_{}; }; diff --git a/test/common/http/utility_test.cc b/test/common/http/utility_test.cc index 1e51af4b5f99..6cce982d00e9 100644 --- a/test/common/http/utility_test.cc +++ b/test/common/http/utility_test.cc @@ -22,6 +22,7 @@ using testing::_; using testing::Invoke; using testing::InvokeWithoutArgs; +using testing::Not; using testing::Return; namespace Envoy { @@ -420,6 +421,16 @@ TEST(HttpUtility, ValidateStreamErrorsWithHcm) { } } +TEST(HttpUtility, ValidateForHttp2AppliesToHttp3) { + envoy::config::core::v3::Http2ProtocolOptions http2_options; + envoy::config::core::v3::Http3ProtocolOptions http3_options; + Envoy::Http3::Utility::initializeAndValidateOptions(http3_options); + EXPECT_THAT(http3_options.http2_protocol_options(), Not(ProtoEq(http2_options))); + + Envoy::Http2::Utility::initializeAndValidateOptions(http2_options, http2_options); + EXPECT_THAT(http3_options.http2_protocol_options(), ProtoEq(http2_options)); +} + TEST(HttpUtility, ValidateStreamErrorConfigurationForHttp1) { envoy::config::core::v3::Http1ProtocolOptions http1_options; Protobuf::BoolValue hcm_value; diff --git a/test/common/upstream/upstream_impl_test.cc b/test/common/upstream/upstream_impl_test.cc index 7d031bd37506..e17968a6605b 100644 --- a/test/common/upstream/upstream_impl_test.cc +++ b/test/common/upstream/upstream_impl_test.cc @@ -3006,6 +3006,8 @@ TEST_F(ClusterInfoImplTest, UseDownstreamHttpProtocol) { cluster->info()->upstreamHttpProtocol({Http::Protocol::Http11})[0]); EXPECT_EQ(Http::Protocol::Http2, cluster->info()->upstreamHttpProtocol({Http::Protocol::Http2})[0]); + EXPECT_EQ(Http::Protocol::Http3, + cluster->info()->upstreamHttpProtocol({Http::Protocol::Http3})[0]); } TEST_F(ClusterInfoImplTest, UpstreamHttp2Protocol) { @@ -3047,6 +3049,54 @@ TEST_F(ClusterInfoImplTest, UpstreamHttp11Protocol) { cluster->info()->upstreamHttpProtocol({Http::Protocol::Http2})[0]); } +TEST_F(ClusterInfoImplTest, Http3) { + const std::string yaml = R"EOF( + name: name + connect_timeout: 0.25s + type: STRICT_DNS + lb_policy: MAGLEV + load_assignment: + endpoints: + - lb_endpoints: + - endpoint: + address: + socket_address: + address: foo.bar.com + port_value: 443 + )EOF"; + + BazFactory baz_factory; + Registry::InjectFactory registered_factory(baz_factory); + auto cluster1 = makeCluster(yaml); + ASSERT_TRUE(cluster1->info()->idleTimeout().has_value()); + EXPECT_EQ(std::chrono::hours(1), cluster1->info()->idleTimeout().value()); + + const std::string explicit_http3 = R"EOF( + typed_extension_protocol_options: + envoy.extensions.upstreams.http.v3.HttpProtocolOptions: + "@type": type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions + explicit_http_config: + http3_protocol_options: {} + )EOF"; + + const std::string downstream_http3 = R"EOF( + typed_extension_protocol_options: + envoy.extensions.upstreams.http.v3.HttpProtocolOptions: + "@type": type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions + common_http_protocol_options: + idle_timeout: 1s + )EOF"; + + { + EXPECT_THROW_WITH_REGEX(makeCluster(yaml + explicit_http3), EnvoyException, + "HTTP3 not yet supported: name.*"); + } + { + EXPECT_THROW_WITH_REGEX(makeCluster(yaml + explicit_http3), EnvoyException, + "HTTP3 not yet supported: name.*"); + } +} + // Validate empty singleton for HostsPerLocalityImpl. TEST(HostsPerLocalityImpl, Empty) { EXPECT_FALSE(HostsPerLocalityImpl::empty()->hasLocalLocality()); From 8f12773109cdf482cc6463f7f82822468a82eb67 Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Mon, 1 Feb 2021 17:00:28 -0500 Subject: [PATCH 2/2] removing knobs we don't need yet Signed-off-by: Alyssa Wilk --- api/envoy/config/core/v3/protocol.proto | 5 ++++- api/envoy/config/core/v4alpha/protocol.proto | 6 ++++-- .../envoy/config/core/v3/protocol.proto | 5 ++++- .../envoy/config/core/v4alpha/protocol.proto | 6 ++++-- source/common/http/utility.cc | 19 ++----------------- source/common/http/utility.h | 12 ------------ source/extensions/upstreams/http/config.cc | 1 - test/common/http/utility_test.cc | 11 ----------- 8 files changed, 18 insertions(+), 47 deletions(-) diff --git a/api/envoy/config/core/v3/protocol.proto b/api/envoy/config/core/v3/protocol.proto index 359d59e0f29b..7108fba33fc2 100644 --- a/api/envoy/config/core/v3/protocol.proto +++ b/api/envoy/config/core/v3/protocol.proto @@ -391,6 +391,9 @@ message GrpcProtocolOptions { } // [#not-implemented-hide:] +// +// A message which allows using HTTP/3 as an upstream protocol. +// +// Eventually this will include configuration for tuning HTTP/3. message Http3ProtocolOptions { - Http2ProtocolOptions http2_protocol_options = 1; } diff --git a/api/envoy/config/core/v4alpha/protocol.proto b/api/envoy/config/core/v4alpha/protocol.proto index 09bcd838dbb7..86bec0ec81a9 100644 --- a/api/envoy/config/core/v4alpha/protocol.proto +++ b/api/envoy/config/core/v4alpha/protocol.proto @@ -384,9 +384,11 @@ message GrpcProtocolOptions { } // [#not-implemented-hide:] +// +// A message which allows using HTTP/3 as an upstream protocol. +// +// Eventually this will include configuration for tuning HTTP/3. message Http3ProtocolOptions { option (udpa.annotations.versioning).previous_message_type = "envoy.config.core.v3.Http3ProtocolOptions"; - - Http2ProtocolOptions http2_protocol_options = 1; } diff --git a/generated_api_shadow/envoy/config/core/v3/protocol.proto b/generated_api_shadow/envoy/config/core/v3/protocol.proto index 359d59e0f29b..7108fba33fc2 100644 --- a/generated_api_shadow/envoy/config/core/v3/protocol.proto +++ b/generated_api_shadow/envoy/config/core/v3/protocol.proto @@ -391,6 +391,9 @@ message GrpcProtocolOptions { } // [#not-implemented-hide:] +// +// A message which allows using HTTP/3 as an upstream protocol. +// +// Eventually this will include configuration for tuning HTTP/3. message Http3ProtocolOptions { - Http2ProtocolOptions http2_protocol_options = 1; } diff --git a/generated_api_shadow/envoy/config/core/v4alpha/protocol.proto b/generated_api_shadow/envoy/config/core/v4alpha/protocol.proto index 269b59ad13ed..829415c69f16 100644 --- a/generated_api_shadow/envoy/config/core/v4alpha/protocol.proto +++ b/generated_api_shadow/envoy/config/core/v4alpha/protocol.proto @@ -394,9 +394,11 @@ message GrpcProtocolOptions { } // [#not-implemented-hide:] +// +// A message which allows using HTTP/3 as an upstream protocol. +// +// Eventually this will include configuration for tuning HTTP/3. message Http3ProtocolOptions { option (udpa.annotations.versioning).previous_message_type = "envoy.config.core.v3.Http3ProtocolOptions"; - - Http2ProtocolOptions http2_protocol_options = 1; } diff --git a/source/common/http/utility.cc b/source/common/http/utility.cc index e05d2eaa1473..86660479c683 100644 --- a/source/common/http/utility.cc +++ b/source/common/http/utility.cc @@ -162,13 +162,6 @@ initializeAndValidateOptions(const envoy::config::core::v3::Http2ProtocolOptions envoy::config::core::v3::Http2ProtocolOptions initializeAndValidateOptions(const envoy::config::core::v3::Http2ProtocolOptions& options) { envoy::config::core::v3::Http2ProtocolOptions options_clone(options); - initializeAndValidateOptions(options, options_clone); - return options_clone; -} - -void initializeAndValidateOptions(const envoy::config::core::v3::Http2ProtocolOptions& options, - envoy::config::core::v3::Http2ProtocolOptions& options_clone) { - // This will throw an exception when a custom parameter and a named parameter collide. validateCustomSettingsParameters(options); @@ -224,21 +217,13 @@ void initializeAndValidateOptions(const envoy::config::core::v3::Http2ProtocolOp options_clone.mutable_max_inbound_window_update_frames_per_data_frame_sent()->set_value( OptionsLimits::DEFAULT_MAX_INBOUND_WINDOW_UPDATE_FRAMES_PER_DATA_FRAME_SENT); } + + return options_clone; } } // namespace Utility } // namespace Http2 -namespace Http3 { -namespace Utility { - -void initializeAndValidateOptions(envoy::config::core::v3::Http3ProtocolOptions& options) { - Http2::Utility::initializeAndValidateOptions(options.http2_protocol_options(), - *options.mutable_http2_protocol_options()); -} - -} // namespace Utility -} // namespace Http3 namespace Http { static const char kDefaultPath[] = "/"; diff --git a/source/common/http/utility.h b/source/common/http/utility.h index 27f1e0cf1d77..d677b097d86c 100644 --- a/source/common/http/utility.h +++ b/source/common/http/utility.h @@ -115,9 +115,6 @@ struct OptionsLimits { envoy::config::core::v3::Http2ProtocolOptions initializeAndValidateOptions(const envoy::config::core::v3::Http2ProtocolOptions& options); -void initializeAndValidateOptions(const envoy::config::core::v3::Http2ProtocolOptions& from_options, - envoy::config::core::v3::Http2ProtocolOptions& to_options); - envoy::config::core::v3::Http2ProtocolOptions initializeAndValidateOptions(const envoy::config::core::v3::Http2ProtocolOptions& options, bool hcm_stream_error_set, @@ -125,15 +122,6 @@ initializeAndValidateOptions(const envoy::config::core::v3::Http2ProtocolOptions } // namespace Utility } // namespace Http2 -namespace Http3 { -namespace Utility { - -/* Does in-place initialization and validation of http3 options. */ -void initializeAndValidateOptions(envoy::config::core::v3::Http3ProtocolOptions& options); - -} // namespace Utility -} // namespace Http3 - namespace Http { namespace Utility { diff --git a/source/extensions/upstreams/http/config.cc b/source/extensions/upstreams/http/config.cc index aa894181ef93..eff6d2d4af20 100644 --- a/source/extensions/upstreams/http/config.cc +++ b/source/extensions/upstreams/http/config.cc @@ -91,7 +91,6 @@ ProtocolOptionsConfigImpl::ProtocolOptionsConfigImpl( : absl::nullopt) { if (http3_options_.has_value()) { use_http3_ = true; - Http3::Utility::initializeAndValidateOptions(http3_options_.value()); } if (options.has_explicit_http_config() && options.explicit_http_config().has_http2_protocol_options()) { diff --git a/test/common/http/utility_test.cc b/test/common/http/utility_test.cc index 6cce982d00e9..1e51af4b5f99 100644 --- a/test/common/http/utility_test.cc +++ b/test/common/http/utility_test.cc @@ -22,7 +22,6 @@ using testing::_; using testing::Invoke; using testing::InvokeWithoutArgs; -using testing::Not; using testing::Return; namespace Envoy { @@ -421,16 +420,6 @@ TEST(HttpUtility, ValidateStreamErrorsWithHcm) { } } -TEST(HttpUtility, ValidateForHttp2AppliesToHttp3) { - envoy::config::core::v3::Http2ProtocolOptions http2_options; - envoy::config::core::v3::Http3ProtocolOptions http3_options; - Envoy::Http3::Utility::initializeAndValidateOptions(http3_options); - EXPECT_THAT(http3_options.http2_protocol_options(), Not(ProtoEq(http2_options))); - - Envoy::Http2::Utility::initializeAndValidateOptions(http2_options, http2_options); - EXPECT_THAT(http3_options.http2_protocol_options(), ProtoEq(http2_options)); -} - TEST(HttpUtility, ValidateStreamErrorConfigurationForHttp1) { envoy::config::core::v3::Http1ProtocolOptions http1_options; Protobuf::BoolValue hcm_value;