transcoding: Teach transcoding filter to be aware of per vhost / per route settings

api/envoy/extensions/filters/http/grpc_json_transcoder/v3/transcoder.proto

@@ -16,6 +16,10 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE;
 // [#extension: envoy.filters.http.grpc_json_transcoder]
 
 // [#next-free-field: 10]
+// GrpcJsonTranscoder filter configuration.
+// The filter itself can be used per route / per virtual host or on the general level. The most
+// specific one is being used for a given route. If the list of services is empty - filter
+// is considered to be disabled.
 message GrpcJsonTranscoder {
   option (udpa.annotations.versioning).previous_message_type =
       "envoy.config.filter.http.transcoder.v2.GrpcJsonTranscoder";
@@ -64,7 +68,8 @@ message GrpcJsonTranscoder {
   // the transcoder will translate. If the service name doesn't exist in ``proto_descriptor``,
   // Envoy will fail at startup. The ``proto_descriptor`` may contain more services than
   // the service names specified here, but they won't be translated.
-  repeated string services = 2 [(validate.rules).repeated = {min_items: 1}];
+  // If the list of services is empty, filter is considered disabled.
+  repeated string services = 2;
 
   // Control options for response JSON. These options are passed directly to
   // `JsonPrintOptions <https://developers.google.com/protocol-buffers/docs/reference/cpp/

source/extensions/filters/http/grpc_json_transcoder/BUILD

@@ -30,6 +30,7 @@ envoy_cc_library(
         "//source/common/grpc:common_lib",
         "//source/common/http:headers_lib",
         "//source/common/protobuf",
+        "//source/extensions/filters/http:well_known_names",
         "@envoy_api//envoy/extensions/filters/http/grpc_json_transcoder/v3:pkg_cc_proto",
     ],
 )

source/extensions/filters/http/grpc_json_transcoder/config.cc

@@ -23,6 +23,15 @@ Http::FilterFactoryCb GrpcJsonTranscoderFilterConfig::createFilterFactoryFromPro
   };
 }
 
+Router::RouteSpecificFilterConfigConstSharedPtr
+GrpcJsonTranscoderFilterConfig::createRouteSpecificFilterConfigTyped(
+    const envoy::extensions::filters::http::grpc_json_transcoder::v3::GrpcJsonTranscoder&
+        proto_config,
+    Server::Configuration::ServerFactoryContext& context, ProtobufMessage::ValidationVisitor&) {
+
+  return std::make_shared<JsonTranscoderConfig>(proto_config, context.api());
+}
+
 /**
  * Static registration for the grpc transcoding filter. @see RegisterNamedHttpFilterConfigFactory.
  */

source/extensions/filters/http/grpc_json_transcoder/config.h

@@ -25,6 +25,11 @@ class GrpcJsonTranscoderFilterConfig
       const envoy::extensions::filters::http::grpc_json_transcoder::v3::GrpcJsonTranscoder&
           proto_config,
       const std::string& stats_prefix, Server::Configuration::FactoryContext& context) override;
+
+  Router::RouteSpecificFilterConfigConstSharedPtr createRouteSpecificFilterConfigTyped(
+      const envoy::extensions::filters::http::grpc_json_transcoder::v3::GrpcJsonTranscoder&,
+      Server::Configuration::ServerFactoryContext& context,
+      ProtobufMessage::ValidationVisitor& validator) override;
 };
 
 } // namespace GrpcJsonTranscoder

source/extensions/filters/http/grpc_json_transcoder/json_transcoder_filter.cc

@@ -17,6 +17,7 @@
 #include "common/protobuf/utility.h"
 
 #include "extensions/filters/http/grpc_json_transcoder/http_body_utils.h"
+#include "extensions/filters/http/well_known_names.h"
 
 #include "google/api/annotations.pb.h"
 #include "google/api/http.pb.h"
@@ -106,6 +107,12 @@ JsonTranscoderConfig::JsonTranscoderConfig(
     const envoy::extensions::filters::http::grpc_json_transcoder::v3::GrpcJsonTranscoder&
         proto_config,
     Api::Api& api) {
+
+  disabled_ = (proto_config.services().size() == 0);
+  if (disabled_) {
+    return;
+  }
+
   FileDescriptorSet descriptor_set;
 
   switch (proto_config.descriptor_set_case()) {
@@ -258,7 +265,13 @@ bool JsonTranscoderConfig::convertGrpcStatus() const { return convert_grpc_statu
 ProtobufUtil::Status JsonTranscoderConfig::createTranscoder(
     const Http::RequestHeaderMap& headers, ZeroCopyInputStream& request_input,
     google::grpc::transcoding::TranscoderInputStream& response_input,
-    std::unique_ptr<Transcoder>& transcoder, MethodInfoSharedPtr& method_info) {
+    std::unique_ptr<Transcoder>& transcoder, MethodInfoSharedPtr& method_info) const {
+
+  if (disabled_) {
+    return ProtobufUtil::Status(Code::INVALID_ARGUMENT,
+                                "JsonTranscoderFilter for this route is disabled");
+  }
+
   if (Grpc::Common::isGrpcRequestHeaders(headers)) {
     return ProtobufUtil::Status(Code::INVALID_ARGUMENT,
                                 "Request headers has application/grpc content-type");
@@ -328,7 +341,7 @@ ProtobufUtil::Status JsonTranscoderConfig::createTranscoder(
 
 ProtobufUtil::Status
 JsonTranscoderConfig::methodToRequestInfo(const MethodInfoSharedPtr& method_info,
-                                          google::grpc::transcoding::RequestInfo* info) {
+                                          google::grpc::transcoding::RequestInfo* info) const {
   const std::string& request_type_full_name = method_info->descriptor_->input_type()->full_name();
   auto request_type_url = Grpc::Common::typeUrl(request_type_full_name);
   info->message_type = type_helper_->Info()->GetTypeByTypeUrl(request_type_url);
@@ -343,18 +356,33 @@ JsonTranscoderConfig::methodToRequestInfo(const MethodInfoSharedPtr& method_info
 
 ProtobufUtil::Status
 JsonTranscoderConfig::translateProtoMessageToJson(const Protobuf::Message& message,
-                                                  std::string* json_out) {
+                                                  std::string* json_out) const {
   return ProtobufUtil::BinaryToJsonString(
       type_helper_->Resolver(), Grpc::Common::typeUrl(message.GetDescriptor()->full_name()),
       message.SerializeAsString(), json_out, print_options_);
 }
 
 JsonTranscoderFilter::JsonTranscoderFilter(JsonTranscoderConfig& config) : config_(config) {}
 
+void JsonTranscoderFilter::initPerRouteConfig() {
+  if (!decoder_callbacks_->route() || !decoder_callbacks_->route()->routeEntry()) {
+    per_route_config_ = &config_;
+    return;
+  }
+
+  const std::string& name = HttpFilterNames::get().GrpcJsonTranscoder;
+  const auto* entry = decoder_callbacks_->route()->routeEntry();
+  const auto* route_local = entry->mostSpecificPerFilterConfigTyped<JsonTranscoderConfig>(name);
+
+  per_route_config_ = route_local ? route_local : &config_;
+}
+
 Http::FilterHeadersStatus JsonTranscoderFilter::decodeHeaders(Http::RequestHeaderMap& headers,
                                                               bool end_stream) {
+
+  initPerRouteConfig();
   const auto status =
-      config_.createTranscoder(headers, request_in_, response_in_, transcoder_, method_);
+      per_route_config_->createTranscoder(headers, request_in_, response_in_, transcoder_, method_);
 
   if (!status.ok()) {
     // If transcoder couldn't be created, it might be a normal gRPC request, so the filter will
@@ -392,7 +420,7 @@ Http::FilterHeadersStatus JsonTranscoderFilter::decodeHeaders(Http::RequestHeade
   headers.setReferenceMethod(Http::Headers::get().MethodValues.Post);
   headers.setReferenceTE(Http::Headers::get().TEValues.Trailers);
 
-  if (!config_.matchIncomingRequestInfo()) {
+  if (!per_route_config_->matchIncomingRequestInfo()) {
     decoder_callbacks_->clearRouteCache();
   }
 
@@ -554,7 +582,7 @@ JsonTranscoderFilter::encodeTrailers(Http::ResponseTrailerMap& trailers) {
 }
 
 void JsonTranscoderFilter::doTrailers(Http::ResponseHeaderOrTrailerMap& headers_or_trailers) {
-  if (error_ || !transcoder_) {
+  if (error_ || !transcoder_ || per_route_config_->disabled()) {
     return;
   }
 
@@ -705,7 +733,8 @@ bool JsonTranscoderFilter::buildResponseFromHttpBodyOutput(
 
 bool JsonTranscoderFilter::maybeConvertGrpcStatus(Grpc::Status::GrpcStatus grpc_status,
                                                   Http::ResponseHeaderOrTrailerMap& trailers) {
-  if (!config_.convertGrpcStatus()) {
+  ASSERT(!per_route_config_->disabled());
+  if (!per_route_config_->convertGrpcStatus()) {
     return false;
   }
 
@@ -734,7 +763,8 @@ bool JsonTranscoderFilter::maybeConvertGrpcStatus(Grpc::Status::GrpcStatus grpc_
   }
 
   std::string json_status;
-  auto translate_status = config_.translateProtoMessageToJson(*status_details, &json_status);
+  auto translate_status =
+      per_route_config_->translateProtoMessageToJson(*status_details, &json_status);
   if (!translate_status.ok()) {
     ENVOY_LOG(debug, "Transcoding status error {}", translate_status.ToString());
     return false;

source/extensions/filters/http/grpc_json_transcoder/json_transcoder_filter.h

@@ -56,7 +56,9 @@ void createHttpBodyEnvelope(Buffer::Instance& output,
 /**
  * Global configuration for the gRPC JSON transcoder filter. Factory for the Transcoder interface.
  */
-class JsonTranscoderConfig : public Logger::Loggable<Logger::Id::config> {
+class JsonTranscoderConfig : public Logger::Loggable<Logger::Id::config>,
+                             public Router::RouteSpecificFilterConfig {
+
 public:
   /**
    * constructor that loads protobuf descriptors from the file specified in the JSON config.
@@ -81,13 +83,13 @@ class JsonTranscoderConfig : public Logger::Loggable<Logger::Id::config> {
                    Protobuf::io::ZeroCopyInputStream& request_input,
                    google::grpc::transcoding::TranscoderInputStream& response_input,
                    std::unique_ptr<google::grpc::transcoding::Transcoder>& transcoder,
-                   MethodInfoSharedPtr& method_info);
+                   MethodInfoSharedPtr& method_info) const;
 
   /**
    * Converts an arbitrary protobuf message to JSON.
    */
   ProtobufUtil::Status translateProtoMessageToJson(const Protobuf::Message& message,
-                                                   std::string* json_out);
+                                                   std::string* json_out) const;
 
   /**
    * If true, skip clearing the route cache after the incoming request has been modified.
@@ -102,12 +104,19 @@ class JsonTranscoderConfig : public Logger::Loggable<Logger::Id::config> {
    */
   bool convertGrpcStatus() const;
 
+  bool disabled() const { return disabled_; }
+
 private:
   /**
    * Convert method descriptor to RequestInfo that needed for transcoding library
    */
   ProtobufUtil::Status methodToRequestInfo(const MethodInfoSharedPtr& method_info,
-                                           google::grpc::transcoding::RequestInfo* info);
+                                           google::grpc::transcoding::RequestInfo* info) const;
+
+  JsonTranscoderConfig(
+      const envoy::extensions::filters::http::grpc_json_transcoder::v3::GrpcJsonTranscoder&
+          proto_config,
+      Api::Api& api, bool disabled);
 
 private:
   void addFileDescriptor(const Protobuf::FileDescriptorProto& file);
@@ -124,6 +133,8 @@ class JsonTranscoderConfig : public Logger::Loggable<Logger::Id::config> {
   bool match_incoming_request_route_{false};
   bool ignore_unknown_query_parameters_{false};
   bool convert_grpc_status_{false};
+
+  bool disabled_;
 };
 
 using JsonTranscoderConfigSharedPtr = std::shared_ptr<JsonTranscoderConfig>;
@@ -172,15 +183,17 @@ class JsonTranscoderFilter : public Http::StreamFilter, public Logger::Loggable<
                               Http::ResponseHeaderOrTrailerMap& trailers);
   bool hasHttpBodyAsOutputType();
   void doTrailers(Http::ResponseHeaderOrTrailerMap& headers_or_trailers);
+  void initPerRouteConfig();
 
   JsonTranscoderConfig& config_;
+  const JsonTranscoderConfig* per_route_config_{};
   std::unique_ptr<google::grpc::transcoding::Transcoder> transcoder_;
   TranscoderInputStreamImpl request_in_;
   TranscoderInputStreamImpl response_in_;
-  Http::StreamDecoderFilterCallbacks* decoder_callbacks_{nullptr};
-  Http::StreamEncoderFilterCallbacks* encoder_callbacks_{nullptr};
+  Http::StreamDecoderFilterCallbacks* decoder_callbacks_{};
+  Http::StreamEncoderFilterCallbacks* encoder_callbacks_{};
   MethodInfoSharedPtr method_;
-  Http::ResponseHeaderMap* response_headers_{nullptr};
+  Http::ResponseHeaderMap* response_headers_{};
   Grpc::Decoder decoder_;
 
   // Data of the initial request message, initialized from query arguments, path, etc.

test/extensions/filters/http/grpc_json_transcoder/BUILD

@@ -69,6 +69,7 @@ envoy_extension_cc_test(
         "//test/integration:http_integration_lib",
         "//test/proto:bookstore_proto_cc_proto",
         "//test/test_common:utility_lib",
+        "@envoy_api//envoy/extensions/filters/http/grpc_json_transcoder/v3:pkg_cc_proto",
     ],
 )
 

test/extensions/filters/http/grpc_json_transcoder/grpc_json_transcoder_integration_test.cc

@@ -1,8 +1,12 @@
+#include "envoy/extensions/filters/http/grpc_json_transcoder/v3/transcoder.pb.h"
+
 #include "common/grpc/codec.h"
 #include "common/grpc/common.h"
 #include "common/http/message_impl.h"
 #include "common/protobuf/protobuf.h"
 
+#include "extensions/filters/http/well_known_names.h"
+
 #include "test/integration/http_integration.h"
 #include "test/mocks/http/mocks.h"
 #include "test/proto/bookstore.pb.h"
@@ -165,8 +169,28 @@ class GrpcJsonTranscoderIntegrationTest
     ASSERT_TRUE(fake_upstream_connection_->close());
     ASSERT_TRUE(fake_upstream_connection_->waitForDisconnect());
   }
-};
 
+  // override configuration on per-route basis
+  void overrideConfig(const std::string& json_config) {
+
+    envoy::extensions::filters::http::grpc_json_transcoder::v3::GrpcJsonTranscoder per_route_config;
+    TestUtility::loadFromJson(json_config, per_route_config);
+    ConfigHelper::HttpModifierFunction modifier =
+        [per_route_config](
+            envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager&
+                cfg) {
+          auto* config = cfg.mutable_route_config()
+                             ->mutable_virtual_hosts()
+                             ->Mutable(0)
+                             ->mutable_typed_per_filter_config();
+
+          (*config)[Extensions::HttpFilters::HttpFilterNames::get().GrpcJsonTranscoder].PackFrom(
+              per_route_config);
+        };
+
+    config_helper_.addConfigModifier(modifier);
+  }
+};
 INSTANTIATE_TEST_SUITE_P(IpVersions, GrpcJsonTranscoderIntegrationTest,
                          testing::ValuesIn(TestEnvironment::getIpVersionsForTest()),
                          TestUtility::ipTestParamsToString);
@@ -848,5 +872,68 @@ TEST_P(GrpcJsonTranscoderIntegrationTest, UTF8) {
       false);
 }
 
+TEST_P(GrpcJsonTranscoderIntegrationTest, RouteDisabled) {
+  overrideConfig(R"EOF({"services": [], "proto_descriptor_bin": ""})EOF");
+  HttpIntegrationTest::initialize();
+
+  codec_client_ = makeHttpConnection(lookupPort("http"));
+  auto response = codec_client_->makeHeaderOnlyRequest(Http::TestRequestHeaderMapImpl{
+      {":method", "GET"}, {":path", "/shelves"}, {":authority", "host"}});
+  waitForNextUpstreamRequest();
+  upstream_request_->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "200"}}, true);
+  response->waitForEndStream();
+  ASSERT_TRUE(response->complete());
+  EXPECT_EQ("200", response->headers().Status()->value().getStringView());
+};
+
+class OverrideConfigGrpcJsonTranscoderIntegrationTest : public GrpcJsonTranscoderIntegrationTest {
+public:
+  /**
+   * Global initializer for all integration tests.
+   */
+  void SetUp() override {
+    setUpstreamProtocol(FakeHttpConnection::Type::HTTP2);
+    // creates filter but doesn't apply it to bookstore services
+    const std::string filter =
+        R"EOF(
+            name: grpc_json_transcoder
+            typed_config:
+              "@type": type.googleapis.com/envoy.config.filter.http.transcoder.v2.GrpcJsonTranscoder
+              "proto_descriptor": ""
+            )EOF";
+    config_helper_.addFilter(filter);
+  }
+};
+INSTANTIATE_TEST_SUITE_P(IpVersions, OverrideConfigGrpcJsonTranscoderIntegrationTest,
+                         testing::ValuesIn(TestEnvironment::getIpVersionsForTest()),
+                         TestUtility::ipTestParamsToString);
+
+TEST_P(OverrideConfigGrpcJsonTranscoderIntegrationTest, RouteOverride) {
+  // add bookstore per-route override
+  const std::string filter =
+      R"EOF({{
+              "services": ["bookstore.Bookstore"],
+              "proto_descriptor": "{}"
+          }})EOF";
+  overrideConfig(
+      fmt::format(filter, TestEnvironment::runfilesPath("test/proto/bookstore.descriptor")));
+
+  HttpIntegrationTest::initialize();
+
+  // testing the path that's defined in bookstore.descriptor file (should work  the same way
+  // as it does when grpc filter is applied to base config)
+  testTranscoding<Empty, bookstore::ListShelvesResponse>(
+      Http::TestRequestHeaderMapImpl{
+          {":method", "GET"}, {":path", "/shelves"}, {":authority", "host"}},
+      "", {""}, {R"(shelves { id: 20 theme: "Children" }
+          shelves { id: 1 theme: "Foo" } )"},
+      Status(),
+      Http::TestResponseHeaderMapImpl{{":status", "200"},
+                                      {"content-type", "application/json"},
+                                      {"content-length", "69"},
+                                      {"grpc-status", "0"}},
+      R"({"shelves":[{"id":"20","theme":"Children"},{"id":"1","theme":"Foo"}]})");
+};
+
 } // namespace
 } // namespace Envoy