chore(lint): updates to latest version (opendatahub-io#1074)

Fixes

- removed irrelavant //nolint comments (see config updates below)
- converted unused func params to blank identifier when impl does not
  need it

New linters

perfsprint

- disabled fmt.Sprintf checks for one element
- adjusted errors creation to use errors.New instead of fmt.Errorf when
  no-arg strings are used as messages

Configuration updates

- configured revive to not complain about dot-imports for testing
  libraries
- adjusted configuration yaml due to changes in the structure
  (skip-dirs)
- some deprecated linters were removed from golangci-lint toolchain so they are now gone from the config too

.github/workflows/linter.yaml

@@ -18,6 +18,6 @@ jobs:
       - name: golangci-lint
         uses: golangci/golangci-lint-action@v4
         with:
-          version: v1.54.0
+          version: v1.59.1
           skip-cache: true
           args: --timeout 5m0s

.golangci.yml

@@ -1,7 +1,5 @@
 run:
   deadline: 10m
-  skip-dirs:
-    - apis
 
 linters-settings:
   gocyclo:
@@ -30,38 +28,38 @@ linters-settings:
   nolintlint:
     allow-no-explanation: [ funlen, lll ]
     require-specific: true
+  revive:
+    rules:
+      - name: dot-imports
+        arguments:
+          - allowedPackages: ["github.com/onsi/ginkgo/v2","github.com/onsi/gomega","github.com/onsi/gomega/gstruct"]
+  perfsprint:
+    sprintf1: false
+    strconcat: false
+
 linters:
   enable-all: true
   disable:
     - containedctx # detects struct contained context.Context field
-    - deadcode # deprecated
     - depguard # [replaced by gomodguard] checks if package imports are in a list of acceptable packages
-    - exhaustivestruct # Prevents empty struct. We use a lot of these so I think it is safe to disable.
     - exhaustruct # Prevents empty struct. We use a lot of these so I think it is safe to disable.c
     - forbidigo
     - gochecknoglobals # Prevents use of global vars.
     - gofumpt
-    - golint # deprecated
     - gomnd # Doesnot allow hardcoded numbers
     - gomoddirectives # Doesnot allow replace in go mod file
-    - ifshort # deprecated
-    - interfacer
-    - maligned # deprecated
+    - mnd
     - nestif
     - nilnil
-    - nosnakecase # snakecase is used in a lot of places. Need to check if that is required.
     - paralleltest # [too many false positives] detects missing usage of t.Parallel() method in your Go test
-    - scopelint # deprecated
-    - structcheck # deprecated
     - tagliatelle
-    - varcheck # deprecated
     - varnamelen # doesnot allow shorter names like c,k etc. But golang prefers short named vars.
     - wsl # [too strict and mostly code is not more readable] whitespace linter forces you to use empty lines
     - wrapcheck # check if this is required. Prevents direct return of err.
 
     # Need to check
     - nlreturn # [too strict and mostly code is not more readable] checks for a new line before return and branch statements to increase code clarity
-    - goerr113 # [too strict] checks the errors handling expressions
+    - err113 # [too strict] checks the errors handling expressions
     - contextcheck # Requires to pass context to all function.
 
     # To be fixed
@@ -71,6 +69,8 @@ linters:
     - godox    # https://github.com/opendatahub-io/opendatahub-operator/issues/699
 
 issues:
+  exclude-dirs:
+    - apis
   exclude-rules:
     - path: tests/*/(.+)_test\.go
       linters:

Makefile

@@ -70,7 +70,7 @@ YQ ?= $(LOCALBIN)/yq
 KUSTOMIZE_VERSION ?= v3.8.7
 CONTROLLER_GEN_VERSION ?= v0.9.2
 OPERATOR_SDK_VERSION ?= v1.31.0
-GOLANGCI_LINT_VERSION ?= v1.54.0
+GOLANGCI_LINT_VERSION ?= v1.59.1
 YQ_VERSION ?= v4.12.2
 # ENVTEST_K8S_VERSION refers to the version of kubebuilder assets to be downloaded by envtest binary.
 ENVTEST_K8S_VERSION = 1.24.2

apis/datasciencecluster/v1/datasciencecluster_types.go

@@ -18,7 +18,6 @@ package v1
 
 import (
 	"errors"
-	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster"
 	"reflect"
 
 	conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1"
@@ -36,6 +35,7 @@ import (
 	"github.com/opendatahub-io/opendatahub-operator/v2/components/trainingoperator"
 	"github.com/opendatahub-io/opendatahub-operator/v2/components/trustyai"
 	"github.com/opendatahub-io/opendatahub-operator/v2/components/workbenches"
+	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster"
 )
 
 // DataScienceClusterSpec defines the desired state of the cluster.

apis/dscinitialization/v1/dscinitialization_types.go

@@ -17,13 +17,13 @@ limitations under the License.
 package v1
 
 import (
-	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster"
 	operatorv1 "github.com/openshift/api/operator/v1"
 	conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	infrav1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/infrastructure/v1"
+	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster"
 )
 
 // +operator-sdk:csv:customresourcedefinitions:order=1

components/kserve/kserve_config_handler.go

@@ -3,6 +3,7 @@ package kserve
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 
 	operatorv1 "github.com/openshift/api/operator/v1"
@@ -36,7 +37,7 @@ func (k *Kserve) setupKserveConfig(ctx context.Context, cli client.Client, dscis
 		}
 	case operatorv1.Removed:
 		if k.DefaultDeploymentMode == Serverless {
-			return fmt.Errorf("setting defaultdeployment mode as Serverless is incompatible with having Serving 'Removed'")
+			return errors.New("setting defaultdeployment mode as Serverless is incompatible with having Serving 'Removed'")
 		}
 		if k.DefaultDeploymentMode == "" {
 			fmt.Println("Serving is removed, Kserve will default to rawdeployment")
@@ -128,7 +129,7 @@ func (k *Kserve) configureServerless(instance *dsciv1.DSCInitializationSpec) err
 	case operatorv1.Managed: // standard workflow to create CR
 		switch instance.ServiceMesh.ManagementState {
 		case operatorv1.Unmanaged, operatorv1.Removed:
-			return fmt.Errorf("ServiceMesh is need to set to 'Managed' in DSCI CR, it is required by KServe serving field")
+			return errors.New("ServiceMesh is need to set to 'Managed' in DSCI CR, it is required by KServe serving field")
 		}
 
 		serverlessFeatures := feature.ComponentFeaturesHandler(k.GetComponentName(), instance, k.configureServerlessFeatures())

controllers/certconfigmapgenerator/certconfigmapgenerator_controller.go

@@ -27,7 +27,7 @@ import (
 )
 
 // CertConfigmapGeneratorReconciler holds the controller configuration.
-type CertConfigmapGeneratorReconciler struct { //nolint:golint,revive // Readability
+type CertConfigmapGeneratorReconciler struct {
 	Client client.Client
 	Scheme *runtime.Scheme
 	Log    logr.Logger

controllers/datasciencecluster/datasciencecluster_controller.go

@@ -62,7 +62,7 @@ import (
 )
 
 // DataScienceClusterReconciler reconciles a DataScienceCluster object.
-type DataScienceClusterReconciler struct { //nolint:golint,revive
+type DataScienceClusterReconciler struct {
 	client.Client
 	Scheme *runtime.Scheme
 	Log    logr.Logger
@@ -72,7 +72,7 @@ type DataScienceClusterReconciler struct { //nolint:golint,revive
 }
 
 // DataScienceClusterConfig passing Spec of DSCI for reconcile DataScienceCluster.
-type DataScienceClusterConfig struct { //nolint:golint,revive
+type DataScienceClusterConfig struct {
 	DSCISpec *dsci.DSCInitializationSpec
 }
 
@@ -537,7 +537,7 @@ func (r *DataScienceClusterReconciler) getRequestName() (string, error) {
 	case len(instanceList.Items) == 0:
 		return "default-dsc", nil
 	default:
-		return "", fmt.Errorf("multiple DataScienceCluster instances found")
+		return "", errors.New("multiple DataScienceCluster instances found")
 	}
 }
 

controllers/dscinitialization/dscinitialization_controller.go

@@ -62,7 +62,7 @@ const (
 var managementStateChangeTrustedCA = false
 
 // DSCInitializationReconciler reconciles a DSCInitialization object.
-type DSCInitializationReconciler struct { //nolint:golint,revive // Readability
+type DSCInitializationReconciler struct {
 	client.Client
 	Scheme                *runtime.Scheme
 	Log                   logr.Logger

controllers/secretgenerator/secret.go

@@ -10,7 +10,7 @@ import (
 	annotation "github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/annotations"
 )
 
-//nolint:golint,revive,stylecheck //CAPS is preferred for const
+//nolint:golint,stylecheck //CAPS is preferred for const
 const (
 	SECRET_DEFAULT_COMPLEXITY = 16
 

controllers/secretgenerator/secretgenerator_controller.go

@@ -50,7 +50,7 @@ const (
 )
 
 // SecretGeneratorReconciler holds the controller configuration.
-type SecretGeneratorReconciler struct { //nolint:golint,revive // Readability
+type SecretGeneratorReconciler struct {
 	Client client.Client
 	Scheme *runtime.Scheme
 	Log    logr.Logger

controllers/status/reporter.go

@@ -3,7 +3,7 @@ package status
 
 import (
 	"context"
-	"fmt"
+	"errors"
 
 	"k8s.io/client-go/util/retry"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -43,7 +43,7 @@ type SaveStatusFunc[T client.Object] func(saved T)
 func UpdateWithRetry[T client.Object](ctx context.Context, cli client.Client, original T, update SaveStatusFunc[T]) (T, error) {
 	saved, ok := original.DeepCopyObject().(T)
 	if !ok {
-		return *new(T), fmt.Errorf("failed to deep copy object")
+		return *new(T), errors.New("failed to deep copy object")
 	}
 	err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
 		if err := cli.Get(ctx, client.ObjectKeyFromObject(original), saved); err != nil {

pkg/cluster/resources.go

@@ -2,6 +2,7 @@ package cluster
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"time"
 
@@ -87,7 +88,7 @@ func CreateSecret(ctx context.Context, cli client.Client, name, namespace string
 // ConfigMap.ObjectMeta.Name and ConfigMap.ObjectMeta.Namespace are both required, it returns an error otherwise.
 func CreateOrUpdateConfigMap(ctx context.Context, c client.Client, desiredCfgMap *corev1.ConfigMap, metaOptions ...MetaOptions) error {
 	if desiredCfgMap.GetName() == "" || desiredCfgMap.GetNamespace() == "" {
-		return fmt.Errorf("configmap name and namespace must be set")
+		return errors.New("configmap name and namespace must be set")
 	}
 
 	existingCfgMap := &corev1.ConfigMap{}

pkg/deploy/deploy.go

@@ -142,7 +142,7 @@ func DownloadManifests(componentName string, manifestConfig components.Manifests
 	return err
 }
 
-func DeployManifestsFromPath(cli client.Client, owner metav1.Object, manifestPath string, namespace string, componentName string, componentEnabled bool) error { //nolint:golint,revive,lll
+func DeployManifestsFromPath(cli client.Client, owner metav1.Object, manifestPath string, namespace string, componentName string, componentEnabled bool) error {
 	// Render the Kustomize manifests
 	k := krusty.MakeKustomizer(krusty.MakeDefaultOptions())
 	fs := filesys.MakeFsOnDisk()

pkg/feature/builder.go

@@ -29,7 +29,7 @@ type featureBuilder struct {
 }
 
 // CreateFeature creates a new feature builder with the given name.
-func CreateFeature(name string) *usingFeaturesHandler { //nolint:golint,revive //No need to export featureBuilder.
+func CreateFeature(name string) *usingFeaturesHandler {
 	return &usingFeaturesHandler{
 		name: name,
 	}

pkg/feature/feature_tracker_handler.go

@@ -87,7 +87,7 @@ func (f *Feature) ensureGVKSet(obj runtime.Object) error {
 		return fmt.Errorf("failed to get group, version, & kinds for object: %w", err)
 	}
 	if unversioned {
-		return fmt.Errorf("object is unversioned")
+		return errors.New("object is unversioned")
 	}
 	// Update the target object back with one of the discovered GVKs.
 	obj.GetObjectKind().SetGroupVersionKind(gvks[0])

pkg/feature/manifest.go

@@ -143,7 +143,7 @@ func loadManifestsFrom(fsys fs.FS, path string) ([]Manifest, error) {
 	return manifests, nil
 }
 
-func CreateRawManifestFrom(fsys fs.FS, path string) *rawManifest { //nolint:golint,revive //No need to export rawManifest.
+func CreateRawManifestFrom(fsys fs.FS, path string) *rawManifest {
 	basePath := filepath.Base(path)
 
 	return &rawManifest{
@@ -154,7 +154,7 @@ func CreateRawManifestFrom(fsys fs.FS, path string) *rawManifest { //nolint:goli
 	}
 }
 
-func CreateTemplateManifestFrom(fsys fs.FS, path string) *templateManifest { //nolint:golint,revive //No need to export templateManifest.
+func CreateTemplateManifestFrom(fsys fs.FS, path string) *templateManifest {
 	basePath := filepath.Base(path)
 
 	return &templateManifest{

pkg/feature/serverless/conditions.go

@@ -2,6 +2,7 @@ package serverless
 
 import (
 	"context"
+	"errors"
 	"fmt"
 
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -28,7 +29,7 @@ func EnsureServerlessAbsent(f *feature.Feature) error {
 	}
 
 	if len(list.Items) > 1 {
-		return fmt.Errorf("multiple KNativeServing resources found, which is an unsupported state")
+		return errors.New("multiple KNativeServing resources found, which is an unsupported state")
 	}
 
 	servingOwners := list.Items[0].GetOwnerReferences()
@@ -42,7 +43,7 @@ func EnsureServerlessAbsent(f *feature.Feature) error {
 		}
 	}
 
-	return fmt.Errorf("existing KNativeServing resource was found; integrating to an existing installation is not supported")
+	return errors.New("existing KNativeServing resource was found; integrating to an existing installation is not supported")
 }
 
 func EnsureServerlessOperatorInstalled(f *feature.Feature) error {

tests/e2e/controller_setup_test.go

@@ -57,7 +57,7 @@ type testContext struct {
 	ctx context.Context
 }
 
-func NewTestContext() (*testContext, error) { //nolint:golint,revive // Only used in tests
+func NewTestContext() (*testContext, error) {
 	// GetConfig(): If KUBECONFIG env variable is set, it is used to create
 	// the client, else the inClusterConfig() is used.
 	// Lastly if none of them are set, it uses  $HOME/.kube/config to create the client.

tests/e2e/dsc_cfmap_deletion_test.go

@@ -2,6 +2,7 @@ package e2e_test
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"testing"
 
@@ -63,7 +64,7 @@ func (tc *testContext) testOwnedNamespacesAllExist() error {
 		return fmt.Errorf("failed getting owned namespaces %w", err)
 	}
 	if len(namespaces.Items) == 0 {
-		return fmt.Errorf("all namespaces are gone")
+		return errors.New("all namespaces are gone")
 	}
 
 	return nil

tests/e2e/dsc_creation_test.go

@@ -2,6 +2,7 @@ package e2e_test
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"log"
 	"reflect"
@@ -12,7 +13,7 @@ import (
 	operatorv1 "github.com/openshift/api/operator/v1"
 	"github.com/stretchr/testify/require"
 	autoscalingv1 "k8s.io/api/autoscaling/v1"
-	"k8s.io/apimachinery/pkg/api/errors"
+	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/wait"
@@ -102,7 +103,7 @@ func (tc *testContext) testDSCICreation() error {
 	// create one for you
 	err = tc.customClient.Get(tc.ctx, dscLookupKey, createdDSCI)
 	if err != nil {
-		if errors.IsNotFound(err) {
+		if k8serrors.IsNotFound(err) {
 			nberr := wait.PollUntilContextTimeout(tc.ctx, tc.resourceRetryInterval, tc.resourceCreationTimeout, false, func(ctx context.Context) (bool, error) {
 				creationErr := tc.customClient.Create(tc.ctx, tc.testDSCI)
 				if creationErr != nil {
@@ -160,7 +161,7 @@ func (tc *testContext) testDSCCreation() error {
 
 	err = tc.customClient.Get(tc.ctx, dscLookupKey, createdDSC)
 	if err != nil {
-		if errors.IsNotFound(err) {
+		if k8serrors.IsNotFound(err) {
 			nberr := wait.PollUntilContextTimeout(tc.ctx, tc.resourceRetryInterval, tc.resourceCreationTimeout, false, func(ctx context.Context) (bool, error) {
 				creationErr := tc.customClient.Create(tc.ctx, tc.testDsc)
 				if creationErr != nil {
@@ -473,7 +474,7 @@ func (tc *testContext) testUpdateDSCComponentEnabled() error {
 			}
 		}
 	} else {
-		return fmt.Errorf("dashboard spec should be in 'enabled: true' state in order to perform test")
+		return errors.New("dashboard spec should be in 'enabled: true' state in order to perform test")
 	}
 
 	// Disable component Dashboard
@@ -503,7 +504,7 @@ func (tc *testContext) testUpdateDSCComponentEnabled() error {
 	time.Sleep(4 * tc.resourceRetryInterval)
 	_, err = tc.kubeClient.AppsV1().Deployments(tc.applicationsNamespace).Get(context.TODO(), dashboardDeploymentName, metav1.GetOptions{})
 	if err != nil {
-		if errors.IsNotFound(err) {
+		if k8serrors.IsNotFound(err) {
 			return nil // correct result: should not find deployment after we disable it already
 		}
 		return fmt.Errorf("error getting component resource after reconcile: %w", err)

tests/envtestutil/cleaner.go

@@ -14,7 +14,7 @@ import (
 	"k8s.io/client-go/rest"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
-	. "github.com/onsi/gomega" //nolint:revive,golint,stylecheck // This is the standard for ginkgo and gomega.
+	. "github.com/onsi/gomega" //nolint:stylecheck // This is the standard for ginkgo and gomega.
 )
 
 // Cleaner is a struct to perform deletion of resources,