-
Notifications
You must be signed in to change notification settings - Fork 3
Add a CLI command that generates OpenAPI spec files from proto files #134
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
Conversation
erates OpenAPI spec files from proto files
✅ Deploy Preview for docs-extensions-and-macros ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthrough
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant DT as doc-tools (CLI)
participant BO as bundle-openapi.js
participant Git as Git Repo
participant Buf as buf
participant FS as FS (temp/out)
participant B as Bundler (redocly|swagger-cli)
Dev->>DT: doc-tools generate bundle-openapi --tag --surface [...]
DT->>BO: bundleOpenAPI(options)
BO->>Git: clone repo@tag to tempDir
BO->>Buf: buf generate (protos -> OpenAPI fragments)
BO->>FS: discover fragments by surface (admin/connect)
BO->>FS: create entrypoint (merge fragments if needed)
BO->>BO: detectBundler()
BO->>B: runBundler(entrypoint -> bundled.yaml)
B-->>BO: bundled file
BO->>FS: postProcessBundle (normalize metadata, sort keys)
BO-->>DT: paths to outputs
DT-->>Dev: Completed
note over BO,B: Handles timeouts, missing bundler, and cleanup
sequenceDiagram
autonumber
actor Dev as Developer
participant Tests as Jest
participant BO as bundle-openapi.js (exports)
Tests->>BO: normalizeTag/getMajorMinor/sortObjectKeys
Tests->>BO: detectBundler (mocked child_process)
Tests->>BO: createEntrypoint/postProcessBundle (temp FS)
Tests-->>Dev: Assertions on outputs and errors
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Docstrings generation was requested by @JakeSCahill. * #134 (comment) The following files were modified: * `cli-utils/install-test-dependencies.sh` * `tools/bundle-openapi.js`
Note Generated docstrings for this pull request at #135 |
Docstrings generation was requested by @JakeSCahill. * #134 (comment) The following files were modified: * `cli-utils/install-test-dependencies.sh` * `tools/bundle-openapi.js` Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (6)
__tests__/tools/bundle-openapi.test.js
(1 hunks)admin/redpanda-admin-api.yaml
(1 hunks)bin/doc-tools.js
(2 hunks)cli-utils/install-test-dependencies.sh
(1 hunks)package.json
(3 hunks)tools/bundle-openapi.js
(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
__tests__/tools/bundle-openapi.test.js (1)
tools/bundle-openapi.js (9)
fs
(3-3)require
(5-5)require
(762-762)path
(4-4)bundler
(691-691)commonDir
(204-204)options
(475-475)options
(581-581)normalized
(37-37)
tools/bundle-openapi.js (2)
bin/doc-tools.js (11)
require
(3-3)require
(5-5)require
(10-10)require
(12-12)require
(13-13)require
(15-15)require
(16-19)require
(853-853)require
(1349-1349)require
(1561-1561)require
(1568-1568)__tests__/tools/bundle-openapi.test.js (2)
require
(4-4)require
(10-19)
bin/doc-tools.js (2)
tools/bundle-openapi.js (4)
require
(5-5)require
(762-762)options
(475-475)options
(581-581)__tests__/tools/bundle-openapi.test.js (2)
require
(4-4)require
(10-19)
cli-utils/install-test-dependencies.sh (3)
tools/docusaurus-to-antora-conversion-scripts/pre-process-markdown.js (2)
command
(35-35)pandoc
(3-3)bin/doc-tools.js (1)
make
(1080-1122)extensions/archive-attachments.js (1)
tar
(5-5)
🪛 Checkov (3.2.334)
admin/redpanda-admin-api.yaml
[high] 1-1582: Ensure that the global security field has rules defined
(CKV_OPENAPI_4)
[high] 1-1582: Ensure that security operations is not empty.
(CKV_OPENAPI_5)
[medium] 41-46: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
🪛 Shellcheck (0.11.0)
cli-utils/install-test-dependencies.sh
[warning] 27-27: Declare and assign separately to avoid masking return values.
(SC2155)
admin/redpanda-admin-api.yaml
Outdated
components: | ||
schemas: | ||
connect-protocol-version: | ||
const: 1 | ||
description: Define the version of the Connect protocol | ||
enum: | ||
- 1 | ||
title: Connect-Protocol-Version | ||
type: number | ||
connect-timeout-header: | ||
description: Define the timeout, in ms | ||
title: Connect-Timeout-Ms | ||
type: number | ||
connect.error: | ||
additionalProperties: true | ||
description: "Error type returned by Connect: https://connectrpc.com/docs/go/errors/#http-representation" | ||
properties: | ||
code: | ||
description: The status code, which should be an enum value of [google.rpc.Code][google.rpc.Code]. | ||
enum: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add security schemes and requirements.
The bundle omits components.securitySchemes
and a corresponding security
requirement, so every operation is documented as publicly accessible. The Admin API isn’t open—in practice it requires authenticated access (TLS client certs, HTTP basic/SASL credentials, etc.). Client code generated from this spec will therefore skip auth entirely. Please declare the appropriate security scheme(s) in components.securitySchemes
and apply them globally (or per operation) so the contract matches the real service.
🧰 Tools
🪛 Checkov (3.2.334)
[high] 1-1582: Ensure that the global security field has rules defined
(CKV_OPENAPI_4)
[high] 1-1582: Ensure that security operations is not empty.
(CKV_OPENAPI_5)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @rockwotj
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no option for this (I also don't know anything about openapi format). Do we have this for other generated proto files?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we might need to add overlays with the security schema? https://learn.openapis.org/specification/security.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cloud has it, but this was custom: https://github.com/redpanda-data/cloudv2/blob/dev/tools/openapi-converter/main.go#L302
The overlay option also works for me
From following the instructions
fwiw I checked out this branch and ran the commands but maybe I shouldn't have done that for testing |
Might be better to install via npm, then there should be no additional setup as I think it will be on the path for npm scripts: https://buf.build/docs/cli/installation/#npm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Oh nice. I didn't realise there was an npm package. Thanks! |
That's all fine. I'll add as an npm module. The error is about file execution permissions. I'll update. You don't actually need to call that command. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
This PR introduces a new CLI command
bundle-openapi
that automates the generation of complete OpenAPI 3.1 specification files from Redpanda protocol buffer definitions. This tool addresses the need for up-to-date OpenAPI documentation that can be automatically generated and maintained as part of our CI/CD pipeline.The new CLI tool automates the entire process of:
buf generate
to create OpenAPI fragments from protobuf definitions🛠️ Key Features
Example
An example of the bundled spec is here: https://github.com/redpanda-data/docs-extensions-and-macros/pull/134/files/38b6a24d493557bee2d7af0ecca4200922645704#diff-8e633cd03c898beb85637f6ce113c5f325bd2e700f36c8109ad2929e6215c231
Testing
Install dependencies:
Link the package for local testing:
Install CLI dependencies:
Test OpenAPI bundling functionality:
# Test admin API bundling doc-tools generate bundle-openapi --surface admin --tag dev --use-admin-major-version
You should get a bundled spec file inside the
admin/
directory.2025-10-03_15-47-53.mp4