CNF-23398: Upgrade golangci-lint from v1.54.2 to v2.12.2#692
Conversation
|
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:
WalkthroughAdds a new GolangCI-Lint configuration ( ChangesGolangCI-Lint upgrade & config
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (11 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: sebrandon1 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@Makefile`:
- Line 423: The Makefile pins GOLANGCI_LINT_VERSION = v2.0.2 but hack/lint.sh
still sets VERSION=v1.54.2, causing inconsistent lint runs; update the VERSION
variable in hack/lint.sh from v1.54.2 to v2.0.2 so the script pulls the same
golangci-lint image as the Makefile (look for the VERSION= assignment near the
top of hack/lint.sh and change it to VERSION=v2.0.2).
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (2)
.golangci.ymlMakefile
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
Makefile
🔇 Additional comments (2)
Makefile (1)
432-432: Verify prereqs still finds the v2 binary in your environments.Line 432 still checks
$(shell go env GOPATH)/bin; if CI or local dev usesGOBIN, confirm the guard and PATH align so the v2 binary is discovered and not reinstalled unnecessarily..golangci.yml (1)
1-3: Config is minimal and aligned with v2; ensure CI runs golangci-lint ≥ 2.0.Line 1 introduces
version: "2", which v1 will reject; please verify all lint invocations use the v2 binary.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
|
|
||
| CILIUM_EBPF_VERSION := v0.18.0 | ||
| GOLANGCI_LINT_VERSION = v1.54.2 | ||
| GOLANGCI_LINT_VERSION = v2.0.2 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for remaining v1 references and v1.54.2 version pins
echo "=== Searching for v1.54.2 references ==="
rg -n 'v1\.54\.2'
echo "=== Searching for v1 binary path references ==="
rg -n 'golangci-lint/cmd/golangci-lint'
echo "=== Checking Makefile for golangci-lint references ==="
rg -n -A2 -B2 'GOLANGCI_LINT' Makefile
echo "=== Looking for CI scripts that may reference golangci-lint ==="
fd -t f '(lint|ci|github)' hack/ | head -20Repository: openshift/ingress-node-firewall
Length of output: 1056
🏁 Script executed:
#!/bin/bash
# Check hack/lint.sh to understand the version impact
echo "=== Content of hack/lint.sh ==="
cat -n hack/lint.sh
echo ""
echo "=== Search for references to hack/lint.sh in CI workflows ==="
fd -t f '\.(yml|yaml)' .github/workflows/ 2>/dev/null | while read f; do
if rg -l 'hack/lint' "$f" 2>/dev/null; then
echo "Found in: $f"
rg -n 'hack/lint' "$f"
fi
done
echo ""
echo "=== Check if hack/lint.sh is referenced elsewhere ==="
rg -l 'hack/lint'Repository: openshift/ingress-node-firewall
Length of output: 813
🏁 Script executed:
#!/bin/bash
# Check if hack/lint.sh is actively maintained/referenced in Makefile targets
echo "=== Check Makefile for hack/lint.sh usage ==="
rg -n 'hack/lint' Makefile
echo ""
echo "=== Check for any documentation/comments about hack/lint.sh usage ==="
rg -B2 -A2 'hack/lint' README* 2>/dev/null || echo "No README references found"
echo ""
echo "=== Check if hack/lint.sh target exists in Makefile ==="
rg -n '^lint:' MakefileRepository: openshift/ingress-node-firewall
Length of output: 379
Update hack/lint.sh to use golangci-lint v2.0.2 to match Makefile pin.
The v1.54.2 container image reference at hack/lint.sh:6 conflicts with the v2.0.2 version pinned in Makefile:423. Since the make lint target calls hack/lint.sh, developers using that command get inconsistent linting behavior. Update VERSION=v1.54.2 to VERSION=v2.0.2 in hack/lint.sh.
🤖 Prompt for AI Agents
In `@Makefile` at line 423, The Makefile pins GOLANGCI_LINT_VERSION = v2.0.2 but
hack/lint.sh still sets VERSION=v1.54.2, causing inconsistent lint runs; update
the VERSION variable in hack/lint.sh from v1.54.2 to v2.0.2 so the script pulls
the same golangci-lint image as the Makefile (look for the VERSION= assignment
near the top of hack/lint.sh and change it to VERSION=v2.0.2).
There was a problem hiding this comment.
♻️ Duplicate comments (1)
Makefile (1)
423-432:⚠️ Potential issue | 🟠 MajorAlign lint execution with the v2.11.4 upgrade.
Makefilenow pins/install-paths golangci-lint v2, butmake lintstill callshack/lint.sh, which remains pinned tov1.54.2(Line 6 inhack/lint.sh). This keeps lint behavior inconsistent with the upgrade objective.#!/bin/bash # Verify golangci-lint version consistency between Makefile and lint script rg -n 'GOLANGCI_LINT_VERSION|golangci-lint/v2/cmd/golangci-lint|^\s*lint:' Makefile rg -n 'VERSION=|golangci/golangci-lint:' hack/lint.shExpected:
hack/lint.shandMakefileshould both reference the same golangci-lint v2 version.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 423 - 432, Makefile now installs golangci-lint using GOLANGCI_LINT_VERSION (v2.11.4) but hack/lint.sh still pins v1.54.2; update hack/lint.sh to reference the same v2 version or read GOLANGCI_LINT_VERSION (e.g., set VERSION to v2.11.4 or switch install lines from github.com/golangci/golangci-lint@v1.54.2 to github.com/golangci/golangci-lint/v2/cmd/golangci-lint@${GOLANGCI_LINT_VERSION}), and ensure the Makefile lint target invokes the prereqs or otherwise uses the same variable so both the Makefile (GOLANGCI_LINT_VERSION) and hack/lint.sh are consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@Makefile`:
- Around line 423-432: Makefile now installs golangci-lint using
GOLANGCI_LINT_VERSION (v2.11.4) but hack/lint.sh still pins v1.54.2; update
hack/lint.sh to reference the same v2 version or read GOLANGCI_LINT_VERSION
(e.g., set VERSION to v2.11.4 or switch install lines from
github.com/golangci/golangci-lint@v1.54.2 to
github.com/golangci/golangci-lint/v2/cmd/golangci-lint@${GOLANGCI_LINT_VERSION}),
and ensure the Makefile lint target invokes the prereqs or otherwise uses the
same variable so both the Makefile (GOLANGCI_LINT_VERSION) and hack/lint.sh are
consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f4d9e5ee-02ca-4d8e-9f40-2d89fef88127
📒 Files selected for processing (1)
Makefile
sebrandon1
force-pushed
the
golangci-lint-v2-upgrade
branch
from
baf78b8 to
3d1c1a6
Compare
sebrandon1
changed the title
sebrandon1
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@sebrandon1: This pull request references CNF-23398 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
sebrandon1
force-pushed
the
golangci-lint-v2-upgrade
branch
from
3d1c1a6 to
3be6177
Compare
|
/retest |
sebrandon1
force-pushed
the
golangci-lint-v2-upgrade
branch
from
3be6177 to
59b0c3e
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`:
- Line 431: The Makefile currently checks for golangci-lint only under $(shell
go env GOPATH)/bin causing installs when GOBIN is set; update the existence
check to use $(or $(GOBIN),$(shell go env GOPATH)/bin)/golangci-lint (or
equivalent shell logic) so it prefers $(GOBIN) when present, then fallbacks to
GOPATH/bin, and keep the install command using ${GOLANGCI_LINT_VERSION}
unchanged; specifically modify the test command that references $(shell go env
GOPATH)/bin/golangci-lint to use the computed bin dir via $(or $(GOBIN),$(shell
go env GOPATH)/bin)/golangci-lint.
🪄 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: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 9264fd52-a635-45d4-ac2b-97321049e457
📒 Files selected for processing (2)
.golangci.ymlMakefile
✅ Files skipped from review due to trivial changes (1)
- .golangci.yml
| .PHONY: prereqs | ||
| prereqs: ## Check if prerequisites are met, and installing missing dependencies | ||
| test -f $(shell go env GOPATH)/bin/golangci-lint || GOFLAGS="" go install github.com/golangci/golangci-lint/cmd/golangci-lint@${GOLANGCI_LINT_VERSION} | ||
| test -f $(shell go env GOPATH)/bin/golangci-lint || GOFLAGS="" go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@${GOLANGCI_LINT_VERSION} |
There was a problem hiding this comment.
Use $(GOBIN) for the golangci-lint existence check.
Line 431 checks only GOPATH/bin; when GOBIN is set, this can cause redundant installs on every make prereqs.
Suggested fix
- test -f $(shell go env GOPATH)/bin/golangci-lint || GOFLAGS="" go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@${GOLANGCI_LINT_VERSION}
+ test -f $(GOBIN)/golangci-lint || GOFLAGS="" go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@${GOLANGCI_LINT_VERSION}🤖 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 431, The Makefile currently checks for golangci-lint only
under $(shell go env GOPATH)/bin causing installs when GOBIN is set; update the
existence check to use $(or $(GOBIN),$(shell go env GOPATH)/bin)/golangci-lint
(or equivalent shell logic) so it prefers $(GOBIN) when present, then fallbacks
to GOPATH/bin, and keep the install command using ${GOLANGCI_LINT_VERSION}
unchanged; specifically modify the test command that references $(shell go env
GOPATH)/bin/golangci-lint to use the computed bin dir via $(or $(GOBIN),$(shell
go env GOPATH)/bin)/golangci-lint.
- Update GOLANGCI_LINT_VERSION from v1.54.2 to v2.12.2 - Update module path to use /v2/ prefix - Add .golangci.yml with version: "2" declaration for v2 compatibility Tracking: redhat-best-practices-for-k8s/telco-bot#49
sebrandon1
changed the title
sebrandon1
force-pushed
the
golangci-lint-v2-upgrade
branch
from
59b0c3e to
f6caa4a
Compare
|
@sebrandon1: all tests passed! Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
2 participants
Summary
/v2/prefix.golangci.ymlwith v2 version declarationChanges Made
GOLANGCI_LINT_VERSIONfrom v1.54.2 to v2.12.2 and module path to/v2/Test plan
Tracking: redhat-best-practices-for-k8s/telco-bot#49