diff --git a/changelogs/current.yaml b/changelogs/current.yaml index eee40903099a..7a51dd282778 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -302,5 +302,8 @@ new_features: - area: access_log change: | Added new access log command operator ``%EMIT_TIME%`` to get the time when the log entry is emitted. +- area: access_log + change: | + Added support for node data in ``%CEL%`` formatter. deprecated: diff --git a/source/extensions/access_loggers/filters/cel/cel.cc b/source/extensions/access_loggers/filters/cel/cel.cc index 2dba1b18e80e..20f80b70e2a0 100644 --- a/source/extensions/access_loggers/filters/cel/cel.cc +++ b/source/extensions/access_loggers/filters/cel/cel.cc @@ -9,17 +9,18 @@ namespace CEL { namespace Expr = Envoy::Extensions::Filters::Common::Expr; CELAccessLogExtensionFilter::CELAccessLogExtensionFilter( - Expr::BuilderInstanceSharedPtr builder, const google::api::expr::v1alpha1::Expr& input_expr) - : builder_(builder), parsed_expr_(input_expr) { + const ::Envoy::LocalInfo::LocalInfo& local_info, Expr::BuilderInstanceSharedPtr builder, + const google::api::expr::v1alpha1::Expr& input_expr) + : local_info_(local_info), builder_(builder), parsed_expr_(input_expr) { compiled_expr_ = Expr::createExpression(builder_->builder(), parsed_expr_); } bool CELAccessLogExtensionFilter::evaluate(const Formatter::HttpFormatterContext& log_context, const StreamInfo::StreamInfo& stream_info) const { Protobuf::Arena arena; - auto eval_status = - Expr::evaluate(*compiled_expr_, arena, stream_info, &log_context.requestHeaders(), - &log_context.responseHeaders(), &log_context.responseTrailers()); + auto eval_status = Expr::evaluate(*compiled_expr_, arena, &local_info_, stream_info, + &log_context.requestHeaders(), &log_context.responseHeaders(), + &log_context.responseTrailers()); if (!eval_status.has_value() || eval_status.value().IsError()) { return false; } diff --git a/source/extensions/access_loggers/filters/cel/cel.h b/source/extensions/access_loggers/filters/cel/cel.h index 3fe63981a951..74247d5187d6 100644 --- a/source/extensions/access_loggers/filters/cel/cel.h +++ b/source/extensions/access_loggers/filters/cel/cel.h @@ -19,13 +19,15 @@ namespace CEL { class CELAccessLogExtensionFilter : public AccessLog::Filter { public: - CELAccessLogExtensionFilter(Extensions::Filters::Common::Expr::BuilderInstanceSharedPtr, + CELAccessLogExtensionFilter(const ::Envoy::LocalInfo::LocalInfo& local_info, + Extensions::Filters::Common::Expr::BuilderInstanceSharedPtr, const google::api::expr::v1alpha1::Expr&); bool evaluate(const Formatter::HttpFormatterContext& log_context, const StreamInfo::StreamInfo& stream_info) const override; private: + const ::Envoy::LocalInfo::LocalInfo& local_info_; Extensions::Filters::Common::Expr::BuilderInstanceSharedPtr builder_; const google::api::expr::v1alpha1::Expr parsed_expr_; Extensions::Filters::Common::Expr::ExpressionPtr compiled_expr_; diff --git a/source/extensions/access_loggers/filters/cel/config.cc b/source/extensions/access_loggers/filters/cel/config.cc index c8ce15d00b48..9a1a1be32435 100644 --- a/source/extensions/access_loggers/filters/cel/config.cc +++ b/source/extensions/access_loggers/filters/cel/config.cc @@ -33,6 +33,7 @@ Envoy::AccessLog::FilterPtr CELAccessLogExtensionFilterFactory::createFilter( } return std::make_unique( + context.serverFactoryContext().localInfo(), Extensions::Filters::Common::Expr::getBuilder(context.serverFactoryContext()), parse_status.value().expr()); #else diff --git a/source/extensions/filters/common/expr/context.cc b/source/extensions/filters/common/expr/context.cc index 748e46b5643e..a40fdd735e5d 100644 --- a/source/extensions/filters/common/expr/context.cc +++ b/source/extensions/filters/common/expr/context.cc @@ -385,6 +385,10 @@ absl::optional XDSWrapper::operator[](CelValue key) const { const absl::string_view filter_chain_name = filter_chain_info.has_value() ? filter_chain_info->name() : absl::string_view{}; return CelValue::CreateStringView(filter_chain_name); + } else if (value == Node) { + if (local_info_) { + return CelProtoWrapper::CreateMessage(&local_info_->node(), &arena_); + } } return {}; } diff --git a/source/extensions/filters/common/expr/context.h b/source/extensions/filters/common/expr/context.h index 4fd159046b90..0308671984f8 100644 --- a/source/extensions/filters/common/expr/context.h +++ b/source/extensions/filters/common/expr/context.h @@ -89,6 +89,7 @@ constexpr absl::string_view RouteName = "route_name"; constexpr absl::string_view RouteMetadata = "route_metadata"; constexpr absl::string_view UpstreamHostMetadata = "upstream_host_metadata"; constexpr absl::string_view FilterChainName = "filter_chain_name"; +constexpr absl::string_view Node = "node"; class WrapperFieldValues { public: @@ -232,12 +233,14 @@ class FilterStateWrapper : public BaseWrapper { class XDSWrapper : public BaseWrapper { public: - XDSWrapper(Protobuf::Arena& arena, const StreamInfo::StreamInfo& info) - : BaseWrapper(arena), info_(info) {} + XDSWrapper(Protobuf::Arena& arena, const StreamInfo::StreamInfo& info, + const LocalInfo::LocalInfo* local_info) + : BaseWrapper(arena), info_(info), local_info_(local_info) {} absl::optional operator[](CelValue key) const override; private: const StreamInfo::StreamInfo& info_; + const LocalInfo::LocalInfo* local_info_; }; } // namespace Expr diff --git a/source/extensions/filters/common/expr/evaluator.cc b/source/extensions/filters/common/expr/evaluator.cc index f7f965e9b3b9..a6eec90a949a 100644 --- a/source/extensions/filters/common/expr/evaluator.cc +++ b/source/extensions/filters/common/expr/evaluator.cc @@ -64,23 +64,26 @@ absl::optional StreamActivation::FindValue(absl::string_view name, return CelValue::CreateMap( Protobuf::Arena::Create(arena, *arena, info.filterState())); case ActivationToken::XDS: - return CelValue::CreateMap(Protobuf::Arena::Create(arena, *arena, info)); - }; + return CelValue::CreateMap( + Protobuf::Arena::Create(arena, *arena, info, local_info_)); + } return {}; } void StreamActivation::resetActivation() const { + local_info_ = nullptr; activation_info_ = nullptr; activation_request_headers_ = nullptr; activation_response_headers_ = nullptr; activation_response_trailers_ = nullptr; } -ActivationPtr createActivation(const StreamInfo::StreamInfo& info, +ActivationPtr createActivation(const LocalInfo::LocalInfo* local_info, + const StreamInfo::StreamInfo& info, const Http::RequestHeaderMap* request_headers, const Http::ResponseHeaderMap* response_headers, const Http::ResponseTrailerMap* response_trailers) { - return std::make_unique(info, request_headers, response_headers, + return std::make_unique(local_info, info, request_headers, response_headers, response_trailers); } @@ -131,11 +134,13 @@ ExpressionPtr createExpression(Builder& builder, const google::api::expr::v1alph } absl::optional evaluate(const Expression& expr, Protobuf::Arena& arena, + const LocalInfo::LocalInfo* local_info, const StreamInfo::StreamInfo& info, const Http::RequestHeaderMap* request_headers, const Http::ResponseHeaderMap* response_headers, const Http::ResponseTrailerMap* response_trailers) { - auto activation = createActivation(info, request_headers, response_headers, response_trailers); + auto activation = + createActivation(local_info, info, request_headers, response_headers, response_trailers); auto eval_status = expr.Evaluate(*activation, &arena); if (!eval_status.ok()) { return {}; @@ -147,7 +152,7 @@ absl::optional evaluate(const Expression& expr, Protobuf::Arena& arena bool matches(const Expression& expr, const StreamInfo::StreamInfo& info, const Http::RequestHeaderMap& headers) { Protobuf::Arena arena; - auto eval_status = Expr::evaluate(expr, arena, info, &headers, nullptr, nullptr); + auto eval_status = Expr::evaluate(expr, arena, nullptr, info, &headers, nullptr, nullptr); if (!eval_status.has_value()) { return false; } diff --git a/source/extensions/filters/common/expr/evaluator.h b/source/extensions/filters/common/expr/evaluator.h index 656a9c7e676f..e250469298fe 100644 --- a/source/extensions/filters/common/expr/evaluator.h +++ b/source/extensions/filters/common/expr/evaluator.h @@ -26,11 +26,13 @@ using ExpressionPtr = std::unique_ptr; // Base class for the context used by the CEL evaluator to look up attributes. class StreamActivation : public google::api::expr::runtime::BaseActivation { public: - StreamActivation(const StreamInfo::StreamInfo& info, + StreamActivation(const ::Envoy::LocalInfo::LocalInfo* local_info, + const StreamInfo::StreamInfo& info, const ::Envoy::Http::RequestHeaderMap* request_headers, const ::Envoy::Http::ResponseHeaderMap* response_headers, const ::Envoy::Http::ResponseTrailerMap* response_trailers) - : activation_info_(&info), activation_request_headers_(request_headers), + : local_info_(local_info), activation_info_(&info), + activation_request_headers_(request_headers), activation_response_headers_(response_headers), activation_response_trailers_(response_trailers) {} @@ -44,6 +46,7 @@ class StreamActivation : public google::api::expr::runtime::BaseActivation { protected: void resetActivation() const; + mutable const ::Envoy::LocalInfo::LocalInfo* local_info_{nullptr}; mutable const StreamInfo::StreamInfo* activation_info_{nullptr}; mutable const ::Envoy::Http::RequestHeaderMap* activation_request_headers_{nullptr}; mutable const ::Envoy::Http::ResponseHeaderMap* activation_response_headers_{nullptr}; @@ -52,7 +55,8 @@ class StreamActivation : public google::api::expr::runtime::BaseActivation { // Creates an activation providing the common context attributes. // The activation lazily creates wrappers during an evaluation using the evaluation arena. -ActivationPtr createActivation(const StreamInfo::StreamInfo& info, +ActivationPtr createActivation(const ::Envoy::LocalInfo::LocalInfo* local_info, + const StreamInfo::StreamInfo& info, const ::Envoy::Http::RequestHeaderMap* request_headers, const ::Envoy::Http::ResponseHeaderMap* response_headers, const ::Envoy::Http::ResponseTrailerMap* response_trailers); @@ -84,6 +88,7 @@ ExpressionPtr createExpression(Builder& builder, const google::api::expr::v1alph // Evaluates an expression for a request. The arena is used to hold intermediate computational // results and potentially the final value. absl::optional evaluate(const Expression& expr, Protobuf::Arena& arena, + const ::Envoy::LocalInfo::LocalInfo* local_info, const StreamInfo::StreamInfo& info, const ::Envoy::Http::RequestHeaderMap* request_headers, const ::Envoy::Http::ResponseHeaderMap* response_headers, diff --git a/source/extensions/formatter/cel/cel.cc b/source/extensions/formatter/cel/cel.cc index 74160a6651bb..ffee3e8c73ff 100644 --- a/source/extensions/formatter/cel/cel.cc +++ b/source/extensions/formatter/cel/cel.cc @@ -15,10 +15,12 @@ namespace Formatter { namespace Expr = Filters::Common::Expr; -CELFormatter::CELFormatter(Expr::BuilderInstanceSharedPtr expr_builder, +CELFormatter::CELFormatter(const ::Envoy::LocalInfo::LocalInfo& local_info, + Expr::BuilderInstanceSharedPtr expr_builder, const google::api::expr::v1alpha1::Expr& input_expr, absl::optional& max_length) - : expr_builder_(expr_builder), parsed_expr_(input_expr), max_length_(max_length) { + : local_info_(local_info), expr_builder_(expr_builder), parsed_expr_(input_expr), + max_length_(max_length) { compiled_expr_ = Expr::createExpression(expr_builder_->builder(), parsed_expr_); } @@ -26,8 +28,9 @@ absl::optional CELFormatter::formatWithContext(const Envoy::Formatter::HttpFormatterContext& context, const StreamInfo::StreamInfo& stream_info) const { Protobuf::Arena arena; - auto eval_status = Expr::evaluate(*compiled_expr_, arena, stream_info, &context.requestHeaders(), - &context.responseHeaders(), &context.responseTrailers()); + auto eval_status = + Expr::evaluate(*compiled_expr_, arena, &local_info_, stream_info, &context.requestHeaders(), + &context.responseHeaders(), &context.responseTrailers()); if (!eval_status.has_value() || eval_status.value().IsError()) { return absl::nullopt; } @@ -60,7 +63,8 @@ CELFormatterCommandParser::parse(const std::string& command, const std::string& parse_status.status().ToString()); } - return std::make_unique(expr_builder_, parse_status.value().expr(), max_length); + return std::make_unique(local_info_, expr_builder_, parse_status.value().expr(), + max_length); } return nullptr; diff --git a/source/extensions/formatter/cel/cel.h b/source/extensions/formatter/cel/cel.h index b1135f5ffe03..2c5a600a3397 100644 --- a/source/extensions/formatter/cel/cel.h +++ b/source/extensions/formatter/cel/cel.h @@ -14,7 +14,8 @@ namespace Formatter { class CELFormatter : public ::Envoy::Formatter::FormatterProvider { public: - CELFormatter(Extensions::Filters::Common::Expr::BuilderInstanceSharedPtr, + CELFormatter(const ::Envoy::LocalInfo::LocalInfo& local_info, + Extensions::Filters::Common::Expr::BuilderInstanceSharedPtr, const google::api::expr::v1alpha1::Expr&, absl::optional&); absl::optional @@ -24,6 +25,7 @@ class CELFormatter : public ::Envoy::Formatter::FormatterProvider { const StreamInfo::StreamInfo&) const override; private: + const ::Envoy::LocalInfo::LocalInfo& local_info_; Extensions::Filters::Common::Expr::BuilderInstanceSharedPtr expr_builder_; const google::api::expr::v1alpha1::Expr parsed_expr_; const absl::optional max_length_; @@ -33,12 +35,14 @@ class CELFormatter : public ::Envoy::Formatter::FormatterProvider { class CELFormatterCommandParser : public ::Envoy::Formatter::CommandParser { public: CELFormatterCommandParser(Server::Configuration::CommonFactoryContext& context) - : expr_builder_(Extensions::Filters::Common::Expr::getBuilder(context)){}; + : local_info_(context.localInfo()), + expr_builder_(Extensions::Filters::Common::Expr::getBuilder(context)){}; ::Envoy::Formatter::FormatterProviderPtr parse(const std::string& command, const std::string& subcommand, absl::optional& max_length) const override; private: + const ::Envoy::LocalInfo::LocalInfo& local_info_; Extensions::Filters::Common::Expr::BuilderInstanceSharedPtr expr_builder_; }; diff --git a/source/extensions/matching/http/cel_input/cel_input.h b/source/extensions/matching/http/cel_input/cel_input.h index 324d2fd264cd..d1d718a4705b 100644 --- a/source/extensions/matching/http/cel_input/cel_input.h +++ b/source/extensions/matching/http/cel_input/cel_input.h @@ -43,6 +43,7 @@ class HttpCelDataInput : public Matcher::DataInput activation = Extensions::Filters::Common::Expr::createActivation( + nullptr, // TODO: pass local_info to CEL activation. data.streamInfo(), maybe_request_headers.ptr(), maybe_response_headers.ptr(), maybe_response_trailers.ptr()); diff --git a/source/extensions/rate_limit_descriptors/expr/config.cc b/source/extensions/rate_limit_descriptors/expr/config.cc index a50dec82f0f2..e6e4b8044a59 100644 --- a/source/extensions/rate_limit_descriptors/expr/config.cc +++ b/source/extensions/rate_limit_descriptors/expr/config.cc @@ -36,7 +36,7 @@ class ExpressionDescriptor : public RateLimit::DescriptorProducer { const Http::RequestHeaderMap& headers, const StreamInfo::StreamInfo& info) const override { ProtobufWkt::Arena arena; - const auto result = Filters::Common::Expr::evaluate(*compiled_expr_.get(), arena, info, + const auto result = Filters::Common::Expr::evaluate(*compiled_expr_.get(), arena, nullptr, info, &headers, nullptr, nullptr); if (!result.has_value() || result.value().IsError()) { // If result is an error and if skip_if_error is true skip this descriptor, diff --git a/test/extensions/filters/common/expr/BUILD b/test/extensions/filters/common/expr/BUILD index aff36a60b926..ddda2980b9b8 100644 --- a/test/extensions/filters/common/expr/BUILD +++ b/test/extensions/filters/common/expr/BUILD @@ -24,6 +24,7 @@ envoy_extension_cc_test( "//source/extensions/clusters/original_dst:original_dst_cluster_lib", "//source/extensions/filters/common/expr:cel_state_lib", "//source/extensions/filters/common/expr:context_lib", + "//test/mocks/local_info:local_info_mocks", "//test/mocks/network:network_mocks", "//test/mocks/router:router_mocks", "//test/mocks/ssl:ssl_mocks", diff --git a/test/extensions/filters/common/expr/context_test.cc b/test/extensions/filters/common/expr/context_test.cc index 180116b8ac56..0c4f449d9ba2 100644 --- a/test/extensions/filters/common/expr/context_test.cc +++ b/test/extensions/filters/common/expr/context_test.cc @@ -7,6 +7,7 @@ #include "source/extensions/filters/common/expr/cel_state.h" #include "source/extensions/filters/common/expr/context.h" +#include "test/mocks/local_info/mocks.h" #include "test/mocks/network/mocks.h" #include "test/mocks/router/mocks.h" #include "test/mocks/ssl/mocks.h" @@ -829,6 +830,7 @@ TEST(Context, FilterStateAttributes) { } TEST(Context, XDSAttributes) { + NiceMock local_info; NiceMock info; std::shared_ptr> cluster_info( new NiceMock()); @@ -847,7 +849,7 @@ TEST(Context, XDSAttributes) { info.downstream_connection_info_provider_->setFilterChainInfo(filter_chain_info); Protobuf::Arena arena; - XDSWrapper wrapper(arena, info); + XDSWrapper wrapper(arena, info, &local_info); { const auto value = wrapper[CelValue::CreateStringView(ClusterName)]; @@ -893,6 +895,11 @@ TEST(Context, XDSAttributes) { const auto value = wrapper[CelValue::CreateInt64(5)]; EXPECT_FALSE(value.has_value()); } + { + const auto value = wrapper[CelValue::CreateStringView(Node)]; + EXPECT_TRUE(value.has_value()); + ASSERT_TRUE(value.value().IsMessage()); + } } } // namespace diff --git a/test/extensions/filters/common/expr/evaluator_fuzz_test.cc b/test/extensions/filters/common/expr/evaluator_fuzz_test.cc index 2446fb4a8ac3..a09c51bb4a2a 100644 --- a/test/extensions/filters/common/expr/evaluator_fuzz_test.cc +++ b/test/extensions/filters/common/expr/evaluator_fuzz_test.cc @@ -44,7 +44,7 @@ DEFINE_PROTO_FUZZER(const test::extensions::filters::common::expr::EvaluatorTest // Evaluate the CEL expression. Protobuf::Arena arena; - Expr::evaluate(*expr, arena, *stream_info, &request_headers, &response_headers, + Expr::evaluate(*expr, arena, nullptr, *stream_info, &request_headers, &response_headers, &response_trailers); } catch (const CelException& e) { ENVOY_LOG_MISC(debug, "CelException: {}", e.what()); diff --git a/test/extensions/formatter/cel/cel_test.cc b/test/extensions/formatter/cel/cel_test.cc index f3680af88c4b..2ada1036d0b5 100644 --- a/test/extensions/formatter/cel/cel_test.cc +++ b/test/extensions/formatter/cel/cel_test.cc @@ -34,6 +34,14 @@ class CELFormatterTest : public ::testing::Test { }; #ifdef USE_CEL_PARSER +TEST_F(CELFormatterTest, TestNodeId) { + auto cel_parser = std::make_unique(context_.server_factory_context_); + absl::optional max_length = absl::nullopt; + auto formatter = cel_parser->parse("CEL", "xds.node.id", max_length); + EXPECT_THAT(formatter->formatValueWithContext(formatter_context_, stream_info_), + ProtoEq(ValueUtil::stringValue("node_name"))); +} + TEST_F(CELFormatterTest, TestFormatValue) { auto cel_parser = std::make_unique(context_.server_factory_context_); absl::optional max_length = absl::nullopt;