wasm: removed automatical route refreshment and add a foreign functio…

…n to clear the route cache (#36671)

Commit Message: wasm: removed automatical route refreshment and add a
foreign function to clear the route cache

Additional Description:

Here are the reasons to do this change:
1. It make it's impossible to only mofify the host or path or some
headers but don't effect the route result.
2. Basically, refreshing route is not encouraged. Even the filters
(ext_authz, jwt_authz, ext_proc, etc) who acutally need or want to clear
the route cache will also provide a flag (and basically default to
`false`) to control the behavior.
3. Refreshing route have big effect to the traffic and should only be
done explicitly (code or configurations), we should never change it
implicitly.

Risk Level: low.
Testing: n/a.
Docs Changes: n/a.
Release Notes: n/a.
Platform Specific Features: n/a.

---------

Signed-off-by: wangbaiping/wbpcode <wangbaiping@bytedance.com>
Signed-off-by: wangbaiping(wbpcode) <wangbaiping@bytedance.com>

changelogs/current.yaml

@@ -2,6 +2,10 @@ date: Pending
 
 behavior_changes:
 # *Changes that are expected to cause an incompatibility if applicable; deployment changes are likely required*
+- area: wasm
+  change: |
+    The route cache will not be cleared by default if the wasm extension modified the request headers and
+    the ABI version of wasm extension is larger then 0.2.1.
 - area: wasm
   change: |
     Remove previously deprecated xDS attributes from ``get_property``, use ``xds`` attributes instead.
@@ -90,6 +94,9 @@ new_features:
 - area: tls
   change: |
     Added support for P-384 and P-521 curves for TLS server certificates.
+- area: wasm
+  change: |
+    added ``clear_route_cache`` foreign function to clear the route cache.
 - area: access_log
   change: |
     Added %DOWNSTREAM_LOCAL_EMAIL_SAN%, %DOWNSTREAM_PEER_EMAIL_SAN%, %DOWNSTREAM_LOCAL_OTHERNAME_SAN% and

source/extensions/common/wasm/context.cc

@@ -153,12 +153,23 @@ WasmResult Buffer::copyFrom(size_t start, size_t length, std::string_view data)
 }
 
 Context::Context() = default;
-Context::Context(Wasm* wasm) : ContextBase(wasm) {}
+Context::Context(Wasm* wasm) : ContextBase(wasm) {
+  if (wasm != nullptr) {
+    abi_version_ = wasm->abi_version_;
+  }
+}
 Context::Context(Wasm* wasm, const PluginSharedPtr& plugin) : ContextBase(wasm, plugin) {
+  if (wasm != nullptr) {
+    abi_version_ = wasm->abi_version_;
+  }
   root_local_info_ = &std::static_pointer_cast<Plugin>(plugin)->localInfo();
 }
 Context::Context(Wasm* wasm, uint32_t root_context_id, PluginHandleSharedPtr plugin_handle)
-    : ContextBase(wasm, root_context_id, plugin_handle), plugin_handle_(plugin_handle) {}
+    : ContextBase(wasm, root_context_id, plugin_handle), plugin_handle_(plugin_handle) {
+  if (wasm != nullptr) {
+    abi_version_ = wasm->abi_version_;
+  }
+}
 
 Wasm* Context::wasm() const { return static_cast<Wasm*>(wasm_); }
 Plugin* Context::plugin() const { return static_cast<Plugin*>(plugin_.get()); }
@@ -671,9 +682,7 @@ WasmResult Context::addHeaderMapValue(WasmHeaderMapType type, std::string_view k
   }
   const Http::LowerCaseString lower_key{std::string(key)};
   map->addCopy(lower_key, std::string(value));
-  if (type == WasmHeaderMapType::RequestHeaders) {
-    clearRouteCache();
-  }
+  onHeadersModified(type);
   return WasmResult::Ok;
 }
 
@@ -746,9 +755,7 @@ WasmResult Context::setHeaderMapPairs(WasmHeaderMapType type, const Pairs& pairs
     const Http::LowerCaseString lower_key{std::string(p.first)};
     map->addCopy(lower_key, std::string(p.second));
   }
-  if (type == WasmHeaderMapType::RequestHeaders) {
-    clearRouteCache();
-  }
+  onHeadersModified(type);
   return WasmResult::Ok;
 }
 
@@ -759,9 +766,7 @@ WasmResult Context::removeHeaderMapValue(WasmHeaderMapType type, std::string_vie
   }
   const Http::LowerCaseString lower_key{std::string(key)};
   map->remove(lower_key);
-  if (type == WasmHeaderMapType::RequestHeaders) {
-    clearRouteCache();
-  }
+  onHeadersModified(type);
   return WasmResult::Ok;
 }
 
@@ -773,9 +778,7 @@ WasmResult Context::replaceHeaderMapValue(WasmHeaderMapType type, std::string_vi
   }
   const Http::LowerCaseString lower_key{std::string(key)};
   map->setCopy(lower_key, toAbslStringView(value));
-  if (type == WasmHeaderMapType::RequestHeaders) {
-    clearRouteCache();
-  }
+  onHeadersModified(type);
   return WasmResult::Ok;
 }
 

source/extensions/common/wasm/context.h

@@ -374,6 +374,14 @@ class Context : public proxy_wasm::ContextBase,
   Http::HeaderMap* getMap(WasmHeaderMapType type);
   const Http::HeaderMap* getConstMap(WasmHeaderMapType type);
 
+  void onHeadersModified(WasmHeaderMapType type) {
+    if (type != WasmHeaderMapType::RequestHeaders ||
+        abi_version_ > proxy_wasm::AbiVersion::ProxyWasm_0_2_1) {
+      return;
+    }
+    clearRouteCache();
+  }
+
   const LocalInfo::LocalInfo* root_local_info_{nullptr}; // set only for root_context.
   PluginHandleSharedPtr plugin_handle_{nullptr};
 
@@ -450,6 +458,8 @@ class Context : public proxy_wasm::ContextBase,
   // Filter state prototype declaration.
   absl::flat_hash_map<std::string, Filters::Common::Expr::CelStatePrototypeConstPtr>
       state_prototypes_;
+
+  proxy_wasm::AbiVersion abi_version_{proxy_wasm::AbiVersion::Unknown};
 };
 using ContextSharedPtr = std::shared_ptr<Context>;
 

source/extensions/common/wasm/foreign.cc

@@ -126,6 +126,14 @@ RegisterForeignFunction registerSetEnvoyFilterStateForeignFunction(
       return WasmResult::BadArgument;
     });
 
+RegisterForeignFunction registerClearRouteCacheForeignFunction(
+    "clear_route_cache",
+    [](WasmBase&, std::string_view, const std::function<void*(size_t size)>&) -> WasmResult {
+      auto context = static_cast<Context*>(proxy_wasm::current_context_);
+      context->clearRouteCache();
+      return WasmResult::Ok;
+    });
+
 #if defined(WASM_USE_CEL_PARSER)
 class ExpressionFactory : public Logger::Loggable<Logger::Id::wasm> {
 protected:

test/extensions/common/wasm/BUILD

@@ -131,6 +131,7 @@ envoy_cc_test(
         "//test/mocks/http:http_mocks",
         "//test/mocks/network:network_mocks",
         "//test/mocks/stream_info:stream_info_mocks",
+        "//test/test_common:test_runtime_lib",
     ],
 )
 
@@ -143,19 +144,25 @@ envoy_cc_test(
             "-DWASM_USE_CEL_PARSER",
         ],
     }),
+    data = envoy_select_wasm_cpp_tests([
+        "//test/extensions/common/wasm/test_data:test_context_cpp.wasm",
+    ]),
     rbe_pool = "6gig",
     tags = ["skip_on_windows"],
     deps = [
+        ":wasm_runtime",
         "//source/common/network:filter_state_dst_address_lib",
         "//source/common/tcp_proxy",
         "//source/extensions/clusters/original_dst:original_dst_cluster_lib",
         "//source/extensions/common/wasm:wasm_hdr",
         "//source/extensions/common/wasm:wasm_lib",
+        "//test/extensions/common/wasm/test_data:test_context_cpp_plugin",
         "//test/mocks/http:http_mocks",
         "//test/mocks/local_info:local_info_mocks",
         "//test/mocks/network:network_mocks",
         "//test/mocks/upstream:upstream_mocks",
         "//test/test_common:environment_lib",
+        "//test/test_common:wasm_lib",
         "@proxy_wasm_cpp_host//:base_lib",
     ],
 )

test/extensions/common/wasm/context_test.cc

@@ -5,6 +5,7 @@
 #include "test/mocks/http/mocks.h"
 #include "test/mocks/network/mocks.h"
 #include "test/mocks/stream_info/mocks.h"
+#include "test/test_common/test_runtime.h"
 
 #include "eval/public/cel_value.h"
 #include "gmock/gmock.h"
@@ -21,6 +22,12 @@ using google::api::expr::runtime::CelValue;
 
 class TestContext : public Context {
 public:
+  TestContext(absl::optional<proxy_wasm::AbiVersion> mock_abi_version = {}) {
+    if (mock_abi_version.has_value()) {
+      abi_version_ = mock_abi_version.value();
+    }
+  }
+
   void setEncoderFilterCallbacksPtr(Envoy::Http::StreamEncoderFilterCallbacks* cb) {
     encoder_callbacks_ = cb;
   }
@@ -247,7 +254,8 @@ TEST_F(ContextTest, FindValueTest) {
   EXPECT_FALSE(ctx_.FindValue("plugin_name", &arena).has_value());
 }
 
-TEST_F(ContextTest, ClearRouteCacheCalledInDownstreamConfiguration) {
+TEST_F(ContextTest, ClearRouteCacheCalledInDownstreamConfigurationForLegacyWasmPlugin) {
+  TestContext test_ctx(proxy_wasm::AbiVersion::ProxyWasm_0_2_1);
 
   Http::MockDownstreamStreamFilterCallbacks downstream_callbacks;
   EXPECT_CALL(downstream_callbacks, clearRouteCache()).Times(5).WillRepeatedly(testing::Return());
@@ -256,34 +264,57 @@ TEST_F(ContextTest, ClearRouteCacheCalledInDownstreamConfiguration) {
   EXPECT_CALL(decoder_callbacks, downstreamCallbacks())
       .WillRepeatedly(testing::Return(
           makeOptRef(dynamic_cast<Http::DownstreamStreamFilterCallbacks&>(downstream_callbacks))));
-  ctx_.setDecoderFilterCallbacksPtr(&decoder_callbacks);
+  test_ctx.setDecoderFilterCallbacksPtr(&decoder_callbacks);
+
+  Http::TestRequestHeaderMapImpl request_headers{{":path", "/123"}};
+  test_ctx.setRequestHeaders(&request_headers);
+
+  test_ctx.clearRouteCache();
+  test_ctx.addHeaderMapValue(WasmHeaderMapType::RequestHeaders, "key", "value");
+  test_ctx.setHeaderMapPairs(WasmHeaderMapType::RequestHeaders, Pairs{{"key2", "value2"}});
+  test_ctx.replaceHeaderMapValue(WasmHeaderMapType::RequestHeaders, "key", "value2");
+  test_ctx.removeHeaderMapValue(WasmHeaderMapType::RequestHeaders, "key");
+}
+
+TEST_F(ContextTest, NoAutoClearRouteCacheCalledInDownstreamConfiguration) {
+  TestContext test_ctx;
+
+  Http::MockDownstreamStreamFilterCallbacks downstream_callbacks;
+  EXPECT_CALL(downstream_callbacks, clearRouteCache()).WillOnce(testing::Return());
+
+  Http::MockStreamDecoderFilterCallbacks decoder_callbacks;
+  EXPECT_CALL(decoder_callbacks, downstreamCallbacks())
+      .WillRepeatedly(testing::Return(
+          makeOptRef(dynamic_cast<Http::DownstreamStreamFilterCallbacks&>(downstream_callbacks))));
+  test_ctx.setDecoderFilterCallbacksPtr(&decoder_callbacks);
 
   Http::TestRequestHeaderMapImpl request_headers{{":path", "/123"}};
-  ctx_.setRequestHeaders(&request_headers);
+  test_ctx.setRequestHeaders(&request_headers);
 
-  ctx_.clearRouteCache();
-  ctx_.addHeaderMapValue(WasmHeaderMapType::RequestHeaders, "key", "value");
-  ctx_.setHeaderMapPairs(WasmHeaderMapType::RequestHeaders, Pairs{{"key2", "value2"}});
-  ctx_.replaceHeaderMapValue(WasmHeaderMapType::RequestHeaders, "key", "value2");
-  ctx_.removeHeaderMapValue(WasmHeaderMapType::RequestHeaders, "key");
+  test_ctx.clearRouteCache();
+  test_ctx.addHeaderMapValue(WasmHeaderMapType::RequestHeaders, "key", "value");
+  test_ctx.setHeaderMapPairs(WasmHeaderMapType::RequestHeaders, Pairs{{"key2", "value2"}});
+  test_ctx.replaceHeaderMapValue(WasmHeaderMapType::RequestHeaders, "key", "value2");
+  test_ctx.removeHeaderMapValue(WasmHeaderMapType::RequestHeaders, "key");
 }
 
 TEST_F(ContextTest, ClearRouteCacheDoesNothingInUpstreamConfiguration) {
+  TestContext test_ctx;
 
   Http::MockStreamDecoderFilterCallbacks decoder_callbacks;
   EXPECT_CALL(decoder_callbacks, downstreamCallbacks())
       .WillRepeatedly(testing::Return(absl::nullopt));
-  ctx_.setDecoderFilterCallbacksPtr(&decoder_callbacks);
+  test_ctx.setDecoderFilterCallbacksPtr(&decoder_callbacks);
 
   Http::TestRequestHeaderMapImpl request_headers{{":path", "/123"}};
-  ctx_.setRequestHeaders(&request_headers);
+  test_ctx.setRequestHeaders(&request_headers);
 
   // Calling methods should be safe.
-  ctx_.clearRouteCache();
-  ctx_.addHeaderMapValue(WasmHeaderMapType::RequestHeaders, "key", "value");
-  ctx_.setHeaderMapPairs(WasmHeaderMapType::RequestHeaders, Pairs{{"key2", "value2"}});
-  ctx_.replaceHeaderMapValue(WasmHeaderMapType::RequestHeaders, "key", "value2");
-  ctx_.removeHeaderMapValue(WasmHeaderMapType::RequestHeaders, "key");
+  test_ctx.clearRouteCache();
+  test_ctx.addHeaderMapValue(WasmHeaderMapType::RequestHeaders, "key", "value");
+  test_ctx.setHeaderMapPairs(WasmHeaderMapType::RequestHeaders, Pairs{{"key2", "value2"}});
+  test_ctx.replaceHeaderMapValue(WasmHeaderMapType::RequestHeaders, "key", "value2");
+  test_ctx.removeHeaderMapValue(WasmHeaderMapType::RequestHeaders, "key");
 }
 
 } // namespace Wasm

test/extensions/common/wasm/foreign_test.cc

@@ -6,9 +6,13 @@
 #include "source/extensions/common/wasm/ext/set_envoy_filter_state.pb.h"
 #include "source/extensions/common/wasm/wasm.h"
 
+#include "test/extensions/common/wasm/wasm_runtime.h"
+#include "test/mocks/http/mocks.h"
 #include "test/mocks/local_info/mocks.h"
+#include "test/mocks/server/factory_context.h"
 #include "test/mocks/upstream/cluster_manager.h"
 #include "test/test_common/utility.h"
+#include "test/test_common/wasm_base.h"
 
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
@@ -113,6 +117,34 @@ TEST_F(ForeignTest, ForeignFunctionSetEnvoyFilterTest) {
       Upstream::OriginalDstClusterFilterStateKey));
 }
 
+class StreamForeignTest : public WasmPluginConfigTestBase<
+                              testing::TestWithParam<std::tuple<std::string, std::string>>> {
+public:
+  StreamForeignTest() = default;
+};
+
+INSTANTIATE_TEST_SUITE_P(PluginConfigRuntimes, StreamForeignTest,
+                         Envoy::Extensions::Common::Wasm::runtime_and_cpp_values,
+                         Envoy::Extensions::Common::Wasm::wasmTestParamsToString);
+
+TEST_P(StreamForeignTest, ForeignFunctionClearRouteCache) {
+  auto [runtime, language] = GetParam();
+
+  auto plugin_config = getWasmPluginConfigForTest(
+      runtime, "test/extensions/common/wasm/test_data/test_context_cpp.wasm",
+      "CommonWasmTestContextCpp", "send local reply twice");
+
+  setUp(plugin_config);
+
+  createStreamContext();
+
+  proxy_wasm::current_context_ = context_.get();
+  auto function = proxy_wasm::getForeignFunction("clear_route_cache");
+
+  EXPECT_CALL(decoder_callbacks_.downstream_callbacks_, clearRouteCache());
+  function(*plugin_config_->wasm(), "", [](size_t size) { return malloc(size); });
+}
+
 } // namespace Wasm
 } // namespace Common
 } // namespace Extensions

test/test_common/wasm_base.h

@@ -170,6 +170,70 @@ class WasmNetworkFilterTestBase : public WasmTestBase<Base> {
   NiceMock<Network::MockWriteFilterCallbacks> write_filter_callbacks_;
 };
 
+inline envoy::extensions::wasm::v3::PluginConfig
+getWasmPluginConfigForTest(absl::string_view runtime, absl::string_view wasm_file_path,
+                           absl::string_view wasm_module_name, absl::string_view plugin_root_id,
+                           bool singleton = false, absl::string_view plugin_configuration = {}) {
+
+  const std::string plugin_config_yaml = fmt::format(
+      R"EOF(
+      name: 'test_wasm_singleton_{}'
+      root_id: '{}'
+      vm_config:
+        runtime: 'envoy.wasm.runtime.{}'
+        configuration:
+          "@type": "type.googleapis.com/google.protobuf.StringValue"
+          value: '{}'
+      )EOF",
+      singleton, plugin_root_id, runtime, plugin_configuration);
+
+  envoy::extensions::wasm::v3::PluginConfig plugin_config;
+  TestUtility::loadFromYaml(plugin_config_yaml, plugin_config);
+
+  if (runtime == "null") {
+    plugin_config.mutable_vm_config()->mutable_code()->mutable_local()->set_inline_bytes(
+        std::string(wasm_module_name));
+  } else {
+    const std::string code = TestEnvironment::readFileToStringForTest(TestEnvironment::substitute(
+        absl::StrCat("{{ test_rundir }}/" + std::string(wasm_file_path))));
+    plugin_config.mutable_vm_config()->mutable_code()->mutable_local()->set_inline_bytes(code);
+  }
+
+  return plugin_config;
+}
+
+template <typename Base = testing::Test> class WasmPluginConfigTestBase : public Base {
+public:
+  WasmPluginConfigTestBase() = default;
+
+  // NOLINTNEXTLINE(readability-identifier-naming)
+  void SetUp() override { clearCodeCacheForTesting(); }
+
+  void setUp(const envoy::extensions::wasm::v3::PluginConfig plugin_config,
+             bool singleton = false) {
+    plugin_config_ = std::make_shared<PluginConfig>(
+        plugin_config, server_, server_.scope(), server_.initManager(),
+        envoy::config::core::v3::TrafficDirection::UNSPECIFIED, /*metadata=*/nullptr, singleton);
+  }
+
+  void createStreamContext() {
+    context_ = plugin_config_->createContext();
+    if (context_ != nullptr) {
+      context_->setDecoderFilterCallbacks(decoder_callbacks_);
+      context_->setEncoderFilterCallbacks(encoder_callbacks_);
+      context_->onCreate();
+    }
+  }
+
+  NiceMock<Server::Configuration::MockServerFactoryContext> server_;
+  PluginConfigSharedPtr plugin_config_;
+
+  NiceMock<Http::MockStreamDecoderFilterCallbacks> decoder_callbacks_;
+  NiceMock<Http::MockStreamEncoderFilterCallbacks> encoder_callbacks_;
+
+  std::shared_ptr<Context> context_;
+};
+
 } // namespace Wasm
 } // namespace Common
 } // namespace Extensions