diff --git a/api/envoy/extensions/filters/http/header_mutation/v3/header_mutation.proto b/api/envoy/extensions/filters/http/header_mutation/v3/header_mutation.proto index 6215eab8356f4..4e2e514063590 100644 --- a/api/envoy/extensions/filters/http/header_mutation/v3/header_mutation.proto +++ b/api/envoy/extensions/filters/http/header_mutation/v3/header_mutation.proto @@ -4,6 +4,7 @@ package envoy.extensions.filters.http.header_mutation.v3; import "envoy/config/common/mutation_rules/v3/mutation_rules.proto"; import "envoy/config/core/v3/base.proto"; +import "envoy/config/core/v3/extension.proto"; import "udpa/annotations/status.proto"; @@ -17,7 +18,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // Mutate HTTP headers and trailers in requests and responses. // [#extension: envoy.filters.http.header_mutation] -// [#next-free-field: 6] +// [#next-free-field: 7] message Mutations { // The request mutations are applied before the request is forwarded to the upstream cluster. repeated config.common.mutation_rules.v3.HeaderMutation request_mutations = 1; @@ -34,6 +35,12 @@ message Mutations { // The request trailer mutations are applied before the request is sent to the upstream cluster. repeated config.common.mutation_rules.v3.HeaderMutation request_trailers_mutations = 5; + + // Formatter command parsers used for mutation values. Built-in command parsers are available + // without this field; use it to add extension parsers or override a built-in parser's + // configuration. + // [#extension-category: envoy.formatter] + repeated config.core.v3.TypedExtensionConfig formatters = 6; } // Per route configuration for the header mutation filter. diff --git a/api/envoy/extensions/formatter/cel/v3/BUILD b/api/envoy/extensions/formatter/cel/v3/BUILD index 5f552f08145ca..504c6c70514ac 100644 --- a/api/envoy/extensions/formatter/cel/v3/BUILD +++ b/api/envoy/extensions/formatter/cel/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/formatter/cel/v3/cel.proto b/api/envoy/extensions/formatter/cel/v3/cel.proto index ced34e735f00d..0cf4b7769284f 100644 --- a/api/envoy/extensions/formatter/cel/v3/cel.proto +++ b/api/envoy/extensions/formatter/cel/v3/cel.proto @@ -2,6 +2,8 @@ syntax = "proto3"; package envoy.extensions.formatter.cel.v3; +import "envoy/config/core/v3/cel.proto"; + import "udpa/annotations/status.proto"; option java_package = "io.envoyproxy.envoy.extensions.formatter.cel.v3"; @@ -50,7 +52,10 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // Configuration for the CEL formatter. // // .. warning:: -// This extension is treated as built-in extension and will be enabled by default now. -// It is unnecessary to configure this extension. +// This extension is treated as a built-in extension and is enabled by default. +// It is unnecessary to configure this extension unless overriding the CEL expression runtime +// via ``cel_config``. message Cel { + // Configuration for the CEL expression runtime used by this formatter. + config.core.v3.CelExpressionConfig cel_config = 1; } diff --git a/api/envoy/extensions/http/early_header_mutation/header_mutation/v3/BUILD b/api/envoy/extensions/http/early_header_mutation/header_mutation/v3/BUILD index c244c8407fed3..b8dc04c8eb776 100644 --- a/api/envoy/extensions/http/early_header_mutation/header_mutation/v3/BUILD +++ b/api/envoy/extensions/http/early_header_mutation/header_mutation/v3/BUILD @@ -7,6 +7,7 @@ licenses(["notice"]) # Apache 2 api_proto_package( deps = [ "//envoy/config/common/mutation_rules/v3:pkg", + "//envoy/config/core/v3:pkg", "@xds//udpa/annotations:pkg", ], ) diff --git a/api/envoy/extensions/http/early_header_mutation/header_mutation/v3/header_mutation.proto b/api/envoy/extensions/http/early_header_mutation/header_mutation/v3/header_mutation.proto index 8cad541eded97..054ad30e77004 100644 --- a/api/envoy/extensions/http/early_header_mutation/header_mutation/v3/header_mutation.proto +++ b/api/envoy/extensions/http/early_header_mutation/header_mutation/v3/header_mutation.proto @@ -3,6 +3,7 @@ syntax = "proto3"; package envoy.extensions.http.early_header_mutation.header_mutation.v3; import "envoy/config/common/mutation_rules/v3/mutation_rules.proto"; +import "envoy/config/core/v3/extension.proto"; import "udpa/annotations/status.proto"; import "validate/validate.proto"; @@ -20,4 +21,10 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; message HeaderMutation { repeated config.common.mutation_rules.v3.HeaderMutation mutations = 1 [(validate.rules).repeated = {min_items: 1}]; + + // Formatter command parsers used for header values. Built-in command parsers are available + // without this field; use it to add extension parsers or override a built-in parser's + // configuration. + // [#extension-category: envoy.formatter] + repeated config.core.v3.TypedExtensionConfig formatters = 2; } diff --git a/changelogs/current/new_features/early_header_mutation__added-formatters-config.rst b/changelogs/current/new_features/early_header_mutation__added-formatters-config.rst new file mode 100644 index 0000000000000..6c6f3ad20c0ea --- /dev/null +++ b/changelogs/current/new_features/early_header_mutation__added-formatters-config.rst @@ -0,0 +1,3 @@ +Added :ref:`formatters ` +to the early header mutation extension, allowing extension formatter command parsers to be +configured for header mutation substitution values. diff --git a/changelogs/current/new_features/formatter__added-cel-expression-config.rst b/changelogs/current/new_features/formatter__added-cel-expression-config.rst new file mode 100644 index 0000000000000..25542ddf763f9 --- /dev/null +++ b/changelogs/current/new_features/formatter__added-cel-expression-config.rst @@ -0,0 +1,3 @@ +Exposed :ref:`cel_config ` on the +``envoy.formatter.cel`` formatter configuration, allowing CEL runtime options such as string +functions to be enabled for configured CEL formatters. diff --git a/changelogs/current/new_features/header_mutation__added-formatters-config.rst b/changelogs/current/new_features/header_mutation__added-formatters-config.rst new file mode 100644 index 0000000000000..5d2e940634a8f --- /dev/null +++ b/changelogs/current/new_features/header_mutation__added-formatters-config.rst @@ -0,0 +1,3 @@ +Added :ref:`formatters ` +to the header mutation filter, allowing extension formatter command parsers to be configured for +header mutation substitution values. diff --git a/source/common/formatter/substitution_format_string.cc b/source/common/formatter/substitution_format_string.cc index 60cabc98f997d..fe8b63146537b 100644 --- a/source/common/formatter/substitution_format_string.cc +++ b/source/common/formatter/substitution_format_string.cc @@ -25,6 +25,18 @@ absl::StatusOr> SubstitutionFormatStringUtils::par return commands; } +absl::StatusOr> SubstitutionFormatStringUtils::parseFormatters( + const FormattersConfig& formatters, Server::Configuration::ServerFactoryContext& server_context, + ProtobufMessage::ValidationVisitor& validation_visitor, + std::vector&& commands_parsers) { + if (formatters.empty()) { + return std::move(commands_parsers); + } + + Server::GenericFactoryContextImpl generic_context(server_context, validation_visitor); + return parseFormatters(formatters, generic_context, std::move(commands_parsers)); +} + absl::StatusOr SubstitutionFormatStringUtils::fromProtoConfig( const envoy::config::core::v3::SubstitutionFormatString& config, Server::Configuration::GenericFactoryContext& context, diff --git a/source/common/formatter/substitution_format_string.h b/source/common/formatter/substitution_format_string.h index 686f54542ad39..ceccfa7b951d6 100644 --- a/source/common/formatter/substitution_format_string.h +++ b/source/common/formatter/substitution_format_string.h @@ -33,6 +33,11 @@ class SubstitutionFormatStringUtils { parseFormatters(const FormattersConfig& formatters, Server::Configuration::GenericFactoryContext& context, std::vector&& commands_parsers = {}); + static absl::StatusOr> + parseFormatters(const FormattersConfig& formatters, + Server::Configuration::ServerFactoryContext& server_context, + ProtobufMessage::ValidationVisitor& validation_visitor, + std::vector&& commands_parsers = {}); /** * Generate a formatter object from config SubstitutionFormatString. diff --git a/source/extensions/filters/http/header_mutation/BUILD b/source/extensions/filters/http/header_mutation/BUILD index 0d999deb96ae1..e324e0f49b9b8 100644 --- a/source/extensions/filters/http/header_mutation/BUILD +++ b/source/extensions/filters/http/header_mutation/BUILD @@ -14,12 +14,15 @@ envoy_cc_library( srcs = ["header_mutation.cc"], hdrs = ["header_mutation.h"], deps = [ + "//envoy/protobuf:message_validator_interface", "//envoy/server:filter_config_interface", "//source/common/config:utility_lib", + "//source/common/formatter:substitution_format_string_lib", "//source/common/http:header_map_lib", "//source/common/http:header_mutation_lib", "//source/common/protobuf:utility_lib", "//source/extensions/filters/http/common:pass_through_filter_lib", + "//source/server:generic_factory_context_lib", "@envoy_api//envoy/extensions/filters/http/header_mutation/v3:pkg_cc_proto", ], ) diff --git a/source/extensions/filters/http/header_mutation/config.cc b/source/extensions/filters/http/header_mutation/config.cc index c79b9f1dc627b..bf91819f014fd 100644 --- a/source/extensions/filters/http/header_mutation/config.cc +++ b/source/extensions/filters/http/header_mutation/config.cc @@ -40,10 +40,10 @@ HeaderMutationFactoryConfig::createFilterFactoryFromProtoWithServerContextTyped( absl::StatusOr HeaderMutationFactoryConfig::createRouteSpecificFilterConfigTyped( const PerRouteProtoConfig& proto_config, Server::Configuration::ServerFactoryContext& context, - ProtobufMessage::ValidationVisitor&) { + ProtobufMessage::ValidationVisitor& validation_visitor) { absl::Status creation_status = absl::OkStatus(); - auto route_config = - std::make_shared(proto_config, context, creation_status); + auto route_config = std::make_shared(proto_config, context, + validation_visitor, creation_status); RETURN_IF_NOT_OK_REF(creation_status); return route_config; } diff --git a/source/extensions/filters/http/header_mutation/header_mutation.cc b/source/extensions/filters/http/header_mutation/header_mutation.cc index bde8ce31b8fec..d85686419c09f 100644 --- a/source/extensions/filters/http/header_mutation/header_mutation.cc +++ b/source/extensions/filters/http/header_mutation/header_mutation.cc @@ -3,7 +3,7 @@ #include #include -#include "source/common/config/utility.h" +#include "source/common/formatter/substitution_format_string.h" #include "source/common/http/header_map_impl.h" #include "source/common/http/utility.h" #include "source/common/runtime/runtime_features.h" @@ -54,22 +54,31 @@ void QueryParameterMutationAppend::mutateQueryParameter( Mutations::Mutations(const MutationsProto& config, Server::Configuration::ServerFactoryContext& context, + ProtobufMessage::ValidationVisitor& validation_visitor, absl::Status& creation_status) { - auto request_mutations_or_error = HeaderMutations::create(config.request_mutations(), context); + auto command_parsers_or_error = Formatter::SubstitutionFormatStringUtils::parseFormatters( + config.formatters(), context, validation_visitor); + SET_AND_RETURN_IF_NOT_OK(command_parsers_or_error.status(), creation_status); + Formatter::CommandParserPtrVector command_parsers = + std::move(command_parsers_or_error.value()); + + auto request_mutations_or_error = + HeaderMutations::create(config.request_mutations(), context, command_parsers); SET_AND_RETURN_IF_NOT_OK(request_mutations_or_error.status(), creation_status); request_mutations_ = std::move(request_mutations_or_error.value()); - auto response_mutations_or_error = HeaderMutations::create(config.response_mutations(), context); + auto response_mutations_or_error = + HeaderMutations::create(config.response_mutations(), context, command_parsers); SET_AND_RETURN_IF_NOT_OK(response_mutations_or_error.status(), creation_status); response_mutations_ = std::move(response_mutations_or_error.value()); auto response_trailers_mutations_or_error = - HeaderMutations::create(config.response_trailers_mutations(), context); + HeaderMutations::create(config.response_trailers_mutations(), context, command_parsers); SET_AND_RETURN_IF_NOT_OK(response_trailers_mutations_or_error.status(), creation_status); response_trailers_mutations_ = std::move(response_trailers_mutations_or_error.value()); auto request_trailers_mutations_or_error = - HeaderMutations::create(config.request_trailers_mutations(), context); + HeaderMutations::create(config.request_trailers_mutations(), context, command_parsers); SET_AND_RETURN_IF_NOT_OK(request_trailers_mutations_or_error.status(), creation_status); request_trailers_mutations_ = std::move(request_trailers_mutations_or_error.value()); @@ -92,8 +101,8 @@ Mutations::Mutations(const MutationsProto& config, return; } - auto value_or_error = - Formatter::FormatterImpl::create(mutation.append().record().value().string_value(), true); + auto value_or_error = Formatter::FormatterImpl::create( + mutation.append().record().value().string_value(), true, command_parsers); SET_AND_RETURN_IF_NOT_OK(value_or_error.status(), creation_status); query_query_parameter_mutations_.emplace_back(std::make_unique( mutation.append().record().key(), std::move(value_or_error.value()), @@ -145,15 +154,15 @@ void Mutations::mutateRequestTrailers(Http::RequestTrailerMap& trailers, request_trailers_mutations_->evaluateHeaders(trailers, context, stream_info); } -PerRouteHeaderMutation::PerRouteHeaderMutation(const PerRouteProtoConfig& config, - Server::Configuration::ServerFactoryContext& context, - absl::Status& creation_status) - : mutations_(config.mutations(), context, creation_status) {} +PerRouteHeaderMutation::PerRouteHeaderMutation( + const PerRouteProtoConfig& config, Server::Configuration::ServerFactoryContext& context, + ProtobufMessage::ValidationVisitor& validation_visitor, absl::Status& creation_status) + : mutations_(config.mutations(), context, validation_visitor, creation_status) {} HeaderMutationConfig::HeaderMutationConfig(const ProtoConfig& config, Server::Configuration::ServerFactoryContext& context, absl::Status& creation_status) - : mutations_(config.mutations(), context, creation_status), + : mutations_(config.mutations(), context, context.messageValidationVisitor(), creation_status), most_specific_header_mutations_wins_(config.most_specific_header_mutations_wins()) {} void HeaderMutation::maybeInitializeRouteConfigs(Http::StreamFilterCallbacks* callbacks) { diff --git a/source/extensions/filters/http/header_mutation/header_mutation.h b/source/extensions/filters/http/header_mutation/header_mutation.h index d7d6535d2a91c..c3eae3309b9c7 100644 --- a/source/extensions/filters/http/header_mutation/header_mutation.h +++ b/source/extensions/filters/http/header_mutation/header_mutation.h @@ -7,6 +7,7 @@ #include "envoy/extensions/filters/http/header_mutation/v3/header_mutation.pb.h" #include "envoy/http/query_params.h" +#include "envoy/protobuf/message_validator.h" #include "source/common/common/logger.h" #include "source/common/formatter/substitution_formatter.h" @@ -83,7 +84,7 @@ class Mutations { using HeaderMutations = Http::HeaderMutations; Mutations(const MutationsProto& config, Server::Configuration::ServerFactoryContext& context, - absl::Status& creation_status); + ProtobufMessage::ValidationVisitor& validation_visitor, absl::Status& creation_status); void mutateRequestHeaders(Http::RequestHeaderMap& headers, const Formatter::Context& context, const StreamInfo::StreamInfo& stream_info) const; @@ -107,6 +108,7 @@ class PerRouteHeaderMutation : public Router::RouteSpecificFilterConfig { public: PerRouteHeaderMutation(const PerRouteProtoConfig& config, Server::Configuration::ServerFactoryContext& context, + ProtobufMessage::ValidationVisitor& validation_visitor, absl::Status& creation_status); const Mutations& mutations() const { return mutations_; } diff --git a/source/extensions/formatter/cel/BUILD b/source/extensions/formatter/cel/BUILD index e45ac071674b7..0486c7e5a87b5 100644 --- a/source/extensions/formatter/cel/BUILD +++ b/source/extensions/formatter/cel/BUILD @@ -50,6 +50,8 @@ envoy_cc_extension( ], deps = [ "//envoy/registry", + "//source/common/protobuf:utility_lib", + "//source/extensions/filters/common/expr:evaluator_lib", "//source/extensions/formatter/cel:cel_lib", "@envoy_api//envoy/extensions/formatter/cel/v3:pkg_cc_proto", ] + select( diff --git a/source/extensions/formatter/cel/cel.cc b/source/extensions/formatter/cel/cel.cc index db67d626dc1fb..d10db4f36541e 100644 --- a/source/extensions/formatter/cel/cel.cc +++ b/source/extensions/formatter/cel/cel.cc @@ -76,6 +76,13 @@ Protobuf::Value CELFormatter::formatValue(const Envoy::Formatter::Context& conte } } +CELFormatterCommandParser::CELFormatterCommandParser( + const ::Envoy::LocalInfo::LocalInfo& local_info, + Expr::BuilderInstanceSharedConstPtr expr_builder) + : configured_state_(ConfiguredState{local_info, std::move(expr_builder)}) { + ASSERT(configured_state_->expr_builder != nullptr); +} + ::Envoy::Formatter::FormatterProviderPtr CELFormatterCommandParser::parse(absl::string_view command, absl::string_view subcommand, absl::optional max_length) const { @@ -85,10 +92,18 @@ CELFormatterCommandParser::parse(absl::string_view command, absl::string_view su if (!parse_status.ok()) { throw EnvoyException("Not able to parse expression: " + parse_status.status().ToString()); } - Server::Configuration::ServerFactoryContext& context = - Server::Configuration::ServerFactoryContextInstance::get(); + + // Lazily resolve the active server context at CEL-command parse time: built-in command parsers + // are process-global, so they cannot capture server-owned CEL state. + if (!configured_state_.has_value()) { + Server::Configuration::ServerFactoryContext& context = + Server::Configuration::ServerFactoryContextInstance::get(); + return std::make_unique(context.localInfo(), Expr::getBuilder(context), + parse_status.value().expr(), max_length, + command == "TYPED_CEL"); + } return std::make_unique( - context.localInfo(), Extensions::Filters::Common::Expr::getBuilder(context), + configured_state_->local_info.get(), configured_state_->expr_builder, parse_status.value().expr(), max_length, command == "TYPED_CEL"); } diff --git a/source/extensions/formatter/cel/cel.h b/source/extensions/formatter/cel/cel.h index 08b85c78920b0..6cf5af7e75f27 100644 --- a/source/extensions/formatter/cel/cel.h +++ b/source/extensions/formatter/cel/cel.h @@ -1,5 +1,6 @@ #pragma once +#include #include #include "envoy/config/typed_config.h" @@ -8,6 +9,8 @@ #include "source/common/formatter/substitution_formatter.h" #include "source/extensions/filters/common/expr/evaluator.h" +#include "absl/types/optional.h" + namespace Envoy { namespace Extensions { namespace Formatter { @@ -33,9 +36,23 @@ class CELFormatter : public ::Envoy::Formatter::FormatterProvider { class CELFormatterCommandParser : public ::Envoy::Formatter::CommandParser { public: CELFormatterCommandParser() = default; + CELFormatterCommandParser( + const ::Envoy::LocalInfo::LocalInfo& local_info, + Extensions::Filters::Common::Expr::BuilderInstanceSharedConstPtr expr_builder); ::Envoy::Formatter::FormatterProviderPtr parse(absl::string_view command, absl::string_view subcommand, absl::optional max_length) const override; + +private: + struct ConfiguredState { + std::reference_wrapper local_info; + Extensions::Filters::Common::Expr::BuilderInstanceSharedConstPtr expr_builder; + }; + + // Present only for parsers created from explicit `envoy.formatter.cel` config. Keeping the + // server-owned values together avoids a half-configured parser; absence means built-in parser + // mode, where `parse()` resolves the active server context for each CEL command. + absl::optional configured_state_; }; } // namespace Formatter diff --git a/source/extensions/formatter/cel/config.cc b/source/extensions/formatter/cel/config.cc index 3c0e5ab9c7878..587eae164d72e 100644 --- a/source/extensions/formatter/cel/config.cc +++ b/source/extensions/formatter/cel/config.cc @@ -1,22 +1,29 @@ #include "source/extensions/formatter/cel/config.h" #include "envoy/extensions/formatter/cel/v3/cel.pb.h" +#include "envoy/extensions/formatter/cel/v3/cel.pb.validate.h" +#include "source/common/protobuf/utility.h" #include "source/extensions/formatter/cel/cel.h" namespace Envoy { namespace Extensions { namespace Formatter { -::Envoy::Formatter::CommandParserPtr -CELFormatterFactory::createCommandParserFromProto(const Protobuf::Message&, - Server::Configuration::GenericFactoryContext&) { +::Envoy::Formatter::CommandParserPtr CELFormatterFactory::createCommandParserFromProto( + const Protobuf::Message& proto_config, Server::Configuration::GenericFactoryContext& context) { #if defined(USE_CEL_PARSER) - ENVOY_LOG_TO_LOGGER(Logger::Registry::getLog(Logger::Id::config), warn, - "'CEL' formatter is treated as a built-in formatter and does not " - "require configuration."); - return std::make_unique(); + const auto& config = + MessageUtil::downcastAndValidate( + proto_config, context.messageValidationVisitor()); + const auto config_ref = config.has_cel_config() + ? Envoy::makeOptRef(config.cel_config()) + : Envoy::OptRef{}; + return std::make_unique( + context.serverFactoryContext().localInfo(), + Filters::Common::Expr::getBuilder(context.serverFactoryContext(), config_ref)); #else + UNREFERENCED_PARAMETER(proto_config); UNREFERENCED_PARAMETER(context); throw EnvoyException("CEL is not available for use in this environment."); #endif diff --git a/source/extensions/http/early_header_mutation/header_mutation/BUILD b/source/extensions/http/early_header_mutation/header_mutation/BUILD index 3cb66447d1871..f44117a95237a 100644 --- a/source/extensions/http/early_header_mutation/header_mutation/BUILD +++ b/source/extensions/http/early_header_mutation/header_mutation/BUILD @@ -15,7 +15,10 @@ envoy_cc_library( hdrs = ["header_mutation.h"], deps = [ "//envoy/http:early_header_mutation_interface", + "//envoy/protobuf:message_validator_interface", + "//source/common/formatter:substitution_format_string_lib", "//source/common/http:header_mutation_lib", + "//source/server:generic_factory_context_lib", "@envoy_api//envoy/config/common/mutation_rules/v3:pkg_cc_proto", "@envoy_api//envoy/extensions/http/early_header_mutation/header_mutation/v3:pkg_cc_proto", ], diff --git a/source/extensions/http/early_header_mutation/header_mutation/config.cc b/source/extensions/http/early_header_mutation/header_mutation/config.cc index 8079804092ba1..0e4e4b95e2226 100644 --- a/source/extensions/http/early_header_mutation/header_mutation/config.cc +++ b/source/extensions/http/early_header_mutation/header_mutation/config.cc @@ -17,7 +17,8 @@ Factory::createExtension(const Protobuf::Message& message, const auto& proto_config = MessageUtil::downcastAndValidate< const envoy::extensions::http::early_header_mutation::header_mutation::v3::HeaderMutation&>( *mptr, context.messageValidationVisitor()); - return std::make_unique(proto_config, context.serverFactoryContext()); + return std::make_unique(proto_config, context.serverFactoryContext(), + context.messageValidationVisitor()); } REGISTER_FACTORY(Factory, Envoy::Http::EarlyHeaderMutationFactory); diff --git a/source/extensions/http/early_header_mutation/header_mutation/header_mutation.cc b/source/extensions/http/early_header_mutation/header_mutation/header_mutation.cc index c1d0d5c368ec4..af7418f8fd7a8 100644 --- a/source/extensions/http/early_header_mutation/header_mutation/header_mutation.cc +++ b/source/extensions/http/early_header_mutation/header_mutation/header_mutation.cc @@ -1,7 +1,10 @@ #include "source/extensions/http/early_header_mutation/header_mutation/header_mutation.h" +#include + #include "envoy/config/common/mutation_rules/v3/mutation_rules.pb.h" +#include "source/common/formatter/substitution_format_string.h" #include "source/common/http/header_map_impl.h" namespace Envoy { @@ -10,11 +13,30 @@ namespace Http { namespace EarlyHeaderMutation { namespace HeaderMutation { +namespace { + +absl::StatusOr> +createHeaderMutations(const ProtoHeaderMutation& mutations, + Server::Configuration::ServerFactoryContext& context, + ProtobufMessage::ValidationVisitor& validation_visitor) { + auto command_parsers_or_error = Formatter::SubstitutionFormatStringUtils::parseFormatters( + mutations.formatters(), context, validation_visitor); + if (!command_parsers_or_error.ok()) { + return command_parsers_or_error.status(); + } + Formatter::CommandParserPtrVector command_parsers = + std::move(command_parsers_or_error.value()); + return Envoy::Http::HeaderMutations::create(mutations.mutations(), context, command_parsers); +} + +} // namespace + HeaderMutation::HeaderMutation(const ProtoHeaderMutation& mutations, - Server::Configuration::ServerFactoryContext& context) - : mutations_(THROW_OR_RETURN_VALUE( - Envoy::Http::HeaderMutations::create(mutations.mutations(), context), - std::unique_ptr)) {} + Server::Configuration::ServerFactoryContext& context, + ProtobufMessage::ValidationVisitor& validation_visitor) + : mutations_( + THROW_OR_RETURN_VALUE(createHeaderMutations(mutations, context, validation_visitor), + std::unique_ptr)) {} bool HeaderMutation::mutate(Envoy::Http::RequestHeaderMap& headers, const StreamInfo::StreamInfo& stream_info) const { diff --git a/source/extensions/http/early_header_mutation/header_mutation/header_mutation.h b/source/extensions/http/early_header_mutation/header_mutation/header_mutation.h index d2f02f6cf75d2..57ada5d29b035 100644 --- a/source/extensions/http/early_header_mutation/header_mutation/header_mutation.h +++ b/source/extensions/http/early_header_mutation/header_mutation/header_mutation.h @@ -3,6 +3,7 @@ #include "envoy/extensions/http/early_header_mutation/header_mutation/v3/header_mutation.pb.h" #include "envoy/extensions/http/early_header_mutation/header_mutation/v3/header_mutation.pb.validate.h" #include "envoy/http/early_header_mutation.h" +#include "envoy/protobuf/message_validator.h" #include "source/common/http/header_mutation.h" @@ -20,7 +21,8 @@ using ProtoHeaderMutation = class HeaderMutation : public Envoy::Http::EarlyHeaderMutation { public: HeaderMutation(const ProtoHeaderMutation& mutations, - Server::Configuration::ServerFactoryContext& context); + Server::Configuration::ServerFactoryContext& context, + ProtobufMessage::ValidationVisitor& validation_visitor); bool mutate(Envoy::Http::RequestHeaderMap& headers, const StreamInfo::StreamInfo& stream_info) const override; diff --git a/test/extensions/filters/http/header_mutation/BUILD b/test/extensions/filters/http/header_mutation/BUILD index ecee60a74e08c..e069d7789512d 100644 --- a/test/extensions/filters/http/header_mutation/BUILD +++ b/test/extensions/filters/http/header_mutation/BUILD @@ -16,7 +16,16 @@ envoy_extension_cc_test( srcs = [ "header_mutation_test.cc", ], - extension_names = ["envoy.filters.http.header_mutation"], + copts = select({ + "//bazel:windows_x86_64": [], + "//conditions:default": [ + "-DUSE_CEL_PARSER", + ], + }), + extension_names = [ + "envoy.filters.http.header_mutation", + "envoy.formatter.cel", + ], rbe_pool = "6gig", deps = [ "//source/common/formatter:formatter_extension_lib", @@ -25,7 +34,12 @@ envoy_extension_cc_test( "//test/mocks/server:factory_context_mocks", "//test/test_common:registry_lib", "//test/test_common:utility_lib", - ], + ] + select({ + "//bazel:windows_x86_64": [], + "//conditions:default": [ + "//source/extensions/formatter/cel:config", + ], + }), ) envoy_extension_cc_test( diff --git a/test/extensions/filters/http/header_mutation/header_mutation_test.cc b/test/extensions/filters/http/header_mutation/header_mutation_test.cc index baa7a99975552..5e6c719c04443 100644 --- a/test/extensions/filters/http/header_mutation/header_mutation_test.cc +++ b/test/extensions/filters/http/header_mutation/header_mutation_test.cc @@ -66,14 +66,14 @@ TEST(HeaderMutationFilterTest, RequestMutationTest) { append_action: "ADD_IF_ABSENT" )EOF"; - Server::Configuration::MockServerFactoryContext context; + NiceMock context; PerRouteProtoConfig per_route_proto_config; TestUtility::loadFromYaml(route_config_yaml, per_route_proto_config); absl::Status creation_status = absl::OkStatus(); - PerRouteHeaderMutationSharedPtr config = - std::make_shared(per_route_proto_config, context, creation_status); + PerRouteHeaderMutationSharedPtr config = std::make_shared( + per_route_proto_config, context, context.messageValidationVisitor(), creation_status); ProtoConfig proto_config; TestUtility::loadFromYaml(config_yaml, proto_config); @@ -132,6 +132,168 @@ TEST(HeaderMutationFilterTest, RequestMutationTest) { } } +#if defined(USE_CEL_PARSER) + +// Each mutation site formats a string-function expression that only a configured CEL parser +// (`cel_config`) can build, so a site that didn't get the parser fails config load instead of +// passing silently. Confirms the parser reaches every site. +TEST(HeaderMutationFilterTest, CelConfigAppliedToAllMutationSites) { + const std::string config_yaml = R"EOF( + mutations: + request_mutations: + - append: + header: + key: "flag-request-header" + value: "%CEL('prefix-old-suffix'.replace('old', 'new'))%" + append_action: "OVERWRITE_IF_EXISTS_OR_ADD" + response_mutations: + - append: + header: + key: "flag-response-header" + value: "%CEL('prefix-old-suffix'.replace('old', 'new'))%" + append_action: "OVERWRITE_IF_EXISTS_OR_ADD" + request_trailers_mutations: + - append: + header: + key: "flag-request-trailer" + value: "%CEL('prefix-old-suffix'.replace('old', 'new'))%" + append_action: "OVERWRITE_IF_EXISTS_OR_ADD" + response_trailers_mutations: + - append: + header: + key: "flag-response-trailer" + value: "%CEL('prefix-old-suffix'.replace('old', 'new'))%" + append_action: "OVERWRITE_IF_EXISTS_OR_ADD" + query_parameter_mutations: + - append: + record: + key: "flag-query" + value: "%CEL('prefix-old-suffix'.replace('old', 'new'))%" + action: "OVERWRITE_IF_EXISTS_OR_ADD" + formatters: + - name: envoy.formatter.cel + typed_config: + "@type": type.googleapis.com/envoy.extensions.formatter.cel.v3.Cel + cel_config: + enable_string_functions: true + )EOF"; + + NiceMock context; + + ProtoConfig proto_config; + TestUtility::loadFromYaml(config_yaml, proto_config); + + absl::Status creation_status = absl::OkStatus(); + HeaderMutationConfigSharedPtr global_config = + std::make_shared(proto_config, context, creation_status); + ASSERT_TRUE(creation_status.ok()) << creation_status; + + NiceMock decoder_callbacks; + NiceMock encoder_callbacks; + + HeaderMutation filter{global_config}; + filter.setDecoderFilterCallbacks(decoder_callbacks); + filter.setEncoderFilterCallbacks(encoder_callbacks); + + EXPECT_CALL(*decoder_callbacks.route_, perFilterConfigs(_)) + .WillOnce( + Invoke([&](absl::string_view) -> Router::RouteSpecificFilterConfigs { return {}; })); + + Envoy::Http::TestRequestHeaderMapImpl request_headers = { + {":method", "GET"}, + {":path", "/path?flag-query=old"}, + {":scheme", "http"}, + {":authority", "host"}}; + Envoy::Http::TestRequestTrailerMapImpl request_trailers{{"placeholder", "value"}}; + Envoy::Http::TestResponseHeaderMapImpl response_headers = {{":status", "200"}}; + Envoy::Http::TestResponseTrailerMapImpl response_trailers{{"placeholder", "value"}}; + + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter.decodeHeaders(request_headers, true)); + EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter.decodeTrailers(request_trailers)); + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter.encodeHeaders(response_headers, true)); + EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter.encodeTrailers(response_trailers)); + + EXPECT_EQ("prefix-new-suffix", request_headers.get_("flag-request-header")); + EXPECT_EQ("prefix-new-suffix", response_headers.get_("flag-response-header")); + EXPECT_EQ("prefix-new-suffix", request_trailers.get_("flag-request-trailer")); + EXPECT_EQ("prefix-new-suffix", response_trailers.get_("flag-response-trailer")); + auto params = + Http::Utility::QueryParamsMulti::parseAndDecodeQueryString(request_headers.getPathValue()); + EXPECT_EQ("prefix-new-suffix", params.data().at("flag-query").front()); +} + +TEST(HeaderMutationFilterTest, StringFunctionExpressionRejectedWhenDisabled) { + const std::string config_yaml = R"EOF( + mutations: + request_mutations: + - append: + header: + key: "flag-header" + value: "%CEL(request.headers['source-header'].replace('old', 'new'))%" + append_action: "OVERWRITE_IF_EXISTS_OR_ADD" + formatters: + - name: envoy.formatter.cel + typed_config: + "@type": type.googleapis.com/envoy.extensions.formatter.cel.v3.Cel + cel_config: + enable_string_functions: false + )EOF"; + + NiceMock context; + + ProtoConfig proto_config; + TestUtility::loadFromYaml(config_yaml, proto_config); + + absl::Status creation_status = absl::OkStatus(); + EXPECT_THROW(HeaderMutationConfig config(proto_config, context, creation_status), EnvoyException); +} + +TEST(HeaderMutationFilterTest, BuiltInCelWorksWithoutFormatters) { + const std::string config_yaml = R"EOF( + mutations: + request_mutations: + - append: + header: + key: "flag-header" + value: "%CEL(request.headers[':method'])%" + append_action: "OVERWRITE_IF_EXISTS_OR_ADD" + )EOF"; + + NiceMock context; + ScopedThreadLocalServerContextSetter server_context_singleton_setter(context); + + ProtoConfig proto_config; + TestUtility::loadFromYaml(config_yaml, proto_config); + + absl::Status creation_status = absl::OkStatus(); + HeaderMutationConfigSharedPtr global_config = + std::make_shared(proto_config, context, creation_status); + ASSERT_TRUE(creation_status.ok()) << creation_status; + + NiceMock decoder_callbacks; + NiceMock encoder_callbacks; + + HeaderMutation filter{global_config}; + filter.setDecoderFilterCallbacks(decoder_callbacks); + filter.setEncoderFilterCallbacks(encoder_callbacks); + + EXPECT_CALL(*decoder_callbacks.route_, perFilterConfigs(_)) + .WillOnce( + Invoke([&](absl::string_view) -> Router::RouteSpecificFilterConfigs { return {}; })); + + Envoy::Http::TestRequestHeaderMapImpl headers = { + {":method", "GET"}, + {":path", "/path"}, + {":scheme", "http"}, + {":authority", "host"}}; + + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter.decodeHeaders(headers, true)); + + EXPECT_EQ("GET", headers.get_("flag-header")); +} + +#endif // USE_CEL_PARSER + TEST(HeaderMutationFilterTest, ResponseMutationTest) { const std::string route_config_yaml = R"EOF( mutations: @@ -180,14 +342,14 @@ TEST(HeaderMutationFilterTest, ResponseMutationTest) { - remove: "global-flag-header" )EOF"; - Server::Configuration::MockServerFactoryContext context; + NiceMock context; PerRouteProtoConfig per_route_proto_config; TestUtility::loadFromYaml(route_config_yaml, per_route_proto_config); absl::Status creation_status = absl::OkStatus(); - PerRouteHeaderMutationSharedPtr config = - std::make_shared(per_route_proto_config, context, creation_status); + PerRouteHeaderMutationSharedPtr config = std::make_shared( + per_route_proto_config, context, context.messageValidationVisitor(), creation_status); ProtoConfig proto_config; TestUtility::loadFromYaml(config_yaml, proto_config); @@ -358,14 +520,14 @@ TEST(HeaderMutationFilterTest, ResponseTrailerMutationTest) { - remove: "global-flag-header" )EOF"; - Server::Configuration::MockServerFactoryContext context; + NiceMock context; PerRouteProtoConfig per_route_proto_config; TestUtility::loadFromYaml(route_config_yaml, per_route_proto_config); absl::Status creation_status = absl::OkStatus(); - PerRouteHeaderMutationSharedPtr config = - std::make_shared(per_route_proto_config, context, creation_status); + PerRouteHeaderMutationSharedPtr config = std::make_shared( + per_route_proto_config, context, context.messageValidationVisitor(), creation_status); ProtoConfig proto_config; TestUtility::loadFromYaml(config_yaml, proto_config); @@ -538,14 +700,14 @@ TEST(HeaderMutationFilterTest, HybridMutationTest) { - remove: "global-flag-header" )EOF"; - Server::Configuration::MockServerFactoryContext context; + NiceMock context; PerRouteProtoConfig per_route_proto_config; TestUtility::loadFromYaml(route_config_yaml, per_route_proto_config); absl::Status creation_status = absl::OkStatus(); - PerRouteHeaderMutationSharedPtr config = - std::make_shared(per_route_proto_config, context, creation_status); + PerRouteHeaderMutationSharedPtr config = std::make_shared( + per_route_proto_config, context, context.messageValidationVisitor(), creation_status); ProtoConfig proto_config; TestUtility::loadFromYaml(config_yaml, proto_config); @@ -670,14 +832,14 @@ TEST(HeaderMutationFilterTest, QueryParameterMutationTest) { - remove: "global-flag-header" )EOF"; - Server::Configuration::MockServerFactoryContext context; + NiceMock context; PerRouteProtoConfig per_route_proto_config; TestUtility::loadFromYaml(route_config_yaml, per_route_proto_config); absl::Status creation_status = absl::OkStatus(); - PerRouteHeaderMutationSharedPtr config = - std::make_shared(per_route_proto_config, context, creation_status); + PerRouteHeaderMutationSharedPtr config = std::make_shared( + per_route_proto_config, context, context.messageValidationVisitor(), creation_status); ProtoConfig proto_config; TestUtility::loadFromYaml(config_yaml, proto_config); @@ -784,14 +946,14 @@ TEST(HeaderMutationFilterTest, RequestTrailerMutationTest) { - remove: "global-flag-header" )EOF"; - Server::Configuration::MockServerFactoryContext context; + NiceMock context; PerRouteProtoConfig per_route_proto_config; TestUtility::loadFromYaml(route_config_yaml, per_route_proto_config); absl::Status creation_status = absl::OkStatus(); - PerRouteHeaderMutationSharedPtr config = - std::make_shared(per_route_proto_config, context, creation_status); + PerRouteHeaderMutationSharedPtr config = std::make_shared( + per_route_proto_config, context, context.messageValidationVisitor(), creation_status); ProtoConfig proto_config; TestUtility::loadFromYaml(config_yaml, proto_config); @@ -918,7 +1080,7 @@ TEST(HeaderMutationFilterTest, QueryParameterMutationUrlEncodingTest) { action: "APPEND_IF_EXISTS_OR_ADD" )EOF"; - Server::Configuration::MockServerFactoryContext context; + NiceMock context; ProtoConfig proto_config; TestUtility::loadFromYaml(config_yaml, proto_config); diff --git a/test/extensions/formatter/cel/cel_test.cc b/test/extensions/formatter/cel/cel_test.cc index 080ee3aee621f..e066f424ef1d3 100644 --- a/test/extensions/formatter/cel/cel_test.cc +++ b/test/extensions/formatter/cel/cel_test.cc @@ -313,7 +313,7 @@ TEST_F(CELFormatterTest, TestFormatConversionV1AlphaToDevCel) { ProtoEq(ValueUtil::stringValue("true"))); } -TEST_F(CELFormatterTest, TestRequestHeaderWithLegacyConfiguration) { +TEST_F(CELFormatterTest, TestConfiguredFormatterWithoutCelConfig) { const std::string yaml = R"EOF( text_format_source: inline_string: "%CEL(request.headers[':method'])%" @@ -329,6 +329,42 @@ TEST_F(CELFormatterTest, TestRequestHeaderWithLegacyConfiguration) { EXPECT_EQ("GET", formatter->format(formatter_context_, stream_info_)); } +TEST_F(CELFormatterTest, TestCelConfigEnablesStringFunctions) { + const std::string yaml = R"EOF( + text_format_source: + inline_string: "%CEL(request.headers['x-envoy-original-path'].replace('/original', '/mutated'))%" + formatters: + - name: envoy.formatter.cel + typed_config: + "@type": type.googleapis.com/envoy.extensions.formatter.cel.v3.Cel + cel_config: + enable_string_functions: true +)EOF"; + TestUtility::loadFromYaml(yaml, config_); + + auto formatter = + *Envoy::Formatter::SubstitutionFormatStringUtils::fromProtoConfig(config_, context_); + EXPECT_EQ("/mutated/path?secret=parameter", formatter->format(formatter_context_, stream_info_)); +} + +// Without `cel_config`, string functions default off, so `replace()` is +// unregistered and building the expression fails. +TEST_F(CELFormatterTest, TestStringFunctionExpressionRejectedWithoutCelConfig) { + const std::string yaml = R"EOF( + text_format_source: + inline_string: "%CEL(request.headers['x-envoy-original-path'].replace('/original', '/mutated'))%" + formatters: + - name: envoy.formatter.cel + typed_config: + "@type": type.googleapis.com/envoy.extensions.formatter.cel.v3.Cel +)EOF"; + TestUtility::loadFromYaml(yaml, config_); + + EXPECT_THROW_WITH_REGEX( + *Envoy::Formatter::SubstitutionFormatStringUtils::fromProtoConfig(config_, context_), + EnvoyException, "failed to create an expression: .*"); +} + TEST_F(CELFormatterTest, TestRequestHeader) { const std::string yaml = R"EOF( text_format_source: diff --git a/test/extensions/http/early_header_mutation/header_mutation/BUILD b/test/extensions/http/early_header_mutation/header_mutation/BUILD index ca1e85566c3f8..7ec4913790a33 100644 --- a/test/extensions/http/early_header_mutation/header_mutation/BUILD +++ b/test/extensions/http/early_header_mutation/header_mutation/BUILD @@ -16,7 +16,16 @@ envoy_extension_cc_test( srcs = [ "header_mutation_test.cc", ], - extension_names = ["envoy.http.early_header_mutation.header_mutation"], + copts = select({ + "//bazel:windows_x86_64": [], + "//conditions:default": [ + "-DUSE_CEL_PARSER", + ], + }), + extension_names = [ + "envoy.http.early_header_mutation.header_mutation", + "envoy.formatter.cel", + ], rbe_pool = "6gig", deps = [ "//source/common/formatter:formatter_extension_lib", @@ -24,7 +33,12 @@ envoy_extension_cc_test( "//test/mocks/server:server_factory_context_mocks", "//test/mocks/stream_info:stream_info_mocks", "//test/test_common:utility_lib", - ], + ] + select({ + "//bazel:windows_x86_64": [], + "//conditions:default": [ + "//source/extensions/formatter/cel:config", + ], + }), ) envoy_extension_cc_test( diff --git a/test/extensions/http/early_header_mutation/header_mutation/header_mutation_test.cc b/test/extensions/http/early_header_mutation/header_mutation/header_mutation_test.cc index 756b63c289b7f..e1f8063499a55 100644 --- a/test/extensions/http/early_header_mutation/header_mutation/header_mutation_test.cc +++ b/test/extensions/http/early_header_mutation/header_mutation/header_mutation_test.cc @@ -48,7 +48,7 @@ TEST(HeaderMutationTest, TestAll) { ProtoHeaderMutation proto_mutation; TestUtility::loadFromYaml(config, proto_mutation); - HeaderMutation mutation(proto_mutation, context); + HeaderMutation mutation(proto_mutation, context, ProtobufMessage::getNullValidationVisitor()); NiceMock stream_info; Envoy::Http::TestRequestHeaderMapImpl headers = { @@ -74,6 +74,99 @@ TEST(HeaderMutationTest, TestAll) { EXPECT_EQ("flag-header-4-value", headers.get_("flag-header-4")); } +#if defined(USE_CEL_PARSER) + +TEST(HeaderMutationTest, CelConfigEnablesStringFunctionsForRequestHeaders) { + const std::string config = R"EOF( + mutations: + - append: + header: + key: "flag-header" + value: "%CEL(request.headers['source-header'].replace('old', 'new'))%" + append_action: "OVERWRITE_IF_EXISTS_OR_ADD" + formatters: + - name: envoy.formatter.cel + typed_config: + "@type": type.googleapis.com/envoy.extensions.formatter.cel.v3.Cel + cel_config: + enable_string_functions: true + )EOF"; + + NiceMock context; + ScopedThreadLocalServerContextSetter server_context_singleton_setter(context); + + ProtoHeaderMutation proto_mutation; + TestUtility::loadFromYaml(config, proto_mutation); + + HeaderMutation mutation(proto_mutation, context, ProtobufMessage::getNullValidationVisitor()); + NiceMock stream_info; + + Envoy::Http::TestRequestHeaderMapImpl headers = { + {":method", "GET"}, + {"source-header", "prefix-old-suffix"}, + }; + + mutation.mutate(headers, stream_info); + + EXPECT_EQ("prefix-new-suffix", headers.get_("flag-header")); +} + +TEST(HeaderMutationTest, StringFunctionExpressionRejectedWhenDisabled) { + const std::string config = R"EOF( + mutations: + - append: + header: + key: "flag-header" + value: "%CEL(request.headers['source-header'].replace('old', 'new'))%" + append_action: "OVERWRITE_IF_EXISTS_OR_ADD" + formatters: + - name: envoy.formatter.cel + typed_config: + "@type": type.googleapis.com/envoy.extensions.formatter.cel.v3.Cel + cel_config: + enable_string_functions: false + )EOF"; + + NiceMock context; + + ProtoHeaderMutation proto_mutation; + TestUtility::loadFromYaml(config, proto_mutation); + + EXPECT_THROW( + HeaderMutation mutation(proto_mutation, context, ProtobufMessage::getNullValidationVisitor()), + EnvoyException); +} + +TEST(HeaderMutationTest, BuiltInCelWorksWithoutFormatters) { + const std::string config = R"EOF( + mutations: + - append: + header: + key: "flag-header" + value: "%CEL(request.headers[':method'])%" + append_action: "OVERWRITE_IF_EXISTS_OR_ADD" + )EOF"; + + NiceMock context; + ScopedThreadLocalServerContextSetter server_context_singleton_setter(context); + + ProtoHeaderMutation proto_mutation; + TestUtility::loadFromYaml(config, proto_mutation); + + HeaderMutation mutation(proto_mutation, context, ProtobufMessage::getNullValidationVisitor()); + NiceMock stream_info; + + Envoy::Http::TestRequestHeaderMapImpl headers = { + {":method", "GET"}, + }; + + mutation.mutate(headers, stream_info); + + EXPECT_EQ("GET", headers.get_("flag-header")); +} + +#endif // USE_CEL_PARSER + } // namespace } // namespace HeaderMutation } // namespace EarlyHeaderMutation