ext_proc: allow override metadata without grpc_service

api/envoy/extensions/filters/http/ext_proc/v3/ext_proc.proto

@@ -3,6 +3,7 @@ syntax = "proto3";
 package envoy.extensions.filters.http.ext_proc.v3;
 
 import "envoy/config/common/mutation_rules/v3/mutation_rules.proto";
+import "envoy/config/core/v3/base.proto";
 import "envoy/config/core/v3/grpc_service.proto";
 import "envoy/extensions/filters/http/ext_proc/v3/processing_mode.proto";
 import "envoy/type/matcher/v3/string.proto";
@@ -248,7 +249,7 @@ message ExtProcPerRoute {
 }
 
 // Overrides that may be set on a per-route basis
-// [#next-free-field: 6]
+// [#next-free-field: 7]
 message ExtProcOverrides {
   // Set a different processing mode for this route than the default.
   ProcessingMode processing_mode = 1;
@@ -269,4 +270,10 @@ message ExtProcOverrides {
 
   // Set a different gRPC service for this route than the default.
   config.core.v3.GrpcService grpc_service = 5;
+
+  // Additional metadata to include into streams initiated to the ext_proc gRPC
+  // service. This can be used for scenarios in which additional ad hoc
+  // authorization headers (e.g. ``x-foo-bar: baz-key``) are to be injected or
+  // when a route needs to partially override inherited metadata.
+  repeated config.core.v3.HeaderValue metadata = 6;
 }

changelogs/current.yaml

@@ -16,5 +16,12 @@ removed_config_or_runtime:
 # *Normally occurs at the end of the* :ref:`deprecation period <deprecated>`
 
 new_features:
+- area: ext_proc
+  change: |
+    Added
+    :ref:`metadata <envoy_v3_api_field_extensions.filters.http.ext_proc.v3.ExtProcOverrides.metadata>`
+    config API to allow extending inherited metadata from ExternalProcessor
+    :ref:`grpc_service <envoy_v3_api_field_extensions.filters.http.ext_proc.v3.ExternalProcessor.grpc_service>`
+    with the new or updated values.
 
 deprecated:

source/extensions/filters/http/ext_proc/ext_proc.cc

@@ -129,6 +129,15 @@ FilterConfigPerRoute::initGrpcService(const ExtProcPerRoute& config) {
   }
   return absl::nullopt;
 }
+namespace {
+std::vector<envoy::config::core::v3::HeaderValue> initMetadata(const ExtProcPerRoute& config) {
+  std::vector<envoy::config::core::v3::HeaderValue> metadata;
+  for (const auto& header : config.overrides().metadata()) {
+    metadata.emplace_back(header);
+  }
+  return metadata;
+}
+} // namespace
 
 absl::optional<ProcessingMode>
 FilterConfigPerRoute::mergeProcessingMode(const FilterConfigPerRoute& less_specific,
@@ -140,16 +149,46 @@ FilterConfigPerRoute::mergeProcessingMode(const FilterConfigPerRoute& less_speci
                                                     : less_specific.processingMode();
 }
 
+namespace {
+// replace all entries with the same name or append one.
+void mergeHeaderValue(std::vector<envoy::config::core::v3::HeaderValue>& metadata,
+                      const envoy::config::core::v3::HeaderValue& header) {
+  size_t count = 0;
+  for (auto& dest : metadata) {
+    if (dest.key() == header.key()) {
+      dest.CopyFrom(header);
+      count++;
+    }
+  }
+  if (!count) {
+    metadata.emplace_back(header);
+  }
+}
+
+std::vector<envoy::config::core::v3::HeaderValue>
+mergeMetadata(const FilterConfigPerRoute& less_specific,
+              const FilterConfigPerRoute& more_specific) {
+  std::vector<envoy::config::core::v3::HeaderValue> metadata(less_specific.metadata());
+
+  for (const auto& header : more_specific.metadata()) {
+    mergeHeaderValue(metadata, header);
+  }
+
+  return metadata;
+}
+} // namespace
+
 FilterConfigPerRoute::FilterConfigPerRoute(const ExtProcPerRoute& config)
     : disabled_(config.disabled()), processing_mode_(initProcessingMode(config)),
-      grpc_service_(initGrpcService(config)) {}
+      grpc_service_(initGrpcService(config)), metadata_(initMetadata(config)) {}
 
 FilterConfigPerRoute::FilterConfigPerRoute(const FilterConfigPerRoute& less_specific,
                                            const FilterConfigPerRoute& more_specific)
     : disabled_(more_specific.disabled()),
       processing_mode_(mergeProcessingMode(less_specific, more_specific)),
       grpc_service_(more_specific.grpcService().has_value() ? more_specific.grpcService()
-                                                            : less_specific.grpcService()) {}
+                                                            : less_specific.grpcService()),
+      metadata_(mergeMetadata(less_specific, more_specific)) {}
 
 void Filter::setDecoderFilterCallbacks(Http::StreamDecoderFilterCallbacks& callbacks) {
   Http::PassThroughFilter::setDecoderFilterCallbacks(callbacks);
@@ -866,6 +905,23 @@ static ProcessingMode allDisabledMode() {
   return pm;
 }
 
+namespace {
+// replace all entries with the same name or append one.
+void mergeHeaderValue(Protobuf::RepeatedPtrField<::envoy::config::core::v3::HeaderValue>& metadata,
+                      const envoy::config::core::v3::HeaderValue& header) {
+  size_t count = 0;
+  for (auto& dest : metadata) {
+    if (dest.key() == header.key()) {
+      dest.CopyFrom(header);
+      count++;
+    }
+  }
+  if (!count) {
+    metadata.Add()->CopyFrom(header);
+  }
+}
+} // namespace
+
 void Filter::mergePerRouteConfig() {
   if (route_config_merged_) {
     return;
@@ -912,6 +968,16 @@ void Filter::mergePerRouteConfig() {
     grpc_service_ = *merged_config->grpcService();
     config_with_hash_key_.setConfig(*merged_config->grpcService());
   }
+  if (!merged_config->metadata().empty()) {
+    ENVOY_LOG(trace, "Overriding metadata from per-route configuration");
+    envoy::config::core::v3::GrpcService config = config_with_hash_key_.config();
+    auto ptr = config.mutable_initial_metadata();
+    for (const auto& header : merged_config->metadata()) {
+      ENVOY_LOG(trace, "Setting metadata {} = {}", header.key(), header.value());
+      mergeHeaderValue(*ptr, header);
+    }
+    config_with_hash_key_.setConfig(config);
+  }
 }
 
 std::string responseCaseToString(const ProcessingResponse::ResponseCase response_case) {

source/extensions/filters/http/ext_proc/ext_proc.h

@@ -224,6 +224,7 @@ class FilterConfigPerRoute : public Router::RouteSpecificFilterConfig {
   const absl::optional<const envoy::config::core::v3::GrpcService>& grpcService() const {
     return grpc_service_;
   }
+  const std::vector<envoy::config::core::v3::HeaderValue>& metadata() const { return metadata_; }
 
 private:
   absl::optional<envoy::extensions::filters::http::ext_proc::v3::ProcessingMode>
@@ -240,6 +241,7 @@ class FilterConfigPerRoute : public Router::RouteSpecificFilterConfig {
   const absl::optional<const envoy::extensions::filters::http::ext_proc::v3::ProcessingMode>
       processing_mode_;
   const absl::optional<const envoy::config::core::v3::GrpcService> grpc_service_;
+  std::vector<envoy::config::core::v3::HeaderValue> metadata_;
 };
 
 class Filter : public Logger::Loggable<Logger::Id::ext_proc>,
@@ -266,6 +268,9 @@ class Filter : public Logger::Loggable<Logger::Id::ext_proc>,
         encoding_state_(*this, config->processingMode()) {}
 
   const FilterConfig& config() const { return *config_; }
+  const envoy::config::core::v3::GrpcService& grpc_service_config() const {
+    return config_with_hash_key_.config();
+  }
 
   ExtProcFilterStats& stats() { return stats_; }
   ExtProcLoggingInfo* loggingInfo() { return logging_info_; }

test/extensions/filters/http/ext_proc/config_test.cc

@@ -80,6 +80,23 @@ TEST(HttpExtProcConfigTest, CorrectConfigServerContext) {
   cb(filter_callback);
 }
 
+TEST(HttpExtProcConfigTest, CorrectRouteMetadataOnlyConfig) {
+  std::string yaml = R"EOF(
+  overrides:
+    metadata:
+      - key: "a"
+        value: "a"
+  )EOF";
+
+  ExternalProcessingFilterConfig factory;
+  ProtobufTypes::MessagePtr proto_config = factory.createEmptyRouteConfigProto();
+  TestUtility::loadFromYaml(yaml, *proto_config);
+
+  testing::NiceMock<Server::Configuration::MockServerFactoryContext> context;
+  Router::RouteSpecificFilterConfigConstSharedPtr cb = factory.createRouteSpecificFilterConfig(
+      *proto_config, context, context.messageValidationVisitor());
+}
+
 } // namespace
 } // namespace ExternalProcessing
 } // namespace HttpFilters

test/extensions/filters/http/ext_proc/ext_proc_integration_test.cc

@@ -38,6 +38,7 @@ using envoy::service::ext_proc::v3::ProcessingResponse;
 using envoy::service::ext_proc::v3::TrailersResponse;
 using Extensions::HttpFilters::ExternalProcessing::HasNoHeader;
 using Extensions::HttpFilters::ExternalProcessing::HeaderProtosEqual;
+using Extensions::HttpFilters::ExternalProcessing::makeHeaderValue;
 using Extensions::HttpFilters::ExternalProcessing::SingleHeaderValueIs;
 
 using Http::LowerCaseString;
@@ -2503,6 +2504,35 @@ TEST_P(ExtProcIntegrationTest, PerRouteGrpcService) {
   EXPECT_THAT(response->headers(), SingleHeaderValueIs("x-response-processed", "1"));
 }
 
+// Set up per-route configuration that extends original metadata.
+TEST_P(ExtProcIntegrationTest, PerRouteMetadata) {
+  initializeConfig();
+
+  // Override metadata from route config.
+  config_helper_.addConfigModifier([this](HttpConnectionManager& cm) {
+    // Set up "/foo" so that it will use a different GrpcService
+    auto* vh = cm.mutable_route_config()->mutable_virtual_hosts()->Mutable(0);
+    auto* route = vh->mutable_routes()->Mutable(0);
+    route->mutable_match()->set_path("/foo");
+    ExtProcPerRoute per_route;
+    *per_route.mutable_overrides()->mutable_metadata()->Add() = makeHeaderValue("b", "c");
+    *per_route.mutable_overrides()->mutable_metadata()->Add() = makeHeaderValue("c", "c");
+    setPerRouteConfig(route, per_route);
+  });
+
+  HttpIntegrationTest::initialize();
+
+  // Request that matches route directed to ext_proc_server_0
+  auto response =
+      sendDownstreamRequest([](Http::RequestHeaderMap& headers) { headers.setPath("/foo"); });
+
+  processRequestHeadersMessage(*grpc_upstreams_[0], true, absl::nullopt);
+  handleUpstreamRequest();
+
+  processResponseHeadersMessage(*grpc_upstreams_[0], false, absl::nullopt);
+  verifyDownstreamResponse(*response, 200);
+}
+
 // Sending new timeout API in both downstream request and upstream response
 // handling path with header mutation.
 TEST_P(ExtProcIntegrationTest, RequestAndResponseMessageNewTimeoutWithHeaderMutation) {

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

@@ -2979,6 +2979,23 @@ TEST(OverrideTest, GrpcServiceNonOverride) {
   EXPECT_THAT(*merged_route.grpcService(), ProtoEq(cfg1.overrides().grpc_service()));
 }
 
+// When merging two configurations, second metadata override only extends the first's one.
+TEST(OverrideTest, MetadataOverride) {
+  ExtProcPerRoute cfg1;
+  cfg1.mutable_overrides()->mutable_metadata()->Add()->CopyFrom(makeHeaderValue("a", "a"));
+  cfg1.mutable_overrides()->mutable_metadata()->Add()->CopyFrom(makeHeaderValue("b", "b"));
+  ExtProcPerRoute cfg2;
+  cfg2.mutable_overrides()->mutable_metadata()->Add()->CopyFrom(makeHeaderValue("b", "c"));
+  cfg2.mutable_overrides()->mutable_metadata()->Add()->CopyFrom(makeHeaderValue("c", "c"));
+  FilterConfigPerRoute route1(cfg1);
+  FilterConfigPerRoute route2(cfg2);
+  FilterConfigPerRoute merged_route(route1, route2);
+  ASSERT_TRUE(merged_route.metadata().size() == 3);
+  EXPECT_THAT(merged_route.metadata()[0], ProtoEq(cfg1.overrides().metadata()[0]));
+  EXPECT_THAT(merged_route.metadata()[1], ProtoEq(cfg2.overrides().metadata()[0]));
+  EXPECT_THAT(merged_route.metadata()[2], ProtoEq(cfg2.overrides().metadata()[1]));
+}
+
 // Verify that attempts to change headers that are not allowed to be changed
 // are ignored and a counter is incremented.
 TEST_F(HttpFilterTest, IgnoreInvalidHeaderMutations) {
@@ -3302,6 +3319,61 @@ TEST_F(HttpFilter2Test, LastEncodeDataCallExceedsStreamBufferLimitWouldJustRaise
   conn_manager_->onData(fake_input, false);
 }
 
+// Test that per route metadata override does override inherited grpc_service configuration.
+TEST_F(HttpFilterTest, GrpcServiceMetadataOverride) {
+  initialize(R"EOF(
+  grpc_service:
+    envoy_grpc:
+      cluster_name: "ext_proc_server"
+    initial_metadata:
+      - key: "a"
+        value: "a"
+      - key: "b"
+        value: "b"
+  )EOF");
+
+  // Route configuration overrides the grpc_service metadata.
+  ExtProcPerRoute route_proto;
+  *route_proto.mutable_overrides()->mutable_metadata()->Add() = makeHeaderValue("b", "c");
+  *route_proto.mutable_overrides()->mutable_metadata()->Add() = makeHeaderValue("c", "c");
+  FilterConfigPerRoute route_config(route_proto);
+  EXPECT_CALL(decoder_callbacks_, traversePerFilterConfig(_))
+      .WillOnce(
+          testing::Invoke([&](std::function<void(const Router::RouteSpecificFilterConfig&)> cb) {
+            cb(route_config);
+          }));
+
+  // Build expected merged grpc_service configuration.
+  {
+    std::string expected_config = (R"EOF(
+      grpc_service:
+        envoy_grpc:
+          cluster_name: "ext_proc_server"
+        initial_metadata:
+          - key: "a"
+            value: "a"
+          - key: "b"
+            value: "c"
+          - key: "c"
+            value: "c"
+    )EOF");
+    envoy::extensions::filters::http::ext_proc::v3::ExternalProcessor expected_proto{};
+    TestUtility::loadFromYaml(expected_config, expected_proto);
+    final_expected_grpc_service_.emplace(expected_proto.grpc_service());
+    config_with_hash_key_.setConfig(expected_proto.grpc_service());
+  }
+
+  EXPECT_EQ(FilterHeadersStatus::StopIteration, filter_->decodeHeaders(request_headers_, true));
+  processRequestHeaders(false, absl::nullopt);
+
+  const auto& meta = filter_->grpc_service_config().initial_metadata();
+  EXPECT_EQ(meta[0].value(), "a"); // a = a inherited
+  EXPECT_EQ(meta[1].value(), "c"); // b = c overridden
+  EXPECT_EQ(meta[2].value(), "c"); // c = c added
+
+  filter_->onDestroy();
+}
+
 } // namespace
 } // namespace ExternalProcessing
 } // namespace HttpFilters

test/extensions/filters/http/ext_proc/utils.cc

@@ -29,6 +29,14 @@ bool ExtProcTestUtility::headerProtosEqualIgnoreOrder(
   return TestUtility::headerMapEqualIgnoreOrder(expected, actual_headers);
 }
 
+envoy::config::core::v3::HeaderValue makeHeaderValue(const std::string& key,
+                                                     const std::string& value) {
+  envoy::config::core::v3::HeaderValue v;
+  v.set_key(key);
+  v.set_value(value);
+  return v;
+}
+
 } // namespace ExternalProcessing
 } // namespace HttpFilters
 } // namespace Extensions

test/extensions/filters/http/ext_proc/utils.h

@@ -50,6 +50,8 @@ MATCHER_P2(SingleProtoHeaderValueIs, key, value,
   return false;
 }
 
+envoy::config::core::v3::HeaderValue makeHeaderValue(const std::string& key,
+                                                     const std::string& value);
 } // namespace ExternalProcessing
 } // namespace HttpFilters
 } // namespace Extensions