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,233 @@ 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
+  # Disable unparam "always receives" which might not be really
+  # useful when building libraries.
+  - linters:
+    - unparam
+    text: always receives
+
+  # 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/clustermodule_reconciler_test.go

@@ -369,8 +369,6 @@ func TestReconciler_fetchMachineOwnerObjects(t *testing.T) {
 		g.Expect(objMap).To(gomega.HaveLen(2))
 	})
 }
-
-//nolint:unparam
 func machineDeployment(name, namespace, cluster string) *clusterv1.MachineDeployment {
 	return &clusterv1.MachineDeployment{
 		TypeMeta: metav1.TypeMeta{
@@ -384,7 +382,6 @@ func machineDeployment(name, namespace, cluster string) *clusterv1.MachineDeploy
 	}
 }
 
-//nolint:unparam
 func controlPlane(name, namespace, cluster string) *controlplanev1.KubeadmControlPlane {
 	return &controlplanev1.KubeadmControlPlane{
 		TypeMeta: metav1.TypeMeta{

controllers/serviceaccount_controller_suite_test.go

@@ -102,7 +102,7 @@ func assertServiceAccountAndUpdateSecret(ctx goctx.Context, ctrlClient client.Cl
 	Expect(ctrlClient.Update(ctx, secret)).To(Succeed())
 }
 
-func assertTargetSecret(ctx goctx.Context, guestClient client.Client, namespace, name string) { //nolint
+func assertTargetSecret(ctx goctx.Context, guestClient client.Client, namespace, name string) {
 	secret := &corev1.Secret{}
 	assertEventuallyExistsInNamespace(ctx, guestClient, namespace, name, secret)
 	EventuallyWithOffset(2, func() []byte {
@@ -157,7 +157,7 @@ func assertRoleBinding(_ *helpers.UnitTestContextForController, ctrlClient clien
 }
 
 // assertProviderServiceAccountsCondition asserts the condition on the ProviderServiceAccount CR.
-func assertProviderServiceAccountsCondition(vCluster *vmwarev1.VSphereCluster, status corev1.ConditionStatus, message string, reason string, severity clusterv1.ConditionSeverity) { //nolint
+func assertProviderServiceAccountsCondition(vCluster *vmwarev1.VSphereCluster, status corev1.ConditionStatus, message string, reason string, severity clusterv1.ConditionSeverity) {
 	c := conditions.Get(vCluster, vmwarev1.ProviderServiceAccountsReadyCondition)
 	Expect(c).NotTo(BeNil())
 	Expect(c.Status).To(Equal(status))

controllers/svcdiscovery_controller_suite_test.go

@@ -81,7 +81,7 @@ func assertHeadlessSvc(ctx context.Context, guestClient client.Client, namespace
 	Expect(headlessSvc.Spec.Ports[0].TargetPort.IntVal).To(Equal(int32(supervisorAPIServerPort)))
 }
 
-func assertHeadlessSvcWithNoEndpoints(ctx context.Context, guestClient client.Client, namespace, name string) { //nolint
+func assertHeadlessSvcWithNoEndpoints(ctx context.Context, guestClient client.Client, namespace, name string) {
 	assertHeadlessSvc(ctx, guestClient, namespace, name)
 	headlessEndpoints := &corev1.Endpoints{}
 	assertEventuallyDoesNotExistInNamespace(ctx, guestClient, namespace, name, headlessEndpoints)

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/e2e/common.go

@@ -50,7 +50,6 @@ type GlobalInput struct {
 	E2EConfig             *clusterctl.E2EConfig
 }
 
-//nolint:unparam
 func defaultConfigCluster(clusterName, namespace, flavor string, controlPlaneNodeCount, workerNodeCount int64,
 	input GlobalInput) clusterctl.ConfigClusterInput {
 	configClusterInput := clusterctl.ConfigClusterInput{

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})

test/helpers/mod_test.go

@@ -32,7 +32,7 @@ require (
 )
 
 replace (
-	github.com/foo/bar v1.0.0 => github.com/foo/bar v1.0.1 
+	github.com/foo/bar v1.0.0 => github.com/foo/bar v1.0.1
 )
 `
 	tempPath, err := createTempGoMod(goModData)