-
Notifications
You must be signed in to change notification settings - Fork 16
Add support for buf to generate protos #86
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
cb1224e to
1df91da
Compare
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.
Pull request overview
This PR switches the proto generation pipeline from protobuild + in-repo containerd protos to buf, aligning with containerd’s current approach and using buf’s remote modules for dependencies.
Changes:
- Introduces
buf.yaml,buf.gen.yaml, andbuf.lockunderapi/and wires theprotosMakefile target tobuf dep update,buf generate, andbuf build -o next.txtpb. - Moves/defines service protos under
api/proto/nerdbox/services/...(bundle, system, vmevents), updates imports to use buf module paths (e.g.,containerd/types/event.proto), and replaces the descriptor snapshot filenext.pb.txtwithnext.txtpb. - Regenerates the Go protobuf and TTRPC stubs to match the new proto locations and descriptor names.
Reviewed changes
Copilot reviewed 16 out of 19 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
api/services/vmevents/v1/events_ttrpc.pb.go |
Updates generated ttrpc stub metadata to reference the new proto path under proto/nerdbox/.... |
api/services/vmevents/v1/events.pb.go |
Regenerates the vmevents Go proto file to reflect the new on-disk proto path and buf-style file descriptor symbols. |
api/services/system/v1/info_ttrpc.pb.go |
Updates generated ttrpc stub metadata to use the new proto/nerdbox/services/system/v1/info.proto source path. |
api/services/system/v1/info.pb.go |
Regenerates the system info Go proto file with new descriptor symbol names and raw descriptor content pointing to proto/nerdbox/.... |
api/services/bundle/v1/bundle_ttrpc.pb.go |
Updates generated ttrpc stub metadata to reference proto/nerdbox/services/bundle/v1/bundle.proto. |
api/services/bundle/v1/bundle.pb.go |
Regenerates the bundle service Go proto file with new descriptor symbols and raw descriptor referencing proto/nerdbox/.... |
api/proto/nerdbox/services/vmevents/v1/events.proto |
Adjusts the import to use containerd/types/event.proto, matching the buf containerd/api-dev module layout. |
api/proto/nerdbox/services/system/v1/info.proto |
Adds a new system service proto (Info RPC + InfoResponse) with appropriate package and go_package options. |
api/proto/nerdbox/services/bundle/v1/bundle.proto |
Adds a new bundle service proto (Create RPC, CreateRequest/Response) with correct package and go_package. |
api/next.txtpb |
Adds a new buf-generated descriptor snapshot covering the nerdbox bundle, system, and vmevents protos. |
api/next.pb.txt |
Removes the old protobuild-generated descriptor snapshot that referenced legacy proto import paths. |
api/buf.yaml |
Adds buf workspace configuration, including dependencies on buf.build/containerd/api-dev and buf.build/googleapis/googleapis. |
api/buf.lock |
Locks buf module dependency versions and digests for reproducible builds. |
api/buf.gen.yaml |
Configures buf code generation for Go and TTRPC using the remote protoc-gen-go plugin and the local protoc-gen-go-ttrpc. |
api/Protobuild.toml |
Removes the legacy Protobuild configuration in favor of buf-based generation. |
Makefile |
Switches the protos target to use buf for deps, generation, and descriptor output; updates check-api-descriptors to look for .txtpb files (with a minor inconsistency in the git diff path pattern noted above). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Derek McGowan <derek@mcg.dev>
1df91da to
e34e0a3
Compare
Signed-off-by: Derek McGowan <derek@mcg.dev>
bf4ba0c to
041e4d5
Compare
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.
Pull request overview
Copilot reviewed 16 out of 19 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
script/install-protobuf:1
- This script previously downloaded and installed protobuf binaries and sources via
wgetfrom GitHub and executed build steps (./autogen.sh,make,make install) without verifying any checksum or signature. If an attacker compromised the upstream release or tampered with the download channel, they could supply a malicious archive that would run with build-time privileges, resulting in arbitrary code execution in the build environment. Removing this script (or alternatively adding strict integrity verification such as pinned checksums or signatures) mitigates this supply-chain risk.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - uses: actions/setup-node@v6 | ||
| - run: npm install -g @bufbuild/buf | ||
| - run: script/install-proto-tools | ||
| - run: make proto-fmt | ||
| - run: make check-protos check-api-descriptors |
Copilot
AI
Jan 24, 2026
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.
The npm install @bufbuild/buf step installs the buf CLI into node_modules/.bin, but this directory is not added to PATH, so subsequent make invocations that call buf (via the protos target) will fail with buf: command not found on a clean runner. Consider either installing Buf globally (e.g. using a dedicated Buf GitHub Action or npm install -g @bufbuild/buf) or adjusting the workflow/Makefile to invoke npx buf or to add ./node_modules/.bin to PATH before running make.
| @(cd ${ROOTDIR}/api && buf dep update) | ||
| @(cd ${ROOTDIR}/api && PATH="${ROOTDIR}/bin:${PATH}" buf generate) | ||
| @(cd ${ROOTDIR}/api && buf build --exclude-imports -o next.txtpb) |
Copilot
AI
Jan 24, 2026
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.
Running buf dep update on every make protos invocation will continuously mutate api/buf.lock, which means the protobuf/check-api-descriptors CI jobs can start failing purely due to upstream dependency changes even when no .proto files were modified. To keep CI stable and descriptor checks deterministic, it would be better to move buf dep update into a separate, explicitly-run target (e.g. for dependency bumps) and have protos just consume the pinned dependencies from buf.lock.
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.
Will address this in a follow up
Fixes proto generation by using buf. containerd switched to buf and currently testing publishing through the buf registry to make importing easier.