From c8206abd25865031ef35e0439b4138f21a674045 Mon Sep 17 00:00:00 2001 From: wbpcode Date: Sat, 22 Aug 2026 10:42:18 +0000 Subject: [PATCH 1/3] wasm: fix multiple bug of vm key and plugin key calculation Signed-off-by: wbpcode --- api/envoy/extensions/wasm/v3/wasm.proto | 17 ++- .../wasm__vm-key-derived-from-vm-config.rst | 7 + ...m__plugin-level-capability-restriction.rst | 7 + ..._plugin-key-derived-from-plugin-config.rst | 7 + .../stat_sinks/wasm_filter/source/config.cc | 3 +- .../extensions/access_loggers/wasm/config.cc | 6 +- source/extensions/bootstrap/wasm/config.cc | 3 +- source/extensions/common/wasm/BUILD | 1 - source/extensions/common/wasm/plugin.cc | 40 ++++- source/extensions/common/wasm/plugin.h | 9 +- source/extensions/common/wasm/wasm.cc | 23 +-- source/extensions/common/wasm/wasm.h | 3 +- .../filters/http/wasm/wasm_filter.cc | 14 +- .../filters/network/wasm/wasm_filter.cc | 3 +- source/extensions/stat_sinks/wasm/config.cc | 3 +- .../bootstrap/wasm/wasm_speed_test.cc | 3 +- test/extensions/bootstrap/wasm/wasm_test.cc | 3 +- test/extensions/common/wasm/BUILD | 1 + test/extensions/common/wasm/foreign_test.cc | 6 +- test/extensions/common/wasm/plugin_test.cc | 129 ++++++++++++++++ test/extensions/common/wasm/wasm_test.cc | 144 ++++++++++++------ .../filters/network/wasm/config_test.cc | 45 +++++- test/test_common/wasm_base.h | 8 +- 23 files changed, 374 insertions(+), 111 deletions(-) create mode 100644 changelogs/current/bug_fixes/wasm__vm-key-derived-from-vm-config.rst create mode 100644 changelogs/current/deprecated/wasm__plugin-level-capability-restriction.rst create mode 100644 changelogs/current/minor_behavior_changes/wasm__plugin-key-derived-from-plugin-config.rst diff --git a/api/envoy/extensions/wasm/v3/wasm.proto b/api/envoy/extensions/wasm/v3/wasm.proto index e8fea672553a8..0bc96ceca987c 100644 --- a/api/envoy/extensions/wasm/v3/wasm.proto +++ b/api/envoy/extensions/wasm/v3/wasm.proto @@ -71,7 +71,7 @@ message SanitizationConfig { } // Configuration for a Wasm VM. -// [#next-free-field: 8] +// [#next-free-field: 9] message VmConfig { // An ID which will be used along with a hash of the wasm code (or the name of the registered Null // VM plugin) to determine which VM will be used for the plugin. All plugins which use the same @@ -137,6 +137,12 @@ message VmConfig { // .. warning:: // Envoy rejects the configuration if there's conflict of key space. EnvironmentVariables environment_variables = 7; + + // Configuration for restricting Proxy-Wasm capabilities available to modules. + // + // The restrictions are applied when the VM is created and are shared by every plugin running in + // that VM, so they are a property of the VM rather than of an individual plugin. + CapabilityRestrictionConfig capability_restriction_config = 8; } message EnvironmentVariables { @@ -192,7 +198,14 @@ message PluginConfig { ReloadConfig reload_config = 8; // Configuration for restricting Proxy-Wasm capabilities available to modules. - CapabilityRestrictionConfig capability_restriction_config = 6; + // + // This field is deprecated in favor of the :ref:`vm_config.capability_restriction_config + // ` field, because + // the restrictions are applied to the Wasm VM and are therefore shared by every plugin running in + // it. If this field is set and ``vm_config.capability_restriction_config`` is not, this field is + // used to populate it. + CapabilityRestrictionConfig capability_restriction_config = 6 + [deprecated = true, (envoy.annotations.deprecated_at_minor_version) = "3.0"]; // Whether or not to allow plugin onRequestHeaders and onResponseHeaders callbacks to return // FilterHeadersStatus::StopIteration. diff --git a/changelogs/current/bug_fixes/wasm__vm-key-derived-from-vm-config.rst b/changelogs/current/bug_fixes/wasm__vm-key-derived-from-vm-config.rst new file mode 100644 index 0000000000000..29c9c108f6c0b --- /dev/null +++ b/changelogs/current/bug_fixes/wasm__vm-key-derived-from-vm-config.rst @@ -0,0 +1,7 @@ +Fixed a bug where Wasm plugins whose :ref:`VM configurations +` differed could still share a single Wasm VM, and so +silently run with the VM configuration of whichever plugin happened to be configured first. The +:ref:`runtime ` and the :ref:`capability +restrictions ` are now +part of the VM identity, alongside the ``vm_id``, the ``configuration``, the ``code`` and the +``environment_variables``, so plugins differing in either of them no longer share a VM. diff --git a/changelogs/current/deprecated/wasm__plugin-level-capability-restriction.rst b/changelogs/current/deprecated/wasm__plugin-level-capability-restriction.rst new file mode 100644 index 0000000000000..fa113dc6c6aea --- /dev/null +++ b/changelogs/current/deprecated/wasm__plugin-level-capability-restriction.rst @@ -0,0 +1,7 @@ +The :ref:`PluginConfig.capability_restriction_config +` field is +deprecated in favor of the new :ref:`VmConfig.capability_restriction_config +` field. The +restrictions are applied when the Wasm VM is created and are shared by every plugin running in that +VM, so they are a property of the VM rather than of an individual plugin. The deprecated field keeps +working: when it is set and the VM level field is not, it is used to populate the VM level one. diff --git a/changelogs/current/minor_behavior_changes/wasm__plugin-key-derived-from-plugin-config.rst b/changelogs/current/minor_behavior_changes/wasm__plugin-key-derived-from-plugin-config.rst new file mode 100644 index 0000000000000..587c6b686c37a --- /dev/null +++ b/changelogs/current/minor_behavior_changes/wasm__plugin-key-derived-from-plugin-config.rst @@ -0,0 +1,7 @@ +The identity of a Wasm plugin (which plugin configurations share a single root context and +thread-local plugin instance inside a Wasm VM) is now derived from the whole +:ref:`plugin configuration ` instead of from the +plugin name and the traffic direction of the listener the plugin was configured on. Configurations +that differ in any field other than :ref:`vm_config +` no longer share an instance, and +identical configurations now share one regardless of the traffic direction they are configured on. diff --git a/contrib/stat_sinks/wasm_filter/source/config.cc b/contrib/stat_sinks/wasm_filter/source/config.cc index 087aeff1bcca2..5c76596762a17 100644 --- a/contrib/stat_sinks/wasm_filter/source/config.cc +++ b/contrib/stat_sinks/wasm_filter/source/config.cc @@ -29,8 +29,7 @@ WasmFilterSinkFactory::createStatsSink(const Protobuf::Message& proto_config, setGlobalTags(&startup_tags); auto plugin_config = std::make_unique( - config.wasm_config(), context, context.scope(), context.initManager(), - envoy::config::core::v3::TrafficDirection::UNSPECIFIED, true); + config.wasm_config(), context, context.scope(), context.initManager(), true); setGlobalTags(nullptr); diff --git a/source/extensions/access_loggers/wasm/config.cc b/source/extensions/access_loggers/wasm/config.cc index be53e441a243f..c88aa323c0761 100644 --- a/source/extensions/access_loggers/wasm/config.cc +++ b/source/extensions/access_loggers/wasm/config.cc @@ -23,9 +23,9 @@ WasmAccessLogFactory::createAccessLogInstance(const Protobuf::Message& proto_con const envoy::extensions::access_loggers::wasm::v3::WasmAccessLog&>( proto_config, context.messageValidationVisitor()); - auto plugin_config = std::make_unique( - config.config(), context.serverFactoryContext(), context.scope(), context.initManager(), - envoy::config::core::v3::TrafficDirection::UNSPECIFIED, false); + auto plugin_config = + std::make_unique(config.config(), context.serverFactoryContext(), + context.scope(), context.initManager(), false); auto access_log = std::make_shared(std::move(plugin_config), std::move(filter)); context.serverFactoryContext().api().customStatNamespaces().registerStatNamespace( diff --git a/source/extensions/bootstrap/wasm/config.cc b/source/extensions/bootstrap/wasm/config.cc index 5a0d1162ae032..1d9329ab17c01 100644 --- a/source/extensions/bootstrap/wasm/config.cc +++ b/source/extensions/bootstrap/wasm/config.cc @@ -17,8 +17,7 @@ void WasmServiceExtension::onServerInitialized(Server::Instance&) { createWasm(c void WasmServiceExtension::createWasm(Server::Configuration::ServerFactoryContext& context) { plugin_config_ = std::make_unique( - config_.config(), context, context.scope(), context.initManager(), - envoy::config::core::v3::TrafficDirection::UNSPECIFIED, config_.singleton()); + config_.config(), context, context.scope(), context.initManager(), config_.singleton()); } Server::BootstrapExtensionPtr diff --git a/source/extensions/common/wasm/BUILD b/source/extensions/common/wasm/BUILD index 4d5c33c1f3c5f..e5ec11f66beb1 100644 --- a/source/extensions/common/wasm/BUILD +++ b/source/extensions/common/wasm/BUILD @@ -47,7 +47,6 @@ envoy_cc_library( "//source/common/version:version_includes", "//source/extensions/filters/common/expr:cel_state_lib", "//source/extensions/filters/common/expr:evaluator_lib", - "@envoy_api//envoy/config/core/v3:pkg_cc_proto", "@envoy_api//envoy/extensions/wasm/v3:pkg_cc_proto", "@proxy-wasm-cpp-host//:headers", "@proxy-wasm-cpp-sdk//:common_lib", diff --git a/source/extensions/common/wasm/plugin.cc b/source/extensions/common/wasm/plugin.cc index ae1628981e870..1b3aa6f9457b6 100644 --- a/source/extensions/common/wasm/plugin.cc +++ b/source/extensions/common/wasm/plugin.cc @@ -2,6 +2,7 @@ #include "envoy/common/exception.h" +#include "absl/strings/str_cat.h" #include "include/proxy-wasm/wasm.h" namespace Envoy { @@ -9,8 +10,26 @@ namespace Extensions { namespace Common { namespace Wasm { -WasmConfig::WasmConfig(const envoy::extensions::wasm::v3::PluginConfig& config) : config_(config) { - for (auto& capability : config_.capability_restriction_config().allowed_capabilities()) { +envoy::extensions::wasm::v3::PluginConfig +normalizeConfig(const envoy::extensions::wasm::v3::PluginConfig& config) { + // The capability restrictions are applied when the Wasm VM is created and are shared by every + // plugin running in it, so they belong to the VM configuration. The plugin level field is + // deprecated in favor of the VM level one: honor it only when the VM configuration doesn't set + // its own restrictions. + if (!config.has_capability_restriction_config() || + config.vm_config().has_capability_restriction_config()) { + return config; + } + envoy::extensions::wasm::v3::PluginConfig normalized = config; + *normalized.mutable_vm_config()->mutable_capability_restriction_config() = + config.capability_restriction_config(); + return normalized; +} + +WasmConfig::WasmConfig(const envoy::extensions::wasm::v3::PluginConfig& config) + : config_(normalizeConfig(config)) { + for (auto& capability : + config_.vm_config().capability_restriction_config().allowed_capabilities()) { // TODO(rapilado): Set the SanitizationConfig fields once sanitization is implemented. allowed_capabilities_[capability.first] = proxy_wasm::SanitizationConfig(); } @@ -54,6 +73,23 @@ WasmConfig::WasmConfig(const envoy::extensions::wasm::v3::PluginConfig& config) } } +std::string Plugin::createPluginKey(const envoy::extensions::wasm::v3::PluginConfig& config) { + // Every field of the plugin configuration is part of the plugin identity, so that distinct + // configurations never share an instance and identical configurations always do (which keeps the + // plugin reusable across xDS updates and bounds the number of root contexts). `vm_config` is + // excluded because two plugins can only share a root context when they share a VM, and VM + // identity is the VM key that proxy-wasm prepends to this key when caching thread-local plugins. + // That key covers the fields of `vm_config` the VM is built from (see the `makeVmKey()` call in + // wasm.cc), so between the two keys the only fields left out are the ones that affect neither the + // VM nor the plugin, namely `allow_precompiled` and `nack_on_code_cache_miss`. + // + // The copy below only happens when a plugin is configured, and the plugin configuration is deep + // copied by WasmConfig anyway. + envoy::extensions::wasm::v3::PluginConfig key_config = config; + key_config.clear_vm_config(); + return absl::StrCat(config.name(), "||", MessageUtil::hash(key_config)); +} + } // namespace Wasm } // namespace Common } // namespace Extensions diff --git a/source/extensions/common/wasm/plugin.h b/source/extensions/common/wasm/plugin.h index fe22b4024bc23..9307dd14bef2d 100644 --- a/source/extensions/common/wasm/plugin.h +++ b/source/extensions/common/wasm/plugin.h @@ -2,7 +2,6 @@ #include -#include "envoy/config/core/v3/base.pb.h" #include "envoy/extensions/wasm/v3/wasm.pb.validate.h" #include "envoy/local_info/local_info.h" @@ -39,23 +38,19 @@ using WasmConfigPtr = std::unique_ptr; class Plugin : public proxy_wasm::PluginBase { public: Plugin(const envoy::extensions::wasm::v3::PluginConfig& config, - envoy::config::core::v3::TrafficDirection direction, const LocalInfo::LocalInfo& local_info) : PluginBase( config.name(), config.root_id(), config.vm_config().vm_id(), config.vm_config().runtime(), THROW_OR_RETURN_VALUE(MessageUtil::anyToBytes(config.configuration()), std::string), - config.fail_open(), createPluginKey(config, direction)), + config.fail_open(), createPluginKey(config)), local_info_(local_info), wasm_config_(std::make_unique(config)) {} const LocalInfo::LocalInfo& localInfo() { return local_info_; } WasmConfig& wasmConfig() { return *wasm_config_; } private: - static std::string createPluginKey(const envoy::extensions::wasm::v3::PluginConfig& config, - envoy::config::core::v3::TrafficDirection direction) { - return config.name() + "||" + envoy::config::core::v3::TrafficDirection_Name(direction); - } + static std::string createPluginKey(const envoy::extensions::wasm::v3::PluginConfig& config); private: const LocalInfo::LocalInfo& local_info_; diff --git a/source/extensions/common/wasm/wasm.cc b/source/extensions/common/wasm/wasm.cc index 13c666ea435cf..9083e0249cb48 100644 --- a/source/extensions/common/wasm/wasm.cc +++ b/source/extensions/common/wasm/wasm.cc @@ -374,13 +374,19 @@ bool createWasm(const PluginSharedPtr& plugin, const Stats::ScopeSharedPtr& scop .value_or(code.empty() ? EMPTY_STRING : INLINE_STRING); } - // Include environment_variables in the vm_key so that a change to env vars triggers VM - // recreation, the same way a code change does. The env vars hash is appended to vm_id (which is - // small) rather than to code (which can be O(MB)), with a separator to avoid key collisions. - const std::size_t env_vars_hash = MessageUtil::hash(vm_config.environment_variables()); - const std::string vm_id_with_env = absl::StrCat(vm_config.vm_id(), "|", env_vars_hash); + // Ideally, all fields of vm_config that affect the Wasm VM should be part of the vm_key, but + // the proxy_wasm::makeVmKey() takes the configuration and the code as separate arguments, so + // we only include the other fields here. If any other fields are added to vm_config that affect + // the Wasm VM, they should be added here. + envoy::extensions::wasm::v3::VmConfig vm_key_config; + vm_key_config.set_runtime(vm_config.runtime()); + *vm_key_config.mutable_environment_variables() = vm_config.environment_variables(); + *vm_key_config.mutable_capability_restriction_config() = + vm_config.capability_restriction_config(); + const std::string vm_id_with_config = + absl::StrCat(vm_config.vm_id(), "|", MessageUtil::hash(vm_key_config)); auto vm_key = proxy_wasm::makeVmKey( - vm_id_with_env, + vm_id_with_config, THROW_OR_RETURN_VALUE(MessageUtil::anyToBytes(vm_config.configuration()), std::string), code); auto complete_cb = [cb, vm_key, plugin, scope, &api, &cluster_manager, &dispatcher, &lifecycle_notifier, create_root_context_for_testing, @@ -567,8 +573,7 @@ std::pair, Wasm*> PluginConfig::getPlug PluginConfig::PluginConfig(const envoy::extensions::wasm::v3::PluginConfig& config, Server::Configuration::ServerFactoryContext& context, - Stats::Scope& scope, Init::Manager& init_manager, - envoy::config::core::v3::TrafficDirection direction, bool singleton) + Stats::Scope& scope, Init::Manager& init_manager, bool singleton) : is_singleton_handle_(singleton) { if (config.fail_open()) { @@ -612,7 +617,7 @@ PluginConfig::PluginConfig(const envoy::extensions::wasm::v3::PluginConfig& conf } stats_handler_ = std::make_shared(scope, absl::StrCat("wasm.", config.name(), ".")); - plugin_ = std::make_shared(config, direction, context.localInfo()); + plugin_ = std::make_shared(config, context.localInfo()); auto callback = [this, &context](WasmHandleSharedPtr base_wasm) { base_wasm_ = base_wasm; diff --git a/source/extensions/common/wasm/wasm.h b/source/extensions/common/wasm/wasm.h index f3f706078c68e..01a6041bf12f3 100644 --- a/source/extensions/common/wasm/wasm.h +++ b/source/extensions/common/wasm/wasm.h @@ -191,8 +191,7 @@ class PluginConfig : Logger::Loggable { // the type of the plugin if needed. PluginConfig(const envoy::extensions::wasm::v3::PluginConfig& config, Server::Configuration::ServerFactoryContext& context, Stats::Scope& scope, - Init::Manager& init_manager, envoy::config::core::v3::TrafficDirection direction, - bool singleton); + Init::Manager& init_manager, bool singleton); std::shared_ptr createContext(); Wasm* wasm(); diff --git a/source/extensions/filters/http/wasm/wasm_filter.cc b/source/extensions/filters/http/wasm/wasm_filter.cc index 3dc85fdc65b9e..387fe57a94112 100644 --- a/source/extensions/filters/http/wasm/wasm_filter.cc +++ b/source/extensions/filters/http/wasm/wasm_filter.cc @@ -22,20 +22,18 @@ Stats::Scope& upstreamWasmStatsScope(Server::Configuration::UpstreamFactoryConte FilterConfig::FilterConfig(const envoy::extensions::filters::http::wasm::v3::Wasm& config, Server::Configuration::FactoryContext& context) : Extensions::Common::Wasm::PluginConfig(config.config(), context.serverFactoryContext(), - context.scope(), context.initManager(), - context.direction(), false) {} + context.scope(), context.initManager(), false) {} FilterConfig::FilterConfig(const envoy::extensions::filters::http::wasm::v3::Wasm& config, Server::Configuration::UpstreamFactoryContext& context) - : Extensions::Common::Wasm::PluginConfig( - config.config(), context.serverFactoryContext(), upstreamWasmStatsScope(context), - context.initManager(), envoy::config::core::v3::TrafficDirection::OUTBOUND, false) {} + : Extensions::Common::Wasm::PluginConfig(config.config(), context.serverFactoryContext(), + upstreamWasmStatsScope(context), context.initManager(), + false) {} FilterConfig::FilterConfig(const envoy::extensions::filters::http::wasm::v3::Wasm& config, Server::Configuration::ServerFactoryContext& context) - : Extensions::Common::Wasm::PluginConfig( - config.config(), context, context.scope(), context.initManager(), - envoy::config::core::v3::TrafficDirection::OUTBOUND, false) {} + : Extensions::Common::Wasm::PluginConfig(config.config(), context, context.scope(), + context.initManager(), false) {} } // namespace Wasm } // namespace HttpFilters diff --git a/source/extensions/filters/network/wasm/wasm_filter.cc b/source/extensions/filters/network/wasm/wasm_filter.cc index 71bb798823c0a..8401e823eb4ca 100644 --- a/source/extensions/filters/network/wasm/wasm_filter.cc +++ b/source/extensions/filters/network/wasm/wasm_filter.cc @@ -8,8 +8,7 @@ namespace Wasm { FilterConfig::FilterConfig(const envoy::extensions::filters::network::wasm::v3::Wasm& config, Server::Configuration::FactoryContext& context) : Extensions::Common::Wasm::PluginConfig(config.config(), context.serverFactoryContext(), - context.scope(), context.initManager(), - context.direction(), false) {} + context.scope(), context.initManager(), false) {} } // namespace Wasm } // namespace NetworkFilters diff --git a/source/extensions/stat_sinks/wasm/config.cc b/source/extensions/stat_sinks/wasm/config.cc index 81f59d80b2fb8..7168d55fdb171 100644 --- a/source/extensions/stat_sinks/wasm/config.cc +++ b/source/extensions/stat_sinks/wasm/config.cc @@ -22,8 +22,7 @@ WasmSinkFactory::createStatsSink(const Protobuf::Message& proto_config, proto_config, context.messageValidationContext().staticValidationVisitor()); auto plugin_config = std::make_unique( - config.config(), context, context.scope(), context.initManager(), - envoy::config::core::v3::TrafficDirection::UNSPECIFIED, true); + config.config(), context, context.scope(), context.initManager(), true); context.api().customStatNamespaces().registerStatNamespace( Extensions::Common::Wasm::CustomStatNamespace); diff --git a/test/extensions/bootstrap/wasm/wasm_speed_test.cc b/test/extensions/bootstrap/wasm/wasm_speed_test.cc index 4725107273366..5ae8c12590329 100644 --- a/test/extensions/bootstrap/wasm/wasm_speed_test.cc +++ b/test/extensions/bootstrap/wasm/wasm_speed_test.cc @@ -54,8 +54,7 @@ static void bmWasmSimpleCallSpeedTest(benchmark::State& state, std::string test, *plugin_config.mutable_root_id() = "some_long_root_id"; plugin_config.mutable_vm_config()->mutable_configuration()->set_value(test); plugin_config.mutable_vm_config()->set_runtime(absl::StrCat("envoy.wasm.runtime.", runtime)); - auto plugin = std::make_shared( - plugin_config, envoy::config::core::v3::TrafficDirection::UNSPECIFIED, local_info); + auto plugin = std::make_shared(plugin_config, local_info); auto wasm = std::make_unique( plugin->wasmConfig(), "vm_key", scope, *api, cluster_manager, *dispatcher); std::string code; diff --git a/test/extensions/bootstrap/wasm/wasm_test.cc b/test/extensions/bootstrap/wasm/wasm_test.cc index b9ba396694e57..bcb6ae7f65410 100644 --- a/test/extensions/bootstrap/wasm/wasm_test.cc +++ b/test/extensions/bootstrap/wasm/wasm_test.cc @@ -51,8 +51,7 @@ class WasmTestBase { *plugin_config.mutable_vm_config()->mutable_vm_id() = vm_id_; plugin_config.mutable_vm_config()->mutable_configuration()->set_value(vm_configuration_); plugin_config.mutable_configuration()->set_value(plugin_configuration_); - plugin_ = std::make_shared( - plugin_config, envoy::config::core::v3::TrafficDirection::UNSPECIFIED, local_info_); + plugin_ = std::make_shared(plugin_config, local_info_); auto config = plugin_->wasmConfig(); config.allowedCapabilities() = allowed_capabilities_; config.environmentVariables() = envs_; diff --git a/test/extensions/common/wasm/BUILD b/test/extensions/common/wasm/BUILD index 0b56120339c7a..0c2033cca526e 100644 --- a/test/extensions/common/wasm/BUILD +++ b/test/extensions/common/wasm/BUILD @@ -75,6 +75,7 @@ envoy_cc_test( tags = ["skip_on_windows"], deps = [ "//source/extensions/common/wasm:wasm_lib", + "//test/mocks/local_info:local_info_mocks", "//test/test_common:environment_lib", ], ) diff --git a/test/extensions/common/wasm/foreign_test.cc b/test/extensions/common/wasm/foreign_test.cc index 9596d72ed4434..b4a2c974d69fb 100644 --- a/test/extensions/common/wasm/foreign_test.cc +++ b/test/extensions/common/wasm/foreign_test.cc @@ -46,8 +46,7 @@ TEST_F(ForeignTest, ForeignFunctionEdgeCaseTest) { testing::NiceMock local_info; envoy::extensions::wasm::v3::PluginConfig plugin_config; - auto plugin = std::make_shared( - plugin_config, envoy::config::core::v3::TrafficDirection::UNSPECIFIED, local_info); + auto plugin = std::make_shared(plugin_config, local_info); Wasm wasm(plugin->wasmConfig(), "", scope, *api, cluster_manager, *dispatcher); proxy_wasm::current_context_ = &ctx_; @@ -74,8 +73,7 @@ TEST_F(ForeignTest, ForeignFunctionSetEnvoyFilterTest) { testing::NiceMock local_info; envoy::extensions::wasm::v3::PluginConfig plugin_config; - auto plugin = std::make_shared( - plugin_config, envoy::config::core::v3::TrafficDirection::UNSPECIFIED, local_info); + auto plugin = std::make_shared(plugin_config, local_info); Wasm wasm(plugin->wasmConfig(), "", scope, *api, cluster_manager, *dispatcher); proxy_wasm::current_context_ = &ctx_; diff --git a/test/extensions/common/wasm/plugin_test.cc b/test/extensions/common/wasm/plugin_test.cc index 4fba43bb94cf2..46c03289bd642 100644 --- a/test/extensions/common/wasm/plugin_test.cc +++ b/test/extensions/common/wasm/plugin_test.cc @@ -4,6 +4,7 @@ #include "source/extensions/common/wasm/plugin.h" +#include "test/mocks/local_info/mocks.h" #include "test/test_common/environment.h" #include "test/test_common/utility.h" @@ -12,6 +13,8 @@ using testing::Contains; using testing::Key; +using testing::NiceMock; +using testing::Not; namespace Envoy { namespace Extensions { @@ -46,6 +49,60 @@ TEST(TestWasmConfig, Basic) { EXPECT_EQ(envs[key], value); } +// The capability restrictions are read from the VM configuration. +TEST(TestWasmConfig, VmLevelCapabilityRestriction) { + envoy::extensions::wasm::v3::PluginConfig plugin_config; + plugin_config.mutable_vm_config() + ->mutable_capability_restriction_config() + ->mutable_allowed_capabilities() + ->insert({"proxy_log", envoy::extensions::wasm::v3::SanitizationConfig()}); + + auto wasm_config = WasmConfig(plugin_config); + EXPECT_THAT(wasm_config.allowedCapabilities(), Contains(Key("proxy_log"))); + EXPECT_THAT( + wasm_config.config().vm_config().capability_restriction_config().allowed_capabilities(), + Contains(Key("proxy_log"))); +} + +// The deprecated plugin level capability restrictions are copied into the VM configuration. +TEST(TestWasmConfig, DeprecatedPluginLevelCapabilityRestriction) { + envoy::extensions::wasm::v3::PluginConfig plugin_config; + plugin_config.mutable_capability_restriction_config()->mutable_allowed_capabilities()->insert( + {"proxy_log", envoy::extensions::wasm::v3::SanitizationConfig()}); + + auto wasm_config = WasmConfig(plugin_config); + EXPECT_THAT(wasm_config.allowedCapabilities(), Contains(Key("proxy_log"))); + EXPECT_THAT( + wasm_config.config().vm_config().capability_restriction_config().allowed_capabilities(), + Contains(Key("proxy_log"))); +} + +// The VM level capability restrictions win when both are set. +TEST(TestWasmConfig, VmLevelCapabilityRestrictionWins) { + envoy::extensions::wasm::v3::PluginConfig plugin_config; + plugin_config.mutable_capability_restriction_config()->mutable_allowed_capabilities()->insert( + {"proxy_log", envoy::extensions::wasm::v3::SanitizationConfig()}); + plugin_config.mutable_vm_config() + ->mutable_capability_restriction_config() + ->mutable_allowed_capabilities() + ->insert({"proxy_on_vm_start", envoy::extensions::wasm::v3::SanitizationConfig()}); + + auto wasm_config = WasmConfig(plugin_config); + EXPECT_THAT(wasm_config.allowedCapabilities(), Contains(Key("proxy_on_vm_start"))); + EXPECT_THAT(wasm_config.allowedCapabilities(), Not(Contains(Key("proxy_log")))); +} + +// A configuration without any capability restriction leaves the VM unrestricted, and no empty VM +// configuration is materialized. +TEST(TestWasmConfig, NoCapabilityRestriction) { + envoy::extensions::wasm::v3::PluginConfig plugin_config; + plugin_config.set_name("my-plugin"); + + auto wasm_config = WasmConfig(plugin_config); + EXPECT_TRUE(wasm_config.allowedCapabilities().empty()); + EXPECT_FALSE(wasm_config.config().has_vm_config()); +} + TEST(TestWasmConfig, EnvKeyException) { { // Duplication in host_env_keys. @@ -88,6 +145,78 @@ TEST(TestWasmConfig, NullVMEnv) { "not be set for NullVm."); } +class PluginKeyTest : public testing::Test { +protected: + std::string key(const envoy::extensions::wasm::v3::PluginConfig& config) { + return Plugin(config, local_info_).key(); + } + + envoy::extensions::wasm::v3::PluginConfig baseConfig() { + envoy::extensions::wasm::v3::PluginConfig config; + config.set_name("my-plugin"); + config.set_root_id("my-root"); + config.mutable_vm_config()->set_runtime("envoy.wasm.runtime.null"); + config.mutable_configuration()->set_value("plugin-configuration"); + return config; + } + + NiceMock local_info_; +}; + +// Any difference in the plugin configuration produces a distinct plugin identity, including the +// fields proxy-wasm does not hash into the plugin identity itself. +TEST_F(PluginKeyTest, ConfigDifferencesProduceDistinctKeys) { + const auto config = baseConfig(); + const std::string base_key = key(config); + + { + auto other = config; + other.set_name("other-plugin"); + EXPECT_NE(base_key, key(other)); + } + { + auto other = config; + other.set_root_id("other-root"); + EXPECT_NE(base_key, key(other)); + } + { + auto other = config; + other.mutable_configuration()->set_value("other-configuration"); + EXPECT_NE(base_key, key(other)); + } + { + auto other = config; + other.set_failure_policy(envoy::extensions::wasm::v3::FailurePolicy::FAIL_OPEN); + EXPECT_NE(base_key, key(other)); + } + { + auto other = config; + other.mutable_allow_on_headers_stop_iteration()->set_value(true); + EXPECT_NE(base_key, key(other)); + } + { + auto other = config; + other.mutable_capability_restriction_config()->mutable_allowed_capabilities()->insert( + {"proxy_log", envoy::extensions::wasm::v3::SanitizationConfig()}); + EXPECT_NE(base_key, key(other)); + } + { + auto other = config; + other.mutable_reload_config()->mutable_backoff()->mutable_base_interval()->set_seconds(30); + EXPECT_NE(base_key, key(other)); + } +} + +// The VM configuration is not part of the plugin key: VM identity is covered by the VM key, which +// is prepended to the plugin key when caching thread-local plugins. +TEST_F(PluginKeyTest, VmConfigIsNotPartOfTheKey) { + const auto config = baseConfig(); + auto other = config; + other.mutable_vm_config()->set_vm_id("other-vm"); + other.mutable_vm_config()->mutable_code()->mutable_local()->set_inline_string("code"); + EXPECT_EQ(key(config), key(other)); +} + } // namespace } // namespace Wasm } // namespace Common diff --git a/test/extensions/common/wasm/wasm_test.cc b/test/extensions/common/wasm/wasm_test.cc index 65f4ac29dbe57..9e51215b8da21 100644 --- a/test/extensions/common/wasm/wasm_test.cc +++ b/test/extensions/common/wasm/wasm_test.cc @@ -99,8 +99,7 @@ INSTANTIATE_TEST_SUITE_P(Runtimes, WasmCommonTest, TEST_P(WasmCommonTest, WasmFailState) { envoy::extensions::wasm::v3::PluginConfig plugin_config; - auto plugin = std::make_shared( - plugin_config, envoy::config::core::v3::TrafficDirection::UNSPECIFIED, local_info_); + auto plugin = std::make_shared(plugin_config, local_info_); auto wasm = std::make_shared(std::make_unique( plugin->wasmConfig(), "", scope_, *api_, cluster_manager_, *dispatcher_)); @@ -174,8 +173,7 @@ TEST_P(WasmCommonTest, Logging) { code = "CommonWasmTestCpp"; } EXPECT_FALSE(code.empty()); - auto plugin = std::make_shared( - plugin_config, envoy::config::core::v3::TrafficDirection::UNSPECIFIED, local_info_); + auto plugin = std::make_shared(plugin_config, local_info_); auto vm_key = proxy_wasm::makeVmKey("", vm_configuration, code); auto wasm = std::make_shared( @@ -238,8 +236,7 @@ TEST_P(WasmCommonTest, BadSignature) { envoy::extensions::wasm::v3::PluginConfig plugin_config; *plugin_config.mutable_vm_config()->mutable_runtime() = absl::StrCat("envoy.wasm.runtime.", std::get<0>(GetParam())); - auto plugin = std::make_shared( - plugin_config, envoy::config::core::v3::TrafficDirection::UNSPECIFIED, local_info_); + auto plugin = std::make_shared(plugin_config, local_info_); auto vm_key = proxy_wasm::makeVmKey("", "", code); auto wasm = std::make_unique( plugin->wasmConfig(), vm_key, scope_, *api_, cluster_manager_, *dispatcher_); @@ -267,8 +264,7 @@ TEST_P(WasmCommonTest, Segv) { *plugin_config.mutable_vm_config()->mutable_runtime() = absl::StrCat("envoy.wasm.runtime.", std::get<0>(GetParam())); plugin_config.mutable_vm_config()->mutable_configuration()->set_value(vm_configuration); - auto plugin = std::make_shared( - plugin_config, envoy::config::core::v3::TrafficDirection::UNSPECIFIED, local_info_); + auto plugin = std::make_shared(plugin_config, local_info_); auto vm_key = proxy_wasm::makeVmKey("", vm_configuration, code); auto wasm = std::make_unique( plugin->wasmConfig(), vm_key, scope_, *api_, cluster_manager_, *dispatcher_); @@ -310,8 +306,7 @@ TEST_P(WasmCommonTest, DivByZero) { const auto code = TestEnvironment::readFileToStringForTest(TestEnvironment::substitute( "{{ test_rundir }}/test/extensions/common/wasm/test_data/test_cpp.wasm")); EXPECT_FALSE(code.empty()); - auto plugin = std::make_shared( - plugin_config, envoy::config::core::v3::TrafficDirection::UNSPECIFIED, local_info_); + auto plugin = std::make_shared(plugin_config, local_info_); auto vm_key = proxy_wasm::makeVmKey("", vm_configuration, code); auto wasm = std::make_unique( plugin->wasmConfig(), vm_key, scope_, *api_, cluster_manager_, *dispatcher_); @@ -348,8 +343,7 @@ TEST_P(WasmCommonTest, IntrinsicGlobals) { code = "CommonWasmTestCpp"; } EXPECT_FALSE(code.empty()); - auto plugin = std::make_shared( - plugin_config, envoy::config::core::v3::TrafficDirection::UNSPECIFIED, local_info_); + auto plugin = std::make_shared(plugin_config, local_info_); auto vm_key = proxy_wasm::makeVmKey("", vm_configuration, code); auto wasm = std::make_unique( plugin->wasmConfig(), vm_key, scope_, *api_, cluster_manager_, *dispatcher_); @@ -383,8 +377,7 @@ TEST_P(WasmCommonTest, Utilities) { code = "CommonWasmTestCpp"; } EXPECT_FALSE(code.empty()); - auto plugin = std::make_shared( - plugin_config, envoy::config::core::v3::TrafficDirection::UNSPECIFIED, local_info_); + auto plugin = std::make_shared(plugin_config, local_info_); auto vm_key = proxy_wasm::makeVmKey("", vm_configuration, code); auto wasm = std::make_unique( plugin->wasmConfig(), vm_key, scope_, *api_, cluster_manager_, *dispatcher_); @@ -446,8 +439,7 @@ TEST_P(WasmCommonTest, Stats) { code = "CommonWasmTestCpp"; } EXPECT_FALSE(code.empty()); - auto plugin = std::make_shared( - plugin_config, envoy::config::core::v3::TrafficDirection::UNSPECIFIED, local_info_); + auto plugin = std::make_shared(plugin_config, local_info_); auto vm_key = proxy_wasm::makeVmKey("", vm_configuration, code); auto wasm = std::make_unique( plugin->wasmConfig(), vm_key, scope_, *api_, cluster_manager_, *dispatcher_); @@ -489,8 +481,7 @@ TEST_P(WasmCommonTest, WasmVmCountGauge) { code = "CommonWasmTestCpp"; } EXPECT_FALSE(code.empty()); - auto plugin = std::make_shared( - plugin_config, envoy::config::core::v3::TrafficDirection::UNSPECIFIED, local_info_); + auto plugin = std::make_shared(plugin_config, local_info_); auto vm_key = proxy_wasm::makeVmKey("", vm_configuration, code); EXPECT_EQ(nullptr, TestUtility::findGauge(stats_store_, "wasm.wasm_vm_count")); @@ -514,8 +505,7 @@ TEST_P(WasmCommonTest, Foreign) { absl::StrCat("envoy.wasm.runtime.", std::get<0>(GetParam())); plugin_config.mutable_vm_config()->mutable_configuration()->set_value(vm_configuration); - auto plugin = std::make_shared( - plugin_config, envoy::config::core::v3::TrafficDirection::UNSPECIFIED, local_info_); + auto plugin = std::make_shared(plugin_config, local_info_); auto wasm = std::make_unique( plugin->wasmConfig(), "", scope_, *api_, cluster_manager_, *dispatcher_); EXPECT_NE(wasm, nullptr); @@ -552,8 +542,7 @@ TEST_P(WasmCommonTest, OnForeign) { absl::StrCat("envoy.wasm.runtime.", std::get<0>(GetParam())); plugin_config.mutable_vm_config()->mutable_configuration()->set_value(vm_configuration); - auto plugin = std::make_shared( - plugin_config, envoy::config::core::v3::TrafficDirection::UNSPECIFIED, local_info_); + auto plugin = std::make_shared(plugin_config, local_info_); proxy_wasm::AllowedCapabilitiesMap allowed_capabilities; auto wasm = std::make_unique( plugin->wasmConfig(), "", scope_, *api_, cluster_manager_, *dispatcher_); @@ -594,8 +583,7 @@ TEST_P(WasmCommonTest, WASI) { absl::StrCat("envoy.wasm.runtime.", std::get<0>(GetParam())); plugin_config.mutable_vm_config()->mutable_configuration()->set_value(vm_configuration); - auto plugin = std::make_shared( - plugin_config, envoy::config::core::v3::TrafficDirection::UNSPECIFIED, local_info_); + auto plugin = std::make_shared(plugin_config, local_info_); auto wasm = std::make_unique( plugin->wasmConfig(), "", scope_, *api_, cluster_manager_, *dispatcher_); @@ -647,8 +635,7 @@ TEST_P(WasmCommonTest, VmCache) { } EXPECT_FALSE(code.empty()); vm_config->mutable_code()->mutable_local()->set_inline_bytes(code); - auto plugin = std::make_shared( - plugin_config, envoy::config::core::v3::TrafficDirection::UNSPECIFIED, local_info_); + auto plugin = std::make_shared(plugin_config, local_info_); WasmHandleSharedPtr wasm_handle; createWasm(plugin, scope_, cluster_manager_, init_manager, *dispatcher_, *api_, @@ -714,8 +701,7 @@ TEST_P(WasmCommonTest, VmCacheEnvVarChange) { EXPECT_FALSE(code.empty()); vm_config->mutable_code()->mutable_local()->set_inline_bytes(code); - auto plugin1 = std::make_shared( - plugin_config, envoy::config::core::v3::TrafficDirection::UNSPECIFIED, local_info_); + auto plugin1 = std::make_shared(plugin_config, local_info_); WasmHandleSharedPtr wasm_handle1; createWasm(plugin1, scope_, cluster_manager_, init_manager, *dispatcher_, *api_, lifecycle_notifier_, remote_data_provider_, @@ -724,8 +710,7 @@ TEST_P(WasmCommonTest, VmCacheEnvVarChange) { // Same config but with an env var added — must produce a new VM, not a cache hit. (*vm_config->mutable_environment_variables()->mutable_key_values())["K"] = "V"; - auto plugin2 = std::make_shared( - plugin_config, envoy::config::core::v3::TrafficDirection::UNSPECIFIED, local_info_); + auto plugin2 = std::make_shared(plugin_config, local_info_); WasmHandleSharedPtr wasm_handle2; createWasm(plugin2, scope_, cluster_manager_, init_manager, *dispatcher_, *api_, lifecycle_notifier_, remote_data_provider_, @@ -736,6 +721,76 @@ TEST_P(WasmCommonTest, VmCacheEnvVarChange) { proxy_wasm::clearWasmCachesForTesting(); } +// The fields of `vm_config` the VM is built from are part of the VM identity: plugins differing in +// any of them must not share a VM, because the VM is created and configured by whichever plugin +// creates it first. The fields that only govern how the code is loaded are not. +TEST_P(WasmCommonTest, VmCacheVmConfigChange) { + // NullVm has no separate VM configuration to vary. + if (std::get<0>(GetParam()) == "null") { + return; + } + NiceMock init_manager; + + const std::string code = TestEnvironment::readFileToStringForTest(TestEnvironment::substitute( + "{{ test_rundir }}/test/extensions/common/wasm/test_data/test_cpp.wasm")); + EXPECT_FALSE(code.empty()); + + envoy::extensions::wasm::v3::PluginConfig plugin_config; + auto* base_vm_config = plugin_config.mutable_vm_config(); + base_vm_config->set_runtime(absl::StrCat("envoy.wasm.runtime.", std::get<0>(GetParam()))); + base_vm_config->mutable_code()->mutable_local()->set_inline_bytes(code); + + auto create = [&](const envoy::extensions::wasm::v3::PluginConfig& config) { + auto plugin = std::make_shared(config, local_info_); + WasmHandleSharedPtr wasm_handle; + createWasm(plugin, scope_, cluster_manager_, init_manager, *dispatcher_, *api_, + lifecycle_notifier_, remote_data_provider_, + [&wasm_handle](const WasmHandleSharedPtr& w) { wasm_handle = w; }); + return wasm_handle; + }; + + // Held for the whole test: the base Wasm cache keeps only a weak reference to it. + auto base_handle = create(plugin_config); + EXPECT_NE(base_handle, nullptr); + EXPECT_EQ(create(plugin_config), base_handle); + + // `allow_precompiled` and `nack_on_code_cache_miss` govern how the code is loaded and fetched, + // not what the VM ends up being, so they must not split the VM cache. + { + auto config = plugin_config; + config.mutable_vm_config()->set_allow_precompiled(true); + EXPECT_EQ(create(config), base_handle); + } + { + auto config = plugin_config; + config.mutable_vm_config()->set_nack_on_code_cache_miss(true); + EXPECT_EQ(create(config), base_handle); + } + + // Capability restrictions are applied when the VM is created, so a restricted plugin must never + // end up on the unrestricted VM. Only distinctness is asserted: whether the restricted VM starts + // depends on which capabilities the test module needs, but a VM key that ignored the restrictions + // would have returned `base_handle` itself. + { + auto config = plugin_config; + config.mutable_vm_config() + ->mutable_capability_restriction_config() + ->mutable_allowed_capabilities() + ->insert({"proxy_log", envoy::extensions::wasm::v3::SanitizationConfig()}); + EXPECT_NE(create(config), base_handle); + } + // The deprecated plugin level field is normalized onto the VM configuration, so it must be + // covered by the VM key as well. + { + auto config = plugin_config; + config.mutable_capability_restriction_config()->mutable_allowed_capabilities()->insert( + {"proxy_log", envoy::extensions::wasm::v3::SanitizationConfig()}); + EXPECT_NE(create(config), base_handle); + } + + proxy_wasm::clearWasmCachesForTesting(); +} + TEST_P(WasmCommonTest, RemoteCode) { if (std::get<0>(GetParam()) == "null") { return; @@ -774,8 +829,7 @@ TEST_P(WasmCommonTest, RemoteCode) { vm_config->mutable_code()->mutable_remote()->mutable_http_uri()->mutable_timeout()->set_seconds( 5); - plugin = std::make_shared( - plugin_config, envoy::config::core::v3::TrafficDirection::UNSPECIFIED, local_info_); + plugin = std::make_shared(plugin_config, local_info_); } WasmHandleSharedPtr wasm_handle; @@ -881,8 +935,7 @@ TEST_P(WasmCommonTest, RemoteCodeMultipleRetry) { ->mutable_retry_policy() ->mutable_num_retries() ->set_value(num_retries); - auto plugin = std::make_shared( - plugin_config, envoy::config::core::v3::TrafficDirection::UNSPECIFIED, local_info_); + auto plugin = std::make_shared(plugin_config, local_info_); WasmHandleSharedPtr wasm_handle; NiceMock client; @@ -971,8 +1024,7 @@ TEST_P(WasmCommonTest, RestrictCapabilities) { const auto code = TestEnvironment::readFileToStringForTest(TestEnvironment::substitute( "{{ test_rundir }}/test/extensions/common/wasm/test_data/test_restriction_cpp.wasm")); EXPECT_FALSE(code.empty()); - auto plugin = std::make_shared( - plugin_config, envoy::config::core::v3::TrafficDirection::UNSPECIFIED, local_info_); + auto plugin = std::make_shared(plugin_config, local_info_); auto vm_key = proxy_wasm::makeVmKey("", vm_configuration, code); // restriction enforced if allowed_capabilities is non-empty @@ -1019,8 +1071,7 @@ TEST_P(WasmCommonTest, AllowOnVmStart) { const auto code = TestEnvironment::readFileToStringForTest(TestEnvironment::substitute( "{{ test_rundir }}/test/extensions/common/wasm/test_data/test_restriction_cpp.wasm")); EXPECT_FALSE(code.empty()); - auto plugin = std::make_shared( - plugin_config, envoy::config::core::v3::TrafficDirection::UNSPECIFIED, local_info_); + auto plugin = std::make_shared(plugin_config, local_info_); auto vm_key = proxy_wasm::makeVmKey("", vm_configuration, code); proxy_wasm::AllowedCapabilitiesMap allowed_capabilities{ {"proxy_on_vm_start", proxy_wasm::SanitizationConfig()}}; @@ -1069,8 +1120,7 @@ TEST_P(WasmCommonTest, AllowLog) { const auto code = TestEnvironment::readFileToStringForTest(TestEnvironment::substitute( "{{ test_rundir }}/test/extensions/common/wasm/test_data/test_restriction_cpp.wasm")); EXPECT_FALSE(code.empty()); - auto plugin = std::make_shared( - plugin_config, envoy::config::core::v3::TrafficDirection::UNSPECIFIED, local_info_); + auto plugin = std::make_shared(plugin_config, local_info_); auto vm_key = proxy_wasm::makeVmKey("", vm_configuration, code); proxy_wasm::AllowedCapabilitiesMap allowed_capabilities{ {"proxy_on_vm_start", proxy_wasm::SanitizationConfig()}, @@ -1116,8 +1166,7 @@ TEST_P(WasmCommonTest, AllowWASI) { const auto code = TestEnvironment::readFileToStringForTest(TestEnvironment::substitute( "{{ test_rundir }}/test/extensions/common/wasm/test_data/test_restriction_cpp.wasm")); EXPECT_FALSE(code.empty()); - auto plugin = std::make_shared( - plugin_config, envoy::config::core::v3::TrafficDirection::UNSPECIFIED, local_info_); + auto plugin = std::make_shared(plugin_config, local_info_); auto vm_key = proxy_wasm::makeVmKey("", vm_configuration, code); proxy_wasm::AllowedCapabilitiesMap allowed_capabilities{ {"proxy_on_vm_start", proxy_wasm::SanitizationConfig()}, @@ -1163,8 +1212,7 @@ TEST_P(WasmCommonTest, AllowOnContextCreate) { const auto code = TestEnvironment::readFileToStringForTest(TestEnvironment::substitute( "{{ test_rundir }}/test/extensions/common/wasm/test_data/test_restriction_cpp.wasm")); EXPECT_FALSE(code.empty()); - auto plugin = std::make_shared( - plugin_config, envoy::config::core::v3::TrafficDirection::UNSPECIFIED, local_info_); + auto plugin = std::make_shared(plugin_config, local_info_); auto vm_key = proxy_wasm::makeVmKey("", vm_configuration, code); proxy_wasm::AllowedCapabilitiesMap allowed_capabilities{ {"proxy_on_vm_start", proxy_wasm::SanitizationConfig()}, @@ -1212,8 +1260,7 @@ TEST_P(WasmCommonTest, ThreadLocalCopyRetainsEnforcement) { const auto code = TestEnvironment::readFileToStringForTest(TestEnvironment::substitute( "{{ test_rundir }}/test/extensions/common/wasm/test_data/test_restriction_cpp.wasm")); EXPECT_FALSE(code.empty()); - auto plugin = std::make_shared( - plugin_config, envoy::config::core::v3::TrafficDirection::UNSPECIFIED, local_info_); + auto plugin = std::make_shared(plugin_config, local_info_); auto vm_key = proxy_wasm::makeVmKey("", vm_configuration, code); proxy_wasm::AllowedCapabilitiesMap allowed_capabilities{ {"proxy_on_vm_start", proxy_wasm::SanitizationConfig()}, @@ -1556,9 +1603,8 @@ class PluginConfigTest : public testing::TestWithParam( - plugin_config, server_, server_.scope(), server_.initManager(), - envoy::config::core::v3::TrafficDirection::UNSPECIFIED, singleton); + plugin_config_ = std::make_shared(plugin_config, server_, server_.scope(), + server_.initManager(), singleton); } void createContext() { diff --git a/test/extensions/filters/network/wasm/config_test.cc b/test/extensions/filters/network/wasm/config_test.cc index 56743546ef1bf..d61f2001a7892 100644 --- a/test/extensions/filters/network/wasm/config_test.cc +++ b/test/extensions/filters/network/wasm/config_test.cc @@ -279,8 +279,8 @@ TEST_P(WasmNetworkFilterConfigTest, FilterConfigCapabilitiesUnrestrictedByDefaul code: local: filename: "{{ test_rundir }}/test/extensions/filters/network/wasm/test_data/test_cpp.wasm" - capability_restriction_config: - allowed_capabilities: + capability_restriction_config: + allowed_capabilities: )EOF")); envoy::extensions::filters::network::wasm::v3::Wasm proto_config; @@ -295,6 +295,37 @@ TEST_P(WasmNetworkFilterConfigTest, FilterConfigCapabilitiesUnrestrictedByDefaul } TEST_P(WasmNetworkFilterConfigTest, FilterConfigCapabilityRestriction) { + if (std::get<0>(GetParam()) == "null") { + return; + } + const std::string yaml = TestEnvironment::substitute(absl::StrCat(R"EOF( + config: + vm_config: + runtime: "envoy.wasm.runtime.)EOF", + std::get<0>(GetParam()), R"EOF(" + code: + local: + filename: "{{ test_rundir }}/test/extensions/filters/network/wasm/test_data/test_cpp.wasm" + capability_restriction_config: + allowed_capabilities: + proxy_log: + proxy_on_new_connection: + )EOF")); + + envoy::extensions::filters::network::wasm::v3::Wasm proto_config; + TestUtility::loadFromYaml(yaml, proto_config); + NetworkFilters::Wasm::FilterConfig filter_config(proto_config, context_); + auto wasm = filter_config.wasm(); + EXPECT_TRUE(wasm->capabilityAllowed("proxy_log")); + EXPECT_TRUE(wasm->capabilityAllowed("proxy_on_new_connection")); + EXPECT_FALSE(wasm->capabilityAllowed("proxy_http_call")); + EXPECT_FALSE(wasm->capabilityAllowed("proxy_on_log")); + EXPECT_FALSE(filter_config.createContext() == nullptr); +} + +// The deprecated plugin level capability_restriction_config is still honored, by populating the VM +// level one. +TEST_P(WasmNetworkFilterConfigTest, FilterConfigDeprecatedPluginLevelCapabilityRestriction) { if (std::get<0>(GetParam()) == "null") { return; } @@ -335,11 +366,11 @@ TEST_P(WasmNetworkFilterConfigTest, FilterConfigAllowOnVmStart) { code: local: filename: "{{ test_rundir }}/test/extensions/filters/network/wasm/test_data/test_cpp.wasm" - capability_restriction_config: - allowed_capabilities: - proxy_on_vm_start: - proxy_get_property: - proxy_on_context_create: + capability_restriction_config: + allowed_capabilities: + proxy_on_vm_start: + proxy_get_property: + proxy_on_context_create: )EOF")); envoy::extensions::filters::network::wasm::v3::Wasm proto_config; diff --git a/test/test_common/wasm_base.h b/test/test_common/wasm_base.h index e71eb147b272b..a87a2c525cbae 100644 --- a/test/test_common/wasm_base.h +++ b/test/test_common/wasm_base.h @@ -80,8 +80,7 @@ template class WasmTestBase : public Base { std::ignore = vm_config->mutable_configuration()->PackFrom(vm_configuration_string); vm_config->mutable_code()->mutable_local()->set_inline_bytes(code); - plugin_ = std::make_shared( - plugin_config, envoy::config::core::v3::TrafficDirection::INBOUND, local_info_); + plugin_ = std::make_shared(plugin_config, local_info_); plugin_->wasmConfig().allowedCapabilities() = allowed_capabilities_; // Passes ownership of root_context_. Extensions::Common::Wasm::createWasm( @@ -215,9 +214,8 @@ template class WasmPluginConfigTestBase : public void setUp(const envoy::extensions::wasm::v3::PluginConfig plugin_config, bool singleton = false) { - plugin_config_ = std::make_shared( - plugin_config, server_, server_.scope(), server_.initManager(), - envoy::config::core::v3::TrafficDirection::UNSPECIFIED, singleton); + plugin_config_ = std::make_shared(plugin_config, server_, server_.scope(), + server_.initManager(), singleton); } void createStreamContext() { From 5787bb02aff0b2ee8243c0a402d920740994736c Mon Sep 17 00:00:00 2001 From: wbpcode Date: Sat, 22 Aug 2026 12:08:37 +0000 Subject: [PATCH 2/3] minor update Signed-off-by: wbpcode --- source/extensions/common/wasm/plugin.cc | 15 +++++++++------ test/extensions/common/wasm/plugin_test.cc | 5 ++++- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/source/extensions/common/wasm/plugin.cc b/source/extensions/common/wasm/plugin.cc index 1b3aa6f9457b6..56e7227c31588 100644 --- a/source/extensions/common/wasm/plugin.cc +++ b/source/extensions/common/wasm/plugin.cc @@ -14,15 +14,18 @@ envoy::extensions::wasm::v3::PluginConfig normalizeConfig(const envoy::extensions::wasm::v3::PluginConfig& config) { // The capability restrictions are applied when the Wasm VM is created and are shared by every // plugin running in it, so they belong to the VM configuration. The plugin level field is - // deprecated in favor of the VM level one: honor it only when the VM configuration doesn't set - // its own restrictions. - if (!config.has_capability_restriction_config() || - config.vm_config().has_capability_restriction_config()) { + // deprecated in favor of the VM level one: move it there and clear it, so that the VM level field + // is the only place the restrictions are ever read from. + if (!config.has_capability_restriction_config()) { return config; } envoy::extensions::wasm::v3::PluginConfig normalized = config; - *normalized.mutable_vm_config()->mutable_capability_restriction_config() = - config.capability_restriction_config(); + // The VM level restrictions win when both are set. + if (!config.vm_config().has_capability_restriction_config()) { + *normalized.mutable_vm_config()->mutable_capability_restriction_config() = + config.capability_restriction_config(); + } + normalized.clear_capability_restriction_config(); return normalized; } diff --git a/test/extensions/common/wasm/plugin_test.cc b/test/extensions/common/wasm/plugin_test.cc index 46c03289bd642..e3a43393253b5 100644 --- a/test/extensions/common/wasm/plugin_test.cc +++ b/test/extensions/common/wasm/plugin_test.cc @@ -64,7 +64,8 @@ TEST(TestWasmConfig, VmLevelCapabilityRestriction) { Contains(Key("proxy_log"))); } -// The deprecated plugin level capability restrictions are copied into the VM configuration. +// The deprecated plugin level capability restrictions are moved into the VM configuration, leaving +// the VM level field as the only place the restrictions can be read from. TEST(TestWasmConfig, DeprecatedPluginLevelCapabilityRestriction) { envoy::extensions::wasm::v3::PluginConfig plugin_config; plugin_config.mutable_capability_restriction_config()->mutable_allowed_capabilities()->insert( @@ -75,6 +76,7 @@ TEST(TestWasmConfig, DeprecatedPluginLevelCapabilityRestriction) { EXPECT_THAT( wasm_config.config().vm_config().capability_restriction_config().allowed_capabilities(), Contains(Key("proxy_log"))); + EXPECT_FALSE(wasm_config.config().has_capability_restriction_config()); } // The VM level capability restrictions win when both are set. @@ -90,6 +92,7 @@ TEST(TestWasmConfig, VmLevelCapabilityRestrictionWins) { auto wasm_config = WasmConfig(plugin_config); EXPECT_THAT(wasm_config.allowedCapabilities(), Contains(Key("proxy_on_vm_start"))); EXPECT_THAT(wasm_config.allowedCapabilities(), Not(Contains(Key("proxy_log")))); + EXPECT_FALSE(wasm_config.config().has_capability_restriction_config()); } // A configuration without any capability restriction leaves the VM unrestricted, and no empty VM From 54d12bd45cab7829294acb93eda09f36d12b70ed Mon Sep 17 00:00:00 2001 From: wbpcode Date: Mon, 24 Aug 2026 08:20:22 +0000 Subject: [PATCH 3/3] used unified factory base and update tests Signed-off-by: wbpcode --- source/extensions/filters/http/wasm/config.h | 46 +++---------------- .../filters/http/wasm/wasm_filter.cc | 43 ++++++++++------- .../filters/http/wasm/wasm_filter.h | 9 +--- test/extensions/common/wasm/plugin_test.cc | 20 ++++---- test/extensions/common/wasm/wasm_test.cc | 45 +++++++++++++++--- .../filters/http/wasm/config_test.cc | 11 ++++- .../filters/network/wasm/config_test.cc | 3 +- 7 files changed, 96 insertions(+), 81 deletions(-) diff --git a/source/extensions/filters/http/wasm/config.h b/source/extensions/filters/http/wasm/config.h index f11a92d5dd910..dfde1b05cf362 100644 --- a/source/extensions/filters/http/wasm/config.h +++ b/source/extensions/filters/http/wasm/config.h @@ -18,51 +18,19 @@ namespace Wasm { * Config registration for the Wasm filter. @see NamedHttpFilterConfigFactory. */ class WasmFilterConfig - : public Common::CommonFactoryBase, - public Server::Configuration::NamedHttpFilterConfigFactory, - public Server::Configuration::UpstreamHttpFilterConfigFactory { + : public Common::UnifiedFactoryBase { public: WasmFilterConfig() - : Common::CommonFactoryBase( + : Common::UnifiedFactoryBase( "envoy.filters.http.wasm") {} - absl::StatusOr - createFilterFactoryFromProto(const Protobuf::Message& proto_config, - const std::string& stats_prefix, - Server::Configuration::FactoryContext& context) override { - return createFilterFactoryFromProtoTyped( - MessageUtil::downcastAndValidate( - proto_config, context.messageValidationVisitor()), - stats_prefix, context, context.serverFactoryContext()); - } - - absl::StatusOr - createFilterFactoryFromProto(const Protobuf::Message& proto_config, - const std::string& stats_prefix, - Server::Configuration::UpstreamFactoryContext& context) override { - return createFilterFactoryFromProtoTyped( - MessageUtil::downcastAndValidate( - proto_config, context.serverFactoryContext().messageValidationVisitor()), - stats_prefix, context, context.serverFactoryContext()); - } - - absl::StatusOr createHttpFilterFactoryFromProto( - const Protobuf::Message& proto_config, Server::Configuration::ServerFactoryContext& context, + absl::StatusOr createHttpFilterFactoryFromProtoTyped( + const envoy::extensions::filters::http::wasm::v3::Wasm& proto_config, + Server::Configuration::ServerFactoryContext& context, Server::Configuration::ExtraFactoryContext& extra_context) override { - return createFilterFactoryFromProtoTyped( - MessageUtil::downcastAndValidate( - proto_config, context.messageValidationVisitor()), - extra_context.stats_prefix, context, context); - } - -private: - template - Http::FilterFactoryCb createFilterFactoryFromProtoTyped( - const envoy::extensions::filters::http::wasm::v3::Wasm& proto_config, const std::string&, - FactoryContext& context, Server::Configuration::ServerFactoryContext& server_context) { - server_context.api().customStatNamespaces().registerStatNamespace( + context.api().customStatNamespaces().registerStatNamespace( Extensions::Common::Wasm::CustomStatNamespace); - auto filter_config = std::make_shared(proto_config, context); + auto filter_config = std::make_shared(proto_config, context, extra_context); return [filter_config](Http::FilterChainFactoryCallbacks& callbacks) -> void { auto filter = filter_config->createContext(); if (!filter) { // Fail open diff --git a/source/extensions/filters/http/wasm/wasm_filter.cc b/source/extensions/filters/http/wasm/wasm_filter.cc index 387fe57a94112..d3bfc4f2a8521 100644 --- a/source/extensions/filters/http/wasm/wasm_filter.cc +++ b/source/extensions/filters/http/wasm/wasm_filter.cc @@ -9,31 +9,40 @@ namespace Wasm { namespace { -Stats::Scope& upstreamWasmStatsScope(Server::Configuration::UpstreamFactoryContext& context) { +Stats::Scope& statsScope(Server::Configuration::ServerFactoryContext& context, + Server::Configuration::ExtraFactoryContext& extra_context) { + // Server scope for filters without a specified scope. + if (!extra_context.scope.has_value()) { + return context.scope(); + } + + // Downstream filters. + if (!extra_context.is_upstream) { + return *extra_context.scope; + } + + // Upstream filters. if (Runtime::runtimeFeatureEnabled( "envoy.reloadable_features.upstream_wasm_filter_uses_root_scope")) { - return context.serverFactoryContext().serverScope(); + return context.serverScope(); } - return context.scope(); + return *extra_context.scope; } -} // namespace - -FilterConfig::FilterConfig(const envoy::extensions::filters::http::wasm::v3::Wasm& config, - Server::Configuration::FactoryContext& context) - : Extensions::Common::Wasm::PluginConfig(config.config(), context.serverFactoryContext(), - context.scope(), context.initManager(), false) {} +Init::Manager& initManager(Server::Configuration::ServerFactoryContext& context, + Server::Configuration::ExtraFactoryContext& extra_context) { + return extra_context.init_manager.has_value() ? *extra_context.init_manager + : context.initManager(); +} -FilterConfig::FilterConfig(const envoy::extensions::filters::http::wasm::v3::Wasm& config, - Server::Configuration::UpstreamFactoryContext& context) - : Extensions::Common::Wasm::PluginConfig(config.config(), context.serverFactoryContext(), - upstreamWasmStatsScope(context), context.initManager(), - false) {} +} // namespace FilterConfig::FilterConfig(const envoy::extensions::filters::http::wasm::v3::Wasm& config, - Server::Configuration::ServerFactoryContext& context) - : Extensions::Common::Wasm::PluginConfig(config.config(), context, context.scope(), - context.initManager(), false) {} + Server::Configuration::ServerFactoryContext& context, + Server::Configuration::ExtraFactoryContext& extra_context) + : Extensions::Common::Wasm::PluginConfig(config.config(), context, + statsScope(context, extra_context), + initManager(context, extra_context), false) {} } // namespace Wasm } // namespace HttpFilters diff --git a/source/extensions/filters/http/wasm/wasm_filter.h b/source/extensions/filters/http/wasm/wasm_filter.h index 332b283c4cd13..384333d2a09cd 100644 --- a/source/extensions/filters/http/wasm/wasm_filter.h +++ b/source/extensions/filters/http/wasm/wasm_filter.h @@ -19,13 +19,8 @@ namespace Wasm { class FilterConfig : public Extensions::Common::Wasm::PluginConfig { public: FilterConfig(const envoy::extensions::filters::http::wasm::v3::Wasm& config, - Server::Configuration::FactoryContext& context); - - FilterConfig(const envoy::extensions::filters::http::wasm::v3::Wasm& config, - Server::Configuration::UpstreamFactoryContext& context); - - FilterConfig(const envoy::extensions::filters::http::wasm::v3::Wasm& config, - Server::Configuration::ServerFactoryContext& context); + Server::Configuration::ServerFactoryContext& context, + Server::Configuration::ExtraFactoryContext& extra_context); }; using FilterConfigSharedPtr = std::shared_ptr; diff --git a/test/extensions/common/wasm/plugin_test.cc b/test/extensions/common/wasm/plugin_test.cc index e3a43393253b5..a900910ad944c 100644 --- a/test/extensions/common/wasm/plugin_test.cc +++ b/test/extensions/common/wasm/plugin_test.cc @@ -66,7 +66,7 @@ TEST(TestWasmConfig, VmLevelCapabilityRestriction) { // The deprecated plugin level capability restrictions are moved into the VM configuration, leaving // the VM level field as the only place the restrictions can be read from. -TEST(TestWasmConfig, DeprecatedPluginLevelCapabilityRestriction) { +TEST(TestWasmConfig, DEPRECATED_FEATURE_TEST(DeprecatedPluginLevelCapabilityRestriction)) { envoy::extensions::wasm::v3::PluginConfig plugin_config; plugin_config.mutable_capability_restriction_config()->mutable_allowed_capabilities()->insert( {"proxy_log", envoy::extensions::wasm::v3::SanitizationConfig()}); @@ -80,7 +80,7 @@ TEST(TestWasmConfig, DeprecatedPluginLevelCapabilityRestriction) { } // The VM level capability restrictions win when both are set. -TEST(TestWasmConfig, VmLevelCapabilityRestrictionWins) { +TEST(TestWasmConfig, DEPRECATED_FEATURE_TEST(VmLevelCapabilityRestrictionWins)) { envoy::extensions::wasm::v3::PluginConfig plugin_config; plugin_config.mutable_capability_restriction_config()->mutable_allowed_capabilities()->insert( {"proxy_log", envoy::extensions::wasm::v3::SanitizationConfig()}); @@ -197,12 +197,6 @@ TEST_F(PluginKeyTest, ConfigDifferencesProduceDistinctKeys) { other.mutable_allow_on_headers_stop_iteration()->set_value(true); EXPECT_NE(base_key, key(other)); } - { - auto other = config; - other.mutable_capability_restriction_config()->mutable_allowed_capabilities()->insert( - {"proxy_log", envoy::extensions::wasm::v3::SanitizationConfig()}); - EXPECT_NE(base_key, key(other)); - } { auto other = config; other.mutable_reload_config()->mutable_backoff()->mutable_base_interval()->set_seconds(30); @@ -210,6 +204,16 @@ TEST_F(PluginKeyTest, ConfigDifferencesProduceDistinctKeys) { } } +// Same as above for the deprecated plugin level capability restrictions, which are part of the +// plugin configuration and so of the plugin identity. +TEST_F(PluginKeyTest, DEPRECATED_FEATURE_TEST(DeprecatedCapabilityRestrictionProducesDistinctKey)) { + const auto config = baseConfig(); + auto other = config; + other.mutable_capability_restriction_config()->mutable_allowed_capabilities()->insert( + {"proxy_log", envoy::extensions::wasm::v3::SanitizationConfig()}); + EXPECT_NE(key(config), key(other)); +} + // The VM configuration is not part of the plugin key: VM identity is covered by the VM key, which // is prepended to the plugin key when caching thread-local plugins. TEST_F(PluginKeyTest, VmConfigIsNotPartOfTheKey) { diff --git a/test/extensions/common/wasm/wasm_test.cc b/test/extensions/common/wasm/wasm_test.cc index 9e51215b8da21..79e414a81f8a9 100644 --- a/test/extensions/common/wasm/wasm_test.cc +++ b/test/extensions/common/wasm/wasm_test.cc @@ -779,14 +779,45 @@ TEST_P(WasmCommonTest, VmCacheVmConfigChange) { ->insert({"proxy_log", envoy::extensions::wasm::v3::SanitizationConfig()}); EXPECT_NE(create(config), base_handle); } - // The deprecated plugin level field is normalized onto the VM configuration, so it must be - // covered by the VM key as well. - { - auto config = plugin_config; - config.mutable_capability_restriction_config()->mutable_allowed_capabilities()->insert( - {"proxy_log", envoy::extensions::wasm::v3::SanitizationConfig()}); - EXPECT_NE(create(config), base_handle); + proxy_wasm::clearWasmCachesForTesting(); +} + +// The deprecated plugin level capability restrictions are normalized onto the VM configuration, so +// they must be covered by the VM key just like the VM level ones. As above, only distinctness from +// the unrestricted VM is asserted. +TEST_P(WasmCommonTest, DEPRECATED_FEATURE_TEST(VmCacheDeprecatedCapabilityRestriction)) { + // NullVm has no separate VM configuration to vary. + if (std::get<0>(GetParam()) == "null") { + return; } + NiceMock init_manager; + + const std::string code = TestEnvironment::readFileToStringForTest(TestEnvironment::substitute( + "{{ test_rundir }}/test/extensions/common/wasm/test_data/test_cpp.wasm")); + EXPECT_FALSE(code.empty()); + + envoy::extensions::wasm::v3::PluginConfig plugin_config; + auto* base_vm_config = plugin_config.mutable_vm_config(); + base_vm_config->set_runtime(absl::StrCat("envoy.wasm.runtime.", std::get<0>(GetParam()))); + base_vm_config->mutable_code()->mutable_local()->set_inline_bytes(code); + + auto create = [&](const envoy::extensions::wasm::v3::PluginConfig& config) { + auto plugin = std::make_shared(config, local_info_); + WasmHandleSharedPtr wasm_handle; + createWasm(plugin, scope_, cluster_manager_, init_manager, *dispatcher_, *api_, + lifecycle_notifier_, remote_data_provider_, + [&wasm_handle](const WasmHandleSharedPtr& w) { wasm_handle = w; }); + return wasm_handle; + }; + + // Held for the whole test: the base Wasm cache keeps only a weak reference to it. + auto base_handle = create(plugin_config); + EXPECT_NE(base_handle, nullptr); + + auto config = plugin_config; + config.mutable_capability_restriction_config()->mutable_allowed_capabilities()->insert( + {"proxy_log", envoy::extensions::wasm::v3::SanitizationConfig()}); + EXPECT_NE(create(config), base_handle); proxy_wasm::clearWasmCachesForTesting(); } diff --git a/test/extensions/filters/http/wasm/config_test.cc b/test/extensions/filters/http/wasm/config_test.cc index 50ac7560dcd95..5ed88a9a3046b 100644 --- a/test/extensions/filters/http/wasm/config_test.cc +++ b/test/extensions/filters/http/wasm/config_test.cc @@ -116,9 +116,15 @@ class WasmFilterConfigTest std::unique_ptr getFilterConfig(const envoy::extensions::filters::http::wasm::v3::Wasm& proto_config) { if (std::get<2>(GetParam())) { - return std::make_unique(proto_config, context_); + auto extra_context = + Server::Configuration::ExtraFactoryContext::create(context_, stats_prefix_); + return std::make_unique(proto_config, context_.server_factory_context_, + extra_context); } - return std::make_unique(proto_config, upstream_factory_context_); + auto extra_context = Server::Configuration::ExtraFactoryContext::create( + upstream_factory_context_, stats_prefix_); + return std::make_unique( + proto_config, upstream_factory_context_.server_factory_context_, extra_context); } envoy::extensions::filters::http::wasm::v3::Wasm localWasmConfig(const std::string& name) { @@ -144,6 +150,7 @@ class WasmFilterConfigTest return proto_config; } + const std::string stats_prefix_{"stats"}; NiceMock listener_info_; Stats::IsolatedStoreImpl stats_store_; Stats::Scope& stats_scope_{*stats_store_.rootScope()}; diff --git a/test/extensions/filters/network/wasm/config_test.cc b/test/extensions/filters/network/wasm/config_test.cc index d61f2001a7892..d5ed048d54ccd 100644 --- a/test/extensions/filters/network/wasm/config_test.cc +++ b/test/extensions/filters/network/wasm/config_test.cc @@ -325,7 +325,8 @@ TEST_P(WasmNetworkFilterConfigTest, FilterConfigCapabilityRestriction) { // The deprecated plugin level capability_restriction_config is still honored, by populating the VM // level one. -TEST_P(WasmNetworkFilterConfigTest, FilterConfigDeprecatedPluginLevelCapabilityRestriction) { +TEST_P(WasmNetworkFilterConfigTest, + DEPRECATED_FEATURE_TEST(FilterConfigDeprecatedPluginLevelCapabilityRestriction)) { if (std::get<0>(GetParam()) == "null") { return; }