🌱 Update golangci-lint to v1.40.1, enable importas and revive

Signed-off-by: Vince Prignano <[email protected]>

Author: vincepri

.github/workflows/golangci-lint.yml

@@ -14,4 +14,4 @@ jobs:
       - name: golangci-lint
         uses: golangci/golangci-lint-action@v2
         with:
-          version: v1.38
+          version: v1.40.1

.golangci.yml

@@ -11,18 +11,20 @@ linters:
   - goconst
   - gocritic
   - gocyclo
+  - godot
   - gofmt
   - goimports
-  - golint
   - goprintffuncname
   - gosec
   - gosimple
   - govet
+  - importas
   - ineffassign
   - misspell
   - nakedret
   - nolintlint
   - prealloc
+  - revive
   - rowserrcheck
   - staticcheck
   - structcheck
@@ -31,9 +33,31 @@ linters:
   - unconvert
   - unparam
   - varcheck
-  - godot
   - whitespace
 
+linters-settings:
+  staticcheck:
+    go: "1.16"
+  stylecheck:
+    go: "1.16"
+  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
+
 issues:
   max-same-issues: 0
   max-issues-per-linter: 0
@@ -44,30 +68,30 @@ issues:
   exclude:
   - "G108: Profiling endpoint is automatically exposed on /debug/pprof"
   - Error return value of .((os\.)?std(out|err)\..*|.*Close|.*Flush|os\.Remove(All)?|.*print(f|ln)?|os\.(Un)?Setenv). is not checked
-  - exported method `.*.(Reconcile|SetupWithManager|SetupWebhookWithManager)` should have comment or be unexported
+  - "exported: exported method .*\\.(Reconcile|SetupWithManager|SetupWebhookWithManager) should have comment or be unexported"
   # 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:
   # With Go 1.16, the new embed directive can be used with an un-named import,
-  # golint only allows these to be imported in a main.go, which wouldn't work for us.
+  # 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:
-    - golint
+    - revive
     source: _ "embed"
   # Exclude some packages or code to require comments, for example test code, or fake clients.
   - linters:
-    - golint
+    - revive
     text: exported (method|function|type|const) (.+) should have comment or be unexported
     source: (func|type).*Fake.*
   - linters:
-    - golint
+    - revive
     text: exported (method|function|type|const) (.+) should have comment or be unexported
     path: fake_\.go
   - linters:
-    - golint
+    - revive
     text: exported (method|function|type|const) (.+) should have comment or be unexported
     path: .*test/(providers|framework|e2e).*.go
   # Disable unparam "always receives" which might not be really
@@ -83,6 +107,10 @@ issues:
     text: cyclomatic complexity
   - path: test/(framework|e2e).*.go
     text: should not use dot imports
+  # 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

bootstrap/kubeadm/controllers/token.go

@@ -21,7 +21,7 @@ import (
 	"time"
 
 	"github.com/pkg/errors"
-	v1 "k8s.io/api/core/v1"
+	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	bootstrapapi "k8s.io/cluster-bootstrap/token/api"
 	bootstraputil "k8s.io/cluster-bootstrap/token/util"
@@ -48,7 +48,7 @@ func createToken(ctx context.Context, c client.Client) (string, error) {
 	tokenSecret := substrs[2]
 
 	secretName := bootstraputil.BootstrapTokenSecretName(tokenID)
-	secretToken := &v1.Secret{
+	secretToken := &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      secretName,
 			Namespace: metav1.NamespaceSystem,
@@ -72,15 +72,15 @@ func createToken(ctx context.Context, c client.Client) (string, error) {
 }
 
 // getToken fetches the token Secret and returns an error if it is invalid.
-func getToken(ctx context.Context, c client.Client, token string) (*v1.Secret, error) {
+func getToken(ctx context.Context, c client.Client, token string) (*corev1.Secret, error) {
 	substrs := bootstraputil.BootstrapTokenRegexp.FindStringSubmatch(token)
 	if len(substrs) != 3 {
 		return nil, errors.Errorf("the bootstrap token %q was not of the form %q", token, bootstrapapi.BootstrapTokenPattern)
 	}
 	tokenID := substrs[1]
 
 	secretName := bootstraputil.BootstrapTokenSecretName(tokenID)
-	secret := &v1.Secret{}
+	secret := &corev1.Secret{}
 	if err := c.Get(ctx, client.ObjectKey{Name: secretName, Namespace: metav1.NamespaceSystem}, secret); err != nil {
 		return secret, err
 	}

bootstrap/kubeadm/internal/locking/control_plane_init_mutex.go

@@ -24,7 +24,7 @@ import (
 
 	"github.com/go-logr/logr"
 	"github.com/pkg/errors"
-	apicorev1 "k8s.io/api/core/v1"
+	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha4"
@@ -133,11 +133,11 @@ type information struct {
 }
 
 type semaphore struct {
-	*apicorev1.ConfigMap
+	*corev1.ConfigMap
 }
 
 func newSemaphore() *semaphore {
-	return &semaphore{&apicorev1.ConfigMap{}}
+	return &semaphore{&corev1.ConfigMap{}}
 }
 
 func configMapName(clusterName string) string {

bootstrap/kubeadm/types/v1beta1/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"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
@@ -222,7 +222,7 @@ type NodeRegistrationOptions struct {
 	// it will be defaulted to []v1.Taint{'node-role.kubernetes.io/master=""'}. If you don't want to taint your control-plane node, set this field to an
 	// empty slice, i.e. `taints: {}` in the YAML file. This field is solely used for Node registration.
 	// +optional
-	Taints []v1.Taint `json:"taints,omitempty"`
+	Taints []corev1.Taint `json:"taints,omitempty"`
 
 	// KubeletExtraArgs passes through extra arguments to the kubelet. The arguments here are passed to the kubelet command line via the environment file
 	// kubeadm writes at runtime for the kubelet to source. This overrides the generic base-level configuration in the kubelet-config-1.X ConfigMap
@@ -422,5 +422,5 @@ type HostPathMount struct {
 	// ReadOnly controls write access to the volume
 	ReadOnly bool `json:"readOnly,omitempty"`
 	// PathType is the type of the HostPath.
-	PathType v1.HostPathType `json:"pathType,omitempty"`
+	PathType corev1.HostPathType `json:"pathType,omitempty"`
 }

cmd/clusterctl/client/cluster/cert_manager.go

@@ -177,11 +177,7 @@ func (cm *certManagerClient) install() error {
 	}
 
 	// Wait for the cert-manager API to be ready to accept requests
-	if err := cm.waitForAPIReady(ctx, true); err != nil {
-		return err
-	}
-
-	return nil
+	return cm.waitForAPIReady(ctx, true)
 }
 
 // PlanUpgrade retruns a CertManagerUpgradePlan with information regarding
@@ -438,11 +434,7 @@ func (cm *certManagerClient) deleteObj(obj unstructured.Unstructured) error {
 		return err
 	}
 
-	if err := cl.Delete(ctx, &obj); err != nil {
-		return err
-	}
-
-	return nil
+	return cl.Delete(ctx, &obj)
 }
 
 // waitForAPIReady will attempt to create the cert-manager 'test assets' (i.e. a basic

cmd/clusterctl/client/cluster/installer.go

@@ -114,11 +114,7 @@ func installComponentsAndUpdateInventory(components repository.Components, provi
 	}
 
 	log.V(1).Info("Creating inventory entry", "Provider", components.ManifestLabel(), "Version", components.Version(), "TargetNamespace", components.TargetNamespace())
-	if err := providerInventory.Create(inventoryObject); err != nil {
-		return err
-	}
-
-	return nil
+	return providerInventory.Create(inventoryObject)
 }
 
 // shouldInstallSharedComponents checks if it is required to install shared components for a provider.

cmd/clusterctl/client/cluster/mover.go

@@ -99,11 +99,7 @@ func (o *objectMover) Move(namespace string, toCluster Client, dryRun bool) erro
 		proxy = toCluster.Proxy()
 	}
 
-	if err := o.move(objectGraph, proxy); err != nil {
-		return err
-	}
-
-	return nil
+	return o.move(objectGraph, proxy)
 }
 
 func newObjectMover(fromProxy Proxy, fromProviderInventory InventoryClient) *objectMover {
@@ -250,11 +246,7 @@ func (o *objectMover) move(graph *objectGraph, toProxy Proxy) error {
 
 	// Reset the pause field on the Cluster object in the target management cluster, so the controllers start reconciling it.
 	log.V(1).Info("Resuming the target cluster")
-	if err := setClusterPause(toProxy, clusters, false, o.dryRun); err != nil {
-		return err
-	}
-
-	return nil
+	return setClusterPause(toProxy, clusters, false, o.dryRun)
 }
 
 // moveSequence defines a list of group of moveGroups.

cmd/clusterctl/client/config/reader_viper.go

@@ -148,7 +148,7 @@ func downloadFile(url string, filepath string) error {
 		return errors.Wrapf(err, "failed to download the clusterctl config file from %s", url)
 	}
 	if resp.StatusCode != http.StatusOK {
-		return errors.New(fmt.Sprintf("failed to download the clusterctl config file from %s got %d", url, resp.StatusCode))
+		return errors.Errorf("failed to download the clusterctl config file from %s got %d", url, resp.StatusCode)
 	}
 	defer resp.Body.Close()
 

cmd/clusterctl/client/init_test.go

@@ -781,7 +781,7 @@ func templateYAML(ns string, clusterName string) []byte {
 // infraComponentsYAML defines a namespace and deployment with container
 // images and a variable.
 func infraComponentsYAML(namespace string) []byte {
-	var infraComponentsYAML string = `---
+	var infraComponentsYAML = `---
 apiVersion: v1
 kind: Namespace
 metadata:

cmd/clusterctl/client/move.go

@@ -83,9 +83,5 @@ func (c *clusterctlClient) Move(options MoveOptions) error {
 		options.Namespace = currentNamespace
 	}
 
-	if err := fromCluster.ObjectMover().Move(options.Namespace, toCluster, options.DryRun); err != nil {
-		return err
-	}
-
-	return nil
+	return fromCluster.ObjectMover().Move(options.Namespace, toCluster, options.DryRun)
 }

cmd/clusterctl/client/upgrade.go

@@ -170,19 +170,11 @@ func (c *clusterctlClient) ApplyUpgrade(options ApplyUpgradeOptions) error {
 		}
 
 		// Execute the upgrade using the custom upgrade items
-		if err := clusterClient.ProviderUpgrader().ApplyCustomPlan(upgradeItems...); err != nil {
-			return err
-		}
-
-		return nil
+		return clusterClient.ProviderUpgrader().ApplyCustomPlan(upgradeItems...)
 	}
 
 	// Otherwise we are upgrading a whole management cluster according to a clusterctl generated upgrade plan.
-	if err := clusterClient.ProviderUpgrader().ApplyPlan(options.Contract); err != nil {
-		return err
-	}
-
-	return nil
+	return clusterClient.ProviderUpgrader().ApplyPlan(options.Contract)
 }
 
 func addUpgradeItems(upgradeItems []cluster.UpgradeItem, providerType clusterctlv1.ProviderType, providers ...string) ([]cluster.UpgradeItem, error) {

cmd/clusterctl/cmd/delete.go

@@ -127,7 +127,7 @@ func runDelete() error {
 		return errors.New("At least one of --core, --bootstrap, --control-plane, --infrastructure should be specified or the --all flag should be set")
 	}
 
-	if err := c.Delete(client.DeleteOptions{
+	return c.Delete(client.DeleteOptions{
 		Kubeconfig:              client.Kubeconfig{Path: dd.kubeconfig, Context: dd.kubeconfigContext},
 		IncludeNamespace:        dd.includeNamespace,
 		IncludeCRDs:             dd.includeCRDs,
@@ -136,9 +136,5 @@ func runDelete() error {
 		InfrastructureProviders: dd.infrastructureProviders,
 		ControlPlaneProviders:   dd.controlPlaneProviders,
 		DeleteAll:               dd.deleteAll,
-	}); err != nil {
-		return err
-	}
-
-	return nil
+	})
 }

cmd/clusterctl/cmd/move.go

@@ -78,13 +78,10 @@ func runMove() error {
 		return err
 	}
 
-	if err := c.Move(client.MoveOptions{
+	return c.Move(client.MoveOptions{
 		FromKubeconfig: client.Kubeconfig{Path: mo.fromKubeconfig, Context: mo.fromKubeconfigContext},
 		ToKubeconfig:   client.Kubeconfig{Path: mo.toKubeconfig, Context: mo.toKubeconfigContext},
 		Namespace:      mo.namespace,
 		DryRun:         mo.dryRun,
-	}); err != nil {
-		return err
-	}
-	return nil
+	})
 }

cmd/clusterctl/cmd/rollout/pause.go

@@ -74,12 +74,9 @@ func runPause(cfgFile string, args []string) error {
 		return err
 	}
 
-	if err := c.RolloutPause(client.RolloutOptions{
+	return c.RolloutPause(client.RolloutOptions{
 		Kubeconfig: client.Kubeconfig{Path: pauseOpt.kubeconfig, Context: pauseOpt.kubeconfigContext},
 		Namespace:  pauseOpt.namespace,
 		Resources:  pauseOpt.resources,
-	}); err != nil {
-		return err
-	}
-	return nil
+	})
 }

cmd/clusterctl/cmd/rollout/restart.go

@@ -72,12 +72,9 @@ func runRestart(cfgFile string, _ *cobra.Command, args []string) error {
 		return err
 	}
 
-	if err := c.RolloutRestart(client.RolloutOptions{
+	return c.RolloutRestart(client.RolloutOptions{
 		Kubeconfig: client.Kubeconfig{Path: restartOpt.kubeconfig, Context: restartOpt.kubeconfigContext},
 		Namespace:  restartOpt.namespace,
 		Resources:  restartOpt.resources,
-	}); err != nil {
-		return err
-	}
-	return nil
+	})
 }

cmd/clusterctl/cmd/rollout/resume.go

@@ -72,12 +72,9 @@ func runResume(cfgFile string, args []string) error {
 		return err
 	}
 
-	if err := c.RolloutResume(client.RolloutOptions{
+	return c.RolloutResume(client.RolloutOptions{
 		Kubeconfig: client.Kubeconfig{Path: resumeOpt.kubeconfig, Context: resumeOpt.kubeconfigContext},
 		Namespace:  resumeOpt.namespace,
 		Resources:  resumeOpt.resources,
-	}); err != nil {
-		return err
-	}
-	return nil
+	})
 }