Upgrade golangci-lint and fix warnings

Author: ash2k

.golangci.yml

@@ -2,7 +2,7 @@
 # SPDX-License-Identifier: Apache-2.0
 
 run:
-  deadline: 30m
+  timeout: 30m
 
 linters:
   # please, do not use `enable-all`: it's deprecated and will be removed soon.

Makefile

@@ -16,7 +16,7 @@ all: generate license fix vet fmt test lint tidy
 	go install github.com/google/[email protected]
 
 "$(MYGOBIN)/golangci-lint":
-	go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.53.3
+	go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.62.2
 
 "$(MYGOBIN)/deepcopy-gen":
 	go install k8s.io/code-generator/cmd/[email protected]

pkg/apply/applier.go

@@ -213,13 +213,13 @@ func (a *Applier) Run(ctx context.Context, invInfo inventory.Info, objects objec
 		}
 		// Create a new TaskStatusRunner to execute the taskQueue.
 		klog.V(4).Infoln("applier building TaskStatusRunner...")
-		allIds := object.UnstructuredSetToObjMetadataSet(append(applyObjs, pruneObjs...))
+		allIDs := object.UnstructuredSetToObjMetadataSet(append(applyObjs, pruneObjs...))
 		statusWatcher := a.statusWatcher
 		// Disable watcher for dry runs
 		if opts.DryRunStrategy.ClientOrServerDryRun() {
 			statusWatcher = watcher.BlindStatusWatcher{}
 		}
-		runner := taskrunner.NewTaskStatusRunner(allIds, statusWatcher)
+		runner := taskrunner.NewTaskStatusRunner(allIDs, statusWatcher)
 		klog.V(4).Infoln("applier running TaskStatusRunner...")
 		err = runner.Run(ctx, taskContext, taskQueue.ToChannel(), taskrunner.Options{
 			EmitStatusEvents:         options.EmitStatusEvents,

pkg/apply/destroyer.go

@@ -176,13 +176,13 @@ func (d *Destroyer) Run(ctx context.Context, invInfo inventory.Info, options Des
 		}
 		// Create a new TaskStatusRunner to execute the taskQueue.
 		klog.V(4).Infoln("destroyer building TaskStatusRunner...")
-		deleteIds := object.UnstructuredSetToObjMetadataSet(deleteObjs)
+		deleteIDs := object.UnstructuredSetToObjMetadataSet(deleteObjs)
 		statusWatcher := d.statusWatcher
 		// Disable watcher for dry runs
 		if opts.DryRunStrategy.ClientOrServerDryRun() {
 			statusWatcher = watcher.BlindStatusWatcher{}
 		}
-		runner := taskrunner.NewTaskStatusRunner(deleteIds, statusWatcher)
+		runner := taskrunner.NewTaskStatusRunner(deleteIDs, statusWatcher)
 		klog.V(4).Infoln("destroyer running TaskStatusRunner...")
 		err = runner.Run(ctx, taskContext, taskQueue.ToChannel(), taskrunner.Options{
 			EmitStatusEvents: options.EmitStatusEvents,

pkg/apply/prune/prune_test.go

@@ -574,9 +574,9 @@ func TestPrune(t *testing.T) {
 			for _, obj := range tc.clusterObjs {
 				clusterObjs = append(clusterObjs, obj)
 			}
-			pruneIds := object.UnstructuredSetToObjMetadataSet(tc.pruneObjs)
+			pruneIDs := object.UnstructuredSetToObjMetadataSet(tc.pruneObjs)
 			po := Pruner{
-				InvClient: inventory.NewFakeClient(pruneIds),
+				InvClient: inventory.NewFakeClient(pruneIDs),
 				Client:    fake.NewSimpleDynamicClient(scheme.Scheme, clusterObjs...),
 				Mapper: testrestmapper.TestOnlyStaticRESTMapper(scheme.Scheme,
 					scheme.Scheme.PrioritizedVersionsAllGroups()...),
@@ -620,21 +620,21 @@ func TestPrune(t *testing.T) {
 			for _, id := range tc.expectedFailed {
 				assert.Truef(t, im.IsFailedDelete(id), "Prune() should mark object as failed: %s", id)
 			}
-			for _, id := range pruneIds.Diff(tc.expectedFailed) {
+			for _, id := range pruneIDs.Diff(tc.expectedFailed) {
 				assert.Falsef(t, im.IsFailedDelete(id), "Prune() should NOT mark object as failed: %s", id)
 			}
 			// validate record of skipped prunes
 			for _, id := range tc.expectedSkipped {
 				assert.Truef(t, im.IsSkippedDelete(id), "Prune() should mark object as skipped: %s", id)
 			}
-			for _, id := range pruneIds.Diff(tc.expectedSkipped) {
+			for _, id := range pruneIDs.Diff(tc.expectedSkipped) {
 				assert.Falsef(t, im.IsSkippedDelete(id), "Prune() should NOT mark object as skipped: %s", id)
 			}
 			// validate record of abandoned objects
 			for _, id := range tc.expectedAbandoned {
 				assert.Truef(t, taskContext.IsAbandonedObject(id), "Prune() should mark object as abandoned: %s", id)
 			}
-			for _, id := range pruneIds.Diff(tc.expectedAbandoned) {
+			for _, id := range pruneIDs.Diff(tc.expectedAbandoned) {
 				assert.Falsef(t, taskContext.IsAbandonedObject(id), "Prune() should NOT mark object as abandoned: %s", id)
 			}
 		})
@@ -761,9 +761,9 @@ func TestPruneWithErrors(t *testing.T) {
 	}
 	for name, tc := range tests {
 		t.Run(name, func(t *testing.T) {
-			pruneIds := object.UnstructuredSetToObjMetadataSet(tc.pruneObjs)
+			pruneIDs := object.UnstructuredSetToObjMetadataSet(tc.pruneObjs)
 			po := Pruner{
-				InvClient: inventory.NewFakeClient(pruneIds),
+				InvClient: inventory.NewFakeClient(pruneIDs),
 				// Set up the fake dynamic client to recognize all objects, and the RESTMapper.
 				Client: &fakeDynamicClient{
 					resourceInterface: &failureNamespaceClient{},
@@ -867,10 +867,10 @@ func TestGetPruneObjs(t *testing.T) {
 			if len(tc.expectedObjs) != len(actualObjs) {
 				t.Fatalf("expected %d prune objs, got %d", len(tc.expectedObjs), len(actualObjs))
 			}
-			actualIds := object.UnstructuredSetToObjMetadataSet(actualObjs)
-			expectedIds := object.UnstructuredSetToObjMetadataSet(tc.expectedObjs)
-			if !object.ObjMetadataSetEquals(expectedIds, actualIds) {
-				t.Errorf("expected prune objects (%v), got (%v)", expectedIds, actualIds)
+			actualIDs := object.UnstructuredSetToObjMetadataSet(actualObjs)
+			expectedIDs := object.UnstructuredSetToObjMetadataSet(tc.expectedObjs)
+			if !object.ObjMetadataSetEquals(expectedIDs, actualIDs) {
+				t.Errorf("expected prune objects (%v), got (%v)", expectedIDs, actualIDs)
 			}
 		})
 	}

pkg/apply/solver/solver.go

@@ -182,9 +182,9 @@ func (t *TaskQueueBuilder) Build(taskContext *taskrunner.TaskContext, o Options)
 				t.newApplyTask(applySet, t.ApplyFilters, t.ApplyMutators, o))
 			// dry-run skips wait tasks
 			if !o.DryRunStrategy.ClientOrServerDryRun() {
-				applyIds := object.UnstructuredSetToObjMetadataSet(applySet)
+				applyIDs := object.UnstructuredSetToObjMetadataSet(applySet)
 				tasks = append(tasks,
-					t.newWaitTask(applyIds, taskrunner.AllCurrent, o.ReconcileTimeout))
+					t.newWaitTask(applyIDs, taskrunner.AllCurrent, o.ReconcileTimeout))
 			}
 		}
 	}
@@ -206,14 +206,14 @@ func (t *TaskQueueBuilder) Build(taskContext *taskrunner.TaskContext, o Options)
 				t.newPruneTask(pruneSet, t.PruneFilters, o))
 			// dry-run skips wait tasks
 			if !o.DryRunStrategy.ClientOrServerDryRun() {
-				pruneIds := object.UnstructuredSetToObjMetadataSet(pruneSet)
+				pruneIDs := object.UnstructuredSetToObjMetadataSet(pruneSet)
 				tasks = append(tasks,
-					t.newWaitTask(pruneIds, taskrunner.AllNotFound, o.PruneTimeout))
+					t.newWaitTask(pruneIDs, taskrunner.AllNotFound, o.PruneTimeout))
 			}
 		}
 	}
 
-	prevInvIds, _ := t.InvClient.GetClusterObjs(t.invInfo)
+	prevInvIDs, _ := t.InvClient.GetClusterObjs(t.invInfo)
 	klog.V(2).Infoln("adding delete/update inventory task")
 	var taskName string
 	if o.Destroy {
@@ -225,7 +225,7 @@ func (t *TaskQueueBuilder) Build(taskContext *taskrunner.TaskContext, o Options)
 		TaskName:      taskName,
 		InvClient:     t.InvClient,
 		InvInfo:       t.invInfo,
-		PrevInventory: prevInvIds,
+		PrevInventory: prevInvIDs,
 		DryRun:        o.DryRunStrategy,
 		Destroy:       o.Destroy,
 	})
@@ -257,13 +257,13 @@ func (t *TaskQueueBuilder) newApplyTask(applyObjs object.UnstructuredSet,
 
 // AppendWaitTask appends a task to wait on the passed objects to the task queue.
 // Returns a pointer to the Builder to chain function calls.
-func (t *TaskQueueBuilder) newWaitTask(waitIds object.ObjMetadataSet, condition taskrunner.Condition,
+func (t *TaskQueueBuilder) newWaitTask(waitIDs object.ObjMetadataSet, condition taskrunner.Condition,
 	waitTimeout time.Duration) taskrunner.Task {
-	waitIds = t.Collector.FilterInvalidIds(waitIds)
+	waitIDs = t.Collector.FilterInvalidIds(waitIDs)
 	klog.V(2).Infoln("adding wait task")
 	task := taskrunner.NewWaitTask(
 		fmt.Sprintf("wait-%d", t.waitCounter),
-		waitIds,
+		waitIDs,
 		condition,
 		waitTimeout,
 		t.Mapper,

pkg/apply/solver/solver_test.go

@@ -808,8 +808,8 @@ func TestTaskQueueBuilder_ApplyBuild(t *testing.T) {
 				}
 			}
 
-			applyIds := object.UnstructuredSetToObjMetadataSet(tc.applyObjs)
-			fakeInvClient := inventory.NewFakeClient(applyIds)
+			applyIDs := object.UnstructuredSetToObjMetadataSet(tc.applyObjs)
+			fakeInvClient := inventory.NewFakeClient(applyIDs)
 			vCollector := &validation.Collector{}
 			tqb := TaskQueueBuilder{
 				Pruner:    pruner,
@@ -1484,8 +1484,8 @@ func TestTaskQueueBuilder_PruneBuild(t *testing.T) {
 				}
 			}
 
-			pruneIds := object.UnstructuredSetToObjMetadataSet(tc.pruneObjs)
-			fakeInvClient := inventory.NewFakeClient(pruneIds)
+			pruneIDs := object.UnstructuredSetToObjMetadataSet(tc.pruneObjs)
+			fakeInvClient := inventory.NewFakeClient(pruneIDs)
 			vCollector := &validation.Collector{}
 			tqb := TaskQueueBuilder{
 				Pruner:    pruner,

pkg/apply/task/apply_task_test.go

@@ -492,22 +492,22 @@ func TestApplyTaskWithError(t *testing.T) {
 				assert.Equal(t, tc.expectedEvents[i].ApplyEvent.Error.Error(), e.ApplyEvent.Error.Error())
 			}
 
-			applyIds := object.UnstructuredSetToObjMetadataSet(tc.objs)
+			applyIDs := object.UnstructuredSetToObjMetadataSet(tc.objs)
 
 			im := taskContext.InventoryManager()
 
 			// validate record of failed prunes
 			for _, id := range tc.expectedFailed {
 				assert.Truef(t, im.IsFailedApply(id), "ApplyTask should mark object as failed: %s", id)
 			}
-			for _, id := range applyIds.Diff(tc.expectedFailed) {
+			for _, id := range applyIDs.Diff(tc.expectedFailed) {
 				assert.Falsef(t, im.IsFailedApply(id), "ApplyTask should NOT mark object as failed: %s", id)
 			}
 			// validate record of skipped prunes
 			for _, id := range tc.expectedSkipped {
 				assert.Truef(t, im.IsSkippedApply(id), "ApplyTask should mark object as skipped: %s", id)
 			}
-			for _, id := range applyIds.Diff(tc.expectedSkipped) {
+			for _, id := range applyIDs.Diff(tc.expectedSkipped) {
 				assert.Falsef(t, im.IsSkippedApply(id), "ApplyTask should NOT mark object as skipped: %s", id)
 			}
 		})

pkg/apply/task/inv_add_task_test.go

@@ -158,9 +158,9 @@ func TestInvAddTask(t *testing.T) {
 			if taskName != task.Name() {
 				t.Errorf("expected task name (%s), got (%s)", taskName, task.Name())
 			}
-			applyIds := object.UnstructuredSetToObjMetadataSet(tc.applyObjs)
-			if !task.Identifiers().Equal(applyIds) {
-				t.Errorf("expected task ids (%s), got (%s)", applyIds, task.Identifiers())
+			applyIDs := object.UnstructuredSetToObjMetadataSet(tc.applyObjs)
+			if !task.Identifiers().Equal(applyIDs) {
+				t.Errorf("expected task ids (%s), got (%s)", applyIDs, task.Identifiers())
 			}
 			task.Start(context)
 			result := <-context.TaskChannel()

pkg/apply/taskrunner/task.go

@@ -56,7 +56,7 @@ type WaitTask struct {
 	// TaskName allows providing a name for the task.
 	TaskName string
 	// Ids is the full list of resources that we are waiting for.
-	Ids object.ObjMetadataSet
+	Ids object.ObjMetadataSet //nolint:revive
 	// Condition defines the status we want all resources to reach
 	Condition Condition
 	// Timeout defines how long we are willing to wait for the condition

pkg/inventory/inventory-client.go

@@ -96,11 +96,11 @@ func NewClient(factory cmdutil.Factory,
 // objects if an inventory object does not exist. Returns an error if one
 // occurred.
 func (cic *ClusterClient) Merge(localInv Info, objs object.ObjMetadataSet, dryRun common.DryRunStrategy) (object.ObjMetadataSet, error) {
-	pruneIds := object.ObjMetadataSet{}
+	pruneIDs := object.ObjMetadataSet{}
 	invObj := cic.invToUnstructuredFunc(localInv)
 	clusterInv, err := cic.getClusterInventoryInfo(localInv)
 	if err != nil {
-		return pruneIds, err
+		return pruneIDs, err
 	}
 
 	// Inventory does not exist on the cluster.
@@ -128,34 +128,34 @@ func (cic *ClusterClient) Merge(localInv Info, objs object.ObjMetadataSet, dryRu
 	// Update existing cluster inventory with merged union of objects
 	clusterObjs, err := cic.GetClusterObjs(localInv)
 	if err != nil {
-		return pruneIds, err
+		return pruneIDs, err
 	}
-	pruneIds = clusterObjs.Diff(objs)
+	pruneIDs = clusterObjs.Diff(objs)
 	unionObjs := clusterObjs.Union(objs)
 	var status []actuation.ObjectStatus
 	if cic.statusPolicy == StatusPolicyAll {
-		status = getObjStatus(pruneIds, unionObjs)
+		status = getObjStatus(pruneIDs, unionObjs)
 	}
-	klog.V(4).Infof("num objects to prune: %d", len(pruneIds))
+	klog.V(4).Infof("num objects to prune: %d", len(pruneIDs))
 	klog.V(4).Infof("num merged objects to store in inventory: %d", len(unionObjs))
 	wrappedInv := cic.InventoryFactoryFunc(clusterInv)
 	if err = wrappedInv.Store(unionObjs, status); err != nil {
-		return pruneIds, err
+		return pruneIDs, err
 	}
 
 	// Update not required when all objects in inventory are the same and
 	// status does not need to be updated. If status is stored, always update the
 	// inventory to store the latest status.
 	if objs.Equal(clusterObjs) && cic.statusPolicy == StatusPolicyNone {
-		return pruneIds, nil
+		return pruneIDs, nil
 	}
 
 	if dryRun.ClientOrServerDryRun() {
 		klog.V(4).Infof("dry-run create inventory object: not created")
-		return pruneIds, nil
+		return pruneIDs, nil
 	}
 	err = wrappedInv.Apply(cic.dc, cic.mapper, cic.statusPolicy)
-	return pruneIds, err
+	return pruneIDs, err
 }
 
 // Replace stores the passed objects in the cluster inventory object, or
@@ -451,9 +451,9 @@ func (cic *ClusterClient) getMapping(obj *unstructured.Unstructured) (*meta.REST
 
 // getObjStatus returns the list of object status
 // at the beginning of an apply process.
-func getObjStatus(pruneIds, unionIds []object.ObjMetadata) []actuation.ObjectStatus {
+func getObjStatus(pruneIDs, unionIDs []object.ObjMetadata) []actuation.ObjectStatus {
 	status := []actuation.ObjectStatus{}
-	for _, obj := range unionIds {
+	for _, obj := range unionIDs {
 		status = append(status,
 			actuation.ObjectStatus{
 				ObjectReference: ObjectReferenceFromObjMetadata(obj),
@@ -462,7 +462,7 @@ func getObjStatus(pruneIds, unionIds []object.ObjMetadata) []actuation.ObjectSta
 				Reconcile:       actuation.ReconcilePending,
 			})
 	}
-	for _, obj := range pruneIds {
+	for _, obj := range pruneIDs {
 		status = append(status,
 			actuation.ObjectStatus{
 				ObjectReference: ObjectReferenceFromObjMetadata(obj),

pkg/inventory/inventory-client_test.go

@@ -409,7 +409,7 @@ func TestReplace(t *testing.T) {
 				t.Fatalf("unexpected error received: %s", err)
 			}
 			if diff := cmp.Diff(data, tc.data); diff != "" {
-				t.Fatalf(diff)
+				t.Fatal(diff)
 			}
 		})
 	}

pkg/inventory/inventorycm_test.go

@@ -65,7 +65,7 @@ func TestBuildObjMap(t *testing.T) {
 		t.Run(name, func(t *testing.T) {
 			actual := buildObjMap(tc.objSet, tc.objStatus)
 			if diff := cmp.Diff(actual, tc.expected); diff != "" {
-				t.Errorf(diff)
+				t.Error(diff)
 			}
 		})
 	}

pkg/kstatus/polling/clusterreader/caching_reader_test.go

@@ -388,9 +388,9 @@ func (f *fakeReader) List(_ context.Context, list client.ObjectList, opts ...cli
 			}
 		}
 		end := start + listOpts.Limit
-		max := int64(len(results))
-		if end > max {
-			end = max
+		maxResult := int64(len(results))
+		if end > maxResult {
+			end = maxResult
 		} else {
 			// set continue if more results are available
 			uList.SetContinue(strconv.FormatInt(end, 10))

pkg/object/unstructured_test.go

@@ -12,7 +12,7 @@ import (
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	cmdtesting "k8s.io/kubectl/pkg/cmd/testing"
-	. "sigs.k8s.io/cli-utils/pkg/object"
+	. "sigs.k8s.io/cli-utils/pkg/object" //nolint:revive
 	"sigs.k8s.io/cli-utils/pkg/testutil"
 )