Skip to content

Add a filter to support HTTP/JSON<->gRPC transcoding #1079

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mattklein123 merged 42 commits into from
Jul 1, 2017
Merged

Add a filter to support HTTP/JSON<->gRPC transcoding #1079

Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
6572487
Add TranscoderInputStreamImpl
lizan Jun 9, 2017
a8f8f33
Merge remote-tracking branch 'upstream/master' into grpc_transcoder_i…
lizan Jun 14, 2017
579d342
Refactor ZeroCopyInputStreamImpl out
lizan Jun 15, 2017
118d1e7
Merge remote-tracking branch 'upstream/master' into grpc_transcoder_i…
lizan Jun 15, 2017
ca46f94
fix
lizan Jun 15, 2017
8034159
Use ZeroCopyInputStreamImpl in AsyncClientImpl and RpcChannelImpl
lizan Jun 15, 2017
77f9c00
fix
lizan Jun 15, 2017
dd68af1
fix format
lizan Jun 15, 2017
8d235bd
fix finish test
lizan Jun 15, 2017
98c5ccf
Address review comments
lizan Jun 19, 2017
547b92f
Merge remote-tracking branch 'upstream/master' into grpc_transcoder_i…
lizan Jun 19, 2017
8f1f307
fix format
lizan Jun 19, 2017
b88b0d5
fix test include
lizan Jun 20, 2017
9850c9b
fix format
lizan Jun 20, 2017
53fcf47
fix grammer in comments
lizan Jun 20, 2017
6be5d0d
fix format
lizan Jun 20, 2017
ae9094f
Add transcoding docs
lizan Jun 7, 2017
13b66db
Add transcoding filter from istio/proxy
lizan Jun 9, 2017
5eabe6f
Test WIP
lizan Jun 9, 2017
5446914
align with master
lizan Jun 21, 2017
db48843
Add unittest
lizan Jun 21, 2017
f92d527
Add integration test
lizan Jun 21, 2017
b68400e
Merge remote-tracking branch 'upstream/master' into grpc_json_transco…
lizan Jun 21, 2017
cc27cec
fix format
lizan Jun 21, 2017
c99496d
Merge remote-tracking branch 'upstream/master' into grpc_json_transco…
lizan Jun 22, 2017
c45226a
Address @htuch reviews
lizan Jun 22, 2017
3acba06
refactor method signatures
lizan Jun 22, 2017
65567f4
Add patch and custom test
lizan Jun 22, 2017
41416cd
Add comment for config class
lizan Jun 23, 2017
79791ab
fix code comment, rename to grpc_json_transcoder
lizan Jun 27, 2017
af5ac95
docs
lizan Jun 27, 2017
b2265b0
fix format
lizan Jun 27, 2017
ce354b4
Merge remote-tracking branch 'upstream/master' into grpc_json_transco…
lizan Jun 27, 2017
8b154c7
Update docs
lizan Jun 28, 2017
6894025
Merge remote-tracking branch 'upstream/master' into grpc_json_transco…
lizan Jun 29, 2017
efba771
increase test coverage
lizan Jun 29, 2017
80de522
move descriptor generation into bzl rule
lizan Jun 30, 2017
2a35000
fix format
lizan Jun 30, 2017
d41f9d9
fix format
lizan Jun 30, 2017
38d08fa
Update grpc_json_transcoder_filter.rst
mattklein123 Jun 30, 2017
c82fd4a
assert on error_
lizan Jun 30, 2017
cdeb87d
fix BUILD, add more comments
lizan Jun 30, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions docs/configuration/http_filters/grpc_json_transcoding_filter.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
.. _config_http_filters_grpc_json_transcoding:

gRPC-JSON transcoding filter
============================

gRPC :ref:`architecture overview <arch_overview_grpc>`.

This is a filter which enables the bridging of a JSON-REST client to a gRPC server.

@mattklein123 mattklein123 Jun 23, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is enough documentation for someone to actually use this filter effectively. Do you have any other documentation somewhere that we can link to? Otherwise, can we add some more documentation? And maybe do a follow up at some point with a full docker example? I think there should be some overview discussion of this functionality on this page also: https://lyft.github.io/envoy/docs/intro/arch_overview/grpc.html

@lizan lizan Jun 27, 2017

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the confusing PR, the docs was WIP, now it is ready, PTAL again.


.. code-block:: json

{
"type": "both",
"name": "grpc_json_transcoding",
"config": {
"proto_descriptors": "proto.pb",

@mattklein123 mattklein123 Jun 23, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'm a little confused about what this filter does. My understanding is that through reflection it can operate in a generic transcoding mode without knowing the pb definition ahead of time. Is that possible? I think many use cases are going to want that. Having to predefine the pb is kind of a pain from operations perspective. Do we predefine the pb so that we can do better error checking? (Maybe just related to needing more documentation).

@lizan lizan Jun 27, 2017

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe it is possible with grpc reflection, however we have no experience with that and reflection itself is still v1alpha. I will take this on later task, not in this PR. predefined pb doesn't provide better error checking.

"services": ["grpc.service.Service"]
}
}
1 change: 1 addition & 0 deletions docs/configuration/http_filters/http_filters.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ HTTP filters
fault_filter
dynamodb_filter
grpc_http1_bridge_filter
grpc_json_transcoding_filter
grpc_web_filter
health_check_filter
ip_tagging_filter
Expand Down
4 changes: 2 additions & 2 deletions source/common/buffer/zero_copy_input_stream_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ bool ZeroCopyInputStreamImpl::Next(const void** data, int* size) {
bool ZeroCopyInputStreamImpl::Skip(int) { NOT_IMPLEMENTED; }

void ZeroCopyInputStreamImpl::BackUp(int count) {
ASSERT(count > 0);
ASSERT(uint64_t(count) < position_);
ASSERT(count >= 0);
ASSERT(uint64_t(count) <= position_);

// Preconditions for BackUp:
// - The last method called must have been Next().
Expand Down
29 changes: 29 additions & 0 deletions source/common/grpc/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,25 @@ envoy_cc_library(
],
)

envoy_cc_library(
name = "transcoding_filter_lib",
srcs = ["transcoding_filter.cc"],
hdrs = ["transcoding_filter.h"],
external_deps = [
"path_matcher",
"grpc_transcoding",
],
deps = [
":codec_lib",
":common_lib",
":transcoder_input_stream_lib",
"//include/envoy/http:filter_interface",
"//source/common/common:base64_lib",
"//source/common/http:headers_lib",
"@googleapis//:http_api_protos",

@htuch htuch Jun 29, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't reference external deps like googleapis with "@googleapis", they need to appear in external_deps instead. We won't be able to perform the remapping during the Google import otherwise.

],
)

envoy_cc_library(
name = "grpc_web_filter_lib",
srcs = ["grpc_web_filter.cc"],
Expand Down Expand Up @@ -114,3 +133,13 @@ envoy_cc_library(
"//source/common/buffer:zero_copy_input_stream_lib",
],
)

envoy_cc_library(
name = "transcoder_input_stream_lib",
srcs = ["transcoder_input_stream_impl.cc"],
hdrs = ["transcoder_input_stream_impl.h"],
external_deps = ["grpc_transcoding"],
deps = [
"//source/common/buffer:zero_copy_input_stream_lib",
],
)
Loading