🌱 Add addtional linters

.golangci.yml

@@ -1,3 +1,45 @@
+linters:
+  disable-all: true
+  enable:
+  - asciicheck
+  - bodyclose
+  - deadcode
+  - depguard
+  - dogsled
+  - errcheck
+  - exportloopref
+  - goconst
+  - gocritic
+  - gocyclo
+  - godot
+  - gofmt
+  - goheader
+  - goimports
+  - goprintffuncname
+  - gosec
+  - gosimple
+  - govet
+  - ifshort
+  - importas
+  - ineffassign
+  - maligned
+  - misspell
+  - nakedret
+  - nilerr
+  - noctx
+  - nolintlint
+  - prealloc
+  - revive
+  - rowserrcheck
+  - staticcheck
+  - structcheck
+  - stylecheck
+  - typecheck
+  - unconvert
+  - unparam
+  - varcheck
+  - whitespace
+
 linters-settings:
   # Restrict revive to exported.
   revive:
@@ -7,48 +49,43 @@ linters-settings:
     rules:
       - name: exported
         severity: warning
-linters:
-  disable-all: true
-  enable:
-    - asciicheck
-    - bodyclose
-    - deadcode
-    - depguard
-    - dogsled
-    - goconst
-    - gocyclo
-    - gofmt
-    - goheader
-    - goimports
-    - goprintffuncname
-    - gosimple
-    - govet
-    - ineffassign
-    - maligned
-    - misspell
-    - nakedret
-    - noctx
-    - nolintlint
-    - rowserrcheck
-    - staticcheck
-    - structcheck
-    - typecheck
-    - unconvert
-    - unparam
-    - unused
-    - varcheck
-    - revive
-  # Run with --fast=false for more extensive checks
-  fast: true
+  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
-  # List of regexps of issue texts to exclude, empty list by default.
+  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:
-    - "G108: Profiling endpoint is automatically exposed on /debug/pprof"
-    - "exported: exported method .*\\.(Reconcile|SetupWithManager|SetupWebhookWithManager) should have comment or be unexported"
+    - (Expect directory permissions to be 0750 or less|Expect file permissions to be 0600 or less)
+    - "exported: exported (const|function|method|type|var) (.+) should have comment or be unexported"
     - "exported: (func|type) name will be used as (.+) by other packages, and that stutters; consider calling this (.+)"
+    - (G104|G107|G404|G505|ST1000)
+    - "G108: Profiling endpoint is automatically exposed on /debug/pprof"
     # Exclude noctx error for calling http.Get directly.
     # See https://pkg.go.dev/github.com/sonatard/noctx#readme-how-to-fix for reasons it breaks and ways to fix it.
     # This exclusion should be removed if the decision is made to fix the error.
@@ -67,11 +104,55 @@ issues:
       - revive
       text: exported (method|function|type|const) (.+) should have comment or be unexported
       path: .*test/(providers|framework|e2e).*.go
+    - linters:
+      - errcheck
+      text: Error return value is not checked
+      path: _test\.go
+    - linters:
+      - errcheck
+      text: Error return value of (.+) is not checked
+      path: _test\.go
+    - linters:
+      - gosec
+      text: "G108: Profiling endpoint is automatically exposed on /debug/pprof"
+    - linters:
+      - godot
+      text: "Comment should end in a period"
+      path: "(.*)/(v1alpha3|v1alpha4|v1alpha1)/(.*)types.go"
+    - 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"
+    # This directive allows the variable in defaults.go files to have underscore   
+    - linters:
+      - revive
+      text: "var-naming: don't use underscores in Go names; func (.+) should be (.+)" 
+      path: .*/defaults.go
+    # Disable unparam "always receives" which might not be really
+    # useful when building libraries.
+    - linters:
+      - unparam
+      text: always receives
+    - linters:
+      - unparam
+      text: (.+) - (`t`|`g`) is unused  
+    - 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: 10m
-  tests: false
   skip-files:
-    - "zz_generated.*\\.go$"
-    # Skip autogenerated doc.go files 
-    - "doc\\.go"
-    - "mock\\.go"
+  - "zz_generated.*\\.go$"
+  - ".*conversion.*\\.go$"
+  skip-dirs:
+  - third_party
+  allow-parallel-runners: true

api/v1alpha3/awscluster_types.go

@@ -26,7 +26,7 @@ const (
 	// removing it from the apiserver.
 	ClusterFinalizer = "awscluster.infrastructure.cluster.x-k8s.io"
 
-	// AWSClusterControllerIdentityName is the name of the AWSClusterControllerIdentity singleton
+	// AWSClusterControllerIdentityName is the name of the AWSClusterControllerIdentity singleton.
 	AWSClusterControllerIdentityName = "default"
 )
 
@@ -95,13 +95,13 @@ type AWSClusterSpec struct {
 type AWSIdentityKind string
 
 var (
-	// ControllerIdentityKind defines identity reference kind as AWSClusterControllerIdentity
+	// ControllerIdentityKind defines identity reference kind as AWSClusterControllerIdentity.
 	ControllerIdentityKind = AWSIdentityKind("AWSClusterControllerIdentity")
 
-	// ClusterRoleIdentityKind defines identity reference kind as AWSClusterRoleIdentity
+	// ClusterRoleIdentityKind defines identity reference kind as AWSClusterRoleIdentity.
 	ClusterRoleIdentityKind = AWSIdentityKind("AWSClusterRoleIdentity")
 
-	// ClusterStaticIdentityKind defines identity reference kind as AWSClusterStaticIdentity
+	// ClusterStaticIdentityKind defines identity reference kind as AWSClusterStaticIdentity.
 	ClusterStaticIdentityKind = AWSIdentityKind("AWSClusterStaticIdentity")
 )
 
@@ -144,7 +144,7 @@ type Bastion struct {
 	AMI string `json:"ami,omitempty"`
 }
 
-// AWSLoadBalancerSpec defines the desired state of an AWS load balancer
+// AWSLoadBalancerSpec defines the desired state of an AWS load balancer.
 type AWSLoadBalancerSpec struct {
 	// Scheme sets the scheme of the load balancer (defaults to Internet-facing)
 	// +kubebuilder:default=Internet-facing
@@ -167,7 +167,7 @@ type AWSLoadBalancerSpec struct {
 	// +optional
 	Subnets []string `json:"subnets,omitempty"`
 
-	// AdditionalSecurityGroups sets the security groups used by the load balancer. Expected to be security group IDs.
+	// AdditionalSecurityGroups sets the security groups used by the load balancer. Expected to be security group IDs
 	// This is optional - if not provided new security groups will be created for the load balancer
 	// +optional
 	AdditionalSecurityGroups []string `json:"additionalSecurityGroups,omitempty"`
@@ -203,7 +203,7 @@ type AWSCluster struct {
 
 // +kubebuilder:object:root=true
 
-// AWSClusterList contains a list of AWSCluster
+// AWSClusterList contains a list of AWSCluster.
 type AWSClusterList struct {
 	metav1.TypeMeta `json:",inline"`
 	metav1.ListMeta `json:"metadata,omitempty"`

api/v1alpha3/awsidentity_types.go

@@ -85,7 +85,7 @@ type AWSClusterStaticIdentity struct {
 
 // +kubebuilder:object:root=true
 
-// AWSClusterStaticIdentityList contains a list of AWSClusterStaticIdentity
+// AWSClusterStaticIdentityList contains a list of AWSClusterStaticIdentity.
 type AWSClusterStaticIdentityList struct {
 	metav1.TypeMeta `json:",inline"`
 	metav1.ListMeta `json:"metadata,omitempty"`
@@ -118,7 +118,7 @@ type AWSClusterRoleIdentity struct {
 
 // +kubebuilder:object:root=true
 
-// AWSClusterRoleIdentityList contains a list of AWSClusterRoleIdentity
+// AWSClusterRoleIdentityList contains a list of AWSClusterRoleIdentity.
 type AWSClusterRoleIdentityList struct {
 	metav1.TypeMeta `json:",inline"`
 	metav1.ListMeta `json:"metadata,omitempty"`
@@ -161,7 +161,7 @@ type AWSClusterControllerIdentity struct {
 
 // +kubebuilder:object:root=true
 
-// AWSClusterControllerIdentityList contains a list of AWSClusterControllerIdentity
+// AWSClusterControllerIdentityList contains a list of AWSClusterControllerIdentity.
 type AWSClusterControllerIdentityList struct {
 	metav1.TypeMeta `json:",inline"`
 	metav1.ListMeta `json:"metadata,omitempty"`

api/v1alpha3/awsmachine_types.go

@@ -32,10 +32,10 @@ const (
 type SecretBackend string
 
 var (
-	// SecretBackendSSMParameterStore defines AWS Systems Manager Parameter Store as the secret backend
+	// SecretBackendSSMParameterStore defines AWS Systems Manager Parameter Store as the secret backend.
 	SecretBackendSSMParameterStore = SecretBackend("ssm-parameter-store")
 
-	// SecretBackendSecretsManager defines AWS Secrets Manager as the secret backend
+	// SecretBackendSecretsManager defines AWS Secrets Manager as the secret backend.
 	SecretBackendSecretsManager = SecretBackend("secrets-manager")
 )
 
@@ -267,7 +267,7 @@ func (r *AWSMachine) SetConditions(conditions clusterv1.Conditions) {
 
 // +kubebuilder:object:root=true
 
-// AWSMachineList contains a list of AWSMachine
+// AWSMachineList contains a list of AWSMachine.
 type AWSMachineList struct {
 	metav1.TypeMeta `json:",inline"`
 	metav1.ListMeta `json:"metadata,omitempty"`

api/v1alpha3/awsmachinetemplate_types.go

@@ -38,7 +38,7 @@ type AWSMachineTemplate struct {
 
 // +kubebuilder:object:root=true
 
-// AWSMachineTemplateList contains a list of AWSMachineTemplate
+// AWSMachineTemplateList contains a list of AWSMachineTemplate.
 type AWSMachineTemplateList struct {
 	metav1.TypeMeta `json:",inline"`
 	metav1.ListMeta `json:"metadata,omitempty"`

api/v1alpha3/conditions_consts.go

@@ -31,32 +31,32 @@ const (
 	// PrincipalUsageUnauthorizedReason used when AWSCluster namespace is not in the identity's allowed namespaces list.
 	PrincipalUsageUnauthorizedReason = "PrincipalUsageUnauthorized"
 	// SourcePrincipalUsageUnauthorizedReason used when AWSCluster is not in the intersection of source identity allowed namespaces
-	// and allowed namespaces of the identities that source identity depends to
+	// and allowed namespaces of the identities that source identity depends to.
 	SourcePrincipalUsageUnauthorizedReason = "SourcePrincipalUsageUnauthorized"
 )
 
 const (
-	// VpcReadyCondition reports on the successful reconciliation of a VPC
+	// VpcReadyCondition reports on the successful reconciliation of a VPC.
 	VpcReadyCondition clusterv1.ConditionType = "VpcReady"
 	// VpcCreationStartedReason used when attempting to create a VPC for a managed cluster.
 	// Will not be applied to unmanaged clusters.
 	VpcCreationStartedReason = "VpcCreationStarted"
-	// VpcReconciliationFailedReason used when errors occur during VPC reconciliation
+	// VpcReconciliationFailedReason used when errors occur during VPC reconciliation.
 	VpcReconciliationFailedReason = "VpcReconciliationFailed"
 )
 
 const (
 	// SubnetsReadyCondition reports on the successful reconciliation of subnets.
 	SubnetsReadyCondition clusterv1.ConditionType = "SubnetsReady"
-	// SubnetsReconciliationFailedReason used to report failures while reconciling subnets
+	// SubnetsReconciliationFailedReason used to report failures while reconciling subnets.
 	SubnetsReconciliationFailedReason = "SubnetsReconciliationFailed"
 )
 
 const (
 	// InternetGatewayReadyCondition reports on the successful reconciliation of internet gateways.
 	// Only applicable to managed clusters.
 	InternetGatewayReadyCondition clusterv1.ConditionType = "InternetGatewayReady"
-	// InternetGatewayFailedReason used when errors occur during internet gateway reconciliation
+	// InternetGatewayFailedReason used when errors occur during internet gateway reconciliation.
 	InternetGatewayFailedReason = "InternetGatewayFailed"
 )
 
@@ -95,22 +95,22 @@ const (
 
 const (
 	// BastionHostReadyCondition reports whether a bastion host is ready. Depending on the configuration, a cluster
-	// may not require a bastion host and this condition will be skipped
+	// may not require a bastion host and this condition will be skipped.
 	BastionHostReadyCondition clusterv1.ConditionType = "BastionHostReady"
-	// BastionCreationStartedReason used when creating a new bastion host
+	// BastionCreationStartedReason used when creating a new bastion host.
 	BastionCreationStartedReason = "BastionCreationStarted"
-	// BastionHostFailedReason used when an error occurs during the creation of a bastion host
+	// BastionHostFailedReason used when an error occurs during the creation of a bastion host.
 	BastionHostFailedReason = "BastionHostFailed"
 )
 
 const (
 	// LoadBalancerReadyCondition reports on whether a control plane load balancer was successfully reconciled.
 	LoadBalancerReadyCondition clusterv1.ConditionType = "LoadBalancerReady"
-	// WaitForDNSNameReason used while waiting for a DNS name for the API server to be populated
+	// WaitForDNSNameReason used while waiting for a DNS name for the API server to be populated.
 	WaitForDNSNameReason = "WaitForDNSName"
-	// WaitForDNSNameResolveReason used while waiting for DNS name to resolve
+	// WaitForDNSNameResolveReason used while waiting for DNS name to resolve.
 	WaitForDNSNameResolveReason = "WaitForDNSNameResolve"
-	// LoadBalancerFailedReason used when an error occurs during load balancer reconciliation
+	// LoadBalancerFailedReason used when an error occurs during load balancer reconciliation.
 	LoadBalancerFailedReason = "LoadBalancerFailed"
 )
 
@@ -140,7 +140,7 @@ const (
 	// SecurityGroupsReadyCondition indicates the security groups are up to date on the AWSMachine.
 	SecurityGroupsReadyCondition clusterv1.ConditionType = "SecurityGroupsReady"
 
-	// SecurityGroupsFailedReason used when the security groups could not be synced
+	// SecurityGroupsFailedReason used when the security groups could not be synced.
 	SecurityGroupsFailedReason = "SecurityGroupsSyncFailed"
 )
 
@@ -150,8 +150,8 @@ const (
 	// Note this is only applicable to control plane machines.
 	ELBAttachedCondition clusterv1.ConditionType = "ELBAttached"
 
-	// ELBAttachFailedReason used when a control plane node fails to attach to the ELB
+	// ELBAttachFailedReason used when a control plane node fails to attach to the ELB.
 	ELBAttachFailedReason = "ELBAttachFailed"
-	// ELBDetachFailedReason used when a control plane node fails to detach from an ELB
+	// ELBDetachFailedReason used when a control plane node fails to detach from an ELB.
 	ELBDetachFailedReason = "ELBDetachFailed"
 )