Skip to content

Commit

Permalink
grpcJsonTranscoder: add match_unregistered_custom_verb (envoyproxy#19203
Browse files Browse the repository at this point in the history
)

Mainly update the grpcJsonTranscoder to include the latest path matcher filter change for always checking custom verb no matter if it is registered or not.

Risk Level: Low
Testing: added the unit test
Docs Changes: None
Release Notes: Yes

Signed-off-by: Xuyang Tao <[email protected]>
  • Loading branch information
TAOXUY authored Dec 10, 2021
1 parent 3049720 commit c68cf0d
Show file tree
Hide file tree
Showing 6 changed files with 95 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE;
// gRPC-JSON transcoder :ref:`configuration overview <config_http_filters_grpc_json_transcoder>`.
// [#extension: envoy.filters.http.grpc_json_transcoder]

// [#next-free-field: 13]
// [#next-free-field: 14]
// 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
Expand Down Expand Up @@ -222,6 +222,17 @@ message GrpcJsonTranscoder {
// This is to support `HTML 2.0 <https://tools.ietf.org/html/rfc1866#section-8.2.1>`_
bool query_param_unescape_plus = 12;

// If true, try to match the custom verb even if it is unregistered. By
// default, only match when it is registered.
//
// According to the http template `syntax <https://github.com/googleapis/googleapis/blob/master/google/api/http.proto#L226-L231>`_,
// the custom verb is **":" LITERAL** at the end of http template.
//
// For a request with */foo/bar:baz* and *:baz* is not registered in any url_template, here is the behavior change
// - if the field is not set, *:baz* will not be treated as custom verb, so it will match **/foo/{x=*}**.
// - if the field is set, *:baz* is treated as custom verb, so it will NOT match **/foo/{x=*}** since the template doesn't use any custom verb.
bool match_unregistered_custom_verb = 13;

// Configure the behavior when handling requests that cannot be transcoded.
//
// By default, the transcoder will silently pass through HTTP requests that are malformed.
Expand Down
6 changes: 3 additions & 3 deletions bazel/repository_locations.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -652,13 +652,13 @@ REPOSITORY_LOCATIONS_SPEC = dict(
project_name = "grpc-httpjson-transcoding",
project_desc = "Library that supports transcoding so that HTTP/JSON can be converted to gRPC",
project_url = "https://github.com/grpc-ecosystem/grpc-httpjson-transcoding",
version = "6acdde98c94b70453b39a81fe7bf59b847188fc3",
sha256 = "a076ca60fca2719b505c49dc1175fd27485dc76deaaef21581e6bd37c84da890",
version = "bdd203e981d5ec25166aa5f5df6b443986eea556",
sha256 = "2ce3a6306b245cf46834a3538bcac327359fc4b1f8b0e2d4881c9ff0acfe5ba5",
strip_prefix = "grpc-httpjson-transcoding-{version}",
urls = ["https://github.com/grpc-ecosystem/grpc-httpjson-transcoding/archive/{version}.tar.gz"],
use_category = ["dataplane_ext"],
extensions = ["envoy.filters.http.grpc_json_transcoder"],
release_date = "2021-11-04",
release_date = "2021-12-03",
cpe = "N/A",
),
io_bazel_rules_go = dict(
Expand Down
1 change: 1 addition & 0 deletions docs/root/version_history/current.rst
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ New Features
* dns_resolver: added :ref:`CaresDnsResolverConfig<envoy_v3_api_msg_extensions.network.dns_resolver.cares.v3.CaresDnsResolverConfig>` to support c-ares DNS resolver as an extension.
* dns_resolver: added :ref:`AppleDnsResolverConfig<envoy_v3_api_msg_extensions.network.dns_resolver.apple.v3.AppleDnsResolverConfig>` to support apple DNS resolver as an extension.
* ext_authz: added :ref:`query_parameters_to_set <envoy_v3_api_field_service.auth.v3.OkHttpResponse.query_parameters_to_set>` and :ref:`query_parameters_to_remove <envoy_v3_api_field_service.auth.v3.OkHttpResponse.query_parameters_to_remove>` for adding and removing query string parameters when using a gRPC authorization server.
* grpc_json_transcoder: added support for matching unregistered custom verb :ref:`match_unregistered_custom_verb <envoy_v3_api_field_extensions.filters.http.grpc_json_transcoder.v3.GrpcJsonTranscoder.match_unregistered_custom_verb>`.
* http: added support for %REQUESTED_SERVER_NAME% to extract SNI as a custom header.
* http: added support for %VIRTUAL_CLUSTER_NAME% to extract the matched Virtual Cluster name as a custom header.
* http: added support for :ref:`retriable health check status codes <envoy_v3_api_field_config.core.v3.HealthCheck.HttpHealthCheck.retriable_statuses>`.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ JsonTranscoderConfig::JsonTranscoderConfig(
NOT_REACHED_GCOVR_EXCL_LINE;
}
pmb.SetQueryParamUnescapePlus(proto_config.query_param_unescape_plus());
pmb.SetMatchUnregisteredCustomVerb(proto_config.match_unregistered_custom_verb());

path_matcher_ = pmb.Build();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,79 @@ TEST_F(GrpcJsonTranscoderConfigTest, InvalidVariableBinding) {
EXPECT_FALSE(transcoder);
}

// By default, the transcoder will treat unregistered custom verb as part of path segment,
// which can be captured in a wildcard.
TEST_F(GrpcJsonTranscoderConfigTest, UnregisteredCustomVerb) {
JsonTranscoderConfig config(
getProtoConfig(TestEnvironment::runfilesPath("test/proto/bookstore.descriptor"),
"bookstore.Bookstore", false),
*api_);

// It is matched to PostWildcard `POST /wildcard/{arg=**}`.
// ":unknown" was not treated as custom verb but as part of path segment,
// so it matches *.
Http::TestRequestHeaderMapImpl headers{{":method", "POST"},
{":path", "/wildcard/random:unknown"}};

TranscoderInputStreamImpl request_in, response_in;
TranscoderPtr transcoder;
MethodInfoSharedPtr method_info;
const auto status =
config.createTranscoder(headers, request_in, response_in, transcoder, method_info);

EXPECT_TRUE(status.ok());
EXPECT_TRUE(transcoder);
EXPECT_EQ("bookstore.Bookstore.PostWildcard", method_info->descriptor_->full_name());
}

// By default, the transcoder will always try to match the registered custom
// verbs.
TEST_F(GrpcJsonTranscoderConfigTest, RegisteredCustomVerb) {
JsonTranscoderConfig config(
getProtoConfig(TestEnvironment::runfilesPath("test/proto/bookstore.descriptor"),
"bookstore.Bookstore", false),
*api_);

// Now, the `verb` is registered by PostCustomVerb `POST /foo/bar:verb`,
// so the transcoder will strictly match `verb`.
Http::TestRequestHeaderMapImpl headers{{":method", "POST"}, {":path", "/wildcard/random:verb"}};

TranscoderInputStreamImpl request_in, response_in;
TranscoderPtr transcoder;
MethodInfoSharedPtr method_info;
const auto status =
config.createTranscoder(headers, request_in, response_in, transcoder, method_info);

EXPECT_EQ(status.code(), StatusCode::kNotFound);
EXPECT_EQ(status.message(), "Could not resolve /wildcard/random:verb to a method.");
EXPECT_FALSE(transcoder);
}

// When `set_match_unregistered_custom_verb=true`, the transcoder will always
// try to match the unregistered custom verbs like the registered ones.
TEST_F(GrpcJsonTranscoderConfigTest, MatchUnregisteredCustomVerb) {
auto proto_config =
getProtoConfig(TestEnvironment::runfilesPath("test/proto/bookstore.descriptor"),
"bookstore.Bookstore", false);
proto_config.set_match_unregistered_custom_verb(true);
JsonTranscoderConfig config(proto_config, *api_);

// Even though the `unknown` is not registered, but as match_unregistered_custom_verb=true, the
// transcoder will strictly try to match it.
Http::TestRequestHeaderMapImpl headers{{":method", "POST"},
{":path", "/wildcard/random:unknown"}};

TranscoderInputStreamImpl request_in, response_in;
TranscoderPtr transcoder;
MethodInfoSharedPtr method_info;
const auto status =
config.createTranscoder(headers, request_in, response_in, transcoder, method_info);

EXPECT_EQ(status.code(), StatusCode::kNotFound);
EXPECT_EQ(status.message(), "Could not resolve /wildcard/random:unknown to a method.");
EXPECT_FALSE(transcoder);
}

class GrpcJsonTranscoderFilterTest : public testing::Test, public GrpcJsonTranscoderFilterTestBase {
protected:
GrpcJsonTranscoderFilterTest(
Expand Down
5 changes: 5 additions & 0 deletions test/proto/bookstore.proto
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,11 @@ service Bookstore {
post: "/wildcard/{arg=**}"
};
}
rpc PostCustomVerb(EchoBodyRequest) returns (google.protobuf.Empty) {
option (google.api.http) = {
post: "/foo/bar:verb"
};
}
}

service ServiceWithResponseBody {
Expand Down

0 comments on commit c68cf0d

Please sign in to comment.