Add support for basic gRPC API Configuration YAML files #521

hacst · 2018-01-14T08:27:58Z

Sometimes the requirement to have to annotate your .proto files to expose them using grpc-gateway can be inconvenient. For one all your backends and clients using the .proto now need to know the google api types for the annotation which can be a hassle. More importantly in a number of situations you cannot/do not want to change the .proto at all (e.g. if you don't own the .proto or only want to use the gateway for experiments).

Google offers a way to achieve this for services hosted with them called "gRPC API Configuration" files. This PR implements a small part of those for grpc-gateway:

Add support for basic gRPC API Configuration YAML files

Separate gRPC API Configuration files as described by google at
https://cloud.google.com/endpoints/docs/grpc/grpc-service-config
can be used to separate the description of the basic API surface
in .proto files from their mapping to HTTP REST. In grpc-gateway
this allows exposing gRPC services without having to add any
annotations to their .proto files.

The format of gRPC API Configuration YAML files is defined by the
proto message schema google.api.service which can be found as
service.proto in the googleapis repository. This patch exposes
the Http part of the service configuration to the gateway
generator.

To enable this all HttpRules contained in the Http part of the
description are loaded into the generator registry as a map from
service method to HttpRules called externalHttpRules. During
loading of the proto files we can now use these externalHttpRules
to look for additional rules to apply to a service method in addition
to the ones from the annotations found in the proto file. As the
types are the same no fundamentally new codepaths are required in
either of the generators.

google.api.service is quite a complex protobuf message with lots
of dependencies on other google api. To not have to pull in all
other dependent types this patch constructs a reduced service
message type with only the Http field. This relies on protobufs
backwards compatibility guarantees which allows us to remove all
other fields. Also we do not need all protobuf features on this
custom message.

To load the actual YAML file as a protobuf message this patch uses
a new dependency on github.com/ghodss/yaml to convert the YAML into
JSON which we can then unmarshal into protobuf using jsonpb.

The code already works quite nicely for what I had in mind but before doing the boring parts of completing test coverage and examples I wanted to gauge interest in integrating this feature as well as get some feedback on the approach taken.

googlebot · 2018-01-14T08:28:00Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.
If your company signed a CLA, they designated a Point of Contact who decides which employees are authorized to participate. You may need to contact the Point of Contact for your company and ask to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the project maintainer to go/cla#troubleshoot. The email used to register you as an authorized contributor must be the email used for the Git commit.
In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again. If the bot doesn't comment, it means it doesn't think anything has changed.

hacst · 2018-01-14T08:35:20Z

I signed it!

googlebot · 2018-01-14T08:35:22Z

CLAs look good, thanks!

achew22 · 2018-01-21T03:55:38Z

@hacst, thanks so much for sending this in! I'm definitely intrigued by the concept here. Would the idea be to entirely leave this at code generation time, or would it become a runtime configuration option? As I understand it, the YAML files exist to modify configuration at runtime in the endpoints project. Can you help me understand a little better?

tmc · 2018-01-21T04:12:33Z

Very cool! My read (haven't combed the diff yet) is that this is just an alternate http.proto specification avenue ala https://cloud.google.com/endpoints/docs/grpc/transcoding#configuring_transcoding_in_yaml

hacst · 2018-01-21T04:54:42Z

Great to see there is interest.

@achew22 Now that you mention it, being able to have this a runtime configuration would be kinda neat. Google uses them this way but for grpc-gateway that would be a pretty large conceptual change I imagine. This PR really only adds another way to get http configuration into the grpc-gateway protoc plugins during generation time.

Taking the README.md echo example instead of doing step 2 you would create a your_api_config.yaml containing something like:

type: google.api.Service
config_version: 3

http:
  rules:
  - selector: example.YourService.Echo
    post: /v1/example/echo
    body: "*"

That can then be passed to the gateway and swagger generator:

protoc -I/usr/local/include -I. \
  -I$GOPATH/src \
  -I$GOPATH/src/github.com/grpc-ecosystem/grpc-gateway/third_party/googleapis \
  --grpc-gateway_out=logtostderr=true,grpc_api_configuration=path/to/your_api_config.yaml:. \
  path/to/your_service.proto

protoc -I/usr/local/include -I. \
  -I$GOPATH/src \
  -I$GOPATH/src/github.com/grpc-ecosystem/grpc-gateway/third_party/googleapis \
  --swagger_out=logtostderr=true,grpc_api_configuration=path/to/your_api_config.yaml:. \
  path/to/your_service.proto

Hope that clarifies things.

achew22 · 2018-01-21T07:59:34Z

Could you comment on this

Sometimes the requirement to have to annotate your .proto files to expose them using grpc-gateway can be inconvenient. For one all your backends and clients using the .proto now need to know the google api types for the annotation which can be a hassle. More importantly in a number of situations you cannot/do not want to change the .proto at all (e.g. if you don't own the .proto or only want to use the gateway for experiments).

As I understand it, you wouldn't need to include the annotation types in the outputted code. Since they are service definitions they would be available in the proto reflection as an extension, but wouldn't be included as a compiled component. Do you mean that dependent projects would need to configure the protoc to have those extensions available?

hacst · 2018-01-21T08:33:21Z

@achew22 Yes. That and more. My understand is that as soon as my service proto includes annotations every user (whether they care about the http annotations or - imho more likely - not) not only has to make sure annotations.proto (and transitively http.proto) are available to protoc during generation but also has to have these files protoc'd for their implementation language for the client/service compilation. As these types are part of the google.api which is not shipped with protobuf this can create quite a lot of friction if the service proto is widely used.

As a concrete example in C++ I need google/api/annotations.pb.h and google/api/http.pb.h headers to be able to build as soon as the service proto includes annotations. I assumed it was the same for other languages. Is this not the case? Is there a way around this?

achew22 · 2018-02-13T08:13:53Z

Upon reflection you're totally 100% right about that. There is no way to even have that without importing the other libraries. This is also the only way we can go forward that allows someone to use grpc-gateway on a file they don't have edit permissions on.

Out of curiosity, do you just invoke this on the command line through protoc but with an additional argument grpc_api_configuration with a path to the file? For some reason in my mind it seems weird to load a file that isn't passed in on stdin for a protoc plugin. Does protoc do anything weird like sandboxing to prevent doing that?

hacst · 2018-02-15T21:17:38Z

@achew22 Passing the file is pretty natural. You do it the same way you pass other parameters directly to the plugin right now.

E.g to make the gateway or swagger plugin itself log to stderr you currently do:

protoc --grpc-gateway_out=logtostderr=true:. my.proto
protoc --swagger_out=logtostderr=true:. my.proto

accordingly passing a yaml file is:

protoc --grpc-gateway_out=grpc_api_configuration=my.yaml:. my.proto
protoc --swagger_out=grpc_api_configuration=my.yaml:. my.proto

Everything between *_out= and the : after which you specify the output location is handed to the plugin as part of the request. This is a protoc feature and the gateway and swagger plugin explicitely implement support for it (see https://github.com/grpc-ecosystem/grpc-gateway/blob/master/protoc-gen-grpc-gateway/main.go#L60 ff.). You can pass an arbitrary number of comma separate param=value which are interpreted like commandline parameters would be. No special magic needed.

Works a treat.

hacst · 2018-02-15T21:17:57Z

With regards to this PR in general: It would be great to receive some guidance on what would be needed to make this ready to land. E.g. is the current implementation approach acceptable and what kind of tests and documentation is required for people to be comfortable landing this. Thanks.

achew22

With regards to this PR in general: I would be great to receive some guidance on what would be needed to make this ready to land. E.g. is the current implementation approach acceptable and what kind of tests and documentation is required for people to be comfortable landing this. Thanks.

I think the code here is basically good to go. I have a few questions that are in the review. I think the strategy we have adopted thus far is to add an E2E test through the make examples of the makefile. Make a new proto that is completely devoid of annotations in the examples dir, and create a .yaml to go with it that exercises every option you have made available. Once you do that you should be able to run the generator against it and generate a swagger client (in the clients dir) and then use the generated swagger client to call a trivial implementation of the service backing your proto. I know that sounds like a lot of work, I'm sorry. If you run into any problems there please don't hesitate to bother me. With that level of testing I'll be confident that this will work for years to come without an accidental regression.

WRT documentation, I would like you to write a new section at the end of the README titled # Generating for unannotated protos (or something like that) and try to put enough words in there that a person reasonably well acquainted with the proto ecosystem could use it. Your examples provided in previous comments would be a great place to start.

Thanks so much! Also, since moving goalposts are the absolute worst (especially when you're adding a feature), once you have addressed these concerns we will merge this. I may ask you to do a follow-up PR with something else, but the scope on this will not grow.

achew22 · 2018-02-16T02:15:29Z

protoc-gen-grpc-gateway/descriptor/grpc_api_configuration.go

+// Note that for the purposes of the gateway generator we only consider a subset of all
+// available features google supports in their service descriptions and hence we do not
+// bother having the full service protobuf which has a lot of other dependencies. Thanks
+// to backwards compatibility this should be relatively safe.


"Thanks to backwards compatibility" doesn't seem to follow from the rest of the comment. It seems like "thanks to structural typing" may be a better way to phrase that?

I actually meant the backwards compatibility guarantees given by protobuf there. As in you can just remove fields as long as numbering stays intact. You are of course correct that I also have to rely on the "interface" of the protobuf generator for golang. Will try to clarify the comment.

achew22 · 2018-02-16T02:16:52Z

protoc-gen-grpc-gateway/descriptor/grpc_api_configuration.go

+	}
+
+	// As our GrpcAPIService is incomplete accept unkown fields.
+	unmarshaler := jsonpb.Unmarshaler{


This is the same configuration as the default global unmarshaller. Is it important to keep them in sync or are you just using the unmarshaller to load the data into a proto shaped object for use during generation?

This is just for loading the httprule during generation time. It is independent from the gateway runtime marshalling/unmarshalling of JSON protobuf messages received/sent through the gateway. It didn't seem prudent to bind one to the other as the requirements might be different or diverge.

achew22 · 2018-02-16T02:17:21Z

protoc-gen-grpc-gateway/descriptor/grpc_api_configuration.go

+	}
+
+	// As our GrpcAPIService is incomplete accept unkown fields.
+	unmarshaler := jsonpb.Unmarshaler{


Can you confirm for me that this is only used during the protoc stage and not during runtime? If it is used during runtime, can you describe the circumstances when it is used?

Nothing of this is running during grpc-gateway runtime. Generation time only. I don't think any code outside of the template strings and runtime can even end up in the code of the actual gateway or am I missing something?

achew22 · 2018-02-16T02:20:38Z

protoc-gen-grpc-gateway/descriptor/grpc_api_configuration.go

+)
+
+// GrpcAPIService represents a stripped down version of google.api.Service .
+// Compare to https://github.com/googleapis/googleapis/blob/master/google/api/service.proto


Could you add a comment in here that the file you're avoiding importing has 23 import statements and that is why it's being avoided as well as a note that says "If a significant subset of these (>50%) are being reproduced in this file, swap it out for the generated version.

achew22 · 2018-02-16T02:21:02Z

protoc-gen-grpc-gateway/descriptor/grpc_api_configuration.go

+	"google.golang.org/genproto/googleapis/api/annotations"
+)
+
+// GrpcAPIService represents a stripped down version of google.api.Service .


Can you move this into its own file?

achew22 · 2018-02-16T02:27:21Z

protoc-gen-grpc-gateway/descriptor/grpc_api_configuration_test.go

+	if len(second.GetAdditionalBindings()) != 0 {
+		t.Errorf("some.other.service has %v additional bindings when it should not have any. Got: %v", len(second.GetAdditionalBindings()), second.GetAdditionalBindings())
+	}
+}


Solid tests. Good work!

achew22 · 2018-02-16T02:28:04Z

protoc-gen-grpc-gateway/descriptor/grpc_api_configuration.go

+			return fmt.Errorf("Selector '%v' in %v must specify a single service method without wildcards", rule.GetSelector(), sourceLogName)
+		}
+
+		registry.externalHTTPRules[selector] = append(registry.externalHTTPRules[selector], rule)


This feels like something that should be added to the public interface of the registry rather than mucking around in the internals of the registry object

achew22 · 2018-02-16T02:29:02Z

protoc-gen-grpc-gateway/descriptor/registry.go

@@ -205,6 +210,11 @@ func (r *Registry) LookupFile(name string) (*File, error) {
 	return f, nil
 }

+// LookupExternalHTTPRules looks up external http rules by fully qualified service method name
+func (r *Registry) LookupExternalHTTPRules(qualifiedMethodName string) []*annotations.HttpRule {


You have a getter for external http rules, I think you should add a setter and use it instead of direct manipulation.

achew22 · 2018-02-16T02:29:39Z

protoc-gen-grpc-gateway/descriptor/services.go

@@ -49,7 +53,7 @@ func (r *Registry) loadServices(file *File) error {
 	return nil
 }

-func (r *Registry) newMethod(svc *Service, md *descriptor.MethodDescriptorProto, opts *options.HttpRule) (*Method, error) {
+func (r *Registry) newMethod(svc *Service, md *descriptor.MethodDescriptorProto, optss []*options.HttpRule) (*Method, error) {


s/optss/options/

Let's be explicit that we are talking about plural of opts instead of it being a typo of opts.

achew22 · 2018-02-16T02:34:17Z

protoc-gen-grpc-gateway/gengateway/generator.go

@@ -103,7 +102,7 @@ func (g *generator) generate(file *descriptor.File) (string, error) {
 	for _, svc := range file.Services {
 		for _, m := range svc.Methods {
 			pkg := m.RequestType.File.GoPkg
-			if m.Options == nil || !proto.HasExtension(m.Options, options.E_Http) ||
+			if len(m.Bindings) == 0 ||


Can you help me understand why this is preferential? Could you also add a test demonstrating why this is preferred

This is from 3356bb1 :

Previously the generator generator checked whether the http extension
was used to decide whether imports were required for a method. This
patch makes it check whether a method has bindings instead as those
are what code is actually generated for.

Basically the previous check skipped adding imports when the protobuf file wasn't using the Http annotation. That check is done on data protoc passed from the proto file. Since the whole point of this PR is to no longer have the Http annotations in the proto this check had to be amended. Now it checks whether we created bindings for the method (it should have a binding for each endpoint). This will happen based on annotation or the yaml.

What you see without this change is that the generated .gw.go file might be missing imports (e.g. if the method uses google.protobuf.Duration for which an import is required). In that case it just won't compile.

I will see if I can do a test for that.

ensonic · 2018-04-17T09:50:30Z

Any update?

hacst · 2018-04-18T18:00:17Z

@ensonic Unfortunately I have been pretty busy lately. Will try to finish this up this weekend.

Separate gRPC API Configuration files as described by google at https://cloud.google.com/endpoints/docs/grpc/grpc-service-config can be used to separate the description of the basic API surface in .proto files from their mapping to HTTP REST. In grpc-gateway this allows exposing gRPC services without having to add any annotations to their .proto files. The format of gRPC API Configuration YAML files is defined by the proto message schema google.api.service which can be found as service.proto in the googleapis repository. This patch exposes the Http part of the service configuration to the gateway generator. To enable this all HttpRules contained in the Http part of the description are loaded into the generator registry as a map from service method to HttpRules called externalHttpRules. During loading of the proto files we can now use these externalHttpRules to look for additional rules to apply to a service method in addition to the ones from the annotations found in the proto file. As the types are the same no fundamentally new codepaths are required in either of the generators. google.api.service is quite a complex protobuf message with lots of dependencies on other google api. To not have to pull in all other dependent types this patch constructs a reduced service message type with only the Http field. This relies on protobufs backwards compatibility guarantees which allows us to remove all other fields. Also we do not need all protobuf features on this custom message. To load the actual YAML file as a protobuf message this patch uses a new dependency on github.com/ghodss/yaml to convert the YAML into JSON which we can then unmarshal into protobuf using jsonpb.

Previously the generator generator checked whether the http extension was used to decide whether imports were required for a method. This patch makes it check whether a method has bindings instead as those are what code is actually generated for.

Refactor the patch to collect external dependencies earlier so the actual option count can be checked before emitting this message.

* Extract GrpcAPIService to its own file and clarify comments on why do not generate it from the proto file * Consistently access Registry.externalHTTPRules through accessors. For this the AddExternalHTTPRule setter was added * Renamed registerGrpcAPIService to registerHTTPRulesFromGrpcAPIService * Replace use of optss as plural of opts with optsList. Cannot use suggested options because of name collision.

Explains the use of gRPC API Configuration YAML files to expose unannotated .proto files in contrast to the annotation approach.

To make this work we have to adjust the makefile to pass the additional grpc api configuration file. We also have to provide a basic server implementation and use the generated client from the tests. The e2e test is basically just the echo example but with an additional inclusion of the google Duration WKT to ensure we properly generate the imports for those even if the annotation WKT is not used.

hacst · 2018-04-23T00:57:27Z

@achew22 Ok. I think I addressed the remaining review feedback and rebased onto current master.

For the e2e test I derived a "unannotated" echo service from the echo service example. I added in a import of the duration well known type for good measure. This should break without my len(m.Bindings) == 0 change. Imho this should exercise the new codepaths sufficiently. The functionality expressable by the HTTP rule annotations is directly re-used by the PR so I see no reason to test it again in this in this E2E.

I'm not 100% happy with my README.md changes. I tried to keep it as short as possible but it still is quite a chunk of text. Maybe putting the information in a separate .md file would be better?

In the makefile I did a bit of cargo-culting from the echo service (I've never seen a ExamplepbSimpleMessage.go or git_push.sh in my tree) so there might be some clutter that ought to be removed.

Also my generation run insisted on changing some occurances of "context" imports to "golang.org/x/net/context" which I understand is a legacy construct. I was using go 1.10 and a 3.0.0 protoc so I am not quite sure whether that is something that needs fixing. It does compile.

My bazel file edits were blind as I do not have a bazel environment to use. The bazel build on travis fails due to what looks like my new dependency missing which makes a certain amount of sense. I have no idea how to get a 3rd party go dependency into a bazel build so help is welcome there.

Also the travis builds with disabled context seem to fail. This looks like a makefile issue to me but I'm not quite sure. protoc line looks like I expect it to be but the error output is strange. Like a space slipped in somewhere where it shouldn't. If you have an idea I'm all ears. Otherwise I will have to revisit this at a later date.

…onal flags

hacst · 2018-04-25T22:23:00Z

@achew22 I addressed the two remaining issues.

For the ignore files I simply copy and pasted from the other samples.

The documentation for gRPC API Configuration file usage is now a separate part of the jekyll documentation. Unfortunately the theme does not support nested categories so I had to add a new top-level item. I did another pass over the rest of the documentation to make sure the new example and the feature were mentioned where useful.

One thing I noticed was that besides the landing page none of the jekyll documentation pages where themed. See for example https://grpc-ecosystem.github.io/grpc-gateway/docs/features.html which is just black text on white background. I added another commit that fixes this by ensuring the "default" theme is used on all pages. This was the first time I had contact with jekyll though so I'm not sure if this is the right way (TM) to do things.

From my POV this PR should be ready for final review and subsequent landing. If you want commits squashed or a final rebase let me know.

With regards to Bazel: The vast majority of people finding the project will not have used it as it is not widely used outside of google OSS projects at this point. I had investigated it before for use with C++ so I had a basic idea of the general approach it is taking but I feel like it will be very unnatural and opaque for someone used to the normal go or node way of managing build and dependencies.

For me personally getting to the point where I could run the build in Bazel was quite straight-forward as you can pretty much read that directly from the .travis.yml . However what you cannot see there is how to correctly update the build files if you added files or dependencies. For example it is really not obvious where external go dependencies come from and how they fit into the bazel world unless you know where to look. Also at first I manually edited the BAZEL files to add sources and internal dependencies before figuring out that in this project "gazelle" (never heard of it before) manages that. However running the "gazelle fix" target in this project is a bit overeager and actually breaks things so you have to revert some of what it does (good thing the PR introducing it mentioned that).

TL;DR: A short list of steps mentioning the correct steps for updating the build description files would have been most helpful to me. A link to a basic bazel with go documentation (if there is something like that matching what is implemented here) would not hurt either.

achew22 · 2018-04-26T00:47:15Z

I think this is good to go! Thanks so much for all your work in here. Could you rebase this, fix the merge conflicts, and run go fix, then I will merge it?

PS: Thanks for looking into the Jekyl stuff. I'm also totally new to it so I don't know much much at all about how to fix things. I'm glad you took a look at it.

…n similar to echo_service

hacst · 2018-04-26T20:25:24Z

@achew22 This was more involved than I expected. Quite a few relevant changes since the last rebase a few days ago. I hope it is ok that I merged instead of rebased but I really didn't feel rebasing all those commits was a sane thing to do now that the examplepb folder got moved.

Added new commits for:

Adding a delete method to the unattended echo example as one was added to the stock echo one
Fix the documentation to correctly point to the new location of the examplepb folder. Seems like that was forgotten before

Hoping for no more breaking changes till we manage to land this ;)

achew22 · 2018-04-26T21:35:45Z

No more breaking changes before it gets merged -- I promise 😉. Sorry about all the relevant changes.

Thanks so much for your contribution! This is really great work and I look forward to doing more of this with you going forward. Please, don't be a stranger this was fun and I hope you had fun too. Plus new features for everyone 🎉!

hacst · 2018-04-26T21:50:37Z

Awesome. Thanks a lot for the guidance along the way. Having someone so responsive handle PRs is really awesome for a first time contributor to a project.

yugui · 2018-04-27T00:19:23Z

Thank you for your contribution, hacst. This feature is what I really wanted to have.

no more breaking changes

Sorry, the breaking changes on examples are my fault.

hacst · 2018-04-27T16:35:51Z

@yugui No worries. Pretty much my own fault for letting this sit so long ;)

PR grpc-ecosystem#521 added support for grpc service configurations. Add an option to let users specify this config in the bazel rule. Also add a simple doc blob to the rule.

ensonic · 2018-04-30T09:32:35Z

Now from reading the code, this seems to not do what I think it would :/

We have a proto with e.g.

service AbcService {
  rpc CreateAbc(Abc) returns (Abc) {
    option (google.api.http) = {
      post: "/apis/abc/v1/abcs"
      body: "*"
    };
  }

in the grpc service config, we have:

type: google.api.Service
config_version: 3
name: abc.endpoints.${GCP_PROJECT_ID}.cloud.goog
endpoints:
- name:abc.endpoints.${GCP_PROJECT_ID}.cloud.goog
  allow_cors: true
title: Abc gRPC API
apis:
- name: domain.abc.v1alpha1.AbcService
  version: 1.0.0
authentication:
  providers:
  - id: xxx_auth
    ...
  rules:
  - selector: "*"
    allow_without_credential: false
    requirements:
    - provider_id: xxx_auth
usage:
  rules:
  - selector: "*"
    allow_unregistered_calls: true

Now with or without using grpc_api_configuration, I get the same resulting open-api.json. I was expecting:
1.) the 'title' from the grpc yaml to go to '.info.description'.
2.) security settings being configured

Should any of those work?

hacst · 2018-04-30T22:58:31Z

@ensonic No. As described in https://grpc-ecosystem.github.io/grpc-gateway/docs/grpcapiconfiguration.html this PR only implements the HTTP rule part of the specification. The goal was not having to annotate the .proto file for basic use with grpc-gateway. I have not looked into how other parts of gRPC API Configurations might be mapped to grpc-gateway, though I am sure there are some that could have meaningful mappings.

…632) * Add support for the grpc_api_configuration option in the bazel rule. PR #521 added support for grpc service configurations. Add an option to let users specify this config in the bazel rule.

…em#521) * Add support for basic gRPC API Configuration YAML files Separate gRPC API Configuration files as described by google at https://cloud.google.com/endpoints/docs/grpc/grpc-service-config can be used to separate the description of the basic API surface in .proto files from their mapping to HTTP REST. In grpc-gateway this allows exposing gRPC services without having to add any annotations to their .proto files. The format of gRPC API Configuration YAML files is defined by the proto message schema google.api.service which can be found as service.proto in the googleapis repository. This patch exposes the Http part of the service configuration to the gateway generator. To enable this all HttpRules contained in the Http part of the description are loaded into the generator registry as a map from service method to HttpRules called externalHttpRules. During loading of the proto files we can now use these externalHttpRules to look for additional rules to apply to a service method in addition to the ones from the annotations found in the proto file. As the types are the same no fundamentally new codepaths are required in either of the generators. google.api.service is quite a complex protobuf message with lots of dependencies on other google api. To not have to pull in all other dependent types this patch constructs a reduced service message type with only the Http field. This relies on protobufs backwards compatibility guarantees which allows us to remove all other fields. Also we do not need all protobuf features on this custom message. To load the actual YAML file as a protobuf message this patch uses a new dependency on github.com/ghodss/yaml to convert the YAML into JSON which we can then unmarshal into protobuf using jsonpb. * Add a "Generating for unannotated protos" section to README.md Create a new documentation page describing how to use grpc-gateway without annotations by the use of gRPC API Configuration files. This new section explains the use of gRPC API Configuration YAML files to expose unannotated .proto files in contrast to the annotation approach. Adjusted other parts of the documentation to hint/link at that capability. * Add unannotated example for e2e testing The e2e test is basically just the echo example but with an additional inclusion of the google Duration WKT to ensure we properly generate the imports for those even if the annotation WKT is not used. To make this work we have to adjust the makefile to pass the additional grpc api configuration file. We also have to provide a basic server implementation and use the generated client from the tests.

…rpc-ecosystem#632) * Add support for the grpc_api_configuration option in the bazel rule. PR grpc-ecosystem#521 added support for grpc service configurations. Add an option to let users specify this config in the bazel rule.

achew22 requested a review from tmc January 21, 2018 03:47

achew22 mentioned this pull request Feb 13, 2018

Document protoc-gen-swagger #427

Open

googlebot added the cla: yes label Feb 15, 2018

achew22 requested changes Feb 16, 2018

View reviewed changes

hacst and others added 7 commits April 23, 2018 00:06

Fix "Found non-target method" log message with external http options

d3a0798

Refactor the patch to collect external dependencies earlier so the actual option count can be checked before emitting this message.

Add a "Generating for unannotated protos" section to README.md

8300cdc

Explains the use of gRPC API Configuration YAML files to expose unannotated .proto files in contrast to the annotation approach.

Changes from 'make examples' run

4f5a208

hacst force-pushed the grpc-api-configuration branch from a4776de to bf613d9 Compare April 23, 2018 00:12

Update BAZEL files

98be4c6

hacst force-pushed the grpc-api-configuration branch from bf613d9 to 98be4c6 Compare April 23, 2018 00:38

hacst added 2 commits April 23, 2018 20:51

Makefile no longer incorrectly adds a space when concatenating additi…

b7ed608

…onal flags

Selective gazelle run to fix BAZEL build

f1a1d75

hacst added 5 commits April 26, 2018 22:12

Merge remote-tracking branch 'origin/master' into grpc-api-configuration

ccfa95d

Also add EchoDeletee method to unannotated_echo_service so it is agai…

4d0a037

…n similar to echo_service

Update generated files

2ecc24d

Documentation was not updated when example protos were moved

778f104

Use "context" in unannotatedecho.go

acc027a

achew22 approved these changes Apr 26, 2018

View reviewed changes

achew22 merged commit 5ae7f11 into grpc-ecosystem:master Apr 26, 2018

ensonic mentioned this pull request Apr 27, 2018

feat(bazel): Add rule for generating .swagger.json files #613

Merged

hacst deleted the grpc-api-configuration branch April 27, 2018 16:49

ensonic mentioned this pull request Apr 30, 2018

Add support for the grpc_api_configuration option in the bazel rule. #632

Merged

ivucica mentioned this pull request Jul 24, 2018

YAML support for Swagger annotations #701

Closed

shi-yang mentioned this pull request Jul 9, 2022

middleware: middleware registry & selector config go-kratos/kratos#2011

Closed

Add support for basic gRPC API Configuration YAML files #521

Add support for basic gRPC API Configuration YAML files #521

Conversation

hacst commented Jan 14, 2018 • edited Loading

googlebot commented Jan 14, 2018

hacst commented Jan 14, 2018

googlebot commented Jan 14, 2018

achew22 commented Jan 21, 2018

tmc commented Jan 21, 2018

hacst commented Jan 21, 2018

achew22 commented Jan 21, 2018

hacst commented Jan 21, 2018

achew22 commented Feb 13, 2018

hacst commented Feb 15, 2018 • edited Loading

hacst commented Feb 15, 2018 • edited Loading

achew22 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ensonic commented Apr 17, 2018

hacst commented Apr 18, 2018

hacst commented Apr 23, 2018 • edited Loading

hacst commented Apr 25, 2018 • edited Loading

achew22 commented Apr 26, 2018

hacst commented Apr 26, 2018

achew22 commented Apr 26, 2018

hacst commented Apr 26, 2018

yugui commented Apr 27, 2018

hacst commented Apr 27, 2018

ensonic commented Apr 30, 2018

hacst commented Apr 30, 2018 • edited Loading

hacst commented Jan 14, 2018 •

edited

Loading

hacst commented Feb 15, 2018 •

edited

Loading

hacst commented Feb 15, 2018 •

edited

Loading

hacst commented Apr 23, 2018 •

edited

Loading

hacst commented Apr 25, 2018 •

edited

Loading

hacst commented Apr 30, 2018 •

edited

Loading