diff --git a/golangci.yaml b/golangci.yaml index 88b3b77237..97ee4229da 100644 --- a/golangci.yaml +++ b/golangci.yaml @@ -255,6 +255,23 @@ linters-settings: rowserrcheck: packages: - github.com/jmoiron/sqlx + testifylint: + # TODO: enable them all + disable: + - bool-compare + - compares + - error-is-as + - error-nil + - expected-actual + - go-require + - float-compare + - require-error + - suite-dont-use-pkg + - suite-extra-assert-call + - suite-thelper + enable: + - empty + - len testpackage: # regexp pattern to skip files skip-regexp: (export|internal)_test\.go @@ -302,8 +319,10 @@ linters: - bodyclose - dogsled - durationcheck + - dupword - errcheck - exportloopref + - errchkjson - goconst - gofmt - goheader @@ -312,26 +331,25 @@ linters: - gosec - gosimple - govet + - ginkgolinter - importas - ineffassign - misspell - nakedret - nosprintfhostport + - nilerr + - noctx + - nolintlint + - revive - staticcheck - stylecheck - - revive + - testifylint - typecheck - unconvert - unparam - unused - usestdlibvars - whitespace - - dupword - - errchkjson - - ginkgolinter - - nilerr - - noctx - - nolintlint fast: false diff --git a/hack/build-image/Dockerfile b/hack/build-image/Dockerfile index 7165f93704..a60266fc78 100644 --- a/hack/build-image/Dockerfile +++ b/hack/build-image/Dockerfile @@ -93,7 +93,7 @@ RUN ARCH=$(go env GOARCH) && \ chmod +x /usr/bin/goreleaser # get golangci-lint -RUN curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.54.2 +RUN curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.55.0 # install kubectl RUN curl -LO https://storage.googleapis.com/kubernetes-release/release/$(curl -s https://storage.googleapis.com/kubernetes-release/release/stable.txt)/bin/linux/$(go env GOARCH)/kubectl diff --git a/internal/hook/item_hook_handler_test.go b/internal/hook/item_hook_handler_test.go index f8efb5f089..8b73572b64 100644 --- a/internal/hook/item_hook_handler_test.go +++ b/internal/hook/item_hook_handler_test.go @@ -2474,7 +2474,7 @@ func TestRestoreHookTrackerAdd(t *testing.T) { t.Run(tc.name, func(t *testing.T) { _, _ = GroupRestoreExecHooks(tc.resourceRestoreHooks, tc.pod, velerotest.NewLogger(), tc.hookTracker) tracker := tc.hookTracker.GetTracker() - assert.Equal(t, tc.expectedCnt, len(tracker)) + assert.Len(t, tracker, tc.expectedCnt) }) } } diff --git a/pkg/backup/backup_pv_action_test.go b/pkg/backup/backup_pv_action_test.go index d9e0a5b333..d9d2b52cfa 100644 --- a/pkg/backup/backup_pv_action_test.go +++ b/pkg/backup/backup_pv_action_test.go @@ -50,14 +50,14 @@ func TestBackupPVAction(t *testing.T) { // and no additional items _, additional, err := a.Execute(pvc, backup) assert.NoError(t, err) - assert.Len(t, additional, 0) + assert.Empty(t, additional) // empty spec.volumeName should result in no error // and no additional items pvc.Object["spec"].(map[string]interface{})["volumeName"] = "" _, additional, err = a.Execute(pvc, backup) assert.NoError(t, err) - assert.Len(t, additional, 0) + assert.Empty(t, additional) // Action should clean the spec.Selector when the StorageClassName is not set. input := builder.ForPersistentVolumeClaim("abc", "abc").VolumeName("pv").Selector(&metav1.LabelSelector{MatchLabels: map[string]string{"abc": "abc"}}).Phase(corev1.ClaimBound).Result() @@ -119,21 +119,21 @@ func TestBackupPVAction(t *testing.T) { pvc.Object["spec"].(map[string]interface{})["volumeName"] = "myVolume" _, additional, err = a.Execute(pvc, backup) require.NoError(t, err) - require.Len(t, additional, 0) + require.Empty(t, additional) // non-empty spec.volumeName when status.phase is 'Pending' // should result in no error and no additional items pvc.Object["status"].(map[string]interface{})["phase"] = corev1api.ClaimPending _, additional, err = a.Execute(pvc, backup) require.NoError(t, err) - require.Len(t, additional, 0) + require.Empty(t, additional) // non-empty spec.volumeName when status.phase is 'Lost' // should result in no error and no additional items pvc.Object["status"].(map[string]interface{})["phase"] = corev1api.ClaimLost _, additional, err = a.Execute(pvc, backup) require.NoError(t, err) - require.Len(t, additional, 0) + require.Empty(t, additional) // non-empty spec.volumeName when status.phase is 'Bound' // should result in no error and one additional item for the PV @@ -148,5 +148,5 @@ func TestBackupPVAction(t *testing.T) { pvc.Object["spec"].(map[string]interface{})["volumeName"] = "" _, additional, err = a.Execute(pvc, backup) assert.NoError(t, err) - assert.Len(t, additional, 0) + assert.Empty(t, additional) } diff --git a/pkg/backup/pv_skip_tracker_test.go b/pkg/backup/pv_skip_tracker_test.go index 8e75a808cf..bd258dd855 100644 --- a/pkg/backup/pv_skip_tracker_test.go +++ b/pkg/backup/pv_skip_tracker_test.go @@ -60,5 +60,5 @@ func TestTrackUntrack(t *testing.T) { tracker.Track("pv3", podVolumeApproach, "it's set to opt-out") tracker.Untrack("pv3") tracker.Track("pv3", csiSnapshotApproach, "not applicable for CSI ") - assert.Equal(t, 0, len(tracker.Summary())) + assert.Empty(t, tracker.Summary()) } diff --git a/pkg/client/config_test.go b/pkg/client/config_test.go index 938345b5ae..56b198f793 100644 --- a/pkg/client/config_test.go +++ b/pkg/client/config_test.go @@ -91,7 +91,7 @@ func TestConfigOperations(t *testing.T) { // Test Features feature := savedConfig.Features() - assert.Equal(t, 1, len(feature)) + assert.Len(t, feature, 1) assert.Equal(t, expectedFeature, feature[0]) // Test Colorized diff --git a/pkg/cmd/cli/backuplocation/delete_test.go b/pkg/cmd/cli/backuplocation/delete_test.go index 9736c9883a..0ed5d3130b 100644 --- a/pkg/cmd/cli/backuplocation/delete_test.go +++ b/pkg/cmd/cli/backuplocation/delete_test.go @@ -89,13 +89,13 @@ func TestDeleteFunctions(t *testing.T) { bkrepoList := velerov1api.BackupRepositoryList{} t.Run("findAssociatedBackups", func(t *testing.T) { bkList, e := findAssociatedBackups(kbclient, "bk-loc-1", "ns1") - assert.Equal(t, 0, len(bkList.Items)) + assert.Empty(t, bkList.Items) assert.NoError(t, e) }) t.Run("findAssociatedBackupRepos", func(t *testing.T) { bkrepoList, e := findAssociatedBackupRepos(kbclient, "bk-loc-1", "ns1") - assert.Equal(t, 0, len(bkrepoList.Items)) + assert.Empty(t, bkrepoList.Items) assert.NoError(t, e) }) @@ -104,7 +104,7 @@ func TestDeleteFunctions(t *testing.T) { bk.Name = "bk-name-last" bkList.Items = append(bkList.Items, bk) errList := deleteBackups(kbclient, bkList) - assert.Equal(t, 1, len(errList)) + assert.Len(t, errList, 1) assert.Contains(t, errList[0].Error(), fmt.Sprintf("delete backup \"%s\" associated with deleted BSL: backups.velero.io \"%s\" not found", bk.Name, bk.Name)) }) t.Run("deleteBackupRepos", func(t *testing.T) { @@ -112,7 +112,7 @@ func TestDeleteFunctions(t *testing.T) { bkrepo.Name = "bk-repo-name-last" bkrepoList.Items = append(bkrepoList.Items, bkrepo) errList := deleteBackupRepos(kbclient, bkrepoList) - assert.Equal(t, 1, len(errList)) + assert.Len(t, errList, 1) assert.Contains(t, errList[0].Error(), fmt.Sprintf("delete backup repository \"%s\" associated with deleted BSL: backuprepositories.velero.io \"%s\" not found", bkrepo.Name, bkrepo.Name)) }) } diff --git a/pkg/controller/backup_deletion_controller_test.go b/pkg/controller/backup_deletion_controller_test.go index 0dbb0a3e97..820076634a 100644 --- a/pkg/controller/backup_deletion_controller_test.go +++ b/pkg/controller/backup_deletion_controller_test.go @@ -138,7 +138,7 @@ func TestBackupDeletionControllerReconcile(t *testing.T) { err = td.fakeClient.Get(ctx, td.req.NamespacedName, res) require.NoError(t, err) assert.Equal(t, "Processed", string(res.Status.Phase)) - assert.Equal(t, 1, len(res.Status.Errors)) + assert.Len(t, res.Status.Errors, 1) assert.Equal(t, "spec.backupName is required", res.Status.Errors[0]) }) @@ -206,7 +206,7 @@ func TestBackupDeletionControllerReconcile(t *testing.T) { err = td.fakeClient.Get(ctx, td.req.NamespacedName, res) require.NoError(t, err) assert.Equal(t, "Processed", string(res.Status.Phase)) - assert.Equal(t, 1, len(res.Status.Errors)) + assert.Len(t, res.Status.Errors, 1) assert.Equal(t, "backup is still in progress", res.Status.Errors[0]) }) @@ -221,7 +221,7 @@ func TestBackupDeletionControllerReconcile(t *testing.T) { err = td.fakeClient.Get(ctx, td.req.NamespacedName, res) require.NoError(t, err) assert.Equal(t, "Processed", string(res.Status.Phase)) - assert.Equal(t, 1, len(res.Status.Errors)) + assert.Len(t, res.Status.Errors, 1) assert.Equal(t, "backup not found", res.Status.Errors[0]) }) t.Run("unable to find backup storage location", func(t *testing.T) { @@ -236,7 +236,7 @@ func TestBackupDeletionControllerReconcile(t *testing.T) { err = td.fakeClient.Get(ctx, td.req.NamespacedName, res) require.NoError(t, err) assert.Equal(t, "Processed", string(res.Status.Phase)) - assert.Equal(t, 1, len(res.Status.Errors)) + assert.Len(t, res.Status.Errors, 1) assert.Equal(t, "backup storage location default not found", res.Status.Errors[0]) }) @@ -253,7 +253,7 @@ func TestBackupDeletionControllerReconcile(t *testing.T) { err = td.fakeClient.Get(ctx, td.req.NamespacedName, res) require.NoError(t, err) assert.Equal(t, "Processed", string(res.Status.Phase)) - assert.Equal(t, 1, len(res.Status.Errors)) + assert.Len(t, res.Status.Errors, 1) assert.Equal(t, "cannot delete backup because backup storage location default is currently in read-only mode", res.Status.Errors[0]) }) t.Run("full delete, no errors", func(t *testing.T) { @@ -684,7 +684,7 @@ func TestBackupDeletionControllerReconcile(t *testing.T) { err = td.fakeClient.Get(ctx, td.req.NamespacedName, res) require.NoError(t, err) assert.Equal(t, "Processed", string(res.Status.Phase)) - assert.Equal(t, 1, len(res.Status.Errors)) + assert.Len(t, res.Status.Errors, 1) assert.Equal(t, "backup not found", res.Status.Errors[0]) }) diff --git a/pkg/controller/pod_volume_restore_controller_test.go b/pkg/controller/pod_volume_restore_controller_test.go index 0ef0165f5b..f24aed6536 100644 --- a/pkg/controller/pod_volume_restore_controller_test.go +++ b/pkg/controller/pod_volume_restore_controller_test.go @@ -492,7 +492,7 @@ func TestFindVolumeRestoresForPod(t *testing.T) { logger: logrus.New(), } requests := reconciler.findVolumeRestoresForPod(context.Background(), pod) - assert.Len(t, requests, 0) + assert.Empty(t, requests) // contain one matching PVR reconciler.Client = clientBuilder.WithLists(&velerov1api.PodVolumeRestoreList{ diff --git a/pkg/controller/restore_controller_test.go b/pkg/controller/restore_controller_test.go index 65df4af57d..7b67aa5693 100644 --- a/pkg/controller/restore_controller_test.go +++ b/pkg/controller/restore_controller_test.go @@ -592,7 +592,7 @@ func TestRestoreReconcile(t *testing.T) { return } if !test.addValidFinalizer { - assert.Equal(t, 1, len(restorer.Calls)) + assert.Len(t, restorer.Calls, 1) } // validate Patch call 2 (setting phase) diff --git a/pkg/controller/restore_finalizer_controller_test.go b/pkg/controller/restore_finalizer_controller_test.go index 3a17bf637c..5bc25fca36 100644 --- a/pkg/controller/restore_finalizer_controller_test.go +++ b/pkg/controller/restore_finalizer_controller_test.go @@ -442,7 +442,7 @@ func TestPatchDynamicPVWithVolumeInfo(t *testing.T) { errs := ctx.patchDynamicPVWithVolumeInfo() if tc.expectedErrNum > 0 { - assert.Equal(t, tc.expectedErrNum, len(errs.Namespaces)) + assert.Len(t, errs.Namespaces, tc.expectedErrNum) } for pvName, expectedPVInfo := range tc.expectedPatch { diff --git a/pkg/controller/schedule_controller_test.go b/pkg/controller/schedule_controller_test.go index b2150fc494..e7c0c25a5f 100644 --- a/pkg/controller/schedule_controller_test.go +++ b/pkg/controller/schedule_controller_test.go @@ -219,16 +219,16 @@ func TestReconcileOfSchedule(t *testing.T) { // new backup shouldn't be submitted. if test.backup != nil && (test.backup.Status.Phase == velerov1.BackupPhaseNew || test.backup.Status.Phase == velerov1.BackupPhaseInProgress) { - assert.Equal(t, 1, len(backups.Items)) + assert.Len(t, backups.Items, 1) require.Nil(t, client.Delete(ctx, test.backup)) } require.Nil(t, client.List(ctx, backups)) if test.expectedBackupCreate == nil { - assert.Equal(t, 0, len(backups.Items)) + assert.Empty(t, backups.Items) } else { - assert.Equal(t, 1, len(backups.Items)) + assert.Len(t, backups.Items, 1) } }) } diff --git a/pkg/datapath/manager_test.go b/pkg/datapath/manager_test.go index e4b4d40c63..ebe18d1a03 100644 --- a/pkg/datapath/manager_test.go +++ b/pkg/datapath/manager_test.go @@ -42,10 +42,10 @@ func TestManager(t *testing.T) { assert.Equal(t, async_job_1, ret) m.RemoveAsyncBR("job-0") - assert.Equal(t, 2, len(m.tracker)) + assert.Len(t, m.tracker, 2) m.RemoveAsyncBR("job-1") - assert.Equal(t, 1, len(m.tracker)) + assert.Len(t, m.tracker, 1) ret = m.GetAsyncBR("job-1") assert.Equal(t, nil, ret) diff --git a/pkg/install/daemonset_test.go b/pkg/install/daemonset_test.go index 017d5004d2..a9cce29ea4 100644 --- a/pkg/install/daemonset_test.go +++ b/pkg/install/daemonset_test.go @@ -34,8 +34,8 @@ func TestDaemonSet(t *testing.T) { assert.Equal(t, corev1.PullIfNotPresent, ds.Spec.Template.Spec.Containers[0].ImagePullPolicy) ds = DaemonSet("velero", WithSecret(true)) - assert.Equal(t, 7, len(ds.Spec.Template.Spec.Containers[0].Env)) - assert.Equal(t, 4, len(ds.Spec.Template.Spec.Volumes)) + assert.Len(t, ds.Spec.Template.Spec.Containers[0].Env, 7) + assert.Len(t, ds.Spec.Template.Spec.Volumes, 4) ds = DaemonSet("velero", WithFeatures([]string{"foo,bar,baz"})) assert.Len(t, ds.Spec.Template.Spec.Containers[0].Args, 3) diff --git a/pkg/install/deployment_test.go b/pkg/install/deployment_test.go index 8e9649737d..faa78afec4 100644 --- a/pkg/install/deployment_test.go +++ b/pkg/install/deployment_test.go @@ -43,8 +43,8 @@ func TestDeployment(t *testing.T) { assert.Equal(t, corev1.PullIfNotPresent, deploy.Spec.Template.Spec.Containers[0].ImagePullPolicy) deploy = Deployment("velero", WithSecret(true)) - assert.Equal(t, 7, len(deploy.Spec.Template.Spec.Containers[0].Env)) - assert.Equal(t, 3, len(deploy.Spec.Template.Spec.Volumes)) + assert.Len(t, deploy.Spec.Template.Spec.Containers[0].Env, 7) + assert.Len(t, deploy.Spec.Template.Spec.Volumes, 3) deploy = Deployment("velero", WithDefaultRepoMaintenanceFrequency(24*time.Hour)) assert.Len(t, deploy.Spec.Template.Spec.Containers[0].Args, 2) diff --git a/pkg/metrics/metrics.go b/pkg/metrics/metrics.go index 61b91812d8..3e273a9fde 100644 --- a/pkg/metrics/metrics.go +++ b/pkg/metrics/metrics.go @@ -69,7 +69,7 @@ const ( // data mover metrics DataUploadSuccessTotal = "data_upload_success_total" DataUploadFailureTotal = "data_upload_failure_total" - DataUploadCancelTotal = "data_upload_cancel_total" //nolint:gosec // Not a hard code secret. + DataUploadCancelTotal = "data_upload_cancel_total" DataDownloadSuccessTotal = "data_download_success_total" DataDownloadFailureTotal = "data_download_failure_total" DataDownloadCancelTotal = "data_download_cancel_total" diff --git a/pkg/podvolume/backupper_test.go b/pkg/podvolume/backupper_test.go index e08b772dd9..618d380aa4 100644 --- a/pkg/podvolume/backupper_test.go +++ b/pkg/podvolume/backupper_test.go @@ -642,22 +642,22 @@ func TestPVCBackupSummary(t *testing.T) { // it won't be added if the volme is not in the pvc map. pbs.addSkipped("vol-3", "whatever reason") - assert.Equal(t, 0, len(pbs.Skipped)) + assert.Empty(t, pbs.Skipped) pbs.addBackedup("vol-3") - assert.Equal(t, 0, len(pbs.Backedup)) + assert.Empty(t, pbs.Backedup) // only can be added as skipped when it's not in backedup set pbs.addBackedup("vol-1") - assert.Equal(t, 1, len(pbs.Backedup)) + assert.Len(t, pbs.Backedup, 1) assert.Equal(t, "pvc-1", pbs.Backedup["vol-1"].Name) pbs.addSkipped("vol-1", "whatever reason") - assert.Equal(t, 0, len(pbs.Skipped)) + assert.Empty(t, pbs.Skipped) pbs.addSkipped("vol-2", "vol-2 has to be skipped") - assert.Equal(t, 1, len(pbs.Skipped)) + assert.Len(t, pbs.Skipped, 1) assert.Equal(t, "pvc-2", pbs.Skipped["vol-2"].PVC.Name) // adding a vol as backedup removes it from skipped set pbs.addBackedup("vol-2") - assert.Equal(t, 0, len(pbs.Skipped)) - assert.Equal(t, 2, len(pbs.Backedup)) + assert.Empty(t, pbs.Skipped) + assert.Len(t, pbs.Backedup, 2) } diff --git a/pkg/repository/udmrepo/kopialib/backend/utils.go b/pkg/repository/udmrepo/kopialib/backend/utils.go index 19fa7b278c..a740a0b7b6 100644 --- a/pkg/repository/udmrepo/kopialib/backend/utils.go +++ b/pkg/repository/udmrepo/kopialib/backend/utils.go @@ -30,7 +30,7 @@ func mustHaveString(key string, flags map[string]string) (string, error) { if value, exist := flags[key]; exist { return value, nil } - return "", errors.New("key " + key + " not found") + return "", errors.Errorf("key %s not found", key) } func optionalHaveString(key string, flags map[string]string) string { diff --git a/pkg/util/stringslice/stringslice_test.go b/pkg/util/stringslice/stringslice_test.go index b3fdeedf8b..9962610266 100644 --- a/pkg/util/stringslice/stringslice_test.go +++ b/pkg/util/stringslice/stringslice_test.go @@ -36,7 +36,7 @@ func TestHas(t *testing.T) { func TestExcept(t *testing.T) { items := []string{} except := Except(items, "asdf") - assert.Len(t, except, 0) + assert.Empty(t, except) items = []string{"a", "b", "c"} assert.Equal(t, []string{"b", "c"}, Except(items, "a"))