fix: pin otelcol build, rest-api CLI tools in support of hermetic builds#4129
fix: pin otelcol build, rest-api CLI tools in support of hermetic builds#4129ianderson-nvidia wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (14)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (12)
Summary by CodeRabbit
WalkthroughThe PR standardizes pinned REST tooling and protobuf generation, removes redundant CI tool installation, and introduces a committed OpenTelemetry Collector module graph used by local and Docker builds. It also adds regeneration documentation and ignores local development metadata. ChangesREST tooling reproducibility
OpenTelemetry module reproducibility
Repository hygiene
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)REST tool executionsequenceDiagram
participant Makefile
participant Buf
participant ProtobufPlugins
Makefile->>ProtobufPlugins: Build protoc-gen-go and protoc-gen-go-grpc
Makefile->>Buf: Run pinned buf generate
Buf->>ProtobufPlugins: Invoke local protobuf plugins
OpenTelemetry Collector buildsequenceDiagram
participant BuildTask
participant OCB
participant GoBuild
BuildTask->>OCB: Generate sources without compilation
BuildTask->>GoBuild: Copy committed go.mod and go.sum
GoBuild->>GoBuild: Cross-compile otelcol-contrib for linux/arm64
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
🔐 TruffleHog Secret Scan✅ No secrets or credentials found! Your code has been scanned for 700+ types of secrets and credentials. All clear! 🎉 🕐 Last updated: 2026-07-24 20:10:30 UTC | Commit: d205ef9 |
d205ef9 to
4b708bd
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.gitignore:
- Around line 63-64: Remove the .pre-commit-config.yaml entry from .gitignore so
the repository-wide pre-commit configuration remains tracked. Do not add a
replacement ignore rule; local-only exclusions should use a personal ignore
filename or .git/info/exclude.
In `@bluefield/Makefile.toml`:
- Around line 297-300: Update the Go build commands in bluefield/Makefile.toml
lines 297-300 and bluefield/containers/otelcol-contrib/Dockerfile lines 80-85 to
use -mod=readonly instead of -mod=mod. Apply the same change at both sites so
builds fail when the pinned module graph is incomplete without modifying go.mod
or go.sum.
In `@bluefield/otel/ocb-generated/go.mod`:
- Line 389: Update the generator inputs or dependency override controlling
google.golang.org/grpc, requiring version 1.82.1 or later, then regenerate the
generated go.mod and go.sum files. Do not manually edit
bluefield/otel/ocb-generated/go.mod; verify the generated dependency no longer
uses v1.81.1.
In `@bluefield/otel/update-ocb-modules.sh`:
- Line 65: Correct the user-facing commit command in update-ocb-modules.sh by
changing “Updatingd” to “Updating” and adding the missing closing single quote
after ${VERSION}, while preserving the surrounding echo behavior and shell
quoting.
In `@rest-api/tools/tools.go`:
- Line 15: Align all Buf version references to v1.72.0: update the stale version
comment in rest-api/tools/tools.go at line 15 and the README example in
rest-api/tools/README.md at lines 10-16. Leave the already-correct
rest-api/go.mod reference at line 18 unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 975a2198-5f53-4975-b37e-652b593fd9f8
⛔ Files ignored due to path filters (2)
bluefield/otel/ocb-generated/go.sumis excluded by!**/*.sumrest-api/go.sumis excluded by!**/*.sum
📒 Files selected for processing (16)
.github/workflows/ci.yaml.github/workflows/rest-lint-and-test.yml.gitignorebluefield/Makefile.tomlbluefield/containers/otelcol-contrib/Dockerfilebluefield/otel/README.mdbluefield/otel/ocb-generated/go.modbluefield/otel/otelcol_builder_config_yaml.txtbluefield/otel/update-ocb-modules.shrest-api/.tools-stamprest-api/.tools/protoc-gen-gorest-api/.tools/protoc-gen-go-grpcrest-api/Makefilerest-api/go.modrest-api/tools/README.mdrest-api/tools/tools.go
💤 Files with no reviewable changes (1)
- .github/workflows/ci.yaml
There was a problem hiding this comment.
♻️ Duplicate comments (1)
bluefield/otel/ocb-generated/go.mod (1)
389-389:⚠️ Potential issue | 🟠 MajorRegenerate with
google.golang.org/grpc1.82.1 or later.The generated graph still pins
google.golang.org/grpc v1.81.1. The advisory marks versions below1.82.1as affected and1.82.1as patched, including xDS RBAC authorization-bypass and HTTP/2 denial-of-service risks. (github.com)Update the OCB dependency inputs or override, then regenerate both
go.modandgo.sum; do not hand-edit this generated file. This duplicates the unresolved previous review finding.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@bluefield/otel/ocb-generated/go.mod` at line 389, Update the OCB dependency inputs or override to require google.golang.org/grpc v1.82.1 or later, then regenerate the generated go.mod and go.sum files so the dependency graph no longer contains v1.81.1; do not hand-edit the generated output.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@bluefield/otel/ocb-generated/go.mod`:
- Line 389: Update the OCB dependency inputs or override to require
google.golang.org/grpc v1.82.1 or later, then regenerate the generated go.mod
and go.sum files so the dependency graph no longer contains v1.81.1; do not
hand-edit the generated output.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 66e45d49-6151-4c0a-b594-9b565bc3de45
⛔ Files ignored due to path filters (2)
bluefield/otel/ocb-generated/go.sumis excluded by!**/*.sumrest-api/go.sumis excluded by!**/*.sum
📒 Files selected for processing (14)
.github/workflows/ci.yaml.github/workflows/rest-lint-and-test.yml.gitignorebluefield/Makefile.tomlbluefield/containers/otelcol-contrib/Dockerfilebluefield/otel/README.mdbluefield/otel/ocb-generated/go.modbluefield/otel/otelcol_builder_config_yaml.txtbluefield/otel/update-ocb-modules.shrest-api/.gitignorerest-api/Makefilerest-api/go.modrest-api/tools/README.mdrest-api/tools/tools.go
💤 Files with no reviewable changes (1)
- .github/workflows/ci.yaml
🚧 Files skipped from review as they are similar to previous changes (9)
- bluefield/otel/README.md
- bluefield/otel/otelcol_builder_config_yaml.txt
- .gitignore
- bluefield/otel/update-ocb-modules.sh
- .github/workflows/rest-lint-and-test.yml
- bluefield/Makefile.toml
- bluefield/containers/otelcol-contrib/Dockerfile
- rest-api/Makefile
- rest-api/go.mod
The `docker-build-otelcol` Dockerfile and the `build-otelcol` cargo-make task previously ran `ocb` without `--skip-get-modules`, causing `go mod tidy` to execute inside every build. Because `go mod tidy` queries and possibly updates go modules, two builds from the same commit could resolve different indirect dependency versions and produce different binaries. Both build paths now run OCB with `--skip-compilation --skip-get-modules` to generate Go source only, then overwrite the incomplete OCB-generated `go.mod`/`go.sum` with the committed files from `bluefield/otel/ocb-generated/` before calling `go build -mod=mod`. The `include-otelcol` task condition is updated to invalidate the cached binary when the committed module files change. The trade-off: developers must run `bluefield/otel/update-ocb-modules.sh` and commit the outputs whenever the OCB config or component versions change. This is a manual step, but it makes the build reproducible and prevents silent dependency drift. `rest-api/tools/tools.go` (new) pins `buf`, `protoc-gen-go`, `protoc-gen-go-grpc`, `golangci-lint`, and `revive` in `go.mod`. `rest-api/Makefile` is updated to invoke these tools via `$(BUF)`, `$(GOLANGCI_LINT)`, and `$(REVIVE)` using `go run`. `protoc-gen-go` and `protoc-gen-go-grpc` must be real executables on PATH because `buf generate` spawns them as subprocesses — `go run` cannot be used directly. A `.tools-stamp` target builds them from the pinned `go.mod` versions into `.tools/` and injects that directory into PATH for `core-protogen` and `flow-protogen`. The manual version-check guards previously in `core-protogen` are removed; the version is now enforced by `go.mod` rather than runtime assertions. The `Install buf and protoc plugins` steps in `ci.yaml` and `rest-lint-and-test.yml` are removed — `make core-proto` now handles tool installation via `.tools-stamp`. The `go install github.com/mgechev/revive@v1.15.0` call in the `style` CI job is replaced with `go run github.com/mgechev/revive` so the version from `go.mod` is used.
4b708bd to
36701f4
Compare
| go install github.com/mgechev/revive@v1.15.0 | ||
| pkgs=$(go list ./... | grep -v $(go env GOMODCACHE)) | ||
| go list $pkgs | grep -v $(go env GOMODCACHE) | xargs -L1 revive -config .revive.toml -set_exit_status | ||
| go list $pkgs | grep -v $(go env GOMODCACHE) | xargs -L1 go run github.com/mgechev/revive -config .revive.toml -set_exit_status |
There was a problem hiding this comment.
runs a random github thing?
There was a problem hiding this comment.
It’s just running the version of ‘revive’ that is pinned in go.mod instead of downloading it on every CI run.
The
docker-build-otelcolDockerfile and thebuild-otelcolcargo-make task previously ranocbwithout--skip-get-modules, causinggo mod tidyto execute inside every build. Becausego mod tidyqueries and possibly updates Go modules, two builds from the same commit could resolve different indirect dependency versions and produce different binaries.Both build paths now run OCB with
--skip-compilation --skip-get-modulesto generate Go source only, then overwrite the incomplete OCB-generatedgo.mod/go.sumwith the committed files frombluefield/otel/ocb-generated/before callinggo build -mod=mod. Theinclude-otelcoltask condition is updated to invalidate the cached binary when the committed module files change.The trade-off: developers must run
bluefield/otel/update-ocb-modules.shand commit the outputs whenever the OCB config or component versions change. This is a manual step, but it makes the build reproducible and prevents silent dependency drift.rest-api/tools/tools.go(new) pinsbuf,protoc-gen-go,protoc-gen-go-grpc,golangci-lint, andreviveingo.mod.rest-api/Makefileis updated to invoke these tools via$(BUF),$(GOLANGCI_LINT), and$(REVIVE)usinggo run.protoc-gen-goandprotoc-gen-go-grpcmust be real executables on PATH becausebuf generatespawns them as subprocesses —go runcannot be used directly. A.tools-stamptarget builds them from the pinnedgo.modversions into.tools/and injects that directory into PATH forcore-protogenandflow-protogen. The manual version-check guards previously incore-protogenare removed; the version is now enforced bygo.modrather than runtime assertions.The
Install buf and protoc pluginssteps inci.yamlandrest-lint-and-test.ymlare removed —make core-protonow handles tool installation via.tools-stamp. Thego install github.com/mgechev/revive@v1.15.0call in thestyleCI job is replaced withgo run github.com/mgechev/reviveso the version fromgo.modis used.Type of Change
Testing