sync linter settings with cluster-api repo and fix findings

Author: sbueringer

.golangci.yml

@@ -1,31 +1,87 @@
 linters:
-  enable-all: true
-  disable:
-    - dupl
-    - errorlint
-    - exhaustivestruct
-    - funlen
-    - gochecknoglobals
-    - gochecknoinits
-    - gocognit
-    - godox
-    - goerr113
-    - gomnd
-    - interfacer
-    - lll
-    - maligned
-    - nlreturn
-    - whitespace
-    - wrapcheck
-    - wsl
-  # Run with --fast=false for more extensive checks
-  fast: true
+  disable-all: true
+  enable:
+  - asciicheck
+  - bodyclose
+  - cyclop
+  - deadcode
+  - depguard
+  - dogsled
+  - durationcheck
+  - errcheck
+  - exportloopref
+  - forbidigo
+  - gci
+  - goconst
+  - gocritic
+  - gocyclo
+  - godot
+  - gofmt
+  - gofumpt
+  - goheader
+  - goimports
+  - gomodguard
+  - goprintffuncname
+  - gosec
+  - gosimple
+  - govet
+  - ifshort
+  - importas
+  - ineffassign
+  - makezero
+  - misspell
+  - nakedret
+  - nestif
+  - nilerr
+  - noctx
+  - nolintlint
+  - prealloc
+  - predeclared
+  - revive
+  - rowserrcheck
+  - sqlclosecheck
+  - staticcheck
+  - structcheck
+  - stylecheck
+  - thelper
+  - typecheck
+  - unconvert
+  - unparam
+  - unused
+  - varcheck
+  - wastedassign
+  - whitespace
+
 linters-settings:
   cyclop:
     # TODO(sbuerin) fix remaining findings and set to 20 afterwards
     max-complexity: 30
+  ifshort:
+    # Maximum length of variable declaration measured in number of characters, after which linter won't suggest using short syntax.
+    max-decl-chars: 50
   goimports:
     local-prefixes: sigs.k8s.io/cluster-api-provider-openstack
+  importas:
+    no-unaliased: true
+    alias:
+      # Kubernetes
+      - pkg: k8s.io/api/core/v1
+        alias: corev1
+      - pkg: k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1
+        alias: apiextensionsv1
+      - pkg: k8s.io/apimachinery/pkg/apis/meta/v1
+        alias: metav1
+      - pkg: k8s.io/apimachinery/pkg/api/errors
+        alias: apierrors
+      - pkg: k8s.io/apimachinery/pkg/util/errors
+        alias: kerrors
+      # Controller Runtime
+      - pkg: sigs.k8s.io/controller-runtime
+        alias: ctrl
+  staticcheck:
+    go: "1.16"
+  stylecheck:
+    go: "1.16"
   nestif:
     # minimal complexity of if statements to report, 5 by default
     # TODO(sbuerin) fix remaining findings and set to 5 after:
@@ -34,16 +90,34 @@ linters-settings:
 
 issues:
   max-same-issues: 0
-  max-per-linter: 0
+  max-issues-per-linter: 0
   # List of regexps of issue texts to exclude, empty list by default.
-  exclude:
-    - Using the variable on range scope `(tc)|(rt)|(tt)|(test)|(testcase)|(testCase)` in function literal
-    - "G108: Profiling endpoint is automatically exposed on /debug/pprof"
+  exclude-rules:
+  - linters:
+    - gosec
+    text: "G108: Profiling endpoint is automatically exposed on /debug/pprof"
+  - linters:
+    - gosec
+    text: "G108: Profiling endpoint is automatically exposed on /debug/pprof"
+  # This directive allows the embed package to be imported with an underscore everywhere.
+  - linters:
+    - revive
+    source: _ "embed"
+  - linters:
+    - revive
+    - stylecheck
+    path: (test)/.*.go
+    text: should not use dot imports
+  - linters:
+    - revive
+    path: test/e2e/shared/defaults.go
+    text: "exported: exported const .* should have comment \\(or a comment on this block\\) or be unexported"
 run:
-  timeout: 6m
+  timeout: 10m
   build-tags:
   - e2e
 
   skip-files:
     - "zz_generated.*\\.go$"
-    - ".*_mock\\.go"
+    - ".*conversion.*\\.go$"
+  allow-parallel-runners: true

api/v1alpha3/types.go

@@ -227,14 +227,19 @@ func (r SecurityGroupRule) Equal(x SecurityGroupRule) bool {
 type InstanceState string
 
 var (
+	// InstanceStateBuilding is the string representing an instance in a building state.
 	InstanceStateBuilding = InstanceState("BUILDING")
 
+	// InstanceStateActive is the string representing an instance in an active state.
 	InstanceStateActive = InstanceState("ACTIVE")
 
+	// InstanceStateError is the string representing an instance in an error state.
 	InstanceStateError = InstanceState("ERROR")
 
+	// InstanceStateStopped is the string representing an instance in a stopped state.
 	InstanceStateStopped = InstanceState("STOPPED")
 
+	// InstanceStateShutoff is the string representing an instance in a shutoff state.
 	InstanceStateShutoff = InstanceState("SHUTOFF")
 )
 

api/v1alpha4/openstackmachinetemplate_webhook.go

@@ -42,9 +42,8 @@ var _ webhook.Validator = &OpenStackMachineTemplate{}
 // ValidateCreate implements webhook.Validator so a webhook will be registered for the type.
 func (r *OpenStackMachineTemplate) ValidateCreate() error {
 	var allErrs field.ErrorList
-	spec := r.Spec.Template.Spec
 
-	if spec.ProviderID != nil {
+	if r.Spec.Template.Spec.ProviderID != nil {
 		allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "template", "spec", "providerID"), "cannot be set in templates"))
 	}
 

api/v1alpha4/openstackmachinetemplate_webhook_test.go

@@ -20,7 +20,7 @@ import (
 	"testing"
 
 	. "github.com/onsi/gomega"
-	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 func TestOpenStackMachineTemplate_ValidateUpdate(t *testing.T) {
@@ -67,7 +67,7 @@ func TestOpenStackMachineTemplate_ValidateUpdate(t *testing.T) {
 						},
 					},
 				},
-				ObjectMeta: v1.ObjectMeta{
+				ObjectMeta: metav1.ObjectMeta{
 					Name: "foo",
 				},
 			},
@@ -80,7 +80,7 @@ func TestOpenStackMachineTemplate_ValidateUpdate(t *testing.T) {
 						},
 					},
 				},
-				ObjectMeta: v1.ObjectMeta{
+				ObjectMeta: metav1.ObjectMeta{
 					Name: "bar",
 				},
 			},

api/v1alpha4/types.go

@@ -260,14 +260,19 @@ func (r SecurityGroupRule) Equal(x SecurityGroupRule) bool {
 type InstanceState string
 
 var (
+	// InstanceStateBuilding is the string representing an instance in a building state.
 	InstanceStateBuilding = InstanceState("BUILDING")
 
+	// InstanceStateActive is the string representing an instance in an active state.
 	InstanceStateActive = InstanceState("ACTIVE")
 
+	// InstanceStateError is the string representing an instance in an error state.
 	InstanceStateError = InstanceState("ERROR")
 
+	// InstanceStateStopped is the string representing an instance in a stopped state.
 	InstanceStateStopped = InstanceState("STOPPED")
 
+	// InstanceStateShutoff is the string representing an instance in a shutoff state.
 	InstanceStateShutoff = InstanceState("SHUTOFF")
 )
 

controllers/openstackmachine_controller.go

@@ -399,7 +399,7 @@ func (r *OpenStackMachineReconciler) reconcileLoadBalancerMember(logger logr.Log
 	return loadbalancerService.ReconcileLoadBalancerMember(openStackCluster, machine, openStackMachine, clusterName, ip)
 }
 
-// OpenStackClusterToOpenStackMachine is a handler.ToRequestsFunc to be used to enqeue requests for reconciliation
+// OpenStackClusterToOpenStackMachines is a handler.ToRequestsFunc to be used to enqeue requests for reconciliation
 // of OpenStackMachines.
 func (r *OpenStackMachineReconciler) OpenStackClusterToOpenStackMachines(ctx context.Context) handler.MapFunc {
 	log := ctrl.LoggerFrom(ctx)

hack/tools/go.mod

@@ -5,7 +5,7 @@ go 1.16
 require (
 	github.com/a8m/envsubst v1.2.0
 	github.com/golang/mock v1.4.4
-	github.com/golangci/golangci-lint v1.38.0
+	github.com/golangci/golangci-lint v1.40.1
 	github.com/itchyny/gojq v0.12.2
 	github.com/onsi/ginkgo v1.16.4
 	k8s.io/code-generator v0.21.1