grpc_field_extraction: add value_type: STRING to fix ratelimit filter integration

api/envoy/extensions/filters/http/grpc_field_extraction/v3/config.proto

@@ -181,10 +181,51 @@ message FieldExtractions {
 }
 
 message RequestFieldValueDisposition {
+  // Controls how the extracted value is written to dynamic metadata.
+  //
+  // The default LIST wraps every extracted value in a ``ListValue``, which
+  // preserves the repeated-field semantics of the original proto field.
+  // However, the Envoy ratelimit filter's ``metadata`` descriptor action
+  // reads ``StringValue`` entries via ``.string_value()``; a ``ListValue``
+  // always yields the empty string, so rate limiting never fires for gRPC
+  // fields when using the default mode.
+  //
+  // Set ``value_type`` to ``STRING`` to promote the first element of the
+  // extracted list to a ``StringValue`` so that the ratelimit filter can
+  // consume it directly. Only correct for singular (non-repeated) field
+  // paths — see the ``STRING`` enum value for the full constraint and the
+  // undercounting hazard for repeated fields.
+  enum ValueType {
+    // Default: extracted values are wrapped in a ListValue.
+    LIST = 0;
+    // Extracted value is promoted to a StringValue, making it consumable by
+    // the Envoy ratelimit filter's ``metadata`` descriptor action via
+    // ``.string_value()``.
+    //
+    // Only correct when every segment of the field path is singular
+    // (non-repeated). Examples: ``tenant_id``, ``request.header.authority``.
+    //
+    // If any path segment is repeated, only the first extracted value is
+    // written to dynamic metadata; all subsequent values are silently
+    // discarded. This causes rate-limit undercounting — a single request
+    // touching N values is only charged against the first one's bucket.
+    // Use ``LIST`` mode with a Lua bridge if you need to rate-limit across
+    // all values of a repeated field.
+    //
+    // Falls back to an empty ``ListValue`` if the field is absent from the
+    // message (same as ``LIST`` mode), so a missing field never promotes to
+    // an empty string.
+    STRING = 1;
+  }
+
   oneof disposition {
     // The dynamic metadata namespace. If empty, "envoy.filters.http.grpc_field_extraction" will be used by default.
     //
     // Unimplemented. Uses "envoy.filters.http.grpc_field_extraction" for now.
     string dynamic_metadata = 1;
   }
+
+  // Controls the type of the value written to dynamic metadata.
+  // Defaults to ``LIST`` for backward compatibility.
+  ValueType value_type = 2;
 }

changelogs/current/bug_fixes/grpc_field_extraction__fixed-ratelimit-metadata-descriptor-always-empty.rst

@@ -0,0 +1,9 @@
+``grpc_field_extraction``: fixed a bug where the ratelimit filter's ``metadata`` descriptor action
+was silently disabled for all gRPC requests. The extraction filter always wrote extracted field
+values as a ``ListValue``, but ``MetaDataAction::populateDescriptor`` calls ``.string_value()`` on
+the result; calling ``.string_value()`` on a ``ListValue`` returns ``""`` (wrong ``oneof`` arm),
+causing ``populateDescriptor`` to return ``false`` and skip the rate limit check entirely. A new
+``value_type: STRING`` option on ``RequestFieldValueDisposition`` promotes the first extracted
+element to a ``StringValue`` before writing to dynamic metadata, allowing the ratelimit filter to
+consume it correctly. The default ``LIST`` mode is unchanged for backward
+compatibility.

source/extensions/filters/http/grpc_field_extraction/extractor_impl.cc

@@ -39,13 +39,17 @@ absl::Status ExtractorImpl::init(
     auto extractor_or_error = extractor_factory.Create(request_type_url, it.first);
     RETURN_IF_NOT_OK(extractor_or_error.status());
     per_field_extractors_.emplace(it.first, std::move(extractor_or_error.value()));
+    per_field_value_types_.emplace(it.first, it.second.value_type());
   }
   return absl::OkStatus();
 }
 
 absl::StatusOr<ExtractionResult>
 ExtractorImpl::processRequest(Protobuf::field_extraction::MessageData& message) const {
 
+  using RequestFieldValueDisposition =
+      envoy::extensions::filters::http::grpc_field_extraction::v3::RequestFieldValueDisposition;
+
   ExtractionResult result;
   for (const auto& it : per_field_extractors_) {
     absl::StatusOr<Protobuf::Value> extracted_value = it.second->ExtractValue(message);
@@ -55,7 +59,20 @@ ExtractorImpl::processRequest(Protobuf::field_extraction::MessageData& message)
 
     ENVOY_LOG_MISC(debug, "extracted the following resource values from the {} field: {}", it.first,
                    extracted_value->DebugString());
-    result.push_back({it.first, std::move(*extracted_value)});
+
+    // When value_type is STRING, promote the first list element to a StringValue
+    // so the Envoy ratelimit filter's metadata descriptor action can consume it.
+    // ExtractValue() always returns a ListValue on success; absent fields return
+    // an empty ListValue, so we guard only on size before promoting.
+    const auto value_type = per_field_value_types_.at(it.first);
+    if (value_type == RequestFieldValueDisposition::STRING &&
+        extracted_value->list_value().values_size() > 0) {
+      Protobuf::Value promoted;
+      promoted.set_string_value(extracted_value->list_value().values(0).string_value());
+      result.push_back({it.first, std::move(promoted)});
+    } else {
+      result.push_back({it.first, std::move(*extracted_value)});
+    }
   }
 
   return result;

source/extensions/filters/http/grpc_field_extraction/extractor_impl.h

@@ -20,6 +20,9 @@ namespace GrpcFieldExtraction {
 
 using FieldValueExtractorPtr =
     std::unique_ptr<Protobuf::field_extraction::FieldValueExtractorInterface>;
+using RequestFieldValueType = envoy::extensions::filters::http::grpc_field_extraction::v3::
+    RequestFieldValueDisposition::ValueType;
+
 class ExtractorImpl : public Extractor {
 public:
   static absl::StatusOr<ExtractorImpl>
@@ -39,6 +42,9 @@ class ExtractorImpl : public Extractor {
            field_extractions);
 
   absl::flat_hash_map<absl::string_view, FieldValueExtractorPtr> per_field_extractors_;
+  // Mirrors per_field_extractors_ key-for-key. Stores the configured value_type
+  // so processRequest() can promote ListValue → StringValue when STRING is set.
+  absl::flat_hash_map<absl::string_view, RequestFieldValueType> per_field_value_types_;
 };
 
 class ExtractorFactoryImpl : public ExtractorFactory {

test/extensions/filters/http/grpc_field_extraction/filter_test.cc

@@ -1014,6 +1014,166 @@ TEST_F(FilterTestPassThrough, UnconfiguredRequest) {
   EXPECT_EQ(Envoy::Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(req_headers, true));
 }
 
+// Tests for value_type = STRING — the extracted ListValue should be promoted
+// to a StringValue so the Envoy ratelimit filter's metadata descriptor action
+// can consume it via .string_value().
+
+using FilterTestValueTypeString = FilterTestBase;
+
+// A populated singular string field with value_type = STRING should produce a
+// StringValue in dynamic metadata, not a ListValue.
+TEST_F(FilterTestValueTypeString, SingleStringFieldPromotedToStringValue) {
+  setUp(R"pb(
+extractions_by_method: {
+  key: "apikeys.ApiKeys.CreateApiKey"
+  value: {
+    request_field_extractions: {
+      key: "parent"
+      value: {
+        value_type: STRING
+      }
+    }
+  }
+})pb");
+  TestRequestHeaderMapImpl req_headers =
+      TestRequestHeaderMapImpl{{":method", "POST"},
+                               {":path", "/apikeys.ApiKeys/CreateApiKey"},
+                               {"content-type", "application/grpc"}};
+  EXPECT_EQ(Envoy::Http::FilterHeadersStatus::StopIteration,
+            filter_->decodeHeaders(req_headers, true));
+
+  CreateApiKeyRequest request = makeCreateApiKeyRequest(R"pb(parent: "project-id")pb");
+  Envoy::Buffer::InstancePtr request_data = Envoy::Grpc::Common::serializeToGrpcFrame(request);
+  EXPECT_CALL(mock_decoder_callbacks_.stream_info_, setDynamicMetadata(_, _))
+      .WillOnce(Invoke([](const std::string& ns, const Protobuf::Struct& metadata) {
+        EXPECT_EQ(ns, "envoy.filters.http.grpc_field_extraction");
+        const auto it = metadata.fields().find("parent");
+        ASSERT_TRUE(it != metadata.fields().end());
+        // Must be a StringValue, not a ListValue.
+        EXPECT_EQ(it->second.kind_case(), Protobuf::Value::KindCase::kStringValue);
+        EXPECT_EQ(it->second.string_value(), "project-id");
+      }));
+  EXPECT_EQ(Envoy::Http::FilterDataStatus::Continue, filter_->decodeData(*request_data, true));
+
+  // Body must be passed through unmodified.
+  checkSerializedData<CreateApiKeyRequest>(*request_data, {request});
+}
+
+// A missing field with value_type = STRING should fall back to an empty
+// ListValue (same behaviour as the default LIST mode) rather than crashing.
+TEST_F(FilterTestValueTypeString, MissingFieldFallsBackToEmptyListValue) {
+  setUp(R"pb(
+extractions_by_method: {
+  key: "apikeys.ApiKeys.CreateApiKey"
+  value: {
+    request_field_extractions: {
+      key: "key.display_name"
+      value: {
+        value_type: STRING
+      }
+    }
+  }
+})pb");
+  TestRequestHeaderMapImpl req_headers =
+      TestRequestHeaderMapImpl{{":method", "POST"},
+                               {":path", "/apikeys.ApiKeys/CreateApiKey"},
+                               {"content-type", "application/grpc"}};
+  EXPECT_EQ(Envoy::Http::FilterHeadersStatus::StopIteration,
+            filter_->decodeHeaders(req_headers, true));
+
+  // Request does not set key.display_name.
+  CreateApiKeyRequest request = makeCreateApiKeyRequest(R"pb(parent: "project-id")pb");
+  Envoy::Buffer::InstancePtr request_data = Envoy::Grpc::Common::serializeToGrpcFrame(request);
+  EXPECT_CALL(mock_decoder_callbacks_.stream_info_, setDynamicMetadata(_, _))
+      .WillOnce(Invoke([](const std::string& ns, const Protobuf::Struct& metadata) {
+        EXPECT_EQ(ns, "envoy.filters.http.grpc_field_extraction");
+        const auto it = metadata.fields().find("key.display_name");
+        ASSERT_TRUE(it != metadata.fields().end());
+        // Absent field falls back to an empty ListValue, not a StringValue.
+        EXPECT_EQ(it->second.kind_case(), Protobuf::Value::KindCase::kListValue);
+        EXPECT_EQ(it->second.list_value().values_size(), 0);
+      }));
+  EXPECT_EQ(Envoy::Http::FilterDataStatus::Continue, filter_->decodeData(*request_data, true));
+
+  checkSerializedData<CreateApiKeyRequest>(*request_data, {request});
+}
+
+// A repeated field with value_type = STRING promotes only the first element;
+// subsequent values are intentionally dropped.
+TEST_F(FilterTestValueTypeString, RepeatedFieldPromotesFirstElementOnly) {
+  setUp(R"pb(
+extractions_by_method: {
+  key: "apikeys.ApiKeys.CreateApiKey"
+  value: {
+    request_field_extractions: {
+      key: "repeated_supported_types.string"
+      value: {
+        value_type: STRING
+      }
+    }
+  }
+})pb");
+  TestRequestHeaderMapImpl req_headers =
+      TestRequestHeaderMapImpl{{":method", "POST"},
+                               {":path", "/apikeys.ApiKeys/CreateApiKey"},
+                               {"content-type", "application/grpc"}};
+  EXPECT_EQ(Envoy::Http::FilterHeadersStatus::StopIteration,
+            filter_->decodeHeaders(req_headers, true));
+
+  // Two string values in the repeated field; only the first should survive.
+  CreateApiKeyRequest request = makeCreateApiKeyRequest(R"pb(
+      repeated_supported_types: { string: "first" string: "second" }
+  )pb");
+  Envoy::Buffer::InstancePtr request_data = Envoy::Grpc::Common::serializeToGrpcFrame(request);
+  EXPECT_CALL(mock_decoder_callbacks_.stream_info_, setDynamicMetadata(_, _))
+      .WillOnce(Invoke([](const std::string& ns, const Protobuf::Struct& metadata) {
+        EXPECT_EQ(ns, "envoy.filters.http.grpc_field_extraction");
+        const auto it = metadata.fields().find("repeated_supported_types.string");
+        ASSERT_TRUE(it != metadata.fields().end());
+        EXPECT_EQ(it->second.kind_case(), Protobuf::Value::KindCase::kStringValue);
+        EXPECT_EQ(it->second.string_value(), "first");
+      }));
+  EXPECT_EQ(Envoy::Http::FilterDataStatus::Continue, filter_->decodeData(*request_data, true));
+
+  checkSerializedData<CreateApiKeyRequest>(*request_data, {request});
+}
+
+// With the default value_type (LIST / omitted), behaviour is unchanged from
+// before this change — extracted values remain wrapped in a ListValue.
+TEST_F(FilterTestValueTypeString, DefaultValueTypePreservesListValue) {
+  setUp(R"pb(
+extractions_by_method: {
+  key: "apikeys.ApiKeys.CreateApiKey"
+  value: {
+    request_field_extractions: {
+      key: "parent"
+      value: {}
+    }
+  }
+})pb");
+  TestRequestHeaderMapImpl req_headers =
+      TestRequestHeaderMapImpl{{":method", "POST"},
+                               {":path", "/apikeys.ApiKeys/CreateApiKey"},
+                               {"content-type", "application/grpc"}};
+  EXPECT_EQ(Envoy::Http::FilterHeadersStatus::StopIteration,
+            filter_->decodeHeaders(req_headers, true));
+
+  CreateApiKeyRequest request = makeCreateApiKeyRequest(R"pb(parent: "project-id")pb");
+  Envoy::Buffer::InstancePtr request_data = Envoy::Grpc::Common::serializeToGrpcFrame(request);
+  EXPECT_CALL(mock_decoder_callbacks_.stream_info_, setDynamicMetadata(_, _))
+      .WillOnce(Invoke([](const std::string& ns, const Protobuf::Struct& metadata) {
+        EXPECT_EQ(ns, "envoy.filters.http.grpc_field_extraction");
+        const auto it = metadata.fields().find("parent");
+        ASSERT_TRUE(it != metadata.fields().end());
+        EXPECT_EQ(it->second.kind_case(), Protobuf::Value::KindCase::kListValue);
+        EXPECT_EQ(it->second.list_value().values_size(), 1);
+        EXPECT_EQ(it->second.list_value().values(0).string_value(), "project-id");
+      }));
+  EXPECT_EQ(Envoy::Http::FilterDataStatus::Continue, filter_->decodeData(*request_data, true));
+
+  checkSerializedData<CreateApiKeyRequest>(*request_data, {request});
+}
+
 } // namespace
 
 } // namespace GrpcFieldExtraction