Sync golangci-lint configuration to core CAPI

Author: chrischdi

.golangci.yml

@@ -1,15 +1,29 @@
+run:
+  timeout: 10m
+  go: "1.20"
+  skip-files:
+    - "zz_generated.*\\.go$"
+    - apis/v1alpha3
+  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
@@ -23,7 +37,8 @@ linters:
   - nakedret
   - nilerr
   - noctx
-  - nolintlint
+  # - nolintlint
+  - nosprintfhostport
   - prealloc
   - predeclared
   - revive
@@ -35,42 +50,250 @@ 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"
-    # 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."
-    - 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.
+  # 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 (.+) (.+) should have comment (.*)or be unexported
+    path: "^(test/|packaging/|pkg/.*/fake/|pkg/util/testutil).*.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.
+  - linters:
+    - revive
+    - stylecheck
+    path: test/.*.go
+    text: should not use dot imports
+  # 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: ^apis\/.*\/.*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: ^apis\/.*\/.*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: ^apis\/.*\/.*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: ^apis\/.*\/.*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: ^apis\/.*\/.*conversion.*\.go$
+  - linters:
+    - stylecheck
+    text: "ST1003: should not use underscores in Go names;"
+    path: ^apis\/.*\/.*conversion.*\.go$
+  - linters:
+    - stylecheck
+    text: "ST1016: methods on the same type should have the same receiver name"
+    path: ^apis\/.*\/.*conversion.*\.go$
+
+  # FIXME: All below excludes should get removed over time.
+  - linters:
+    - staticcheck
+    text: "SA1019: failureDomain.AutoConfigure is deprecated"
+  - path: "test/e2e/*"
+    linters:
+      - gosec
+    text: "G106:"
+
+  # missing comments
+  - linters:
+    - revive
+    text: "package-comments"
+    path: ^(main|(packaging|apis|controllers|feature|test|pkg)\/.*)\.go$
+  - linters:
+    - stylecheck
+    text: "ST1000"
+    path: ^(main|(packaging|apis|controllers|feature|test|pkg)\/.*)\.go$
+  - linters:
+    - revive
+    text: exported (.*) should have comment (.*)or be unexported
+    path: "^(apis|controllers|packaging|pkg)/.*.go"
+
+  # wrong comment
+  - linters:
+    - revive
+    text: comment on exported (.*) should be of the form (.*)
+    path: "^(apis|packaging|pkg/(constants|services|util))/"
+  - linters:
+    - stylecheck
+    text: ST1021|ST1020
+    path: "^(apis|packaging|pkg/(constants|services|util))/"
+
+  # gosec
+  - linters:
+    - gosec
+    text: "G104: Errors unhandled."
+    path: ^(controllers/vspherecluster_reconciler|pkg/manager/options_test|test/helpers/webhook)\.go$
+  - linters:
+    - gosec
+    text: "(G204|G301|G304): "
+    path: ^test/

controllers/clustermodule_reconciler_test.go

@@ -518,7 +518,6 @@ func TestReconciler_fetchMachineOwnerObjects(t *testing.T) {
 	})
 }
 
-//nolint:unparam
 func machineDeployment(name, namespace, cluster string) *clusterv1.MachineDeployment {
 	return &clusterv1.MachineDeployment{
 		TypeMeta: metav1.TypeMeta{
@@ -532,7 +531,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{