Enable golang-ci linters

Additional fixes (to be squashed)

.golangci.yml

@@ -35,113 +35,36 @@ linters-settings:
       - "github.com/onsi/gomega"
       - "github.com/onsi/ginkgo/v2"
 linters:
-  enable:
-    - asasalint
-    - asciicheck
-    - bidichk
-    - bodyclose
-    - canonicalheader
-    - containedctx
-    - contextcheck
-    - copyloopvar
-    - decorder
-    - dogsled
-    - dupword
-    - durationcheck
-    - errcheck
-    - errchkjson
-    - errname
-    - errorlint
-    - exhaustive
-    - fatcontext
-    - forbidigo
-    - forcetypeassert
-    - gci
-    - ginkgolinter
-    - gocheckcompilerdirectives
-    - gochecksumtype
-    - gocritic
-    - gofmt
-    - goheader
-    - goimports
-    - gomodguard
-    - goprintffuncname
-    - gosimple
-    - gosmopolitan
-    - grouper
-    - importas
-    - inamedparam
-    - ineffassign
-    - intrange
-    - loggercheck
-    - makezero
-    - mirror
-    - misspell
-    - musttag
-    - nakedret
-    - nilerr
-    - nilnil
-    - noctx
-    - nolintlint
-    - nosprintfhostport
-    - prealloc
-    - predeclared
-    - promlinter
-    - protogetter
-    - reassign
-    - rowserrcheck
-    - sloglint
-    - spancheck
-    - sqlclosecheck
-    - staticcheck
-    - stylecheck
-    - tagalign
-    - tagliatelle
-    - tenv
-    - testableexamples
-    - testifylint
-    - thelper
-    - tparallel
-    - unconvert
-    - unparam
-    - unused
-    - usestdlibvars
-    - wastedassign
-    - whitespace
-    - zerologlint
-    - goconst
-    - gochecknoinits
-    - perfsprint
-    - revive
-    - gosec
-    - godot # End all godoc comments with period
+  enable-all: true
   disable:
     - govet # We already run with `make verify/govet`
     # We do not use
-    - gomoddirectives # We need `replace` in `go.mod`
+    - cyclop # Cyclomatic complexity
     - depguard # We don't guard against dependencies
-    - testpackage # Require separate test package to catch leaky unexported dependencies
+    - dupl # Tracks code duplication. Too much duplication in tests. False positives in non-tests
+    - execinquery # Deprecated but enabled-by-default
+    - exhaustruct # Explicitly instantiating all structs is painful for K8s structs
+    - exportloopref # Deprecated but enabled-by-default
     - funlen # Long func names happen
-    - varnamelen # Long var names happen
-    - maintidx # Maintainability index
-    - cyclop # Cyclomatic complexity
     - gocognit # Cognitive complexity
-    - wsl # Too strict of a whitespace linter
-    # TODO Investigate FIXME
-    - err113 # TODO FIXME do not create errors dynamically from scratch. Instead wrap static (package-level) error. 2 points
-    - wrapcheck # TODO ^^
-    - gochecknoglobals  # TODO FIXME A whole 2 point task on its own
-    - paralleltest # TODO FIXME (Too many tests that aren't parallelized right now, probably half a day to attempt to mark the ones we can)
-    - nestif # TODO whole refactoring/readability task, should split up
-    - godox # TODO FIXME audit our project TODOs
-    # TODO Q: Consult with team
+    - gocyclo # Cyclomatic complexity
+    - gofumpt # We don't rely on gofumpt
+    - gomnd # Magic Number Detection. Many false positives.
+    - gomoddirectives # We need `replace` in `go.mod`
+    - interfacebloat # No more than 10 interface methods
+    - ireturn # Accept interfaces return concrete types
     - lll # Limit line length
-    - gofumpt # Rely on gofumpt's stricter formatting opinions
-    - ireturn # Always accept interfaces return concrete types (gopherism)
-    - nlreturn # Always have emptyline before return
-    - nonamedreturns # No using named returns in functions. Maybe nolint a few of the places it actually leads to cleaner code
-    - exhaustruct # Forces you to explicitly instantiate all structs. REALLY painful for kubernetes structs...
-    - interfacebloat # Cloud and Mounter have 15 methods instead of linter's recommended 10... Smell or necessary? We can nolint specific ones
-    - mnd # Magic Number Detection. Many false positives, still worth with nolint?
-    - gomnd # Many false positives, still worth with nolint?
-    - dupl # Tracks code duplication. Brutal amount of duplication in tests. False positives in non-tests.
+    - maintidx # Maintainability index
+    - mnd # Magic Number Detection. Many false positives
+    - nestif # Don't allow too many nested if statements
+    - nlreturn # Always have empty line before return
+    - testpackage # Require separate test package to catch leaky unexported dependencies
+    - varnamelen # Long var names happen
+    - wsl # Too strict of a whitespace linter
+    # Consider adding in future
+    - err113 # Do not create errors dynamically from scratch. Instead, wrap static (package-level) error.
+    - wrapcheck # Same as err113
+    - gochecknoglobals  # Do not allow global variables
+    - godox # Do not allow TODOs
+    - nonamedreturns # Need to nolint/refactor a few places our code
+    - paralleltest # There are many tests that aren't parallelized

cmd/main.go

@@ -18,6 +18,7 @@ package main
 
 import (
 	"context"
+	"fmt"
 	"os"
 	"strings"
 	"time"
@@ -107,7 +108,8 @@ func main() {
 			klog.ErrorS(err, "failed to get version")
 			klog.FlushAndExit(klog.ExitFlushTimeout, 1)
 		}
-		klog.Info(versionInfo)
+		//nolint:forbidigo // Print version info without klog/timestamp
+		fmt.Println(versionInfo)
 		os.Exit(0)
 	}
 

pkg/driver/controller.go

@@ -486,11 +486,11 @@ func (d *ControllerService) ControllerGetCapabilities(ctx context.Context, req *
 	klog.V(4).InfoS("ControllerGetCapabilities: called", "args", req)
 
 	caps := make([]*csi.ControllerServiceCapability, 0, len(controllerCaps))
-	for _, cap := range controllerCaps {
+	for _, capability := range controllerCaps {
 		c := &csi.ControllerServiceCapability{
 			Type: &csi.ControllerServiceCapability_Rpc{
 				Rpc: &csi.ControllerServiceCapability_RPC{
-					Type: cap,
+					Type: capability,
 				},
 			},
 		}

pkg/mounter/mount_linux.go

@@ -54,9 +54,9 @@ func NewSafeMounterV2() (*mountutils.SafeFormatAndMount, error) {
 
 // FindDevicePath finds path of device and verifies its existence
 // if the device is not nvme, return the path directly
-// if the device is nvme, finds and returns the nvme device path eg. /dev/nvme1n1
+// if the device is nvme, finds and returns the nvme device path eg. /dev/nvme1n1.
 func (m *NodeMounter) FindDevicePath(devicePath, volumeID, partition, region string) (string, error) {
-	strippedVolumeName := strings.Replace(volumeID, "-", "", -1)
+	strippedVolumeName := strings.ReplaceAll(volumeID, "-", "")
 	canonicalDevicePath := ""
 
 	// If the given path exists, the device MAY be nvme. Further, it MAY be a
@@ -132,7 +132,7 @@ func (m *NodeMounter) FindDevicePath(devicePath, volumeID, partition, region str
 }
 
 // findNvmeVolume looks for the nvme volume with the specified name
-// It follows the symlink (if it exists) and returns the absolute path to the device
+// It follows the symlink (if it exists) and returns the absolute path to the device.
 func findNvmeVolume(findName string) (device string, err error) {
 	p := filepath.Join("/dev/disk/by-id/", findName)
 	stat, err := os.Lstat(p)
@@ -163,12 +163,12 @@ func findNvmeVolume(findName string) (device string, err error) {
 }
 
 // execRunner is a helper to inject exec.Comamnd().CombinedOutput() for verifyVolumeSerialMatch
-// Tests use a mocked version that does not actually execute any binaries
+// Tests use a mocked version that does not actually execute any binaries.
 func execRunner(name string, arg ...string) ([]byte, error) {
 	return exec.Command(name, arg...).CombinedOutput()
 }
 
-// verifyVolumeSerialMatch checks the volume serial of the device against the expected volume
+// verifyVolumeSerialMatch checks the volume serial of the device against the expected volume.
 func verifyVolumeSerialMatch(canonicalDevicePath string, strippedVolumeName string, execRunner func(string, ...string) ([]byte, error)) error {
 	// In some rare cases, a race condition can lead to the /dev/disk/by-id/ symlink becoming out of date
 	// See https://github.com/kubernetes-sigs/aws-ebs-csi-driver/issues/1224 for more info
@@ -194,7 +194,7 @@ func verifyVolumeSerialMatch(canonicalDevicePath string, strippedVolumeName stri
 	return nil
 }
 
-// PreparePublishTarget creates the target directory for the volume to be mounted
+// PreparePublishTarget creates the target directory for the volume to be mounted.
 func (m *NodeMounter) PreparePublishTarget(target string) error {
 	klog.V(4).InfoS("NodePublishVolume: creating dir", "target", target)
 	if err := m.MakeDir(target); err != nil {
@@ -203,7 +203,7 @@ func (m *NodeMounter) PreparePublishTarget(target string) error {
 	return nil
 }
 
-// IsBlockDevice checks if the given path is a block device
+// IsBlockDevice checks if the given path is a block device.
 func (m *NodeMounter) IsBlockDevice(fullPath string) (bool, error) {
 	var st unix.Stat_t
 	err := unix.Stat(fullPath, &st)
@@ -214,7 +214,7 @@ func (m *NodeMounter) IsBlockDevice(fullPath string) (bool, error) {
 	return (st.Mode & unix.S_IFMT) == unix.S_IFBLK, nil
 }
 
-// GetBlockSizeBytes gets the size of the disk in bytes
+// GetBlockSizeBytes gets the size of the disk in bytes.
 func (m *NodeMounter) GetBlockSizeBytes(devicePath string) (int64, error) {
 	output, err := m.Exec.Command("blockdev", "--getsize64", devicePath).Output()
 	if err != nil {
@@ -228,7 +228,7 @@ func (m *NodeMounter) GetBlockSizeBytes(devicePath string) (int64, error) {
 	return gotSizeBytes, nil
 }
 
-// appendPartition appends the partition to the device path
+// appendPartition appends the partition to the device path.
 func (m *NodeMounter) appendPartition(devicePath, partition string) string {
 	if partition == "" {
 		return devicePath
@@ -246,13 +246,13 @@ func (m NodeMounter) GetDeviceNameFromMount(mountPath string) (string, int, erro
 	return mountutils.GetDeviceNameFromMount(m, mountPath)
 }
 
-// IsCorruptedMnt return true if err is about corrupted mount point
+// IsCorruptedMnt return true if err is about corrupted mount point.
 func (m NodeMounter) IsCorruptedMnt(err error) bool {
 	return mountutils.IsCorruptedMnt(err)
 }
 
 // This function is mirrored in ./sanity_test.go to make sure sanity test covered this block of code
-// Please mirror the change to func MakeFile in ./sanity_test.go
+// Please mirror the change to func MakeFile in ./sanity_test.go.
 func (m *NodeMounter) MakeFile(path string) error {
 	f, err := os.OpenFile(path, os.O_CREATE, os.FileMode(0644))
 	if err != nil {
@@ -267,7 +267,7 @@ func (m *NodeMounter) MakeFile(path string) error {
 }
 
 // This function is mirrored in ./sanity_test.go to make sure sanity test covered this block of code
-// Please mirror the change to func MakeFile in ./sanity_test.go
+// Please mirror the change to func MakeFile in ./sanity_test.go.
 func (m *NodeMounter) MakeDir(path string) error {
 	err := os.MkdirAll(path, os.FileMode(0755))
 	if err != nil {
@@ -279,28 +279,28 @@ func (m *NodeMounter) MakeDir(path string) error {
 }
 
 // This function is mirrored in ./sanity_test.go to make sure sanity test covered this block of code
-// Please mirror the change to func MakeFile in ./sanity_test.go
+// Please mirror the change to func MakeFile in ./sanity_test.go.
 func (m *NodeMounter) PathExists(path string) (bool, error) {
 	return mountutils.PathExists(path)
 }
 
-// Resize resizes the filesystem of the given devicePath
+// Resize resizes the filesystem of the given devicePath.
 func (m *NodeMounter) Resize(devicePath, deviceMountPath string) (bool, error) {
 	return mountutils.NewResizeFs(m.Exec).Resize(devicePath, deviceMountPath)
 }
 
-// NeedResize checks if the filesystem of the given devicePath needs to be resized
+// NeedResize checks if the filesystem of the given devicePath needs to be resized.
 func (m *NodeMounter) NeedResize(devicePath string, deviceMountPath string) (bool, error) {
 	return mountutils.NewResizeFs(m.Exec).NeedResize(devicePath, deviceMountPath)
 }
 
-// Unpublish unmounts the given path
+// Unpublish unmounts the given path.
 func (m *NodeMounter) Unpublish(path string) error {
 	// On linux, unpublish and unstage both perform an unmount
 	return m.Unstage(path)
 }
 
-// Unstage unmounts the given path
+// Unstage unmounts the given path.
 func (m *NodeMounter) Unstage(path string) error {
 	err := mountutils.CleanupMountPoint(path, m, false)
 	// Ignore the error when it contains "not mounted", because that indicates the

pkg/mounter/mount_linux_test.go

@@ -27,7 +27,6 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"k8s.io/mount-utils"
-
 	utilexec "k8s.io/utils/exec"
 	fakeexec "k8s.io/utils/exec/testing"
 )
@@ -138,7 +137,6 @@ func TestMakeFile(t *testing.T) {
 	if exists, err := mountObj.PathExists(targetPath); !exists {
 		t.Fatalf("Expect no error but got: %v", err)
 	}
-
 }
 
 func TestPathExists(t *testing.T) {
@@ -165,7 +163,6 @@ func TestPathExists(t *testing.T) {
 	if exists {
 		t.Fatalf("Expected file %s to not exist", targetPath)
 	}
-
 }
 
 func TestGetDeviceName(t *testing.T) {
@@ -186,7 +183,6 @@ func TestGetDeviceName(t *testing.T) {
 	if _, _, err := mountObj.GetDeviceNameFromMount(targetPath); err != nil {
 		t.Fatalf("Expect no error but got: %v", err)
 	}
-
 }
 
 func TestFindDevicePath(t *testing.T) {

pkg/util/util_test.go

@@ -24,9 +24,8 @@ import (
 	"reflect"
 	"testing"
 
-	"github.com/stretchr/testify/assert"
-
 	csi "github.com/container-storage-interface/spec/lib/go/csi"
+	"github.com/stretchr/testify/assert"
 )
 
 func TestRoundUpBytes(t *testing.T) {
@@ -119,7 +118,6 @@ func TestParseEndpoint(t *testing.T) {
 				if err.Error() != tc.expErr.Error() {
 					t.Fatalf("Expecting err: expected %v, got %v", tc.expErr, err)
 				}
-
 			} else {
 				if err != nil {
 					t.Fatalf("err is not nil. got: %v", err)
@@ -134,7 +132,6 @@ func TestParseEndpoint(t *testing.T) {
 			}
 		})
 	}
-
 }
 
 func TestGetAccessModes(t *testing.T) {