golangci-lint: update golangci rules and fix the lints (#269)

* golangci-lint: bump to v1.42.0 Signed-off-by: Carlos Panato <[email protected]> * golangci-lint: update golangci rules and fix the lints Signed-off-by: Carlos Panato <[email protected]>
kubernetes-sigs · Sep 14, 2021 · d9898f1 · d9898f1
1 parent 6e448e1
commit d9898f1
Show file tree

Hide file tree

Showing 30 changed files with 218 additions and 61 deletions.
diff --git a/.golangci.yml b/.golangci.yml
@@ -1,31 +1,134 @@
 linters:
   disable-all: true
   enable:
-    - deadcode
-    - goconst
-    - gofmt
-    - golint
-    - gosec
-    - govet
-    - ineffassign
-    - interfacer
-    - maligned
-    - misspell
-    - nakedret
-    - prealloc
-    - structcheck
-    - unconvert
-    - varcheck
-  # Run with --fast=false for more extensive checks
-  fast: true
+  - asciicheck
+  - bodyclose
+  - deadcode
+  - depguard
+  - dogsled
+  - errcheck
+  - exportloopref
+  - goconst
+  - gocritic
+  - gocyclo
+  - godot
+  - gofmt
+  - goimports
+  - goprintffuncname
+  - gosec
+  - gosimple
+  - govet
+  - ifshort
+  - importas
+  - ineffassign
+  - misspell
+  - nakedret
+  - nilerr
+  - nolintlint
+  - prealloc
+  - revive
+  - rowserrcheck
+  - staticcheck
+  - structcheck
+  - stylecheck
+  - typecheck
+  - unconvert
+  - unparam
+  - varcheck
+  - whitespace
+
+linters-settings:
+  ifshort:
+    # Maximum length of variable declaration measured in number of characters, after which linter won't suggest using short syntax.
+    max-decl-chars: 50
+  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"
+
 issues:
   max-same-issues: 0
-  max-per-linter: 0
+  max-issues-per-linter: 0
+  # We are disabling default golangci exclusions because we want to help reviewers to focus on reviewing the most relevant
+  # changes in PRs and avoid nitpicking.
+  exclude-use-default: false
   # 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
+  # The following are being worked on to remove their exclusion. This list should be reduced or go away all together over time.
+  # If it is decided they will not be addressed they should be moved above this comment.
+  - Subprocess launch(ed with variable|ing should be audited)
+  - (Expect directory permissions to be 0750 or less|Expect file permissions to be 0600 or less)
+  - (G104|G307)
+  exclude-rules:
+  - linters:
+    - gosec
+    text: "G108: Profiling endpoint is automatically exposed on /debug/pprof"
+  - linters:
+    - revive
+    text: "exported: exported method .*\\.(Reconcile|SetupWithManager|SetupWebhookWithManager) should have comment or be unexported"
+  - linters:
+    - errcheck
+    text: Error return value of .((os\.)?std(out|err)\..*|.*Close|.*Flush|os\.Remove(All)?|.*print(f|ln)?|os\.(Un)?Setenv). is not checked
+  # With Go 1.16, the new embed directive can be used with an un-named import,
+  # revive (previously, golint) only allows these to be imported in a main.go, which wouldn't work for us.
+  # This directive allows the embed package to be imported with an underscore everywhere.
+  - linters:
+    - revive
+    source: _ "embed"
+  # Exclude some packages or code to require comments, for example test code, or fake clients.
+  - linters:
+    - revive
+    text: exported (method|function|type|const) (.+) should have comment or be unexported
+    source: (func|type).*Fake.*
+  - linters:
+    - revive
+    text: exported (method|function|type|const) (.+) should have comment or be unexported
+    path: fake_\.go
+  - linters:
+    - revive
+    text: exported (method|function|type|const) (.+) should have comment or be unexported
+    path: "(framework|e2e|infrastructure/docker)/.*.go"
+  # Disable unparam "always receives" which might not be really
+  # useful when building libraries.
+  - linters:
+    - unparam
+    text: always receives
+  # Dot imports for gomega or ginkgo are allowed
+  # within test files.
+  - path: _test\.go
+    text: should not use dot imports
+  - path: (framework|e2e)/.*.go
+    text: should not use dot imports
+  - path: _test\.go
+    text: cyclomatic complexity
+  # Append should be able to assign to a different var/slice.
+  - linters:
+    - gocritic
+    text: "appendAssign: append result not assigned to the same slice"
+
+
 run:
-  timeout: 6m
+  timeout: 10m
   skip-files:
-    - "zz_generated.*\\.go$"
-    - ".*_mock\\.go"
+  - "zz_generated.*\\.go$"
+  - ".*conversion.*\\.go$"
+  skip-dirs:
+  - mock*
+  allow-parallel-runners: true
diff --git a/Makefile b/Makefile
@@ -107,7 +107,7 @@ CONVERSION_GEN := $(TOOLS_BIN_DIR)/$(CONVERSION_GEN_BIN)-$(CONVERSION_GEN_VER)
 ENVSUBST_BIN := envsubst
 ENVSUBST := $(TOOLS_BIN_DIR)/$(ENVSUBST_BIN)-drone
 
-GOLANGCI_LINT_VER := v1.40.1
+GOLANGCI_LINT_VER := v1.42.0
 GOLANGCI_LINT_BIN := golangci-lint
 GOLANGCI_LINT := $(TOOLS_BIN_DIR)/$(GOLANGCI_LINT_BIN)-$(GOLANGCI_LINT_VER)
 

diff --git a/api/v1alpha3/docluster_types.go b/api/v1alpha3/docluster_types.go
@@ -61,6 +61,7 @@ type DOClusterStatus struct {
 	Network DONetworkResource `json:"network,omitempty"`
 }
 
+// DOControlPlaneDNS ...
 type DOControlPlaneDNS struct {
 	// Domain is the DO domain that this record should live in. It must be pre-existing in your DO account.
 	// The format must be a string that conforms to the definition of a subdomain in DNS (RFC 1123)

diff --git a/api/v1alpha3/groupversion_info.go b/api/v1alpha3/groupversion_info.go
@@ -26,10 +26,10 @@ import (
 )
 
 var (
-	// GroupVersion is group version used to register these objects
+	// GroupVersion is group version used to register these objects.
 	GroupVersion = schema.GroupVersion{Group: "infrastructure.cluster.x-k8s.io", Version: "v1alpha3"}
 
-	// SchemeBuilder is used to add go types to the GroupVersionKind scheme
+	// SchemeBuilder is used to add go types to the GroupVersionKind scheme.
 	SchemeBuilder = &scheme.Builder{GroupVersion: GroupVersion}
 
 	// AddToScheme adds the types in this group-version to the given scheme.

diff --git a/api/v1alpha3/tags.go b/api/v1alpha3/tags.go
@@ -25,11 +25,11 @@ type Tags []string
 
 const (
 	// NameDigitalOceanProviderPrefix is the tag prefix for
-	// cluster-api-provider-digitalocean owned components
+	// cluster-api-provider-digitalocean owned components.
 	NameDigitalOceanProviderPrefix = "sigs-k8s-io:capdo"
-	// APIServerRoleTagValue describes the value for the apiserver role
+	// APIServerRoleTagValue describes the value for the apiserver role.
 	APIServerRoleTagValue = "apiserver"
-	// NodeRoleTagValue describes the value for the node role
+	// NodeRoleTagValue describes the value for the node role.
 	NodeRoleTagValue = "node"
 )
 

diff --git a/api/v1alpha3/types.go b/api/v1alpha3/types.go
@@ -133,12 +133,18 @@ type DOVPC struct {
 }
 
 var (
-	DefaultLBPort                          = 6443
-	DefaultLBAlgorithm                     = "round_robin"
-	DefaultLBHealthCheckInterval           = 10
-	DefaultLBHealthCheckTimeout            = 5
+	// DefaultLBPort default LoadBalancer port.
+	DefaultLBPort = 6443
+	// DefaultLBAlgorithm default LoadBalancer algorithm.
+	DefaultLBAlgorithm = "round_robin"
+	// DefaultLBHealthCheckInterval default LoadBalancer health check interval.
+	DefaultLBHealthCheckInterval = 10
+	// DefaultLBHealthCheckTimeout default LoadBalancer health check timeout.
+	DefaultLBHealthCheckTimeout = 5
+	// DefaultLBHealthCheckUnhealthyThreshold default LoadBalancer unhealthy threshold.
 	DefaultLBHealthCheckUnhealthyThreshold = 3
-	DefaultLBHealthCheckHealthyThreshold   = 5
+	// DefaultLBHealthCheckHealthyThreshold default LoadBalancer healthy threshold.
+	DefaultLBHealthCheckHealthyThreshold = 5
 )
 
 // ApplyDefault give APIServerLoadbalancers default values.

diff --git a/api/v1alpha4/docluster_webhook.go b/api/v1alpha4/docluster_webhook.go
@@ -47,15 +47,15 @@ func (r *DOCluster) SetupWebhookWithManager(mgr ctrl.Manager) error {
 		Complete()
 }
 
-// Default implements webhook.Defaulter so a webhook will be registered for the type
+// Default implements webhook.Defaulter so a webhook will be registered for the type.
 func (r *DOCluster) Default() {}
 
 // ValidateCreate implements webhook.Validator so a webhook will be registered for the type.
 func (r *DOCluster) ValidateCreate() error {
 	return nil
 }
 
-// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
+// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type.
 func (r *DOCluster) ValidateUpdate(old runtime.Object) error {
 	var allErrs field.ErrorList
 
@@ -79,7 +79,7 @@ func (r *DOCluster) ValidateUpdate(old runtime.Object) error {
 	return apierrors.NewInvalid(r.GroupVersionKind().GroupKind(), r.Name, allErrs)
 }
 
-// ValidateDelete implements webhook.Validator so a webhook will be registered for the type
+// ValidateDelete implements webhook.Validator so a webhook will be registered for the type.
 func (r *DOCluster) ValidateDelete() error {
 	return nil
 }
diff --git a/api/v1alpha4/domachine_webhook.go b/api/v1alpha4/domachine_webhook.go
@@ -47,15 +47,15 @@ func (r *DOMachine) SetupWebhookWithManager(mgr ctrl.Manager) error {
 		Complete()
 }
 
-// Default implements webhook.Defaulter so a webhook will be registered for the type
+// Default implements webhook.Defaulter so a webhook will be registered for the type.
 func (r *DOMachine) Default() {}
 
 // ValidateCreate implements webhook.Validator so a webhook will be registered for the type.
 func (r *DOMachine) ValidateCreate() error {
 	return nil
 }
 
-// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
+// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type.
 func (r *DOMachine) ValidateUpdate(old runtime.Object) error {
 	var allErrs field.ErrorList
 
@@ -91,7 +91,7 @@ func (r *DOMachine) ValidateUpdate(old runtime.Object) error {
 	return apierrors.NewInvalid(r.GroupVersionKind().GroupKind(), r.Name, allErrs)
 }
 
-// ValidateDelete implements webhook.Validator so a webhook will be registered for the type
+// ValidateDelete implements webhook.Validator so a webhook will be registered for the type.
 func (r *DOMachine) ValidateDelete() error {
 	return nil
 }
diff --git a/api/v1alpha4/domachinetemplate_webhook.go b/api/v1alpha4/domachinetemplate_webhook.go
@@ -43,12 +43,11 @@ func (r *DOMachineTemplate) SetupWebhookWithManager(mgr ctrl.Manager) error {
 		Complete()
 }
 
-// ValidateCreate implements webhook.Validator so a webhook will be registered for the type
+// ValidateCreate implements webhook.Validator so a webhook will be registered for the type.
 func (r *DOMachineTemplate) 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"))
 	}
 
@@ -59,7 +58,7 @@ func (r *DOMachineTemplate) ValidateCreate() error {
 	return apierrors.NewInvalid(r.GroupVersionKind().GroupKind(), r.Name, allErrs)
 }
 
-// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
+// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type.
 func (r *DOMachineTemplate) ValidateUpdate(old runtime.Object) error {
 	var allErrs field.ErrorList
 
@@ -75,7 +74,7 @@ func (r *DOMachineTemplate) ValidateUpdate(old runtime.Object) error {
 	return apierrors.NewInvalid(r.GroupVersionKind().GroupKind(), r.Name, allErrs)
 }
 
-// ValidateDelete implements webhook.Validator so a webhook will be registered for the type
+// ValidateDelete implements webhook.Validator so a webhook will be registered for the type.
 func (r *DOMachineTemplate) ValidateDelete() error {
 	return nil
 }
diff --git a/api/v1alpha4/groupversion_info.go b/api/v1alpha4/groupversion_info.go
@@ -25,10 +25,10 @@ import (
 )
 
 var (
-	// GroupVersion is group version used to register these objects
+	// GroupVersion is group version used to register these objects.
 	GroupVersion = schema.GroupVersion{Group: "infrastructure.cluster.x-k8s.io", Version: "v1alpha4"}
 
-	// SchemeBuilder is used to add go types to the GroupVersionKind scheme
+	// SchemeBuilder is used to add go types to the GroupVersionKind scheme.
 	SchemeBuilder = &scheme.Builder{GroupVersion: GroupVersion}
 
 	// AddToScheme adds the types in this group-version to the given scheme.

diff --git a/api/v1alpha4/tags.go b/api/v1alpha4/tags.go
@@ -25,11 +25,11 @@ type Tags []string
 
 const (
 	// NameDigitalOceanProviderPrefix is the tag prefix for
-	// cluster-api-provider-digitalocean owned components
+	// cluster-api-provider-digitalocean owned components.
 	NameDigitalOceanProviderPrefix = "sigs-k8s-io:capdo"
-	// APIServerRoleTagValue describes the value for the apiserver role
+	// APIServerRoleTagValue describes the value for the apiserver role.
 	APIServerRoleTagValue = "apiserver"
-	// NodeRoleTagValue describes the value for the node role
+	// NodeRoleTagValue describes the value for the node role.
 	NodeRoleTagValue = "node"
 )
 

diff --git a/api/v1alpha4/types.go b/api/v1alpha4/types.go
@@ -28,6 +28,7 @@ func DOSafeName(name string) string {
 	return r.Replace(name)
 }
 
+// DOControlPlaneDNS ...
 type DOControlPlaneDNS struct {
 	// Domain is the DO domain that this record should live in. It must be pre-existing in your DO account.
 	// The format must be a string that conforms to the definition of a subdomain in DNS (RFC 1123)
@@ -136,12 +137,18 @@ type DOVPC struct {
 }
 
 var (
-	DefaultLBPort                          = 6443
-	DefaultLBAlgorithm                     = "round_robin"
-	DefaultLBHealthCheckInterval           = 10
-	DefaultLBHealthCheckTimeout            = 5
+	// DefaultLBPort default LoadBalancer port.
+	DefaultLBPort = 6443
+	// DefaultLBAlgorithm default LoadBalancer algorithm.
+	DefaultLBAlgorithm = "round_robin"
+	// DefaultLBHealthCheckInterval default LoadBalancer health check interval.
+	DefaultLBHealthCheckInterval = 10
+	// DefaultLBHealthCheckTimeout default LoadBalancer health check timeout.
+	DefaultLBHealthCheckTimeout = 5
+	// DefaultLBHealthCheckUnhealthyThreshold default LoadBalancer unhealthy threshold.
 	DefaultLBHealthCheckUnhealthyThreshold = 3
-	DefaultLBHealthCheckHealthyThreshold   = 5
+	// DefaultLBHealthCheckHealthyThreshold default LoadBalancer healthy threshold.
+	DefaultLBHealthCheckHealthyThreshold = 5
 )
 
 // ApplyDefault give APIServerLoadbalancers default values.

diff --git a/cloud/scope/clients.go b/cloud/scope/clients.go
@@ -13,12 +13,15 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 See the License for the specific language governing permissions and
 limitations under the License.
 */
+
+// Package scope implements scope types.
 package scope
 
 import (
 	"github.com/digitalocean/godo"
 )
 
+// DOClients hold all necessary clients to work with the DO API.
 type DOClients struct {
 	Actions       godo.ActionsService
 	Droplets      godo.DropletsService

diff --git a/cloud/scope/cluster.go b/cloud/scope/cluster.go
@@ -127,6 +127,7 @@ func (s *ClusterScope) Namespace() string {
 	return s.Cluster.GetNamespace()
 }
 
+// UID returns the cluster UID.
 func (s *ClusterScope) UID() string {
 	return string(s.Cluster.UID)
 }