Use dynamicMetadata to construct metadataMatchCriteria()

This enables filter writers to construct metadataMatchCriteria() from a RequestInfo (e.g.: header value). This is useful given that currently routes need to be statically matched when using metadata, e.g.: ``` --- match: prefix: / route: cluster: cluster-name metadata_match: filter_metadata: envoy.lb: version: '1.0' ``` For setups in which RDS isn't used and routes cannot be constructed dynamically, a filter can be used to match which endpoints should be used. So a filter could do something like: ``` auto header_value = std::string(header_entry->value().getStringView()); auto keyval = MessageUtil::keyValueStruct("version", header_value); callbacks_->requestInfo().setDynamicMetadata("envoy.lb", keyval); ``` to determine the version for this request. Signed-off-by: Raul Gutierrez Segales <[email protected]>
envoyproxy · Apr 30, 2018 · bde0c9c · bde0c9c
1 parent 2447cb7
commit bde0c9c
Show file tree

Hide file tree

Showing 14 changed files with 41 additions and 21 deletions.
diff --git a/include/envoy/router/router.h b/include/envoy/router/router.h
@@ -334,6 +334,9 @@ class MetadataMatchCriterion {
 
 typedef std::shared_ptr<const MetadataMatchCriterion> MetadataMatchCriterionConstSharedPtr;
 
+class MetadataMatchCriteria;
+typedef std::unique_ptr<const MetadataMatchCriteria> MetadataMatchCriteriaConstPtr;
+
 class MetadataMatchCriteria {
 public:
   virtual ~MetadataMatchCriteria() {}
@@ -345,6 +348,15 @@ class MetadataMatchCriteria {
    */
   virtual const std::vector<MetadataMatchCriterionConstSharedPtr>&
   metadataMatchCriteria() const PURE;
+
+  /**
+   * Creates a new MetadataMatchCriteriaImpl, merging existing
+   * metadata criteria this criteria. The result criteria is the
+   * combination of both sets of criteria, with those from the
+   * ProtobufWkt::Struct taking precedence.
+   */
+  virtual MetadataMatchCriteriaConstPtr
+  mergeMatchCriteria(const ProtobufWkt::Struct& metadata_matches) const PURE;
 };
 
 /**

diff --git a/include/envoy/upstream/load_balancer.h b/include/envoy/upstream/load_balancer.h
@@ -30,7 +30,7 @@ class LoadBalancerContext {
    * @return Router::MetadataMatchCriteria* metadata for use in selecting a subset of hosts
    *         during load balancing.
    */
-  virtual const Router::MetadataMatchCriteria* metadataMatchCriteria() const PURE;
+  virtual const Router::MetadataMatchCriteria* metadataMatchCriteria() PURE;
 
   /**
    * @return const Network::Connection* the incoming connection or nullptr to use during load

diff --git a/source/common/router/config_impl.h b/source/common/router/config_impl.h
@@ -256,14 +256,8 @@ class MetadataMatchCriteriaImpl : public MetadataMatchCriteria {
   MetadataMatchCriteriaImpl(const ProtobufWkt::Struct& metadata_matches)
       : metadata_match_criteria_(extractMetadataMatchCriteria(nullptr, metadata_matches)){};
 
-  /**
-   * Creates a new MetadataMatchCriteriaImpl, merging existing
-   * metadata criteria this criteria. The result criteria is the
-   * combination of both sets of criteria, with those from the
-   * ProtobufWkt::Struct taking precedence.
-   */
-  MetadataMatchCriteriaImplConstPtr
-  mergeMatchCriteria(const ProtobufWkt::Struct& metadata_matches) const {
+  MetadataMatchCriteriaConstPtr
+  mergeMatchCriteria(const ProtobufWkt::Struct& metadata_matches) const override {
     return MetadataMatchCriteriaImplConstPtr(
         new MetadataMatchCriteriaImpl(extractMetadataMatchCriteria(this, metadata_matches)));
   }
@@ -495,7 +489,7 @@ class RouteEntryImplBase : public RouteEntry,
     const std::string runtime_key_;
     Runtime::Loader& loader_;
     const uint64_t cluster_weight_;
-    MetadataMatchCriteriaImplConstPtr cluster_metadata_match_criteria_;
+    MetadataMatchCriteriaConstPtr cluster_metadata_match_criteria_;
     HeaderParserPtr request_headers_parser_;
     HeaderParserPtr response_headers_parser_;
     PerFilterConfigs per_filter_configs_;
@@ -539,7 +533,7 @@ class RouteEntryImplBase : public RouteEntry,
   std::vector<WeightedClusterEntrySharedPtr> weighted_clusters_;
   const uint64_t total_cluster_weight_;
   std::unique_ptr<const HashPolicyImpl> hash_policy_;
-  MetadataMatchCriteriaImplConstPtr metadata_match_criteria_;
+  MetadataMatchCriteriaConstPtr metadata_match_criteria_;
   HeaderParserPtr request_headers_parser_;
   HeaderParserPtr response_headers_parser_;
   envoy::api::v2::core::Metadata metadata_;

diff --git a/source/common/router/router.h b/source/common/router/router.h
@@ -21,6 +21,7 @@
 #include "common/common/hash.h"
 #include "common/common/hex.h"
 #include "common/common/logger.h"
+#include "common/config/well_known_names.h"
 #include "common/http/utility.h"
 #include "common/request_info/request_info_impl.h"
 
@@ -161,8 +162,15 @@ class Filter : Logger::Loggable<Logger::Id::router>,
     }
     return {};
   }
-  const Router::MetadataMatchCriteria* metadataMatchCriteria() const override {
+  const Router::MetadataMatchCriteria* metadataMatchCriteria() override {
     if (route_entry_) {
+      // The request's metadata, if present, takes precedence over the route's.
+      const auto& request_metadata = callbacks_->requestInfo().dynamicMetadata().filter_metadata();
+      const auto filter_it = request_metadata.find(Envoy::Config::MetadataFilters::get().ENVOY_LB);
+      if (filter_it != request_metadata.end()) {
+        metadata_match_ = route_entry_->metadataMatchCriteria()->mergeMatchCriteria(filter_it->second);
+        return metadata_match_.get();
+      }
       return route_entry_->metadataMatchCriteria();
     }
     return nullptr;
@@ -343,6 +351,7 @@ class Filter : Logger::Loggable<Logger::Id::router>,
   MonotonicTime downstream_request_complete_time_;
   uint32_t buffer_limit_{0};
   bool stream_destroyed_{};
+  MetadataMatchCriteriaConstPtr metadata_match_;
 
   // list of cookies to add to upstream headers
   std::vector<std::string> downstream_set_cookies_;

diff --git a/source/extensions/filters/network/redis_proxy/conn_pool_impl.h b/source/extensions/filters/network/redis_proxy/conn_pool_impl.h
@@ -166,7 +166,7 @@ class InstanceImpl : public Instance {
     // TODO(danielhochman): convert to HashUtil::xxHash64 when we have a migration strategy.
     // Upstream::LoadBalancerContext
     absl::optional<uint64_t> computeHashKey() override { return hash_key_; }
-    const Router::MetadataMatchCriteria* metadataMatchCriteria() const override { return nullptr; }
+    const Router::MetadataMatchCriteria* metadataMatchCriteria() override { return nullptr; }
     const Network::Connection* downstreamConnection() const override { return nullptr; }
 
     const absl::optional<uint64_t> hash_key_;

diff --git a/source/extensions/filters/network/tcp_proxy/tcp_proxy.h b/source/extensions/filters/network/tcp_proxy/tcp_proxy.h
@@ -154,7 +154,7 @@ class TcpProxyFilter : public Network::ReadFilter,
 
   // Upstream::LoadBalancerContext
   absl::optional<uint64_t> computeHashKey() override { return {}; }
-  const Router::MetadataMatchCriteria* metadataMatchCriteria() const override {
+  const Router::MetadataMatchCriteria* metadataMatchCriteria() override {
     if (config_) {
       return config_->metadataMatchCriteria();
     }

diff --git a/test/common/router/config_impl_test.cc b/test/common/router/config_impl_test.cc
@@ -3622,7 +3622,7 @@ TEST(MetadataMatchCriteriaImpl, Merge) {
   mutable_fields->insert({"b++", v2});
   mutable_fields->insert({"c", v3});
 
-  MetadataMatchCriteriaImplConstPtr matches = parent_matches.mergeMatchCriteria(metadata_struct);
+  MetadataMatchCriteriaConstPtr matches = parent_matches.mergeMatchCriteria(metadata_struct);
 
   EXPECT_EQ(matches->metadataMatchCriteria().size(), 4);
   auto it = matches->metadataMatchCriteria().begin();

diff --git a/test/common/router/router_test.cc b/test/common/router/router_test.cc
@@ -466,8 +466,9 @@ TEST_F(RouterTest, AddMultipleCookies) {
 TEST_F(RouterTest, MetadataNoOp) { EXPECT_EQ(nullptr, router_.metadataMatchCriteria()); }
 
 TEST_F(RouterTest, MetadataMatchCriteria) {
-  MockMetadataMatchCriteria matches;
+  envoy::api::v2::core::Metadata metadata;
 
+  EXPECT_CALL(callbacks_.request_info_, dynamicMetadata()).WillRepeatedly(ReturnRef(metadata));
   ON_CALL(callbacks_.route_->route_entry_, metadataMatchCriteria())
       .WillByDefault(Return(&callbacks_.route_->route_entry_.metadata_matches_criteria_));
   EXPECT_CALL(cm_, httpConnPoolForCluster(_, _, _, _))
@@ -491,6 +492,9 @@ TEST_F(RouterTest, MetadataMatchCriteria) {
 }
 
 TEST_F(RouterTest, NoMetadataMatchCriteria) {
+  envoy::api::v2::core::Metadata metadata;
+
+  EXPECT_CALL(callbacks_.request_info_, dynamicMetadata()).WillRepeatedly(ReturnRef(metadata));
   ON_CALL(callbacks_.route_->route_entry_, metadataMatchCriteria()).WillByDefault(Return(nullptr));
   EXPECT_CALL(cm_, httpConnPoolForCluster(_, _, _, _))
       .WillOnce(

diff --git a/test/common/upstream/load_balancer_benchmark.cc b/test/common/upstream/load_balancer_benchmark.cc
@@ -101,7 +101,7 @@ class TestLoadBalancerContext : public LoadBalancerContext {
 public:
   // Upstream::LoadBalancerContext
   absl::optional<uint64_t> computeHashKey() override { return hash_key_; }
-  const Router::MetadataMatchCriteria* metadataMatchCriteria() const override { return nullptr; }
+  const Router::MetadataMatchCriteria* metadataMatchCriteria() override { return nullptr; }
   const Network::Connection* downstreamConnection() const override { return nullptr; }
 
   absl::optional<uint64_t> hash_key_;

diff --git a/test/common/upstream/maglev_lb_test.cc b/test/common/upstream/maglev_lb_test.cc
@@ -12,7 +12,7 @@ class TestLoadBalancerContext : public LoadBalancerContext {
 
   // Upstream::LoadBalancerContext
   absl::optional<uint64_t> computeHashKey() override { return hash_key_; }
-  const Router::MetadataMatchCriteria* metadataMatchCriteria() const override { return nullptr; }
+  const Router::MetadataMatchCriteria* metadataMatchCriteria() override { return nullptr; }
   const Network::Connection* downstreamConnection() const override { return nullptr; }
 
   absl::optional<uint64_t> hash_key_;

diff --git a/test/common/upstream/original_dst_cluster_test.cc b/test/common/upstream/original_dst_cluster_test.cc
@@ -38,7 +38,7 @@ class TestLoadBalancerContext : public LoadBalancerContext {
   // Upstream::LoadBalancerContext
   absl::optional<uint64_t> computeHashKey() override { return 0; }
   const Network::Connection* downstreamConnection() const override { return connection_; }
-  const Router::MetadataMatchCriteria* metadataMatchCriteria() const override { return nullptr; }
+  const Router::MetadataMatchCriteria* metadataMatchCriteria() override { return nullptr; }
 
   absl::optional<uint64_t> hash_key_;
   const Network::Connection* connection_;

diff --git a/test/common/upstream/ring_hash_lb_test.cc b/test/common/upstream/ring_hash_lb_test.cc
@@ -28,7 +28,7 @@ class TestLoadBalancerContext : public LoadBalancerContext {
 
   // Upstream::LoadBalancerContext
   absl::optional<uint64_t> computeHashKey() override { return hash_key_; }
-  const Router::MetadataMatchCriteria* metadataMatchCriteria() const override { return nullptr; }
+  const Router::MetadataMatchCriteria* metadataMatchCriteria() override { return nullptr; }
   const Network::Connection* downstreamConnection() const override { return nullptr; }
 
   absl::optional<uint64_t> hash_key_;

diff --git a/test/common/upstream/subset_lb_test.cc b/test/common/upstream/subset_lb_test.cc
@@ -92,7 +92,7 @@ class TestLoadBalancerContext : public LoadBalancerContext {
   // Upstream::LoadBalancerContext
   absl::optional<uint64_t> computeHashKey() override { return {}; }
   const Network::Connection* downstreamConnection() const override { return nullptr; }
-  const Router::MetadataMatchCriteria* metadataMatchCriteria() const override {
+  const Router::MetadataMatchCriteria* metadataMatchCriteria() override {
     return matches_.get();
   }
 

diff --git a/test/mocks/router/mocks.h b/test/mocks/router/mocks.h
@@ -189,6 +189,7 @@ class MockMetadataMatchCriteria : public MetadataMatchCriteria {
   // Router::MetadataMatchCriteria
   MOCK_CONST_METHOD0(metadataMatchCriteria,
                      const std::vector<MetadataMatchCriterionConstSharedPtr>&());
+  MOCK_CONST_METHOD1(mergeMatchCriteria, MetadataMatchCriteriaConstPtr(const google::protobuf::Struct&));
 };
 
 class MockPathMatchCriterion : public PathMatchCriterion {