Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update linter #2725

Merged
merged 6 commits into from
Mar 20, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .ci/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Docker image used for continuous integration
FROM golang:1.13-stretch

ENV GOLANGCILINT_VERSION=1.21.0
ENV GOLANGCILINT_VERSION=1.24.0
ENV KUBEBUILDER_VERSION=2.0.0
ENV GCLOUD_VERSION=277.0.0
ENV KUBECTL_VERSION=1.14.7
Expand Down
6 changes: 3 additions & 3 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ linters:
- scopelint
- whitespace # added in 1.19
- wsl # added in 1.20
- gomnd # added in 1.23
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: it would be nice to maintain the sorted order in this list

linters-settings:
maligned:
suggest-new: true
Expand Down Expand Up @@ -98,6 +99,5 @@ 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".*'
- path: cmd/manager/main\.go
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, this list was roughly sorted by path. No big deal though.

Copy link
Contributor Author

@anyasabo anyasabo Mar 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I didn't realize for either of them, fixing

text: 'cyclomatic complexity 31 of func `execute` is high (> 30)'
8 changes: 4 additions & 4 deletions pkg/apis/elasticsearch/v1/elasticsearch_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/elasticsearch/v1/validations.go
Original file line number Diff line number Diff line change
Expand Up @@ -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...)
}
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/elasticsearch/v1/warnings.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand Down
28 changes: 14 additions & 14 deletions pkg/apis/elasticsearch/v1/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/apis/elasticsearch/v1beta1/elasticsearch_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/elasticsearch/v1beta1/validations.go
Original file line number Diff line number Diff line change
Expand Up @@ -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...)
}
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/elasticsearch/v1beta1/warnings.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand Down
28 changes: 14 additions & 14 deletions pkg/apis/elasticsearch/v1beta1/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
}
Expand Down
5 changes: 2 additions & 3 deletions pkg/controller/apmserver/apmserver_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -244,7 +243,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
}
Expand All @@ -256,7 +255,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
}
Expand Down
19 changes: 9 additions & 10 deletions pkg/controller/apmserver/apmserver_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,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"
Expand Down Expand Up @@ -191,22 +190,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",
ApmServerSecret: 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",
},
},
Expand All @@ -231,7 +230,7 @@ func TestReconcileApmServer_deploymentParams(t *testing.T) {
podSpecParams: defaultPodSpecParams,
initialObjects: []runtime.Object{
&corev1.Secret{
ObjectMeta: v1.ObjectMeta{
ObjectMeta: metav1.ObjectMeta{
Name: certSecretName,
},
},
Expand All @@ -247,7 +246,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{
Expand All @@ -266,7 +265,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{
Expand All @@ -277,7 +276,7 @@ func TestReconcileApmServer_deploymentParams(t *testing.T) {
}(),
initialObjects: []runtime.Object{
&corev1.Secret{
ObjectMeta: v1.ObjectMeta{
ObjectMeta: metav1.ObjectMeta{
Name: certSecretName,
},
},
Expand All @@ -303,7 +302,7 @@ func TestReconcileApmServer_deploymentParams(t *testing.T) {
}(),
initialObjects: []runtime.Object{
&corev1.Secret{
ObjectMeta: v1.ObjectMeta{
ObjectMeta: metav1.ObjectMeta{
Name: certSecretName,
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -342,7 +341,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
}
Expand All @@ -361,7 +360,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)
Expand Down
3 changes: 1 addition & 2 deletions pkg/controller/common/association/gc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import (
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
)
Expand All @@ -34,7 +33,7 @@ func newUserSecret(
associationNamespaceValue, associationNameValue string,
) runtime.Object {
return &corev1.Secret{
ObjectMeta: v1.ObjectMeta{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: namespace,
Labels: map[string]string{
Expand Down
Loading