Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dynamic_modules: scaffolds config API & HTTP Filter #36448

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,7 @@ extensions/filters/http/oauth2 @derekargueta @mattklein123
/*/extensions/filters/network/generic_proxy/ @wbpcode @soulxu
# Dynamic Modules
/*/extensions/dynamic_modules @mattklein123 @mathetake @marc-barry
/*/extensions/filters/http/dynamic_modules @mattklein123 @mathetake @marc-barry

# HTTP credential injector
/*/extensions/filters/http/credential_injector @zhaohuabing @kyessenov
Expand Down
2 changes: 2 additions & 0 deletions api/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ proto_library(
"//envoy/extensions/compression/zstd/compressor/v3:pkg",
"//envoy/extensions/compression/zstd/decompressor/v3:pkg",
"//envoy/extensions/config/validators/minimum_clusters/v3:pkg",
"//envoy/extensions/dynamic_modules/v3:pkg",
"//envoy/extensions/early_data/v3:pkg",
"//envoy/extensions/filters/common/dependency/v3:pkg",
"//envoy/extensions/filters/common/fault/v3:pkg",
Expand All @@ -175,6 +176,7 @@ proto_library(
"//envoy/extensions/filters/http/custom_response/v3:pkg",
"//envoy/extensions/filters/http/decompressor/v3:pkg",
"//envoy/extensions/filters/http/dynamic_forward_proxy/v3:pkg",
"//envoy/extensions/filters/http/dynamic_modules/v3:pkg",
"//envoy/extensions/filters/http/ext_authz/v3:pkg",
"//envoy/extensions/filters/http/ext_proc/v3:pkg",
"//envoy/extensions/filters/http/fault/v3:pkg",
Expand Down
13 changes: 13 additions & 0 deletions api/envoy/extensions/dynamic_modules/v3/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# DO NOT EDIT. This file is generated by tools/proto_format/proto_sync.py.

load("@envoy_api//bazel:api_build_system.bzl", "api_proto_package")

licenses(["notice"]) # Apache 2

api_proto_package(
deps = [
"//envoy/config/core/v3:pkg",
"@com_github_cncf_xds//udpa/annotations:pkg",
"@com_github_cncf_xds//xds/annotations/v3:pkg",
],
)
45 changes: 45 additions & 0 deletions api/envoy/extensions/dynamic_modules/v3/dynamic_modules.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
syntax = "proto3";

package envoy.extensions.dynamic_modules.v3;

import "envoy/config/core/v3/base.proto";

import "xds/annotations/v3/status.proto";

import "udpa/annotations/status.proto";

option java_package = "io.envoyproxy.envoy.extensions.dynamic_modules.v3";
option java_outer_classname = "DynamicModulesProto";
option java_multiple_files = true;
option go_package = "github.com/envoyproxy/go-control-plane/envoy/extensions/dynamic_modules/v3;dynamic_modulesv3";
option (udpa.annotations.file_status).package_version_status = ACTIVE;
option (xds.annotations.v3.file_status).work_in_progress = true;

// [#protodoc-title: Dynamic Modules common configuration]

// Configuration of a dynamic module. A dynamic module is a shared object file that can be loaded via dlopen
// by a various Envoy extension points. Currently, only HTTP filter (envoy.filters.http.dynamic_modules) is supported.
//
// How the module is loaded is determined by the extension point that uses it. For example, the HTTP filter
// loads the module with dlopen on the first creation of the filter chain for the same shared object file.
//
// Whether or not the shared object is the same is determined by the file path as well as the file's inode depending
// on the platform. Notably, if the file path and the content of the file are the same, the shared object will be reused.
//
// A module must be compatible with the ABI specified in :repo:`abi.h <source/extensions/dynamic_modules/abi.h>`.
// Currently, compatibility is only guaranteed by an exact version match between the Envoy
// codebase and the dynamic module SDKs. In the future, after the ABI is stabilized, we will revisit
// this restriction and hopefully provide a wider compatibility guarantee. 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.
//
// Currently, the implementation is work in progress and not usable.
// [#next-free-field: 8]
message DynamicModuleConfig {
// The location of the object file. Currently, only the local path is supported.
config.core.v3.DataSource object_file = 7;

// Set true to prevent the module from being unloaded with dlclose.
// This is useful for modules that have global state that should not be unloaded.
bool do_not_close = 3;
}
13 changes: 13 additions & 0 deletions api/envoy/extensions/filters/http/dynamic_modules/v3/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# DO NOT EDIT. This file is generated by tools/proto_format/proto_sync.py.

load("@envoy_api//bazel:api_build_system.bzl", "api_proto_package")

licenses(["notice"]) # Apache 2

api_proto_package(
deps = [
"//envoy/extensions/dynamic_modules/v3:pkg",
"@com_github_cncf_xds//udpa/annotations:pkg",
"@com_github_cncf_xds//xds/annotations/v3:pkg",
],
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
syntax = "proto3";

package envoy.extensions.filters.http.dynamic_modules.v3;

import "envoy/extensions/dynamic_modules/v3/dynamic_modules.proto";

import "xds/annotations/v3/status.proto";

import "udpa/annotations/status.proto";

option java_package = "io.envoyproxy.envoy.extensions.filters.http.dynamic_modules.v3";
option java_outer_classname = "DynamicModulesProto";
option java_multiple_files = true;
option go_package = "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/dynamic_modules/v3;dynamic_modulesv3";
option (udpa.annotations.file_status).package_version_status = ACTIVE;
option (xds.annotations.v3.file_status).work_in_progress = true;

// [#protodoc-title: HTTP filter for dynamic modules]

// Configuration of the HTTP filter for dynamic modules. This filter allows loading shared object files
// that can be loaded via dlopen by the HTTP filter.
//
// A module can be loaded by multiple HTTP filters, hence the program can be structured in a way that
// the module is loaded only once and shared across multiple filters providing multiple functionalities.
//
// Currently, the implementation is work in progress and not usable.
message DynamicModuleFilter {
// Specifies the shared-object level configuration.
envoy.extensions.dynamic_modules.v3.DynamicModuleConfig dynamic_module_config = 1;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it really make sense for the control plane to have to know the actual path to the dynamic module, or will that be something local controlled by the workload or deployment? I'm wondering if it would be valuable to have a layer of indirection here, where the bootstrap file defines some dynamic module instances, giving each one a name, and the control plane sends the instance name of the module to use. This way, the API between the control plane and the workload is defined in terms of these abstract instance names rather than in terms of specific paths.

Copy link
Member Author

@mathetake mathetake Oct 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a good question. In fact, for the case of Wasm, even though it has the same DataSource config like this PR, the reality of Istio is that the control plane always passes a local path and the sidecar agent is the one downloading the binary from remote source and place it at the location specified by the control plane because the Envoy's filter chain set up routine cannot be made asynchronous, hence the filter chain would end up in a dangerous filter state (e.g. downloading failed but the filter chain initialized).

For the case of dynamic modules, now I think at least DataSource configuration is overkill since unlike Wasm case, dlopen can only refer a local file and cannot use the in-memory bytes. So at the end of the day, we would end up using only local file paths in reality just like the Wasm case, e.g. Envoy would need to write inline_bytes into a temporary files etc (which I think is messy tbh).

Regarding your idea of using the bootstrap file stuff, if I understand correctly, we would lose the ability to load/distribute module files at runtime without restarting Envoy vs if we can specify the local file path here, at least we can achieve that with the help of a side car agent etc.

From implementation point of view, there's almost no difference between these choices,, so I think the question is here is whether or not we want to tie the Envoy deployment with loadable dynamic modules by having a dynamic modules specific config in the Envoy bootstrap config, or have the ability to load/discover modules at runtime. wdyt? @mattklein123 @marc-barry

cc @arkodg @zirain @zhaohuabing

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess in practice, users wouldn't distribute native binaries at runtime as it's tied to the machine arch/os Envoy is running on, and therefore they would deploy the modules together with Envoy. So, personally either choice works for me.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so basically what I am thinking right now is to stop using DataSource (the reason I described above) and have a dedicated oneof so that we can later add another option like the bootstrap config thingy discussed here like

diff --git a/api/envoy/extensions/dynamic_modules/v3/dynamic_modules.proto b/api/envoy/extensions/dynamic_modules/v3/dynamic_modules.proto
index 204c482b02..5e612e2a7c 100644
--- a/api/envoy/extensions/dynamic_modules/v3/dynamic_modules.proto
+++ b/api/envoy/extensions/dynamic_modules/v3/dynamic_modules.proto
@@ -36,8 +36,15 @@ option (xds.annotations.v3.file_status).work_in_progress = true;
 // Currently, the implementation is work in progress and not usable.
 // [#next-free-field: 8]
 message DynamicModuleConfig {
-  // The location of the object file. Currently, only the local path is supported.
-  config.core.v3.DataSource object_file = 7;
+  // The location of the object file. Intentionally this does not use config.core.v3.DataSource, as the
+  // object file must be loaded via dlopen, which requires a physical file path, not a in-memory buffer.
+  oneof location {
+    // The object file's local path. It can be a relative path or an absolute path.
+    // E.g. /path/to/module.so or module.so
+    string local_path = 2;
+
+    // TODO: maybe a "name" pointing to a file configured in the bootstrap.
+  }
 
   // Set true to prevent the module from being unloaded with dlclose.
   // This is useful for modules that have global state that should not be unloaded.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that the xDS API style guidelines discourage use of oneof.


// The name for this filter configuration. This can be used to distinguish between different filter implementations
// inside a dynamic module. For example, a module can have completely different filter implementations.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this level of indirection useful? Why not just have multiple modules in this case?

Copy link
Member Author

@mathetake mathetake Oct 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason is that each module must have its own copy of language runtime which is an unnecessary overhead (~a few MB) in the memory space as well as some languages do not support multiple language runtime existence in one process such as Go.

// At the start up of a filter chain, this name is passed to the module's HTTP filter initialization function.
// That way the module can decide which in-module filter implementation to use based on the name.
string filter_name = 2;

// The configuration for the filter chosen by filter_name. This is passed to the module's HTTP filter initialization function.
// Together with the filter_name, the module can decide which in-module filter implementation to use and
// fine-tune the behavior of the filter.
//
// For example, if a module has two filter implementations, one for logging and one for header manipulation,
// filter_name is used to choose either logging or header manipulation. The filter_config can be used to
// configure the logging level or the header manipulation behavior.
string filter_config = 3;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this a string instead of a google.protobuf.Any?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good q and this choice of string is intentional - this config will go across the C ABI boundary as-is, hence we need a plain binary representation, otherwise this will end up forcing all dynamic modules to be able to understand protobuf which is unnecessary. For example, each module would need a copy of protobuf dependency.

}
2 changes: 2 additions & 0 deletions api/versioning/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ proto_library(
"//envoy/extensions/compression/zstd/compressor/v3:pkg",
"//envoy/extensions/compression/zstd/decompressor/v3:pkg",
"//envoy/extensions/config/validators/minimum_clusters/v3:pkg",
"//envoy/extensions/dynamic_modules/v3:pkg",
"//envoy/extensions/early_data/v3:pkg",
"//envoy/extensions/filters/common/dependency/v3:pkg",
"//envoy/extensions/filters/common/fault/v3:pkg",
Expand All @@ -113,6 +114,7 @@ proto_library(
"//envoy/extensions/filters/http/custom_response/v3:pkg",
"//envoy/extensions/filters/http/decompressor/v3:pkg",
"//envoy/extensions/filters/http/dynamic_forward_proxy/v3:pkg",
"//envoy/extensions/filters/http/dynamic_modules/v3:pkg",
"//envoy/extensions/filters/http/ext_authz/v3:pkg",
"//envoy/extensions/filters/http/ext_proc/v3:pkg",
"//envoy/extensions/filters/http/fault/v3:pkg",
Expand Down
1 change: 1 addition & 0 deletions docs/root/api-v3/common_messages/common_messages.rst
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,4 @@ Common messages
../config/core/v3/udp_socket_config.proto
../extensions/filters/common/set_filter_state/v3/value.proto
../config/core/v3/socket_cmsg_headers.proto
../extensions/dynamic_modules/v3/dynamic_modules.proto
48 changes: 48 additions & 0 deletions source/extensions/filters/http/dynamic_modules/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
load(
"//bazel:envoy_build_system.bzl",
"envoy_cc_library",
"envoy_extension_package",
)

licenses(["notice"]) # Apache 2

envoy_extension_package()

envoy_cc_library(
name = "filter_config_lib",
srcs = ["filter_config.cc"],
hdrs = ["filter_config.h"],
deps = [
"//source/extensions/dynamic_modules:dynamic_modules_lib",
"//source/extensions/filters/http/common:pass_through_filter_lib",
],
)

envoy_cc_library(
name = "filter_lib",
srcs = ["filter.cc"],
hdrs = ["filter.h"],
deps = [
":filter_config_lib",
"//source/extensions/filters/http/common:pass_through_filter_lib",
],
)

envoy_cc_library(
name = "factory_lib",
srcs = ["factory.cc"],
hdrs = ["factory.h"],
deps = [
":filter_config_lib",
":filter_lib",
"@envoy_api//envoy/extensions/filters/http/dynamic_modules/v3:pkg_cc_proto",
],
)

envoy_cc_library(
name = "factory_registration",
srcs = ["factory_registration.cc"],
deps = [
":factory_lib",
],
)
43 changes: 43 additions & 0 deletions source/extensions/filters/http/dynamic_modules/factory.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
#include "source/extensions/filters/http/dynamic_modules/factory.h"

#include "source/extensions/filters/http/dynamic_modules/filter.h"
#include "source/extensions/filters/http/dynamic_modules/filter_config.h"

namespace Envoy {
namespace Server {
namespace Configuration {

absl::StatusOr<Http::FilterFactoryCb> DynamicModuleConfigFactory::createFilterFactoryFromProto(
const Protobuf::Message& raw_config, const std::string&, FactoryContext& context) {

const auto proto_config = Envoy::MessageUtil::downcastAndValidate<const FilterConfig&>(
raw_config, context.messageValidationVisitor());

const auto& module_config = proto_config.dynamic_module_config();
// TODO(mathetake): add support for other data source.
if (!module_config.object_file().has_filename()) {
return absl::InvalidArgumentError(
"Only filename is supported as a data source of dynamic module object file");
}
const auto dynamic_module = Extensions::DynamicModules::newDynamicModule(
module_config.object_file().filename(), module_config.do_not_close());
if (!dynamic_module.ok()) {
return absl::InvalidArgumentError("Failed to load dynamic module: " +
std::string(dynamic_module.status().message()));
}
auto filter_config = std::make_shared<
Envoy::Extensions::DynamicModules::HttpFilters::DynamicModuleHttpFilterConfig>(
proto_config.filter_name(), proto_config.filter_config(), dynamic_module.value());

return [filter_config](Http::FilterChainFactoryCallbacks& callbacks) -> void {
auto filter =
std::make_shared<Envoy::Extensions::DynamicModules::HttpFilters::DynamicModuleHttpFilter>(
filter_config);
callbacks.addStreamDecoderFilter(filter);
callbacks.addStreamEncoderFilter(filter);
};
}

} // namespace Configuration
} // namespace Server
} // namespace Envoy
30 changes: 30 additions & 0 deletions source/extensions/filters/http/dynamic_modules/factory.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
#pragma once

#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 "envoy/server/filter_config.h"

#include "source/extensions/dynamic_modules/dynamic_modules.h"

namespace Envoy {
namespace Server {
namespace Configuration {

using FilterConfig = envoy::extensions::filters::http::dynamic_modules::v3::DynamicModuleFilter;

class DynamicModuleConfigFactory : public NamedHttpFilterConfigFactory {
public:
absl::StatusOr<Http::FilterFactoryCb>
createFilterFactoryFromProto(const Protobuf::Message& raw_config, const std::string&,
FactoryContext& context) override;

ProtobufTypes::MessagePtr createEmptyConfigProto() override {
return ProtobufTypes::MessagePtr{new FilterConfig()};
}

std::string name() const override { return "envoy.extensions.filters.http.dynamic_modules"; }
};

} // namespace Configuration
} // namespace Server
} // namespace Envoy
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
#pragma once

#include "envoy/registry/registry.h"
#include "envoy/server/filter_config.h"

#include "source/extensions/filters/http/dynamic_modules/factory.h"

namespace Envoy {
namespace Server {
namespace Configuration {

REGISTER_FACTORY(DynamicModuleConfigFactory, NamedHttpFilterConfigFactory);

} // namespace Configuration
} // namespace Server
} // namespace Envoy
57 changes: 57 additions & 0 deletions source/extensions/filters/http/dynamic_modules/filter.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
#include "source/extensions/filters/http/dynamic_modules/filter.h"

#include "envoy/server/filter_config.h"

namespace Envoy {
namespace Extensions {
namespace DynamicModules {
namespace HttpFilters {

void DynamicModuleHttpFilter::onStreamComplete() {}

void DynamicModuleHttpFilter::onDestroy(){};

FilterHeadersStatus DynamicModuleHttpFilter::decodeHeaders(RequestHeaderMap&, bool) {
return FilterHeadersStatus::Continue;
};

FilterDataStatus DynamicModuleHttpFilter::decodeData(Buffer::Instance&, bool) {
return FilterDataStatus::Continue;
};

FilterTrailersStatus DynamicModuleHttpFilter::decodeTrailers(RequestTrailerMap&) {
return FilterTrailersStatus::Continue;
}

FilterMetadataStatus DynamicModuleHttpFilter::decodeMetadata(MetadataMap&) {
return FilterMetadataStatus::Continue;
}

void DynamicModuleHttpFilter::decodeComplete() {}

Filter1xxHeadersStatus DynamicModuleHttpFilter::encode1xxHeaders(ResponseHeaderMap&) {
return Filter1xxHeadersStatus::Continue;
}

FilterHeadersStatus DynamicModuleHttpFilter::encodeHeaders(ResponseHeaderMap&, bool) {
return FilterHeadersStatus::Continue;
};

FilterDataStatus DynamicModuleHttpFilter::encodeData(Buffer::Instance&, bool) {
return FilterDataStatus::Continue;
};

FilterTrailersStatus DynamicModuleHttpFilter::encodeTrailers(ResponseTrailerMap&) {
return FilterTrailersStatus::Continue;
};

FilterMetadataStatus DynamicModuleHttpFilter::encodeMetadata(MetadataMap&) {
return FilterMetadataStatus::Continue;
}

void DynamicModuleHttpFilter::encodeComplete(){};

} // namespace HttpFilters
} // namespace DynamicModules
} // namespace Extensions
} // namespace Envoy
Loading