Skip to content

Commit

Permalink
grpc-json: add option to convert gRPC status into JSON body (envoypro…
Browse files Browse the repository at this point in the history
…xy#3383) (envoyproxy#8009)

When trailer indicates a gRPC error and there was no HTTP body, with the `convert_grpc_status option` enabled, take `google.rpc.Status` from the `grpc-status-details-bin` header and use it as a JSON body.
If there was no such header, make `google.rpc.Status` out of the `grpc-status` and `grpc-message` headers.

This also adds `google.rpc.Status` to user-provided protobuf descriptor.

Risk Level: Small-medium
Testing: Added unit and integration tests tests, tested manually.
Docs Changes:
Added field description in api/envoy/config/filter/http/transcoder/v2/transcoder.proto
Release Notes:
Fixes envoyproxy#3383

Signed-off-by: Anatoly Scheglov <[email protected]>
  • Loading branch information
ascheglov committed Sep 19, 2019
1 parent ada91c2 commit 60e2ad9
Show file tree
Hide file tree
Showing 14 changed files with 363 additions and 19 deletions.
30 changes: 30 additions & 0 deletions api/envoy/config/filter/http/transcoder/v2/transcoder.proto
Original file line number Diff line number Diff line change
Expand Up @@ -119,4 +119,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 <config_grpc_json_generate_proto_descriptor_set>`.
bool convert_grpc_status = 9;
}
1 change: 1 addition & 0 deletions docs/root/intro/version_history.rst
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ Version history
* fault: added overrides for default runtime keys in :ref:`HTTPFault <envoy_api_msg_config.filter.http.fault.v2.HTTPFault>` filter.
* grpc: added :ref:`AWS IAM grpc credentials extension <envoy_api_file_envoy/config/grpc_credential/v2alpha/aws_iam.proto>` for AWS-managed xDS.
* grpc-json: added support for :ref:`ignoring unknown query parameters<envoy_api_field_config.filter.http.transcoder.v2.GrpcJsonTranscoder.ignore_unknown_query_parameters>`.
* grpc-json: added support for :ref:`the grpc-status-details-bin header<envoy_api_field_config.filter.http.transcoder.v2.GrpcJsonTranscoder.convert_grpc_status>`.
* header to metadata: added :ref:`PROTOBUF_VALUE <envoy_api_enum_value_config.filter.http.header_to_metadata.v2.Config.ValueType.PROTOBUF_VALUE>` and :ref:`ValueEncode <envoy_api_enum_config.filter.http.header_to_metadata.v2.Config.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: changed Envoy to forward existing x-forwarded-proto from upstream trusted proxies. Guarded by `envoy.reloadable_features.trusted_forwarded_proto` which defaults true.
Expand Down
1 change: 1 addition & 0 deletions source/common/grpc/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
24 changes: 23 additions & 1 deletion source/common/grpc/common.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -54,7 +55,7 @@ absl::optional<Status::GrpcStatus> 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) {
Expand All @@ -68,6 +69,27 @@ std::string Common::getGrpcMessage(const Http::HeaderMap& trailers) {
return entry ? std::string(entry->value().getStringView()) : EMPTY_STRING;
}

absl::optional<google::rpc::Status>
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.
Expand Down
9 changes: 9 additions & 0 deletions source/common/grpc/common.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<google::rpc::Status> the gRPC status message or empty pointer if no
* grpc-status-details-bin trailer found or it was invalid.
*/
static absl::optional<google::rpc::Status>
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.
Expand Down
1 change: 1 addition & 0 deletions source/common/http/headers.h
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,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 Http2Settings{"http2-settings"};
Expand Down
1 change: 1 addition & 0 deletions source/common/protobuf/protobuf.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,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<const Protobuf::MethodDescriptor*> pmb;
Expand Down Expand Up @@ -162,10 +166,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,
Expand Down Expand Up @@ -242,6 +270,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,
Expand Down Expand Up @@ -383,6 +419,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);
Expand Down Expand Up @@ -418,25 +456,42 @@ Http::FilterTrailersStatus JsonTranscoderFilter::encodeTrailers(Http::HeaderMap&

if (data.length()) {
encoder_callbacks_->addEncodedData(data, true);
has_body_ = true;
}

if (method_->server_streaming()) {
// For streaming case, the headers are already sent, so just continue here.
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::GrpcStatus> 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
Expand Down Expand Up @@ -494,6 +549,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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,25 @@ class JsonTranscoderConfig : public Logger::Loggable<Logger::Id::config> {
std::unique_ptr<google::grpc::transcoding::Transcoder>& 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
* rather than the outgoing.
*/
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
Expand All @@ -83,13 +95,17 @@ class JsonTranscoderConfig : public Logger::Loggable<Logger::Id::config> {
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<const Protobuf::MethodDescriptor*> path_matcher_;
std::unique_ptr<google::grpc::transcoding::TypeHelper> type_helper_;
Protobuf::util::JsonPrintOptions print_options_;

bool match_incoming_request_route_{false};
bool ignore_unknown_query_parameters_{false};
bool convert_grpc_status_{false};
};

using JsonTranscoderConfigSharedPtr = std::shared_ptr<JsonTranscoderConfig>;
Expand Down Expand Up @@ -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_;
Expand All @@ -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
Expand Down
22 changes: 22 additions & 0 deletions test/common/grpc/common_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Loading

0 comments on commit 60e2ad9

Please sign in to comment.