From 7101e70189c40a8e77fe5dc102cca58886d434cc Mon Sep 17 00:00:00 2001 From: Yanjun Xiang Date: Tue, 21 Nov 2023 22:46:40 +0000 Subject: [PATCH] Revert ext_proc: send attributes #30781 Signed-off-by: Yanjun Xiang --- .../filters/http/ext_proc/v3/ext_proc.proto | 4 + changelogs/current.yaml | 7 -- source/extensions/filters/http/ext_proc/BUILD | 25 +---- .../filters/http/ext_proc/config.cc | 13 ++- .../extensions/filters/http/ext_proc/config.h | 3 +- .../filters/http/ext_proc/ext_proc.cc | 91 +------------------ .../filters/http/ext_proc/ext_proc.h | 38 +------- .../filters/http/ext_proc/processor_state.h | 6 -- test/extensions/filters/http/ext_proc/BUILD | 18 ---- .../filters/http/ext_proc/config_test.cc | 16 +--- .../ext_proc/ext_proc_integration_test.cc | 68 +------------- .../filters/http/ext_proc/filter_test.cc | 6 +- .../filters/http/ext_proc/ordering_test.cc | 6 +- ...d-ext_proc_unit_test_fuzz-5854655254757376 | 1 + ..._proc_unit_test_fuzz-6492125776445440.fuzz | 17 ++++ .../unit_test_fuzz/ext_proc_unit_test_fuzz.cc | 4 +- 16 files changed, 47 insertions(+), 276 deletions(-) create mode 100644 test/extensions/filters/http/ext_proc/unit_test_fuzz/ext_proc_corpus/clusterfuzz-testcase-minimized-ext_proc_unit_test_fuzz-5854655254757376 create mode 100644 test/extensions/filters/http/ext_proc/unit_test_fuzz/ext_proc_corpus/clusterfuzz-testcase-minimized-ext_proc_unit_test_fuzz-6492125776445440.fuzz diff --git a/api/envoy/extensions/filters/http/ext_proc/v3/ext_proc.proto b/api/envoy/extensions/filters/http/ext_proc/v3/ext_proc.proto index 37d289ec6a6f..c9500486a3a5 100644 --- a/api/envoy/extensions/filters/http/ext_proc/v3/ext_proc.proto +++ b/api/envoy/extensions/filters/http/ext_proc/v3/ext_proc.proto @@ -125,6 +125,7 @@ message ExternalProcessor { // for a reply. bool async_mode = 4; + // [#not-implemented-hide:] // Envoy provides a number of :ref:`attributes ` // for expressive policies. Each attribute name provided in this field will be // matched against that list and populated in the request_headers message. @@ -132,6 +133,7 @@ message ExternalProcessor { // for the list of supported attributes and their types. repeated string request_attributes = 5; + // [#not-implemented-hide:] // Envoy provides a number of :ref:`attributes ` // for expressive policies. Each attribute name provided in this field will be // matched against that list and populated in the response_headers message. @@ -255,10 +257,12 @@ message ExtProcOverrides { // Set a different asynchronous processing option than the default. bool async_mode = 2; + // [#not-implemented-hide:] // Set different optional attributes than the default setting of the // ``request_attributes`` field. repeated string request_attributes = 3; + // [#not-implemented-hide:] // Set different optional properties than the default setting of the // ``response_attributes`` field. repeated string response_attributes = 4; diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 69f2167c96b9..5a3852ecd813 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -203,12 +203,5 @@ new_features: - area: tracing change: | Provide initial span attributes to a sampler used in the OpenTelemetry tracer. -- area: ext_proc - change: | - implemented - :ref:`request_attributes ` - and - :ref:`response_attributes ` - config APIs to enable sending and receiving attributes from/to the external processing server. deprecated: diff --git a/source/extensions/filters/http/ext_proc/BUILD b/source/extensions/filters/http/ext_proc/BUILD index ef2c32690f8e..484395f170d3 100644 --- a/source/extensions/filters/http/ext_proc/BUILD +++ b/source/extensions/filters/http/ext_proc/BUILD @@ -19,12 +19,6 @@ envoy_cc_library( "ext_proc.h", "processor_state.h", ], - copts = select({ - "//bazel:windows_x86_64": [], - "//conditions:default": [ - "-DUSE_CEL_PARSER", - ], - }), deps = [ ":client_interface", ":mutation_utils_lib", @@ -35,41 +29,24 @@ envoy_cc_library( "//source/common/buffer:buffer_lib", "//source/common/protobuf", "//source/common/runtime:runtime_features_lib", - "//source/extensions/filters/common/expr:evaluator_lib", "//source/extensions/filters/common/mutation_rules:mutation_rules_lib", "//source/extensions/filters/http/common:pass_through_filter_lib", "@com_google_absl//absl/status", "@com_google_absl//absl/strings:str_format", - "@com_google_cel_cpp//eval/public:builtin_func_registrar", - "@com_google_cel_cpp//eval/public:cel_expr_builder_factory", "@envoy_api//envoy/config/common/mutation_rules/v3:pkg_cc_proto", "@envoy_api//envoy/config/core/v3:pkg_cc_proto", "@envoy_api//envoy/extensions/filters/http/ext_proc/v3:pkg_cc_proto", "@envoy_api//envoy/service/ext_proc/v3:pkg_cc_proto", - ] + select( - { - "//bazel:windows_x86_64": [], - "//conditions:default": [ - "@com_google_cel_cpp//parser", - ], - }, - ), + ], ) envoy_cc_extension( name = "config", srcs = ["config.cc"], hdrs = ["config.h"], - copts = select({ - "//bazel:windows_x86_64": [], - "//conditions:default": [ - "-DUSE_CEL_PARSER", - ], - }), deps = [ ":client_lib", ":ext_proc", - "//source/extensions/filters/common/expr:evaluator_lib", "//source/extensions/filters/http/common:factory_base_lib", "@envoy_api//envoy/extensions/filters/http/ext_proc/v3:pkg_cc_proto", ], diff --git a/source/extensions/filters/http/ext_proc/config.cc b/source/extensions/filters/http/ext_proc/config.cc index 24edb8dd988e..c38dae46b50f 100644 --- a/source/extensions/filters/http/ext_proc/config.cc +++ b/source/extensions/filters/http/ext_proc/config.cc @@ -15,9 +15,9 @@ 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( - proto_config, std::chrono::milliseconds(message_timeout_ms), max_message_timeout_ms, - context.scope(), stats_prefix, Envoy::Extensions::Filters::Common::Expr::getBuilder(context)); + const auto filter_config = + std::make_shared(proto_config, std::chrono::milliseconds(message_timeout_ms), + max_message_timeout_ms, context.scope(), stats_prefix); return [filter_config, grpc_service = proto_config.grpc_service(), &context](Http::FilterChainFactoryCallbacks& callbacks) { @@ -45,10 +45,9 @@ 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( - 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)); + const auto filter_config = + std::make_shared(proto_config, std::chrono::milliseconds(message_timeout_ms), + max_message_timeout_ms, server_context.scope(), stats_prefix); return [filter_config, grpc_service = proto_config.grpc_service(), &server_context](Http::FilterChainFactoryCallbacks& callbacks) { diff --git a/source/extensions/filters/http/ext_proc/config.h b/source/extensions/filters/http/ext_proc/config.h index e18452e262d0..a2912466eb6b 100644 --- a/source/extensions/filters/http/ext_proc/config.h +++ b/source/extensions/filters/http/ext_proc/config.h @@ -5,7 +5,6 @@ #include "envoy/extensions/filters/http/ext_proc/v3/ext_proc.pb.h" #include "envoy/extensions/filters/http/ext_proc/v3/ext_proc.pb.validate.h" -#include "source/extensions/filters/common/expr/evaluator.h" #include "source/extensions/filters/http/common/factory_base.h" namespace Envoy { @@ -30,7 +29,7 @@ class ExternalProcessingFilterConfig Router::RouteSpecificFilterConfigConstSharedPtr createRouteSpecificFilterConfigTyped( const envoy::extensions::filters::http::ext_proc::v3::ExtProcPerRoute& proto_config, - Server::Configuration::ServerFactoryContext&, + Server::Configuration::ServerFactoryContext& context, ProtobufMessage::ValidationVisitor& validator) override; Http::FilterFactoryCb createFilterFactoryFromProtoWithServerContextTyped( diff --git a/source/extensions/filters/http/ext_proc/ext_proc.cc b/source/extensions/filters/http/ext_proc/ext_proc.cc index 8f921a7659e8..754cc6d17d5a 100644 --- a/source/extensions/filters/http/ext_proc/ext_proc.cc +++ b/source/extensions/filters/http/ext_proc/ext_proc.cc @@ -8,10 +8,6 @@ #include "absl/strings/str_format.h" -#if defined(USE_CEL_PARSER) -#include "parser/parser.h" -#endif - namespace Envoy { namespace Extensions { namespace HttpFilters { @@ -117,27 +113,6 @@ ExtProcLoggingInfo::grpcCalls(envoy::config::core::v3::TrafficDirection traffic_ : encoding_processor_grpc_calls_; } -absl::flat_hash_map -FilterConfig::initExpressions(const Protobuf::RepeatedPtrField& matchers) const { - absl::flat_hash_map expressions; -#if defined(USE_CEL_PARSER) - for (const auto& matcher : matchers) { - auto parse_status = google::api::expr::parser::Parse(matcher); - if (!parse_status.ok()) { - throw EnvoyException("Unable to parse descriptor expression: " + - parse_status.status().ToString()); - } - expressions.emplace(matcher, Extensions::Filters::Common::Expr::createExpression( - builder_->builder(), parse_status.value().expr())); - } -#else - ENVOY_LOG(warn, "CEL expression parsing is not available for use in this environment." - " Attempted to parse " + - std::to_string(matchers.size()) + " expressions"); -#endif - return expressions; -} - FilterConfigPerRoute::FilterConfigPerRoute(const ExtProcPerRoute& config) : disabled_(config.disabled()) { if (config.has_overrides()) { @@ -226,8 +201,7 @@ void Filter::onDestroy() { } FilterHeadersStatus Filter::onHeaders(ProcessorState& state, - Http::RequestOrResponseHeaderMap& headers, bool end_stream, - absl::optional proto) { + Http::RequestOrResponseHeaderMap& headers, bool end_stream) { switch (openStream()) { case StreamOpenState::Error: return FilterHeadersStatus::StopIteration; @@ -245,9 +219,6 @@ 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.has_value()) { - (*headers_req->mutable_attributes())[FilterName] = proto.value(); - } state.onStartProcessorCall(std::bind(&Filter::onMessageTimeout, this), config_->messageTimeout(), ProcessorState::CallbackState::HeadersCallback); ENVOY_LOG(debug, "Sending headers message"); @@ -257,48 +228,6 @@ FilterHeadersStatus Filter::onHeaders(ProcessorState& state, return FilterHeadersStatus::StopIteration; } -const absl::optional Filter::evaluateAttributes( - Filters::Common::Expr::ActivationPtr activation, - const absl::flat_hash_map& - expr) { - absl::optional proto; - if (expr.size() > 0) { - proto.emplace(ProtobufWkt::Struct{}); - for (const auto& hash_entry : expr) { - ProtobufWkt::Arena arena; - const auto result = hash_entry.second.get()->Evaluate(*activation, &arena); - if (!result.ok()) { - // TODO: Stats? - continue; - } - - if (result.value().IsError()) { - ENVOY_LOG(trace, "error parsing cel expression {}", hash_entry.first); - continue; - } - - ProtobufWkt::Value value; - switch (result.value().type()) { - case google::api::expr::runtime::CelValue::Type::kBool: - value.set_bool_value(result.value().BoolOrDie()); - break; - case google::api::expr::runtime::CelValue::Type::kNullType: - value.set_null_value(ProtobufWkt::NullValue{}); - break; - case google::api::expr::runtime::CelValue::Type::kDouble: - value.set_number_value(result.value().DoubleOrDie()); - break; - default: - value.set_string_value(Filters::Common::Expr::print(result.value())); - } - - (*(proto.value()).mutable_fields())[hash_entry.first] = value; - } - } - - return proto; -} - FilterHeadersStatus Filter::decodeHeaders(RequestHeaderMap& headers, bool end_stream) { ENVOY_LOG(trace, "decodeHeaders: end_stream = {}", end_stream); mergePerRouteConfig(); @@ -308,14 +237,7 @@ FilterHeadersStatus Filter::decodeHeaders(RequestHeaderMap& headers, bool end_st FilterHeadersStatus status = FilterHeadersStatus::Continue; if (decoding_state_.sendHeaders()) { - absl::optional proto; - if (!config_->requestExpr().empty()) { - auto activation_ptr = Filters::Common::Expr::createActivation(decoding_state_.streamInfo(), - &headers, nullptr, nullptr); - proto = evaluateAttributes(std::move(activation_ptr), config_->requestExpr()); - } - - status = onHeaders(decoding_state_, headers, end_stream, proto); + status = onHeaders(decoding_state_, headers, end_stream); ENVOY_LOG(trace, "onHeaders returning {}", static_cast(status)); } else { ENVOY_LOG(trace, "decodeHeaders: Skipped header processing"); @@ -593,14 +515,7 @@ FilterHeadersStatus Filter::encodeHeaders(ResponseHeaderMap& headers, bool end_s FilterHeadersStatus status = FilterHeadersStatus::Continue; if (!processing_complete_ && encoding_state_.sendHeaders()) { - absl::optional proto; - if (!config_->responseExpr().empty()) { - auto activation_ptr = Filters::Common::Expr::createActivation(encoding_state_.streamInfo(), - nullptr, &headers, nullptr); - proto = evaluateAttributes(std::move(activation_ptr), config_->responseExpr()); - } - - status = onHeaders(encoding_state_, headers, end_stream, proto); + status = onHeaders(encoding_state_, headers, end_stream); ENVOY_LOG(trace, "onHeaders returns {}", static_cast(status)); } else { ENVOY_LOG(trace, "encodeHeaders: Skipped header processing"); diff --git a/source/extensions/filters/http/ext_proc/ext_proc.h b/source/extensions/filters/http/ext_proc/ext_proc.h index 5500cee6c7af..6338cc4e2aae 100644 --- a/source/extensions/filters/http/ext_proc/ext_proc.h +++ b/source/extensions/filters/http/ext_proc/ext_proc.h @@ -21,7 +21,6 @@ #include "source/common/common/logger.h" #include "source/common/common/matchers.h" #include "source/common/protobuf/protobuf.h" -#include "source/extensions/filters/common/expr/evaluator.h" #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" @@ -122,13 +121,12 @@ class ExtProcLoggingInfo : public Envoy::StreamInfo::FilterState::Object { Upstream::HostDescriptionConstSharedPtr upstream_host_; }; -class FilterConfig : public Logger::Loggable { +class FilterConfig { public: 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, - Extensions::Filters::Common::Expr::BuilderInstanceSharedPtr builder) + const std::string& stats_prefix) : 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), @@ -138,9 +136,7 @@ class FilterConfig : public Logger::Loggable { 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())), - builder_(builder), request_expr_(initExpressions(config.request_attributes())), - response_expr_(initExpressions(config.response_attributes())) {} + disallowed_headers_(initHeaderMatchers(config.forward_rules().disallowed_headers())) {} bool failureModeAllow() const { return failure_mode_allow_; } @@ -170,16 +166,6 @@ class FilterConfig : public Logger::Loggable { const Envoy::ProtobufWkt::Struct& filterMetadata() const { return filter_metadata_; } - const absl::flat_hash_map& - requestExpr() const { - return request_expr_; - } - - const absl::flat_hash_map& - responseExpr() const { - return response_expr_; - } - private: ExtProcFilterStats generateStats(const std::string& prefix, const std::string& filter_stats_prefix, Stats::Scope& scope) { @@ -197,9 +183,6 @@ class FilterConfig : public Logger::Loggable { return header_matchers; } - absl::flat_hash_map - initExpressions(const Protobuf::RepeatedPtrField& matchers) const; - const bool failure_mode_allow_; const bool disable_clear_route_cache_; const std::chrono::milliseconds message_timeout_; @@ -218,13 +201,6 @@ class FilterConfig : public Logger::Loggable { const std::vector allowed_headers_; // Empty disallowed_header_ means disallow nothing, i.e, allow all. const std::vector disallowed_headers_; - - Extensions::Filters::Common::Expr::BuilderInstanceSharedPtr builder_; - - const absl::flat_hash_map - request_expr_; - const absl::flat_hash_map - response_expr_; }; using FilterConfigSharedPtr = std::shared_ptr; @@ -324,13 +300,7 @@ class Filter : public Logger::Loggable, void sendImmediateResponse(const envoy::service::ext_proc::v3::ImmediateResponse& response); Http::FilterHeadersStatus onHeaders(ProcessorState& state, - Http::RequestOrResponseHeaderMap& headers, bool end_stream, - absl::optional proto); - - const absl::optional evaluateAttributes( - Filters::Common::Expr::ActivationPtr activation, - const absl::flat_hash_map& - expr); + Http::RequestOrResponseHeaderMap& headers, bool end_stream); // Return a pair of whether to terminate returning the current result. std::pair sendStreamChunk(ProcessorState& state); Http::FilterDataStatus onData(ProcessorState& state, Buffer::Instance& data, bool end_stream); diff --git a/source/extensions/filters/http/ext_proc/processor_state.h b/source/extensions/filters/http/ext_proc/processor_state.h index c6c50fdecbbb..de4628864941 100644 --- a/source/extensions/filters/http/ext_proc/processor_state.h +++ b/source/extensions/filters/http/ext_proc/processor_state.h @@ -172,8 +172,6 @@ class ProcessorState : public Logger::Loggable { virtual envoy::service::ext_proc::v3::HttpTrailers* mutableTrailers(envoy::service::ext_proc::v3::ProcessingRequest& request) const PURE; - virtual StreamInfo::StreamInfo& streamInfo() PURE; - protected: void setBodyMode( envoy::extensions::filters::http::ext_proc::v3::ProcessingMode_BodySendMode body_mode); @@ -285,8 +283,6 @@ class DecodingProcessorState : public ProcessorState { void requestWatermark() override; void clearWatermark() override; - StreamInfo::StreamInfo& streamInfo() override { return decoder_callbacks_->streamInfo(); } - private: void setProcessingModeInternal( const envoy::extensions::filters::http::ext_proc::v3::ProcessingMode& mode); @@ -360,8 +356,6 @@ class EncodingProcessorState : public ProcessorState { void requestWatermark() override; void clearWatermark() override; - StreamInfo::StreamInfo& streamInfo() override { return encoder_callbacks_->streamInfo(); } - private: void setProcessingModeInternal( const envoy::extensions::filters::http::ext_proc::v3::ProcessingMode& mode); diff --git a/test/extensions/filters/http/ext_proc/BUILD b/test/extensions/filters/http/ext_proc/BUILD index 86489d81c259..a99fde2191ee 100644 --- a/test/extensions/filters/http/ext_proc/BUILD +++ b/test/extensions/filters/http/ext_proc/BUILD @@ -30,12 +30,6 @@ envoy_extension_cc_test( name = "filter_test", size = "small", srcs = ["filter_test.cc"], - copts = select({ - "//bazel:windows_x86_64": [], - "//conditions:default": [ - "-DUSE_CEL_PARSER", - ], - }), extension_names = ["envoy.filters.http.ext_proc"], deps = [ ":mock_server_lib", @@ -123,12 +117,6 @@ envoy_extension_cc_test( name = "ext_proc_integration_test", size = "large", # This test can take a while under tsan. srcs = ["ext_proc_integration_test.cc"], - copts = select({ - "//bazel:windows_x86_64": [], - "//conditions:default": [ - "-DUSE_CEL_PARSER", - ], - }), extension_names = ["envoy.filters.http.ext_proc"], shard_count = 4, tags = [ @@ -151,12 +139,6 @@ envoy_extension_cc_test( name = "streaming_integration_test", size = "large", srcs = ["streaming_integration_test.cc"], - copts = select({ - "//bazel:windows_x86_64": [], - "//conditions:default": [ - "-DUSE_CEL_PARSER", - ], - }), extension_names = ["envoy.filters.http.ext_proc"], tags = [ "cpu:3", diff --git a/test/extensions/filters/http/ext_proc/config_test.cc b/test/extensions/filters/http/ext_proc/config_test.cc index 47e36558a260..6cfa1d9196a9 100644 --- a/test/extensions/filters/http/ext_proc/config_test.cc +++ b/test/extensions/filters/http/ext_proc/config_test.cc @@ -21,12 +21,8 @@ TEST(HttpExtProcConfigTest, CorrectConfig) { target_uri: ext_proc_server stat_prefix: google failure_mode_allow: true - request_attributes: - - 'Foo' - - 'Bar' - - 'Baz' - response_attributes: - - 'More' + request_attributes: 'Foo, Bar, Baz' + response_attributes: More processing_mode: request_header_mode: send response_header_mode: skip @@ -58,12 +54,8 @@ TEST(HttpExtProcConfigTest, CorrectConfigServerContext) { target_uri: ext_proc_server stat_prefix: google failure_mode_allow: true - request_attributes: - - 'Foo' - - 'Bar' - - 'Baz' - response_attributes: - - 'More' + request_attributes: 'Foo, Bar, Baz' + response_attributes: More processing_mode: request_header_mode: send response_header_mode: skip diff --git a/test/extensions/filters/http/ext_proc/ext_proc_integration_test.cc b/test/extensions/filters/http/ext_proc/ext_proc_integration_test.cc index f15cff4fab68..a7e111a13ffd 100644 --- a/test/extensions/filters/http/ext_proc/ext_proc_integration_test.cc +++ b/test/extensions/filters/http/ext_proc/ext_proc_integration_test.cc @@ -79,7 +79,7 @@ class ExtProcIntegrationTest : public HttpIntegrationTest, scoped_runtime_.mergeValues( {{"envoy.reloadable_features.send_header_raw_value", header_raw_value_}}); scoped_runtime_.mergeValues( - {{"envoy.reloadable_features.immediate_response_use_filter_mutation_rule", + {{"envoy_reloadable_features_immediate_response_use_filter_mutation_rule", filter_mutation_rule_}}); config_helper_.addConfigModifier([this, config_option]( @@ -1570,7 +1570,6 @@ TEST_P(ExtProcIntegrationTest, GetAndRespondImmediately) { EXPECT_THAT(response->headers(), SingleHeaderValueIs("content-type", "application/json")); EXPECT_EQ("{\"reason\": \"Not authorized\"}", response->body()); } - TEST_P(ExtProcIntegrationTest, GetAndRespondImmediatelyWithLogging) { ConfigOptions config_option = {}; config_option.add_logging_filter = true; @@ -1837,28 +1836,6 @@ TEST_P(ExtProcIntegrationTest, GetAndImmediateRespondMutationAllowEnvoy) { EXPECT_THAT(response->headers(), SingleHeaderValueIs("x-envoy-foo", "bar")); } -// Test the filter using an ext_proc server that responds to the request_header message -// by sending back an immediate_response message with x-envoy header mutation. -// The deprecated default checker allows x-envoy headers to be mutated and should -// override config-level checkers if the runtime guard is disabled. -TEST_P(ExtProcIntegrationTest, GetAndRespondImmediatelyWithDefaultHeaderMutationChecker) { - // this is default, but setting explicitly for test clarity - filter_mutation_rule_ = "false"; - proto_config_.mutable_mutation_rules()->mutable_allow_envoy()->set_value(false); - initializeConfig(); - HttpIntegrationTest::initialize(); - auto response = sendDownstreamRequest(absl::nullopt); - processAndRespondImmediately(*grpc_upstreams_[0], true, [](ImmediateResponse& immediate) { - immediate.mutable_status()->set_code(envoy::type::v3::StatusCode::Unauthorized); - auto* hdr = immediate.mutable_headers()->add_set_headers(); - // Adding x-envoy header is allowed since default overrides config. - hdr->mutable_header()->set_key("x-envoy-foo"); - hdr->mutable_header()->set_value("bar"); - }); - verifyDownstreamResponse(*response, 401); - EXPECT_FALSE(response->headers().get(LowerCaseString("x-envoy-foo")).empty()); -} - // Test the filter with request body buffering enabled using // an ext_proc server that responds to the request_body message // by modifying a header that should cause an error. @@ -3308,47 +3285,4 @@ TEST_P(ExtProcIntegrationTest, SendBodyBufferedPartialWithTrailer) { verifyDownstreamResponse(*response, 200); } -#if defined(USE_CEL_PARSER) -// Test the filter using the default configuration by connecting to -// an ext_proc server that responds to the request_headers message -// by requesting to modify the request headers. -TEST_P(ExtProcIntegrationTest, GetAndSetRequestResponseAttributes) { - proto_config_.mutable_processing_mode()->set_request_header_mode(ProcessingMode::SEND); - proto_config_.mutable_processing_mode()->set_response_header_mode(ProcessingMode::SEND); - proto_config_.mutable_request_attributes()->Add("request.path"); - proto_config_.mutable_request_attributes()->Add("request.method"); - proto_config_.mutable_request_attributes()->Add("request.scheme"); - proto_config_.mutable_request_attributes()->Add("connection.mtls"); - proto_config_.mutable_response_attributes()->Add("response.code"); - proto_config_.mutable_response_attributes()->Add("response.code_details"); - - initializeConfig(); - HttpIntegrationTest::initialize(); - auto response = sendDownstreamRequest(absl::nullopt); - processRequestHeadersMessage( - *grpc_upstreams_[0], true, [](const HttpHeaders& req, HeadersResponse&) { - EXPECT_EQ(req.attributes().size(), 1); - auto proto_struct = req.attributes().at("envoy.filters.http.ext_proc"); - EXPECT_EQ(proto_struct.fields().at("request.path").string_value(), "/"); - EXPECT_EQ(proto_struct.fields().at("request.method").string_value(), "GET"); - EXPECT_EQ(proto_struct.fields().at("request.scheme").string_value(), "http"); - EXPECT_EQ(proto_struct.fields().at("connection.mtls").bool_value(), false); - return true; - }); - - handleUpstreamRequest(); - - processResponseHeadersMessage( - *grpc_upstreams_[0], false, [](const HttpHeaders& req, HeadersResponse&) { - EXPECT_EQ(req.attributes().size(), 1); - auto proto_struct = req.attributes().at("envoy.filters.http.ext_proc"); - EXPECT_EQ(proto_struct.fields().at("response.code").string_value(), "200"); - EXPECT_EQ(proto_struct.fields().at("response.code_details").string_value(), "via_upstream"); - return true; - }); - - verifyDownstreamResponse(*response, 200); -} -#endif - } // namespace Envoy diff --git a/test/extensions/filters/http/ext_proc/filter_test.cc b/test/extensions/filters/http/ext_proc/filter_test.cc index ad79eaadbe5a..7e68bd4f1ac5 100644 --- a/test/extensions/filters/http/ext_proc/filter_test.cc +++ b/test/extensions/filters/http/ext_proc/filter_test.cc @@ -121,10 +121,8 @@ class HttpFilterTest : public testing::Test { if (!yaml.empty()) { TestUtility::loadFromYaml(yaml, proto_config); } - config_ = std::make_shared( - proto_config, 200ms, 10000, *stats_store_.rootScope(), "", - std::make_shared( - Envoy::Extensions::Filters::Common::Expr::createBuilder(nullptr))); + config_ = + std::make_shared(proto_config, 200ms, 10000, *stats_store_.rootScope(), ""); filter_ = std::make_unique(config_, std::move(client_), proto_config.grpc_service()); filter_->setEncoderFilterCallbacks(encoder_callbacks_); EXPECT_CALL(encoder_callbacks_, encoderBufferLimit()).WillRepeatedly(Return(BufferSize)); diff --git a/test/extensions/filters/http/ext_proc/ordering_test.cc b/test/extensions/filters/http/ext_proc/ordering_test.cc index 79fbd7580042..744692ef22ff 100644 --- a/test/extensions/filters/http/ext_proc/ordering_test.cc +++ b/test/extensions/filters/http/ext_proc/ordering_test.cc @@ -70,10 +70,8 @@ class OrderingTest : public testing::Test { if (cb) { (*cb)(proto_config); } - config_ = std::make_shared( - proto_config, kMessageTimeout, kMaxMessageTimeoutMs, *stats_store_.rootScope(), "", - std::make_shared( - Envoy::Extensions::Filters::Common::Expr::createBuilder(nullptr))); + config_ = std::make_shared(proto_config, kMessageTimeout, kMaxMessageTimeoutMs, + *stats_store_.rootScope(), ""); filter_ = std::make_unique(config_, std::move(client_), proto_config.grpc_service()); filter_->setEncoderFilterCallbacks(encoder_callbacks_); filter_->setDecoderFilterCallbacks(decoder_callbacks_); diff --git a/test/extensions/filters/http/ext_proc/unit_test_fuzz/ext_proc_corpus/clusterfuzz-testcase-minimized-ext_proc_unit_test_fuzz-5854655254757376 b/test/extensions/filters/http/ext_proc/unit_test_fuzz/ext_proc_corpus/clusterfuzz-testcase-minimized-ext_proc_unit_test_fuzz-5854655254757376 new file mode 100644 index 000000000000..b89ceaf3addb --- /dev/null +++ b/test/extensions/filters/http/ext_proc/unit_test_fuzz/ext_proc_corpus/clusterfuzz-testcase-minimized-ext_proc_unit_test_fuzz-5854655254757376 @@ -0,0 +1 @@ +config { grpc_service { envoy_grpc { cluster_name: " " } } request_attributes: "!" } request { } response { response_body { } } \ No newline at end of file diff --git a/test/extensions/filters/http/ext_proc/unit_test_fuzz/ext_proc_corpus/clusterfuzz-testcase-minimized-ext_proc_unit_test_fuzz-6492125776445440.fuzz b/test/extensions/filters/http/ext_proc/unit_test_fuzz/ext_proc_corpus/clusterfuzz-testcase-minimized-ext_proc_unit_test_fuzz-6492125776445440.fuzz new file mode 100644 index 000000000000..1098365c84cc --- /dev/null +++ b/test/extensions/filters/http/ext_proc/unit_test_fuzz/ext_proc_corpus/clusterfuzz-testcase-minimized-ext_proc_unit_test_fuzz-6492125776445440.fuzz @@ -0,0 +1,17 @@ +config { + grpc_service { + envoy_grpc { + cluster_name: "2" + } + } + async_mode: true + request_attributes: "type.googleapis.com/test.fuzz.ProtoBody" + stat_prefix: "t. \"t.e...\n rewpo...1.e.pc0&...2..6..*..econ\"p\"\n}\nree.key: hod&\n }\n voy.t\331\236pe.m.v3.nree.\\00ree.\000{\000\000\000\"t.\n.e . .respo\\210\\252\\270\\354\\254\\254S....\\316\\254....\\3........... }\\\\n voy.tchtype.m.v3.nre.e.p\\\"p\\z\\n}\\nree.key: \\\":method\\\"\\n e.\\\\\\\\00ree.\\\\000{\\\\000\\\\000\\\\000\\\\\\\"t.e... \"t. \\\"t.e...\\n respo... \\\\\\\"t.espo...1.e.p\\\\\\\"p..ze. st.type.m.\\326\\261.Vee.ree\316\261.Vee.ree.M0..p^.I{.....z.nan.......\\\\\\\\31efixext_proc>/.[ethod\\\\x-envo..[.r..*.6..g.g&...r!..,..:p.-.`.s`.strevo3.ValueMpat3\\\\\\\\254ersle_f//////...............\\\\\\\\177\\\\\\\\177\\\\X177\\\\\\\\177\\\\\\\\177\\\\\\\\177\\\\\\\\177?.........Trve+......................envoy.reloazable_features.runtim........m\\177\\177\\177\\177\\177\\177H0\\\\\\\\\\\\\\\\000\\\\\\\\\\\\\\\\010.p\\\\\\\\\\\\\\\"\\\\\\\"\n stat_prefix: \"t. \\\"t.e...\\.M0e.p^.{...ze{.3.zep\"\n}\nrequest {\n}\nresponse {\n reqthod\"\n \322\202Hw\001 voy.tme.y.pv3.Va\\35;\\3eqn}entchervo3.ValueMatcher\"ValueMatcher\"\n \322\202H\\\":methodX\"\\n \\322\\202\\310\\215\\001 voy.type.m.v3.Va\\\\35;\\S\003e...........Q..........-A...........\316\247\322\234\002Q\002r\002P\001P\330\222\002P\001P\001t\302\277\002V\334\234\001\332\271\336\254\302\225\001P\001N\001q\335\201\002S\001|t\002r\302\203\002P\001N\327\202\302\240\001z~\330\265\332\275.......\\316\\205.................}.......nan.......\\\\316e: \"#\"\n }\n }\n processing_mode {\n response_header_mode: SEND\n request_trailer_mode: SEND\n n }\\n voy.type.m.4v.n1.\\\\n.\\\"e\\\"\\n}\\n }\\n voy.type.m.3v.nree.\\\\00re..ze. ste.\\000{\\000tat_prefix: }\\n voy.type.m.3v.nree.\\\\\\000tat_prefix: \\\"t. \\\\\\\"t\\003.e\\343\\220\\220QK.1.e.p\\\\\\\"p..ze. st.typVe..3m.vee.r.ee.M0e.p^.I{.....z.nan.../.\\\\1.63.\\\\\\\\\\\\256.......\\\\\\\\006.\\\\\\\\020\\\\\\\\001.........-.......7......{.Q.nfig {\\\\\\\\ngoogle.protobuf.Any ore.v...1.e.p\\\\\\\\\\\\\\\"p..ze. st.type.m.\\\\326\\\\261.Vee.ree.M0..p^.I{z#tatus\\\\\\\\n Bore.oy.configkcore.v.@....)....[.....c.ro.y.....or.y.config.c0.e.p\\\\010.\\\\\\\\\\\\..........\\\\316\\\\233....spo... }\\\\n xext_proc./.[r.....[*.6..g.g&...r!..,..:p.-.`.strevo3.ValueMp4\\000\\000E\\003~\\177\\177\\177\\000\\000\\000\\000v........>...equest_body {\\n }\\n}\\nonse_trailer_...NmQ............@/.\\\\..00\\356\\222\\274.p\\\\\\\"\\\\n}\\\\nre000\\\\\\\\@000te.p\\\\\\\"\\\\n}Z\\361\\265\\276\\245Q\\\\000{\\\\rb&..ec0&...2.-6..*..econC\302\207\001fig.core.v3.Back\\\\177\\\\177.n..7e.p2\\n}\\nrequest {nre/.\\\\\\\\000{\\\\000tat_prefix: \\\\\\\"t. \\\\\\\\\\\\\\\"t@003.e\\\\343\\\\220X220Q.\\\\\\\\n ore.body {\\n }\\n}\\nonse_.voy.type.m.3v.nree.\\\\00re..z\\\\\\\\\\\\\\\"t.e.....\\\\Ln }\\\\n .ype.m.v3.Va\\\\\\\\^S35;\\\\\\\\\\\\\\\\3 value: \\\\\\\"envmy.ytpe.matcher.veqn}entchervo3.Va].[cPpL0\\\\PS\\\\PL0].[c=z=z\\\\SVPL0].[.[c=z=z\\\\SVP\\\\3 value: \\\"envoy.type.matcher.veqn}entchervo6.Vaesponse {\n response_body {\n }w\\001 {\n response_bodyvoy.type.m.v3.Va\\\\35;\\\\3 ...................%.matche {\nr.veqn}ge_timeout {entchervo3.Value\n nMpatcanos: 102341 0176\n }\n}}\n}\n\n" + disable_immediate_response: true +} +request { +} +response { + request_body { + } +} diff --git a/test/extensions/filters/http/ext_proc/unit_test_fuzz/ext_proc_unit_test_fuzz.cc b/test/extensions/filters/http/ext_proc/unit_test_fuzz/ext_proc_unit_test_fuzz.cc index d8aac45b885b..984b828e0e02 100644 --- a/test/extensions/filters/http/ext_proc/unit_test_fuzz/ext_proc_unit_test_fuzz.cc +++ b/test/extensions/filters/http/ext_proc/unit_test_fuzz/ext_proc_unit_test_fuzz.cc @@ -72,9 +72,7 @@ DEFINE_PROTO_FUZZER( try { config = std::make_shared( proto_config, std::chrono::milliseconds(200), 200, *stats_store.rootScope(), - "ext_proc_prefix", - std::make_shared( - Envoy::Extensions::Filters::Common::Expr::createBuilder(nullptr))); + "ext_proc_prefix"); } catch (const EnvoyException& e) { ENVOY_LOG_MISC(debug, "EnvoyException during ext_proc filter config validation: {}", e.what()); return;