HYPERFLEET-1330 - chore: replace bingo with Go 1.24 tool directives#301
HYPERFLEET-1330 - chore: replace bingo with Go 1.24 tool directives#301kuudori wants to merge 1 commit into
Conversation
|
[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 |
📝 WalkthroughWalkthroughThe project removes bingo-managed tool configuration and adds Go module tool declarations. Make targets now invoke pinned tools through Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 10 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (10 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
Risk Score: 4 —
|
| Signal | Detail | Points |
|---|---|---|
| PR size | 2504 lines (>500) | +2 |
| Sensitive paths | none | +0 |
| Test coverage | No _test.go files in diff | +2 |
Computed by hyperfleet-risk-scorer
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
scripts/test-helm.sh (1)
33-34: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueUnquoted
$KUBECONFORM/$YQ— fragile, SC2086.Both variables now hold multi-word commands (
"$(GO) tool kubeconform"), so quoting them as a single token would break word-splitting — the unquoted form is functionally required here, not accidental. But this makes correctness depend on the caller (Makefile) never injecting a path containing spaces or shell metacharacters. Since the value is fully Makefile-controlled (not external/user input), this isn't exploitable today (no CWE-78 vector), but it is brittle: switching either variable to an array (KUBECONFORM=("$(GO)" tool kubeconform)) would remove the reliance on word-splitting.Also applies to: 40-41
🤖 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 `@scripts/test-helm.sh` around lines 33 - 34, Update the command variables used by the test script, including KUBECONFORM and YQ, to arrays containing each executable argument separately rather than multi-word command strings. Invoke them with array expansion in the relevant command calls, preserving support for the existing flags and "$@" arguments without relying on unquoted word splitting.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 `@go.mod`:
- Around line 197-205: Remove the tool block from go.mod and relocate these
development tool pins to the repository’s established Makefile-based tool
management, following the existing golangci-lint pattern. Ensure all listed
generators, linters, and test tools remain invocable through the relevant Make
targets without expanding the shared module graph.
In `@Makefile`:
- Line 60: Update the GO_VERSION variable in the Makefile to remove the trailing
period, preserving the intended go1.26 version string for verify checks.
---
Nitpick comments:
In `@scripts/test-helm.sh`:
- Around line 33-34: Update the command variables used by the test script,
including KUBECONFORM and YQ, to arrays containing each executable argument
separately rather than multi-word command strings. Invoke them with array
expansion in the relevant command calls, preserving support for the existing
flags and "$@" arguments without relying on unquoted word splitting.
🪄 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: 1bb812ce-e1f0-45c2-abbc-ea1760664a65
⛔ Files ignored due to path filters (8)
.bingo/golangci-lint.sumis excluded by!**/*.sum.bingo/gotestsum.sumis excluded by!**/*.sum.bingo/helm-docs.sumis excluded by!**/*.sum.bingo/kubeconform.sumis excluded by!**/*.sum.bingo/mockgen.sumis excluded by!**/*.sum.bingo/oapi-codegen.sumis excluded by!**/*.sum.bingo/yq.sumis excluded by!**/*.sumgo.sumis excluded by!**/*.sum,!**/go.sum
📒 Files selected for processing (22)
.bingo/.gitignore.bingo/README.md.bingo/Variables.mk.bingo/go.mod.bingo/golangci-lint.mod.bingo/gotestsum.mod.bingo/helm-docs.mod.bingo/kubeconform.mod.bingo/mockgen.mod.bingo/oapi-codegen.mod.bingo/variables.env.bingo/yq.mod.golangciversionMakefilego.modpkg/handlers/openapi.gopkg/services/adapter_status.gopkg/services/cluster.gopkg/services/generic.gopkg/services/node_pool.gopkg/services/resource.goscripts/test-helm.sh
🔗 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 (13)
- .bingo/gotestsum.mod
- .bingo/go.mod
- .golangciversion
- .bingo/mockgen.mod
- .bingo/yq.mod
- .bingo/README.md
- .bingo/kubeconform.mod
- .bingo/variables.env
- .bingo/golangci-lint.mod
- .bingo/.gitignore
- .bingo/helm-docs.mod
- .bingo/Variables.mk
- .bingo/oapi-codegen.mod
|
|
||
| tool ( | ||
| github.com/mikefarah/yq/v4 | ||
| github.com/norwoodj/helm-docs/cmd/helm-docs | ||
| github.com/oapi-codegen/oapi-codegen/v2/cmd/oapi-codegen | ||
| github.com/yannh/kubeconform/cmd/kubeconform | ||
| go.uber.org/mock/mockgen | ||
| gotest.tools/gotestsum | ||
| ) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '### go.mod relevant slice'
sed -n '170,230p' go.mod | cat -n
echo
echo '### Makefile relevant slice'
rg -n -A4 -B4 'golangci-lint|bingo|tool directives|go.mod' MakefileRepository: openshift-hyperfleet/hyperfleet-api
Length of output: 3256
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '### dependency-pinning.md relevant slices'
rg -n -A4 -B4 'Bingo|tool directives|production dependency graph|go.mod' hyperfleet/standards/dependency-pinning.mdRepository: openshift-hyperfleet/architecture
Length of output: 7249
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '### go.mod relevant slice'
sed -n '170,230p' go.mod | cat -n
echo
echo '### Makefile relevant slice'
rg -n -A4 -B4 'golangci-lint|bingo|tool directives|go.mod' MakefileRepository: openshift-hyperfleet/hyperfleet-api
Length of output: 3256
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '### dependency-pinning.md relevant slices'
rg -n -A4 -B4 'Bingo|tool directives|production dependency graph|go.mod' hyperfleet/standards/dependency-pinning.mdRepository: openshift-hyperfleet/architecture
Length of output: 7249
Move the dev tool pins out of go.mod. tool (...) in go.mod:197-205 conflicts with the HyperFleet Bingo standard for Go repos and widens the shared module graph (CWE-829). The Makefile already keeps golangci-lint out of go.mod for that reason; apply the same boundary to the other generators/lint tools here.
🤖 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 `@go.mod` around lines 197 - 205, Remove the tool block from go.mod and
relocate these development tool pins to the repository’s established
Makefile-based tool management, following the existing golangci-lint pattern.
Ensure all listed generators, linters, and test tools remain invocable through
the relevant Make targets without expanding the shared module graph.
Source: Linked repositories
| # Encourage consistent tool versions | ||
| OPENAPI_GENERATOR_VERSION := 5.4.0 | ||
| GO_VERSION := go1.25. | ||
| GO_VERSION := go1.26. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Trailing dot in GO_VERSION breaks the version string.
GO_VERSION := go1.26. has a stray trailing period. make verify-style checks that compare this against go version output (e.g., prefix or exact match) will fail or silently no-op.
Fix
-GO_VERSION := go1.26.
+GO_VERSION := go1.26📝 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.
| GO_VERSION := go1.26. | |
| GO_VERSION := go1.26 |
🤖 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 60, Update the GO_VERSION variable in the Makefile to
remove the trailing period, preserving the intended go1.26 version string for
verify checks.
kuudori
changed the title
|
/test validate-commits |
|
PR needs rebase. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
1 participant
Summary
Test Plan
make test-allpassesmake lintpassesmake test-helm(if applicable)