HYPERFLEET-1332 - chore: replace bingo with Go 1.24 tool directives#26
HYPERFLEET-1332 - chore: replace bingo with Go 1.24 tool directives#26kuudori wants to merge 5 commits into
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 |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughBingo-managed metadata and lint installation logic were removed. The Makefile now runs Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Makefile
participant GoTool as go tool
participant ToolsModule as tools/go.mod
participant Linter as golangci-lint
Makefile->>GoTool: Run golangci-lint with tools/go.mod
GoTool->>ToolsModule: Resolve tool dependency
GoTool->>Linter: Execute lint ./...
Makefile->>ToolsModule: Tidy and verify module files
Suggested reviewers: 🚥 Pre-merge checks | ✅ 11✅ Passed checks (11 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
424496f to
1fc26d1
Compare
4050f16 to
5d7ac5e
Compare
Risk Score: 1 —
|
| Signal | Detail | Points |
|---|---|---|
| PR size | 391 lines (>200) | +1 |
| Sensitive paths | none | +0 |
Computed by hyperfleet-risk-scorer
5d7ac5e to
2743801
Compare
621e9f2 to
869f517
Compare
869f517 to
594ffa6
Compare
594ffa6 to
19ffcbf
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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`:
- Line 2: Update the gotool definition to quote TOOL_MOD when it is expanded
into the shell command, preventing command-line overrides from being interpreted
as separate commands while preserving the existing go tool invocation.
- Around line 15-16: Update the verify-tools target to compare tools/go.mod and
tools/go.sum against HEAD, including both staged and unstaged changes, and to
reject untracked generated files. Preserve the existing failure message and
tidy-tools remediation while ensuring any missing or modified tool files cause
the target to fail.
In `@tools/go.mod`:
- Around line 1-5: Align the repository’s tooling standards and documentation
with the new tools module and make lint workflow: update the dependency-pinning
and lint-installation guidance, including the relevant AGENTS.md section, to
consistently use the new approach instead of Bingo, or document an approved
exception if Bingo must remain required. Ensure the declared tooling and
supply-chain contract matches the implementation.
🪄 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: 596c3e7f-cb55-460f-86f2-9ce08ce2d0aa
⛔ Files ignored due to path filters (1)
tools/go.sumis excluded by!**/*.sum,!**/go.sum
📒 Files selected for processing (8)
.bingo/.gitignore.bingo/README.md.bingo/Variables.mk.bingo/go.mod.bingo/golangci-lint.mod.bingo/variables.envMakefiletools/go.mod
🔗 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)
💤 Files with no reviewable changes (6)
- .bingo/.gitignore
- .bingo/Variables.mk
- .bingo/golangci-lint.mod
- .bingo/README.md
- .bingo/go.mod
- .bingo/variables.env
e35ac9b to
8a8fe5d
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
Makefile (1)
2-2: 🔒 Security & Privacy | 🟠 MajorQuote
TOOL_MODbefore shell expansion (CWE-78).Line [2] inserts a command-line-overridable Make variable directly into a shell command. An override such as
make TOOL_MOD='tools/go.mod; ...' lintcan terminate the intended command and execute another command. Quote the expansion or enforce the fixed path. This is the same unresolved finding from the previous review.Proposed fix
-gotool = go tool -modfile=$(TOOL_MOD) $(1) +gotool = go tool -modfile="$(TOOL_MOD)" $(1)As per path instructions, “Flag shell injection via unquoted variables in recipes.”
🤖 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` at line 2, Update the gotool definition to quote the TOOL_MOD expansion before it is passed to the shell, preventing command-line overrides from being interpreted as shell syntax. Preserve the existing go tool invocation and $(1) argument behavior.Source: Path instructions
🤖 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`:
- Line 16: Update the tool-module verification command in the Makefile target to
compare tools/go.mod and tools/go.sum against HEAD rather than the index, and
explicitly fail when either file is untracked. Preserve the existing out-of-date
message and make tools remediation behavior.
---
Duplicate comments:
In `@Makefile`:
- Line 2: Update the gotool definition to quote the TOOL_MOD expansion before it
is passed to the shell, preventing command-line overrides from being interpreted
as shell syntax. Preserve the existing go tool invocation and $(1) argument
behavior.
🪄 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: 4561c429-2c24-43b3-861f-261a20cd7a1d
⛔ Files ignored due to path filters (1)
tools/go.sumis excluded by!**/*.sum,!**/go.sum
📒 Files selected for processing (10)
.bingo/.gitignore.bingo/README.md.bingo/Variables.mk.bingo/go.mod.bingo/golangci-lint.mod.bingo/variables.envAGENTS.mdMakefileREADME.mdtools/go.mod
🔗 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) → reviewed against branchchore/replace-bingo-with-go-tool-directivesinstead of the default branchopenshift-hyperfleet/hyperfleet-broker(manual)
💤 Files with no reviewable changes (6)
- .bingo/.gitignore
- .bingo/README.md
- .bingo/Variables.mk
- .bingo/go.mod
- .bingo/golangci-lint.mod
- .bingo/variables.env
🚧 Files skipped from review as they are similar to previous changes (1)
- tools/go.mod
8a8fe5d to
cc63a62
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
- Quote Make variables in gotool and tool recipes (CWE-78) - Compare tool files against HEAD in verify-tools
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`:
- Line 17: Update the tool-module validation command in the Makefile’s
verify-tools flow to detect untracked tools/go.mod and tools/go.sum in addition
to tracked-file differences. Preserve the existing failure message and exit
behavior, while ensuring newly generated metadata causes verification to fail.
🪄 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: 7071aaf1-9f49-42fe-9cc0-43b4fa8a04fc
📒 Files selected for processing (1)
Makefile
🔗 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) → reviewed against branchchore/replace-bingo-with-go-tool-directivesinstead of the default branchopenshift-hyperfleet/hyperfleet-broker(manual)
0417021 to
b71e439
Compare
|
/test unit |
|
/test lint |
1 similar comment
|
/test lint |
HYPERFLEET-1332
Replace Bingo / standalone
go installwith a singletools/go.modusing Go 1.24+ tool directives.Tools are pinned in
tools/go.modand invoked via$(call gotool,<name>)in the Makefile.Tools:
Also adds
toolsandverify-toolsMake targets.Follow-up changes (review comment fixes)
$(GO)and$(TOOL_MOD)in Makefilegotooland tool recipes to prevent shell injection (CWE-78)verify-toolsnow compares againstHEAD(git diff HEAD --) instead of the working tree