Improved linting and fixed linting issues in code

Author: anshuman-agarwala

.golangci.yml

@@ -0,0 +1,202 @@
+linters:
+  disable-all: true
+  enable:
+    - asasalint
+    - asciicheck
+    - bodyclose
+    - containedctx
+    - copyloopvar
+    - decorder
+    - dogsled
+    - errcheck
+    - errchkjson
+    - execinquery
+    # - gci
+    - goconst
+    # - gocritic
+    - gocyclo
+    - godot
+    - gofmt
+    - goimports
+    - goprintffuncname
+    # - gosec
+    - gosimple
+    - govet
+    # - importas
+    - ineffassign
+    - misspell
+    - nakedret
+    - nilerr
+    - noctx
+    - nolintlint
+    - nosprintfhostport
+    - prealloc
+    - predeclared
+    - reassign
+    # - revive
+    - rowserrcheck
+    - staticcheck
+    # - stylecheck
+    # - thelper
+    - typecheck
+    - unconvert
+    - unparam
+    - unused
+    - usestdlibvars
+    - whitespace
+
+linters-settings:
+  gocyclo:
+    min-complexity: 20
+  godot:
+    #   declarations - for top level declaration comments (default);
+    #   toplevel     - for top level comments;
+    #   all          - for all comments.
+    scope: toplevel
+    exclude:
+      - '^ \+.*'
+      - '^ ANCHOR.*'
+  gci:
+    sections:
+      - standard
+      - default
+      - prefix(github.com/IBM)
+      - prefix(k8s.io)
+      - prefix(sigs.k8s.io)
+      - blank
+      - dot
+  importas:
+    no-unaliased: true
+    alias:
+      # Kubernetes
+      - pkg: k8s.io/api/core/v1
+        alias: corev1
+      - pkg: k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1
+        alias: apiextensionsv1
+      - pkg: k8s.io/apimachinery/pkg/apis/meta/v1
+        alias: metav1
+      - pkg: k8s.io/apimachinery/pkg/api/errors
+        alias: apierrors
+      - pkg: k8s.io/apimachinery/pkg/util/errors
+        alias: kerrors
+      # Controller Runtime
+      - pkg: sigs.k8s.io/controller-runtime
+        alias: ctrl
+  nolintlint:
+    allow-unused: false
+    # allow-leading-space: false
+    require-specific: true
+  gosec:
+    excludes:
+      - G307 # Deferring unsafe method "Close" on type "\*os.File"
+      - G108 # Profiling endpoint is automatically exposed on /debug/pprof
+  gocritic:
+    enabled-tags:
+      - experimental
+    disabled-checks:
+      - appendAssign
+      - dupImport # https://github.com/go-critic/go-critic/issues/845
+      - evalOrder
+      - ifElseChain
+      - octalLiteral
+      - regexpSimplify
+      - sloppyReassign
+      - truncateCmp
+      - typeDefFirst
+      - unnamedResult
+      - unnecessaryDefer
+      - whyNoLint
+      - wrapperFunc
+  unused:
+    go: "1.22"
+issues:
+  exclude-files:
+    - "zz_generated.*\\.go$"
+  max-same-issues: 0
+  max-issues-per-linter: 0
+  # We are disabling default golangci exclusions because we want to help reviewers to focus on reviewing the most relevant
+  # changes in PRs and avoid nitpicking.
+  exclude-use-default: false
+  exclude-rules:
+    - linters:
+        - gci
+      path: _test\.go
+    - linters:
+        - revive
+      text: "exported: exported method .*\\.(Reconcile|SetupWithManager|SetupWebhookWithManager) should have comment or be unexported"
+    - linters:
+        - errcheck
+      text: Error return value of .((os\.)?std(out|err)\..*|.*Close|.*Flush|os\.Remove(All)?|.*print(f|ln)?|os\.(Un)?Setenv). is not checked
+    # Exclude some packages or code to require comments, for example test code, or fake clients.
+    - linters:
+        - revive
+      text: exported (method|function|type|const) (.+) should have comment or be unexported
+      source: (func|type).*Fake.*
+    - linters:
+        - revive
+      text: exported (method|function|type|const) (.+) should have comment or be unexported
+      path: fake_\.go
+    - linters:
+        - revive
+      text: exported (method|function|type|const) (.+) should have comment or be unexported
+      path: "(framework|e2e)/.*.go"
+    # Disable unparam "always receives" which might not be really
+    # useful when building libraries.
+    - linters:
+        - unparam
+      text: always receives
+    # Dot imports for gomega or ginkgo are allowed
+    # within test files.
+    - path: _test\.go
+      text: should not use dot imports
+    - path: (framework|e2e)/.*.go
+      text: should not use dot imports
+    - path: _test\.go
+      text: cyclomatic complexity
+    # Append should be able to assign to a different var/slice.
+    - linters:
+        - gocritic
+      text: "appendAssign: append result not assigned to the same slice"
+    # Disable linters for conversion
+    - linters:
+        - staticcheck
+      text: "SA1019: in.(.+) is deprecated"
+      path: .*(api|types)\/.*\/.*conversion.*\.go$
+    - linters:
+        - revive
+      text: exported (method|function|type|const) (.+) should have comment or be unexported
+      path: .*(api|types)\/.*\/.*conversion.*\.go$
+    - linters:
+        - revive
+      text: "var-naming: don't use underscores in Go names;"
+      path: .*(api|types)\/.*\/.*conversion.*\.go$
+    - linters:
+        - revive
+      text: "receiver-naming: receiver name"
+      path: .*(api|types)\/.*\/.*conversion.*\.go$
+    - linters:
+        - stylecheck
+      text: "ST1003: should not use underscores in Go names;"
+      path: .*(api|types)\/.*\/.*conversion.*\.go$
+    - linters:
+        - stylecheck
+      text: "ST1016: methods on the same type should have the same receiver name"
+      path: .*(api|types)\/.*\/.*conversion.*\.go$
+    # hack/tools
+    - linters:
+        - typecheck
+      text: import (".+") is a program, not an importable package
+      path: ^tools\.go$
+    # We don't care about defer in for loops in test files.
+    - linters:
+        - gocritic
+      text: "deferInLoop: Possible resource leak, 'defer' is called in the 'for' loop"
+      path: _test\.go
+
+run:
+  timeout: 10m
+  build-tags:
+    - tools
+    - e2e
+  allow-parallel-runners: true
+  go: "1.22"

adhoc-controllers/controllers/nodeupdate_controller.go

@@ -24,12 +24,13 @@ import (
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/klog/v2"
+
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/ibm-powervs-block-csi-driver/pkg/cloud"
 )
 
-// NodeUpdateReconciler reconciles a NodeUpdate object
+// NodeUpdateReconciler reconciles a NodeUpdate object.
 type NodeUpdateReconciler struct {
 	Client client.Client
 	Scheme *runtime.Scheme
@@ -41,7 +42,6 @@ type NodeUpdateReconciler struct {
 // For more details, check Reconcile and its Result here:
 // - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.10.0/pkg/reconcile
 func (r *NodeUpdateReconciler) Reconcile(_ context.Context, req ctrl.Request) (ctrl.Result, error) {
-
 	// Fetch the Node instance
 	node := corev1.Node{}
 	err := r.Client.Get(context.Background(), req.NamespacedName, &node)

adhoc-controllers/main.go

@@ -20,21 +20,22 @@ import (
 	"flag"
 	"os"
 
-	// Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.)
-	// to ensure that exec-entrypoint and run can make use of them.
-	_ "k8s.io/client-go/plugin/pkg/client/auth"
-
 	"k8s.io/apimachinery/pkg/runtime"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
+
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/healthz"
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 	"sigs.k8s.io/controller-runtime/pkg/metrics/server"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
-
 	"sigs.k8s.io/ibm-powervs-block-csi-driver/adhoc-controllers/controllers"
+
 	//+kubebuilder:scaffold:imports
+
+	// Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.)
+	// to ensure that exec-entrypoint and run can make use of them.
+	_ "k8s.io/client-go/plugin/pkg/client/auth"
 )
 
 var (

cmd/main.go

@@ -16,14 +16,14 @@ limitations under the License.
 
 package main
 
-//DONE
+// DONE.
 
 import (
 	"flag"
 
-	"sigs.k8s.io/ibm-powervs-block-csi-driver/pkg/driver"
-
 	"k8s.io/klog/v2"
+
+	"sigs.k8s.io/ibm-powervs-block-csi-driver/pkg/driver"
 )
 
 func main() {

cmd/options.go

@@ -16,17 +16,17 @@ limitations under the License.
 
 package main
 
-//DONE
+// DONE.
 
 import (
 	"flag"
 	"os"
 	"strings"
 
+	"k8s.io/klog/v2"
+
 	"sigs.k8s.io/ibm-powervs-block-csi-driver/cmd/options"
 	"sigs.k8s.io/ibm-powervs-block-csi-driver/pkg/driver"
-
-	"k8s.io/klog/v2"
 )
 
 // Options is the combined set of options for all operating modes.
@@ -38,7 +38,7 @@ type Options struct {
 	*options.NodeOptions
 }
 
-// used for testing
+// used for testing.
 var osExit = os.Exit
 
 // GetOptions parses the command line options and returns a struct that contains
@@ -63,7 +63,7 @@ func GetOptions(fs *flag.FlagSet) *Options {
 
 		switch {
 		case cmd == string(driver.ControllerMode):
-			//controllerOptions.AddFlags(fs)
+			// controllerOptions.AddFlags(fs)
 			args = os.Args[2:]
 			mode = driver.ControllerMode
 
@@ -73,12 +73,12 @@ func GetOptions(fs *flag.FlagSet) *Options {
 			mode = driver.NodeMode
 
 		case cmd == string(driver.AllMode):
-			//controllerOptions.AddFlags(fs)
+			// controllerOptions.AddFlags(fs)
 			nodeOptions.AddFlags(fs)
 			args = os.Args[2:]
 
 		case strings.HasPrefix(cmd, "-"):
-			//controllerOptions.AddFlags(fs)
+			// controllerOptions.AddFlags(fs)
 			nodeOptions.AddFlags(fs)
 			args = os.Args[1:]
 
@@ -95,7 +95,7 @@ func GetOptions(fs *flag.FlagSet) *Options {
 	if *version {
 		info, err := driver.GetVersionJSON()
 		if err != nil {
-			klog.Fatalf("error while retriving the CSI driver version. err: %v", err)
+			klog.Fatalf("error while retrieving the CSI driver version. err: %v", err)
 		}
 		klog.Info(info)
 		osExit(0)

cmd/options/controller_options.go

@@ -16,23 +16,23 @@ limitations under the License.
 
 package options
 
-//DONE
-
-//// ControllerOptions contains options and configuration settings for the controller service.
-//type ControllerOptions struct {
-//	// ExtraTags is a map of tags that will be attached to each dynamically provisioned
-//	// resource.
-//	ExtraTags map[string]string
-//	//// ExtraVolumeTags is a map of tags that will be attached to each dynamically provisioned
-//	//// volume.
-//	//// DEPRECATED: Use ExtraTags instead.
-//	//ExtraVolumeTags map[string]string
-//	// ID of the kubernetes cluster.
-//	KubernetesClusterID string
-//}
+/*
+// ControllerOptions contains options and configuration settings for the controller service.
+type ControllerOptions struct {
+	// ExtraTags is a map of tags that will be attached to each dynamically provisioned
+	// resource.
+	ExtraTags map[string]string
+	//// ExtraVolumeTags is a map of tags that will be attached to each dynamically provisioned
+	//// volume.
+	//// DEPRECATED: Use ExtraTags instead.
+	//ExtraVolumeTags map[string]string
+	// ID of the kubernetes cluster.
+	KubernetesClusterID string
+}
 
-//func (s *ControllerOptions) AddFlags(fs *flag.FlagSet) {
-//	//fs.Var(cliflag.NewMapStringString(&s.ExtraTags), "extra-tags", "Extra tags to attach to each dynamically provisioned resource. It is a comma separated list of key value pairs like '<key1>=<value1>,<key2>=<value2>'")
-//	//fs.Var(cliflag.NewMapStringString(&s.ExtraVolumeTags), "extra-volume-tags", "DEPRECATED: Please use --extra-tags instead. Extra volume tags to attach to each dynamically provisioned volume. It is a comma separated list of key value pairs like '<key1>=<value1>,<key2>=<value2>'")
-//	//fs.StringVar(&s.KubernetesClusterID, "k8s-tag-cluster-id", "", "ID of the Kubernetes cluster used for tagging provisioned EBS volumes (optional).")
-//}
+func (s *ControllerOptions) AddFlags(fs *flag.FlagSet) {
+	fs.Var(cliflag.NewMapStringString(&s.ExtraTags), "extra-tags", "Extra tags to attach to each dynamically provisioned resource. It is a comma separated list of key value pairs like '<key1>=<value1>,<key2>=<value2>'")
+	fs.Var(cliflag.NewMapStringString(&s.ExtraVolumeTags), "extra-volume-tags", "DEPRECATED: Please use --extra-tags instead. Extra volume tags to attach to each dynamically provisioned volume. It is a comma separated list of key value pairs like '<key1>=<value1>,<key2>=<value2>'")
+	fs.StringVar(&s.KubernetesClusterID, "k8s-tag-cluster-id", "", "ID of the Kubernetes cluster used for tagging provisioned EBS volumes (optional).")
+}.
+*/

cmd/options/node_options.go

@@ -16,7 +16,7 @@ limitations under the License.
 
 package options
 
-//DONE
+// DONE.
 
 import (
 	"flag"

cmd/options/server_options.go

@@ -16,7 +16,7 @@ limitations under the License.
 
 package options
 
-//DONE
+// DONE.
 
 import (
 	"flag"