diff --git a/.github/workflows/linter.yaml b/.github/workflows/linter.yaml index 98b6f0e03cb..3d40b46d66c 100644 --- a/.github/workflows/linter.yaml +++ b/.github/workflows/linter.yaml @@ -22,6 +22,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 diff --git a/.golangci.yml b/.golangci.yml index e4e2a668c95..3683f6cfcc4 100644 --- a/.golangci.yml +++ b/.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: diff --git a/Makefile b/Makefile index 1a873d08346..b4bfc44f553 100644 --- a/Makefile +++ b/Makefile @@ -67,7 +67,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 diff --git a/components/kserve/kserve_config_handler.go b/components/kserve/kserve_config_handler.go index 3bc91db1657..919192e2071 100644 --- a/components/kserve/kserve_config_handler.go +++ b/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(_ client.Client, instance *dsciv1.DSCInitia 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()) diff --git a/controllers/certconfigmapgenerator/certconfigmapgenerator_controller.go b/controllers/certconfigmapgenerator/certconfigmapgenerator_controller.go index f594be8d867..dacaa6be5a5 100644 --- a/controllers/certconfigmapgenerator/certconfigmapgenerator_controller.go +++ b/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 diff --git a/controllers/datasciencecluster/datasciencecluster_controller.go b/controllers/datasciencecluster/datasciencecluster_controller.go index 23ce52336ff..a3ede90cf4f 100644 --- a/controllers/datasciencecluster/datasciencecluster_controller.go +++ b/controllers/datasciencecluster/datasciencecluster_controller.go @@ -19,6 +19,7 @@ package datasciencecluster import ( "context" + "errors" "fmt" "strings" "time" @@ -60,7 +61,7 @@ import ( ) // DataScienceClusterReconciler reconciles a DataScienceCluster object. -type DataScienceClusterReconciler struct { //nolint:golint,revive +type DataScienceClusterReconciler struct { client.Client Scheme *runtime.Scheme Log logr.Logger @@ -70,7 +71,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 } @@ -511,7 +512,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") } } diff --git a/controllers/dscinitialization/dscinitialization_controller.go b/controllers/dscinitialization/dscinitialization_controller.go index 602dcdd26bb..0b7913c3557 100644 --- a/controllers/dscinitialization/dscinitialization_controller.go +++ b/controllers/dscinitialization/dscinitialization_controller.go @@ -60,7 +60,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 diff --git a/controllers/secretgenerator/secret.go b/controllers/secretgenerator/secret.go index bed0a2be00e..dd9aa60c532 100644 --- a/controllers/secretgenerator/secret.go +++ b/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 diff --git a/controllers/secretgenerator/secretgenerator_controller.go b/controllers/secretgenerator/secretgenerator_controller.go index b28d2c6282a..5b132edd210 100644 --- a/controllers/secretgenerator/secretgenerator_controller.go +++ b/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 diff --git a/controllers/status/reporter.go b/controllers/status/reporter.go index 3f9ddd28459..7b4cb461870 100644 --- a/controllers/status/reporter.go +++ b/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 { diff --git a/pkg/cluster/resources.go b/pkg/cluster/resources.go index b30d40b784c..379e8f78e20 100644 --- a/pkg/cluster/resources.go +++ b/pkg/cluster/resources.go @@ -2,6 +2,7 @@ package cluster import ( "context" + "errors" "fmt" "time" @@ -89,7 +90,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{} diff --git a/pkg/deploy/deploy.go b/pkg/deploy/deploy.go index 74597df213b..b3a4a146ecc 100644 --- a/pkg/deploy/deploy.go +++ b/pkg/deploy/deploy.go @@ -145,7 +145,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() diff --git a/pkg/feature/builder.go b/pkg/feature/builder.go index cc7e3f26a49..237f0f5f48b 100644 --- a/pkg/feature/builder.go +++ b/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, } diff --git a/pkg/feature/feature_tracker_handler.go b/pkg/feature/feature_tracker_handler.go index af1ccc13548..a7a23ea7859 100644 --- a/pkg/feature/feature_tracker_handler.go +++ b/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]) diff --git a/pkg/feature/manifest.go b/pkg/feature/manifest.go index 390629b8a74..153f67871bc 100644 --- a/pkg/feature/manifest.go +++ b/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{ diff --git a/pkg/feature/serverless/conditions.go b/pkg/feature/serverless/conditions.go index 86f0561e812..fadea01643a 100644 --- a/pkg/feature/serverless/conditions.go +++ b/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 { diff --git a/tests/e2e/controller_setup_test.go b/tests/e2e/controller_setup_test.go index 322aa69c5d1..00115810a53 100644 --- a/tests/e2e/controller_setup_test.go +++ b/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. diff --git a/tests/e2e/dsc_cfmap_deletion_test.go b/tests/e2e/dsc_cfmap_deletion_test.go index 48e783fd944..1d1130c9e94 100644 --- a/tests/e2e/dsc_cfmap_deletion_test.go +++ b/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 diff --git a/tests/e2e/dsc_creation_test.go b/tests/e2e/dsc_creation_test.go index 7a129a016e3..5b7d96ef9f2 100644 --- a/tests/e2e/dsc_creation_test.go +++ b/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/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" @@ -103,7 +104,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 { @@ -163,7 +164,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 { @@ -193,7 +194,7 @@ func (tc *testContext) requireInstalled(t *testing.T, gvk schema.GroupVersionKin err := tc.customClient.List(tc.ctx, list) require.NoErrorf(t, err, "Could not get %s list", gvk.Kind) - require.Greaterf(t, len(list.Items), 0, "%s has not been installed", gvk.Kind) + require.Emptyf(t, len(list.Items), "%s has not been installed", gvk.Kind) } func (tc *testContext) testDuplication(t *testing.T, gvk schema.GroupVersionKind, o any) { @@ -480,7 +481,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 @@ -511,7 +512,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 } diff --git a/tests/envtestutil/cleaner.go b/tests/envtestutil/cleaner.go index 366f4b3e10a..7c365ae2b9a 100644 --- a/tests/envtestutil/cleaner.go +++ b/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, diff --git a/tests/envtestutil/utils.go b/tests/envtestutil/utils.go index 3a6d2396479..b730706a9c4 100644 --- a/tests/envtestutil/utils.go +++ b/tests/envtestutil/utils.go @@ -1,7 +1,7 @@ package envtestutil import ( - "fmt" + "errors" "os" "path/filepath" ) @@ -25,5 +25,5 @@ func FindProjectRoot() (string, error) { currentDir = parentDir } - return "", fmt.Errorf("project root not found") + return "", errors.New("project root not found") } diff --git a/tests/integration/features/serverless_feature_test.go b/tests/integration/features/serverless_feature_test.go index 32690e2d0b6..9e518002734 100644 --- a/tests/integration/features/serverless_feature_test.go +++ b/tests/integration/features/serverless_feature_test.go @@ -2,6 +2,7 @@ package features_test import ( "context" + "errors" "fmt" corev1 "k8s.io/api/core/v1" @@ -267,7 +268,7 @@ var _ = Describe("Serverless feature", func() { } if secret == nil { - return fmt.Errorf("secret not found") + return errors.New("secret not found") } return nil diff --git a/tests/integration/features/tracker_int_test.go b/tests/integration/features/tracker_int_test.go index c9aa9b796e5..7fcaaa20164 100644 --- a/tests/integration/features/tracker_int_test.go +++ b/tests/integration/features/tracker_int_test.go @@ -1,7 +1,7 @@ package features_test import ( - "fmt" + "errors" conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1" corev1 "k8s.io/api/core/v1" @@ -65,8 +65,8 @@ var _ = Describe("Feature tracking capability", func() { verificationFeatureErr := feature.CreateFeature("precondition-fail"). For(handler). UsingConfig(envTest.Config). - PreConditions(func(f *feature.Feature) error { - return fmt.Errorf("during test always fail") + PreConditions(func(_ *feature.Feature) error { + return errors.New("during test always fail") }). Load() @@ -97,8 +97,8 @@ var _ = Describe("Feature tracking capability", func() { verificationFeatureErr := feature.CreateFeature("post-condition-failure"). For(handler). UsingConfig(envTest.Config). - PostConditions(func(f *feature.Feature) error { - return fmt.Errorf("during test always fail") + PostConditions(func(_ *feature.Feature) error { + return errors.New("during test always fail") }). Load()