Update linter (#2725)

.ci/Dockerfile

@@ -1,7 +1,8 @@
 # Docker image used for continuous integration
 FROM golang:1.13-stretch
 
-ENV GOLANGCILINT_VERSION=1.21.0
+# TODO: update to >1.24.0 once https://github.com/golangci/golangci-lint/issues/994 is fixed, as jenkins OOMs when running 1.24.0
+ENV GOLANGCILINT_VERSION=1.23.8
 ENV KUBEBUILDER_VERSION=2.0.0
 ENV GCLOUD_VERSION=277.0.0
 ENV KUBECTL_VERSION=1.14.7

.golangci.yml

@@ -14,6 +14,7 @@ linters:
     - gochecknoinits
     - gocognit # added in 1.20
     - godox # added in 1.19
+    - gomnd # added in 1.23
     - interfacer
     - lll
     - scopelint
@@ -27,11 +28,17 @@ issues:
     - linters:
         - golint
       text: 'name .* stutters'
-
     - path: (^test/.*\.go|.*_test\.go)
       linters:
         - goconst
         - unparam
+    # TODO: remove this when upgrading to golangci-lint >1.24.0.
+    # currently it checks the receiver names of even generated files, but is fixed in a later version
+    - linters:
+        - stylecheck
+      text: 'ST1016: methods on the same type should have the same receiver name .*'
+    - path: cmd/manager/main\.go
+      text: 'cyclomatic complexity 31 of func `execute` is high (> 30)'
     - path: pkg/controller/apmserver/config/reconcile\.go
       text: 'G101: Potential hardcoded credentials'
     - path: pkg/controller/common/settings/canonical_config\.go
@@ -98,6 +105,3 @@ issues:
       text: 'G101: Potential hardcoded credentials'
     - path: pkg/controller/elasticsearch/driver/version\.go
       text: 'Consider preallocating `vs`'
-    - linters:
-        - stylecheck
-      text: 'ST1016: methods on the same type should have the same receiver name.*"in".*'

pkg/apis/elasticsearch/v1/elasticsearch_types.go

@@ -336,12 +336,12 @@ type Elasticsearch struct {
 }
 
 // IsMarkedForDeletion returns true if the Elasticsearch is going to be deleted
-func (e Elasticsearch) IsMarkedForDeletion() bool {
-	return !e.DeletionTimestamp.IsZero()
+func (es Elasticsearch) IsMarkedForDeletion() bool {
+	return !es.DeletionTimestamp.IsZero()
 }
 
-func (e Elasticsearch) SecureSettings() []commonv1.SecretSource {
-	return e.Spec.SecureSettings
+func (es Elasticsearch) SecureSettings() []commonv1.SecretSource {
+	return es.Spec.SecureSettings
 }
 
 // +kubebuilder:object:root=true

pkg/apis/elasticsearch/v1/validations.go

@@ -52,10 +52,10 @@ var updateValidations = []updateValidation{
 	pvcModification,
 }
 
-func (r *Elasticsearch) check(validations []validation) field.ErrorList {
+func (es *Elasticsearch) check(validations []validation) field.ErrorList {
 	var errs field.ErrorList
 	for _, val := range validations {
-		if err := val(r); err != nil {
+		if err := val(es); err != nil {
 			errs = append(errs, err...)
 		}
 	}

pkg/apis/elasticsearch/v1/warnings.go

@@ -32,8 +32,8 @@ func noUnsupportedSettings(es *Elasticsearch) field.ErrorList {
 	return errs
 }
 
-func (r *Elasticsearch) CheckForWarnings() error {
-	warnings := r.check(warnings)
+func (es *Elasticsearch) CheckForWarnings() error {
+	warnings := es.check(warnings)
 	if len(warnings) > 0 {
 		return warnings.ToAggregate()
 	}

pkg/apis/elasticsearch/v1/webhook.go

@@ -18,53 +18,53 @@ import (
 
 // +kubebuilder:webhook:path=/validate-elasticsearch-k8s-elastic-co-v1-elasticsearch,mutating=false,failurePolicy=ignore,groups=elasticsearch.k8s.elastic.co,resources=elasticsearches,verbs=create;update,versions=v1,name=elastic-es-validation-v1.k8s.elastic.co
 
-func (r *Elasticsearch) SetupWebhookWithManager(mgr ctrl.Manager) error {
+func (es *Elasticsearch) SetupWebhookWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewWebhookManagedBy(mgr).
-		For(r).
+		For(es).
 		Complete()
 }
 
 var eslog = logf.Log.WithName("es-validation")
 
 var _ webhook.Validator = &Elasticsearch{}
 
-func (r *Elasticsearch) ValidateCreate() error {
-	eslog.V(1).Info("validate create", "name", r.Name)
-	return r.validateElasticsearch()
+func (es *Elasticsearch) ValidateCreate() error {
+	eslog.V(1).Info("validate create", "name", es.Name)
+	return es.validateElasticsearch()
 }
 
 // ValidateDelete is required to implement webhook.Validator, but we do not actually validate deletes
-func (r *Elasticsearch) ValidateDelete() error {
+func (es *Elasticsearch) ValidateDelete() error {
 	return nil
 }
 
-func (r *Elasticsearch) ValidateUpdate(old runtime.Object) error {
-	eslog.V(1).Info("validate update", "name", r.Name)
+func (es *Elasticsearch) ValidateUpdate(old runtime.Object) error {
+	eslog.V(1).Info("validate update", "name", es.Name)
 	oldEs, ok := old.(*Elasticsearch)
 	if !ok {
 		return errors.New("cannot cast old object to Elasticsearch type")
 	}
 
 	var errs field.ErrorList
 	for _, val := range updateValidations {
-		if err := val(oldEs, r); err != nil {
+		if err := val(oldEs, es); err != nil {
 			errs = append(errs, err...)
 		}
 	}
 	if len(errs) > 0 {
 		return apierrors.NewInvalid(
 			schema.GroupKind{Group: "elasticsearch.k8s.elastic.co", Kind: "Elasticsearch"},
-			r.Name, errs)
+			es.Name, errs)
 	}
-	return r.validateElasticsearch()
+	return es.validateElasticsearch()
 }
 
-func (r *Elasticsearch) validateElasticsearch() error {
-	errs := r.check(validations)
+func (es *Elasticsearch) validateElasticsearch() error {
+	errs := es.check(validations)
 	if len(errs) > 0 {
 		return apierrors.NewInvalid(
 			schema.GroupKind{Group: "elasticsearch.k8s.elastic.co", Kind: "Elasticsearch"},
-			r.Name,
+			es.Name,
 			errs,
 		)
 	}

pkg/apis/elasticsearch/v1beta1/elasticsearch_types.go

@@ -223,12 +223,12 @@ type Elasticsearch struct {
 }
 
 // IsMarkedForDeletion returns true if the Elasticsearch is going to be deleted
-func (e Elasticsearch) IsMarkedForDeletion() bool {
-	return !e.DeletionTimestamp.IsZero()
+func (es Elasticsearch) IsMarkedForDeletion() bool {
+	return !es.DeletionTimestamp.IsZero()
 }
 
-func (e Elasticsearch) SecureSettings() []commonv1beta1.SecretSource {
-	return e.Spec.SecureSettings
+func (es Elasticsearch) SecureSettings() []commonv1beta1.SecretSource {
+	return es.Spec.SecureSettings
 }
 
 // +kubebuilder:object:root=true

pkg/apis/elasticsearch/v1beta1/validations.go

@@ -50,10 +50,10 @@ var updateValidations = []updateValidation{
 	pvcModification,
 }
 
-func (r *Elasticsearch) check(validations []validation) field.ErrorList {
+func (es *Elasticsearch) check(validations []validation) field.ErrorList {
 	var errs field.ErrorList
 	for _, val := range validations {
-		if err := val(r); err != nil {
+		if err := val(es); err != nil {
 			errs = append(errs, err...)
 		}
 	}

pkg/apis/elasticsearch/v1beta1/warnings.go

@@ -32,8 +32,8 @@ func noUnsupportedSettings(es *Elasticsearch) field.ErrorList {
 	return errs
 }
 
-func (r *Elasticsearch) CheckForWarnings() error {
-	warnings := r.check(warnings)
+func (es *Elasticsearch) CheckForWarnings() error {
+	warnings := es.check(warnings)
 	if len(warnings) > 0 {
 		return warnings.ToAggregate()
 	}

pkg/apis/elasticsearch/v1beta1/webhook.go

@@ -18,53 +18,53 @@ import (
 
 // +kubebuilder:webhook:path=/validate-elasticsearch-k8s-elastic-co-v1beta1-elasticsearch,mutating=false,failurePolicy=ignore,groups=elasticsearch.k8s.elastic.co,resources=elasticsearches,verbs=create;update,versions=v1beta1,name=elastic-es-validation-v1beta1.k8s.elastic.co
 
-func (r *Elasticsearch) SetupWebhookWithManager(mgr ctrl.Manager) error {
+func (es *Elasticsearch) SetupWebhookWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewWebhookManagedBy(mgr).
-		For(r).
+		For(es).
 		Complete()
 }
 
 var eslog = logf.Log.WithName("es-validation")
 
 var _ webhook.Validator = &Elasticsearch{}
 
-func (r *Elasticsearch) ValidateCreate() error {
-	eslog.V(1).Info("validate create", "name", r.Name)
-	return r.validateElasticsearch()
+func (es *Elasticsearch) ValidateCreate() error {
+	eslog.V(1).Info("validate create", "name", es.Name)
+	return es.validateElasticsearch()
 }
 
 // ValidateDelete is required to implement webhook.Validator, but we do not actually validate deletes
-func (r *Elasticsearch) ValidateDelete() error {
+func (es *Elasticsearch) ValidateDelete() error {
 	return nil
 }
 
-func (r *Elasticsearch) ValidateUpdate(old runtime.Object) error {
-	eslog.V(1).Info("validate update", "name", r.Name)
+func (es *Elasticsearch) ValidateUpdate(old runtime.Object) error {
+	eslog.V(1).Info("validate update", "name", es.Name)
 	oldEs, ok := old.(*Elasticsearch)
 	if !ok {
 		return errors.New("cannot cast old object to Elasticsearch type")
 	}
 
 	var errs field.ErrorList
 	for _, val := range updateValidations {
-		if err := val(oldEs, r); err != nil {
+		if err := val(oldEs, es); err != nil {
 			errs = append(errs, err...)
 		}
 	}
 	if len(errs) > 0 {
 		return apierrors.NewInvalid(
 			schema.GroupKind{Group: "elasticsearch.k8s.elastic.co", Kind: "Elasticsearch"},
-			r.Name, errs)
+			es.Name, errs)
 	}
-	return r.validateElasticsearch()
+	return es.validateElasticsearch()
 }
 
-func (r *Elasticsearch) validateElasticsearch() error {
-	errs := r.check(validations)
+func (es *Elasticsearch) validateElasticsearch() error {
+	errs := es.check(validations)
 	if len(errs) > 0 {
 		return apierrors.NewInvalid(
 			schema.GroupKind{Group: "elasticsearch.k8s.elastic.co", Kind: "Elasticsearch"},
-			r.Name,
+			es.Name,
 			errs,
 		)
 	}

pkg/controller/apmserver/apmserver_controller.go

@@ -40,7 +40,6 @@ import (
 
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/api/errors"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
@@ -238,7 +237,7 @@ func (r *ReconcileApmServer) doReconcile(ctx context.Context, request reconcile.
 
 	state, err = r.reconcileApmServerDeployment(ctx, state, as)
 	if err != nil {
-		if errors.IsConflict(err) {
+		if apierrors.IsConflict(err) {
 			log.V(1).Info("Conflict while updating status")
 			return reconcile.Result{Requeue: true}, nil
 		}
@@ -250,7 +249,7 @@ func (r *ReconcileApmServer) doReconcile(ctx context.Context, request reconcile.
 
 	// update status
 	err = r.updateStatus(ctx, state)
-	if err != nil && errors.IsConflict(err) {
+	if err != nil && apierrors.IsConflict(err) {
 		log.V(1).Info("Conflict while updating status", "namespace", as.Namespace, "as", as.Name)
 		return reconcile.Result{Requeue: true}, nil
 	}

pkg/controller/apmserver/apmserver_controller_test.go

@@ -23,7 +23,6 @@ import (
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/tools/record"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
@@ -192,22 +191,22 @@ func expectedDeploymentParams() testParams {
 
 func TestReconcileApmServer_deploymentParams(t *testing.T) {
 	apmFixture := &apmv1.ApmServer{
-		ObjectMeta: v1.ObjectMeta{
+		ObjectMeta: metav1.ObjectMeta{
 			Name: "test-apm-server",
 		},
-		TypeMeta: v1.TypeMeta{
+		TypeMeta: metav1.TypeMeta{
 			Kind: "apmserver",
 		},
 	}
 	defaultPodSpecParams := PodSpecParams{
 		Version: "1.0",
 		TokenSecret: corev1.Secret{
-			ObjectMeta: v1.ObjectMeta{
+			ObjectMeta: metav1.ObjectMeta{
 				Name: "test-apm-server-apm-token",
 			},
 		},
 		ConfigSecret: corev1.Secret{
-			ObjectMeta: v1.ObjectMeta{
+			ObjectMeta: metav1.ObjectMeta{
 				Name: "test-apm-config",
 			},
 		},
@@ -232,7 +231,7 @@ func TestReconcileApmServer_deploymentParams(t *testing.T) {
 				podSpecParams: defaultPodSpecParams,
 				initialObjects: []runtime.Object{
 					&corev1.Secret{
-						ObjectMeta: v1.ObjectMeta{
+						ObjectMeta: metav1.ObjectMeta{
 							Name: certSecretName,
 						},
 					},
@@ -248,7 +247,7 @@ func TestReconcileApmServer_deploymentParams(t *testing.T) {
 				podSpecParams: defaultPodSpecParams,
 				initialObjects: []runtime.Object{
 					&corev1.Secret{
-						ObjectMeta: v1.ObjectMeta{
+						ObjectMeta: metav1.ObjectMeta{
 							Name: certSecretName,
 						},
 						Data: map[string][]byte{
@@ -267,7 +266,7 @@ func TestReconcileApmServer_deploymentParams(t *testing.T) {
 				podSpecParams: func() PodSpecParams {
 					params := defaultPodSpecParams
 					params.ConfigSecret = corev1.Secret{
-						ObjectMeta: v1.ObjectMeta{
+						ObjectMeta: metav1.ObjectMeta{
 							Name: "test-apm-config",
 						},
 						Data: map[string][]byte{
@@ -278,7 +277,7 @@ func TestReconcileApmServer_deploymentParams(t *testing.T) {
 				}(),
 				initialObjects: []runtime.Object{
 					&corev1.Secret{
-						ObjectMeta: v1.ObjectMeta{
+						ObjectMeta: metav1.ObjectMeta{
 							Name: certSecretName,
 						},
 					},
@@ -304,7 +303,7 @@ func TestReconcileApmServer_deploymentParams(t *testing.T) {
 				}(),
 				initialObjects: []runtime.Object{
 					&corev1.Secret{
-						ObjectMeta: v1.ObjectMeta{
+						ObjectMeta: metav1.ObjectMeta{
 							Name: certSecretName,
 						},
 					},

pkg/controller/apmserverelasticsearchassociation/apmserverelasticsearchassociation_controller.go

@@ -28,7 +28,6 @@ import (
 	"github.com/elastic/cloud-on-k8s/pkg/utils/rbac"
 	"go.elastic.co/apm"
 	corev1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/api/errors"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/tools/record"
@@ -332,7 +331,7 @@ func (r *ReconcileApmServerElasticsearchAssociation) getElasticsearch(ctx contex
 			"Failed to find referenced backend %s: %v", elasticsearchRef.NamespacedName(), err)
 		if apierrors.IsNotFound(err) {
 			// ES is not found, remove any existing backend configuration and retry in a bit.
-			if err := association.RemoveAssociationConf(r.Client, apmServer); err != nil && !errors.IsConflict(err) {
+			if err := association.RemoveAssociationConf(r.Client, apmServer); err != nil && !apierrors.IsConflict(err) {
 				log.Error(err, "Failed to remove Elasticsearch output from APMServer object", "namespace", apmServer.Namespace, "name", apmServer.Name)
 				return commonv1.AssociationPending, err
 			}
@@ -351,7 +350,7 @@ func (r *ReconcileApmServerElasticsearchAssociation) updateAssocConf(ctx context
 	if !reflect.DeepEqual(expectedAssocConf, apmServer.AssociationConf()) {
 		log.Info("Updating APMServer spec with Elasticsearch association configuration", "namespace", apmServer.Namespace, "name", apmServer.Name)
 		if err := association.UpdateAssociationConf(r.Client, apmServer, expectedAssocConf); err != nil {
-			if errors.IsConflict(err) {
+			if apierrors.IsConflict(err) {
 				return commonv1.AssociationPending, nil
 			}
 			log.Error(err, "Failed to update APMServer association configuration", "namespace", apmServer.Namespace, "name", apmServer.Name)