rpk: bump golangci-lint to v2 and fix issues#29513
rpk: bump golangci-lint to v2 and fix issues#29513r-vasquez wants to merge 1 commit intoredpanda-data:devfrom
Conversation
aab01b6 to
c213355
Compare
|
/dt |
This bump increased the accuracy and the report showed up to 100 issues. One of the major changes that were not fixed by the `--fix` flag were: - Package naming: common -> better names - net, os, utils -> netutil, osutil, rpkutil to avoid collision with standard naming packages - Context handling in the container package. We were using a context.Background with a timeout instead of the command context. There are further improvements to be made as moving away from some of the code in rpkutil and eliminating the containerutil package but these will be handled in separate commits to not hinder the review of this one. In addition we are also bumping the golangci-lint version in our Github Actions.
c213355 to
fc8fff2
Compare
|
/dt |
There was a problem hiding this comment.
Pull request overview
This PR updates golangci-lint to v2 and resolves the newly reported issues, primarily by renaming internal packages to avoid stdlib name collisions and improving context propagation in container-related code.
Changes:
- Rename internal helper packages (
utils→rpkutil,net→netutil,os→osutil,cli/.../common→.../containerutil/.../debugbundle) and update references. - Improve context handling (notably in container workflows and editor invocation) to rely on command contexts rather than
context.Background(). - Update lint configuration and GitHub Actions to use the newer golangci-lint major version and action versions.
Reviewed changes
Copilot reviewed 182 out of 182 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/go/rpk/pkg/tuners/swappiness_test.go | Update utils usage to rpkutil in tests |
| src/go/rpk/pkg/tuners/redpanda_checkers.go | Switch net/os imports to netutil/osutil |
| src/go/rpk/pkg/tuners/network/nics.go | Refactor conditional logic to switch |
| src/go/rpk/pkg/tuners/network/nic.go | Switch utils usage to rpkutil |
| src/go/rpk/pkg/tuners/network/BUILD | Update Bazel deps from utils to rpkutil |
| src/go/rpk/pkg/tuners/net_tuners_test.go | Switch os/utils usage to osutil/rpkutil |
| src/go/rpk/pkg/tuners/net_tuners.go | Switch Proc type from os to osutil |
| src/go/rpk/pkg/tuners/net_checkers.go | Switch utils usage to rpkutil |
| src/go/rpk/pkg/tuners/irq/utils.go | Switch utils usage to rpkutil |
| src/go/rpk/pkg/tuners/irq/proc_file_test.go | Switch utils usage to rpkutil |
| src/go/rpk/pkg/tuners/irq/proc_file.go | Switch utils usage to rpkutil |
| src/go/rpk/pkg/tuners/irq/irq_tuner_mode.go | Refactor conditional logic to switch |
| src/go/rpk/pkg/tuners/irq/device_info_test.go | Switch utils usage to rpkutil |
| src/go/rpk/pkg/tuners/irq/device_info.go | Switch utils usage to rpkutil |
| src/go/rpk/pkg/tuners/irq/cpu_masks.go | Refactor mode logic to switch; ReplaceAll cleanup |
| src/go/rpk/pkg/tuners/irq/balance_service_test.go | Switch os/utils usage to osutil/rpkutil |
| src/go/rpk/pkg/tuners/irq/balance_service.go | Switch os/utils usage to osutil/rpkutil |
| src/go/rpk/pkg/tuners/irq/BUILD | Update Bazel deps from os/utils to osutil/rpkutil |
| src/go/rpk/pkg/tuners/iotune/io_tune.go | Switch Proc type from os to osutil |
| src/go/rpk/pkg/tuners/iotune/BUILD | Update Bazel deps from os to osutil |
| src/go/rpk/pkg/tuners/iotune.go | Switch Proc creation from os to osutil |
| src/go/rpk/pkg/tuners/hwloc/hwloc_cmd.go | Switch Proc type from os to osutil |
| src/go/rpk/pkg/tuners/hwloc/BUILD | Update Bazel deps from os to osutil |
| src/go/rpk/pkg/tuners/fstrim_test.go | Switch os/utils usage to osutil/rpkutil |
| src/go/rpk/pkg/tuners/fstrim.go | Switch os usage to osutil |
| src/go/rpk/pkg/tuners/factory/factory.go | Switch net/os usage to netutil/osutil |
| src/go/rpk/pkg/tuners/factory/BUILD | Update Bazel deps from net/os to netutil/osutil |
| src/go/rpk/pkg/tuners/executors/commands/write_file_lines.go | Switch utils usage to rpkutil |
| src/go/rpk/pkg/tuners/executors/commands/execute.go | Switch Proc type from os to osutil |
| src/go/rpk/pkg/tuners/executors/commands/backup_file.go | Switch utils usage to rpkutil |
| src/go/rpk/pkg/tuners/executors/commands/BUILD | Update Bazel deps from os/utils to osutil/rpkutil |
| src/go/rpk/pkg/tuners/disk_tuner.go | Switch utils usage to rpkutil |
| src/go/rpk/pkg/tuners/disk/block_devices_test.go | Switch Proc type from os to osutil |
| src/go/rpk/pkg/tuners/disk/block_devices.go | Switch os/utils usage to osutil/rpkutil |
| src/go/rpk/pkg/tuners/disk/block_device_test.go | Switch utils usage to rpkutil |
| src/go/rpk/pkg/tuners/disk/block_device.go | Switch utils usage to rpkutil |
| src/go/rpk/pkg/tuners/disk/BUILD | Update Bazel deps from os/utils to osutil/rpkutil |
| src/go/rpk/pkg/tuners/cpu/tuner.go | Switch utils usage to rpkutil |
| src/go/rpk/pkg/tuners/cpu/BUILD | Update Bazel deps from utils to rpkutil |
| src/go/rpk/pkg/tuners/aio_test.go | Switch utils usage to rpkutil |
| src/go/rpk/pkg/tuners/aio.go | Switch utils usage to rpkutil |
| src/go/rpk/pkg/tuners/BUILD | Update Bazel deps to netutil/osutil/rpkutil |
| src/go/rpk/pkg/system/utils.go | Switch Proc creation from os to osutil |
| src/go/rpk/pkg/system/systemd/dbus.go | Switch utils usage to rpkutil |
| src/go/rpk/pkg/system/systemd/BUILD | Update Bazel deps from utils to rpkutil |
| src/go/rpk/pkg/system/runtime_options.go | Switch utils usage to rpkutil |
| src/go/rpk/pkg/system/ntp.go | Switch Proc type from os to osutil |
| src/go/rpk/pkg/system/metrics.go | Switch utils usage to rpkutil |
| src/go/rpk/pkg/system/grub_test.go | Switch utils usage to rpkutil |
| src/go/rpk/pkg/system/grub.go | Switch os/utils usage to osutil/rpkutil |
| src/go/rpk/pkg/system/cgroup.go | Switch utils usage to rpkutil |
| src/go/rpk/pkg/system/BUILD | Update Bazel deps from os/utils to osutil/rpkutil |
| src/go/rpk/pkg/schemaregistry/client.go | Switch net usage to netutil |
| src/go/rpk/pkg/schemaregistry/BUILD | Update Bazel deps from net to netutil |
| src/go/rpk/pkg/rpkutil/regex_filter_test.go | Rename package to rpkutil |
| src/go/rpk/pkg/rpkutil/regex_filter.go | Rename package to rpkutil |
| src/go/rpk/pkg/rpkutil/os.go | Rename package to rpkutil |
| src/go/rpk/pkg/rpkutil/files_test.go | Update tests to rpkutil import/package |
| src/go/rpk/pkg/rpkutil/files.go | Rename package to rpkutil |
| src/go/rpk/pkg/rpkutil/collect.go | Rename package to rpkutil |
| src/go/rpk/pkg/rpkutil/chained_error.go | Rename package to rpkutil |
| src/go/rpk/pkg/rpkutil/BUILD | Rename Bazel target/importpath to rpkutil |
| src/go/rpk/pkg/plugin/plugin.go | Switch rpkos alias import to osutil |
| src/go/rpk/pkg/plugin/BUILD | Update Bazel deps from os to osutil |
| src/go/rpk/pkg/osutil/proc_test.go | Rename test package; switch to osutil/rpkutil |
| src/go/rpk/pkg/osutil/proc.go | Rename package; switch utils usage to rpkutil |
| src/go/rpk/pkg/osutil/lock.go | Rename package to osutil |
| src/go/rpk/pkg/osutil/file_windows.go | Rename package to osutil |
| src/go/rpk/pkg/osutil/file_test.go | Rename package to osutil |
| src/go/rpk/pkg/osutil/file_all.go | Rename package to osutil |
| src/go/rpk/pkg/osutil/file.go | Add ctx plumbing for editor helpers |
| src/go/rpk/pkg/osutil/directory.go | Rename package to osutil |
| src/go/rpk/pkg/osutil/commands.go | Rename package to osutil |
| src/go/rpk/pkg/osutil/BUILD | Rename Bazel target/importpath to osutil |
| src/go/rpk/pkg/oauth/load.go | Switch rpkos alias import to osutil |
| src/go/rpk/pkg/oauth/BUILD | Update Bazel deps from os to osutil |
| src/go/rpk/pkg/netutil/interfaces.go | Rename package; switch utils usage to rpkutil |
| src/go/rpk/pkg/netutil/hostport_test.go | Rename package to netutil |
| src/go/rpk/pkg/netutil/hostport.go | Rename package to netutil |
| src/go/rpk/pkg/netutil/BUILD | Rename Bazel target/importpath to netutil |
| src/go/rpk/pkg/config/rpk_yaml.go | Switch rpkos alias import; Duration marshal tweak |
| src/go/rpk/pkg/config/redpanda_yaml.go | Switch rpkos alias import to osutil |
| src/go/rpk/pkg/config/profile_doc.go | Replace fmt.Sprintf+WriteString with Fprintf |
| src/go/rpk/pkg/config/params.go | Switch net alias import to netutil |
| src/go/rpk/pkg/config/config_test.go | Switch utils usage to rpkutil |
| src/go/rpk/pkg/config/config.go | Normalize out.Die error strings |
| src/go/rpk/pkg/config/BUILD | Update Bazel deps to netutil/osutil/rpkutil |
| src/go/rpk/pkg/cli/transform/buildpack/buildpack.go | Switch os/utils usage to osutil/rpkutil |
| src/go/rpk/pkg/cli/transform/buildpack/BUILD | Update Bazel deps to osutil/rpkutil |
| src/go/rpk/pkg/cli/topic/utils.go | Switch utils usage to rpkutil |
| src/go/rpk/pkg/cli/topic/trim.go | Normalize out.Die error strings |
| src/go/rpk/pkg/cli/topic/describe_storage.go | Normalize out.Die error strings |
| src/go/rpk/pkg/cli/topic/consume.go | Simplify boolean logic |
| src/go/rpk/pkg/cli/topic/config.go | Normalize out.Die error strings |
| src/go/rpk/pkg/cli/topic/add_partitions.go | Normalize out.Die error strings |
| src/go/rpk/pkg/cli/topic/BUILD | Update Bazel deps from utils to rpkutil |
| src/go/rpk/pkg/cli/shadow/update.go | Switch rpkos alias; pass cmd context into editor |
| src/go/rpk/pkg/cli/shadow/config.go | Switch rpkos alias; replace Sprintf with Fprintf |
| src/go/rpk/pkg/cli/shadow/BUILD | Update Bazel deps from os to osutil |
| src/go/rpk/pkg/cli/security/secret/create.go | Use ReplaceAll for scope parsing |
| src/go/rpk/pkg/cli/root.go | Refactor switch on overrides |
| src/go/rpk/pkg/cli/registry/schema/get.go | Refactor switch; normalize out.Die messages |
| src/go/rpk/pkg/cli/redpanda/tune/tune.go | Simplify boolean expression |
| src/go/rpk/pkg/cli/redpanda/tune/help.go | Switch utils usage to rpkutil |
| src/go/rpk/pkg/cli/redpanda/tune/BUILD | Update Bazel deps from utils to rpkutil |
| src/go/rpk/pkg/cli/redpanda/stop_test.go | Switch os/utils usage to osutil/rpkutil |
| src/go/rpk/pkg/cli/redpanda/stop.go | Switch os/utils usage to osutil/rpkutil |
| src/go/rpk/pkg/cli/redpanda/start.go | Switch net/os usage to netutil/osutil |
| src/go/rpk/pkg/cli/redpanda/config.go | Switch net alias import to netutil |
| src/go/rpk/pkg/cli/redpanda/admin/brokers/decommission.go | Normalize out.Die message formatting |
| src/go/rpk/pkg/cli/redpanda/BUILD | Update Bazel deps to netutil/osutil/rpkutil |
| src/go/rpk/pkg/cli/profile/validate.go | Normalize out.Die error strings |
| src/go/rpk/pkg/cli/profile/prompt.go | Refactor conditional logic to switch |
| src/go/rpk/pkg/cli/profile/edit_gobals.go | Switch rpkos alias; pass cmd context into editor |
| src/go/rpk/pkg/cli/profile/edit.go | Switch rpkos alias; pass cmd context into editor |
| src/go/rpk/pkg/cli/profile/create.go | Switch container util import; refactor switch |
| src/go/rpk/pkg/cli/profile/BUILD | Update Bazel deps to containerutil/osutil |
| src/go/rpk/pkg/cli/plugin/install.go | Switch rpkos alias import to osutil |
| src/go/rpk/pkg/cli/plugin/BUILD | Update Bazel deps from os to osutil |
| src/go/rpk/pkg/cli/group/seek.go | Refactor switch; normalize out.Die messages |
| src/go/rpk/pkg/cli/group/group.go | Normalize out.Die error strings |
| src/go/rpk/pkg/cli/group/describe.go | Switch utils usage to rpkutil |
| src/go/rpk/pkg/cli/group/BUILD | Update Bazel deps from utils to rpkutil |
| src/go/rpk/pkg/cli/generate/license.go | Switch rpkos alias import to osutil |
| src/go/rpk/pkg/cli/generate/grafana.go | Normalize error string; simplify prefix check |
| src/go/rpk/pkg/cli/generate/graf/singlestat.go | Adjust GridPos accessor |
| src/go/rpk/pkg/cli/generate/graf/row.go | Adjust GridPos accessor |
| src/go/rpk/pkg/cli/generate/graf/graph.go | Adjust GridPos accessor |
| src/go/rpk/pkg/cli/generate/app.go | Switch rpkos alias import to osutil |
| src/go/rpk/pkg/cli/generate/BUILD | Update Bazel deps from os to osutil |
| src/go/rpk/pkg/cli/debug/remotebundle/start.go | Rename debug common to debugbundle; message tweak |
| src/go/rpk/pkg/cli/debug/remotebundle/download.go | Rename debug common to debugbundle |
| src/go/rpk/pkg/cli/debug/remotebundle/BUILD | Update Bazel deps to debugbundle |
| src/go/rpk/pkg/cli/debug/debugbundle/common_test.go | Rename package to debugbundle |
| src/go/rpk/pkg/cli/debug/debugbundle/common.go | Rename package; use ReplaceAll |
| src/go/rpk/pkg/cli/debug/debugbundle/BUILD | Rename Bazel target/importpath to debugbundle |
| src/go/rpk/pkg/cli/debug/bundle/bundle_linux.go | Switch debug/common+utils to debugbundle+rpkutil |
| src/go/rpk/pkg/cli/debug/bundle/bundle_k8s_linux.go | Improve DNS lookup to use ctx; rename debugbundle |
| src/go/rpk/pkg/cli/debug/bundle/bundle.go | Rename debug common to debugbundle |
| src/go/rpk/pkg/cli/debug/bundle/BUILD | Update Bazel deps to debugbundle/osutil/rpkutil |
| src/go/rpk/pkg/cli/container/stop.go | Rename container common to containerutil; pass ctx |
| src/go/rpk/pkg/cli/container/status.go | Rename container common to containerutil; pass ctx |
| src/go/rpk/pkg/cli/container/start.go | Rename container common to containerutil; propagate ctx |
| src/go/rpk/pkg/cli/container/purge.go | Rename container common to containerutil; propagate ctx |
| src/go/rpk/pkg/cli/container/containerutil/test.go | Rename package to containerutil |
| src/go/rpk/pkg/cli/container/containerutil/context.go | Rename package to containerutil |
| src/go/rpk/pkg/cli/container/containerutil/common.go | Propagate ctx through docker operations |
| src/go/rpk/pkg/cli/container/containerutil/client.go | Rename package to containerutil |
| src/go/rpk/pkg/cli/container/containerutil/BUILD | Rename Bazel target/importpath to containerutil |
| src/go/rpk/pkg/cli/container/BUILD | Update Bazel deps to containerutil/netutil |
| src/go/rpk/pkg/cli/connect/upgrade.go | Normalize out.Die error strings |
| src/go/rpk/pkg/cli/connect/install.go | Switch rpkos alias; normalize messages |
| src/go/rpk/pkg/cli/connect/connect.go | Normalize out.MaybeDie message |
| src/go/rpk/pkg/cli/connect/BUILD | Update Bazel deps from os to osutil |
| src/go/rpk/pkg/cli/cluster/storage/unmount.go | Normalize out.Die message |
| src/go/rpk/pkg/cli/cluster/storage/recovery/start.go | Switch on status code; normalize messages |
| src/go/rpk/pkg/cli/cluster/storage/mount.go | Normalize out.Die messages |
| src/go/rpk/pkg/cli/cluster/partitions/unsafe_recover.go | Normalize out.Die message |
| src/go/rpk/pkg/cli/cluster/partitions/transfer_leadership.go | Remove stray newlines in error formats |
| src/go/rpk/pkg/cli/cluster/partitions/move_status.go | Normalize out.Die / out.MaybeDie messages |
| src/go/rpk/pkg/cli/cluster/maintenance/status.go | Normalize out.Die messages |
| src/go/rpk/pkg/cli/cluster/maintenance/enable.go | Switch on status code; normalize messages |
| src/go/rpk/pkg/cli/cluster/maintenance/disable.go | Normalize out.MaybeDie / out.Die messages |
| src/go/rpk/pkg/cli/cluster/config/set.go | Switch on operation state; normalize messages |
| src/go/rpk/pkg/cli/cluster/config/reset.go | Normalize out.MaybeDie message |
| src/go/rpk/pkg/cli/cluster/config/import.go | Normalize out.MaybeDie/out.Die messages |
| src/go/rpk/pkg/cli/cluster/config/get.go | Normalize out.Die/out.MaybeDie messages |
| src/go/rpk/pkg/cli/cluster/config/export.go | Normalize out.Die messages |
| src/go/rpk/pkg/cli/cluster/config/edit.go | Use exec.CommandContext |
| src/go/rpk/pkg/cli/cloud/mcp/mcp.go | Switch rpkos alias; normalize out messages |
| src/go/rpk/pkg/cli/cloud/mcp/BUILD | Update Bazel deps from os to osutil |
| src/go/rpk/pkg/cli/cloud/logout.go | Switch rpkos alias import to osutil |
| src/go/rpk/pkg/cli/cloud/login.go | Switch container common to containerutil |
| src/go/rpk/pkg/cli/cloud/byoc/install.go | Switch rpkos alias import to osutil |
| src/go/rpk/pkg/cli/cloud/byoc/BUILD | Update Bazel deps from os to osutil |
| src/go/rpk/pkg/cli/cloud/BUILD | Update Bazel deps to containerutil/osutil |
| src/go/rpk/cmd/rpk/BUILD | Update ldflag symbol path to containerutil |
| src/go/rpk/build.sh | Update ldflag package path to containerutil |
| src/go/rpk/Makefile | Update ldflag package path to containerutil |
| src/go/rpk/.golangci.yml | Migrate golangci-lint configuration to v2 layout |
| src/go/.goreleaser.yaml | Update ldflag symbol path to containerutil |
| .github/workflows/lint-golang.yml | Bump GitHub Actions + golangci-lint action versions |
Comments suppressed due to low confidence (1)
src/go/rpk/pkg/cli/container/start.go:1
- The goroutine mutates the outer-scoped
errvariable (err = ...,state, err = ...) concurrently across iterations, which can introduce a data race and incorrect error handling. Makeerra function-local variable inside the closure (e.g., useif err := ...; err != nil { ... }/state, err := ...) and avoid writing to shared variables from multiple goroutines.
// Copyright 2020 Redpanda Data, Inc.
| return nil, fmt.Errorf("stranded Redpanda Console container detected; please run 'rpk container purge' and try again") | ||
| } | ||
| grp, _ := errgroup.WithContext(context.Background()) | ||
| grp, grpCtx := errgroup.WithContext(ctx) |
There was a problem hiding this comment.
The goroutine mutates the outer-scoped err variable (err = ..., state, err = ...) concurrently across iterations, which can introduce a data race and incorrect error handling. Make err a function-local variable inside the closure (e.g., use if err := ...; err != nil { ... } / state, err := ...) and avoid writing to shared variables from multiple goroutines.
| // Encode output | ||
| outBytes, err := yaml.Marshal(content) | ||
| out.MaybeDie(err, "Serialization error: %v", configCacheFile, err) | ||
| out.MaybeDie(err, "serialization error: %v", configCacheFile, err) |
There was a problem hiding this comment.
The format string has one %v but two formatting arguments (configCacheFile, err), which will produce noisy %!(EXTRA ...) output. Either include both in the format string (e.g., "serialization error writing %s: %v") or pass only the intended argument.
| out.MaybeDie(err, "serialization error: %v", configCacheFile, err) | |
| out.MaybeDie(err, "serialization error writing %q: %v", configCacheFile, err) |
This bump increased the accuracy and the report
showed up to 100 issues. One of the major changes
that were not fixed by the
--fixflag were:There are further improvements to be made as
moving away from some of the code in rpkutil and
eliminating the containerutil package but these
will be handled in separate commits to not hinder
the review of this one.
In addition we are also bumping the golangci-lint
version in our Github Actions.
Backports Required
Release Notes