Merged swagger outputs only require info of first proto file that is input

[llsj14]

🐛 Bug Report

Thanks for the great help and guide, I could merge swagger outputs of different services.

By the way, the problem is that the merged output only shows the service info of the proto file that is used as the first input. If I don't write down service info of first proto file into .swagger.yaml file, swagger output (.swagger.json) does not show any info.
I'm curious about the intended behavior and the best practice. For example, if I have many services in an API group, and let's assume that new proto files are added, I should be aware of the first proto file(first in the alphabetic order) that is used for making swagger output. Moreover, the first proto file is not always a neutral one that represents whole proto files in an API group.

To Reproduce

There are a.proto, b.proto, c.proto

If swagger.yaml doesn't describe a.proto file like the following examples, any info will not show up in the swagger.json file.

openapiOptions:
  file:
  - file: "api/v1/b.proto"
    option:
      info:
        title: API
        version: "v1"
      schemes:
      - HTTPS
      - HTTP
      consumes:
      - application/json
      produces:
      - application/json

openapiOptions:
  file:
  - file: "api/v1/a.proto"
  - file: "api/v1/b.proto"
    option:
      info:
        title: API
        version: "v1"
      schemes:
      - HTTPS
      - HTTP
      consumes:
      - application/json
      produces:
      - application/json
  - file: "api/v1/c.proto"

Expected behavior

While merging swagger output, every possible service info is showed up

Actual Behavior

While merging swagger output, only service info that is input as the first proto file is showed up

Your Environment

macOS
go1.16 darwin/amd64

[johanbrandhorst]

Hi, thanks for the issue! This is annoying yes, but it's also not clear what the behavior should be. Merging all the info from all the files would potentially cause duplicate information to appear in the merged file. It would make sense for us to use the info from the first file found, so that any file could contain the information, don't you think?

If you'd like to help contribute a fix to this, feel free to start by creating an error case. We have a merge test proto here: https://github.com/grpc-ecosystem/grpc-gateway/blob/master/examples/internal/proto/examplepb/openapi_merge.swagger.json, you could try annotating conflicting information in https://github.com/grpc-ecosystem/grpc-gateway/blob/master/examples/internal/proto/examplepb/openapi_merge_a.proto and https://github.com/grpc-ecosystem/grpc-gateway/blob/master/examples/internal/proto/examplepb/openapi_merge_b.proto and iterate on your fix.

[llsj14]

I agree with the way to block duplicated information to be appeared in the merged file and to use the info from the first file found. I'm also relieved that I didn't use your program in a wrong way.
However, how about just giving chance to write specific proto file name(which is not first file found) on the swagger.yaml file and make it possible to use that information? Thinking about the situation where you add new proto files in an API group, I guess it is better to write down a specific proto file name, which is quite representative or neutral on the swagger.yaml file and use that information.

If you like this way as a good solution, I would like to contribute a fix as you guided.

[johanbrandhorst]

However, how about just giving chance to write specific proto file name(which is not first file found) on the swagger.yaml file and make it possible to use that information?

I'm not sure what you mean by this. Do you mean adding a new option to protoc-gen-openapiv2? Something like merge_file_canonical_source=./the/specific/file.proto? I would really like to avoid adding more options, we already have too many and they're hard to understand for new users.

I think the most obvious algorithm for the information is to pick the first time some information is populated in any of the files. That would allow users to pick a single file to add all the annotations to. If they annotate more than one file, whichever one is processed first will win, which kind of sucks but adding an error or something would be a breaking change so we don't have much choice there.

What do you think?

[llsj14]

I didn't think about a new option and I also think it is complex. How about just making users pick a single file to add the information on the .swagger.yaml file. At this moment, there is no chance of that.

Let's say there's a.proto and b.proto. If I wanted to pick b.proto as representative proto file on the .swagger.yaml file and describe information about it, .swagger.json file will not give any information because it is not the first file in alphabetic order. I think making protoc-gen-openapiv2 use the information of first file on .swagger.yaml file will be enough.

• as-is

<api.swagger.yaml>
openapiOptions:
  file:
  - file: "api/v1/b.proto"
    option:
      info:
        title: API
        version: "v1"

<api.swagger.json>
  "swagger": "2.0",
  "info": {
    "title": "api/v1/a.proto",
    "version": "version not set"
  },

• to-be

<api.swagger.yaml>
openapiOptions:
  file:
  - file: "api/v1/b.proto"
    option:
      info:
        title: API
        version: "v1"

<api.swagger.json>
  "swagger": "2.0",
  "info": {
    "title": "API",
    "version": "v1"
  },

[johanbrandhorst]

Now I'm really confused. Are you talking about our external openapi configuration file option? Under normal operation, there is no api.swagger.yaml as an input to the operation. When using the openapi_configuration option, you can specify your annotations in an external file rather than as annotations in the protobuf file. Of course, in this scenario, the settings in the openapi_configuration should be respected, but it's not clear to me that they should take precedent over annotations in the files themselves. What happens if annotations are present in one of the files and you use the external configuration file?

External openapi configuration file example: https://github.com/grpc-ecosystem/grpc-gateway/blob/master/examples/internal/proto/examplepb/unannotated_echo_service.swagger.yaml
Internal openapi annotation example: https://github.com/grpc-ecosystem/grpc-gateway/blob/master/examples/internal/proto/examplepb/a_bit_of_everything.proto

[llsj14]

Oh, I'm sorry, if I made you confused. I only used your external openapi configuration file option. Because I didn't know your internal openapi annotation. I made our Makefile as below without any internal openapi annotaion.

%.swagger.json: $(dir %)*.proto %.swagger.yaml | grpc
	protoc --openapiv2_out . \
	       --openapiv2_opt logtostderr=true \
	       --openapiv2_opt openapi_configuration=$*.swagger.yaml \
	       --openapiv2_opt visibility_restriction_selectors=INTERNAL \
	       --openapiv2_opt allow_merge=true \
	       --openapiv2_opt merge_file_name=$* \
	       $(dir $*)*.proto \

I did the experiment you told me about. Inserting an internal annotation into one of the files takes precedence over the external openapi configuration. Therefore, I can use internal annotations to put information options in a specific file. Maybe I can just use this as a solution. Thank you!

In summary, if I only use an external openapi configuration, I cannot use the information of a specific file in the yaml file (even though it is the only information in yaml), because only the information in the alphabetically first file can be used.

[johanbrandhorst]

Glad you found something that works for you. Would you be willing to contribute a documentation update to help other users? We document the merge file behavior here: https://github.com/grpc-ecosystem/grpc-gateway/blob/master/docs/docs/mapping/customizing_openapi_output.md#merging-output. Might be worth adding a small section about the merge priority.

[llsj14]

Yes, I would be willing to contribute a documentation update. Thank you.