diff --git a/api/envoy/config/filter/http/transcoder/v2/transcoder.proto b/api/envoy/config/filter/http/transcoder/v2/transcoder.proto index 14f54124508d..9583af300ff8 100644 --- a/api/envoy/config/filter/http/transcoder/v2/transcoder.proto +++ b/api/envoy/config/filter/http/transcoder/v2/transcoder.proto @@ -120,4 +120,34 @@ message GrpcJsonTranscoder { // not know them beforehand. Otherwise use ``ignored_query_parameters``. // Defaults to false. bool ignore_unknown_query_parameters = 8; + + // Whether to convert gRPC status headers to JSON. + // When trailer indicates a gRPC error and there was no HTTP body, take ``google.rpc.Status`` + // from the ``grpc-status-details-bin`` header and use it as JSON body. + // If there was no such header, make ``google.rpc.Status`` out of the ``grpc-status`` and + // ``grpc-message`` headers. + // The error details types must be present in the ``proto_descriptor``. + // + // For example, if an upstream server replies with headers: + // + // .. code-block:: none + // + // grpc-status: 5 + // grpc-status-details-bin: + // CAUaMwoqdHlwZS5nb29nbGVhcGlzLmNvbS9nb29nbGUucnBjLlJlcXVlc3RJbmZvEgUKA3ItMQ + // + // The ``grpc-status-details-bin`` header contains a base64-encoded protobuf message + // ``google.rpc.Status``. It will be transcoded into: + // + // .. code-block:: none + // + // HTTP/1.1 404 Not Found + // content-type: application/json + // + // {"code":5,"details":[{"@type":"type.googleapis.com/google.rpc.RequestInfo","requestId":"r-1"}]} + // + // In order to transcode the message, the ``google.rpc.RequestInfo`` type from + // the ``google/rpc/error_details.proto`` should be included in the configured + // :ref:`proto descriptor set `. + bool convert_grpc_status = 9; } diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index 2a049c36c740..9540a73b11bb 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -23,6 +23,7 @@ Version history * fault: added overrides for default runtime keys in :ref:`HTTPFault ` filter. * grpc: added :ref:`AWS IAM grpc credentials extension ` for AWS-managed xDS. * grpc-json: added support for :ref:`ignoring unknown query parameters`. +* grpc-json: added support for :ref:`the grpc-status-details-bin header`. * header to metadata: added :ref:`PROTOBUF_VALUE ` and :ref:`ValueEncode ` to support protobuf Value and Base64 encoding. * http: added the ability to reject HTTP/1.1 requests with invalid HTTP header values, using the runtime feature `envoy.reloadable_features.strict_header_validation`. * http: added the ability to :ref:`merge adjacent slashes` in the path. diff --git a/source/common/grpc/BUILD b/source/common/grpc/BUILD index 603474289767..0993761f3537 100644 --- a/source/common/grpc/BUILD +++ b/source/common/grpc/BUILD @@ -86,6 +86,7 @@ envoy_cc_library( "//source/common/buffer:buffer_lib", "//source/common/buffer:zero_copy_input_stream_lib", "//source/common/common:assert_lib", + "//source/common/common:base64_lib", "//source/common/common:empty_string", "//source/common/common:enum_to_int", "//source/common/common:hash_lib", diff --git a/source/common/grpc/common.cc b/source/common/grpc/common.cc index 2683044b715c..4299fddd6f3e 100644 --- a/source/common/grpc/common.cc +++ b/source/common/grpc/common.cc @@ -10,6 +10,7 @@ #include "common/buffer/buffer_impl.h" #include "common/buffer/zero_copy_input_stream_impl.h" #include "common/common/assert.h" +#include "common/common/base64.h" #include "common/common/empty_string.h" #include "common/common/enum_to_int.h" #include "common/common/fmt.h" @@ -54,7 +55,7 @@ absl::optional Common::getGrpcStatus(const Http::HeaderMap& uint64_t grpc_status_code; if (!grpc_status_header || grpc_status_header->value().empty()) { - return {}; + return absl::nullopt; } if (!absl::SimpleAtoi(grpc_status_header->value().getStringView(), &grpc_status_code) || grpc_status_code > Status::GrpcStatus::MaximumValid) { @@ -68,6 +69,27 @@ std::string Common::getGrpcMessage(const Http::HeaderMap& trailers) { return entry ? std::string(entry->value().getStringView()) : EMPTY_STRING; } +absl::optional +Common::getGrpcStatusDetailsBin(const Http::HeaderMap& trailers) { + const Http::HeaderEntry* details_header = trailers.get(Http::Headers::get().GrpcStatusDetailsBin); + if (!details_header) { + return absl::nullopt; + } + + // Some implementations use non-padded base64 encoding for grpc-status-details-bin. + auto decoded_value = Base64::decodeWithoutPadding(details_header->value().getStringView()); + if (decoded_value.empty()) { + return absl::nullopt; + } + + google::rpc::Status status; + if (!status.ParseFromString(decoded_value)) { + return absl::nullopt; + } + + return {std::move(status)}; +} + Buffer::InstancePtr Common::serializeToGrpcFrame(const Protobuf::Message& message) { // http://www.grpc.io/docs/guides/wire.html // Reserve enough space for the entire message and the 5 byte header. diff --git a/source/common/grpc/common.h b/source/common/grpc/common.h index fa5fe72f7bc7..32f4fd02ee36 100644 --- a/source/common/grpc/common.h +++ b/source/common/grpc/common.h @@ -57,6 +57,15 @@ class Common { */ static std::string getGrpcMessage(const Http::HeaderMap& trailers); + /** + * Returns the decoded google.rpc.Status message from a given set of trailers, if present. + * @param trailers the trailers to parse. + * @return std::unique_ptr the gRPC status message or empty pointer if no + * grpc-status-details-bin trailer found or it was invalid. + */ + static absl::optional + getGrpcStatusDetailsBin(const Http::HeaderMap& trailers); + /** * Parse gRPC header 'grpc-timeout' value to a duration in milliseconds. * @param request_headers the header map from which to extract the value of 'grpc-timeout' header. diff --git a/source/common/http/headers.h b/source/common/http/headers.h index b711fb4b142a..718f19be595b 100644 --- a/source/common/http/headers.h +++ b/source/common/http/headers.h @@ -123,6 +123,7 @@ class HeaderValues { const LowerCaseString GrpcStatus{"grpc-status"}; const LowerCaseString GrpcTimeout{"grpc-timeout"}; const LowerCaseString GrpcAcceptEncoding{"grpc-accept-encoding"}; + const LowerCaseString GrpcStatusDetailsBin{"grpc-status-details-bin"}; const LowerCaseString Host{":authority"}; const LowerCaseString HostLegacy{"host"}; const LowerCaseString KeepAlive{"keep-alive"}; diff --git a/source/common/protobuf/protobuf.h b/source/common/protobuf/protobuf.h index ff6da52694f3..620f3b1c0b4d 100644 --- a/source/common/protobuf/protobuf.h +++ b/source/common/protobuf/protobuf.h @@ -7,6 +7,7 @@ #include "google/protobuf/any.pb.h" #include "google/protobuf/descriptor.h" #include "google/protobuf/descriptor.pb.h" +#include "google/protobuf/descriptor_database.h" #include "google/protobuf/empty.pb.h" #include "google/protobuf/io/coded_stream.h" #include "google/protobuf/io/zero_copy_stream.h" diff --git a/source/extensions/filters/http/grpc_json_transcoder/json_transcoder_filter.cc b/source/extensions/filters/http/grpc_json_transcoder/json_transcoder_filter.cc index fb26f1f39c0d..6bf671133676 100644 --- a/source/extensions/filters/http/grpc_json_transcoder/json_transcoder_filter.cc +++ b/source/extensions/filters/http/grpc_json_transcoder/json_transcoder_filter.cc @@ -111,9 +111,13 @@ JsonTranscoderConfig::JsonTranscoderConfig( } for (const auto& file : descriptor_set.file()) { - if (descriptor_pool_.BuildFile(file) == nullptr) { - throw EnvoyException("transcoding_filter: Unable to build proto descriptor pool"); - } + addFileDescriptor(file); + } + + convert_grpc_status_ = proto_config.convert_grpc_status(); + if (convert_grpc_status_) { + addBuiltinSymbolDescriptor("google.protobuf.Any"); + addBuiltinSymbolDescriptor("google.rpc.Status"); } PathMatcherBuilder pmb; @@ -164,10 +168,34 @@ JsonTranscoderConfig::JsonTranscoderConfig( ignore_unknown_query_parameters_ = proto_config.ignore_unknown_query_parameters(); } +void JsonTranscoderConfig::addFileDescriptor(const Protobuf::FileDescriptorProto& file) { + if (descriptor_pool_.BuildFile(file) == nullptr) { + throw EnvoyException("transcoding_filter: Unable to build proto descriptor pool"); + } +} + +void JsonTranscoderConfig::addBuiltinSymbolDescriptor(const std::string& symbol_name) { + if (descriptor_pool_.FindFileContainingSymbol(symbol_name) != nullptr) { + return; + } + + auto* builtin_pool = Protobuf::DescriptorPool::generated_pool(); + if (!builtin_pool) { + return; + } + + Protobuf::DescriptorPoolDatabase pool_database(*builtin_pool); + Protobuf::FileDescriptorProto file_proto; + pool_database.FindFileContainingSymbol(symbol_name, &file_proto); + addFileDescriptor(file_proto); +} + bool JsonTranscoderConfig::matchIncomingRequestInfo() const { return match_incoming_request_route_; } +bool JsonTranscoderConfig::convertGrpcStatus() const { return convert_grpc_status_; } + ProtobufUtil::Status JsonTranscoderConfig::createTranscoder( const Http::HeaderMap& headers, ZeroCopyInputStream& request_input, google::grpc::transcoding::TranscoderInputStream& response_input, @@ -244,6 +272,14 @@ JsonTranscoderConfig::methodToRequestInfo(const Protobuf::MethodDescriptor* meth return ProtobufUtil::Status(); } +ProtobufUtil::Status +JsonTranscoderConfig::translateProtoMessageToJson(const Protobuf::Message& message, + std::string* json_out) { + 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) {} Http::FilterHeadersStatus JsonTranscoderFilter::decodeHeaders(Http::HeaderMap& headers, @@ -385,6 +421,8 @@ Http::FilterDataStatus JsonTranscoderFilter::encodeData(Buffer::Instance& data, return Http::FilterDataStatus::Continue; } + has_body_ = true; + // TODO(dio): Add support for streaming case. if (has_http_body_output_) { buildResponseFromHttpBodyOutput(*response_headers_, data); @@ -420,6 +458,7 @@ Http::FilterTrailersStatus JsonTranscoderFilter::encodeTrailers(Http::HeaderMap& if (data.length()) { encoder_callbacks_->addEncodedData(data, true); + has_body_ = true; } if (method_->server_streaming()) { @@ -427,18 +466,34 @@ Http::FilterTrailersStatus JsonTranscoderFilter::encodeTrailers(Http::HeaderMap& return Http::FilterTrailersStatus::Continue; } + // If there was no previous headers frame, this |trailers| map is our |response_headers_|, + // so there is no need to copy headers from one to the other. + bool is_trailers_only_response = response_headers_ == &trailers; + const absl::optional grpc_status = Grpc::Common::getGrpcStatus(trailers); + bool status_converted_to_json = grpc_status && maybeConvertGrpcStatus(*grpc_status, trailers); + if (!grpc_status || grpc_status.value() == Grpc::Status::GrpcStatus::InvalidCode) { response_headers_->Status()->value(enumToInt(Http::Code::ServiceUnavailable)); } else { response_headers_->Status()->value(Grpc::Utility::grpcToHttpStatus(grpc_status.value())); - response_headers_->insertGrpcStatus().value(enumToInt(grpc_status.value())); + if (!status_converted_to_json && !is_trailers_only_response) { + response_headers_->insertGrpcStatus().value(enumToInt(grpc_status.value())); + } } - const Http::HeaderEntry* grpc_message_header = trailers.GrpcMessage(); - if (grpc_message_header) { - response_headers_->insertGrpcMessage().value(*grpc_message_header); + if (status_converted_to_json && is_trailers_only_response) { + // Drop the gRPC status headers, we already have them in the JSON body. + response_headers_->removeGrpcStatus(); + response_headers_->removeGrpcMessage(); + response_headers_->remove(Http::Headers::get().GrpcStatusDetailsBin); + } else if (!status_converted_to_json && !is_trailers_only_response) { + // Copy the grpc-message header if it exists. + const Http::HeaderEntry* grpc_message_header = trailers.GrpcMessage(); + if (grpc_message_header) { + response_headers_->insertGrpcMessage().value(*grpc_message_header); + } } // remove Trailer headers if the client connection was http/1 @@ -496,6 +551,56 @@ void JsonTranscoderFilter::buildResponseFromHttpBodyOutput(Http::HeaderMap& resp } } +bool JsonTranscoderFilter::maybeConvertGrpcStatus(Grpc::Status::GrpcStatus grpc_status, + Http::HeaderMap& trailers) { + if (!config_.convertGrpcStatus()) { + return false; + } + + // We do not support responses with a separate trailer frame. + // TODO(ascheglov): remove this if after HCM can buffer data added from |encodeTrailers|. + if (response_headers_ != &trailers) { + return false; + } + + // Send a serialized status only if there was no body. + if (has_body_) { + return false; + } + + if (grpc_status == Grpc::Status::GrpcStatus::Ok || + grpc_status == Grpc::Status::GrpcStatus::InvalidCode) { + return false; + } + + auto status_details = Grpc::Common::getGrpcStatusDetailsBin(trailers); + if (!status_details) { + // If no rpc.Status object was sent in the grpc-status-details-bin header, + // construct it from the grpc-status and grpc-message headers. + status_details.emplace(); + status_details->set_code(grpc_status); + + auto grpc_message_header = trailers.GrpcMessage(); + if (grpc_message_header) { + auto message = grpc_message_header->value().getStringView(); + status_details->set_message(message.data(), message.size()); + } + } + + std::string json_status; + auto translate_status = config_.translateProtoMessageToJson(*status_details, &json_status); + if (!translate_status.ok()) { + ENVOY_LOG(debug, "Transcoding status error {}", translate_status.ToString()); + return false; + } + + response_headers_->insertContentType().value().setReference( + Http::Headers::get().ContentTypeValues.Json); + Buffer::OwnedImpl status_data(json_status); + encoder_callbacks_->addEncodedData(status_data, false); + return true; +} + bool JsonTranscoderFilter::hasHttpBodyAsOutputType() { return method_->output_type()->full_name() == google::api::HttpBody::descriptor()->full_name(); } diff --git a/source/extensions/filters/http/grpc_json_transcoder/json_transcoder_filter.h b/source/extensions/filters/http/grpc_json_transcoder/json_transcoder_filter.h index 21976ef9b760..a3bae5e4e027 100644 --- a/source/extensions/filters/http/grpc_json_transcoder/json_transcoder_filter.h +++ b/source/extensions/filters/http/grpc_json_transcoder/json_transcoder_filter.h @@ -68,6 +68,12 @@ class JsonTranscoderConfig : public Logger::Loggable { std::unique_ptr& transcoder, const Protobuf::MethodDescriptor*& method_descriptor); + /** + * Converts an arbitrary protobuf message to JSON. + */ + ProtobufUtil::Status translateProtoMessageToJson(const Protobuf::Message& message, + std::string* json_out); + /** * If true, skip clearing the route cache after the incoming request has been modified. * This allows Envoy to select the upstream cluster based on the incoming request @@ -75,6 +81,12 @@ class JsonTranscoderConfig : public Logger::Loggable { */ bool matchIncomingRequestInfo() const; + /** + * If true, when trailer indicates a gRPC error and there was no HTTP body, + * make google.rpc.Status out of gRPC status headers and use it as JSON body. + */ + bool convertGrpcStatus() const; + private: /** * Convert method descriptor to RequestInfo that needed for transcoding library @@ -83,6 +95,9 @@ class JsonTranscoderConfig : public Logger::Loggable { google::grpc::transcoding::RequestInfo* info); private: + void addFileDescriptor(const Protobuf::FileDescriptorProto& file); + void addBuiltinSymbolDescriptor(const std::string& symbol_name); + Protobuf::DescriptorPool descriptor_pool_; google::grpc::transcoding::PathMatcherPtr path_matcher_; std::unique_ptr type_helper_; @@ -90,6 +105,7 @@ class JsonTranscoderConfig : public Logger::Loggable { bool match_incoming_request_route_{false}; bool ignore_unknown_query_parameters_{false}; + bool convert_grpc_status_{false}; }; using JsonTranscoderConfigSharedPtr = std::shared_ptr; @@ -125,6 +141,7 @@ class JsonTranscoderFilter : public Http::StreamFilter, public Logger::Loggable< private: bool readToBuffer(Protobuf::io::ZeroCopyInputStream& stream, Buffer::Instance& data); void buildResponseFromHttpBodyOutput(Http::HeaderMap& response_headers, Buffer::Instance& data); + bool maybeConvertGrpcStatus(Grpc::Status::GrpcStatus grpc_status, Http::HeaderMap& trailers); bool hasHttpBodyAsOutputType(); JsonTranscoderConfig& config_; @@ -139,6 +156,7 @@ class JsonTranscoderFilter : public Http::StreamFilter, public Logger::Loggable< bool error_{false}; bool has_http_body_output_{false}; + bool has_body_{false}; }; } // namespace GrpcJsonTranscoder diff --git a/test/common/grpc/common_test.cc b/test/common/grpc/common_test.cc index b2ab23f919ad..68128c7faf71 100644 --- a/test/common/grpc/common_test.cc +++ b/test/common/grpc/common_test.cc @@ -78,6 +78,28 @@ TEST(GrpcContextTest, GetGrpcTimeout) { // so we don't test for them. } +TEST(GrpcCommonTest, GrpcStatusDetailsBin) { + Http::TestHeaderMapImpl empty_trailers; + EXPECT_FALSE(Common::getGrpcStatusDetailsBin(empty_trailers)); + + Http::TestHeaderMapImpl invalid_value{{"grpc-status-details-bin", "invalid"}}; + EXPECT_FALSE(Common::getGrpcStatusDetailsBin(invalid_value)); + + Http::TestHeaderMapImpl unpadded_value{ + {"grpc-status-details-bin", "CAUSElJlc291cmNlIG5vdCBmb3VuZA"}}; + auto status = Common::getGrpcStatusDetailsBin(unpadded_value); + ASSERT_TRUE(status); + EXPECT_EQ(Status::GrpcStatus::NotFound, status->code()); + EXPECT_EQ("Resource not found", status->message()); + + Http::TestHeaderMapImpl padded_value{ + {"grpc-status-details-bin", "CAUSElJlc291cmNlIG5vdCBmb3VuZA=="}}; + status = Common::getGrpcStatusDetailsBin(padded_value); + ASSERT_TRUE(status); + EXPECT_EQ(Status::GrpcStatus::NotFound, status->code()); + EXPECT_EQ("Resource not found", status->message()); +} + TEST(GrpcContextTest, ToGrpcTimeout) { Http::HeaderString value; diff --git a/test/extensions/filters/http/grpc_json_transcoder/grpc_json_transcoder_integration_test.cc b/test/extensions/filters/http/grpc_json_transcoder/grpc_json_transcoder_integration_test.cc index 14f491375a8a..2d4d4c8c2666 100644 --- a/test/extensions/filters/http/grpc_json_transcoder/grpc_json_transcoder_integration_test.cc +++ b/test/extensions/filters/http/grpc_json_transcoder/grpc_json_transcoder_integration_test.cc @@ -21,6 +21,9 @@ using Envoy::ProtobufWkt::Empty; namespace Envoy { namespace { +// A magic header value which marks header as not expected. +constexpr char UnexpectedHeaderValue[] = "Unexpected header value"; + class GrpcJsonTranscoderIntegrationTest : public testing::TestWithParam, public HttpIntegrationTest { @@ -136,8 +139,12 @@ class GrpcJsonTranscoderIntegrationTest [](const Http::HeaderEntry& entry, void* context) -> Http::HeaderMap::Iterate { auto* response = static_cast(context); Http::LowerCaseString lower_key{std::string(entry.key().getStringView())}; - EXPECT_EQ(entry.value().getStringView(), - response->headers().get(lower_key)->value().getStringView()); + if (entry.value() == UnexpectedHeaderValue) { + EXPECT_FALSE(response->headers().get(lower_key)); + } else { + EXPECT_EQ(entry.value().getStringView(), + response->headers().get(lower_key)->value().getStringView()); + } return Http::HeaderMap::Iterate::Continue; }, response.get()); @@ -345,6 +352,30 @@ TEST_P(GrpcJsonTranscoderIntegrationTest, UnaryGetError1) { ""); } +// Test an upstream that returns an error in a trailer-only response. +TEST_P(GrpcJsonTranscoderIntegrationTest, UnaryErrorConvertedToJson) { + const std::string filter = + R"EOF( + name: envoy.grpc_json_transcoder + config: + proto_descriptor: "{}" + services: "bookstore.Bookstore" + convert_grpc_status: true + )EOF"; + config_helper_.addFilter( + fmt::format(filter, TestEnvironment::runfilesPath("/test/proto/bookstore.descriptor"))); + HttpIntegrationTest::initialize(); + testTranscoding( + Http::TestHeaderMapImpl{ + {":method", "GET"}, {":path", "/shelves/100"}, {":authority", "host"}}, + "", {"shelf: 100"}, {}, Status(Code::NOT_FOUND, "Shelf 100 Not Found"), + Http::TestHeaderMapImpl{{":status", "404"}, + {"content-type", "application/json"}, + {"grpc-status", UnexpectedHeaderValue}, + {"grpc-message", UnexpectedHeaderValue}}, + R"({"code":5,"message":"Shelf 100 Not Found"})"); +} + TEST_P(GrpcJsonTranscoderIntegrationTest, UnaryDelete) { HttpIntegrationTest::initialize(); testTranscoding( diff --git a/test/extensions/filters/http/grpc_json_transcoder/json_transcoder_filter_test.cc b/test/extensions/filters/http/grpc_json_transcoder/json_transcoder_filter_test.cc index 673a13e0a80c..6bf9411f318b 100644 --- a/test/extensions/filters/http/grpc_json_transcoder/json_transcoder_filter_test.cc +++ b/test/extensions/filters/http/grpc_json_transcoder/json_transcoder_filter_test.cc @@ -312,24 +312,24 @@ TEST_F(GrpcJsonTranscoderConfigTest, InvalidVariableBinding) { class GrpcJsonTranscoderFilterTest : public testing::Test, public GrpcJsonTranscoderFilterTestBase { protected: - GrpcJsonTranscoderFilterTest(const bool match_incoming_request_route = false) - : config_(bookstoreProtoConfig(match_incoming_request_route), *api_), filter_(config_) { + GrpcJsonTranscoderFilterTest(envoy::config::filter::http::transcoder::v2::GrpcJsonTranscoder + proto_config = bookstoreProtoConfig()) + : config_(proto_config, *api_), filter_(config_) { filter_.setDecoderFilterCallbacks(decoder_callbacks_); filter_.setEncoderFilterCallbacks(encoder_callbacks_); } - const envoy::config::filter::http::transcoder::v2::GrpcJsonTranscoder - bookstoreProtoConfig(const bool match_incoming_request_route) { + static const envoy::config::filter::http::transcoder::v2::GrpcJsonTranscoder + bookstoreProtoConfig() { std::string json_string = "{\"proto_descriptor\": \"" + bookstoreDescriptorPath() + "\",\"services\": [\"bookstore.Bookstore\"]}"; auto json_config = Json::Factory::loadFromString(json_string); envoy::config::filter::http::transcoder::v2::GrpcJsonTranscoder proto_config{}; Envoy::Config::FilterJson::translateGrpcJsonTranscoder(*json_config, proto_config); - proto_config.set_match_incoming_request_route(match_incoming_request_route); return proto_config; } - const std::string bookstoreDescriptorPath() { + static const std::string bookstoreDescriptorPath() { return TestEnvironment::runfilesPath("test/proto/bookstore.descriptor"); } @@ -578,7 +578,15 @@ TEST_F(GrpcJsonTranscoderFilterTest, ForwardUnaryPostGrpc) { class GrpcJsonTranscoderFilterSkipRecalculatingTest : public GrpcJsonTranscoderFilterTest { public: - GrpcJsonTranscoderFilterSkipRecalculatingTest() : GrpcJsonTranscoderFilterTest(true) {} + GrpcJsonTranscoderFilterSkipRecalculatingTest() + : GrpcJsonTranscoderFilterTest(makeProtoConfig()) {} + +private: + const envoy::config::filter::http::transcoder::v2::GrpcJsonTranscoder makeProtoConfig() { + auto proto_config = bookstoreProtoConfig(); + proto_config.set_match_incoming_request_route(true); + return proto_config; + } }; TEST_F(GrpcJsonTranscoderFilterSkipRecalculatingTest, TranscodingUnaryPostSkipRecalculate) { @@ -738,6 +746,97 @@ TEST_F(GrpcJsonTranscoderFilterTest, TranscodingUnaryWithHttpBodyAsOutputAndSpli EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_.decodeTrailers(response_trailers)); } +class GrpcJsonTranscoderFilterConvertGrpcStatusTest : public GrpcJsonTranscoderFilterTest { +public: + GrpcJsonTranscoderFilterConvertGrpcStatusTest() + : GrpcJsonTranscoderFilterTest(makeProtoConfig()) {} + + void SetUp() override { + EXPECT_CALL(decoder_callbacks_, clearRouteCache()); + Http::TestHeaderMapImpl request_headers{ + {"content-type", "application/json"}, {":method", "POST"}, {":path", "/shelf"}}; + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.decodeHeaders(request_headers, false)); + + Buffer::OwnedImpl request_data{R"({"theme": "Children"})"}; + EXPECT_EQ(Http::FilterDataStatus::Continue, filter_.decodeData(request_data, true)); + + Http::TestHeaderMapImpl continue_headers{{":status", "000"}}; + EXPECT_EQ(Http::FilterHeadersStatus::Continue, + filter_.encode100ContinueHeaders(continue_headers)); + } + +private: + const envoy::config::filter::http::transcoder::v2::GrpcJsonTranscoder makeProtoConfig() { + auto proto_config = bookstoreProtoConfig(); + proto_config.set_convert_grpc_status(true); + return proto_config; + } +}; + +// Single headers frame with end_stream flag (trailer), no grpc-status-details-bin header. +TEST_F(GrpcJsonTranscoderFilterConvertGrpcStatusTest, TranscodingTextHeadersInTrailerOnlyResponse) { + std::string expected_response(R"({"code":5,"message":"Resource not found"})"); + EXPECT_CALL(encoder_callbacks_, addEncodedData(_, false)) + .WillOnce(Invoke([&expected_response](Buffer::Instance& data, bool) { + EXPECT_EQ(expected_response, data.toString()); + })); + + Http::TestHeaderMapImpl response_headers{{":status", "200"}, + {"content-type", "application/grpc"}, + {"grpc-status", "5"}, + {"grpc-message", "Resource not found"}}; + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.encodeHeaders(response_headers, true)); + EXPECT_EQ("404", response_headers.get_(":status")); + EXPECT_EQ("application/json", response_headers.get_("content-type")); + EXPECT_FALSE(response_headers.has("grpc-status")); + EXPECT_FALSE(response_headers.has("grpc-message")); +} + +// Trailer-only response with grpc-status-details-bin header. +TEST_F(GrpcJsonTranscoderFilterConvertGrpcStatusTest, + TranscodingBinaryHeaderInTrailerOnlyResponse) { + std::string expected_response(R"({"code":5,"message":"Resource not found"})"); + EXPECT_CALL(encoder_callbacks_, addEncodedData(_, false)) + .WillOnce(Invoke([&expected_response](Buffer::Instance& data, bool) { + EXPECT_EQ(expected_response, data.toString()); + })); + + Http::TestHeaderMapImpl response_headers{ + {":status", "200"}, + {"content-type", "application/grpc"}, + {"grpc-status", "5"}, + {"grpc-message", "unused"}, + {"grpc-status-details-bin", "CAUSElJlc291cmNlIG5vdCBmb3VuZA"}}; + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.encodeHeaders(response_headers, true)); + EXPECT_EQ("404", response_headers.get_(":status")); + EXPECT_EQ("application/json", response_headers.get_("content-type")); + EXPECT_FALSE(response_headers.has("grpc-status")); + EXPECT_FALSE(response_headers.has("grpc-message")); + EXPECT_FALSE(response_headers.has("grpc-status-details-bin")); +} + +// Trailer-only response with grpc-status-details-bin header with details. +// Also tests that a user-defined type from a proto descriptor in config can be used in details. +TEST_F(GrpcJsonTranscoderFilterConvertGrpcStatusTest, + TranscodingBinaryHeaderWithDetailsInTrailerOnlyResponse) { + std::string expected_response( + "{\"code\":5,\"message\":\"Error\",\"details\":" + "[{\"@type\":\"type.googleapis.com/helloworld.HelloReply\",\"message\":\"details\"}]}"); + EXPECT_CALL(encoder_callbacks_, addEncodedData(_, false)) + .WillOnce(Invoke([&expected_response](Buffer::Instance& data, bool) { + EXPECT_EQ(expected_response, data.toString()); + })); + + Http::TestHeaderMapImpl response_headers{ + {":status", "200"}, + {"content-type", "application/grpc"}, + {"grpc-status", "5"}, + {"grpc-message", "unused"}, + {"grpc-status-details-bin", + "CAUSBUVycm9yGjYKKXR5cGUuZ29vZ2xlYXBpcy5jb20vaGVsbG93b3JsZC5IZWxsb1JlcGx5EgkKB2RldGFpbHM"}}; + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.encodeHeaders(response_headers, true)); +} + struct GrpcJsonTranscoderFilterPrintTestParam { std::string config_json_; std::string expected_response_; diff --git a/test/proto/BUILD b/test/proto/BUILD index 08c8384dee44..71f551469cdf 100644 --- a/test/proto/BUILD +++ b/test/proto/BUILD @@ -31,6 +31,9 @@ envoy_proto_descriptor( name = "bookstore_proto_descriptor", srcs = [ "bookstore.proto", + # JSON transcoder doesn't link against ":helloworld_proto_cc", so we can add it to the + # descriptor and test that we can actually transcode types not linked into the test binary. + "helloworld.proto", ], out = "bookstore.descriptor", external_deps = [ diff --git a/tools/check_format.py b/tools/check_format.py index 67292cea223b..20a7c9269e7f 100755 --- a/tools/check_format.py +++ b/tools/check_format.py @@ -57,8 +57,9 @@ "./source/server/overload_manager_impl.cc") # Files in these paths can use MessageLite::SerializeAsString -SERIALIZE_AS_STRING_WHITELIST = ("./test/common/protobuf/utility_test.cc", - "./test/common/grpc/codec_test.cc") +SERIALIZE_AS_STRING_WHITELIST = ( + "./source/extensions/filters/http/grpc_json_transcoder/json_transcoder_filter.cc", + "./test/common/protobuf/utility_test.cc", "./test/common/grpc/codec_test.cc") # Files in these paths can use Protobuf::util::JsonStringToMessage JSON_STRING_TO_MESSAGE_WHITELIST = ("./source/common/protobuf/utility.cc")