chore: Linter configuration: gofumpt, goerr113, godox, goconst (#341)

* Linter: gofumpt

* Linter: goerr113

* Linter: goconst

* Linter: godox

* golangci/golangci-lint-action@v3

* Refine linters

.github/workflows/codequality.yml

@@ -31,7 +31,7 @@ jobs:
           cache: false
       - uses: actions/checkout@v4
       - name: golangci-lint
-        uses: golangci/golangci-lint-action@v4
+        uses: golangci/golangci-lint-action@v3
         with:
           # Optional: version of golangci-lint to use in form of v1.2 or v1.2.3 or `latest` to use the latest version
           version: v1.55

.gitignore

@@ -44,6 +44,5 @@ module-config.yaml
 .env.dev
 
 
-.golangci.yaml
-.golangci.yaml_old
+.golangci.*
 lint-report.json

.golangci.yaml

@@ -36,8 +36,6 @@ linters:
     - tagalign
     - dupword
     - gomoddirectives
-    - gofumpt
-    - goerr113
     - funlen
     - testifylint
     - bodyclose
@@ -47,8 +45,6 @@ linters:
     - errchkjson
     - perfsprint
     - noctx
-    - godox
-    - goconst
 
 linters-settings:
   gomoddirectives:

api/v1alpha1/nats_status.go

@@ -28,8 +28,11 @@ func (ns *NATSStatus) FindCondition(conditionType ConditionType) *kmetav1.Condit
 	return nil
 }
 
-func (ns *NATSStatus) UpdateConditionStatefulSet(status kmetav1.ConditionStatus, reason ConditionReason,
-	message string) {
+func (ns *NATSStatus) UpdateConditionStatefulSet(
+	status kmetav1.ConditionStatus,
+	reason ConditionReason,
+	message string,
+) {
 	condition := kmetav1.Condition{
 		Type:               string(ConditionStatefulSet),
 		Status:             status,
@@ -40,8 +43,11 @@ func (ns *NATSStatus) UpdateConditionStatefulSet(status kmetav1.ConditionStatus,
 	meta.SetStatusCondition(&ns.Conditions, condition)
 }
 
-func (ns *NATSStatus) UpdateConditionAvailable(status kmetav1.ConditionStatus, reason ConditionReason,
-	message string) {
+func (ns *NATSStatus) UpdateConditionAvailable(
+	status kmetav1.ConditionStatus,
+	reason ConditionReason,
+	message string,
+) {
 	condition := kmetav1.Condition{
 		Type:               string(ConditionAvailable),
 		Status:             status,
@@ -84,8 +90,11 @@ func (ns *NATSStatus) SetStateDeleting() {
 	ns.State = StateDeleting
 }
 
-func (ns *NATSStatus) UpdateConditionDeletion(status kmetav1.ConditionStatus, reason ConditionReason,
-	message string) {
+func (ns *NATSStatus) UpdateConditionDeletion(
+	status kmetav1.ConditionStatus,
+	reason ConditionReason,
+	message string,
+) {
 	condition := kmetav1.Condition{
 		Type:               string(ConditionDeleted),
 		Status:             status,

internal/controller/nats/deprovisioner.go

@@ -23,7 +23,8 @@ const (
 )
 
 func (r *Reconciler) handleNATSDeletion(ctx context.Context, nats *nmapiv1alpha1.NATS,
-	log *zap.SugaredLogger) (kcontrollerruntime.Result, error) {
+	log *zap.SugaredLogger,
+) (kcontrollerruntime.Result, error) {
 	// skip reconciliation for deletion if the finalizer is not set.
 	if !r.containsFinalizer(nats) {
 		log.Debugf("skipped reconciliation for deletion as finalizer is not set.")
@@ -116,8 +117,10 @@ func (r *Reconciler) createAndConnectNatsClient(nats *nmapiv1alpha1.NATS) error
 }
 
 func (r *Reconciler) deletePVCsAndRemoveFinalizer(ctx context.Context,
-	nats *nmapiv1alpha1.NATS, log *zap.SugaredLogger) (kcontrollerruntime.Result, error) {
+	nats *nmapiv1alpha1.NATS, log *zap.SugaredLogger,
+) (kcontrollerruntime.Result, error) {
 	labelValue := nats.Name
+	//nolint: godox
 	// TODO: delete the following logic after migrating to modular Kyma
 	if nats.Name == "eventing-nats" {
 		labelValue = "eventing"

internal/controller/nats/deprovisioner_test.go

@@ -26,6 +26,13 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 )
 
+var (
+	ErrConnectionErrorMsg = errors.New("connection cannot be established")
+	ErrUnexpectedErrorMsg = errors.New("unexpected error")
+	ErrInitErrorMsg       = errors.New("init error")
+	ErrDeleteErrorMsg     = errors.New("delete error")
+)
+
 func Test_handleNATSDeletion(t *testing.T) {
 	t.Parallel()
 
@@ -54,7 +61,7 @@ func Test_handleNATSDeletion(t *testing.T) {
 			givenWithNATSCreated: true,
 			mockNatsClientFunc: func() nmnats.Client {
 				natsClient := new(mocks.Client)
-				natsClient.On("Init").Return(errors.New("connection cannot be established"))
+				natsClient.On("Init").Return(ErrConnectionErrorMsg)
 				natsClient.On("Close").Return()
 				return natsClient
 			},
@@ -69,7 +76,7 @@ func Test_handleNATSDeletion(t *testing.T) {
 			mockNatsClientFunc: func() nmnats.Client {
 				natsClient := new(mocks.Client)
 				natsClient.On("Init").Return(nil)
-				natsClient.On("GetStreams").Return(nil, errors.New("unexpected error"))
+				natsClient.On("GetStreams").Return(nil, ErrUnexpectedErrorMsg)
 				natsClient.On("Close").Return()
 				return natsClient
 			},
@@ -90,7 +97,7 @@ func Test_handleNATSDeletion(t *testing.T) {
 						},
 					},
 				}, nil)
-				natsClient.On("ConsumersExist", mock.Anything).Return(false, errors.New("unexpected error"))
+				natsClient.On("ConsumersExist", mock.Anything).Return(false, ErrUnexpectedErrorMsg)
 				natsClient.On("Close").Return()
 				return natsClient
 			},
@@ -311,9 +318,9 @@ func Test_DeletePVCsAndRemoveFinalizer(t *testing.T) {
 				testutils.WithNATSCRFinalizer(NATSFinalizerName),
 			),
 			labelValue:     "test-nats",
-			deleteErr:      errors.New("delete error"),
+			deleteErr:      ErrDeleteErrorMsg,
 			expectedResult: kcontrollerruntime.Result{},
-			expectedErr:    errors.New("delete error"),
+			expectedErr:    ErrDeleteErrorMsg,
 		},
 	}
 
@@ -370,8 +377,8 @@ func Test_CreateAndConnectNatsClient(t *testing.T) {
 				testutils.WithNATSCRName("test-nats"),
 				testutils.WithNATSCRNamespace("test-namespace"),
 			),
-			initErr:     errors.New("init error"),
-			expectedErr: errors.New("init error"),
+			initErr:     ErrInitErrorMsg,
+			expectedErr: ErrInitErrorMsg,
 		},
 	}
 
@@ -383,7 +390,6 @@ func Test_CreateAndConnectNatsClient(t *testing.T) {
 			r.getNatsClient(tt.nats).(*mocks.Client).On("Init").Return(tt.initErr)
 
 			err := r.createAndConnectNatsClient(tt.nats)
-
 			if err != nil {
 				require.Equal(t, tt.expectedErr.Error(), err.Error())
 			}

internal/controller/nats/integrationtests/controller/integration_test.go

@@ -630,7 +630,8 @@ func makeStatefulSetReady(t *testing.T, name, namespace string) {
 type deletionFunc func(env integration.TestEnvironment, natsName, namespace string) error
 
 func ensureK8sResourceDeletion(
-	t *testing.T, env integration.TestEnvironment, natsName, namespace string, fs ...deletionFunc) {
+	t *testing.T, env integration.TestEnvironment, natsName, namespace string, fs ...deletionFunc,
+) {
 	for _, f := range fs {
 		require.NoError(t, f(env, natsName, namespace))
 	}

internal/controller/nats/provisioner.go

@@ -17,7 +17,8 @@ import (
 const RequeueTimeForStatusCheck = 10
 
 func (r *Reconciler) handleNATSReconcile(ctx context.Context,
-	nats *nmapiv1alpha1.NATS, log *zap.SugaredLogger) (kcontrollerruntime.Result, error) {
+	nats *nmapiv1alpha1.NATS, log *zap.SugaredLogger,
+) (kcontrollerruntime.Result, error) {
 	log.Info("handling NATS reconciliation...")
 
 	// set status to processing
@@ -66,7 +67,8 @@ func (r *Reconciler) handleNATSReconcile(ctx context.Context,
 // handleNATSState checks if NATS resources are ready.
 // It also syncs the NATS CR status.
 func (r *Reconciler) handleNATSState(ctx context.Context, nats *nmapiv1alpha1.NATS, instance *chart.ReleaseInstance,
-	log *zap.SugaredLogger) (kcontrollerruntime.Result, error) {
+	log *zap.SugaredLogger,
+) (kcontrollerruntime.Result, error) {
 	// Clear the url until the StatefulSet is ready.
 	nats.Status.ClearURL()
 

internal/controller/nats/provisioner_test.go

@@ -15,6 +15,8 @@ import (
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 )
 
+var ErrDeployErrorMsg = errors.New("deploy error")
+
 func Test_handleNATSState(t *testing.T) {
 	t.Parallel()
 
@@ -158,7 +160,7 @@ func Test_handleNATSReconcile(t *testing.T) {
 				testutils.WithNATSCRNamespace("kyma-system"),
 				testutils.WithNATSCRFinalizer(NATSFinalizerName),
 			),
-			givenDeployError: errors.New("deploy error"),
+			givenDeployError: ErrDeployErrorMsg,
 			wantState:        nmapiv1alpha1.StateError,
 			wantConditions: []kmetav1.Condition{
 				{

internal/controller/nats/status.go

@@ -19,7 +19,8 @@ import (
 // syncNATSStatus syncs NATS status.
 // Returns the relevant error.
 func (r *Reconciler) syncNATSStatusWithErr(ctx context.Context,
-	nats *nmapiv1alpha1.NATS, err error, log *zap.SugaredLogger) error {
+	nats *nmapiv1alpha1.NATS, err error, log *zap.SugaredLogger,
+) error {
 	// set error state in status
 	nats.Status.SetStateError()
 	nats.Status.UpdateConditionAvailable(kmetav1.ConditionFalse, nmapiv1alpha1.ConditionReasonProcessingError, err.Error())
@@ -30,7 +31,8 @@ func (r *Reconciler) syncNATSStatusWithErr(ctx context.Context,
 
 // syncNATSStatus syncs NATS status.
 func (r *Reconciler) syncNATSStatus(ctx context.Context,
-	nats *nmapiv1alpha1.NATS, log *zap.SugaredLogger) error {
+	nats *nmapiv1alpha1.NATS, log *zap.SugaredLogger,
+) error {
 	namespacedName := &ktypes.NamespacedName{
 		Name:      nats.Name,
 		Namespace: nats.Namespace,
@@ -52,7 +54,8 @@ func (r *Reconciler) syncNATSStatus(ctx context.Context,
 
 // updateStatus updates the status to k8s if modified.
 func (r *Reconciler) updateStatus(ctx context.Context, oldNATS, newNATS *nmapiv1alpha1.NATS,
-	logger *zap.SugaredLogger) error {
+	logger *zap.SugaredLogger,
+) error {
 	// compare the status taking into consideration lastTransitionTime in conditions
 	if oldNATS.Status.IsEqual(newNATS.Status) {
 		return nil

internal/controller/nats/status_test.go

@@ -16,6 +16,8 @@ import (
 	kmetav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
+var ErrTestErrorMsg = errors.New("test error")
+
 func Test_syncNATSStatus(t *testing.T) {
 	t.Parallel()
 
@@ -97,7 +99,7 @@ func Test_syncNATSStatusWithErr(t *testing.T) {
 				testutils.WithNATSCRStatusInitialized(),
 				testutils.WithNATSStateProcessing(),
 			),
-			givenError: errors.New("test error"),
+			givenError: ErrTestErrorMsg,
 			wantNATSStatus: nmapiv1alpha1.NATSStatus{
 				State: nmapiv1alpha1.StateError,
 				Conditions: []kmetav1.Condition{

pkg/events/events.go

@@ -9,12 +9,14 @@ import (
 
 // Normal records a normal event for an API object.
 func Normal(recorder record.EventRecorder, obj runtime.Object, rn nmapiv1alpha1.ConditionReason, msgFmt string,
-	args ...interface{}) {
+	args ...interface{},
+) {
 	recorder.Eventf(obj, kcorev1.EventTypeNormal, string(rn), msgFmt, args...)
 }
 
 // Warn records a warning event for an API object.
 func Warn(recorder record.EventRecorder, obj runtime.Object, rn nmapiv1alpha1.ConditionReason, msgFmt string,
-	args ...interface{}) {
+	args ...interface{},
+) {
 	recorder.Eventf(obj, kcorev1.EventTypeWarning, string(rn), msgFmt, args...)
 }

pkg/k8s/chart/helmrenderer.go

@@ -24,7 +24,7 @@ type HelmRenderer struct {
 
 func NewHelmRenderer(chartPath string, logger *zap.SugaredLogger) (Renderer, error) {
 	if !file.DirExists(chartPath) {
-		return nil, fmt.Errorf("HELM chart directory '%s' not found", chartPath)
+		return nil, fmt.Errorf("HELM chart directory '%s' not found", chartPath) //nolint: goerr113 // reason: This is the only place where we use dynamic error message
 	}
 
 	// load chart into memory

pkg/k8s/chart/releaseinstance.go

@@ -9,6 +9,8 @@ import (
 	"dario.cat/mergo"
 )
 
+var ErrFailedToConvertToNestedMap = errors.New("failed to convert to nestedMap to map[string]interface{}")
+
 func NewReleaseInstance(name, namespace string, istioEnabled bool,
 	configuration map[string]interface{},
 ) *ReleaseInstance {
@@ -71,7 +73,7 @@ func (c *ReleaseInstance) convertToNestedMap(key string, value interface{}) (map
 			var ok bool
 			lastNestedMap, ok = lastNestedMap[token].(map[string]interface{})
 			if !ok {
-				return result, errors.New("failed to convert to nestedMap to map[string]interface{}")
+				return result, ErrFailedToConvertToNestedMap
 			}
 		}
 	}

pkg/k8s/client.go

@@ -93,7 +93,8 @@ func (c *KubeClient) DestinationRuleCRDExists(ctx context.Context) (bool, error)
 }
 
 func (c *KubeClient) DeletePVCsWithLabel(ctx context.Context, labelSelector string,
-	mustHaveNamePrefix, namespace string) error {
+	mustHaveNamePrefix, namespace string,
+) error {
 	// create a new labels.Selector object for the label selector
 	selector, err := labels.Parse(labelSelector)
 	if err != nil {

pkg/manager/nats.go

@@ -11,6 +11,8 @@ import (
 	"github.com/kyma-project/nats-manager/pkg/k8s/chart"
 )
 
+var ErrNATSStatefulSetNotFound = fmt.Errorf("NATS StatefulSet not found in manifests")
+
 type NatsConfig struct {
 	ClusterSize int
 }
@@ -42,7 +44,8 @@ func NewNATSManger(kubeClient k8s.Client, chartRenderer chart.Renderer, logger *
 }
 
 func (m NATSManager) GenerateNATSResources(instance *chart.ReleaseInstance,
-	opts ...Option) (*chart.ManifestResources, error) {
+	opts ...Option,
+) (*chart.ManifestResources, error) {
 	manifests, err := m.chartRenderer.RenderManifestAsUnstructured(instance)
 	if err == nil {
 		// apply options
@@ -80,7 +83,7 @@ func (m NATSManager) IsNATSStatefulSetReady(ctx context.Context, instance *chart
 	// get statefulSets from rendered manifests
 	statefulSets := instance.GetStatefulSets()
 	if len(statefulSets) == 0 {
-		return false, fmt.Errorf("NATS StatefulSet not found in manifests")
+		return false, ErrNATSStatefulSetNotFound
 	}
 
 	// fetch statefulSets from cluster and check if they are ready

pkg/manager/nats_test.go

@@ -16,6 +16,12 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 )
 
+var (
+	ErrNATSStatefulSetNotFoundMsg = errors.New("NATS StatefulSet not found in manifests")
+	ErrFailedToDeployMsg          = errors.New("failed to deploy")
+	ErrFailedToDeleteMsg          = errors.New("failed to delete")
+)
+
 func Test_GenerateNATSResources(t *testing.T) {
 	t.Parallel()
 
@@ -106,7 +112,7 @@ func Test_DeployInstance(t *testing.T) {
 		},
 		{
 			name:      "should fail when k8s fails to deploy resource",
-			wantError: errors.New("failed to deploy"),
+			wantError: ErrFailedToDeployMsg,
 		},
 	}
 
@@ -171,7 +177,7 @@ func Test_DeleteInstance(t *testing.T) {
 		},
 		{
 			name:      "should fail when k8s fails to delete resource",
-			wantError: errors.New("failed to delete"),
+			wantError: ErrFailedToDeleteMsg,
 		},
 	}
 
@@ -235,7 +241,7 @@ func Test_IsNATSStatefulSetReady(t *testing.T) {
 		{
 			name:             "should return error if no StatefulSet exists in manifests",
 			givenStatefulSet: nil,
-			wantError:        errors.New("NATS StatefulSet not found in manifests"),
+			wantError:        ErrNATSStatefulSetNotFoundMsg,
 		},
 		{
 			name: "should return not ready when CurrentReplicas is not as needed",