use different alpn override for different upstream protocol (istio#2479)

* use different alpn override for different upstream protocol * update dependent * address comment
durd07 · Oct 22, 2019 · 6051c04 · 6051c04
1 parent ab5b376
commit 6051c04
Show file tree

Hide file tree

Showing 8 changed files with 210 additions and 48 deletions.
diff --git a/repositories.bzl b/repositories.bzl
@@ -105,8 +105,8 @@ cc_library(
 # 1) find the ISTIO_API SHA you want in git
 # 2) wget https://github.com/istio/api/archive/$ISTIO_API_SHA.tar.gz && sha256sum $ISTIO_API_SHA.tar.gz
 #
-ISTIO_API = "593785242b9d8afcdcec176c5f03f3637dbf1ad1"
-ISTIO_API_SHA256 = "2c0f8059464b228476bd772e7d514e373d303b821d6187e55c24956babb11bf2"
+ISTIO_API = "31d048906d97fb7f6b1fa8e250d3fa07456c5acc"
+ISTIO_API_SHA256 = "5bf68ef13f4b9e769b7ca0a9ce83d9da5263eed9b1223c4cbb388a6ad5520e01"
 GOGOPROTO_RELEASE = "1.2.1"
 GOGOPROTO_SHA256 = "99e423905ba8921e86817607a5294ffeedb66fdd4a85efce5eb2848f715fdb3a"
 
@@ -164,6 +164,7 @@ proto_library(
     deps = [
         ":mixer_api_protos_lib",
         "@com_github_gogo_protobuf//:gogo_proto",
+        "@com_google_googleapis//google/api:field_behavior_proto",
         "@com_google_protobuf//:duration_proto",
     ],
 )
@@ -186,6 +187,7 @@ proto_library(
     ),
     visibility = ["//visibility:public"],
     deps = [
+        "@com_google_googleapis//google/api:field_behavior_proto",
         "@com_github_gogo_protobuf//:gogo_proto",
     ],
 )

diff --git a/src/envoy/http/alpn/BUILD b/src/envoy/http/alpn/BUILD
@@ -65,6 +65,7 @@ envoy_cc_test(
         "@envoy//test/mocks/local_info:local_info_mocks",
         "@envoy//test/mocks/network:network_mocks",
         "@envoy//test/mocks/protobuf:protobuf_mocks",
+        "@envoy//test/mocks/upstream:upstream_mocks",
     ],
 )
 

diff --git a/src/envoy/http/alpn/alpn_filter.cc b/src/envoy/http/alpn/alpn_filter.cc
@@ -17,18 +17,66 @@
 
 #include "common/network/application_protocol.h"
 
+#include "envoy/upstream/cluster_manager.h"
+
 namespace Envoy {
 namespace Http {
 namespace Alpn {
 
 AlpnFilterConfig::AlpnFilterConfig(
     const istio::envoy::config::filter::http::alpn::v2alpha1::FilterConfig
-        &proto_config)
-    : alpn_override_(proto_config.alpn_override().begin(),
-                     proto_config.alpn_override().end()) {}
+        &proto_config,
+    Upstream::ClusterManager &cluster_manager)
+    : cluster_manager_(cluster_manager) {
+  for (const auto &pair : proto_config.alpn_override()) {
+    std::vector<std::string> application_protocols;
+    for (const auto &protocol : pair.alpn_override()) {
+      application_protocols.push_back(protocol);
+    }
+
+    alpn_overrides_.insert({getHttpProtocol(pair.upstream_protocol()),
+                            std::move(application_protocols)});
+  }
+}
+
+Http::Protocol AlpnFilterConfig::getHttpProtocol(
+    const istio::envoy::config::filter::http::alpn::v2alpha1::FilterConfig::
+        Protocol &protocol) {
+  switch (protocol) {
+    case istio::envoy::config::filter::http::alpn::v2alpha1::FilterConfig::
+        Protocol::FilterConfig_Protocol_HTTP10:
+      return Http::Protocol::Http10;
+    case istio::envoy::config::filter::http::alpn::v2alpha1::FilterConfig::
+        Protocol::FilterConfig_Protocol_HTTP11:
+      return Http::Protocol::Http11;
+    case istio::envoy::config::filter::http::alpn::v2alpha1::FilterConfig::
+        Protocol::FilterConfig_Protocol_HTTP2:
+      return Http::Protocol::Http2;
+    default:
+      // will not reach here.
+      NOT_IMPLEMENTED_GCOVR_EXCL_LINE;
+  }
+}
 
 Http::FilterHeadersStatus AlpnFilter::decodeHeaders(Http::HeaderMap &, bool) {
-  const auto &alpn_override = config_->getAlpnOverride();
+  Router::RouteConstSharedPtr route = decoder_callbacks_->route();
+  const Router::RouteEntry *route_entry;
+  if (!route || !(route_entry = route->routeEntry())) {
+    ENVOY_LOG(debug, "cannot find route entry");
+    return Http::FilterHeadersStatus::Continue;
+  }
+
+  Upstream::ThreadLocalCluster *cluster =
+      config_->clusterManager().get(route_entry->clusterName());
+  if (!cluster || !cluster->info()) {
+    ENVOY_LOG(debug, "cannot find cluster {}", route_entry->clusterName());
+    return Http::FilterHeadersStatus::Continue;
+  }
+
+  Http::Protocol protocol = cluster->info()->upstreamHttpProtocol(
+      decoder_callbacks_->streamInfo().protocol());
+  const auto &alpn_override = config_->alpnOverrides(protocol);
+
   if (!alpn_override.empty()) {
     ENVOY_LOG(debug, "override with {} ALPNs", alpn_override.size());
     decoder_callbacks_->streamInfo().filterState().setData(

diff --git a/src/envoy/http/alpn/alpn_filter.h b/src/envoy/http/alpn/alpn_filter.h
@@ -22,19 +22,33 @@ namespace Envoy {
 namespace Http {
 namespace Alpn {
 
+using AlpnOverrides =
+    absl::flat_hash_map<Http::Protocol, std::vector<std::string>>;
+
 class AlpnFilterConfig {
  public:
-  AlpnFilterConfig() = default;
-  explicit AlpnFilterConfig(
+  AlpnFilterConfig(
       const istio::envoy::config::filter::http::alpn::v2alpha1::FilterConfig
-          &proto_config);
+          &proto_config,
+      Upstream::ClusterManager &cluster_manager);
+
+  Upstream::ClusterManager &clusterManager() { return cluster_manager_; }
 
-  const std::vector<std::string> &getAlpnOverride() const {
-    return alpn_override_;
+  const std::vector<std::string> alpnOverrides(
+      const Http::Protocol &protocol) const {
+    if (alpn_overrides_.count(protocol)) {
+      return alpn_overrides_.at(protocol);
+    }
+    return {};
   }
 
  private:
-  const std::vector<std::string> alpn_override_;
+  Http::Protocol getHttpProtocol(
+      const istio::envoy::config::filter::http::alpn::v2alpha1::FilterConfig::
+          Protocol &protocol);
+
+  AlpnOverrides alpn_overrides_;
+  Upstream::ClusterManager &cluster_manager_;
 };
 
 using AlpnFilterConfigSharedPtr = std::shared_ptr<AlpnFilterConfig>;

diff --git a/src/envoy/http/alpn/alpn_test.cc b/src/envoy/http/alpn/alpn_test.cc
@@ -18,8 +18,11 @@
 #include "gtest/gtest.h"
 #include "src/envoy/http/alpn/alpn_filter.h"
 #include "test/mocks/http/mocks.h"
+#include "test/mocks/upstream/mocks.h"
 
 using istio::envoy::config::filter::http::alpn::v2alpha1::FilterConfig;
+using istio::envoy::config::filter::http::alpn::v2alpha1::
+    FilterConfig_AlpnOverride;
 using testing::NiceMock;
 using testing::Return;
 using testing::ReturnRef;
@@ -31,55 +34,140 @@ namespace {
 
 class AlpnFilterTest : public testing::Test {
  public:
-  std::unique_ptr<AlpnFilter> makeDefaultFilter() {
-    auto default_config = std::make_shared<AlpnFilterConfig>();
-    auto filter = std::make_unique<AlpnFilter>(default_config);
-    filter->setDecoderFilterCallbacks(callbacks_);
-    return filter;
-  }
-
   std::unique_ptr<AlpnFilter> makeAlpnOverrideFilter(
-      const std::vector<std::string> &alpn) {
+      const AlpnOverrides &alpn) {
     FilterConfig proto_config;
-    for (const auto &protocol : alpn) {
-      proto_config.add_alpn_override(protocol);
+
+    for (const auto &p : alpn) {
+      FilterConfig_AlpnOverride entry;
+      entry.set_upstream_protocol(getProtocol(p.first));
+      for (const auto &v : p.second) {
+        entry.add_alpn_override(v);
+      }
+      proto_config.mutable_alpn_override()->Add(std::move(entry));
     }
-    auto config = std::make_shared<AlpnFilterConfig>(proto_config);
+
+    auto config =
+        std::make_shared<AlpnFilterConfig>(proto_config, cluster_manager_);
     auto filter = std::make_unique<AlpnFilter>(config);
     filter->setDecoderFilterCallbacks(callbacks_);
     return filter;
   }
 
  protected:
+  FilterConfig::Protocol getProtocol(Http::Protocol protocol) {
+    switch (protocol) {
+      case Http::Protocol::Http10:
+        return FilterConfig::Protocol::FilterConfig_Protocol_HTTP10;
+      case Http::Protocol::Http11:
+        return FilterConfig::Protocol::FilterConfig_Protocol_HTTP11;
+      case Http::Protocol::Http2:
+        return FilterConfig::Protocol::FilterConfig_Protocol_HTTP2;
+      default:
+        NOT_IMPLEMENTED_GCOVR_EXCL_LINE;
+    }
+  }
+
   NiceMock<Http::MockStreamDecoderFilterCallbacks> callbacks_;
+  NiceMock<Upstream::MockClusterManager> cluster_manager_;
+  std::shared_ptr<Upstream::MockThreadLocalCluster> fake_cluster_{
+      std::make_shared<NiceMock<Upstream::MockThreadLocalCluster>>()};
+  std::shared_ptr<Upstream::MockClusterInfo> cluster_info_{
+      std::make_shared<NiceMock<Upstream::MockClusterInfo>>()};
   Http::TestHeaderMapImpl headers_;
 };
 
-TEST_F(AlpnFilterTest, NoAlpnOverride) {
+TEST_F(AlpnFilterTest, OverrideAlpnUseDownstreamProtocol) {
   NiceMock<StreamInfo::MockStreamInfo> stream_info;
   ON_CALL(callbacks_, streamInfo()).WillByDefault(ReturnRef(stream_info));
-  auto filter = makeDefaultFilter();
-  EXPECT_CALL(stream_info, filterState()).Times(0);
-  EXPECT_EQ(filter->decodeHeaders(headers_, false),
-            Http::FilterHeadersStatus::Continue);
+  const AlpnOverrides alpn = {{Http::Protocol::Http10, {"foo", "bar"}},
+                              {Http::Protocol::Http11, {"baz"}},
+                              {Http::Protocol::Http2, {"qux"}}};
+  auto filter = makeAlpnOverrideFilter(alpn);
+
+  ON_CALL(cluster_manager_, get(_)).WillByDefault(Return(fake_cluster_.get()));
+  ON_CALL(*fake_cluster_, info()).WillByDefault(Return(cluster_info_));
+  ON_CALL(*cluster_info_, upstreamHttpProtocol(_))
+      .WillByDefault([](absl::optional<Http::Protocol> protocol) {
+        return protocol.value();
+      });
+
+  auto protocols = {Http::Protocol::Http10, Http::Protocol::Http11,
+                    Http::Protocol::Http2};
+  for (const auto p : protocols) {
+    EXPECT_CALL(stream_info, protocol()).WillOnce(Return(p));
+    Envoy::StreamInfo::FilterStateImpl filter_state;
+    EXPECT_CALL(stream_info, filterState()).WillOnce(ReturnRef(filter_state));
+    EXPECT_EQ(filter->decodeHeaders(headers_, false),
+              Http::FilterHeadersStatus::Continue);
+    EXPECT_TRUE(filter_state.hasData<Network::ApplicationProtocols>(
+        Network::ApplicationProtocols::key()));
+    auto alpn_override = filter_state
+                             .getDataReadOnly<Network::ApplicationProtocols>(
+                                 Network::ApplicationProtocols::key())
+                             .value();
+
+    EXPECT_EQ(alpn_override, alpn.at(p));
+  }
 }
 
 TEST_F(AlpnFilterTest, OverrideAlpn) {
   NiceMock<StreamInfo::MockStreamInfo> stream_info;
   ON_CALL(callbacks_, streamInfo()).WillByDefault(ReturnRef(stream_info));
-  std::vector<std::string> alpn{"foo", "bar", "baz"};
+  const AlpnOverrides alpn = {{Http::Protocol::Http10, {"foo", "bar"}},
+                              {Http::Protocol::Http11, {"baz"}},
+                              {Http::Protocol::Http2, {"qux"}}};
   auto filter = makeAlpnOverrideFilter(alpn);
-  Envoy::StreamInfo::FilterStateImpl filter_state;
-  EXPECT_CALL(stream_info, filterState()).WillOnce(ReturnRef(filter_state));
-  EXPECT_EQ(filter->decodeHeaders(headers_, false),
-            Http::FilterHeadersStatus::Continue);
-  EXPECT_TRUE(filter_state.hasData<Network::ApplicationProtocols>(
-      Network::ApplicationProtocols::key()));
-  auto alpn_override = filter_state
-                           .getDataReadOnly<Network::ApplicationProtocols>(
-                               Network::ApplicationProtocols::key())
-                           .value();
-  EXPECT_EQ(alpn_override, alpn);
+
+  ON_CALL(cluster_manager_, get(_)).WillByDefault(Return(fake_cluster_.get()));
+  ON_CALL(*fake_cluster_, info()).WillByDefault(Return(cluster_info_));
+  ON_CALL(*cluster_info_, upstreamHttpProtocol(_))
+      .WillByDefault(
+          [](absl::optional<Http::Protocol>) { return Http::Protocol::Http2; });
+
+  auto protocols = {Http::Protocol::Http10, Http::Protocol::Http11,
+                    Http::Protocol::Http2};
+  for (const auto p : protocols) {
+    EXPECT_CALL(stream_info, protocol()).WillOnce(Return(p));
+    Envoy::StreamInfo::FilterStateImpl filter_state;
+    EXPECT_CALL(stream_info, filterState()).WillOnce(ReturnRef(filter_state));
+    EXPECT_EQ(filter->decodeHeaders(headers_, false),
+              Http::FilterHeadersStatus::Continue);
+    EXPECT_TRUE(filter_state.hasData<Network::ApplicationProtocols>(
+        Network::ApplicationProtocols::key()));
+    auto alpn_override = filter_state
+                             .getDataReadOnly<Network::ApplicationProtocols>(
+                                 Network::ApplicationProtocols::key())
+                             .value();
+
+    EXPECT_EQ(alpn_override, alpn.at(Http::Protocol::Http2));
+  }
+}
+
+TEST_F(AlpnFilterTest, EmptyOverrideAlpn) {
+  NiceMock<StreamInfo::MockStreamInfo> stream_info;
+  ON_CALL(callbacks_, streamInfo()).WillByDefault(ReturnRef(stream_info));
+  const AlpnOverrides alpn = {{Http::Protocol::Http10, {"foo", "bar"}},
+                              {Http::Protocol::Http11, {"baz"}}};
+  auto filter = makeAlpnOverrideFilter(alpn);
+
+  ON_CALL(cluster_manager_, get(_)).WillByDefault(Return(fake_cluster_.get()));
+  ON_CALL(*fake_cluster_, info()).WillByDefault(Return(cluster_info_));
+  ON_CALL(*cluster_info_, upstreamHttpProtocol(_))
+      .WillByDefault(
+          [](absl::optional<Http::Protocol>) { return Http::Protocol::Http2; });
+
+  auto protocols = {Http::Protocol::Http10, Http::Protocol::Http11,
+                    Http::Protocol::Http2};
+  for (const auto p : protocols) {
+    EXPECT_CALL(stream_info, protocol()).WillOnce(Return(p));
+    Envoy::StreamInfo::FilterStateImpl filter_state;
+    EXPECT_CALL(stream_info, filterState()).Times(0);
+    EXPECT_EQ(filter->decodeHeaders(headers_, false),
+              Http::FilterHeadersStatus::Continue);
+    EXPECT_FALSE(filter_state.hasData<Network::ApplicationProtocols>(
+        Network::ApplicationProtocols::key()));
+  }
 }
 
 }  // namespace

diff --git a/src/envoy/http/alpn/config.cc b/src/envoy/http/alpn/config.cc
@@ -27,17 +27,18 @@ namespace Alpn {
 
 Http::FilterFactoryCb AlpnConfigFactory::createFilterFactory(
     const Json::Object &config, const std::string &,
-    Server::Configuration::FactoryContext &) {
+    Server::Configuration::FactoryContext &context) {
   FilterConfig filter_config;
   MessageUtil::loadFromJson(config.asJsonString(), filter_config,
                             ProtobufMessage::getNullValidationVisitor());
-  return createFilterFactory(filter_config);
+  return createFilterFactory(filter_config, context.clusterManager());
 }
 
 Http::FilterFactoryCb AlpnConfigFactory::createFilterFactoryFromProto(
     const Protobuf::Message &config, const std::string &,
-    Server::Configuration::FactoryContext &) {
-  return createFilterFactory(dynamic_cast<const FilterConfig &>(config));
+    Server::Configuration::FactoryContext &context) {
+  return createFilterFactory(dynamic_cast<const FilterConfig &>(config),
+                             context.clusterManager());
 }
 
 ProtobufTypes::MessagePtr AlpnConfigFactory::createEmptyConfigProto() {
@@ -47,9 +48,10 @@ ProtobufTypes::MessagePtr AlpnConfigFactory::createEmptyConfigProto() {
 std::string AlpnConfigFactory::name() { return Utils::IstioFilterName::kAlpn; }
 
 Http::FilterFactoryCb AlpnConfigFactory::createFilterFactory(
-    const FilterConfig &proto_config) {
+    const FilterConfig &proto_config,
+    Upstream::ClusterManager &cluster_manager) {
   AlpnFilterConfigSharedPtr filter_config{
-      std::make_shared<AlpnFilterConfig>(proto_config)};
+      std::make_shared<AlpnFilterConfig>(proto_config, cluster_manager)};
   return [filter_config](Http::FilterChainFactoryCallbacks &callbacks) -> void {
     callbacks.addStreamDecoderFilter(
         std::make_unique<AlpnFilter>(filter_config));

diff --git a/src/envoy/http/alpn/config.h b/src/envoy/http/alpn/config.h
@@ -41,7 +41,8 @@ class AlpnConfigFactory
  private:
   Http::FilterFactoryCb createFilterFactory(
       const istio::envoy::config::filter::http::alpn::v2alpha1::FilterConfig
-          &config_pb);
+          &config_pb,
+      Upstream::ClusterManager &cluster_manager);
 };
 
 }  // namespace Alpn

diff --git a/src/envoy/http/alpn/config_test.cc b/src/envoy/http/alpn/config_test.cc
@@ -37,7 +37,13 @@ namespace {
 
 TEST(AlpnFilterConfigTest, OverrideAlpn) {
   const std::string yaml = R"EOF(
-    alpn_override: ["foo", "bar"]
+    alpn_override:
+    - upstream_protocol: HTTP10
+      alpn_override: ["foo", "bar"]    
+    - upstream_protocol: HTTP11
+      alpn_override: ["baz"]
+    - upstream_protocol: HTTP2
+      alpn_override: ["qux"]
     )EOF";
 
   FilterConfig proto_config;