Merge pull request #3208 from xrstf/bump-golangci-lint

🌱 Bump golangci-lint, remove staticcheck

Author: kcp-ci-bot

.golangci.yaml

@@ -10,11 +10,11 @@ linters:
   - bidichk
   - bodyclose
   - containedctx
+  - copyloopvar
   - dupword
   - durationcheck
   - errcheck
   - errchkjson
-  - exportloopref
   - gocritic
   - godot
   - gofmt
@@ -51,9 +51,11 @@ linters-settings:
     allow-leading-space: false
     require-specific: true
   revive:
-    revive:
+    # make sure error-strings issues actually surface (default confidence is 0.8)
+    confidence: 0.6
     rules:
       - name: context-keys-type
+      - name: duplicated-imports
       - name: error-return
       - name: error-strings
       - name: error-naming

Makefile

@@ -55,14 +55,10 @@ OPENSHIFT_GOIMPORTS_BIN := openshift-goimports
 OPENSHIFT_GOIMPORTS := $(TOOLS_DIR)/$(OPENSHIFT_GOIMPORTS_BIN)-$(OPENSHIFT_GOIMPORTS_VER)
 export OPENSHIFT_GOIMPORTS # so hack scripts can use it
 
-GOLANGCI_LINT_VER := v1.58.1
+GOLANGCI_LINT_VER := v1.62.2
 GOLANGCI_LINT_BIN := golangci-lint
 GOLANGCI_LINT := $(TOOLS_GOBIN_DIR)/$(GOLANGCI_LINT_BIN)-$(GOLANGCI_LINT_VER)
 
-STATICCHECK_VER := 2023.1.7
-STATICCHECK_BIN := staticcheck
-STATICCHECK := $(TOOLS_GOBIN_DIR)/$(STATICCHECK_BIN)-$(STATICCHECK_VER)
-
 GOTESTSUM_VER := v1.8.1
 GOTESTSUM_BIN := gotestsum
 GOTESTSUM := $(abspath $(TOOLS_DIR))/$(GOTESTSUM_BIN)-$(GOTESTSUM_VER)
@@ -140,9 +136,6 @@ install: require-jq require-go require-git verify-go-versions ## Install the pro
 $(GOLANGCI_LINT):
 	GOBIN=$(TOOLS_GOBIN_DIR) $(GO_INSTALL) github.com/golangci/golangci-lint/cmd/golangci-lint $(GOLANGCI_LINT_BIN) $(GOLANGCI_LINT_VER)
 
-$(STATICCHECK):
-	GOBIN=$(TOOLS_GOBIN_DIR) $(GO_INSTALL) honnef.co/go/tools/cmd/staticcheck $(STATICCHECK_BIN) $(STATICCHECK_VER)
-
 $(LOGCHECK):
 	GOBIN=$(TOOLS_GOBIN_DIR) $(GO_INSTALL) sigs.k8s.io/logtools/logcheck $(LOGCHECK_BIN) $(LOGCHECK_VER)
 
@@ -152,9 +145,8 @@ $(CODE_GENERATOR):
 $(KCP_APIGEN_GEN):
 	pushd . && cd sdk && GOBIN=$(TOOLS_GOBIN_DIR) go install ./cmd/apigen && popd
 
-lint: $(GOLANGCI_LINT) $(STATICCHECK) $(LOGCHECK) ## Verify lint
+lint: $(GOLANGCI_LINT) $(LOGCHECK) ## Verify lint
 	$(GOLANGCI_LINT) run --timeout 20m ./...
-	$(STATICCHECK) -checks ST1019,ST1005 ./...
 	./hack/verify-contextual-logging.sh
 .PHONY: lint
 

cmd/virtual-workspaces/options/options.go

@@ -118,6 +118,7 @@ func (o *Options) Validate() error {
 		errs = append(errs, fmt.Errorf("--kubeconfig is required for this command"))
 	}
 	if !strings.HasPrefix(o.RootPathPrefix, "/") {
+		//nolint:revive
 		errs = append(errs, fmt.Errorf("RootPathPrefix %q must start with /", o.RootPathPrefix))
 	}
 

pkg/admission/kubequota/kubequota_clusterworkspace_monitor.go

@@ -106,6 +106,7 @@ func (m *LogicalClusterDeletionMonitor) processNextWorkItem() bool {
 	defer m.queue.Done(key)
 
 	if err := m.process(key); err != nil {
+		//nolint:revive
 		runtime.HandleError(fmt.Errorf("LogicalClusterDeletionMonitor failed to sync %q, err: %w", key, err))
 
 		m.queue.AddRateLimited(key)

pkg/admission/logicalcluster/admission.go

@@ -18,6 +18,7 @@ package logicalcluster
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"io"
 
@@ -157,25 +158,25 @@ func (o *plugin) Validate(ctx context.Context, a admission.Attributes, _ admissi
 		newStatus := toSet(logicalCluster.Status.Initializers)
 
 		if !oldSpec.Equal(newSpec) {
-			return admission.NewForbidden(a, fmt.Errorf("spec.initializers is immutable"))
+			return admission.NewForbidden(a, errors.New("spec.initializers is immutable"))
 		}
 
 		transitioningToInitializing := old.Status.Phase != corev1alpha1.LogicalClusterPhaseInitializing && logicalCluster.Status.Phase == corev1alpha1.LogicalClusterPhaseInitializing
 		if transitioningToInitializing && !newSpec.Equal(newStatus) {
-			return admission.NewForbidden(a, fmt.Errorf("status.initializers do not equal spec.initializers"))
+			return admission.NewForbidden(a, errors.New("status.initializers do not equal spec.initializers"))
 		}
 
 		if !transitioningToInitializing && logicalCluster.Status.Phase == corev1alpha1.LogicalClusterPhaseInitializing && !oldStatus.IsSuperset(newStatus) {
-			return admission.NewForbidden(a, fmt.Errorf("status.initializers must not grow"))
+			return admission.NewForbidden(a, errors.New("status.initializers must not grow"))
 		}
 
 		if logicalCluster.Status.Phase != corev1alpha1.LogicalClusterPhaseInitializing && !oldStatus.Equal(newStatus) {
-			return admission.NewForbidden(a, fmt.Errorf("status.initializers is immutable after initialization"))
+			return admission.NewForbidden(a, errors.New("status.initializers is immutable after initialization"))
 		}
 
 		if old.Status.Phase == corev1alpha1.LogicalClusterPhaseInitializing && logicalCluster.Status.Phase != corev1alpha1.LogicalClusterPhaseInitializing {
 			if len(logicalCluster.Status.Initializers) > 0 {
-				return admission.NewForbidden(a, fmt.Errorf("status.initializers is not empty"))
+				return admission.NewForbidden(a, errors.New("status.initializers is not empty"))
 			}
 		}
 
@@ -188,22 +189,23 @@ func (o *plugin) Validate(ctx context.Context, a admission.Attributes, _ admissi
 	case admission.Delete:
 		logicalCluster, err := o.logicalClusterLister.Cluster(clusterName).Get(corev1alpha1.LogicalClusterName)
 		if err != nil {
+			//nolint:revive
 			return fmt.Errorf("LogicalCluster cannot be deleted: %w", err)
 		}
 		if !logicalCluster.Spec.DirectlyDeletable {
-			return admission.NewForbidden(a, fmt.Errorf("LogicalCluster cannot be deleted"))
+			return admission.NewForbidden(a, errors.New("LogicalCluster cannot be deleted")) //nolint:revive
 		}
 
 	case admission.Create:
-		return admission.NewForbidden(a, fmt.Errorf("LogicalCluster cannot be created"))
+		return admission.NewForbidden(a, errors.New("LogicalCluster cannot be created")) //nolint:revive
 	}
 
 	return nil
 }
 
 func (o *plugin) ValidateInitialization() error {
 	if o.logicalClusterLister == nil {
-		return fmt.Errorf(PluginName + " plugin needs an LogicalCluster lister")
+		return errors.New(PluginName + " plugin needs an LogicalCluster lister")
 	}
 	return nil
 }

pkg/cache/server/config.go

@@ -17,6 +17,7 @@ limitations under the License.
 package server
 
 import (
+	"errors"
 	"fmt"
 	"net/http"
 	"strings"
@@ -165,16 +166,18 @@ func NewConfig(opts *cacheserveroptions.CompletedOptions, optionalLocalShardRest
 		rt,
 		func(rq *http.Request) (string, string, error) {
 			if serverConfig.Config.RequestInfoResolver == nil {
-				return "", "", fmt.Errorf("RequestInfoResolver wasn't provided")
+				return "", "", errors.New("no RequestInfoResolver provided")
 			}
 			// the k8s request info resolver expects a cluster-less path, but the client we're using knows how to
 			// add the cluster we are targeting to the path before this round-tripper fires, so we need to strip it
 			// to use the k8s library
 			parts := strings.Split(rq.URL.Path, "/")
 			if len(parts) < 4 {
+				//nolint:revive
 				return "", "", fmt.Errorf("RequestInfoResolver: got invalid path: %v", rq.URL.Path)
 			}
 			if parts[1] != "clusters" {
+				//nolint:revive
 				return "", "", fmt.Errorf("RequestInfoResolver: got path without cluster prefix: %v", rq.URL.Path)
 			}
 			// we clone the request here to safely mutate the URL path, but this cloned request is never realized

pkg/reconciler/apis/apibinding/apibinding_reconcile.go

@@ -357,7 +357,8 @@ func (r *bindingReconciler) reconcile(ctx context.Context, apiBinding *apisv1alp
 						apisv1alpha1.BindingUpToDate,
 						apisv1alpha1.APIResourceSchemaInvalidReason,
 						conditionsv1alpha1.ConditionSeverityError,
-						fmt.Sprintf("APIResourceSchema %s|%s is invalid: %v\"", schemaClusterName, schemaName, status.Status().Details.Causes),
+						"APIResourceSchema %s|%s is invalid: %v",
+						schemaClusterName, schemaName, status.Status().Details.Causes,
 					)
 					// Only change InitialBindingCompleted if it's false
 					if conditions.IsFalse(apiBinding, apisv1alpha1.InitialBindingCompleted) {
@@ -366,7 +367,8 @@ func (r *bindingReconciler) reconcile(ctx context.Context, apiBinding *apisv1alp
 							apisv1alpha1.InitialBindingCompleted,
 							apisv1alpha1.APIResourceSchemaInvalidReason,
 							conditionsv1alpha1.ConditionSeverityError,
-							fmt.Sprintf("APIResourceSchema %s|%s is invalid: %v\"", schemaClusterName, schemaName, status.Status().Details.Causes),
+							"APIResourceSchema %s|%s is invalid: %v",
+							schemaClusterName, schemaName, status.Status().Details.Causes,
 						)
 					}
 
@@ -453,7 +455,8 @@ func (r *bindingReconciler) reconcile(ctx context.Context, apiBinding *apisv1alp
 			apisv1alpha1.BindingUpToDate,
 			apisv1alpha1.WaitingForEstablishedReason,
 			conditionsv1alpha1.ConditionSeverityInfo,
-			"Waiting for API(s) to be established: %s", strings.Join(needToWaitForRequeueWhenEstablished, ", "),
+			"Waiting for API(s) to be established: %s",
+			strings.Join(needToWaitForRequeueWhenEstablished, ", "),
 		)
 
 		// Only change InitialBindingCompleted if it's false
@@ -463,7 +466,8 @@ func (r *bindingReconciler) reconcile(ctx context.Context, apiBinding *apisv1alp
 				apisv1alpha1.InitialBindingCompleted,
 				apisv1alpha1.WaitingForEstablishedReason,
 				conditionsv1alpha1.ConditionSeverityInfo,
-				"Waiting for API(s) to be established: %s", strings.Join(needToWaitForRequeueWhenEstablished, ", "),
+				"Waiting for API(s) to be established: %s",
+				strings.Join(needToWaitForRequeueWhenEstablished, ", "),
 			)
 		}
 	} else {
@@ -512,7 +516,6 @@ func generateCRD(schema *apisv1alpha1.APIResourceSchema) (*apiextensionsv1.Custo
 	}
 
 	for _, version := range schema.Spec.Versions {
-		version := version
 		crdVersion := apiextensionsv1.CustomResourceDefinitionVersion{
 			Name:                     version.Name,
 			Served:                   version.Served,

pkg/reconciler/apis/apibindingdeletion/apibinding_deletion_controller.go

@@ -239,7 +239,8 @@ func (c *Controller) process(ctx context.Context, key string) error {
 			apisv1alpha1.BindingResourceDeleteSuccess,
 			ResourceDeletionFailedReason,
 			conditionsv1alpha1.ConditionSeverityError,
-			deleteErr.Error(),
+			"%v",
+			deleteErr,
 		)
 
 		newResource := &Resource{ObjectMeta: apibindingCopy.ObjectMeta, Spec: &apibindingCopy.Spec, Status: &apibindingCopy.Status}
@@ -294,7 +295,8 @@ func (c *Controller) mutateResourceRemainingStatus(resourceRemaining gvrDeletion
 			apisv1alpha1.BindingResourceDeleteSuccess,
 			ResourceFinalizersRemainReason,
 			conditionsv1alpha1.ConditionSeverityError,
-			fmt.Sprintf("Some content in the workspace has finalizers remaining: %s", strings.Join(remainingByFinalizer, ", ")),
+			"Some content in the workspace has finalizers remaining: %s",
+			strings.Join(remainingByFinalizer, ", "),
 		)
 
 		return apibinding, &deletion.ResourcesRemainingError{
@@ -320,7 +322,8 @@ func (c *Controller) mutateResourceRemainingStatus(resourceRemaining gvrDeletion
 			apisv1alpha1.BindingResourceDeleteSuccess,
 			ResourceRemainingReason,
 			conditionsv1alpha1.ConditionSeverityError,
-			fmt.Sprintf("Some resources are remaining: %s", strings.Join(remainingResources, ", ")),
+			"Some resources are remaining: %s",
+			strings.Join(remainingResources, ", "),
 		)
 
 		return apibinding, &deletion.ResourcesRemainingError{

pkg/reconciler/apis/apiexport/apiexport_controller_test.go

@@ -138,8 +138,6 @@ func TestReconcile(t *testing.T) {
 	}
 
 	for name, tc := range tests {
-		tc := tc // to avoid t.Parallel() races
-
 		t.Run(name, func(t *testing.T) {
 			createSecretCalled := false
 

pkg/reconciler/apis/apiexport/apiexport_reconcile.go

@@ -91,7 +91,8 @@ func (c *controller) reconcile(ctx context.Context, apiExport *apisv1alpha1.APIE
 			apisv1alpha1.APIExportIdentityValid,
 			apisv1alpha1.IdentityVerificationFailedReason,
 			conditionsv1alpha1.ConditionSeverityError,
-			err.Error(),
+			"%v",
+			err,
 		)
 	}
 
@@ -115,7 +116,8 @@ func (c *controller) reconcile(ctx context.Context, apiExport *apisv1alpha1.APIE
 			apisv1alpha1.APIExportVirtualWorkspaceURLsReady,
 			apisv1alpha1.ErrorGeneratingURLsReason,
 			conditionsv1alpha1.ConditionSeverityError,
-			err.Error(),
+			"%v",
+			err,
 		)
 	}