Skip to content

protoc-gen-openapiv3: add annotations#6681

Open
johanbrandhorst wants to merge 1 commit intomainfrom
jbrandhorst/custom-annotations-v3
Open

protoc-gen-openapiv3: add annotations#6681
johanbrandhorst wants to merge 1 commit intomainfrom
jbrandhorst/custom-annotations-v3

Conversation

@johanbrandhorst
Copy link
Copy Markdown
Collaborator

Fixes #6674

@johanbrandhorst
Copy link
Copy Markdown
Collaborator Author

CC @aidandj

@aidandj
Copy link
Copy Markdown
Contributor

aidandj commented Apr 22, 2026

This one may take me a bit to review. How much of it comes from the old PR? We worked through a number of possibilities there.

syntax = "proto3";

// Package grpc.gateway.protoc_gen_openapiv3.options defines the minimal set of
// OpenAPI 3.1.0 override messages consumed by protoc-gen-openapiv3.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we be differentiating 3.1 and 3.x anywhere?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can keep this 3.1.0 specific right now and evaluate the need for 3.x down the road. It might even be a different generator, maybe we rename this openapiv31 or something.

// gRPC-Gateway OpenAPI v3 generator. All extensions in this file share the
// same ID because they each extend a different descriptor message.
// Ref: https://github.com/protocolbuffers/protobuf/commit/5e0f68530ae6ad47b78714e60861321e1d1c04ae
Operation openapiv3_operation = 1295;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Random thought. But if the v3_operation message wrapped a v3_1_operation message. Then we could support incompatible changes in 3.x without a new extension number.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eh new extensions are not that big of a deal, I assume users will only care about targeting one version of OpenAPI at a time?

@johanbrandhorst
Copy link
Copy Markdown
Collaborator Author

This one may take me a bit to review. How much of it comes from the old PR? We worked through a number of possibilities there.

None of it as far as I know - I wasn't aware, sorry. This is an intentionally minimal list.

@aidandj
Copy link
Copy Markdown
Contributor

aidandj commented Apr 22, 2026

Ok I see it now. The title made me think it was all of them.

@johanbrandhorst johanbrandhorst force-pushed the jbrandhorst/custom-annotations-v3 branch from cd9db15 to e5faae2 Compare April 22, 2026 04:59
// usage of the declared operation. Default value is false. Setting this to
// false does not un-deprecate a method that is marked deprecated in proto
// (`option deprecated = true` on the method, service, or file).
bool deprecated = 6;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know you are going for simple initial implementation. But operation has a repeated servers parameter. And since you already defined Servers for the main doc, it would be a simple addition.

string title = 1;
// A string providing explanation about the purpose of the instance
// described by the schema.
string description = 2;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If adding deprecated to operation, it should be added here as well.

string summary = 2;
// A description of the API. CommonMark syntax MAY be used for rich text
// representation.
string description = 3;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the openapi spec, only title and version are required. Should other fields be marked optional so that they can be omitted from the json if not set?

string name = 1;
// An SPDX expression for the API. The identifier field is mutually
// exclusive of the url field.
string identifier = 2;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These could be a oneof. I like the idea of enforcing these things at the proto level, although I understand theres some reason to keep it simpler.

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

Successfully merging this pull request may close these issues.

Spec override

2 participants