protoc-gen-swagger: add support for arbitrary extensions

[srenatus]

Using google.protobuf.Value perhaps isn't the nicest solution here, but with

map<string, google.protobuf.Any> extensions = 15;

I could not find a way to specify those in a_bit_of_everything.proto
that would have passed through protoc.

Instead of

        extensions: {
                key: "x-grpc-gateway-foo"
                value {
                        string_value: "bar"
                }
        }

I have tried

        extensions: {
                key: "x-grpc-gateway-foo"
                value {
                        [type.googleapis.com/google.protobuf.StringValue] {
                                value: "bar"
                        }
                }
         }

following the example of this blog post,
but I've ended up with a protoc call stuck with futex(0x2534960, FUTEX_WAIT_PRIVATE, 2, NULL, see this backtrace: https://gist.github.com/srenatus/ba63e837673a28ba15fcef64cdb6abe0

Also, it's not clear how we'd have truely arbitrary messages, i.e. nothing like google.protobuf.*, resolved there properly....

---

This is just the first bit -- these extensions go in many places, see https://swagger.io/docs/specification/2-0/swagger-extensions/.

TODO:

• discuss the overall approach
• get ordering of extensions to be stable somehow (💭responses does it, too, I believe, and that's also map[string]something)
• extend the other objects in a similar way
  • toplevel
  • security scheme
  • info
  • paths section, individual
  • paths and operations -- ❔ I think?
  • operation parameters
  • responses
  • tags -- don't seem to be exposed in a way that allows for adding extensions
• tests

[johanbrandhorst]

This seems reasonable to me, thanks for including an example in a_little_bit_of_everything.proto. I'd like @ivucica's thoughts on this before merging though.

[srenatus]

@johanbrandhorst Ultimately, I want extensions on operations, so if @ivucica agrees with the gist of this, I'd add extensions on the other places where they're valid.

[codecov-io]

Codecov Report

Merging #1033 into master will increase coverage by 0.26%.
The diff coverage is 32.43%.

@@            Coverage Diff             @@
##           master    #1033      +/-   ##
==========================================
+ Coverage   53.62%   53.89%   +0.26%     
==========================================
  Files          40       40              
  Lines        4052     4125      +73     
==========================================
+ Hits         2173     2223      +50     
+ Misses       1674     1659      -15     
- Partials      205      243      +38

Impacted Files	Coverage Δ
protoc-gen-swagger/genswagger/types.go	0% <ø> (ø)	⬆️
protoc-gen-swagger/genswagger/generator.go	10% <0%> (-3.83%)	⬇️
protoc-gen-swagger/genswagger/template.go	56.96% <63.15%> (+2.84%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7f57073...8195bb9. Read the comment docs.

[srenatus]

started with tests. added another TODO (ordering!)

[srenatus]

generator.go got a little more complicated to cover nested things (security scheme), but it shouldn't get worse for the other parts. I hope. 😄

[srenatus]

#!/bin/bash -eo pipefail
git diff --exit-code

with no output, but exit code 1, is a bit mysterious to me 😕

[ivucica]

I agree with the switch to google.protobuf.Value, because upon medium-distance inspection, I don't see how to pass a plain string. It looks like types.googleapis.com mechanism only allows specifying proto message types. Perhaps by using types.googleapis.com/google.protobuf.Value with a string value? :)

(Having said this, I think google.protobuf.Any could still be embedded, albeit clumsily (I suppose the value field would have to be the binary-encoded value). Someone did mention using [brackets] to define the type URL, but I can't find this and I don't know how this was meant to be done.)

(Having having said this, yes, google.protobuf.Value is a better option and I like where this is going.)

[ivucica]

If you'd like me to take another look, please re-notify me when this is ready for review. Thanks!

[srenatus]

Sooo.... I've added extension points everywhere I thought they belonged. I'm not 100% certain about the stuff I haven't covered, but I'm also not entire sure about what the OpenAPIv2 docs try to tell me.

I'd be happy if this went in regardless 😅

Also needs squashing, but I kept that for post-review.

[srenatus]

Fixing the missing pb.go and the bazel rule.

[johanbrandhorst]

LGTM, @ivucica?

[johanbrandhorst]

There does seem to be an actual test failure:

 template_test.go:520: applyTemplate(descriptor.File{FileDescriptorProto:(*descriptor.FileDescriptorProto)(0xc00014cb40), GoPkg:descriptor.GoPackage{Path:"example.com/path/to/example/example.pb", Name:"example_pb", Alias:""}, Messages:[]*descriptor.Message{(*descriptor.Message)(0xc0000feaf0)}, Enums:[]*descriptor.Enum(nil), Services:[]*descriptor.Service{(*descriptor.Service)(0xc0000cbe30)}}).response.Extensions = [] want to be [{x-resp-id "resp1000"}]

[srenatus]

Hrm weirdly, those pass locally 😕 I'm looking into it...

[srenatus]

Ah 💡 found the problem, I think. Fixing incoming.

[johanbrandhorst]

Thanks, any final thoughts @ivucica?

[srenatus]

So, not rushing this, but I've got some time on my hands today, and very little of it in the near future. I'd appreciate another look @ivucica 😉

[ivucica]

LG

[johanbrandhorst]

Thanks for your contribution!