[hcm] Add scoped RDS routing into HCM (#7762)

Description: add Scoped RDS routing logic into HCM. Changes include: * in ActiveStream constructor latch a ScopedConfig impl to the activeStream if SRDS is enabled * in the beginning of ActiveStream::decodeHeaders(headers, end_stream), get routeConfig from latched ScopedConfig impl. This PR is the 3rd in the srds impl PR chain: [#7704, #7451, this]. Risk Level: Medium Testing: unit test and integration tests. Release Notes: Add scoped RDS routing support into HCM. Signed-off-by: Xin Zhuang <[email protected]>
envoyproxy · Aug 30, 2019 · 7960564 · 7960564
1 parent 0b0aa3f
commit 7960564
Show file tree

Hide file tree

Showing 17 changed files with 719 additions and 140 deletions.
diff --git a/api/envoy/api/v2/srds.proto b/api/envoy/api/v2/srds.proto
@@ -2,36 +2,27 @@ syntax = "proto3";
 
 package envoy.api.v2;
 
-option java_outer_classname = "SrdsProto";
-option java_package = "io.envoyproxy.envoy.api.v2";
-option java_multiple_files = true;
-option java_generic_services = true;
-
 import "envoy/api/v2/discovery.proto";
-
+import "gogoproto/gogo.proto";
 import "google/api/annotations.proto";
-
 import "validate/validate.proto";
-import "gogoproto/gogo.proto";
 
+option java_outer_classname = "SrdsProto";
+option java_package = "io.envoyproxy.envoy.api.v2";
+option java_multiple_files = true;
+option java_generic_services = true;
 option (gogoproto.equal_all) = true;
 
 // [#protodoc-title: HTTP scoped routing configuration]
 // * Routing :ref:`architecture overview <arch_overview_http_routing>`
 //
-// .. attention::
-//
-//   The Scoped RDS API is not yet fully implemented and *should not* be enabled in
-//   :ref:`envoy_api_msg_config.filter.network.http_connection_manager.v2.HttpConnectionManager`.
-//
-// TODO(AndresGuedez): Update :ref:`arch_overview_http_routing` with scoped routing overview and
-// configuration details.
-
 // The Scoped Routes Discovery Service (SRDS) API distributes
-// :ref:`ScopedRouteConfiguration<envoy_api_msg.ScopedRouteConfiguration>` resources. Each
-// ScopedRouteConfiguration resource represents a "routing scope" containing a mapping that allows
-// the HTTP connection manager to dynamically assign a routing table (specified via
-// a :ref:`RouteConfiguration<envoy_api_msg_RouteConfiguration>` message) to each HTTP request.
+// :ref:`ScopedRouteConfiguration<envoy_api_msg.ScopedRouteConfiguration>`
+// resources. Each ScopedRouteConfiguration resource represents a "routing
+// scope" containing a mapping that allows the HTTP connection manager to
+// dynamically assign a routing table (specified via a
+// :ref:`RouteConfiguration<envoy_api_msg_RouteConfiguration>` message) to each
+// HTTP request.
 // [#proto-status: experimental]
 service ScopedRoutesDiscoveryService {
   rpc StreamScopedRoutes(stream DiscoveryRequest) returns (stream DiscoveryResponse) {
@@ -52,9 +43,9 @@ service ScopedRoutesDiscoveryService {
 // :ref:`Key<envoy_api_msg_ScopedRouteConfiguration.Key>` to a
 // :ref:`envoy_api_msg_RouteConfiguration` (identified by its resource name).
 //
-// The HTTP connection manager builds up a table consisting of these Key to RouteConfiguration
-// mappings, and looks up the RouteConfiguration to use per request according to the algorithm
-// specified in the
+// The HTTP connection manager builds up a table consisting of these Key to
+// RouteConfiguration mappings, and looks up the RouteConfiguration to use per
+// request according to the algorithm specified in the
 // :ref:`scope_key_builder<envoy_api_field_config.filter.network.http_connection_manager.v2.ScopedRoutes.scope_key_builder>`
 // assigned to the HttpConnectionManager.
 //
@@ -104,8 +95,8 @@ service ScopedRoutesDiscoveryService {
 //     Host: foo.com
 //     X-Route-Selector: vip=172.10.10.20
 //
-// would result in the routing table defined by the `route-config1` RouteConfiguration being
-// assigned to the HTTP request/stream.
+// would result in the routing table defined by the `route-config1`
+// RouteConfiguration being assigned to the HTTP request/stream.
 //
 // [#comment:next free field: 4]
 // [#proto-status: experimental]
@@ -115,8 +106,9 @@ message ScopedRouteConfiguration {
 
   // Specifies a key which is matched against the output of the
   // :ref:`scope_key_builder<envoy_api_field_config.filter.network.http_connection_manager.v2.ScopedRoutes.scope_key_builder>`
-  // specified in the HttpConnectionManager. The matching is done per HTTP request and is dependent
-  // on the order of the fragments contained in the Key.
+  // specified in the HttpConnectionManager. The matching is done per HTTP
+  // request and is dependent on the order of the fragments contained in the
+  // Key.
   message Key {
     message Fragment {
       oneof type {
@@ -127,14 +119,15 @@ message ScopedRouteConfiguration {
       }
     }
 
-    // The ordered set of fragments to match against. The order must match the fragments in the
-    // corresponding
+    // The ordered set of fragments to match against. The order must match the
+    // fragments in the corresponding
     // :ref:`scope_key_builder<envoy_api_field_config.filter.network.http_connection_manager.v2.ScopedRoutes.scope_key_builder>`.
     repeated Fragment fragments = 1 [(validate.rules).repeated .min_items = 1];
   }
 
-  // The resource name to use for a :ref:`envoy_api_msg_DiscoveryRequest` to an RDS server to
-  // fetch the :ref:`envoy_api_msg_RouteConfiguration` associated with this scope.
+  // The resource name to use for a :ref:`envoy_api_msg_DiscoveryRequest` to an
+  // RDS server to fetch the :ref:`envoy_api_msg_RouteConfiguration` associated
+  // with this scope.
   string route_configuration_name = 2 [(validate.rules).string.min_bytes = 1];
 
   // The key to match against.

diff --git a/docs/root/intro/arch_overview/http/http_routing.rst b/docs/root/intro/arch_overview/http/http_routing.rst
@@ -50,6 +50,35 @@ request. The router filter supports the following features:
 * :ref:`Hash policy <envoy_api_field_route.RouteAction.hash_policy>` based routing.
 * :ref:`Absolute urls <envoy_api_field_config.filter.network.http_connection_manager.v2.HttpConnectionManager.http_protocol_options>` are supported for non-tls forward proxies.
 
+.. _arch_overview_http_routing_route_scope:
+
+Route Scope
+--------------
+
+Scoped routing enables Envoy to put constraints on search space of domains and route rules.
+A :ref:`Route Scope<envoy_api_msg_ScopedRouteConfiguration>` associates a key with a :ref:`route table <arch_overview_http_routing_route_table>`.
+For each request, a scope key is computed dynamically by the HTTP connection manager to pick the :ref:`route table<envoy_api_msg_RouteConfiguration>`.
+
+The Scoped RDS (SRDS) API contains a set of :ref:`Scopes <envoy_api_msg_ScopedRouteConfiguration>` resources, each defining independent routing configuration,
+along with a :ref:`ScopeKeyBuilder <envoy_api_msg_config.filter.network.http_connection_manager.v2.ScopedRoutes.ScopeKeyBuilder>`
+defining the key construction algorithm used by Envoy to look up the scope corresponding to each request.
+
+For example, for the following scoped route configuration, Envoy will look into the "addr" header value, split the header value by ";" first, and use the first value for key 'x-foo-key' as the scope key.
+If the "addr" header value is "foo=1;x-foo-key=127.0.0.1;x-bar-key=1.1.1.1", then "127.0.0.1" will be computed as the scope key to look up for corresponding route configuration.
+
+.. code-block:: yaml
+
+  name: scope_by_addr
+  fragments:
+    - header_value_extractor:
+        name: Addr
+        element_separator: ;
+        element:
+          key: x-foo-key
+          separator: =
+
+.. _arch_overview_http_routing_route_table:
+
 Route table
 -----------
 

diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst
@@ -43,16 +43,17 @@ Version history
   <deprecated>` for more information.
 * rbac: added conditions to the policy, see :ref:`condition <envoy_api_field_config.rbac.v2.Policy.condition>`.
 * router: added :ref:`rq_retry_skipped_request_not_complete <config_http_filters_router_stats>` counter stat to router stats.
+* router: :ref:`Scoped routing <arch_overview_http_routing_route_scope>` is supported.
 * router check tool: add coverage reporting & enforcement.
 * router check tool: add comprehensive coverage reporting.
-* tracing: added support to the Zipkin reporter for sending list of spans as Zipkin JSON v2 and protobuf message over HTTP.
 * router check tool: add deprecated field check.
 * tls: added verification of IP address SAN fields in certificates against configured SANs in the
+* tracing: added support to the Zipkin reporter for sending list of spans as Zipkin JSON v2 and protobuf message over HTTP.
   certificate validation context.
 * tracing: added tags for gRPC response status and meesage.
+* upstream: added :ref:`an option <envoy_api_field_Cluster.CommonLbConfig.close_connections_on_host_set_change>` that allows draining HTTP, TCP connection pools on cluster membership change.
 * upstream: added network filter chains to upstream connections, see :ref:`filters<envoy_api_field_Cluster.filters>`.
 * upstream: use p2c to select hosts for least-requests load balancers if all host weights are the same, even in cases where weights are not equal to 1.
-* upstream: added :ref:`an option <envoy_api_field_Cluster.CommonLbConfig.close_connections_on_host_set_change>` that allows draining HTTP, TCP connection pools on cluster membership change.
 * zookeeper: parse responses and emit latency stats.
 
 1.11.1 (August 13, 2019)

diff --git a/include/envoy/stream_info/stream_info.h b/include/envoy/stream_info/stream_info.h
@@ -109,6 +109,8 @@ struct ResponseCodeDetailValues {
   // The request was rejected because it attempted an unsupported upgrade.
   const std::string UpgradeFailed = "upgrade_failed";
 
+  // The request was rejected by the HCM because there was no route configuration found.
+  const std::string RouteConfigurationNotFound = "route_configuration_not_found";
   // The request was rejected by the router filter because there was no route found.
   const std::string RouteNotFound = "route_not_found";
   // A direct response was generated by the router filter.

diff --git a/source/common/http/BUILD b/source/common/http/BUILD
@@ -164,6 +164,7 @@ envoy_cc_library(
         "//include/envoy/router:rds_interface",
         "//include/envoy/router:scopes_interface",
         "//include/envoy/runtime:runtime_interface",
+        "//include/envoy/server:admin_interface",
         "//include/envoy/server:overload_manager_interface",
         "//include/envoy/ssl:connection_interface",
         "//include/envoy/stats:stats_interface",

diff --git a/source/common/http/conn_manager_impl.cc b/source/common/http/conn_manager_impl.cc
@@ -12,6 +12,7 @@
 #include "envoy/event/dispatcher.h"
 #include "envoy/network/drain_decision.h"
 #include "envoy/router/router.h"
+#include "envoy/server/admin.h"
 #include "envoy/ssl/connection.h"
 #include "envoy/stats/scope.h"
 #include "envoy/tracing/http_tracer.h"
@@ -431,12 +432,27 @@ void ConnectionManagerImpl::chargeTracingStats(const Tracing::Reason& tracing_re
 
 ConnectionManagerImpl::ActiveStream::ActiveStream(ConnectionManagerImpl& connection_manager)
     : connection_manager_(connection_manager),
-      snapped_route_config_(connection_manager.config_.routeConfigProvider()->config()),
       stream_id_(connection_manager.random_generator_.random()),
       request_response_timespan_(new Stats::Timespan(
           connection_manager_.stats_.named_.downstream_rq_time_, connection_manager_.timeSource())),
       stream_info_(connection_manager_.codec_->protocol(), connection_manager_.timeSource()),
       upstream_options_(std::make_shared<Network::Socket::Options>()) {
+  // For Server::Admin, no routeConfigProvider or SRDS route provider is used.
+  ASSERT(dynamic_cast<Server::Admin*>(&connection_manager_.config_) != nullptr ||
+             ((connection_manager.config_.routeConfigProvider() == nullptr &&
+               connection_manager.config_.scopedRouteConfigProvider() != nullptr) ||
+              (connection_manager.config_.routeConfigProvider() != nullptr &&
+               connection_manager.config_.scopedRouteConfigProvider() == nullptr)),
+         "Either routeConfigProvider or scopedRouteConfigProvider should be set in "
+         "ConnectionManagerImpl.");
+  if (connection_manager.config_.routeConfigProvider() != nullptr) {
+    snapped_route_config_ = connection_manager.config_.routeConfigProvider()->config();
+  } else if (connection_manager.config_.scopedRouteConfigProvider() != nullptr) {
+    snapped_scoped_routes_config_ =
+        connection_manager_.config_.scopedRouteConfigProvider()->config<Router::ScopedConfig>();
+    ASSERT(snapped_scoped_routes_config_ != nullptr,
+           "Scoped rds provider returns null for scoped routes config.");
+  }
   ScopeTrackerScopeState scope(this,
                                connection_manager_.read_callbacks_->connection().dispatcher());
 
@@ -613,6 +629,17 @@ void ConnectionManagerImpl::ActiveStream::decodeHeaders(HeaderMapPtr&& headers,
   ScopeTrackerScopeState scope(this,
                                connection_manager_.read_callbacks_->connection().dispatcher());
   request_headers_ = std::move(headers);
+  // For Admin thread, we don't use routeConfigProvider or SRDS route provider.
+  if (dynamic_cast<Server::Admin*>(&connection_manager_.config_) == nullptr &&
+      connection_manager_.config_.scopedRouteConfigProvider() != nullptr) {
+    ASSERT(snapped_route_config_ == nullptr,
+           "Route config already latched to the active stream when scoped RDS is enabled.");
+    // We need to snap snapped_route_config_ here as it's used in mutateRequestHeaders later.
+    if (!snapScopedRouteConfig()) {
+      return;
+    }
+  }
+
   if (Http::Headers::get().MethodValues.Head ==
       request_headers_->Method()->value().getStringView()) {
     is_head_request_ = true;
@@ -1220,10 +1247,36 @@ void ConnectionManagerImpl::startDrainSequence() {
   drain_timer_->enableTimer(config_.drainTimeout());
 }
 
+bool ConnectionManagerImpl::ActiveStream::snapScopedRouteConfig() {
+  ASSERT(request_headers_ != nullptr,
+         "Try to snap scoped route config when there is no request headers.");
+
+  snapped_route_config_ = snapped_scoped_routes_config_->getRouteConfig(*request_headers_);
+  // NOTE: if a RDS subscription hasn't got a RouteConfiguration back, a Router::NullConfigImpl is
+  // returned, in that case we let it pass.
+  if (snapped_route_config_ == nullptr) {
+    ENVOY_STREAM_LOG(trace, "can't find SRDS scope.", *this);
+    // Stop decoding now.
+    maybeEndDecode(true);
+    sendLocalReply(Grpc::Common::hasGrpcContentType(*request_headers_), Http::Code::NotFound,
+                   "route scope not found", nullptr, is_head_request_, absl::nullopt,
+                   StreamInfo::ResponseCodeDetails::get().RouteConfigurationNotFound);
+    return false;
+  }
+  return true;
+}
+
 void ConnectionManagerImpl::ActiveStream::refreshCachedRoute() {
   Router::RouteConstSharedPtr route;
   if (request_headers_ != nullptr) {
-    route = snapped_route_config_->route(*request_headers_, stream_id_);
+    if (dynamic_cast<Server::Admin*>(&connection_manager_.config_) == nullptr &&
+        connection_manager_.config_.scopedRouteConfigProvider() != nullptr) {
+      // NOTE: re-select scope as well in case the scope key header has been changed by a filter.
+      snapScopedRouteConfig();
+    }
+    if (snapped_route_config_ != nullptr) {
+      route = snapped_route_config_->route(*request_headers_, stream_id_);
+    }
   }
   stream_info_.route_entry_ = route ? route->routeEntry() : nullptr;
   cached_route_ = std::move(route);

diff --git a/source/common/http/conn_manager_impl.h b/source/common/http/conn_manager_impl.h
@@ -496,6 +496,11 @@ class ConnectionManagerImpl : Logger::Loggable<Logger::Id::http>,
 
     void traceRequest();
 
+    // Updates the snapped_route_config_ if scope found, or ends the stream by
+    // sending local reply.
+    // Returns true if scoped route config snapped, false otherwise.
+    bool snapScopedRouteConfig();
+
     void refreshCachedRoute();
 
     // Pass on watermark callbacks to watermark subscribers. This boils down to passing watermark
@@ -585,7 +590,7 @@ class ConnectionManagerImpl : Logger::Loggable<Logger::Id::http>,
 
     ConnectionManagerImpl& connection_manager_;
     Router::ConfigConstSharedPtr snapped_route_config_;
-    Router::ScopedConfigConstSharedPtr snapped_scoped_route_config_;
+    Router::ScopedConfigConstSharedPtr snapped_scoped_routes_config_;
     Tracing::SpanPtr active_span_;
     const uint64_t stream_id_;
     StreamEncoder* response_encoder_{};

diff --git a/source/common/router/scoped_rds.cc b/source/common/router/scoped_rds.cc
@@ -316,7 +316,7 @@ void ScopedRdsConfigSubscription::onConfigUpdate(
     *to_remove_repeated.Add() = scoped_route.first;
   }
   onConfigUpdate(to_add_repeated, to_remove_repeated, version_info);
-} // namespace Router
+}
 
 ScopedRdsConfigProvider::ScopedRdsConfigProvider(
     ScopedRdsConfigSubscriptionSharedPtr&& subscription)

diff --git a/test/common/grpc/grpc_client_integration.h b/test/common/grpc/grpc_client_integration.h
@@ -43,7 +43,6 @@ class GrpcClientIntegrationParamTest
     : public BaseGrpcClientIntegrationParamTest,
       public testing::TestWithParam<std::tuple<Network::Address::IpVersion, ClientType>> {
 public:
-  ~GrpcClientIntegrationParamTest() override = default;
   static std::string protocolTestParamsToString(
       const ::testing::TestParamInfo<std::tuple<Network::Address::IpVersion, ClientType>>& p) {
     return fmt::format("{}_{}",
@@ -54,10 +53,26 @@ class GrpcClientIntegrationParamTest
   ClientType clientType() const override { return std::get<1>(GetParam()); }
 };
 
+class DeltaSotwGrpcClientIntegrationParamTest
+    : public BaseGrpcClientIntegrationParamTest,
+      public testing::TestWithParam<std::tuple<Network::Address::IpVersion, ClientType, bool>> {
+public:
+  static std::string protocolTestParamsToString(
+      const ::testing::TestParamInfo<std::tuple<Network::Address::IpVersion, ClientType, bool>>&
+          p) {
+    return fmt::format("{}_{}",
+                       std::get<0>(p.param) == Network::Address::IpVersion::v4 ? "IPv4" : "IPv6",
+                       std::get<1>(p.param) == ClientType::GoogleGrpc ? "GoogleGrpc" : "EnvoyGrpc",
+                       std::get<2>(p.param) ? "Delta" : "StateOfTheWorld");
+  }
+  Network::Address::IpVersion ipVersion() const override { return std::get<0>(GetParam()); }
+  ClientType clientType() const override { return std::get<1>(GetParam()); }
+  bool isDelta() { return std::get<2>(GetParam()); }
+};
+
 class DeltaSotwIntegrationParamTest
     : public testing::TestWithParam<std::tuple<Network::Address::IpVersion, SotwOrDelta>> {
 public:
-  ~DeltaSotwIntegrationParamTest() override = default;
   static std::string protocolTestParamsToString(
       const ::testing::TestParamInfo<std::tuple<Network::Address::IpVersion, SotwOrDelta>>& p) {
     return fmt::format("{}_{}_{}",
@@ -84,10 +99,17 @@ class DeltaSotwIntegrationParamTest
 #define GRPC_CLIENT_INTEGRATION_PARAMS                                                             \
   testing::Combine(testing::ValuesIn(TestEnvironment::getIpVersionsForTest()),                     \
                    testing::Values(Grpc::ClientType::EnvoyGrpc, Grpc::ClientType::GoogleGrpc))
+#define DELTA_SOTW_GRPC_CLIENT_INTEGRATION_PARAMS                                                  \
+  testing::Combine(testing::ValuesIn(TestEnvironment::getIpVersionsForTest()),                     \
+                   testing::Values(Grpc::ClientType::EnvoyGrpc, Grpc::ClientType::GoogleGrpc),     \
+                   testing::Bool())
 #else
 #define GRPC_CLIENT_INTEGRATION_PARAMS                                                             \
   testing::Combine(testing::ValuesIn(TestEnvironment::getIpVersionsForTest()),                     \
                    testing::Values(Grpc::ClientType::EnvoyGrpc))
+#define DELTA_SOTW_GRPC_CLIENT_INTEGRATION_PARAMS                                                  \
+  testing::Combine(testing::ValuesIn(TestEnvironment::getIpVersionsForTest()),                     \
+                   testing::Values(Grpc::ClientType::EnvoyGrpc), testing::Bool())
 #endif // ENVOY_GOOGLE_GRPC
 
 #define DELTA_INTEGRATION_PARAMS                                                                   \