http: return per route config when direct response is set by ggreenway · Pull Request #17602 · envoyproxy/envoy

ggreenway · 2021-08-05T05:33:30Z

Backport of #17449, e071417

Co-authored-by: He Jie Xu hejie.xu@intel.com
Signed-off-by: Greg Greenway ggreenway@apple.com

Commit Message:
Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Deprecated:]
[Optional API Considerations:]

Backport of envoyproxy#17449, e071417 Co-authored-by: He Jie Xu <hejie.xu@intel.com> Signed-off-by: Greg Greenway <ggreenway@apple.com>

ggreenway · 2021-08-05T05:35:03Z

There were some conflicts to resolve, and I think they're resolved correctly. But here's a diff of the two commits (which is a diff of two diffs, so it's weird to read. Anything that doesn't start with either --, +-, -+, or ++, is context changes around the real change).

--- orig.patch	2021-08-04 22:28:18.000000000 -0700
+++ 17499.patch	2021-08-04 22:30:17.000000000 -0700
@@ -1,13 +1,16 @@
-commit e071417f12809eddbf02052504fd9953eedce8f0
-Author: xuhj <hejie.xu@intel.com>
-Date:   Tue Aug 3 01:58:47 2021 +0800
+commit 249676747a5d212725f0b6d44ab8ac17fe2dc48f
+Author: Greg Greenway <ggreenway@apple.com>
+Date:   Wed Aug 4 22:21:40 2021 -0700
 
-    http: return per route config when direct response is set (#17449)
+    http: return per route config when direct response is set
     
-    Signed-off-by: He Jie Xu <hejie.xu@intel.com>
+    Backport of #17449, e071417f12809eddbf02052504fd9953eedce8f0
+    
+    Co-authored-by: He Jie Xu <hejie.xu@intel.com>
+    Signed-off-by: Greg Greenway <ggreenway@apple.com>
 
 diff --git a/envoy/router/router.h b/envoy/router/router.h
-index f792581b74..48f8bb8f14 100644
+index e8a932262b..985ffdea73 100644
 --- a/envoy/router/router.h
 +++ b/envoy/router/router.h
 @@ -547,21 +547,6 @@ public:
@@ -32,7 +35,7 @@
    /**
     * @return bool whether to include the request count header in upstream requests.
     */
-@@ -902,31 +887,6 @@ public:
+@@ -914,31 +899,6 @@ public:
     */
    virtual const PathMatchCriterion& pathMatchCriterion() const PURE;
  
@@ -64,7 +67,7 @@
    /**
     * True if the virtual host this RouteEntry belongs to is configured to include the attempt
     * count header.
-@@ -1050,19 +1010,21 @@ public:
+@@ -1062,19 +1022,21 @@ public:
    virtual const RouteTracing* tracingConfig() const PURE;
  
    /**
@@ -92,14 +95,14 @@
 +  virtual void traversePerFilterConfig(
 +      const std::string& filter_name,
 +      std::function<void(const Router::RouteSpecificFilterConfig&)> cb) const PURE;
+ };
  
-   /**
-    * @return const envoy::config::core::v3::Metadata& return the metadata provided in the config for
+ using RouteConstSharedPtr = std::shared_ptr<const Route>;
 diff --git a/source/common/http/async_client_impl.h b/source/common/http/async_client_impl.h
-index 545d0f8d4f..0d0273c696 100644
+index d7193564ef..ebcb75b465 100644
 --- a/source/common/http/async_client_impl.h
 +++ b/source/common/http/async_client_impl.h
-@@ -192,9 +192,6 @@ private:
+@@ -190,9 +190,6 @@ private:
      const Router::RateLimitPolicy& rateLimitPolicy() const override { return rate_limit_policy_; }
      const Router::CorsPolicy* corsPolicy() const override { return nullptr; }
      const Router::Config& routeConfig() const override { return route_configuration_; }
@@ -109,7 +112,7 @@
      bool includeAttemptCountInRequest() const override { return false; }
      bool includeAttemptCountInResponse() const override { return false; }
      uint32_t retryShadowBufferLimit() const override {
-@@ -303,9 +300,6 @@ private:
+@@ -295,9 +292,6 @@ private:
        return path_match_criterion_;
      }
  
@@ -119,7 +122,7 @@
      const absl::optional<ConnectConfig>& connectConfig() const override {
        return connect_config_nullopt_;
      }
-@@ -345,9 +339,13 @@ private:
+@@ -338,9 +332,13 @@ private:
      const Router::RouteEntry* routeEntry() const override { return &route_entry_; }
      const Router::Decorator* decorator() const override { return nullptr; }
      const Router::RouteTracing* tracingConfig() const override { return nullptr; }
@@ -131,14 +134,14 @@
 +    void traversePerFilterConfig(
 +        const std::string&,
 +        std::function<void(const Router::RouteSpecificFilterConfig&)>) const override {}
-     const envoy::config::core::v3::Metadata& metadata() const override { return metadata_; }
-     const Envoy::Config::TypedMetadata& typedMetadata() const override { return typed_metadata_; }
  
+     RouteEntryImpl route_entry_;
+   };
 diff --git a/source/common/http/utility.cc b/source/common/http/utility.cc
-index cc472f8e1c..4409f8378c 100644
+index 3320090c1c..f41bef0305 100644
 --- a/source/common/http/utility.cc
 +++ b/source/common/http/utility.cc
-@@ -921,35 +921,6 @@ void Utility::transformUpgradeResponseFromH2toH1(ResponseHeaderMap& headers,
+@@ -914,35 +914,6 @@ void Utility::transformUpgradeResponseFromH2toH1(ResponseHeaderMap& headers,
    }
  }
  
@@ -175,10 +178,10 @@
                                               absl::string_view reserved_chars) {
    absl::flat_hash_set<char> reserved_char_set{reserved_chars.begin(), reserved_chars.end()};
 diff --git a/source/common/http/utility.h b/source/common/http/utility.h
-index 20d5ae98b2..7929e687e5 100644
+index c5dd2e270d..eaf203908b 100644
 --- a/source/common/http/utility.h
 +++ b/source/common/http/utility.h
-@@ -512,40 +512,10 @@ const ConfigType* resolveMostSpecificPerFilterConfig(const std::string& filter_n
+@@ -503,40 +503,10 @@ const ConfigType* resolveMostSpecificPerFilterConfig(const std::string& filter_n
                                                       const Router::RouteConstSharedPtr& route) {
    static_assert(std::is_base_of<Router::RouteSpecificFilterConfig, ConfigType>::value,
                  "ConfigType must be a subclass of Router::RouteSpecificFilterConfig");
@@ -221,7 +224,7 @@
  }
  
  /**
-@@ -567,14 +537,17 @@ getMergedPerFilterConfig(const std::string& filter_name, const Router::RouteCons
+@@ -558,14 +528,17 @@ getMergedPerFilterConfig(const std::string& filter_name, const Router::RouteCons
  
    absl::optional<ConfigType> merged;
  
@@ -248,10 +251,10 @@
    return merged;
  }
 diff --git a/source/common/router/config_impl.cc b/source/common/router/config_impl.cc
-index 29a362cf61..5076f96c7e 100644
+index b1432fa02b..d4e5139881 100644
 --- a/source/common/router/config_impl.cc
 +++ b/source/common/router/config_impl.cc
-@@ -1005,9 +1005,24 @@ void RouteEntryImplBase::validateClusters(
+@@ -1001,9 +1001,24 @@ void RouteEntryImplBase::validateClusters(
    }
  }
  
@@ -279,7 +282,7 @@
  }
  
  RouteEntryImplBase::WeightedClusterEntry::WeightedClusterEntry(
-@@ -1050,10 +1065,15 @@ Http::HeaderTransforms RouteEntryImplBase::WeightedClusterEntry::responseHeaderT
+@@ -1046,10 +1061,15 @@ Http::HeaderTransforms RouteEntryImplBase::WeightedClusterEntry::responseHeaderT
    return transforms;
  }
  
@@ -300,7 +303,7 @@
  
  PrefixRouteEntryImpl::PrefixRouteEntryImpl(
 diff --git a/source/common/router/config_impl.h b/source/common/router/config_impl.h
-index ac0d44db04..cbad85c100 100644
+index a6add7e18e..11d481eb3b 100644
 --- a/source/common/router/config_impl.h
 +++ b/source/common/router/config_impl.h
 @@ -127,9 +127,12 @@ public:
@@ -314,10 +317,10 @@
 +  void traversePerFilterConfig(
 +      const std::string&,
 +      std::function<void(const Router::RouteSpecificFilterConfig&)>) const override {}
-   const envoy::config::core::v3::Metadata& metadata() const override { return metadata_; }
-   const Envoy::Config::TypedMetadata& typedMetadata() const override { return typed_metadata_; }
  
-@@ -212,7 +215,7 @@ public:
+ private:
+   static const SslRedirector SSL_REDIRECTOR;
+@@ -207,7 +210,7 @@ public:
    Stats::StatName statName() const override { return stat_name_storage_.statName(); }
    const RateLimitPolicy& rateLimitPolicy() const override { return rate_limit_policy_; }
    const Config& routeConfig() const override;
@@ -326,7 +329,7 @@
    bool includeAttemptCountInRequest() const override { return include_attempt_count_in_request_; }
    bool includeAttemptCountInResponse() const override { return include_attempt_count_in_response_; }
    const absl::optional<envoy::config::route::v3::RetryPolicy>& retryPolicy() const {
-@@ -591,7 +594,14 @@ public:
+@@ -586,7 +589,14 @@ public:
    const RouteEntry* routeEntry() const override;
    const Decorator* decorator() const override { return decorator_.get(); }
    const RouteTracing* tracingConfig() const override { return route_tracing_.get(); }
@@ -342,7 +345,7 @@
  
  protected:
    const bool case_sensitive_;
-@@ -737,9 +747,14 @@ private:
+@@ -732,9 +742,14 @@ private:
      const RouteEntry* routeEntry() const override { return this; }
      const Decorator* decorator() const override { return parent_->decorator(); }
      const RouteTracing* tracingConfig() const override { return parent_->tracingConfig(); }
@@ -360,7 +363,7 @@
      };
  
    private:
-@@ -789,7 +804,15 @@ private:
+@@ -784,7 +799,15 @@ private:
      Http::HeaderTransforms responseHeaderTransforms(const StreamInfo::StreamInfo& stream_info,
                                                      bool do_formatting = true) const override;
  
@@ -378,7 +381,7 @@
    private:
      const std::string runtime_key_;
 diff --git a/source/common/router/delegating_route_impl.cc b/source/common/router/delegating_route_impl.cc
-index f1d2dddfe3..422b43248d 100644
+index 935a648e9c..d9ffff48ef 100644
 --- a/source/common/router/delegating_route_impl.cc
 +++ b/source/common/router/delegating_route_impl.cc
 @@ -14,10 +14,6 @@ const Decorator* DelegatingRoute::decorator() const { return base_route_->decora
@@ -392,7 +395,7 @@
  // Router:DelegatingRouteEntry
  void DelegatingRouteEntry::finalizeResponseHeaders(
      Http::ResponseHeaderMap& headers, const StreamInfo::StreamInfo& stream_info) const {
-@@ -150,11 +146,6 @@ const PathMatchCriterion& DelegatingRouteEntry::pathMatchCriterion() const {
+@@ -158,11 +154,6 @@ const PathMatchCriterion& DelegatingRouteEntry::pathMatchCriterion() const {
    return base_route_->routeEntry()->pathMatchCriterion();
  }
  
@@ -405,10 +408,10 @@
    return base_route_->routeEntry()->includeAttemptCountInRequest();
  }
 diff --git a/source/common/router/delegating_route_impl.h b/source/common/router/delegating_route_impl.h
-index 87421cb625..f9696e8fc7 100644
+index 53721b394c..71b6ff3da0 100644
 --- a/source/common/router/delegating_route_impl.h
 +++ b/source/common/router/delegating_route_impl.h
-@@ -26,7 +26,17 @@ public:
+@@ -24,7 +24,16 @@ public:
    const Router::RouteEntry* routeEntry() const override;
    const Router::Decorator* decorator() const override;
    const Router::RouteTracing* tracingConfig() const override;
@@ -423,12 +426,11 @@
 +      std::function<void(const Router::RouteSpecificFilterConfig&)> cb) const override {
 +    base_route_->traversePerFilterConfig(filter_name, cb);
 +  }
-+
-   const envoy::config::core::v3::Metadata& metadata() const override {
-     return base_route_->metadata();
-   }
-@@ -90,7 +100,6 @@ public:
-   bool includeVirtualHostRateLimits() const override;
+ 
+ private:
+   const Router::RouteConstSharedPtr base_route_;
+@@ -84,7 +93,6 @@ public:
+   const envoy::config::core::v3::Metadata& metadata() const override;
    const TlsContextMatchCriteria* tlsContextMatchCriteria() const override;
    const PathMatchCriterion& pathMatchCriterion() const override;
 -  const RouteSpecificFilterConfig* perFilterConfig(const std::string& name) const override;
@@ -436,36 +438,36 @@
    bool includeAttemptCountInResponse() const override;
    const UpgradeMap& upgradeMap() const override;
 diff --git a/test/common/http/async_client_impl_test.cc b/test/common/http/async_client_impl_test.cc
-index 8568f0dd31..5728e0fa31 100644
+index ea81164f1c..700f331f39 100644
 --- a/test/common/http/async_client_impl_test.cc
 +++ b/test/common/http/async_client_impl_test.cc
-@@ -1572,7 +1572,6 @@ TEST_F(AsyncClientImplUnitTest, NullRouteImplInitTest) {
- 
-   EXPECT_EQ(nullptr, route_impl_->decorator());
-   EXPECT_EQ(nullptr, route_impl_->tracingConfig());
--  EXPECT_EQ(nullptr, route_impl_->perFilterConfig(""));
-   EXPECT_EQ(Code::InternalServerError, route_entry.clusterNotFoundResponseCode());
-   EXPECT_EQ(nullptr, route_entry.corsPolicy());
-   EXPECT_EQ(nullptr, route_entry.hashPolicy());
-@@ -1587,13 +1586,11 @@ TEST_F(AsyncClientImplUnitTest, NullRouteImplInitTest) {
-   EXPECT_TRUE(route_entry.opaqueConfig().empty());
-   EXPECT_TRUE(route_entry.includeVirtualHostRateLimits());
-   EXPECT_EQ(nullptr, route_impl_->typedMetadata().get<Config::TypedMetadata::Object>("bar"));
--  EXPECT_EQ(nullptr, route_entry.perFilterConfig("bar"));
-   EXPECT_TRUE(route_entry.upgradeMap().empty());
-   EXPECT_EQ(false, route_entry.internalRedirectPolicy().enabled());
-   EXPECT_TRUE(route_entry.shadowPolicies().empty());
-   EXPECT_TRUE(route_entry.virtualHost().rateLimitPolicy().empty());
-   EXPECT_EQ(nullptr, route_entry.virtualHost().corsPolicy());
--  EXPECT_EQ(nullptr, route_entry.virtualHost().perFilterConfig("bar"));
-   EXPECT_FALSE(route_entry.virtualHost().includeAttemptCountInRequest());
-   EXPECT_FALSE(route_entry.virtualHost().includeAttemptCountInResponse());
-   EXPECT_FALSE(route_entry.virtualHost().routeConfig().usesVhds());
+@@ -1556,7 +1556,6 @@ public:
+ TEST_F(AsyncClientImplUnitTest, RouteImplInitTest) {
+   EXPECT_EQ(nullptr, route_impl_.decorator());
+   EXPECT_EQ(nullptr, route_impl_.tracingConfig());
+-  EXPECT_EQ(nullptr, route_impl_.perFilterConfig(""));
+   EXPECT_EQ(Code::InternalServerError, route_impl_.routeEntry()->clusterNotFoundResponseCode());
+   EXPECT_EQ(nullptr, route_impl_.routeEntry()->corsPolicy());
+   EXPECT_EQ(nullptr, route_impl_.routeEntry()->hashPolicy());
+@@ -1573,13 +1572,11 @@ TEST_F(AsyncClientImplUnitTest, RouteImplInitTest) {
+   EXPECT_TRUE(route_impl_.routeEntry()->metadata().filter_metadata().empty());
+   EXPECT_EQ(nullptr,
+             route_impl_.routeEntry()->typedMetadata().get<Config::TypedMetadata::Object>("bar"));
+-  EXPECT_EQ(nullptr, route_impl_.routeEntry()->perFilterConfig("bar"));
+   EXPECT_TRUE(route_impl_.routeEntry()->upgradeMap().empty());
+   EXPECT_EQ(false, route_impl_.routeEntry()->internalRedirectPolicy().enabled());
+   EXPECT_TRUE(route_impl_.routeEntry()->shadowPolicies().empty());
+   EXPECT_TRUE(route_impl_.routeEntry()->virtualHost().rateLimitPolicy().empty());
+   EXPECT_EQ(nullptr, route_impl_.routeEntry()->virtualHost().corsPolicy());
+-  EXPECT_EQ(nullptr, route_impl_.routeEntry()->virtualHost().perFilterConfig("bar"));
+   EXPECT_FALSE(route_impl_.routeEntry()->virtualHost().includeAttemptCountInRequest());
+   EXPECT_FALSE(route_impl_.routeEntry()->virtualHost().includeAttemptCountInResponse());
+   EXPECT_FALSE(route_impl_.routeEntry()->virtualHost().routeConfig().usesVhds());
 diff --git a/test/common/http/conn_manager_impl_test.cc b/test/common/http/conn_manager_impl_test.cc
-index 0e17a20f93..bb1bf85f13 100644
+index 69894daf18..4232ca8ca9 100644
 --- a/test/common/http/conn_manager_impl_test.cc
 +++ b/test/common/http/conn_manager_impl_test.cc
-@@ -1078,8 +1078,6 @@ TEST_F(HttpConnectionManagerImplTest, DelegatingRouteEntryAllCalls) {
+@@ -1088,8 +1088,6 @@ TEST_F(HttpConnectionManagerImplTest, DelegatingRouteEntryAllCalls) {
          EXPECT_EQ(default_route->routeEntry()->pathMatchCriterion().matchType(),
                    delegating_route_foo->routeEntry()->pathMatchCriterion().matchType());
  
@@ -475,7 +477,7 @@
                    delegating_route_foo->routeEntry()->includeAttemptCountInRequest());
          EXPECT_EQ(default_route->routeEntry()->includeAttemptCountInResponse(),
 diff --git a/test/common/http/utility_test.cc b/test/common/http/utility_test.cc
-index 62fcd5f17f..76304a34bf 100644
+index f86935dece..0878449fb0 100644
 --- a/test/common/http/utility_test.cc
 +++ b/test/common/http/utility_test.cc
 @@ -892,9 +892,7 @@ TEST(HttpUtility, ResolveMostSpecificPerFilterConfig) {
@@ -603,10 +605,10 @@
    // merge the configs
    auto merged_cfg = Utility::getMergedPerFilterConfig<TestConfig>(
 diff --git a/test/common/router/config_impl_test.cc b/test/common/router/config_impl_test.cc
-index 17a7403c20..0f950c2ca8 100644
+index a403b85747..92237b4fe2 100644
 --- a/test/common/router/config_impl_test.cc
 +++ b/test/common/router/config_impl_test.cc
-@@ -4495,7 +4495,7 @@ virtual_hosts:
+@@ -4470,7 +4470,7 @@ virtual_hosts:
          genRedirectHeaders("redirect.lyft.com", "/https", false, false);
      EXPECT_EQ("https://redirect.lyft.com/https",
                config.route(headers, 0)->directResponseEntry()->newPath(headers));
@@ -615,7 +617,7 @@
    }
    {
      Http::TestRequestHeaderMapImpl headers =
-@@ -7531,20 +7531,22 @@ public:
+@@ -7498,20 +7498,22 @@ public:
      }
    };
  
@@ -648,7 +650,7 @@
    }
  
    void check(const DerivedFilterConfig* cfg, uint32_t expected_seconds, std::string source) {
-@@ -7560,13 +7562,15 @@ public:
+@@ -7527,13 +7529,15 @@ public:
                                  optional_http_filters);
  
      const auto route = config.route(genHeaders("www.foo.com", "/", "GET"), 0);
@@ -670,7 +672,7 @@
    }
  
    TestFilterConfig factory_;
-@@ -7955,7 +7959,8 @@ virtual_hosts:
+@@ -7922,7 +7926,8 @@ virtual_hosts:
  )EOF";
  
    factory_context_.cluster_manager_.initializeClusters({"baz"}, {});
@@ -680,7 +682,7 @@
  }
  
  TEST_F(PerFilterConfigsTest, RouteLocalTypedConfig) {
-@@ -7979,7 +7984,8 @@ virtual_hosts:
+@@ -7946,7 +7951,8 @@ virtual_hosts:
  )EOF";
  
    factory_context_.cluster_manager_.initializeClusters({"baz"}, {});
@@ -690,7 +692,7 @@
  }
  
  TEST_F(PerFilterConfigsTest, DEPRECATED_FEATURE_TEST(WeightedClusterConfig)) {
-@@ -8000,7 +8006,8 @@ virtual_hosts:
+@@ -7967,7 +7973,8 @@ virtual_hosts:
  )EOF";
  
    factory_context_.cluster_manager_.initializeClusters({"baz"}, {});
@@ -700,7 +702,7 @@
  }
  
  TEST_F(PerFilterConfigsTest, WeightedClusterTypedConfig) {
-@@ -8028,7 +8035,8 @@ virtual_hosts:
+@@ -7995,7 +8002,8 @@ virtual_hosts:
  )EOF";
  
    factory_context_.cluster_manager_.initializeClusters({"baz"}, {});
@@ -710,7 +712,7 @@
  }
  
  TEST_F(PerFilterConfigsTest, DEPRECATED_FEATURE_TEST(WeightedClusterFallthroughConfig)) {
-@@ -8049,7 +8057,8 @@ virtual_hosts:
+@@ -8016,7 +8024,8 @@ virtual_hosts:
  )EOF";
  
    factory_context_.cluster_manager_.initializeClusters({"baz"}, {});
@@ -720,7 +722,7 @@
  }
  
  TEST_F(PerFilterConfigsTest, WeightedClusterFallthroughTypedConfig) {
-@@ -8077,7 +8086,8 @@ virtual_hosts:
+@@ -8044,7 +8053,8 @@ virtual_hosts:
  )EOF";
  
    factory_context_.cluster_manager_.initializeClusters({"baz"}, {});
@@ -924,10 +926,10 @@
  
    auto test_disable_request_body_buffering = [&](bool bypass) {
 diff --git a/test/extensions/filters/http/fault/fault_filter_test.cc b/test/extensions/filters/http/fault/fault_filter_test.cc
-index b4add362fb..9b6e1347c9 100644
+index f09372a74c..5928132458 100644
 --- a/test/extensions/filters/http/fault/fault_filter_test.cc
 +++ b/test/extensions/filters/http/fault/fault_filter_test.cc
-@@ -147,9 +147,6 @@ public:
+@@ -146,9 +146,6 @@ public:
      EXPECT_CALL(*timer_, disableTimer());
    }
  
@@ -937,7 +939,7 @@
    NiceMock<Stats::MockIsolatedStatsStore> stats_;
    FaultFilterConfigSharedPtr config_;
    std::unique_ptr<FaultFilter> filter_;
-@@ -1192,16 +1189,13 @@ TEST_F(FaultFilterTest, FaultWithTargetClusterNullRoute) {
+@@ -1191,16 +1188,13 @@ TEST_F(FaultFilterTest, FaultWithTargetClusterNullRoute) {
    EXPECT_EQ(0UL, config_->stats().aborts_injected_.value());
  }
  
@@ -960,7 +962,7 @@
  
    const std::string upstream_cluster("www1");
  
-@@ -1240,34 +1234,6 @@ void FaultFilterTest::TestPerFilterConfigFault(
+@@ -1239,34 +1233,6 @@ void FaultFilterTest::TestPerFilterConfigFault(
    EXPECT_EQ(0UL, config_->stats().aborts_injected_.value());
  }
  
@@ -1221,10 +1223,10 @@
    ASSERT_EQ(filter_->decodeHeaders(request_headers_, false), Http::FilterHeadersStatus::Continue);
  }
 diff --git a/test/extensions/filters/http/lua/lua_filter_test.cc b/test/extensions/filters/http/lua/lua_filter_test.cc
-index 843537be50..24c575c1da 100644
+index 82d1623ff0..765caf6ee6 100644
 --- a/test/extensions/filters/http/lua/lua_filter_test.cc
 +++ b/test/extensions/filters/http/lua/lua_filter_test.cc
-@@ -2184,7 +2184,7 @@ TEST_F(LuaHttpFilterTest, LuaFilterDisabled) {
+@@ -2185,7 +2185,7 @@ TEST_F(LuaHttpFilterTest, LuaFilterDisabled) {
  
    EXPECT_CALL(decoder_callbacks_, clearRouteCache());
  
@@ -1233,7 +1235,7 @@
        .WillByDefault(Return(nullptr));
  
    Http::TestRequestHeaderMapImpl request_headers_1{{":path", "/"}};
-@@ -2192,7 +2192,7 @@ TEST_F(LuaHttpFilterTest, LuaFilterDisabled) {
+@@ -2193,7 +2193,7 @@ TEST_F(LuaHttpFilterTest, LuaFilterDisabled) {
    EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers_1, true));
    EXPECT_EQ("world", request_headers_1.get_("hello"));
  
@@ -1242,7 +1244,7 @@
        .WillByDefault(Return(per_route_config_.get()));
  
    Http::TestRequestHeaderMapImpl request_headers_2{{":path", "/"}};
-@@ -2228,7 +2228,7 @@ TEST_F(LuaHttpFilterTest, LuaFilterRefSourceCodes) {
+@@ -2229,7 +2229,7 @@ TEST_F(LuaHttpFilterTest, LuaFilterRefSourceCodes) {
    setupConfig(proto_config, per_route_proto_config);
    setupFilter();
  
@@ -1251,7 +1253,7 @@
        .WillByDefault(Return(per_route_config_.get()));
  
    Http::TestRequestHeaderMapImpl request_headers{{":path", "/"}};
-@@ -2257,7 +2257,7 @@ TEST_F(LuaHttpFilterTest, LuaFilterRefSourceCodeNotExist) {
+@@ -2258,7 +2258,7 @@ TEST_F(LuaHttpFilterTest, LuaFilterRefSourceCodeNotExist) {
    setupConfig(proto_config, per_route_proto_config);
    setupFilter();
  
@@ -1355,10 +1357,10 @@
  
    EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.decodeHeaders(headers_, true));
 diff --git a/test/mocks/router/mocks.h b/test/mocks/router/mocks.h
-index 8d7d1c572e..1fafd6d9ac 100644
+index 141c393488..0762061623 100644
 --- a/test/mocks/router/mocks.h
 +++ b/test/mocks/router/mocks.h
-@@ -451,6 +451,11 @@ public:
+@@ -454,6 +454,11 @@ public:
    MOCK_METHOD(const Decorator*, decorator, (), (const));
    MOCK_METHOD(const RouteTracing*, tracingConfig, (), (const));
    MOCK_METHOD(const RouteSpecificFilterConfig*, perFilterConfig, (const std::string&), (const));
@@ -1367,6 +1369,6 @@
 +  MOCK_METHOD(void, traversePerFilterConfig,
 +              (const std::string&, std::function<void(const Router::RouteSpecificFilterConfig&)>),
 +              (const));
-   MOCK_METHOD(const envoy::config::core::v3::Metadata&, metadata, (), (const));
-   MOCK_METHOD(const Envoy::Config::TypedMetadata&, typedMetadata, (), (const));
  
+   testing::NiceMock<MockRouteEntry> route_entry_;
+   testing::NiceMock<MockDecorator> decorator_;

ggreenway · 2021-08-05T05:36:04Z

cc @soulxu

soulxu · 2021-08-05T06:24:20Z

thanks, LGTM!

Maybe I should have a simple fix without refractory change first, then we needn't backport the refractory to the stable.

soulxu · 2021-08-05T10:22:29Z

/retest

repokitteh-read-only · 2021-08-05T10:22:33Z

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #17602 (comment) was created by @soulxu.

see: more, trace.

ggreenway · 2021-08-05T19:08:28Z

/retest

repokitteh-read-only · 2021-08-05T19:08:32Z

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #17602 (comment) was created by @ggreenway.

see: more, trace.

soulxu · 2021-08-06T11:26:57Z

/retest

repokitteh-read-only · 2021-08-06T11:27:00Z

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #17602 (comment) was created by @soulxu.

see: more, trace.

mattklein123

Thanks!

http: return per route config when direct response is set

2496767

Backport of envoyproxy#17449, e071417 Co-authored-by: He Jie Xu <hejie.xu@intel.com> Signed-off-by: Greg Greenway <ggreenway@apple.com>

ggreenway requested review from alyssawilk, htuch, lizan, mattklein123, snowp and zuercher as code owners August 5, 2021 05:33

ggreenway assigned mattklein123 Aug 5, 2021

mattklein123 approved these changes Aug 6, 2021

View reviewed changes

ggreenway merged commit 68920f7 into envoyproxy:release/v1.19 Aug 6, 2021

kdorosh mentioned this pull request Aug 25, 2021

Update envoy-gloo to 1.19.1 rather than 1.19.0 kgateway-dev/kgateway#5237

Closed

dio mentioned this pull request Sep 8, 2021

type.googleapis.com/envoy.extensions.filters.http.lua.v3.LuaPerRoute disabled option not working in version 1.19.0 #17798

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

http: return per route config when direct response is set#17602

http: return per route config when direct response is set#17602
ggreenway merged 1 commit intoenvoyproxy:release/v1.19from
ggreenway:backport-17449

ggreenway commented Aug 5, 2021

Uh oh!

ggreenway commented Aug 5, 2021

Uh oh!

ggreenway commented Aug 5, 2021

Uh oh!

soulxu commented Aug 5, 2021

Uh oh!

soulxu commented Aug 5, 2021

Uh oh!

repokitteh-read-only bot commented Aug 5, 2021

Uh oh!

ggreenway commented Aug 5, 2021

Uh oh!

repokitteh-read-only bot commented Aug 5, 2021

Uh oh!

soulxu commented Aug 6, 2021

Uh oh!

repokitteh-read-only bot commented Aug 6, 2021

Uh oh!

mattklein123 left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

ggreenway commented Aug 5, 2021

Uh oh!

ggreenway commented Aug 5, 2021

Uh oh!

ggreenway commented Aug 5, 2021

Uh oh!

soulxu commented Aug 5, 2021

Uh oh!

soulxu commented Aug 5, 2021

Uh oh!

repokitteh-read-only bot commented Aug 5, 2021

Uh oh!

ggreenway commented Aug 5, 2021

Uh oh!

repokitteh-read-only bot commented Aug 5, 2021

Uh oh!

soulxu commented Aug 6, 2021

Uh oh!

repokitteh-read-only bot commented Aug 6, 2021

Uh oh!

mattklein123 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants