GrpcFieldExtraction: Supports extracting fields of type `map<string, …

…string>` in addition to string. Risk Level: Low Testing: Unit tests Docs Changes: Inline with the filter API proto. Release Notes: This change is backward compatible and no behavior change is expected for existing users. Signed-off-by: Xi Wu <xiwuxw@google.com>
envoyproxy · Jul 25, 2024 · d300abe · d300abe
1 parent bea314b
commit d300abe
Show file tree

Hide file tree

Showing 8 changed files with 50 additions and 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
@@ -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<string, string>`.
 //
 // 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<StringValue>`` -> ``values<ListValue of StringValue>``, which will be added in the dynamic ``metadata<google.protobuf.Struct>``.
+// 1. the extracted field names/values will be wrapped in be ``field<StringValue
+// or MapValue>`` -> ``values<ListValue of StringValue or StructValue>``, which will be added in the dynamic ``metadata<google.protobuf.Struct>``.
 //
-// 2. if the field value is empty, a empty ``<ListValue>`` will be set.
+// 2. if the field value is empty, an empty ``<ListValue>`` will be set.
 //
 // Performance
 // -----------

diff --git a/bazel/repository_locations.bzl b/bazel/repository_locations.bzl
@@ -976,13 +976,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/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<std::string> values;
+  // The request field value.
+  ProtobufWkt::Value value;
 };
 
 using ExtractionResult = std::vector<RequestField>;

diff --git 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<ProtobufWkt::Value> 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
@@ -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
@@ -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
@@ -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
@@ -94,6 +94,8 @@ message RepeatedSupportedTypes {
   repeated float float = 12;
 
   repeated double double = 13;
+
+  map<string, string> map = 14;
 }
 
 message UnsupportedTypes {