Sync golangci-lint configuration to core CAPI

.golangci.yml

@@ -1,15 +1,30 @@
+run:
+  timeout: 10m
+  skip-files:
+    - "zz_generated.*\\.go$"
+    - "contrib/.*"
+    - "apis/v1alpha3/.*"
+    - "apis/v1alpha4/.*"
+  allow-parallel-runners: true
+
 linters:
   disable-all: true
   enable:
+  - asasalint
   - asciicheck
+  - bidichk
   - bodyclose
+  # - containedctx
   - dogsled
+  # - dupword
+  - durationcheck
   - errcheck
+  # - errchkjson
   - exportloopref
   - gci
+  # - ginkgolinter
   - goconst
   - gocritic
-  - gocyclo
   - godot
   - gofmt
   - goimports
@@ -24,6 +39,7 @@ linters:
   - nilerr
   - noctx
   - nolintlint
+  - nosprintfhostport
   - prealloc
   - predeclared
   - revive
@@ -35,37 +51,227 @@ linters:
   - unconvert
   - unparam
   - unused
+  - usestdlibvars
   - whitespace
 
 linters-settings:
   gci:
-    local-prefixes: sigs.k8s.io/cluster-api-provider-vsphere
-run:
-  skip-files:
-  - ".*zz_generated.*\\.go"
-  - "contrib/.*"
-  - "apis/v1alpha3/.*"
-  - "apis/v1alpha4/.*"
-  timeout: 5m
-issue:
-  max-same-issues: 0
-  max-per-linter: 0
+    local-prefixes: "sigs.k8s.io/cluster-api-provider-vsphere"
+  godot:
+    #   declarations - for top level declaration comments (default);
+    #   toplevel     - for top level comments;
+    #   all          - for all comments.
+    scope: toplevel
+    exclude:
+      - '^ \+.*'
+      - '^ ANCHOR.*'
+  # gocritic:
+  #   enabled-tags:
+  #     - diagnostic
+  #     - experimental
+  #     - performance
+  #   disabled-checks:
+  #     - appendAssign
+  #     - dupImport # https://github.com/go-critic/go-critic/issues/845
+  #     - evalOrder
+  #     - ifElseChain
+  #     - octalLiteral
+  #     - regexpSimplify
+  #     - sloppyReassign
+  #     - truncateCmp
+  #     - typeDefFirst
+  #     - unnamedResult
+  #     - unnecessaryDefer
+  #     - whyNoLint
+  #     - wrapperFunc
+  #     - rangeValCopy
+  #     - hugeParam
+  # 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
+  #     - pkg: k8s.io/component-base/logs/api/v1
+  #       alias: logsv1
+  #     # Controller Runtime
+  #     - pkg: sigs.k8s.io/controller-runtime
+  #       alias: ctrl
+  #     # CAPV
+  #     - pkg: sigs.k8s.io/cluster-api-provider-vsphere/apis/v1beta1
+  #       alias: infrav1
+  #     - pkg: sigs.k8s.io/cluster-api-provider-vsphere/apis/v1alpha3
+  #       alias: infrav1alpha3
+  #     - pkg: sigs.k8s.io/cluster-api-provider-vsphere/apis/v1alpha4
+  #       alias: infrav1alpha4
+  #     - pkg: sigs.k8s.io/cluster-api-provider-vsphere/apis/vmware/v1beta1
+  #       alias: vmwarev1
+  #     # VMware Operator
+  #     - pkg: "github.com/vmware-tanzu/vm-operator/api/v1alpha1"
+  #       alias: vmoprv1 
+  #     # CABPK
+  #     - pkg: sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1beta1
+  #       alias: bootstrapv1
+  #     # KCP
+  #     - pkg: sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1beta1
+  #       alias: controlplanev1
+  #     # CAPI
+  #     - pkg: sigs.k8s.io/cluster-api/api/v1alpha3
+  #       alias: clusterv1alpha3
+  #     - pkg: sigs.k8s.io/cluster-api/api/v1alpha4
+  #       alias: clusterv1alpha4
+  #     - pkg: sigs.k8s.io/cluster-api/api/v1beta1
+  #       alias: clusterv1
+  #     # CAPI exp
+  #     - pkg: sigs.k8s.io/cluster-api/exp/api/v1beta1
+  #       alias: expv1
+  #     # CAPI exp addons
+  #     - pkg: sigs.k8s.io/cluster-api/exp/addons/api/v1beta1
+  #       alias: addonsv1
+  # nolintlint:
+  #   allow-unused: false
+  #   allow-leading-space: false
+  #   require-specific: true
+  revive:
+    rules:
+      # The following rules are recommended https://github.com/mgechev/revive#recommended-configuration
+      - name: blank-imports
+      - name: context-as-argument
+      - name: context-keys-type
+      - name: dot-imports
+      - name: error-return
+      - name: error-strings
+      - name: error-naming
+      - name: exported
+      - name: if-return
+      - name: increment-decrement
+      - name: var-naming
+      - name: var-declaration
+      - name: package-comments
+      - name: range
+      - name: receiver-naming
+      - name: time-naming
+      - name: unexported-return
+      - name: indent-error-flow
+      - name: errorf
+      - name: empty-block
+      - name: superfluous-else
+      - name: unused-parameter
+      - name: unreachable-code
+      - name: redefines-builtin-id
+      #
+      # Rules in addition to the recommended configuration above.
+      #
+      - name: bool-literal-in-expr
+      - name: constant-logical-expr
+
 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:
-    # Specific exclude rules for deprecated fields that are still part of the codebase. These
-    # should be removed as the referenced deprecated item is removed from the project.
-    - linters:
-      - staticcheck
-      text: "SA1019: failureDomain.AutoConfigure is deprecated"
-    - path: "test/e2e/*"
-      linters:
-        - gosec
-      text: "G106:"
-    - linters:
-        - revive
-      text: "dot-imports"
-      path: ".*test.*"
-    - linters:
-        - stylecheck
-      text: "ST1001"
-      path: ".*test.*"
+  # Specific exclude rules for deprecated fields that are still part of the codebase. These
+  # should be removed as the referenced deprecated item is removed from the project.
+
+  # Exclude-rules shared with CAPI
+  # Specific exclude rules for deprecated packages that are still part of the codebase. These
+  # should be removed as the referenced deprecated packages are removed from the project.
+  - linters:
+    - staticcheck
+    text: "SA1019: .* is deprecated: This package will be removed in one of the next releases."
+  - 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
+  # Dot imports for gomega and ginkgo are allowed
+  # within test files and test utils.
+  - path: _test\.go
+    text: should not use dot imports
+  - path: ^test/.*.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
+    # Checking if an error is nil to just after return the error or nil is redundant
+    text: "if-return: redundant if ...; err != nil check, just return error instead"
+    # Ignoring stylistic checks for generated code
+    path: .*(api|types|test)\/.*\/conversion.*\.go$
+  - linters:
+    - revive
+    # Exported function and methods should have comments. This warns on undocumented exported functions and methods.
+    text: exported (method|function|type|const) (.+) should have comment or be unexported
+    # Ignoring stylistic checks for generated code
+    path: .*(api|types|test)\/.*\/conversion.*\.go$
+  - linters:
+    - revive
+    # This rule warns when initialism, variable or package naming conventions are not followed.
+    text: "var-naming: don't use underscores in Go names;"
+    # Ignoring stylistic checks for generated code
+    path: .*(api|types|test)\/.*\/conversion.*\.go$
+  - linters:
+    - revive
+    # By convention, receiver names in a method should reflect their identity.
+    text: "receiver-naming: receiver name"
+    # Ignoring stylistic checks for generated code
+    path: .*(api|types)\/.*\/conversion.*\.go$
+  - linters:
+    - stylecheck
+    text: "ST1003: should not use underscores in Go names;"
+    path: .*(api|types|test)\/.*\/conversion.*\.go$
+  - linters:
+    - stylecheck
+    text: "ST1016: methods on the same type should have the same receiver name"
+    path: .*(api|types)\/.*\/conversion.*\.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
+
+  # Exclude-rules unique for CAPV
+  - linters:
+    - staticcheck
+    text: "SA1019: failureDomain.AutoConfigure is deprecated"
+  - path: "test/e2e/*"
+    linters:
+      - gosec
+    text: "G106:"
+  - linters:
+      - revive
+    text: "dot-imports"
+    path: ".*test.*"
+  - linters:
+      - stylecheck
+    text: "ST1001"
+    path: ".*test.*"

controllers/vsphereclusteridentity_controller_test.go

@@ -160,7 +160,7 @@ var _ = Describe("VSphereClusterIdentity Reconciler", func() {
 					return false
 				}
 
-				if i.Status.Ready == false && conditions.GetReason(i, infrav1.CredentialsAvailableCondidtion) == infrav1.SecretAlreadyInUseReason {
+				if !i.Status.Ready && conditions.GetReason(i, infrav1.CredentialsAvailableCondidtion) == infrav1.SecretAlreadyInUseReason {
 					return true
 				}
 				return false
@@ -184,7 +184,7 @@ var _ = Describe("VSphereClusterIdentity Reconciler", func() {
 					return false
 				}
 
-				if i.Status.Ready == false && conditions.GetReason(i, infrav1.CredentialsAvailableCondidtion) == infrav1.SecretNotAvailableReason {
+				if !i.Status.Ready && conditions.GetReason(i, infrav1.CredentialsAvailableCondidtion) == infrav1.SecretNotAvailableReason {
 					return true
 				}
 				return false

pkg/services/govmomi/cluster/vmgroup.go

@@ -53,7 +53,7 @@ type VMGroup struct {
 // Add a VSphere VM object to the VM Group.
 func (vg VMGroup) Add(ctx context.Context, vmObj types.ManagedObjectReference) (*object.Task, error) {
 	vms := vg.listVMs()
-	vg.ClusterVmGroup.Vm = append(vms, vmObj) //nolint:gocritic
+	vg.ClusterVmGroup.Vm = append(vms, vmObj)
 
 	spec := &types.ClusterConfigSpecEx{
 		GroupSpec: []types.ClusterGroupSpec{

test/helpers/framework.go

@@ -30,7 +30,7 @@ import (
 	"sigs.k8s.io/cluster-api/test/framework/clusterctl"
 )
 
-// Util functions to interact with the clusterctl e2e framework
+// Util functions to interact with the clusterctl e2e framework.
 
 func LoadE2EConfig(configPath string) (*clusterctl.E2EConfig, error) {
 	config := clusterctl.LoadE2EConfig(goctx.TODO(), clusterctl.LoadE2EConfigInput{ConfigPath: configPath})