google / gnostic

A compiler for APIs described by the OpenAPI Specification with plugins for code generation and other API support tasks.
Apache License 2.0
2.1k stars 249 forks source link

Support buf build deps #337

Open renyijiu opened 2 years ago

renyijiu commented 2 years ago

In the example code of protoc-gen-openapi, I see that option (openapi.v3.documentation) is already supported, but when using it in the proto file of other projects, the user needs to manually introduce import "openapiv3/annotations.proto"; associated annotations.proto file to be able to compile successfully. Can you consider uploading it to buf.build, similar to https://buf.build/googleapis/googleapis, to make it easier for other users to introduce dependencies for use, thanks to this great project.

#buf.yaml

version: v1
deps:
    - buf.build/googleapis/googleapis
lint:
  use:
    - DEFAULT
breaking:
  use:
    - FILE
jeffsawatzky commented 2 years ago

I would also like to have this, though as far as I know https://buf.build/googleapis/googleapis is actually maintained by buf themselves, and not google. I'm not sure if google has, or ever will, provide stuff on buf.

Though, given that @timburks gave your request a thumbs up, I remain hopeful that this might actually happen. :)

timburks commented 2 years ago

I've been aware of buf but not using it. I'm not surprised that the googleapis repo on buf is maintained by buf directly, but I've been curious about buf so this seems like a good opportunity to try it. I've signed into buf and found that I could create a gnostic organization. Looking at googleapis as an example, it seems that we would want to have a "gnostic" repo with subdirectories for "openapiv3", etc.

I've uploaded a draft repo to buf.build/gnostic/gnostic. Please take a look and, if you're a buf user, please try it and comment here. To pass the buf linter, I made some changes to file names and paths that you can review at timburks/gnostic/tree/buf/buf.

jeffsawatzky commented 2 years ago

I think the change in the package name is good (as I think it follows proto naming conventions more closely), though it means that we can't use those options with protoc-gen-openapi anymore. Is your intention that these would be the new package names going forward and used everywhere?

renyijiu commented 2 years ago

I add deps in buf.yaml

# buf.yaml

deps:
  - buf.build/gnostic/gnostic

and buf.gen.yaml is as follows.

# buf.gen.yaml
version: v1beta1
plugins:
  - name: openapi
    out: gen/openapi
    strategy: all
    opt:
      - title=Openapiv3

Add the relevant definitions to the proto file

import "gnostic/openapi/v3/annotations.proto";

option (gnostic.openapi.v3.document) = {
  info: {
    title: "Title from annotation";
    version: "Version from annotation";
    description: "Description from annotation";
    contact: {
      name: "Contact Name";
      url: "https://github.com/google/gnostic";
      email: "gnostic@google.com";
    }
    license: {
      name: "Apache License";
      url: "https://github.com/google/gnostic/blob/master/LICENSE";
    }
  }
  components: {
    security_schemes: {
      additional_properties: [
        {
          name: "BasicAuth";
          value: {
            security_scheme: {
              type: "http";
              scheme: "basic";
            }
          }
        }
      ]
    }
  }
};

Run the buf generate command, then you can generate openapi.yaml, but it does not take effect, there is no security_schemes in the yaml file. If you change it to this option (openapi.v3.document), buf generatewill report an error: unknown extension openapi.v3.schema. I don't know how to handle this, but I've provided a simple demo to test and verify.

jeffsawatzky commented 2 years ago

@renyijiu its because protoc-gen-openapi is built against a different version of the protos which use a different package name. Right now you can't use the ones on buf.build with protoc-gen-openapi...unless you build your own version of it using the protos on buf.build.

For now I've just copied the OpenApiv3 protos and annotations into its own local buf module.

yzhaoyu commented 2 years ago

I also can't use proto-gen-openapi to generate openapi.v3.document property in the openapi.yaml. Why is this?@jeffsawatzky @renyijiu

timburks commented 2 years ago

I'd like to move this repo to the proto structure in buf, or something similar that's better aligned with common practice for larger-scale proto-based projects. Because kubectl pulls protos from this repo, I've been wanting to wait until we had an alternate source for them to use for their gnostic protos, which I've just set up in github.com/google/gnostic-models -- I've previously gotten complaints about changes despite encouraging clients to used tagged versions, so this isolated distribution seemed best, and also addresses concerns about dependencies discussed in #317.

I've passed information about the new repo to the kubectl team, and when that gets updated, I think we can safely restructure the protos in this repo.

rauanmayemir commented 2 years ago

I just tried this and really love it. I wasn't able to use protoc-gen-openapi as part of buf generate pipeline and had to roll 'protoc' manually.

Could protoc-gen-openapi be published as a buf plugin? Then generating openapi specs would be as trivial as:

# buf.gen.yaml
plugins:
- remote: buf.build/gnostic/plugins/openapiv3:v0.6.8
  out: gen/openapi
  opt:
  - fq_schema_naming=true
  - naming=proto
  - enum_type=string
mgebundy commented 2 years ago

This is largely working on my end with the buf.build repo, thanks @timburks

However, since it hasn't been updated in a while it doesn't have this fix, which is causing issues for me. https://github.com/google/gnostic/pull/344 any chance the fork can be updated with the main branch here?

What would it take to get this to a more mature state? My team is starting to get into Buf pretty heavily, and generating OpenAPI v3 schemas would be really helpful.

ericdstein commented 1 year ago

@timburks any chance you can push the latest updates to https://buf.build/gnostic/gnostic? Similar to the previous comment, I believe I'm running into an issue fixed by https://github.com/google/gnostic/pull/344.

When I execute buf generate I get an invalid import in the generated protobuf files.

import (
    _ "./openapiv3"
    _ "google.golang.org/genproto/googleapis/api/annotations"
...

The correct import is

import (
    _ "github.com/google/gnostic/openapiv3"
    _ "google.golang.org/genproto/googleapis/api/annotations"
...

I am getting around this currently by sed replacing the import post generation.

jeffsawatzky commented 1 year ago

@timburks I noticed that when we install protoc-gen-openapi using:

go get github.com/google/gnostic/cmd/protoc-gen-openapi

We can use the proto definitions from buf, the ones that use gnostic.openapi.v3 as the package name, for the options in the protocol file (so I can add extra options).

However, looking at this repo, it is still using the openapi.v3 package name. So I am very confused as to how this is working for us. Did you make a special build using updated packages? I'm clearly missing something.

EDIT: My best guess is that since we are using buf to generate the openapi, and buf uses the buf.build definitions to pass to protoc, that protoc is able to parse the proto file using the buf package names, and when the openapi plugin gets the input it is already pre-parsed and uses something other than the package name to look up the extensions...like the field id perhaps?

In other words, here where it looks up the extensions: https://github.com/google/gnostic/blob/main/cmd/protoc-gen-openapi/generator/generator.go#L124 It does so by something other than package name, perhaps the Field 1143 from here: https://github.com/google/gnostic/blob/main/openapiv3/annotations.pb.go#L75

timburks commented 1 year ago

@jeffsawatzky I don't recall making any special builds. I think these other definitions are working because their field numbers and types are the same, so deserializing with those protos works the same as the originals. AFAIK, the package name isn't used anywhere in the serialized protos (i.e. the binaries often stored as .pb files).

timburks commented 1 year ago

Catching up (and being dense), assuming the go_package change in the two protos in #344 is what we need to push to buf...

jrc2139 commented 1 year ago

Yeah I've been using tx7do's bsr instead and it works fine:

https://buf.build/tx7do/gnostic/file/main:gnostic/openapi/v3/openapiv3.proto#L45

timburks commented 1 year ago

How's this? https://buf.build/timburks/gnostic-test

If this looks good I'll push it to gnostic/gnostic

timburks commented 1 year ago

I'll take that as LGTM - pushed to https://buf.build/gnostic/gnostic. All four protos were updated.

cfstras commented 7 months ago

Sorry to dig this out -- I've been working with, and noticed reflection does not work in my buf setup (mostly using https://github.com/SchwarzIT/go-template):

$ grpcurl -plaintext -vv localhost:9090 list api.v1.ApiService
Failed to list methods for service "api.v1.ApiService": Symbol not found: api.v1.ApiService
caused by: File not found: gnostic/openapi/v3/annotations.proto

after some debugging, I found out that the file needs to be imported with import "gnostic/openapi/v3/annotations.proto" according to your buf.build push.

However, in the protobuf reflection API, it registers itself as openapiv3/annotations.proto. I can verify that by using the reflection package directly:

protoregistry.GlobalFiles.FindFileByPath("openapiv3/annotations.proto") # works
protoregistry.GlobalFiles.FindFileByPath("gnostic/openapi/v3/annotations.proto") # worksn't 🙁

This seems far-fetched, but maybe we should align these import paths? It's a bit unfortunate, since I also haven't found any way yet to re-map them manually as a workaround.

jeroenrinzema commented 7 months ago

Thanks, @cfstras, this helped me debug the issue where my gRPC client was unable to connect to my server using reflect. I published the OpenAPI specs under the correct path in my buf repo (buf.build/jeroenrinzema/openapi). This dependency resolved the reflection issue on my side.

cfstras commented 7 months ago

As another workaround, I figured I can keep using gnostic if I omit the option (gnostic.openapi.v3.*) annotations from my proto definitions. This makes reflection work, and I don't particularly need the extra description fields they offer at this point.

renyijiu commented 5 months ago

I add deps in buf.yaml

# buf.yaml

deps:
  - buf.build/gnostic/gnostic

and buf.gen.yaml is as follows.

# buf.gen.yaml
version: v1beta1
plugins:
  - name: openapi
    out: gen/openapi
    strategy: all
    opt:
      - title=Openapiv3

Add the relevant definitions to the proto file

import "gnostic/openapi/v3/annotations.proto";

option (gnostic.openapi.v3.document) = {
  info: {
    title: "Title from annotation";
    version: "Version from annotation";
    description: "Description from annotation";
    contact: {
      name: "Contact Name";
      url: "https://github.com/google/gnostic";
      email: "gnostic@google.com";
    }
    license: {
      name: "Apache License";
      url: "https://github.com/google/gnostic/blob/master/LICENSE";
    }
  }
  components: {
    security_schemes: {
      additional_properties: [
        {
          name: "BasicAuth";
          value: {
            security_scheme: {
              type: "http";
              scheme: "basic";
            }
          }
        }
      ]
    }
  }
};

Run the buf generate command, then you can generate openapi.yaml, but it does not take effect, there is no security_schemes in the yaml file. If you change it to this option (openapi.v3.document), buf generatewill report an error: unknown extension openapi.v3.schema. I don't know how to handle this, but I've provided a simple demo to test and verify.

I tried it again and now openapi.yaml is generated correctly, with fields such as security_schemes is correct. Thanks the great work! a simple demo to test and verify.

menta2k commented 2 months ago

Sorry to dig this out -- I've been working with, and noticed reflection does not work in my buf setup (mostly using https://github.com/SchwarzIT/go-template):

$ grpcurl -plaintext -vv localhost:9090 list api.v1.ApiService
Failed to list methods for service "api.v1.ApiService": Symbol not found: api.v1.ApiService
caused by: File not found: gnostic/openapi/v3/annotations.proto

after some debugging, I found out that the file needs to be imported with import "gnostic/openapi/v3/annotations.proto" according to your buf.build push.

However, in the protobuf reflection API, it registers itself as openapiv3/annotations.proto. I can verify that by using the reflection package directly:

protoregistry.GlobalFiles.FindFileByPath("openapiv3/annotations.proto") # works
protoregistry.GlobalFiles.FindFileByPath("gnostic/openapi/v3/annotations.proto") # worksn't 🙁

This seems far-fetched, but maybe we should align these import paths? It's a bit unfortunate, since I also haven't found any way yet to re-map them manually as a workaround.

I have the same problem no matter what i do i keep getting

Service not found: havi.hermes.v1.TemplateService caused by: File not found: gnostic/openapi/v3/annotations.proto when i use a protoreflect (github.com/jhump/protoreflect/grpcreflect) openapi schema is generated correctly just the reflection not working and looking for the descriptor in wrong directory

Can you share your workaround ?

cfstras commented 2 months ago

I just don't import the gnostic/openapi/*.proto files, which also means I can't use the option (gnostic.openapi.v3.*) annotations (haven't needed them so far). However, it makes reflection work, since no gnostic types are involved in the schema anymore.