diff --git a/api/envoy/extensions/dynamic_modules/v3/BUILD b/api/envoy/extensions/dynamic_modules/v3/BUILD index 5f552f08145ca..504c6c70514ac 100644 --- a/api/envoy/extensions/dynamic_modules/v3/BUILD +++ b/api/envoy/extensions/dynamic_modules/v3/BUILD @@ -5,5 +5,8 @@ load("@envoy_api//bazel:api_build_system.bzl", "api_proto_package") licenses(["notice"]) # Apache 2 api_proto_package( - deps = ["@xds//udpa/annotations:pkg"], + deps = [ + "//envoy/config/core/v3:pkg", + "@xds//udpa/annotations:pkg", + ], ) diff --git a/api/envoy/extensions/dynamic_modules/v3/dynamic_modules.proto b/api/envoy/extensions/dynamic_modules/v3/dynamic_modules.proto index cdb79c0dce3a2..d4e4d9a7bcde0 100644 --- a/api/envoy/extensions/dynamic_modules/v3/dynamic_modules.proto +++ b/api/envoy/extensions/dynamic_modules/v3/dynamic_modules.proto @@ -2,8 +2,9 @@ syntax = "proto3"; package envoy.extensions.dynamic_modules.v3; +import "envoy/config/core/v3/base.proto"; + import "udpa/annotations/status.proto"; -import "validate/validate.proto"; option java_package = "io.envoyproxy.envoy.extensions.dynamic_modules.v3"; option java_outer_classname = "DynamicModulesProto"; @@ -30,7 +31,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // the ABI is stabilized, this restriction will be revisited. Until then, Envoy checks the hash of // the ABI header files to ensure that the dynamic modules are built against the same version of the // ABI. -// [#next-free-field: 6] +// [#next-free-field: 7] message DynamicModuleConfig { // The name of the dynamic module. // @@ -42,9 +43,12 @@ message DynamicModuleConfig { // try to find the module from a standard system library path (e.g., ``/usr/lib``) following the // platform's default behavior for ``dlopen``. // + // This field is optional if the ``module`` field is set. When both ``name`` and ``module`` are + // specified, the ``module`` field takes precedence. + // // .. note:: // There is some remaining work to make the search path configurable via command line options. - string name = 1 [(validate.rules).string = {min_len: 1}]; + string name = 1; // If true, prevents the module from being unloaded with ``dlclose``. // @@ -80,4 +84,10 @@ message DynamicModuleConfig { // // Defaults to ``dynamicmodulescustom``. string metrics_namespace = 5; + + // The dynamic module binary to load. Currently only supports local file paths + // via ``local.filename``. + // + // When both ``name`` and ``module`` are set, ``module`` takes precedence. + config.core.v3.AsyncDataSource module = 6; } diff --git a/changelogs/current.yaml b/changelogs/current.yaml index eaaccc5c6492f..cfa358754d2d4 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -458,6 +458,12 @@ new_features: functions by name via ``envoy_dynamic_module_callback_register_function`` and other modules can resolve them via ``envoy_dynamic_module_callback_get_function``, enabling zero-copy cross-module interactions analogous to ``dlsym``. +- area: dynamic_modules + change: | + Added support for loading dynamic module binaries from local file paths via the new + :ref:`module ` + field in ``DynamicModuleConfig``. This allows specifying an absolute path to a ``.so`` file + via ``module.local.filename`` as an alternative to the name-based search path. - area: dynamic_modules change: | Network filter read and write buffers now persist after ``on_read``/``on_write`` callbacks, allowing diff --git a/source/extensions/filters/http/dynamic_modules/BUILD b/source/extensions/filters/http/dynamic_modules/BUILD index 2411cd0bbe8d2..286e6cb1a26e2 100644 --- a/source/extensions/filters/http/dynamic_modules/BUILD +++ b/source/extensions/filters/http/dynamic_modules/BUILD @@ -38,6 +38,7 @@ envoy_cc_library( ":abi_impl", ":filter_config_lib", ":filter_lib", + "//source/extensions/dynamic_modules:dynamic_modules_lib", "//source/extensions/filters/http/common:factory_base_lib", "@envoy_api//envoy/extensions/filters/http/dynamic_modules/v3:pkg_cc_proto", ], diff --git a/source/extensions/filters/http/dynamic_modules/factory.cc b/source/extensions/filters/http/dynamic_modules/factory.cc index cab862d52d0c2..d201e775dccd4 100644 --- a/source/extensions/filters/http/dynamic_modules/factory.cc +++ b/source/extensions/filters/http/dynamic_modules/factory.cc @@ -13,8 +13,25 @@ absl::StatusOr DynamicModuleConfigFactory::createFilterFa Server::Configuration::ServerFactoryContext& context, Stats::Scope& scope) { const auto& module_config = proto_config.dynamic_module_config(); - auto dynamic_module = Extensions::DynamicModules::newDynamicModuleByName( - module_config.name(), module_config.do_not_close(), module_config.load_globally()); + + // Load the module: either from a local file path or by name. + absl::StatusOr dynamic_module; + if (module_config.has_module()) { + if (!module_config.module().has_local() || !module_config.module().local().has_filename()) { + return absl::InvalidArgumentError( + "Only local file path is supported for module sources (via module.local.filename)"); + } + dynamic_module = Extensions::DynamicModules::newDynamicModule( + module_config.module().local().filename(), module_config.do_not_close(), + module_config.load_globally()); + } else { + if (module_config.name().empty()) { + return absl::InvalidArgumentError( + "Either 'name' or 'module' must be specified in dynamic_module_config"); + } + dynamic_module = Extensions::DynamicModules::newDynamicModuleByName( + module_config.name(), module_config.do_not_close(), module_config.load_globally()); + } if (!dynamic_module.ok()) { return absl::InvalidArgumentError("Failed to load dynamic module: " + std::string(dynamic_module.status().message())); diff --git a/test/extensions/dynamic_modules/http/BUILD b/test/extensions/dynamic_modules/http/BUILD index 81e82e97abe74..b1bf3528e8eff 100644 --- a/test/extensions/dynamic_modules/http/BUILD +++ b/test/extensions/dynamic_modules/http/BUILD @@ -8,6 +8,25 @@ licenses(["notice"]) # Apache 2 envoy_package() +envoy_cc_test( + name = "config_test", + srcs = ["config_test.cc"], + data = [ + "//test/extensions/dynamic_modules/test_data/c:no_op", + ], + deps = [ + "//source/common/stats:isolated_store_lib", + "//source/extensions/filters/http/dynamic_modules:factory_lib", + "//test/extensions/dynamic_modules:util", + "//test/mocks/network:network_mocks", + "//test/mocks/server:server_mocks", + "//test/test_common:environment_lib", + "//test/test_common:simulated_time_system_lib", + "//test/test_common:utility_lib", + "@envoy_api//envoy/extensions/filters/http/dynamic_modules/v3:pkg_cc_proto", + ], +) + envoy_cc_test( name = "factory_test", srcs = ["factory_test.cc"], diff --git a/test/extensions/dynamic_modules/http/config_test.cc b/test/extensions/dynamic_modules/http/config_test.cc new file mode 100644 index 0000000000000..7eb9bbe3508cd --- /dev/null +++ b/test/extensions/dynamic_modules/http/config_test.cc @@ -0,0 +1,175 @@ +#include "envoy/extensions/filters/http/dynamic_modules/v3/dynamic_modules.pb.h" +#include "envoy/extensions/filters/http/dynamic_modules/v3/dynamic_modules.pb.validate.h" + +#include "source/common/stats/isolated_store_impl.h" +#include "source/extensions/filters/http/dynamic_modules/factory.h" + +#include "test/extensions/dynamic_modules/util.h" +#include "test/mocks/network/mocks.h" +#include "test/mocks/server/mocks.h" +#include "test/test_common/environment.h" +#include "test/test_common/utility.h" + +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +using testing::ReturnRef; + +namespace Envoy { +namespace Server { +namespace Configuration { + +class DynamicModuleFilterConfigTest : public Event::TestUsingSimulatedTime, public testing::Test { +protected: + DynamicModuleFilterConfigTest() : api_(Api::createApiForTest(stats_store_)) { + ON_CALL(context_.server_factory_context_, api()).WillByDefault(ReturnRef(*api_)); + ON_CALL(context_, scope()).WillByDefault(ReturnRef(stats_scope_)); + ON_CALL(context_, listenerInfo()).WillByDefault(ReturnRef(listener_info_)); + ON_CALL(listener_info_, metadata()).WillByDefault(ReturnRef(listener_metadata_)); + EXPECT_CALL(context_, initManager()).WillRepeatedly(ReturnRef(init_manager_)); + } + + NiceMock listener_info_; + Stats::IsolatedStoreImpl stats_store_; + Stats::Scope& stats_scope_{*stats_store_.rootScope()}; + Api::ApiPtr api_; + envoy::config::core::v3::Metadata listener_metadata_; + Init::ManagerImpl init_manager_{"init_manager"}; + Init::ExpectableWatcherImpl init_watcher_; + + NiceMock context_; +}; + +TEST_F(DynamicModuleFilterConfigTest, LocalFileLoading) { + const std::string module_path = Extensions::DynamicModules::testSharedObjectPath("no_op", "c"); + + const std::string yaml = TestEnvironment::substitute(absl::StrCat(R"EOF( + dynamic_module_config: + module: + local: + filename: ")EOF", + module_path, R"EOF(" + do_not_close: true + filter_name: "test_filter" + )EOF")); + + envoy::extensions::filters::http::dynamic_modules::v3::DynamicModuleFilter proto_config; + TestUtility::loadFromYamlAndValidate(yaml, proto_config); + + DynamicModuleConfigFactory factory; + auto cb_or_error = factory.createFilterFactoryFromProto(proto_config, "stats", context_); + EXPECT_TRUE(cb_or_error.ok()) << cb_or_error.status().message(); + + EXPECT_CALL(init_watcher_, ready()); + context_.initManager().initialize(init_watcher_); + EXPECT_EQ(context_.initManager().state(), Init::Manager::State::Initialized); +} + +TEST_F(DynamicModuleFilterConfigTest, InlineBytesRejected) { + const std::string yaml = R"EOF( + dynamic_module_config: + module: + local: + inline_bytes: "AAAA" + do_not_close: true + filter_name: "test_filter" + )EOF"; + + envoy::extensions::filters::http::dynamic_modules::v3::DynamicModuleFilter proto_config; + TestUtility::loadFromYamlAndValidate(yaml, proto_config); + + DynamicModuleConfigFactory factory; + auto cb_or_error = factory.createFilterFactoryFromProto(proto_config, "stats", context_); + EXPECT_FALSE(cb_or_error.ok()); + EXPECT_THAT(cb_or_error.status().message(), + testing::HasSubstr("Only local file path is supported")); +} + +TEST_F(DynamicModuleFilterConfigTest, NoModuleOrName) { + const std::string yaml = R"EOF( + dynamic_module_config: + do_not_close: true + filter_name: "test_filter" + )EOF"; + + envoy::extensions::filters::http::dynamic_modules::v3::DynamicModuleFilter proto_config; + TestUtility::loadFromYamlAndValidate(yaml, proto_config); + + DynamicModuleConfigFactory factory; + auto cb_or_error = factory.createFilterFactoryFromProto(proto_config, "stats", context_); + EXPECT_FALSE(cb_or_error.ok()); + EXPECT_THAT(cb_or_error.status().message(), + testing::HasSubstr("Either 'name' or 'module' must be specified")); +} + +TEST_F(DynamicModuleFilterConfigTest, RemoteSourceRejected) { + const std::string yaml = R"EOF( + dynamic_module_config: + module: + remote: + http_uri: + uri: https://example.com/module.so + cluster: cluster_1 + timeout: 5s + sha256: "abc123" + do_not_close: true + filter_name: "test_filter" + )EOF"; + + envoy::extensions::filters::http::dynamic_modules::v3::DynamicModuleFilter proto_config; + TestUtility::loadFromYamlAndValidate(yaml, proto_config); + + DynamicModuleConfigFactory factory; + auto cb_or_error = factory.createFilterFactoryFromProto(proto_config, "stats", context_); + EXPECT_FALSE(cb_or_error.ok()); + EXPECT_THAT(cb_or_error.status().message(), + testing::HasSubstr("Only local file path is supported")); +} + +TEST_F(DynamicModuleFilterConfigTest, InvalidLocalFile) { + const std::string yaml = R"EOF( + dynamic_module_config: + module: + local: + filename: "/nonexistent/path/to/module.so" + do_not_close: true + filter_name: "test_filter" + )EOF"; + + envoy::extensions::filters::http::dynamic_modules::v3::DynamicModuleFilter proto_config; + TestUtility::loadFromYamlAndValidate(yaml, proto_config); + + DynamicModuleConfigFactory factory; + auto cb_or_error = factory.createFilterFactoryFromProto(proto_config, "stats", context_); + EXPECT_FALSE(cb_or_error.ok()); + EXPECT_THAT(cb_or_error.status().message(), testing::HasSubstr("Failed to load dynamic module")); +} + +// Verify that when both name and module are set, module takes precedence. +TEST_F(DynamicModuleFilterConfigTest, ModulePrecedenceOverName) { + const std::string module_path = Extensions::DynamicModules::testSharedObjectPath("no_op", "c"); + + const std::string yaml = TestEnvironment::substitute(absl::StrCat(R"EOF( + dynamic_module_config: + name: "nonexistent_module_should_be_ignored" + module: + local: + filename: ")EOF", + module_path, R"EOF(" + do_not_close: true + filter_name: "test_filter" + )EOF")); + + envoy::extensions::filters::http::dynamic_modules::v3::DynamicModuleFilter proto_config; + TestUtility::loadFromYamlAndValidate(yaml, proto_config); + + DynamicModuleConfigFactory factory; + // If name were used, this would fail because "nonexistent_module_should_be_ignored" doesn't + // exist. Since module takes precedence, it should succeed with the local file. + auto cb_or_error = factory.createFilterFactoryFromProto(proto_config, "stats", context_); + EXPECT_TRUE(cb_or_error.ok()) << cb_or_error.status().message(); +} + +} // namespace Configuration +} // namespace Server +} // namespace Envoy