Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support buf build deps #337

Open
renyijiu opened this issue Apr 6, 2022 · 22 comments
Open

Support buf build deps #337

renyijiu opened this issue Apr 6, 2022 · 22 comments

Comments

@renyijiu
Copy link

renyijiu commented Apr 6, 2022

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
Copy link
Contributor

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
Copy link
Contributor

timburks commented Apr 9, 2022

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
Copy link
Contributor

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
Copy link
Author

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: "[email protected]";
    }
    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 generate will 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
Copy link
Contributor

jeffsawatzky commented Apr 10, 2022

@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
Copy link

yzhaoyu commented Apr 23, 2022

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

@timburks
Copy link
Contributor

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
Copy link

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
Copy link

mgebundy commented Sep 20, 2022

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. #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
Copy link

ericdstein commented Mar 28, 2023

@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 #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
Copy link
Contributor

jeffsawatzky commented Apr 11, 2023

@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
Copy link
Contributor

@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
Copy link
Contributor

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
Copy link

jrc2139 commented Apr 14, 2023

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
Copy link
Contributor

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

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

@timburks
Copy link
Contributor

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

@cfstras
Copy link

cfstras commented Mar 14, 2024

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
Copy link

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
Copy link

cfstras commented Apr 5, 2024

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
Copy link
Author

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: "[email protected]";
    }
    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 generate will 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
Copy link

menta2k commented Aug 21, 2024

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
Copy link

cfstras commented Aug 21, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests