Add support for OpenAPI YAML annotations

[jasonewang]

References to other Issues or PRs

Fixes #701

Have you read the Contributing Guidelines?

Yep

Brief description of what is fixed or changed

Added support for passing OpenAPI/Swagger annotations in YAML file to protoc-gen-openapiv2

• Extended annotations.proto to include OpenAPI option messages
• Extended descriptor.Registry to hold OpenAPI options
• Extended template to look up options in Registry if proto descriptors don't have options

Other comments

Extended template tests that tested proto options to set options via registry.

[jasonewang]

I think this is ready for review, though I can't quite figure how to fix the generate CI test.

[johanbrandhorst]

This PR is an amazing achievement! Well done! I'd like to see an example of these new annotations in action, do you think you could add some annotations to the unannotated_echo_service.yaml to show users how to use these new annotations, and as a test to see that it generates correctly?

The generation error is because of a dependency issue in the generation job - make realclean deletes all the generated files, and then make examples tries to install the generators before regenerating the files - but now the grpc-gateway generator depends on the generated openapi options, so we can't build it. We'll probably have to split up cleaning and building, so that we can still have the confidence that the generated files are the same as are checked into the repo (by deleting and regenerating). There is a bit of an interesting chicken-and-egg problem here though in that we can't be sure that the files we regenerated are correct until we've regenerated them and reinstalled the generators and generated them again, since what if the generator was built with an old generated file?

Another option which is tempting is to remove the dependency on protoc-gen-openapiv2 options from protoc-gen-grpc-gateway. This seems a bit of a weird dependency anyway, and the part of apiconfig that needs the new options is actually unused by protoc-gen-grpc-gateway. We may need to split the packages.

What do you think?

[jasonewang]

...do you think you could add some annotations to the unannotated_echo_service.yaml to show users how to use these new annotations, and as a test to see that it generates correctly?

Sure. Should I also add options inline to echo_service.proto to maintain parity between the annotated and unannotated versions for the messages and services that are in both the annotated and unannotated protos?

Another option which is tempting is to remove the dependency on protoc-gen-openapiv2 options from protoc-gen-grpc-gateway. This seems a bit of a weird dependency anyway, and the part of apiconfig that needs the new options is actually unused by protoc-gen-grpc-gateway. We may need to split the packages.

Splitting the package up makes sense to me. Would moving the OpenAPI config options to github.com/grpc-ecosystem/grpc-gateway/v2/internal/descriptor/openapiconfig" suffice?

[jasonewang]

Looks like protoc-gen-grpc-gateway pulls in descriptor.Registry which depends on "internal/descriptor/openapiconfig/openapiconfig.pb.go" and thus "github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2/options", so splitting out the package doesn't have the desired effect of breaking the dependency.

I'm a bit confused at how protoc-gen-openapiv2 is being built because that plugin definitely requires the package github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2/options.

[jasonewang]

I'm a bit confused at how protoc-gen-openapiv2 is being built because that plugin definitely requires the package github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2/options.

Turns out that make realclean deletes the Go stubs at github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2/options. Adding those stubs as a dependency for protoc-gen-grpc-gateway (https://github.com/grpc-ecosystem/grpc-gateway/pull/1665/files#diff-b67911656ef5d18c4ae36cb6741b7965R160) solves it.

[johanbrandhorst]

The final step would be to add a little blurb about these new capabilities to the docs - we already have a page for the grpc api configuration and it would make sense to add this to that page. The docs can be found in the /docs subfolder, and are simple markdown documents.

Thanks again for this work, it's really appreciated!

[johanbrandhorst]

Hi, I merged a generator update so you'll need to rebase on v2, sorry for the inconvenience 😬.

[jasonewang]

Hi, I merged a generator update so you'll need to rebase on v2, sorry for the inconvenience 😬.

No problem! I rebased and updated the docs.

[johanbrandhorst]

Just some small thoughts on the new docs and how we want to display the protoc invocations. Great work on this Jason!

[jasonewang]

What do you think about moving openapiconfig.proto to an exported package instead of under internal? My idea is that it would make generating OpenAPI annotations easier if you could fill out the struct in Go, then marshal to JSON and then to YAML instead of having to work with templates.

[johanbrandhorst]

What do you think about moving openapiconfig.proto to an exported package instead of under internal? My idea is that it would make generating OpenAPI annotations easier if you could fill out the struct in Go, then marshal to JSON and then to YAML instead of having to work with templates.

I'd like to limit our API surface as much as possible. I see the benefit of what you're saying, but I'd be more inclined to let users copy the generated struct for their internal use.

[johanbrandhorst]

This is great, thank you so much for your work!

[johanbrandhorst]

I'm going to make another beta release of v2 once we've cherry picked the new unbound method generator, we've had some incredible contributions lately, yours definitely among them!

[jasonewang]

Thanks for being so responsive to review requests, Johan! Happy to contribute.