From f4aca6ae28d33a58df979f8967dbd30233d6572b Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Mon, 11 Jan 2021 08:40:58 -0500 Subject: [PATCH 1/3] upstream: clean up feature parsing Signed-off-by: Alyssa Wilk --- source/common/upstream/upstream_impl.cc | 4 +-- source/extensions/upstreams/http/config.cc | 28 +++++-------------- source/extensions/upstreams/http/config.h | 7 ++--- .../upstream/cluster_manager_impl_test.cc | 4 +++ 4 files changed, 16 insertions(+), 27 deletions(-) diff --git a/source/common/upstream/upstream_impl.cc b/source/common/upstream/upstream_impl.cc index 18025dfca211..4f0d55355f97 100644 --- a/source/common/upstream/upstream_impl.cc +++ b/source/common/upstream/upstream_impl.cc @@ -727,8 +727,8 @@ ClusterInfoImpl::ClusterInfoImpl( ? std::make_unique( config, *stats_scope_, factory_context.clusterManager()) : nullptr), - features_(ClusterInfoImpl::HttpProtocolOptionsConfigImpl::parseFeatures( - config, http_protocol_options_)), + features_( + ClusterInfoImpl::HttpProtocolOptionsConfigImpl::parseFeatures(*http_protocol_options_)), resource_managers_(config, runtime, name_, *stats_scope_, factory_context.clusterManager().clusterCircuitBreakersStatNames()), maintenance_mode_runtime_key_(absl::StrCat("upstream.maintenance_mode.", name_)), diff --git a/source/extensions/upstreams/http/config.cc b/source/extensions/upstreams/http/config.cc index 6d8a8090c738..8ff73af2444d 100644 --- a/source/extensions/upstreams/http/config.cc +++ b/source/extensions/upstreams/http/config.cc @@ -43,33 +43,19 @@ getHttp2Options(const envoy::extensions::upstreams::http::v3::HttpProtocolOption } // namespace -uint64_t -ProtocolOptionsConfigImpl::parseFeatures(const envoy::config::cluster::v3::Cluster& config, - std::shared_ptr options) { +uint64_t ProtocolOptionsConfigImpl::parseFeatures(const ProtocolOptionsConfigImpl& options) { uint64_t features = 0; - if (options) { - if (options->use_http2_) { - features |= Upstream::ClusterInfo::Features::HTTP2; - } - if (options->use_downstream_protocol_) { - features |= Upstream::ClusterInfo::Features::USE_DOWNSTREAM_PROTOCOL; - } - } else { - if (config.has_http2_protocol_options()) { - features |= Upstream::ClusterInfo::Features::HTTP2; - } - if (config.protocol_selection() == - envoy::config::cluster::v3::Cluster::USE_DOWNSTREAM_PROTOCOL) { - features |= Upstream::ClusterInfo::Features::USE_DOWNSTREAM_PROTOCOL; - } + if (options.use_http2_) { + features |= Upstream::ClusterInfo::Features::HTTP2; } - if (config.close_connections_on_host_health_failure()) { - features |= Upstream::ClusterInfo::Features::CLOSE_CONNECTIONS_ON_HOST_HEALTH_FAILURE; + if (options.use_downstream_protocol_) { + features |= Upstream::ClusterInfo::Features::USE_DOWNSTREAM_PROTOCOL; } - if (options->use_alpn_) { + if (options.use_alpn_) { features |= Upstream::ClusterInfo::Features::USE_ALPN; } + return features; } diff --git a/source/extensions/upstreams/http/config.h b/source/extensions/upstreams/http/config.h index cb4a1cb0dca9..38116145c86b 100644 --- a/source/extensions/upstreams/http/config.h +++ b/source/extensions/upstreams/http/config.h @@ -34,10 +34,9 @@ class ProtocolOptionsConfigImpl : public Upstream::ProtocolOptionsConfig { const absl::optional upstream_options, bool use_downstream_protocol, bool use_http2); - // Given the supplied cluster config, and protocol options configuration, - // returns a unit64_t representing the enabled Upstream::ClusterInfo::Features. - static uint64_t parseFeatures(const envoy::config::cluster::v3::Cluster& config, - std::shared_ptr options); + // Given the supplied protocol options configuration, returns a unit64_t representing the enabled + // Upstream::ClusterInfo::Features. + static uint64_t parseFeatures(const ProtocolOptionsConfigImpl& options); const Envoy::Http::Http1Settings http1_settings_; const envoy::config::core::v3::Http2ProtocolOptions http2_options_; diff --git a/test/common/upstream/cluster_manager_impl_test.cc b/test/common/upstream/cluster_manager_impl_test.cc index df0cfd1aba77..1e6a9175c501 100644 --- a/test/common/upstream/cluster_manager_impl_test.cc +++ b/test/common/upstream/cluster_manager_impl_test.cc @@ -237,6 +237,10 @@ TEST_F(ClusterManagerImplTest, MultipleProtocolCluster) { protocol_selection: USE_DOWNSTREAM_PROTOCOL )EOF"; create(parseBootstrapFromV3Yaml(yaml)); + auto info = + cluster_manager_->clusters().active_clusters_.find("http12_cluster")->second.get().info(); + EXPECT_NE(0, info->features() & Upstream::ClusterInfo::Features::USE_DOWNSTREAM_PROTOCOL); + checkConfigDump(R"EOF( static_clusters: - cluster: From f1789ab58f9f208337ba090bf4b4008db752a7d0 Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Mon, 11 Jan 2021 12:50:06 -0500 Subject: [PATCH 2/3] less oventhusiastic Signed-off-by: Alyssa Wilk --- source/common/upstream/upstream_impl.cc | 4 ++-- source/extensions/upstreams/http/config.cc | 7 ++++++- source/extensions/upstreams/http/config.h | 7 ++++--- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/source/common/upstream/upstream_impl.cc b/source/common/upstream/upstream_impl.cc index 4f0d55355f97..c6f85e7721a6 100644 --- a/source/common/upstream/upstream_impl.cc +++ b/source/common/upstream/upstream_impl.cc @@ -727,8 +727,8 @@ ClusterInfoImpl::ClusterInfoImpl( ? std::make_unique( config, *stats_scope_, factory_context.clusterManager()) : nullptr), - features_( - ClusterInfoImpl::HttpProtocolOptionsConfigImpl::parseFeatures(*http_protocol_options_)), + features_(ClusterInfoImpl::HttpProtocolOptionsConfigImpl::parseFeatures( + config, *http_protocol_options_)), resource_managers_(config, runtime, name_, *stats_scope_, factory_context.clusterManager().clusterCircuitBreakersStatNames()), maintenance_mode_runtime_key_(absl::StrCat("upstream.maintenance_mode.", name_)), diff --git a/source/extensions/upstreams/http/config.cc b/source/extensions/upstreams/http/config.cc index 8ff73af2444d..99f7f058d86f 100644 --- a/source/extensions/upstreams/http/config.cc +++ b/source/extensions/upstreams/http/config.cc @@ -43,7 +43,9 @@ getHttp2Options(const envoy::extensions::upstreams::http::v3::HttpProtocolOption } // namespace -uint64_t ProtocolOptionsConfigImpl::parseFeatures(const ProtocolOptionsConfigImpl& options) { +uint64_t +ProtocolOptionsConfigImpl::parseFeatures(const envoy::config::cluster::v3::Cluster& config, + const ProtocolOptionsConfigImpl& options) { uint64_t features = 0; if (options.use_http2_) { @@ -56,6 +58,9 @@ uint64_t ProtocolOptionsConfigImpl::parseFeatures(const ProtocolOptionsConfigImp features |= Upstream::ClusterInfo::Features::USE_ALPN; } + if (config.close_connections_on_host_health_failure()) { + features |= Upstream::ClusterInfo::Features::CLOSE_CONNECTIONS_ON_HOST_HEALTH_FAILURE; + } return features; } diff --git a/source/extensions/upstreams/http/config.h b/source/extensions/upstreams/http/config.h index 38116145c86b..6d84667b8caa 100644 --- a/source/extensions/upstreams/http/config.h +++ b/source/extensions/upstreams/http/config.h @@ -34,9 +34,10 @@ class ProtocolOptionsConfigImpl : public Upstream::ProtocolOptionsConfig { const absl::optional upstream_options, bool use_downstream_protocol, bool use_http2); - // Given the supplied protocol options configuration, returns a unit64_t representing the enabled - // Upstream::ClusterInfo::Features. - static uint64_t parseFeatures(const ProtocolOptionsConfigImpl& options); + // Given the supplied cluster config, and protocol options configuration, + // returns a unit64_t representing the enabled Upstream::ClusterInfo::Features. + static uint64_t parseFeatures(const envoy::config::cluster::v3::Cluster& config, + const ProtocolOptionsConfigImpl& options); const Envoy::Http::Http1Settings http1_settings_; const envoy::config::core::v3::Http2ProtocolOptions http2_options_; From 1baa373143678763464c20a567ec0b218072ca9c Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Mon, 11 Jan 2021 13:28:22 -0500 Subject: [PATCH 3/3] format Signed-off-by: Alyssa Wilk --- source/extensions/upstreams/http/config.cc | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/source/extensions/upstreams/http/config.cc b/source/extensions/upstreams/http/config.cc index 99f7f058d86f..2d8cbd447c4f 100644 --- a/source/extensions/upstreams/http/config.cc +++ b/source/extensions/upstreams/http/config.cc @@ -43,9 +43,8 @@ getHttp2Options(const envoy::extensions::upstreams::http::v3::HttpProtocolOption } // namespace -uint64_t -ProtocolOptionsConfigImpl::parseFeatures(const envoy::config::cluster::v3::Cluster& config, - const ProtocolOptionsConfigImpl& options) { +uint64_t ProtocolOptionsConfigImpl::parseFeatures(const envoy::config::cluster::v3::Cluster& config, + const ProtocolOptionsConfigImpl& options) { uint64_t features = 0; if (options.use_http2_) {