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

@@ -93,7 +93,7 @@ func (c *CodeFlare) ReconcileComponent(cli client.Client, owner metav1.Object, d
 	}
 
 	// 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

@@ -107,7 +107,7 @@ func (d *Dashboard) ReconcileComponent(cli client.Client, owner metav1.Object, d
 			}
 			// 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)
 			}
 		}
 
@@ -118,7 +118,7 @@ func (d *Dashboard) ReconcileComponent(cli client.Client, owner metav1.Object, d
 			}
 			// 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 {
@@ -172,7 +172,6 @@ func (d *Dashboard) ReconcileComponent(cli client.Client, owner metav1.Object, d
 				return err
 			}
 		}
-
 		return nil
 	default:
 		return nil
@@ -281,7 +280,7 @@ func (d *Dashboard) cleanOauthClientSecrets(cli client.Client, dscispec *dsciv1.
 		} else {
 			err := cli.Delete(context.TODO(), oauthClientSecret)
 			if err != nil {
-				return fmt.Errorf("error deleting oauth client secret: %v", err)
+				return fmt.Errorf("error deleting oauth client secret: %w", err)
 			}
 		}
 	}

components/kserve/kserve.go

@@ -141,9 +141,8 @@ func (k *Kserve) ReconcileComponent(cli client.Client, owner metav1.Object, dsci
 	}
 
 	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
 		}
 	}
@@ -182,10 +181,7 @@ func (k *Kserve) removeServerlessFeatures(instance *dsciv1.DSCInitializationSpec
 		return err
 	}
 
-	if err := serverlessInitializer.Delete(); err != nil {
-		return err
-	}
-	return nil
+	return serverlessInitializer.Delete()
 }
 
 func checkRequiredOperatorsInstalled(cli client.Client) error {

components/modelmeshserving/modelmeshserving.go

@@ -130,9 +130,8 @@ func (m *ModelMeshServing) ReconcileComponent(cli client.Client, owner metav1.Ob
 		}
 	}
 	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
 		}
 	}

controllers/datasciencecluster/datasciencecluster_controller.go

@@ -58,7 +58,7 @@ import (
 )
 
 // DataScienceClusterReconciler reconciles a DataScienceCluster object.
-type DataScienceClusterReconciler struct {
+type DataScienceClusterReconciler struct { //nolint:golint,revive // Readability
 	client.Client
 	Scheme     *runtime.Scheme
 	Log        logr.Logger
@@ -69,7 +69,7 @@ type DataScienceClusterReconciler struct {
 }
 
 // DataScienceClusterConfig passing Spec of DSCI for reconcile DataScienceCluster.
-type DataScienceClusterConfig struct {
+type DataScienceClusterConfig struct { //nolint:golint,revive // Readability
 	DSCISpec *dsci.DSCInitializationSpec
 }
 
@@ -94,7 +94,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",
+				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)
@@ -294,7 +293,7 @@ func (r *DataScienceClusterReconciler) reconcileSubComponent(ctx context.Context
 		})
 
 		return instance, err
-	} else {
+	} else { //nolint:golint,revive // Readability on else
 		// reconciliation succeeded: update status accordingly
 		instance, err = r.updateStatus(ctx, instance, func(saved *dsc.DataScienceCluster) {
 			if saved.Status.InstalledComponents == nil {
@@ -407,7 +406,7 @@ func (r *DataScienceClusterReconciler) updateComponents(ctx context.Context, ori
 	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 {
@@ -421,9 +420,8 @@ func (r *DataScienceClusterReconciler) watchDataScienceClusterResources(a client
 				return []reconcile.Request{{
 					NamespacedName: types.NamespacedName{Name: instanceList.Items[0].Name},
 				}}
-			} else {
-				return nil
 			}
+			return nil
 		}
 
 		return []reconcile.Request{{

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"

controllers/dscinitialization/dscinitialization_controller.go

@@ -57,7 +57,7 @@ const (
 )
 
 // DSCInitializationReconciler reconciles a DSCInitialization object.
-type DSCInitializationReconciler struct {
+type DSCInitializationReconciler struct { //nolint:golint,revive // Readability
 	client.Client
 	Scheme                *runtime.Scheme
 	Log                   logr.Logger
@@ -72,7 +72,7 @@ type DSCInitializationReconciler struct {
 // +kubebuilder:rbac:groups="kfdef.apps.kubeflow.org",resources=kfdefs,verbs=get;list;watch;create;update;patch;delete
 
 // Reconcile contains controller logic specific to DSCInitialization instance updates.
-func (r *DSCInitializationReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { //nolint
+func (r *DSCInitializationReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { //nolint:funlen,gocyclo,maintidx
 	r.Log.Info("Reconciling DSCInitialization.", "DSCInitialization Request.Name", req.Name)
 
 	instances := &dsciv1.DSCInitializationList{}
@@ -328,17 +328,17 @@ func (r *DSCInitializationReconciler) updateStatus(ctx context.Context, original
 
 var SecretContentChangedPredicate = predicate.Funcs{
 	UpdateFunc: func(e event.UpdateEvent) bool {
-		oldSecret := e.ObjectOld.(*corev1.Secret)
-		newSecret := e.ObjectNew.(*corev1.Secret)
+		oldSecret, _ := e.ObjectOld.(*corev1.Secret)
+		newSecret, _ := e.ObjectNew.(*corev1.Secret)
 
 		return !reflect.DeepEqual(oldSecret.Data, newSecret.Data)
 	},
 }
 
 var CMContentChangedPredicate = predicate.Funcs{
 	UpdateFunc: func(e event.UpdateEvent) bool {
-		oldCM := e.ObjectOld.(*corev1.ConfigMap)
-		newCM := e.ObjectNew.(*corev1.ConfigMap)
+		oldCM, _ := e.ObjectOld.(*corev1.ConfigMap)
+		newCM, _ := e.ObjectNew.(*corev1.ConfigMap)
 
 		return !reflect.DeepEqual(oldCM.Data, newCM.Data)
 	},
@@ -351,17 +351,16 @@ var DSCDeletionPredicate = predicate.Funcs{
 	},
 }
 
-func (r *DSCInitializationReconciler) watchMonitoringConfigMapResource(a client.Object) (requests []reconcile.Request) {
+func (r *DSCInitializationReconciler) watchMonitoringConfigMapResource(a client.Object) []reconcile.Request {
 	if a.GetName() == "prometheus" && a.GetNamespace() == "redhat-ods-monitoring" {
 		r.Log.Info("Found monitoring configmap has updated, start reconcile")
 
 		return []reconcile.Request{{NamespacedName: types.NamespacedName{Name: "prometheus", Namespace: "redhat-ods-monitoring"}}}
-	} else {
-		return nil
 	}
+	return nil
 }
 
-func (r *DSCInitializationReconciler) watchMonitoringSecretResource(a client.Object) (requests []reconcile.Request) {
+func (r *DSCInitializationReconciler) watchMonitoringSecretResource(a client.Object) []reconcile.Request {
 	operatorNs, err := upgrade.GetOperatorNamespace()
 	if err != nil {
 		return nil
@@ -370,12 +369,11 @@ func (r *DSCInitializationReconciler) watchMonitoringSecretResource(a client.Obj
 		r.Log.Info("Found monitoring secret has updated, start reconcile")
 
 		return []reconcile.Request{{NamespacedName: types.NamespacedName{Name: "addon-managed-odh-parameters", Namespace: operatorNs}}}
-	} else {
-		return nil
 	}
+	return nil
 }
 
-func (r *DSCInitializationReconciler) watchDSCResource(_ client.Object) (requests []reconcile.Request) {
+func (r *DSCInitializationReconciler) watchDSCResource(_ client.Object) []reconcile.Request {
 	instanceList := &dscv1.DataScienceClusterList{}
 	if err := r.Client.List(context.TODO(), instanceList); err != nil {
 		// do not handle if cannot get list

controllers/dscinitialization/dscinitialization_test.go

@@ -111,7 +111,7 @@ var _ = Describe("DataScienceCluster initialization", func() {
 			Eventually(dscInitializationIsReady(applicationName, workingNamespace, foundDsci), timeout, interval).Should(BeTrue())
 			// then
 			foundMonitoringNamespace := &corev1.Namespace{}
-			Eventually(Eventually(namespaceExists(monitoringNamespace2, foundMonitoringNamespace), timeout, interval).Should(BeTrue()), timeout, interval).Should(BeTrue())
+			Eventually(namespaceExists(monitoringNamespace2, foundMonitoringNamespace), timeout, interval).Should(BeTrue())
 			Expect(foundMonitoringNamespace.Name).Should(Equal(monitoringNamespace2))
 		})
 	})
@@ -228,7 +228,6 @@ var _ = Describe("DataScienceCluster initialization", func() {
 	})
 })
 
-// cleanup utility func.
 func cleanupResources() {
 	defaultNamespace := client.InNamespace(workingNamespace)
 	appNamespace := client.InNamespace(applicationNamespace)
@@ -263,7 +262,7 @@ func namespaceExists(ns string, obj client.Object) func() bool {
 	}
 }
 
-func objectExists(ns string, name string, obj client.Object) func() bool { //nolint
+func objectExists(ns string, name string, obj client.Object) func() bool { //nolint:unparam
 	return func() bool {
 		err := k8sClient.Get(context.Background(), client.ObjectKey{Name: ns, Namespace: name}, obj)
 
@@ -291,7 +290,7 @@ func createDSCI(appName string, enableMonitoring operatorv1.ManagementState, mon
 	}
 }
 
-func dscInitializationIsReady(ns string, name string, dsciObj *dsci.DSCInitialization) func() bool { //nolint
+func dscInitializationIsReady(ns string, name string, dsciObj *dsci.DSCInitialization) func() bool { //nolint:unparam
 	return func() bool {
 		_ = k8sClient.Get(context.Background(), client.ObjectKey{Name: ns, Namespace: name}, dsciObj)