chore: replace bingo with Go 1.24 tool directives#14
Conversation
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe Bingo ignore, variable, and golangci-lint module files are removed. The Makefile now defines a pinned Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 11✅ Passed checks (11 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
8c1589e to
870d2af
Compare
870d2af to
7bc6c3c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@Makefile`:
- Around line 147-149: Replace the curl-piped installer in the $(GOLANGCI_LINT)
target with a checksum-verified installation approach, preferably a pinned Go
module install consistent with the project’s native Go tool directives. Ensure
the target still installs the requested golangci-lint version into ./bin without
executing an unverified remote script.
🪄 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: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 6f893cf8-502e-423a-b076-d7ca8e2aec0c
⛔ Files ignored due to path filters (1)
.bingo/golangci-lint.sumis excluded by!**/*.sum
📒 Files selected for processing (4)
.bingo/.gitignore.bingo/Variables.mk.bingo/golangci-lint.modMakefile
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift-hyperfleet/architecture(manual)openshift-hyperfleet/hyperfleet-api(manual)openshift-hyperfleet/hyperfleet-sentinel(manual)openshift-hyperfleet/hyperfleet-adapter(manual)openshift-hyperfleet/hyperfleet-broker(manual) → reviewed against open PR#26chore/replace-bingo-with-go-tool-directivesinstead of the default branch
💤 Files with no reviewable changes (3)
- .bingo/.gitignore
- .bingo/golangci-lint.mod
- .bingo/Variables.mk
|
|
||
| $(GOLANGCI_LINT): | ||
| curl -sSfL https://golangci-lint.run/install.sh | sh -s -- -b ./bin $(GOLANGCI_LINT_VERSION) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
CWE-494: unverified curl-pipe-sh executes remote script with no integrity check.
curl -sSfL ... | sh -s -- pipes a remote install script directly into a shell with zero checksum/signature verification. A compromised endpoint or DNS/TLS MITM yields arbitrary code execution during make lint / CI, running with the invoking user's/CI privileges. Path instructions for **/Makefile explicitly prohibit this pattern.
🔒 Checksummed-install alternative
$(GOLANGCI_LINT):
- curl -sSfL https://golangci-lint.run/install.sh | sh -s -- -b ./bin $(GOLANGCI_LINT_VERSION)
+ curl -sSfL -o /tmp/golangci-lint-install.sh https://golangci-lint.run/install.sh
+ echo "<pinned-sha256> /tmp/golangci-lint-install.sh" | sha256sum -c -
+ sh /tmp/golangci-lint-install.sh -b ./bin $(GOLANGCI_LINT_VERSION)Or avoid the installer script entirely and use go install with a pinned checksum-verified module (go.sum already covers integrity for go install/go tool), consistent with the PR's stated move to native Go tool directives.
As per path instructions, "No curl-pipe-bash for tool installation (use checksummed downloads)".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $(GOLANGCI_LINT): | |
| curl -sSfL https://golangci-lint.run/install.sh | sh -s -- -b ./bin $(GOLANGCI_LINT_VERSION) | |
| $(GOLANGCI_LINT): | |
| curl -sSfL -o /tmp/golangci-lint-install.sh https://golangci-lint.run/install.sh | |
| echo "<pinned-sha256> /tmp/golangci-lint-install.sh" | sha256sum -c - | |
| sh /tmp/golangci-lint-install.sh -b ./bin $(GOLANGCI_LINT_VERSION) |
🧰 Tools
🪛 checkmake (0.3.2)
[warning] 147-147: Required target "all" is missing from the Makefile.
(minphony)
🤖 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 `@Makefile` around lines 147 - 149, Replace the curl-piped installer in the
$(GOLANGCI_LINT) target with a checksum-verified installation approach,
preferably a pinned Go module install consistent with the project’s native Go
tool directives. Ensure the target still installs the requested golangci-lint
version into ./bin without executing an unverified remote script.
Source: Path instructions
HYPERFLEET-1333
Remove bingo tool management in favor of native
tooldirectives in go.mod (Go 1.24+).Tool versions are pinned in go.mod/go.sum and resolved via
go tool <name>.Pinned versions (unchanged from bingo):