Merge pull request #1855 from mboersma/sync-linters-returns

Add several code linters

.golangci.yml

@@ -8,30 +8,78 @@ run:
 linters:
   disable-all: true
   enable:
+    - asciicheck
+    - bodyclose
     - deadcode
+    - depguard
+    - dogsled
     - errcheck
     - errorlint
+    - exportloopref
     - goconst
     - gocyclo
     - godot
     - gofmt
     - goimports
+    - goprintffuncname
     - gosec
     - gosimple
     - govet
+    - importas
     - ineffassign
     - misspell
     - nakedret
+    - nilerr
+    - nolintlint
     - prealloc
     - revive
+    - rowserrcheck
     - staticcheck
     - structcheck
+    - typecheck
     - unconvert
     - unused
     - varcheck
     - whitespace
   # Run with --fast=false for more extensive checks
   fast: true
+
+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
+  godot:
+    #   declarations - for top level declaration comments (default);
+    #   toplevel     - for top level comments;
+    #   all          - for all comments.
+    scope: toplevel
+    exclude:
+    - '^ \+.*'
+    - '^ ANCHOR.*'
+  gosec:
+    excludes:
+    - G307 # Deferring unsafe method "Close" on type "\*os.File"
+    - G108 # Profiling endpoint is automatically exposed on /debug/pprof
+  staticcheck:
+    go: "1.16"
+
 issues:
   exclude-rules:
     - path: internal/test/

api/v1alpha3/azuremachine_types.go

@@ -17,7 +17,7 @@ limitations under the License.
 package v1alpha3
 
 import (
-	v1 "k8s.io/api/core/v1"
+	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3"
@@ -130,7 +130,7 @@ type AzureMachineStatus struct {
 	Ready bool `json:"ready"`
 
 	// Addresses contains the Azure instance associated addresses.
-	Addresses []v1.NodeAddress `json:"addresses,omitempty"`
+	Addresses []corev1.NodeAddress `json:"addresses,omitempty"`
 
 	// VMState is the provisioning state of the Azure virtual machine.
 	// +optional

api/v1alpha3/azuremachinetemplate_conversion.go

@@ -18,7 +18,6 @@ package v1alpha3
 
 import (
 	apimachineryconversion "k8s.io/apimachinery/pkg/conversion"
-	"sigs.k8s.io/cluster-api-provider-azure/api/v1beta1"
 	infrav1beta1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1"
 	utilconversion "sigs.k8s.io/cluster-api/util/conversion"
 	"sigs.k8s.io/controller-runtime/pkg/conversion"
@@ -93,7 +92,7 @@ func Convert_v1beta1_AzureSharedGalleryImage_To_v1alpha3_AzureSharedGalleryImage
 	return nil
 }
 
-func Convert_v1beta1_AzureMachineTemplateResource_To_v1alpha3_AzureMachineTemplateResource(in *v1beta1.AzureMachineTemplateResource, out *AzureMachineTemplateResource, s apimachineryconversion.Scope) error {
+func Convert_v1beta1_AzureMachineTemplateResource_To_v1alpha3_AzureMachineTemplateResource(in *infrav1beta1.AzureMachineTemplateResource, out *AzureMachineTemplateResource, s apimachineryconversion.Scope) error {
 	if err := autoConvert_v1beta1_AzureMachineTemplateResource_To_v1alpha3_AzureMachineTemplateResource(in, out, s); err != nil {
 		return err
 	}

api/v1alpha4/azuremachine_types.go

@@ -17,7 +17,7 @@ limitations under the License.
 package v1alpha4
 
 import (
-	v1 "k8s.io/api/core/v1"
+	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha4"
@@ -128,7 +128,7 @@ type AzureMachineStatus struct {
 	Ready bool `json:"ready"`
 
 	// Addresses contains the Azure instance associated addresses.
-	Addresses []v1.NodeAddress `json:"addresses,omitempty"`
+	Addresses []corev1.NodeAddress `json:"addresses,omitempty"`
 
 	// VMState is the provisioning state of the Azure virtual machine.
 	// +optional

api/v1beta1/azuremachine_types.go

@@ -17,7 +17,7 @@ limitations under the License.
 package v1beta1
 
 import (
-	v1 "k8s.io/api/core/v1"
+	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
@@ -131,7 +131,7 @@ type AzureMachineStatus struct {
 
 	// Addresses contains the Azure instance associated addresses.
 	// +optional
-	Addresses []v1.NodeAddress `json:"addresses,omitempty"`
+	Addresses []corev1.NodeAddress `json:"addresses,omitempty"`
 
 	// VMState is the provisioning state of the Azure virtual machine.
 	// +optional

azure/defaults_test.go

@@ -280,8 +280,9 @@ func TestMSCorrelationIDSendDecorator(t *testing.T) {
 	g.Expect(err).NotTo(HaveOccurred())
 
 	req = req.WithContext(ctx)
-	_, err = newSender.Do(req)
+	rsp, err := newSender.Do(req)
 	g.Expect(err).NotTo(HaveOccurred())
+	g.Expect(rsp.Body.Close()).To(Succeed())
 	wg.Wait()
 	g.Expect(len(receivedReqs)).To(Equal(1))
 	receivedReq := receivedReqs[0]

azure/services/scalesets/scalesets.go

@@ -544,6 +544,7 @@ func (s *Service) getVirtualMachineScaleSetIfDone(ctx context.Context, future *i
 func (s *Service) generateExtensions() []compute.VirtualMachineScaleSetExtension {
 	extensions := make([]compute.VirtualMachineScaleSetExtension, len(s.Scope.VMSSExtensionSpecs()))
 	for i, extensionSpec := range s.Scope.VMSSExtensionSpecs() {
+		extensionSpec := extensionSpec
 		extensions[i] = compute.VirtualMachineScaleSetExtension{
 			Name: &extensionSpec.Name,
 			VirtualMachineScaleSetExtensionProperties: &compute.VirtualMachineScaleSetExtensionProperties{

controllers/azuremachine_controller_test.go

@@ -23,7 +23,7 @@ import (
 	"github.com/Azure/go-autorest/autorest"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
-	v1 "k8s.io/api/core/v1"
+	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/tools/record"
@@ -101,7 +101,7 @@ func TestConditions(t *testing.T) {
 			},
 			expectedConditions: []clusterv1.Condition{{
 				Type:     "VMRunning",
-				Status:   v1.ConditionFalse,
+				Status:   corev1.ConditionFalse,
 				Severity: clusterv1.ConditionSeverityInfo,
 				Reason:   "WaitingForClusterInfrastructure",
 			}},
@@ -133,7 +133,7 @@ func TestConditions(t *testing.T) {
 			},
 			expectedConditions: []clusterv1.Condition{{
 				Type:     "VMRunning",
-				Status:   v1.ConditionFalse,
+				Status:   corev1.ConditionFalse,
 				Severity: clusterv1.ConditionSeverityInfo,
 				Reason:   "WaitingForBootstrapData",
 			}},

controllers/suite_test.go

@@ -22,13 +22,12 @@ import (
 
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
-	v1 "k8s.io/api/core/v1"
+	corev1 "k8s.io/api/core/v1"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
 	"sigs.k8s.io/controller-runtime/pkg/envtest/printer"
 
 	"sigs.k8s.io/cluster-api-provider-azure/internal/test/env"
 	"sigs.k8s.io/cluster-api-provider-azure/util/reconciler"
-	// +kubebuilder:scaffold:imports
 )
 
 // These tests use Ginkgo (BDD-style Go testing framework). Refer to
@@ -65,7 +64,7 @@ var _ = BeforeSuite(func(done Done) {
 	}()
 
 	Eventually(func() bool {
-		nodes := &v1.NodeList{}
+		nodes := &corev1.NodeList{}
 		if err := testEnv.Client.List(context.Background(), nodes); err != nil {
 			return false
 		}

exp/controllers/azuremachinepool_controller_unit_test.go

@@ -22,7 +22,7 @@ import (
 	"github.com/Azure/go-autorest/autorest/to"
 	"github.com/golang/mock/gomock"
 	. "github.com/onsi/gomega"
-	v1 "k8s.io/api/core/v1"
+	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
@@ -114,7 +114,7 @@ func newAzureMachinePool(clusterName, poolName string) *infrav1exp.AzureMachineP
 
 func newMachinePoolWithInfrastructureRef(clusterName, poolName string) *clusterv1exp.MachinePool {
 	m := newMachinePool(clusterName, poolName)
-	m.Spec.Template.Spec.InfrastructureRef = v1.ObjectReference{
+	m.Spec.Template.Spec.InfrastructureRef = corev1.ObjectReference{
 		Kind:       "AzureMachinePool",
 		Namespace:  m.Namespace,
 		Name:       "azure" + poolName,
@@ -125,7 +125,7 @@ func newMachinePoolWithInfrastructureRef(clusterName, poolName string) *clusterv
 
 func newManagedMachinePoolWithInfrastructureRef(clusterName, poolName string) *clusterv1exp.MachinePool {
 	m := newMachinePool(clusterName, poolName)
-	m.Spec.Template.Spec.InfrastructureRef = v1.ObjectReference{
+	m.Spec.Template.Spec.InfrastructureRef = corev1.ObjectReference{
 		Kind:       "AzureManagedMachinePool",
 		Namespace:  m.Namespace,
 		Name:       "azure" + poolName,

exp/controllers/suite_test.go

@@ -22,14 +22,13 @@ import (
 
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
-	v1 "k8s.io/api/core/v1"
+	corev1 "k8s.io/api/core/v1"
 	"sigs.k8s.io/cluster-api-provider-azure/controllers"
 	"sigs.k8s.io/cluster-api-provider-azure/util/reconciler"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
 	"sigs.k8s.io/controller-runtime/pkg/envtest/printer"
 
 	"sigs.k8s.io/cluster-api-provider-azure/internal/test/env"
-	// +kubebuilder:scaffold:imports
 )
 
 // These tests use Ginkgo (BDD-style Go testing framework). Refer to
@@ -81,7 +80,7 @@ var _ = BeforeSuite(func(done Done) {
 	}()
 
 	Eventually(func() bool {
-		nodes := &v1.NodeList{}
+		nodes := &corev1.NodeList{}
 		if err := testEnv.Client.List(context.Background(), nodes); err != nil {
 			return false
 		}

main.go

@@ -21,7 +21,7 @@ import (
 	"flag"
 	"fmt"
 	"net/http"
-	_ "net/http/pprof" //nolint
+	_ "net/http/pprof"
 	"os"
 	"time"
 

test/e2e/log.go

@@ -29,16 +29,16 @@ func nowStamp() string {
 	return time.Now().Format(time.StampMilli)
 }
 
-func log(level string, format string, args ...interface{}) {
+func logf(level string, format string, args ...interface{}) {
 	fmt.Fprintf(ginkgo.GinkgoWriter, nowStamp()+": "+level+": "+format+"\n", args...)
 }
 
 // Logf prints info logs with a timestamp and formatting.
 func Logf(format string, args ...interface{}) {
-	log("INFO", format, args...)
+	logf("INFO", format, args...)
 }
 
 // Log prints info logs with a timestamp.
 func Log(message string) {
-	log("INFO", message)
+	logf("INFO", message)
 }

util/webhook/validator.go

@@ -23,7 +23,6 @@ import (
 	"errors"
 
 	admissionv1 "k8s.io/api/admission/v1"
-	v1 "k8s.io/api/admission/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -71,7 +70,7 @@ func (h *validatingHandler) Handle(ctx context.Context, req admission.Request) a
 
 	// Get the object in the request
 	obj := h.validator.DeepCopyObject().(Validator)
-	if req.Operation == v1.Create {
+	if req.Operation == admissionv1.Create {
 		err := h.decoder.Decode(req, obj)
 		if err != nil {
 			return admission.Errored(http.StatusBadRequest, err)
@@ -87,7 +86,7 @@ func (h *validatingHandler) Handle(ctx context.Context, req admission.Request) a
 		}
 	}
 
-	if req.Operation == v1.Update {
+	if req.Operation == admissionv1.Update {
 		oldObj := obj.DeepCopyObject()
 
 		err := h.decoder.DecodeRaw(req.Object, obj)
@@ -109,7 +108,7 @@ func (h *validatingHandler) Handle(ctx context.Context, req admission.Request) a
 		}
 	}
 
-	if req.Operation == v1.Delete {
+	if req.Operation == admissionv1.Delete {
 		// In reference to PR: https://github.com/kubernetes/kubernetes/pull/76346
 		// OldObject contains the object being deleted
 		err := h.decoder.DecodeRaw(req.OldObject, obj)