diff --git a/.golangci.yml b/.golangci.yml index f3ed128f44..60e3f8c429 100644 --- a/.golangci.yml +++ b/.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/ diff --git a/controllers/clustermodule_reconciler_test.go b/controllers/clustermodule_reconciler_test.go index b83b5f62eb..ffb7611985 100644 --- a/controllers/clustermodule_reconciler_test.go +++ b/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{ diff --git a/controllers/serviceaccount_controller_suite_test.go b/controllers/serviceaccount_controller_suite_test.go index 82a212e8e0..b840c6121e 100644 --- a/controllers/serviceaccount_controller_suite_test.go +++ b/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)) diff --git a/controllers/svcdiscovery_controller_suite_test.go b/controllers/svcdiscovery_controller_suite_test.go index 0de8fdd05f..5f7b91b2f3 100644 --- a/controllers/svcdiscovery_controller_suite_test.go +++ b/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) diff --git a/controllers/vsphereclusteridentity_controller_test.go b/controllers/vsphereclusteridentity_controller_test.go index e5ca5c0268..55e67207cc 100644 --- a/controllers/vsphereclusteridentity_controller_test.go +++ b/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 diff --git a/pkg/services/govmomi/cluster/vmgroup.go b/pkg/services/govmomi/cluster/vmgroup.go index b520f8f95c..ef93c44de1 100644 --- a/pkg/services/govmomi/cluster/vmgroup.go +++ b/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{ diff --git a/test/e2e/common.go b/test/e2e/common.go index 332f01cb5d..8c5fa29ad5 100644 --- a/test/e2e/common.go +++ b/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{ diff --git a/test/helpers/framework.go b/test/helpers/framework.go index 6d182ee74f..77959e5296 100644 --- a/test/helpers/framework.go +++ b/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}) diff --git a/test/helpers/mod_test.go b/test/helpers/mod_test.go index 65a5af1b4b..5ea6b33d9b 100644 --- a/test/helpers/mod_test.go +++ b/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) diff --git a/test/integration/integration_suite_test.go b/test/integration/integration_suite_test.go index ee3d8b4cbe..7a3ab99a58 100644 --- a/test/integration/integration_suite_test.go +++ b/test/integration/integration_suite_test.go @@ -607,7 +607,6 @@ func createResource(resource schema.GroupVersionResource, obj runtimeObjectWithN Expect(err).NotTo(HaveOccurred(), "Error creating %s %s/%s", resource, obj.GetNamespace(), obj.GetName()) } -//nolint:unparam func deleteResource(resource schema.GroupVersionResource, name, namespace string, propagationPolicy *metav1.DeletionPropagation) { deleteOptions := metav1.DeleteOptions{PropagationPolicy: propagationPolicy} err := k8sClient.Resource(resource).Namespace(namespace).Delete(ctx, name, deleteOptions)