Skip to content

Commit

Permalink
add option to skip clearing the route cache in grpc transcoder filter
Browse files Browse the repository at this point in the history
Signed-off-by: ilackarms <sdw35@cornell.edu>
  • Loading branch information
ilackarms committed Apr 2, 2018
1 parent 833769c commit 73d7d6d
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 5 deletions.
2 changes: 1 addition & 1 deletion bazel/repository_locations.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ REPOSITORY_LOCATIONS = dict(
urls = ["https://github.com/google/protobuf/archive/v3.5.0.tar.gz"],
),
envoy_api = dict(
commit = "e73b8264a26c909f75d38ec2c73d9f47ad1b3b57",
commit = "c4590ec24e33a1205bde3ba13ea025664c4f3bb4",
remote = "https://github.com/envoyproxy/data-plane-api",
),
grpc_httpjson_transcoding = dict(
Expand Down
8 changes: 7 additions & 1 deletion source/common/grpc/json_transcoder_filter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,12 @@ JsonTranscoderConfig::JsonTranscoderConfig(
print_options_.always_print_primitive_fields = print_config.always_print_primitive_fields();
print_options_.always_print_enums_as_ints = print_config.always_print_enums_as_ints();
print_options_.preserve_proto_field_names = print_config.preserve_proto_field_names();

match_incoming_request_route_ = proto_config.match_incoming_request_route();
}

bool JsonTranscoderConfig::matchIncomingRequestRoute() { return match_incoming_request_route_; }

ProtobufUtil::Status JsonTranscoderConfig::createTranscoder(
const Http::HeaderMap& headers, ZeroCopyInputStream& request_input,
google::grpc::transcoding::TranscoderInputStream& response_input,
Expand Down Expand Up @@ -217,7 +221,9 @@ Http::FilterHeadersStatus JsonTranscoderFilter::decodeHeaders(Http::HeaderMap& h
headers.insertMethod().value().setReference(Http::Headers::get().MethodValues.Post);
headers.insertTE().value().setReference(Http::Headers::get().TEValues.Trailers);

decoder_callbacks_->clearRouteCache();
if (!config_.matchIncomingRequestRoute()) {
decoder_callbacks_->clearRouteCache();
}

if (end_stream) {
request_in_.finish();
Expand Down
9 changes: 9 additions & 0 deletions source/common/grpc/json_transcoder_filter.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,13 @@ class JsonTranscoderConfig : public Logger::Loggable<Logger::Id::config> {
std::unique_ptr<google::grpc::transcoding::Transcoder>& transcoder,
const Protobuf::MethodDescriptor*& method_descriptor);

/**
* 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 matchIncomingRequestRoute();

private:
/**
* Convert method descriptor to RequestInfo that needed for transcoding library
Expand All @@ -74,6 +81,8 @@ class JsonTranscoderConfig : public Logger::Loggable<Logger::Id::config> {
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};
};

typedef std::shared_ptr<JsonTranscoderConfig> JsonTranscoderConfigSharedPtr;
Expand Down
40 changes: 37 additions & 3 deletions test/common/grpc/json_transcoder_filter_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,14 @@ namespace Grpc {
class GrpcJsonTranscoderConfigTest : public testing::Test {
public:
const envoy::config::filter::http::transcoder::v2::GrpcJsonTranscoder
getProtoConfig(const std::string& descriptor_path, const std::string& service_name) {
getProtoConfig(const std::string& descriptor_path, const std::string& service_name,
const bool skip_recalculating_route = false) {
std::string json_string = "{\"proto_descriptor\": \"" + descriptor_path +
"\",\"services\": [\"" + service_name + "\"]}";
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_skip_recalculating_route(skip_recalculating_route);
return proto_config;
}

Expand Down Expand Up @@ -105,6 +107,12 @@ TEST_F(GrpcJsonTranscoderConfigTest, ParseConfig) {
TestEnvironment::runfilesPath("test/proto/bookstore.descriptor"), "bookstore.Bookstore")));
}

TEST_F(GrpcJsonTranscoderConfigTest, ParseConfigSkipRecalculating) {
EXPECT_NO_THROW(JsonTranscoderConfig config(
getProtoConfig(TestEnvironment::runfilesPath("test/proto/bookstore.descriptor"),
"bookstore.Bookstore", true)));
}

TEST_F(GrpcJsonTranscoderConfigTest, ParseBinaryConfig) {
envoy::config::filter::http::transcoder::v2::GrpcJsonTranscoder proto_config;
proto_config.set_proto_descriptor_bin(
Expand Down Expand Up @@ -197,17 +205,20 @@ TEST_F(GrpcJsonTranscoderConfigTest, InvalidVariableBinding) {

class GrpcJsonTranscoderFilterTest : public testing::Test {
public:
GrpcJsonTranscoderFilterTest() : config_(bookstoreProtoConfig()), filter_(config_) {
GrpcJsonTranscoderFilterTest(const bool skip_recalculating_route = false)
: config_(bookstoreProtoConfig(skip_recalculating_route)), filter_(config_) {
filter_.setDecoderFilterCallbacks(decoder_callbacks_);
filter_.setEncoderFilterCallbacks(encoder_callbacks_);
}

const envoy::config::filter::http::transcoder::v2::GrpcJsonTranscoder bookstoreProtoConfig() {
const envoy::config::filter::http::transcoder::v2::GrpcJsonTranscoder
bookstoreProtoConfig(const bool skip_recalculating_route) {
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_skip_recalculating_route(skip_recalculating_route);
return proto_config;
}

Expand All @@ -222,6 +233,11 @@ class GrpcJsonTranscoderFilterTest : public testing::Test {
NiceMock<Http::MockStreamEncoderFilterCallbacks> encoder_callbacks_;
};

class GrpcJsonTranscoderFilterSkipRecalculatingTest : public GrpcJsonTranscoderFilterTest {
public:
GrpcJsonTranscoderFilterSkipRecalculatingTest() : GrpcJsonTranscoderFilterTest(true) {}
};

TEST_F(GrpcJsonTranscoderFilterTest, NoTranscoding) {
Http::TestHeaderMapImpl request_headers{{"content-type", "application/grpc"},
{":method", "POST"},
Expand Down Expand Up @@ -324,6 +340,24 @@ TEST_F(GrpcJsonTranscoderFilterTest, TranscodingUnaryPost) {
EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_.decodeTrailers(response_trailers));
}

TEST_F(GrpcJsonTranscoderFilterSkipRecalculatingTest, TranscodingUnaryPostSkipRecalculate) {
Http::TestHeaderMapImpl request_headers{
{"content-type", "application/json"}, {":method", "POST"}, {":path", "/shelf"}};

EXPECT_CALL(decoder_callbacks_, clearRouteCache()).Times(0);

EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.decodeHeaders(request_headers, false));

EXPECT_EQ("application/grpc", request_headers.get_("content-type"));
EXPECT_EQ("/shelf", request_headers.get_("x-envoy-original-path"));
EXPECT_EQ("/bookstore.Bookstore/CreateShelf", request_headers.get_(":path"));
EXPECT_EQ("trailers", request_headers.get_("te"));

Buffer::OwnedImpl request_data{"{\"theme\": \"Children\"}"};

EXPECT_EQ(Http::FilterDataStatus::Continue, filter_.decodeData(request_data, true));
}

TEST_F(GrpcJsonTranscoderFilterTest, TranscodingUnaryError) {
Http::TestHeaderMapImpl request_headers{
{"content-type", "application/json"}, {":method", "POST"}, {":path", "/shelf"}};
Expand Down

0 comments on commit 73d7d6d

Please sign in to comment.