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

ext_proc: send attributes #31090

Merged
Merged
Show file tree
Hide file tree
Changes from 46 commits
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
87cf9ba
extract attributes changes from #29069
jbohanon Nov 8, 2023
45ec23d
fix ordering test constructor
jbohanon Nov 8, 2023
0a9f228
don't do CEL stuff if no attributes are configured
jbohanon Nov 16, 2023
6826a1c
Merge branch 'main' into feature/ext_proc/request-response-attributes
jbohanon Nov 16, 2023
9857119
fix variable shadowing
jbohanon Nov 17, 2023
c26d44c
Merge branch 'main' into feature/ext_proc/request-response-attributes
jbohanon Nov 17, 2023
db599b9
Merge branch 'main' into feature/ext_proc/request-response-attributes
jbohanon Nov 27, 2023
a4454be
refactor matching utils out
jbohanon Nov 28, 2023
34b8fbc
refactor matching utils out
jbohanon Nov 28, 2023
d5ce702
Merge branch 'main' into feature/ext_proc/request-response-attributes
jbohanon Nov 28, 2023
3631683
fix pointer/references
jbohanon Nov 29, 2023
47c2b46
where are my CEL objects going
jbohanon Nov 30, 2023
86ae01f
fix lifetime issue and clean up
jbohanon Nov 30, 2023
0d8f5d4
fix runtime feature format
jbohanon Nov 30, 2023
dc692e1
declare iter as reference
jbohanon Nov 30, 2023
8a28f0d
Merge branch 'main' into feature/ext_proc/request-response-attributes
jbohanon Nov 30, 2023
8d34e78
fix tests and clean up
jbohanon Nov 30, 2023
fd598b9
skip fuzzing request and response attributes on ASAN_FUZZER due to un…
jbohanon Dec 1, 2023
2954fc7
use unique_ptr instead of absl::optional for struct, reference for ac…
jbohanon Dec 4, 2023
520d511
use object to store pair of parsed, compiled expression
jbohanon Dec 5, 2023
2778b97
Merge branch 'main' into feature/ext_proc/request-response-attributes
jbohanon Dec 5, 2023
f6bbcf7
inline CelExpression initialization into emplace call
jbohanon Dec 15, 2023
fbdbfb4
raw ptr working
jbohanon Dec 15, 2023
5192143
Merge branch 'main' into feature/ext_proc/request-response-attributes
jbohanon Dec 15, 2023
58faac1
update dep allowlist
jbohanon Dec 15, 2023
19fdb00
use serverFactoryContext to get builder singleton
jbohanon Dec 15, 2023
8948d0f
update dep allowlist
jbohanon Dec 15, 2023
fe68f15
PR comments
jbohanon Dec 15, 2023
d6cb932
kick CI
jbohanon Dec 15, 2023
ca13534
Merge branch 'main' into feature/ext_proc/request-response-attributes
jbohanon Dec 19, 2023
ae4258d
plumb through local info
jbohanon Dec 19, 2023
8e02036
kick CI -- RBE cache error in iOS
jbohanon Dec 19, 2023
d4d94e6
add inludes for MockLocalInfo
jbohanon Dec 19, 2023
906183f
Merge branch 'main' into feature/ext_proc/request-response-attributes
jbohanon Dec 20, 2023
ebd053c
Merge branch 'main' into feature/ext_proc/request-response-attributes
jbohanon Jan 8, 2024
7019dd1
kick CI
jbohanon Jan 8, 2024
defdd94
kick CI
jbohanon Jan 9, 2024
c86a731
kick CI
jbohanon Jan 9, 2024
0b3e715
kick CI
jbohanon Jan 15, 2024
374770f
Merge branch 'main' into feature/ext_proc/request-response-attributes
jbohanon Jan 15, 2024
a3e689f
revert unwanted local build change
jbohanon Jan 15, 2024
2c716b7
Merge branch 'main' into feature/ext_proc/request-response-attributes
jbohanon Jan 15, 2024
9948c57
add skip_on_windows due to recent CEL bump
jbohanon Jan 16, 2024
7d58055
Merge branch 'main' into feature/ext_proc/request-response-attributes
jbohanon Jan 16, 2024
fcaa5af
fix BUILD
jbohanon Jan 16, 2024
5fac8a5
skip client build on windows
jbohanon Jan 17, 2024
970fb4d
MORE SKIP ON WINDOWS
jbohanon Jan 17, 2024
a43136e
add filter to windows skip in bazel/repositories.bzl
jbohanon Jan 18, 2024
cfe7006
Merge branch 'main' into feature/ext_proc/request-response-attributes
jbohanon Jan 18, 2024
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
3 changes: 0 additions & 3 deletions api/envoy/extensions/filters/http/ext_proc/v3/ext_proc.proto
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE;
// **Current Implementation Status:**
// All options and processing modes are implemented except for the following:
//
// * Request and response attributes are not sent and not processed.
// * Dynamic metadata in responses from the external processor is ignored.
// * "async mode" is not implemented.

Expand Down Expand Up @@ -125,15 +124,13 @@ message ExternalProcessor {
// for a reply.
bool async_mode = 4;

// [#not-implemented-hide:]
// Envoy provides a number of :ref:`attributes <arch_overview_attributes>`
// for expressive policies. Each attribute name provided in this field will be
// matched against that list and populated in the request_headers message.
// See the :ref:`attribute documentation <arch_overview_request_attributes>`
// for the list of supported attributes and their types.
repeated string request_attributes = 5;

// [#not-implemented-hide:]
// Envoy provides a number of :ref:`attributes <arch_overview_attributes>`
// for expressive policies. Each attribute name provided in this field will be
// matched against that list and populated in the response_headers message.
Expand Down
2 changes: 2 additions & 0 deletions bazel/repository_locations.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -1214,6 +1214,7 @@ REPOSITORY_LOCATIONS_SPEC = dict(
"envoy.access_loggers.wasm",
"envoy.bootstrap.wasm",
"envoy.rate_limit_descriptors.expr",
"envoy.filters.http.ext_proc",
"envoy.filters.http.rate_limit_quota",
"envoy.filters.http.rbac",
"envoy.filters.http.wasm",
Expand Down Expand Up @@ -1243,6 +1244,7 @@ REPOSITORY_LOCATIONS_SPEC = dict(
"envoy.formatter.cel",
"envoy.bootstrap.wasm",
"envoy.rate_limit_descriptors.expr",
"envoy.filters.http.ext_proc",
"envoy.filters.http.rate_limit_quota",
"envoy.filters.http.rbac",
"envoy.filters.http.wasm",
Expand Down
7 changes: 7 additions & 0 deletions changelogs/current.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,12 @@ removed_config_or_runtime:
# *Normally occurs at the end of the* :ref:`deprecation period <deprecated>`

new_features:
- area: ext_proc
change: |
implemented
:ref:`request_attributes <envoy_v3_api_field_extensions.filters.http.ext_proc.v3.ExternalProcessor.request_attributes>`
and
:ref:`response_attributes <envoy_v3_api_field_extensions.filters.http.ext_proc.v3.ExternalProcessor.response_attributes>`
config APIs to enable sending and receiving attributes to/from the external processing server.

deprecated:
31 changes: 31 additions & 0 deletions source/extensions/filters/http/ext_proc/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@ envoy_cc_library(
"ext_proc.h",
"processor_state.h",
],
tags = ["skip_on_windows"],
deps = [
":client_interface",
":matching_utils_lib",
":mutation_utils_lib",
"//envoy/event:timer_interface",
"//envoy/http:filter_interface",
Expand All @@ -44,6 +46,7 @@ envoy_cc_extension(
name = "config",
srcs = ["config.cc"],
hdrs = ["config.h"],
tags = ["skip_on_windows"],
deps = [
":client_lib",
":ext_proc",
Expand All @@ -55,6 +58,7 @@ envoy_cc_extension(
envoy_cc_library(
name = "client_interface",
hdrs = ["client.h"],
tags = ["skip_on_windows"],
deps = [
"//envoy/grpc:async_client_manager_interface",
"//envoy/grpc:status",
Expand All @@ -80,10 +84,37 @@ envoy_cc_library(
],
)

envoy_cc_library(
name = "matching_utils_lib",
srcs = ["matching_utils.cc"],
hdrs = ["matching_utils.h"],
copts = select({
"//bazel:windows_x86_64": [],
"//conditions:default": [
"-DUSE_CEL_PARSER",
],
}),
tags = ["skip_on_windows"],
deps = [
"//envoy/http:header_map_interface",
"//source/common/protobuf",
"//source/extensions/filters/common/expr:evaluator_lib",
"@com_google_cel_cpp//eval/public:cel_expr_builder_factory",
] + select(
{
"//bazel:windows_x86_64": [],
"//conditions:default": [
"@com_google_cel_cpp//parser",
],
},
),
)

envoy_cc_library(
name = "client_lib",
srcs = ["client_impl.cc"],
hdrs = ["client_impl.h"],
tags = ["skip_on_windows"],
deps = [
":client_interface",
"//envoy/grpc:async_client_interface",
Expand Down
17 changes: 11 additions & 6 deletions source/extensions/filters/http/ext_proc/config.cc
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include "source/extensions/filters/http/ext_proc/config.h"

#include "source/extensions/filters/common/expr/evaluator.h"
#include "source/extensions/filters/http/ext_proc/client_impl.h"
#include "source/extensions/filters/http/ext_proc/ext_proc.h"

Expand All @@ -15,9 +16,11 @@ Http::FilterFactoryCb ExternalProcessingFilterConfig::createFilterFactoryFromPro
PROTOBUF_GET_MS_OR_DEFAULT(proto_config, message_timeout, DefaultMessageTimeoutMs);
const uint32_t max_message_timeout_ms =
PROTOBUF_GET_MS_OR_DEFAULT(proto_config, max_message_timeout, DefaultMaxMessageTimeoutMs);
const auto filter_config =
std::make_shared<FilterConfig>(proto_config, std::chrono::milliseconds(message_timeout_ms),
max_message_timeout_ms, context.scope(), stats_prefix);
const auto filter_config = std::make_shared<FilterConfig>(
proto_config, std::chrono::milliseconds(message_timeout_ms), max_message_timeout_ms,
context.scope(), stats_prefix,
Envoy::Extensions::Filters::Common::Expr::getBuilder(context.serverFactoryContext()),
context.serverFactoryContext().localInfo());

return [filter_config, grpc_service = proto_config.grpc_service(),
&context](Http::FilterChainFactoryCallbacks& callbacks) {
Expand All @@ -44,9 +47,11 @@ ExternalProcessingFilterConfig::createFilterFactoryFromProtoWithServerContextTyp
PROTOBUF_GET_MS_OR_DEFAULT(proto_config, message_timeout, DefaultMessageTimeoutMs);
const uint32_t max_message_timeout_ms =
PROTOBUF_GET_MS_OR_DEFAULT(proto_config, max_message_timeout, DefaultMaxMessageTimeoutMs);
const auto filter_config =
std::make_shared<FilterConfig>(proto_config, std::chrono::milliseconds(message_timeout_ms),
max_message_timeout_ms, server_context.scope(), stats_prefix);
const auto filter_config = std::make_shared<FilterConfig>(
proto_config, std::chrono::milliseconds(message_timeout_ms), max_message_timeout_ms,
server_context.scope(), stats_prefix,
Envoy::Extensions::Filters::Common::Expr::getBuilder(server_context),
server_context.localInfo());

return [filter_config, grpc_service = proto_config.grpc_service(),
&server_context](Http::FilterChainFactoryCallbacks& callbacks) {
Expand Down
30 changes: 27 additions & 3 deletions source/extensions/filters/http/ext_proc/ext_proc.cc
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,8 @@ void Filter::onDestroy() {
}

FilterHeadersStatus Filter::onHeaders(ProcessorState& state,
Http::RequestOrResponseHeaderMap& headers, bool end_stream) {
Http::RequestOrResponseHeaderMap& headers, bool end_stream,
ProtobufWkt::Struct* proto) {
Copy link
Contributor

@yanjunxiang-google yanjunxiang-google Jan 16, 2024

Choose a reason for hiding this comment

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

Should proto be passed by reference?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We use nullptr to indicate that attributes are not configured. This is on Line 243

switch (openStream()) {
case StreamOpenState::Error:
return FilterHeadersStatus::StopIteration;
Expand All @@ -239,6 +240,9 @@ FilterHeadersStatus Filter::onHeaders(ProcessorState& state,
MutationUtils::headersToProto(headers, config_->allowedHeaders(), config_->disallowedHeaders(),
*headers_req->mutable_headers());
headers_req->set_end_of_stream(end_stream);
if (proto != nullptr) {
(*headers_req->mutable_attributes())[FilterName] = *proto;
}
state.onStartProcessorCall(std::bind(&Filter::onMessageTimeout, this), config_->messageTimeout(),
ProcessorState::CallbackState::HeadersCallback);
ENVOY_LOG(debug, "Sending headers message");
Expand All @@ -257,7 +261,17 @@ FilterHeadersStatus Filter::decodeHeaders(RequestHeaderMap& headers, bool end_st

FilterHeadersStatus status = FilterHeadersStatus::Continue;
if (decoding_state_.sendHeaders()) {
status = onHeaders(decoding_state_, headers, end_stream);
ProtobufWkt::Struct proto;

if (config_->expressionManager().hasRequestExpr()) {
auto activation_ptr = Filters::Common::Expr::createActivation(
&config_->expressionManager().localInfo(), decoding_state_.streamInfo(), &headers,
nullptr, nullptr);
proto = config_->expressionManager().evaluateRequestAttributes(*activation_ptr);
}

status = onHeaders(decoding_state_, headers, end_stream,
config_->expressionManager().hasRequestExpr() ? &proto : nullptr);
ENVOY_LOG(trace, "onHeaders returning {}", static_cast<int>(status));
} else {
ENVOY_LOG(trace, "decodeHeaders: Skipped header processing");
Expand Down Expand Up @@ -535,7 +549,17 @@ FilterHeadersStatus Filter::encodeHeaders(ResponseHeaderMap& headers, bool end_s

FilterHeadersStatus status = FilterHeadersStatus::Continue;
if (!processing_complete_ && encoding_state_.sendHeaders()) {
status = onHeaders(encoding_state_, headers, end_stream);
ProtobufWkt::Struct proto;

if (config_->expressionManager().hasResponseExpr()) {
auto activation_ptr = Filters::Common::Expr::createActivation(
&config_->expressionManager().localInfo(), encoding_state_.streamInfo(), nullptr,
&headers, nullptr);
proto = config_->expressionManager().evaluateResponseAttributes(*activation_ptr);
}

status = onHeaders(encoding_state_, headers, end_stream,
config_->expressionManager().hasResponseExpr() ? &proto : nullptr);
ENVOY_LOG(trace, "onHeaders returns {}", static_cast<int>(status));
} else {
ENVOY_LOG(trace, "encodeHeaders: Skipped header processing");
Expand Down
32 changes: 16 additions & 16 deletions source/extensions/filters/http/ext_proc/ext_proc.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "source/extensions/filters/common/mutation_rules/mutation_rules.h"
#include "source/extensions/filters/http/common/pass_through_filter.h"
#include "source/extensions/filters/http/ext_proc/client.h"
#include "source/extensions/filters/http/ext_proc/matching_utils.h"
#include "source/extensions/filters/http/ext_proc/processor_state.h"

namespace Envoy {
Expand Down Expand Up @@ -126,7 +127,9 @@ class FilterConfig {
FilterConfig(const envoy::extensions::filters::http::ext_proc::v3::ExternalProcessor& config,
const std::chrono::milliseconds message_timeout,
const uint32_t max_message_timeout_ms, Stats::Scope& scope,
const std::string& stats_prefix)
const std::string& stats_prefix,
Extensions::Filters::Common::Expr::BuilderInstanceSharedPtr builder,
const LocalInfo::LocalInfo& local_info)
: failure_mode_allow_(config.failure_mode_allow()),
disable_clear_route_cache_(config.disable_clear_route_cache()),
message_timeout_(message_timeout), max_message_timeout_ms_(max_message_timeout_ms),
Expand All @@ -136,7 +139,9 @@ class FilterConfig {
allow_mode_override_(config.allow_mode_override()),
disable_immediate_response_(config.disable_immediate_response()),
allowed_headers_(initHeaderMatchers(config.forward_rules().allowed_headers())),
disallowed_headers_(initHeaderMatchers(config.forward_rules().disallowed_headers())) {}
disallowed_headers_(initHeaderMatchers(config.forward_rules().disallowed_headers())),
expression_manager_(builder, local_info, config.request_attributes(),
config.response_attributes()) {}

bool failureModeAllow() const { return failure_mode_allow_; }

Expand Down Expand Up @@ -164,25 +169,16 @@ class FilterConfig {
return disallowed_headers_;
}

const Envoy::ProtobufWkt::Struct& filterMetadata() const { return filter_metadata_; }
const ProtobufWkt::Struct& filterMetadata() const { return filter_metadata_; }

const ExpressionManager& expressionManager() const { return expression_manager_; }

private:
ExtProcFilterStats generateStats(const std::string& prefix,
const std::string& filter_stats_prefix, Stats::Scope& scope) {
const std::string final_prefix = absl::StrCat(prefix, "ext_proc.", filter_stats_prefix);
return {ALL_EXT_PROC_FILTER_STATS(POOL_COUNTER_PREFIX(scope, final_prefix))};
}
const std::vector<Matchers::StringMatcherPtr>
initHeaderMatchers(const envoy::type::matcher::v3::ListStringMatcher& header_list) {
std::vector<Matchers::StringMatcherPtr> header_matchers;
for (const auto& matcher : header_list.patterns()) {
header_matchers.push_back(
std::make_unique<Matchers::StringMatcherImpl<envoy::type::matcher::v3::StringMatcher>>(
matcher));
}
return header_matchers;
}

const bool failure_mode_allow_;
const bool disable_clear_route_cache_;
const std::chrono::milliseconds message_timeout_;
Expand All @@ -191,7 +187,7 @@ class FilterConfig {
ExtProcFilterStats stats_;
const envoy::extensions::filters::http::ext_proc::v3::ProcessingMode processing_mode_;
const Filters::Common::MutationRules::Checker mutation_checker_;
const Envoy::ProtobufWkt::Struct filter_metadata_;
const ProtobufWkt::Struct filter_metadata_;
// If set to true, allow the processing mode to be modified by the ext_proc response.
const bool allow_mode_override_;
// If set to true, disable the immediate response from the ext_proc server, which means
Expand All @@ -201,6 +197,8 @@ class FilterConfig {
const std::vector<Matchers::StringMatcherPtr> allowed_headers_;
// Empty disallowed_header_ means disallow nothing, i.e, allow all.
const std::vector<Matchers::StringMatcherPtr> disallowed_headers_;

const ExpressionManager expression_manager_;
};

using FilterConfigSharedPtr = std::shared_ptr<FilterConfig>;
Expand Down Expand Up @@ -315,7 +313,9 @@ class Filter : public Logger::Loggable<Logger::Id::ext_proc>,
void sendImmediateResponse(const envoy::service::ext_proc::v3::ImmediateResponse& response);

Http::FilterHeadersStatus onHeaders(ProcessorState& state,
Http::RequestOrResponseHeaderMap& headers, bool end_stream);
Http::RequestOrResponseHeaderMap& headers, bool end_stream,
ProtobufWkt::Struct* proto);

// Return a pair of whether to terminate returning the current result.
std::pair<bool, Http::FilterDataStatus> sendStreamChunk(ProcessorState& state);
Http::FilterDataStatus onData(ProcessorState& state, Buffer::Instance& data, bool end_stream);
Expand Down
Loading