feat(linters): Enable all linters by default.

.golangci.yml

@@ -1,3 +1,8 @@
+run:
+  deadline: 10m
+  skip-dirs:
+    - apis
+
 linters-settings:
   gocyclo:
     min-complexity: 30
@@ -13,35 +18,51 @@ linters-settings:
       - dot
     skip-generated: false
     custom-order: true
-
+  errcheck:
+    check-type-assertions: true
+  exhaustive:
+    check:
+      - switch
+    default-signifies-exhaustive: true
+  funlen:
+    lines: 100
+    statements: 100
+    ignore-comments: true
+  nolintlint:
+    allow-no-explanation: [ funlen, lll ]
+    require-specific: true
 linters:
-  disable-all: true
-  enable:
-    - bodyclose
-    - dogsled
-    - dupl
-    - errcheck
-    - exportloopref
-    - gci
-    - goconst
-    - gocritic
-    - gocyclo
-    - gofmt
-    - goprintffuncname
-    - gosimple
-    - govet
-    - ineffassign
-    - misspell
-    - nakedret
-    - nolintlint
-    - staticcheck
-    - stylecheck
-    - typecheck
-    - unconvert
-    - unparam
-    - unused
-    - whitespace
-    - lll
+  enable-all: true
+  disable:
+    - containedctx # detects struct contained context.Context field
+    - 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
+    - gomnd # Doesnot allow hardcoded numbers
+    - gomoddirectives # Doesnot allow replace in go mod file
+    - interfacer
+    - 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
+    - tagliatelle
+    - 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
+    - contextcheck # Requires to pass context to all function.
+
+    # To be fixed
+    - gocognit # https://github.com/opendatahub-io/opendatahub-operator/issues/709
+    - cyclop   # https://github.com/opendatahub-io/opendatahub-operator/issues/709
+    - funlen   # https://github.com/opendatahub-io/opendatahub-operator/issues/709
+    - godox    # https://github.com/opendatahub-io/opendatahub-operator/issues/699
 
 issues:
   exclude-rules:

apis/dscinitialization/v1/dscinitialization_types.go

@@ -70,7 +70,7 @@ type Monitoring struct {
 type DevFlags struct {
 	// Custom manifests uri for odh-manifests
 	// +optional
-	ManifestsUri string `json:"manifestsUri,omitempty"` //nolint
+	ManifestsUri string `json:"manifestsUri,omitempty"`
 }
 
 // DSCInitializationStatus defines the observed state of DSCInitialization.

components/codeflare/codeflare.go

@@ -82,7 +82,7 @@ func (c *CodeFlare) ReconcileComponent(ctx context.Context, cli client.Client, r
 		}
 
 		if found, err := deploy.OperatorExists(cli, dependentOperator); err != nil {
-			return fmt.Errorf("operator exists throws error %v", err)
+			return fmt.Errorf("operator exists throws error %w", err)
 		} else if found {
 			return fmt.Errorf("operator %s is found. Please uninstall the operator before enabling %s component",
 				dependentOperator, ComponentName)
@@ -97,7 +97,7 @@ func (c *CodeFlare) ReconcileComponent(ctx context.Context, cli client.Client, r
 	}
 
 	// Deploy Codeflare
-	if err := deploy.DeployManifestsFromPath(cli, owner,
+	if err := deploy.DeployManifestsFromPath(cli, owner, //nolint:revive,nolintlint
 		CodeflarePath,
 		dscispec.ApplicationsNamespace,
 		ComponentName, enabled); err != nil {

components/dashboard/dashboard.go

@@ -117,7 +117,7 @@ func (d *Dashboard) ReconcileComponent(ctx context.Context,
 			}
 			// Deploy CRDs
 			if err := d.deployCRDsForPlatform(cli, owner, dscispec.ApplicationsNamespace, platform); err != nil {
-				return fmt.Errorf("failed to deploy %s crds %s: %v", ComponentName, PathCRDs, err)
+				return fmt.Errorf("failed to deploy %s crds %s: %w", ComponentName, PathCRDs, err)
 			}
 		}
 
@@ -128,7 +128,7 @@ func (d *Dashboard) ReconcileComponent(ctx context.Context,
 			}
 			// Deploy CRDs
 			if err := d.deployCRDsForPlatform(cli, owner, dscispec.ApplicationsNamespace, platform); err != nil {
-				return fmt.Errorf("failed to deploy %s crds %s: %v", ComponentNameSupported, PathCRDs, err)
+				return fmt.Errorf("failed to deploy %s crds %s: %w", ComponentNameSupported, PathCRDs, err)
 			}
 			// Apply RHODS specific configs
 			if err := d.applyRhodsSpecificConfigs(cli, owner, dscispec.ApplicationsNamespace, platform); err != nil {
@@ -190,7 +190,6 @@ func (d *Dashboard) ReconcileComponent(ctx context.Context,
 				return err
 			}
 		}
-
 		return nil
 	default:
 		return nil
@@ -300,7 +299,7 @@ func (d *Dashboard) cleanOauthClient(cli client.Client, dscispec *dsciv1.DSCInit
 			}
 		} else {
 			if err := cli.Delete(context.TODO(), oauthClientSecret); err != nil {
-				return fmt.Errorf("error deleting secret %s in namespace %s : %w", name, dscispec.ApplicationsNamespace, err)
+				return fmt.Errorf("error deleting oauth client secret: %w", err)
 			}
 		}
 	}

components/kserve/kserve.go

@@ -144,9 +144,8 @@ func (k *Kserve) ReconcileComponent(ctx context.Context, cli client.Client, resC
 	}
 
 	if err := deploy.DeployManifestsFromPath(cli, owner, DependentPath, dscispec.ApplicationsNamespace, ComponentName, enabled); err != nil {
-		if strings.Contains(err.Error(), "spec.selector") && strings.Contains(err.Error(), "field is immutable") {
-			// ignore this error
-		} else {
+		if !strings.Contains(err.Error(), "spec.selector") || !strings.Contains(err.Error(), "field is immutable") {
+			// explicitly ignore error if error contains keywords "spec.selector" and "field is immutable" and return all other error.
 			return err
 		}
 	}
@@ -207,10 +206,7 @@ func (k *Kserve) removeServerlessFeatures(instance *dsciv1.DSCInitializationSpec
 		return err
 	}
 
-	if err := serverlessInitializer.Delete(); err != nil {
-		return err
-	}
-	return nil
+	return serverlessInitializer.Delete()
 }
 
 func checkDepedentOps(cli client.Client) *multierror.Error {

components/modelmeshserving/modelmeshserving.go

@@ -141,9 +141,8 @@ func (m *ModelMeshServing) ReconcileComponent(ctx context.Context,
 		}
 	}
 	if err := deploy.DeployManifestsFromPath(cli, owner, DependentPath, dscispec.ApplicationsNamespace, m.GetComponentName(), enabled); err != nil {
-		if strings.Contains(err.Error(), "spec.selector") && strings.Contains(err.Error(), "field is immutable") {
-			// ignore this error
-		} else {
+		// explicitly ignore error if error contains keywords "spec.selector" and "field is immutable" and return all other error.
+		if !strings.Contains(err.Error(), "spec.selector") || !strings.Contains(err.Error(), "field is immutable") {
 			return err
 		}
 	}

components/trustyai/trustyai.go

@@ -51,7 +51,7 @@ func (t *TrustyAI) GetComponentName() string {
 	return ComponentName
 }
 
-func (t *TrustyAI) ReconcileComponent(ctx context.Context, cli client.Client, resConf *rest.Config, owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, _ bool) error {
+func (t *TrustyAI) ReconcileComponent(_ context.Context, cli client.Client, _ *rest.Config, owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, _ bool) error {
 	var imageParamMap = map[string]string{
 		"trustyaiServiceImage":  "RELATED_IMAGE_ODH_TRUSTYAI_SERVICE_IMAGE",
 		"trustyaiOperatorImage": "RELATED_IMAGE_ODH_TRUSTYAI_SERVICE_OPERATOR_IMAGE",

controllers/datasciencecluster/datasciencecluster_controller.go

@@ -59,7 +59,7 @@ import (
 )
 
 // DataScienceClusterReconciler reconciles a DataScienceCluster object.
-type DataScienceClusterReconciler struct {
+type DataScienceClusterReconciler struct { //nolint:golint,revive
 	client.Client
 	Scheme     *runtime.Scheme
 	Log        logr.Logger
@@ -70,7 +70,7 @@ type DataScienceClusterReconciler struct {
 }
 
 // DataScienceClusterConfig passing Spec of DSCI for reconcile DataScienceCluster.
-type DataScienceClusterConfig struct {
+type DataScienceClusterConfig struct { //nolint:golint,revive
 	DSCISpec *dsci.DSCInitializationSpec
 }
 
@@ -95,7 +95,7 @@ func (r *DataScienceClusterReconciler) Reconcile(ctx context.Context, req ctrl.R
 		// Return and don't requeue
 		if upgrade.HasDeleteConfigMap(r.Client) {
 			if uninstallErr := upgrade.OperatorUninstall(r.Client, r.RestConfig); uninstallErr != nil {
-				return ctrl.Result{}, fmt.Errorf("error while operator uninstall: %v", uninstallErr)
+				return ctrl.Result{}, fmt.Errorf("error while operator uninstall: %w", uninstallErr)
 			}
 		}
 
@@ -142,9 +142,8 @@ func (r *DataScienceClusterReconciler) Reconcile(ctx context.Context, req ctrl.R
 			r.reportError(err, instance, "failed to update DataScienceCluster condition")
 
 			return ctrl.Result{}, err
-		} else {
-			return ctrl.Result{}, nil
 		}
+		return ctrl.Result{}, nil
 	case 1:
 		dscInitializationSpec := dsciInstances.Items[0].Spec
 		dscInitializationSpec.DeepCopyInto(r.DataScienceCluster.DSCISpec)
@@ -287,24 +286,23 @@ func (r *DataScienceClusterReconciler) reconcileSubComponent(ctx context.Context
 		})
 
 		return instance, err
-	} else {
-		// reconciliation succeeded: update status accordingly
-		instance, err = r.updateStatus(ctx, instance, func(saved *dsc.DataScienceCluster) {
-			if saved.Status.InstalledComponents == nil {
-				saved.Status.InstalledComponents = make(map[string]bool)
-			}
-			saved.Status.InstalledComponents[componentName] = enabled
-			if enabled {
-				status.SetComponentCondition(&saved.Status.Conditions, componentName, status.ReconcileCompleted, "Component reconciled successfully", corev1.ConditionTrue)
-			} else {
-				status.RemoveComponentCondition(&saved.Status.Conditions, componentName)
-			}
-		})
-		if err != nil {
-			instance = r.reportError(err, instance, "failed to update DataScienceCluster status after reconciling "+componentName)
-
-			return instance, err
+	}
+	// reconciliation succeeded: update status accordingly
+	instance, err = r.updateStatus(ctx, instance, func(saved *dsc.DataScienceCluster) {
+		if saved.Status.InstalledComponents == nil {
+			saved.Status.InstalledComponents = make(map[string]bool)
+		}
+		saved.Status.InstalledComponents[componentName] = enabled
+		if enabled {
+			status.SetComponentCondition(&saved.Status.Conditions, componentName, status.ReconcileCompleted, "Component reconciled successfully", corev1.ConditionTrue)
+		} else {
+			status.RemoveComponentCondition(&saved.Status.Conditions, componentName)
 		}
+	})
+	if err != nil {
+		instance = r.reportError(err, instance, "failed to update DataScienceCluster status after reconciling "+componentName)
+
+		return instance, err
 	}
 
 	return instance, nil
@@ -335,7 +333,7 @@ var configMapPredicates = predicate.Funcs{
 	},
 }
 
-// a workaround for 2.5 due to odh-model-controller serivceaccount keeps updates with label
+// a workaround for 2.5 due to odh-model-controller serivceaccount keeps updates with label.
 var saPredicates = predicate.Funcs{
 	UpdateFunc: func(e event.UpdateEvent) bool {
 		if e.ObjectNew.GetName() == "odh-model-controller" && (e.ObjectNew.GetNamespace() == "redhat-ods-applications" || e.ObjectNew.GetNamespace() == "opendatahub") {
@@ -345,7 +343,7 @@ var saPredicates = predicate.Funcs{
 	},
 }
 
-// a workaround for 2.5 due to modelmesh-servingruntime.serving.kserve.io keeps updates
+// a workaround for 2.5 due to modelmesh-servingruntime.serving.kserve.io keeps updates.
 var modelMeshwebhookPredicates = predicate.Funcs{
 	UpdateFunc: func(e event.UpdateEvent) bool {
 		return e.ObjectNew.GetName() != "modelmesh-servingruntime.serving.kserve.io"
@@ -376,14 +374,13 @@ var modelMeshRBPredicates = predicate.Funcs{
 	},
 }
 
-// ignore label updates if it is from application namespace
+// ignore label updates if it is from application namespace.
 var modelMeshGeneralPredicates = predicate.Funcs{
 	UpdateFunc: func(e event.UpdateEvent) bool {
 		if strings.Contains(e.ObjectNew.GetName(), "odh-model-controller") || strings.Contains(e.ObjectNew.GetName(), "kserve") {
 			return false
-		} else {
-			return true
 		}
+		return true
 	},
 }
 
@@ -439,7 +436,7 @@ func (r *DataScienceClusterReconciler) updateStatus(ctx context.Context, origina
 	return saved, err
 }
 
-func (r *DataScienceClusterReconciler) watchDataScienceClusterResources(a client.Object) (requests []reconcile.Request) {
+func (r *DataScienceClusterReconciler) watchDataScienceClusterResources(a client.Object) []reconcile.Request {
 	instanceList := &dsc.DataScienceClusterList{}
 	err := r.Client.List(context.TODO(), instanceList)
 	if err != nil {
@@ -466,9 +463,8 @@ func (r *DataScienceClusterReconciler) watchDataScienceClusterResources(a client
 			return []reconcile.Request{{
 				NamespacedName: types.NamespacedName{Name: requestName},
 			}}
-		} else {
-			return nil
 		}
+		return nil
 	}
 
 	return nil

controllers/datasciencecluster/suite_test.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package datasciencecluster
+package datasciencecluster_test
 
 import (
 	"path/filepath"