New lint rules, mostly for import organization

.golangci.yml

@@ -18,21 +18,37 @@ issues:
     - text: "Unhandled error in call to function fmt.Print*"
       linters:
         - revive
+    - path: cmd/main.go
+      linters:
+        - maintidx
+    - path: test/
+      linters:
+        - perfsprint
 linters:
   disable-all: true
   enable:
     - asciicheck
     - bodyclose
+    - canonicalheader
     - containedctx
     - contextcheck
     - copyloopvar
     - decorder
     - dogsled
     - dupl
+    - dupword
     - durationcheck
     - errcheck
+    - errchkjson
+    - errname
+    - forbidigo
     - forcetypeassert
+    - gci
+    - ginkgolinter
+    - gocheckcompilerdirectives
+    - gochecksumtype
     - goconst
+    - gocritic
     - gocyclo
     - gofmt
     - gofumpt
@@ -41,20 +57,30 @@ linters:
     - gosimple
     - govet
     - ineffassign
+    - interfacebloat
+    - intrange
+    - loggercheck
+    - maintidx
     - misspell
+    - musttag
     - nakedret
+    - nilerr
+    - nilnil
     - noctx
     - nolintlint
     - nosprintfhostport
+    - paralleltest
+    - perfsprint
     - prealloc
     - predeclared
     - promlinter
     - revive
     - staticcheck
     - stylecheck
     - tenv
+    - testifylint
     - thelper
-    - typecheck
+    - tparallel
     - unconvert
     - unparam
     - unused
@@ -67,15 +93,33 @@ linters-settings:
     # Tokens count to trigger issue.
     # Default: 150
     threshold: 200
+  gci:
+    sections:
+      - standard # Standard section: captures all standard packages.
+      - default # Default section: contains all imports that could not be matched to another section type.
+      - prefix(github.com/Mirantis/hmc) # Custom section: groups all imports with the specified Prefix.
+    skip-generated: false
   gofmt:
     # Apply the rewrite rules to the source before reformatting.
     # https://pkg.go.dev/cmd/gofmt
     # Default: []
     rewrite-rules:
       - pattern: "interface{}"
         replacement: "any"
-  stylecheck:
-    checks: ["all", "-ST1000", "-ST1001", "-ST1021"]
+  gofumpt:
+    extra-rules: true
+  govet:
+    enable-all: true
+    disable:
+      - fieldalignment
+      - shadow
+  loggercheck:
+    kitlog: false
+    klog: false
+    require-string-key: true
+    no-printf-like: true
+  paralleltest:
+    ignore-missing: true
   revive:
     enable-all-rules: true
     rules:
@@ -96,4 +140,6 @@ linters-settings:
       - name: line-length-limit
         disabled: true
       - name: package-comments
-        disabled: true
+        disabled: true
+  stylecheck:
+    checks: ["all", "-ST1000", "-ST1001", "-ST1021"]

cmd/main.go

@@ -19,11 +19,9 @@ import (
 	"flag"
 	"os"
 
-	// Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.)
-	// to ensure that exec-entrypoint and run can make use of them.
-
 	hcv2 "github.com/fluxcd/helm-controller/api/v2"
 	sourcev1 "github.com/fluxcd/source-controller/api/v1"
+	sveltosv1beta1 "github.com/projectsveltos/addon-controller/api/v1beta1"
 	"k8s.io/apimachinery/pkg/runtime"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	"k8s.io/client-go/dynamic"
@@ -37,15 +35,12 @@ import (
 	metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
 
-	sveltosv1beta1 "github.com/projectsveltos/addon-controller/api/v1beta1"
-
 	hmcmirantiscomv1alpha1 "github.com/Mirantis/hmc/api/v1alpha1"
 	"github.com/Mirantis/hmc/internal/controller"
 	"github.com/Mirantis/hmc/internal/helm"
 	"github.com/Mirantis/hmc/internal/telemetry"
 	"github.com/Mirantis/hmc/internal/utils"
 	hmcwebhook "github.com/Mirantis/hmc/internal/webhook"
-	// +kubebuilder:scaffold:imports
 )
 
 var (

internal/controller/managedcluster_controller.go

@@ -169,8 +169,7 @@ func (r *ManagedClusterReconciler) setStatusFromClusterStatus(ctx context.Contex
 func (r *ManagedClusterReconciler) Update(ctx context.Context, managedCluster *hmc.ManagedCluster) (result ctrl.Result, err error) {
 	l := ctrl.LoggerFrom(ctx)
 
-	finalizersUpdated := controllerutil.AddFinalizer(managedCluster, hmc.ManagedClusterFinalizer)
-	if finalizersUpdated {
+	if controllerutil.AddFinalizer(managedCluster, hmc.ManagedClusterFinalizer) {
 		if err := r.Client.Update(ctx, managedCluster); err != nil {
 			return ctrl.Result{}, fmt.Errorf("failed to update managedCluster %s/%s: %w", managedCluster.Namespace, managedCluster.Name, err)
 		}
@@ -460,7 +459,7 @@ func (r *ManagedClusterReconciler) updateStatus(ctx context.Context, managedClus
 
 func (r *ManagedClusterReconciler) getSource(ctx context.Context, ref *hcv2.CrossNamespaceSourceReference) (sourcev1.Source, error) {
 	if ref == nil {
-		return nil, fmt.Errorf("helm chart source is not provided")
+		return nil, errors.New("helm chart source is not provided")
 	}
 	chartRef := client.ObjectKey{Namespace: ref.Namespace, Name: ref.Name}
 	hc := sourcev1.HelmChart{}
@@ -481,8 +480,7 @@ func (r *ManagedClusterReconciler) Delete(ctx context.Context, managedCluster *h
 	if err != nil {
 		if apierrors.IsNotFound(err) {
 			l.Info("Removing Finalizer", "finalizer", hmc.ManagedClusterFinalizer)
-			finalizersUpdated := controllerutil.RemoveFinalizer(managedCluster, hmc.ManagedClusterFinalizer)
-			if finalizersUpdated {
+			if controllerutil.RemoveFinalizer(managedCluster, hmc.ManagedClusterFinalizer) {
 				if err := r.Client.Update(ctx, managedCluster); err != nil {
 					return ctrl.Result{}, fmt.Errorf("failed to update managedCluster %s/%s: %w", managedCluster.Namespace, managedCluster.Name, err)
 				}
@@ -618,8 +616,7 @@ func (r *ManagedClusterReconciler) getCluster(ctx context.Context, namespace, na
 
 func (r *ManagedClusterReconciler) removeClusterFinalizer(ctx context.Context, cluster *metav1.PartialObjectMetadata) error {
 	originalCluster := *cluster
-	finalizersUpdated := controllerutil.RemoveFinalizer(cluster, hmc.BlockingFinalizer)
-	if finalizersUpdated {
+	if controllerutil.RemoveFinalizer(cluster, hmc.BlockingFinalizer) {
 		ctrl.LoggerFrom(ctx).Info("Allow to stop cluster", "finalizer", hmc.BlockingFinalizer)
 		if err := r.Client.Patch(ctx, cluster, client.MergeFrom(&originalCluster)); err != nil {
 			return fmt.Errorf("failed to patch cluster %s/%s: %w", cluster.Namespace, cluster.Name, err)
@@ -654,7 +651,7 @@ func (r *ManagedClusterReconciler) reconcileCredentialPropagation(ctx context.Co
 
 	kubeconfSecret := &corev1.Secret{}
 	if err := r.Client.Get(ctx, client.ObjectKey{
-		Name:      fmt.Sprintf("%s-kubeconfig", managedCluster.Name),
+		Name:      managedCluster.Name + "-kubeconfig",
 		Namespace: managedCluster.Namespace,
 	}, kubeconfSecret); err != nil {
 		return fmt.Errorf("failed to get kubeconfig secret for cluster %s/%s: %s", managedCluster.Namespace, managedCluster.Name, err)
@@ -708,7 +705,7 @@ func (r *ManagedClusterReconciler) reconcileCredentialPropagation(ctx context.Co
 				Type:    hmc.CredentialsPropagatedCondition,
 				Status:  metav1.ConditionFalse,
 				Reason:  hmc.FailedReason,
-				Message: fmt.Sprintf("unsupported infrastructure provider %s", provider),
+				Message: "unsupported infrastructure provider " + provider,
 			})
 		}
 	}
@@ -845,8 +842,8 @@ func (r *ManagedClusterReconciler) propagateVSphereSecrets(ctx context.Context,
 func generateVSphereCCMConfigs(vCl *capv.VSphereCluster, vScrt *corev1.Secret, vMa *capv.VSphereMachine) (*corev1.Secret, *corev1.ConfigMap, error) {
 	secretName := "vsphere-cloud-secret"
 	secretData := map[string][]byte{
-		fmt.Sprintf("%s.username", vCl.Spec.Server): vScrt.Data["username"],
-		fmt.Sprintf("%s.password", vCl.Spec.Server): vScrt.Data["password"],
+		vCl.Spec.Server + ".username": vScrt.Data["username"],
+		vCl.Spec.Server + ".password": vScrt.Data["password"],
 	}
 	ccmCfg := map[string]any{
 		"global": map[string]any{

internal/controller/managedcluster_controller_test.go

@@ -166,7 +166,7 @@ var _ = Describe("ManagedCluster Controller", func() {
 			_, err := controllerReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: typeNamespacedName})
 			Expect(err).NotTo(HaveOccurred())
 
-			Eventually(k8sClient.Get(ctx, typeNamespacedName, managedCluster), 1*time.Minute, 5*time.Second).Should(HaveOccurred())
+			Eventually(k8sClient.Get, 1*time.Minute, 5*time.Second).WithArguments(ctx, typeNamespacedName, managedCluster).Should(HaveOccurred())
 
 			Expect(k8sClient.Delete(ctx, template)).To(Succeed())
 			Expect(k8sClient.Delete(ctx, management)).To(Succeed())

internal/controller/management_controller.go

@@ -85,8 +85,7 @@ func (r *ManagementReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 func (r *ManagementReconciler) Update(ctx context.Context, management *hmc.Management) (ctrl.Result, error) {
 	l := ctrl.LoggerFrom(ctx)
 
-	finalizersUpdated := controllerutil.AddFinalizer(management, hmc.ManagementFinalizer)
-	if finalizersUpdated {
+	if controllerutil.AddFinalizer(management, hmc.ManagementFinalizer) {
 		if err := r.Client.Update(ctx, management); err != nil {
 			l.Error(err, "Failed to update Management finalizers")
 			return ctrl.Result{}, err

internal/controller/management_controller_test.go

@@ -20,11 +20,10 @@ import (
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	"k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-
 	hmcmirantiscomv1alpha1 "github.com/Mirantis/hmc/api/v1alpha1"
 )
 

internal/controller/multiclusterservice_controller.go

@@ -18,17 +18,16 @@ import (
 	"context"
 	"fmt"
 
+	sourcev1 "github.com/fluxcd/source-controller/api/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/types"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 
 	hmc "github.com/Mirantis/hmc/api/v1alpha1"
 	"github.com/Mirantis/hmc/internal/sveltos"
 	"github.com/Mirantis/hmc/internal/utils"
-	sourcev1 "github.com/fluxcd/source-controller/api/v1"
 )
 
 // MultiClusterServiceReconciler reconciles a MultiClusterService object
@@ -111,7 +110,7 @@ func helmChartOpts(ctx context.Context, c client.Client, namespace string, servi
 		// because if the services slice is part of:
 		// 1. ManagedCluster: Then the referred template must be in its own namespace.
 		// 2. MultiClusterService: Then the referred template must be in hmc-system namespace.
-		tmplRef := types.NamespacedName{Name: svc.Template, Namespace: namespace}
+		tmplRef := client.ObjectKey{Name: svc.Template, Namespace: namespace}
 		if err := c.Get(ctx, tmplRef, tmpl); err != nil {
 			return nil, fmt.Errorf("failed to get ServiceTemplate %s: %w", tmplRef.String(), err)
 		}
@@ -121,7 +120,7 @@ func helmChartOpts(ctx context.Context, c client.Client, namespace string, servi
 		}
 
 		chart := &sourcev1.HelmChart{}
-		chartRef := types.NamespacedName{
+		chartRef := client.ObjectKey{
 			Namespace: tmpl.GetCommonStatus().ChartRef.Namespace,
 			Name:      tmpl.GetCommonStatus().ChartRef.Name,
 		}
@@ -130,7 +129,7 @@ func helmChartOpts(ctx context.Context, c client.Client, namespace string, servi
 		}
 
 		repo := &sourcev1.HelmRepository{}
-		repoRef := types.NamespacedName{
+		repoRef := client.ObjectKey{
 			// Using chart's namespace because it's source
 			// should be within the same namespace.
 			Namespace: chart.Namespace,

internal/controller/multiclusterservice_controller_test.go

@@ -22,17 +22,16 @@ import (
 	sourcev1 "github.com/fluxcd/source-controller/api/v1"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	sveltosv1beta1 "github.com/projectsveltos/addon-controller/api/v1beta1"
 	"helm.sh/helm/v3/pkg/chart"
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-
 	hmc "github.com/Mirantis/hmc/api/v1alpha1"
 	"github.com/Mirantis/hmc/internal/utils"
-	sveltosv1beta1 "github.com/projectsveltos/addon-controller/api/v1beta1"
 )
 
 var _ = Describe("MultiClusterService Controller", func() {
@@ -187,7 +186,7 @@ var _ = Describe("MultiClusterService Controller", func() {
 			// Running reconcile to remove the finalizer and delete the MultiClusterService
 			_, err := reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: multiClusterServiceRef})
 			Expect(err).NotTo(HaveOccurred())
-			Eventually(k8sClient.Get(ctx, multiClusterServiceRef, multiClusterService), 1*time.Minute, 5*time.Second).Should(HaveOccurred())
+			Eventually(k8sClient.Get, 1*time.Minute, 5*time.Second).WithArguments(ctx, multiClusterServiceRef, multiClusterService).Should(HaveOccurred())
 
 			Expect(k8sClient.Get(ctx, clusterProfileRef, &sveltosv1beta1.ClusterProfile{})).To(HaveOccurred())
 
@@ -220,7 +219,7 @@ var _ = Describe("MultiClusterService Controller", func() {
 			_, err = multiClusterServiceReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: multiClusterServiceRef})
 			Expect(err).NotTo(HaveOccurred())
 
-			Eventually(k8sClient.Get(ctx, clusterProfileRef, clusterProfile), 1*time.Minute, 5*time.Second).ShouldNot(HaveOccurred())
+			Eventually(k8sClient.Get, 1*time.Minute, 5*time.Second).WithArguments(ctx, clusterProfileRef, clusterProfile).ShouldNot(HaveOccurred())
 		})
 	})
 })

internal/controller/suite_test.go

@@ -26,27 +26,25 @@ import (
 	"testing"
 	"time"
 
+	helmcontrollerv2 "github.com/fluxcd/helm-controller/api/v2"
+	sourcev1 "github.com/fluxcd/source-controller/api/v1"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	sveltosv1beta1 "github.com/projectsveltos/addon-controller/api/v1beta1"
 	admissionv1 "k8s.io/api/admissionregistration/v1"
-	utilyaml "sigs.k8s.io/cluster-api/util/yaml"
-	ctrl "sigs.k8s.io/controller-runtime"
-	metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
-	"sigs.k8s.io/controller-runtime/pkg/webhook"
-
-	helmcontrollerv2 "github.com/fluxcd/helm-controller/api/v2"
-	sourcev1 "github.com/fluxcd/source-controller/api/v1"
 	"k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/rest"
+	utilyaml "sigs.k8s.io/cluster-api/util/yaml"
+	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/envtest"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
+	metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
+	"sigs.k8s.io/controller-runtime/pkg/webhook"
 
 	hmcmirantiscomv1alpha1 "github.com/Mirantis/hmc/api/v1alpha1"
 	hmcwebhook "github.com/Mirantis/hmc/internal/webhook"
-	sveltosv1beta1 "github.com/projectsveltos/addon-controller/api/v1beta1"
-	// +kubebuilder:scaffold:imports
 )
 
 // These tests use Ginkgo (BDD-style Go testing framework). Refer to

internal/controller/template_controller.go

@@ -157,7 +157,7 @@ func (r *TemplateReconciler) ReconcileTemplate(ctx context.Context, template tem
 		}
 	} else {
 		if helmSpec.ChartName == "" {
-			err := fmt.Errorf("neither chartName nor chartRef is set")
+			err := errors.New("neither chartName nor chartRef is set")
 			l.Error(err, "invalid helm chart reference")
 			return ctrl.Result{}, err
 		}
@@ -180,7 +180,7 @@ func (r *TemplateReconciler) ReconcileTemplate(ctx context.Context, template tem
 		}
 	}
 	if hcChart == nil {
-		err := fmt.Errorf("HelmChart is nil")
+		err := errors.New("HelmChart is nil")
 		l.Error(err, "could not get the helm chart")
 		return ctrl.Result{}, err
 	}
@@ -249,7 +249,7 @@ func templateManagedByHMC(template templateCommon) bool {
 
 func fillStatusWithProviders(template templateCommon, helmChart *chart.Chart) error {
 	if helmChart.Metadata == nil {
-		return fmt.Errorf("chart metadata is empty")
+		return errors.New("chart metadata is empty")
 	}
 
 	return template.FillStatusWithProviders(helmChart.Metadata.Annotations)

internal/controller/templatechain_controller.go

@@ -137,7 +137,7 @@ func (r *TemplateChainReconciler) ReconcileTemplateChain(ctx context.Context, te
 		}
 
 		if err := r.Create(ctx, target); err == nil {
-			l.Info(fmt.Sprintf("%s was successfully created", templateChain.TemplateKind()), "template namespace", templateChain.GetNamespace(), "template name", supportedTemplate.Name)
+			l.Info(templateChain.TemplateKind()+" was successfully created", "template namespace", templateChain.GetNamespace(), "template name", supportedTemplate.Name)
 			continue
 		}