add and fix linter findings (#611)

Signed-off-by: Prajyot-Parab <[email protected]>

Author: Prajyot-Parab

.golangci.yml

@@ -1,16 +1,209 @@
 linters:
   disable-all: true
   enable:
+  - asciicheck
+  - bodyclose
   - deadcode
+  - depguard
+  - dogsled
   - errcheck
+  - exportloopref
+  - gci
+  - goconst
+  - gocritic
+  - godot
+  - gofmt
+  - goimports
+  - goprintffuncname
+  - gosec
   - gosimple
   - govet
+  - ifshort
+  - importas
   - ineffassign
+  - misspell
+  - nakedret
+  - nilerr
+  - noctx
+  - nolintlint
+  - prealloc
+  - predeclared
+  - revive
+  - rowserrcheck
   - staticcheck
   - structcheck
+  - stylecheck
+  - thelper
   - typecheck
+  - unconvert
+  - unparam
   - unused
   - varcheck
+  - whitespace
+
+linters-settings:
+  godot:
+    #   declarations - for top level declaration comments (default);
+    #   toplevel     - for top level comments;
+    #   all          - for all comments.
+    scope: toplevel
+    exclude:
+    - '^ \+.*'
+    - '^ ANCHOR.*'
+  ifshort:
+    # Maximum length of variable declaration measured in number of characters, after which linter won't suggest using short syntax.
+    max-decl-chars: 50
+  gci:
+    local-prefixes: "sigs.k8s.io/cluster-api-provider-ibmcloud"
+  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
+      # CAPI
+      - pkg: sigs.k8s.io/cluster-api/api/v1alpha3
+        alias: capiv1alpha3
+      - pkg: sigs.k8s.io/cluster-api/api/v1alpha4
+        alias: capiv1alpha4
+      - pkg: sigs.k8s.io/cluster-api/api/v1beta1
+        alias: capiv1beta1
+      # CAPI-IBMCLOUD
+      - pkg: sigs.k8s.io/cluster-api-provider-ibmcloud/api/v1alpha3
+        alias: infrav1alpha3
+      - pkg: sigs.k8s.io/cluster-api-provider-ibmcloud/api/v1alpha4
+        alias: infrav1alpha4
+      - pkg: sigs.k8s.io/cluster-api-provider-ibmcloud/api/v1beta1
+        alias: infrav1beta1
+  nolintlint:
+    allow-unused: false
+    allow-leading-space: false
+    require-specific: true
+  staticcheck:
+    go: "1.17"
+  stylecheck:
+    go: "1.17"
+  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.17"
+issues:
+  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
+  skip-files:
+  - "zz_generated.*\\.go$"
+  allow-parallel-runners: true

Makefile

@@ -152,6 +152,10 @@ endif
 lint: $(GOLANGCI_LINT) ## Lint codebase
 	$(GOLANGCI_LINT) run -v --fast=false
 
+.PHONY: lint-fix
+lint-fix: $(GOLANGCI_LINT) ## Lint the codebase and run auto-fixers if supported by the linter
+	GOLANGCI_LINT_EXTRA_ARGS=--fix $(MAKE) lint
+
 ## --------------------------------------
 ## Testing
 ## --------------------------------------

api/v1alpha3/conversion.go

@@ -18,88 +18,89 @@ package v1alpha3
 
 import (
 	apiconversion "k8s.io/apimachinery/pkg/conversion"
-	"sigs.k8s.io/cluster-api-provider-ibmcloud/api/v1beta1"
-	clusterv1alpha3 "sigs.k8s.io/cluster-api/api/v1alpha3"
-	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
+	capiv1alpha3 "sigs.k8s.io/cluster-api/api/v1alpha3"
+	capiv1beta1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	"sigs.k8s.io/controller-runtime/pkg/conversion"
+
+	infrav1beta1 "sigs.k8s.io/cluster-api-provider-ibmcloud/api/v1beta1"
 )
 
-func Convert_v1alpha3_APIEndpoint_To_v1beta1_APIEndpoint(in *clusterv1alpha3.APIEndpoint, out *clusterv1.APIEndpoint, s apiconversion.Scope) error {
-	return clusterv1alpha3.Convert_v1alpha3_APIEndpoint_To_v1beta1_APIEndpoint(in, out, s)
+func Convert_v1alpha3_APIEndpoint_To_v1beta1_APIEndpoint(in *capiv1alpha3.APIEndpoint, out *capiv1beta1.APIEndpoint, s apiconversion.Scope) error {
+	return capiv1alpha3.Convert_v1alpha3_APIEndpoint_To_v1beta1_APIEndpoint(in, out, s)
 }
 
-func Convert_v1beta1_APIEndpoint_To_v1alpha3_APIEndpoint(in *clusterv1.APIEndpoint, out *clusterv1alpha3.APIEndpoint, s apiconversion.Scope) error {
-	return clusterv1alpha3.Convert_v1beta1_APIEndpoint_To_v1alpha3_APIEndpoint(in, out, s)
+func Convert_v1beta1_APIEndpoint_To_v1alpha3_APIEndpoint(in *capiv1beta1.APIEndpoint, out *capiv1alpha3.APIEndpoint, s apiconversion.Scope) error {
+	return capiv1alpha3.Convert_v1beta1_APIEndpoint_To_v1alpha3_APIEndpoint(in, out, s)
 }
 
 func (src *IBMVPCCluster) ConvertTo(dstRaw conversion.Hub) error {
-	dst := dstRaw.(*v1beta1.IBMVPCCluster)
+	dst := dstRaw.(*infrav1beta1.IBMVPCCluster)
 
 	return Convert_v1alpha3_IBMVPCCluster_To_v1beta1_IBMVPCCluster(src, dst, nil)
 }
 
 func (dst *IBMVPCCluster) ConvertFrom(srcRaw conversion.Hub) error {
-	src := srcRaw.(*v1beta1.IBMVPCCluster)
+	src := srcRaw.(*infrav1beta1.IBMVPCCluster)
 
 	return Convert_v1beta1_IBMVPCCluster_To_v1alpha3_IBMVPCCluster(src, dst, nil)
 }
 
 func (src *IBMVPCClusterList) ConvertTo(dstRaw conversion.Hub) error {
-	dst := dstRaw.(*v1beta1.IBMVPCClusterList)
+	dst := dstRaw.(*infrav1beta1.IBMVPCClusterList)
 
 	return Convert_v1alpha3_IBMVPCClusterList_To_v1beta1_IBMVPCClusterList(src, dst, nil)
 }
 
 func (dst *IBMVPCClusterList) ConvertFrom(srcRaw conversion.Hub) error {
-	src := srcRaw.(*v1beta1.IBMVPCClusterList)
+	src := srcRaw.(*infrav1beta1.IBMVPCClusterList)
 
 	return Convert_v1beta1_IBMVPCClusterList_To_v1alpha3_IBMVPCClusterList(src, dst, nil)
 }
 
 func (src *IBMVPCMachine) ConvertTo(dstRaw conversion.Hub) error {
-	dst := dstRaw.(*v1beta1.IBMVPCMachine)
+	dst := dstRaw.(*infrav1beta1.IBMVPCMachine)
 
 	return Convert_v1alpha3_IBMVPCMachine_To_v1beta1_IBMVPCMachine(src, dst, nil)
 }
 
 func (dst *IBMVPCMachine) ConvertFrom(srcRaw conversion.Hub) error {
-	src := srcRaw.(*v1beta1.IBMVPCMachine)
+	src := srcRaw.(*infrav1beta1.IBMVPCMachine)
 
 	return Convert_v1beta1_IBMVPCMachine_To_v1alpha3_IBMVPCMachine(src, dst, nil)
 }
 
 func (src *IBMVPCMachineList) ConvertTo(dstRaw conversion.Hub) error {
-	dst := dstRaw.(*v1beta1.IBMVPCMachineList)
+	dst := dstRaw.(*infrav1beta1.IBMVPCMachineList)
 
 	return Convert_v1alpha3_IBMVPCMachineList_To_v1beta1_IBMVPCMachineList(src, dst, nil)
 }
 
 func (dst *IBMVPCMachineList) ConvertFrom(srcRaw conversion.Hub) error {
-	src := srcRaw.(*v1beta1.IBMVPCMachineList)
+	src := srcRaw.(*infrav1beta1.IBMVPCMachineList)
 
 	return Convert_v1beta1_IBMVPCMachineList_To_v1alpha3_IBMVPCMachineList(src, dst, nil)
 }
 
 func (src *IBMVPCMachineTemplate) ConvertTo(dstRaw conversion.Hub) error {
-	dst := dstRaw.(*v1beta1.IBMVPCMachineTemplate)
+	dst := dstRaw.(*infrav1beta1.IBMVPCMachineTemplate)
 
 	return Convert_v1alpha3_IBMVPCMachineTemplate_To_v1beta1_IBMVPCMachineTemplate(src, dst, nil)
 }
 
 func (dst *IBMVPCMachineTemplate) ConvertFrom(srcRaw conversion.Hub) error {
-	src := srcRaw.(*v1beta1.IBMVPCMachineTemplate)
+	src := srcRaw.(*infrav1beta1.IBMVPCMachineTemplate)
 
 	return Convert_v1beta1_IBMVPCMachineTemplate_To_v1alpha3_IBMVPCMachineTemplate(src, dst, nil)
 }
 
 func (src *IBMVPCMachineTemplateList) ConvertTo(dstRaw conversion.Hub) error {
-	dst := dstRaw.(*v1beta1.IBMVPCMachineTemplateList)
+	dst := dstRaw.(*infrav1beta1.IBMVPCMachineTemplateList)
 
 	return Convert_v1alpha3_IBMVPCMachineTemplateList_To_v1beta1_IBMVPCMachineTemplateList(src, dst, nil)
 }
 
 func (dst *IBMVPCMachineTemplateList) ConvertFrom(srcRaw conversion.Hub) error {
-	src := srcRaw.(*v1beta1.IBMVPCMachineTemplateList)
+	src := srcRaw.(*infrav1beta1.IBMVPCMachineTemplateList)
 
 	return Convert_v1beta1_IBMVPCMachineTemplateList_To_v1alpha3_IBMVPCMachineTemplateList(src, dst, nil)
 }

api/v1alpha3/groupversion_info.go

@@ -25,10 +25,10 @@ import (
 )
 
 var (
-	// GroupVersion is group version used to register these objects
+	// GroupVersion is group version used to register these objects.
 	GroupVersion = schema.GroupVersion{Group: "infrastructure.cluster.x-k8s.io", Version: "v1alpha3"}
 
-	// SchemeBuilder is used to add go types to the GroupVersionKind scheme
+	// SchemeBuilder is used to add go types to the GroupVersionKind scheme.
 	SchemeBuilder = &scheme.Builder{GroupVersion: GroupVersion}
 
 	// AddToScheme adds the types in this group-version to the given scheme.