From 1cd67c5821125a246e2a4f13254f8d6c69068705 Mon Sep 17 00:00:00 2001 From: Xi Wu Date: Tue, 13 Aug 2024 07:25:25 -0700 Subject: [PATCH] GrpcFieldExtraction: Supports extracting fields of type `map` in addition to string (#35162) Commit Message: GrpcFieldExtraction: Supports extracting fields of type `map` in addition to string Additional Description: Risk Level: Low Testing: Unit test Docs Changes: Inline with the filter API proto. Release Notes: This change is backward compatible and no behavior change is expected for existing users. Platform Specific Features: --------- Signed-off-by: Xi Wu --- .../grpc_field_extraction/v3/config.proto | 9 ++++--- bazel/repository_locations.bzl | 6 ++--- changelogs/current.yaml | 3 +++ .../http/grpc_field_extraction/extractor.h | 4 +-- .../grpc_field_extraction/extractor_impl.cc | 15 ++++------- .../http/grpc_field_extraction/filter.cc | 5 +--- .../filter_config_test.cc | 5 ++++ .../http/grpc_field_extraction/filter_test.cc | 26 +++++++++++++++++++ test/proto/apikeys.proto | 2 ++ 9 files changed, 53 insertions(+), 22 deletions(-) diff --git a/api/envoy/extensions/filters/http/grpc_field_extraction/v3/config.proto b/api/envoy/extensions/filters/http/grpc_field_extraction/v3/config.proto index 3684f994d65f..aae62145731a 100644 --- a/api/envoy/extensions/filters/http/grpc_field_extraction/v3/config.proto +++ b/api/envoy/extensions/filters/http/grpc_field_extraction/v3/config.proto @@ -52,7 +52,9 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // // Here are config requirements // -// 1. the target field should be among the following primitive types: `string`, `uint32`, `uint64`, `int32`, `int64`, `sint32`, `sint64`, `fixed32`, `fixed64`, `sfixed32`, `sfixed64`, `float`, `double`. +// 1. the target field should be among the following primitive types: `string`, +// `uint32`, `uint64`, `int32`, `int64`, `sint32`, `sint64`, `fixed32`, +// `fixed64`, `sfixed32`, `sfixed64`, `float`, `double`, `map`. // // 2. the target field could be repeated. // @@ -61,9 +63,10 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // Output Format // ------------- // -// 1. the extracted field names/values will be wrapped in be ``field`` -> ``values``, which will be added in the dynamic ``metadata``. +// 1. the extracted field names/values will be wrapped in be ``field`` -> ``values``, which will be added in the dynamic ``metadata``. // -// 2. if the field value is empty, a empty ```` will be set. +// 2. if the field value is empty, an empty ```` will be set. // // Performance // ----------- diff --git a/bazel/repository_locations.bzl b/bazel/repository_locations.bzl index de3fc7cc2ddb..0ef1e9091d91 100644 --- a/bazel/repository_locations.bzl +++ b/bazel/repository_locations.bzl @@ -985,13 +985,13 @@ REPOSITORY_LOCATIONS_SPEC = dict( project_name = "proto-field-extraction", project_desc = "Library that supports the extraction from protobuf binary", project_url = "https://github.com/grpc-ecosystem/proto-field-extraction", - version = "2dfe27548e1f21a665f9068b97b2fc5beb678566", - sha256 = "ddbbd0dd07012339ac467f5fdac5c294e1efcdc93bb4b7152d468ddbfc9772f0", + version = "d5d39f0373e9b6691c32c85929838b1006bcb3fb", + sha256 = "cba864db90806515afa553aaa2fb3683df2859a7535e53a32cb9619da9cebc59", strip_prefix = "proto-field-extraction-{version}", urls = ["https://github.com/grpc-ecosystem/proto-field-extraction/archive/{version}.zip"], use_category = ["dataplane_ext"], extensions = ["envoy.filters.http.grpc_json_transcoder", "envoy.filters.http.grpc_field_extraction"], - release_date = "2023-07-18", + release_date = "2024-07-10", cpe = "N/A", license = "Apache-2.0", license_url = "https://github.com/grpc-ecosystem/proto-field-extraction/blob/{version}/LICENSE", diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 86f5204385de..26e77912e721 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -175,6 +175,9 @@ new_features: change: | Prefer using IPv6 address when addresses from both families are available. Can be reverted by setting ``envoy.reloadable_features.prefer_ipv6_dns_on_macos`` to false. +- area: grpc_field_extraction + change: | + Added ``map`` support: Target fields of type ``map`` can be extracted and added to dynamic metadata. - area: rbac change: | Added :ref:`delay_deny ` to support deny connection after diff --git a/source/extensions/filters/http/grpc_field_extraction/extractor.h b/source/extensions/filters/http/grpc_field_extraction/extractor.h index 4178e73fb090..d321b3651ec4 100644 --- a/source/extensions/filters/http/grpc_field_extraction/extractor.h +++ b/source/extensions/filters/http/grpc_field_extraction/extractor.h @@ -24,8 +24,8 @@ struct RequestField { // The request field path. absl::string_view path; - // The request field values. - std::vector values; + // The request field value. + ProtobufWkt::Value value; }; using ExtractionResult = std::vector; diff --git a/source/extensions/filters/http/grpc_field_extraction/extractor_impl.cc b/source/extensions/filters/http/grpc_field_extraction/extractor_impl.cc index adbaa44181bd..8470310aa09b 100644 --- a/source/extensions/filters/http/grpc_field_extraction/extractor_impl.cc +++ b/source/extensions/filters/http/grpc_field_extraction/extractor_impl.cc @@ -7,7 +7,6 @@ #include "source/common/common/logger.h" -#include "absl/strings/str_format.h" #include "proto_field_extraction/field_value_extractor/field_value_extractor_factory.h" #include "proto_field_extraction/field_value_extractor/field_value_extractor_interface.h" @@ -39,18 +38,14 @@ ExtractorImpl::processRequest(Protobuf::field_extraction::MessageData& message) ExtractionResult result; for (const auto& it : per_field_extractors_) { - auto extracted_values = it.second->Extract(message); - if (!extracted_values.ok()) { - return extracted_values.status(); + absl::StatusOr extracted_value = it.second->ExtractValue(message); + if (!extracted_value.ok()) { + return extracted_value.status(); } ENVOY_LOG_MISC(debug, "extracted the following resource values from the {} field: {}", it.first, - std::accumulate(extracted_values.value().begin(), extracted_values.value().end(), - std::string(), - [](const std::string& lhs, const std::string& rhs) { - return absl::StrFormat("%s, %s", lhs, rhs); - })); - result.push_back({it.first, std::move(extracted_values.value())}); + extracted_value->DebugString()); + result.push_back({it.first, std::move(*extracted_value)}); } return result; diff --git a/source/extensions/filters/http/grpc_field_extraction/filter.cc b/source/extensions/filters/http/grpc_field_extraction/filter.cc index fb66c448dffd..7ae1fb12f326 100644 --- a/source/extensions/filters/http/grpc_field_extraction/filter.cc +++ b/source/extensions/filters/http/grpc_field_extraction/filter.cc @@ -214,10 +214,7 @@ void Filter::handleExtractionResult(const ExtractionResult& result) { ProtobufWkt::Struct dest_metadata; for (const auto& req_field : result) { RELEASE_ASSERT(!req_field.path.empty(), "`req_field.path` shouldn't be empty"); - auto* list = (*dest_metadata.mutable_fields())[req_field.path].mutable_list_value(); - for (const auto& value : req_field.values) { - list->add_values()->set_string_value(value); - } + (*dest_metadata.mutable_fields())[req_field.path] = req_field.value; } if (dest_metadata.fields_size() > 0) { ENVOY_STREAM_LOG(debug, "injected dynamic metadata `{}` with `{}`", *decoder_callbacks_, diff --git a/test/extensions/filters/http/grpc_field_extraction/filter_config_test.cc b/test/extensions/filters/http/grpc_field_extraction/filter_config_test.cc index 55485685d229..e973387b1bd3 100644 --- a/test/extensions/filters/http/grpc_field_extraction/filter_config_test.cc +++ b/test/extensions/filters/http/grpc_field_extraction/filter_config_test.cc @@ -200,6 +200,11 @@ extractions_by_method: { value: { } } + request_field_extractions: { + key: "repeated_supported_types.map" + value: { + } + } } })pb"); *proto_config_.mutable_descriptor_set()->mutable_filename() = diff --git a/test/extensions/filters/http/grpc_field_extraction/filter_test.cc b/test/extensions/filters/http/grpc_field_extraction/filter_test.cc index 166b1b322dd5..b4cf646a959e 100644 --- a/test/extensions/filters/http/grpc_field_extraction/filter_test.cc +++ b/test/extensions/filters/http/grpc_field_extraction/filter_test.cc @@ -639,6 +639,11 @@ extractions_by_method: { value: { } } + request_field_extractions: { + key: "repeated_supported_types.map" + value: { + } + } } })pb"); TestRequestHeaderMapImpl req_headers = @@ -677,6 +682,8 @@ repeated_supported_types: { sfixed64: 1111 float: 1.212 double: 1.313 + map { key: "key1" value: "value1" } + map { key: "key2" value: "value2" } } )pb"); @@ -853,6 +860,25 @@ fields { } } } +} +fields { + key: "repeated_supported_types.map" + value { + list_value { + values { + struct_value { + fields { + key: "key1" + value { string_value: "value1" } + } + fields { + key: "key2" + value { string_value: "value2" } + } + } + } + } + } })pb"); })); EXPECT_EQ(Envoy::Http::FilterDataStatus::Continue, filter_->decodeData(*request_data, true)); diff --git a/test/proto/apikeys.proto b/test/proto/apikeys.proto index a38bbd6f65dc..d292f7ad7d42 100644 --- a/test/proto/apikeys.proto +++ b/test/proto/apikeys.proto @@ -94,6 +94,8 @@ message RepeatedSupportedTypes { repeated float float = 12; repeated double double = 13; + + map map = 14; } message UnsupportedTypes {