Add CI linter

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

.golangci.yml

@@ -0,0 +1,21 @@
+linters:
+  enable:
+    - govet
+    - gofmt
+    - structcheck
+    - varcheck
+    - interfacer
+    - unconvert
+    - ineffassign
+    - goconst
+    - gocyclo
+    - misspell
+    - nakedret
+    - prealloc
+    - gosec
+  disable-all: true
+  # Run with --fast=false for more extensive checks
+  fast: true
+issue:
+  max-same-issues: 0
+  max-per-linter: 0

BUILD.bazel

@@ -1,7 +1,20 @@
 load("@bazel_gazelle//:def.bzl", "gazelle")
+load("//build:run_in_workspace_with_goroot.bzl", "workspace_binary")
 
 # gazelle:prefix sigs.k8s.io/cluster-api
 gazelle(
     name = "gazelle",
     external = "vendored",
 )
+
+workspace_binary(
+    name = "lint",
+    args = ["run"],
+    cmd = "@com_github_golangci_golangci-lint//cmd/golangci-lint",
+)
+
+workspace_binary(
+    name = "lint-full",
+    args = ["run --fast=false"],
+    cmd = "@com_github_golangci_golangci-lint//cmd/golangci-lint",
+)

Makefile

@@ -72,6 +72,13 @@ fmt: ## Run go fmt against code
 vet: ## Run go vet against code
 	go vet ./pkg/... ./cmd/...
 
+.PHONY: lint
+lint: dep-ensure ## Lint codebase
+	bazel run //:lint $(BAZEL_ARGS)
+
+lint-full: dep-ensure ## Run slower linters to detect possible issues
+	bazel run //:lint-full $(BAZEL_ARGS)
+
 .PHONY: generate
 generate: clientset dep-ensure ## Generate code
 	go generate ./pkg/... ./cmd/...
@@ -92,6 +99,10 @@ clientset: ## Generate a typed clientset
 		--output-package sigs.k8s.io/cluster-api/pkg/client/informers_generated \
 		--go-header-file=./hack/boilerplate.go.txt
 
+.PHONY: clean
+clean: ## Remove all generated files
+	rm -f bazel-*
+
 .PHONY: docker-build
 docker-build: generate fmt vet manifests ## Build the docker image
 	docker build . -t ${IMG}

WORKSPACE

@@ -1,4 +1,5 @@
 load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
+load("@bazel_tools//tools/build_defs/repo:git.bzl", "git_repository")
 
 http_archive(
     name = "io_bazel_rules_go",
@@ -29,3 +30,11 @@ go_repository(
     commit = "58492e2d83c59ed63881311f46ad6251f77dabc3",
     importpath = "sigs.k8s.io/kustomize",
 )
+
+go_repository(
+    name = "com_github_golangci_golangci-lint",
+    build_file_generation = "on",
+    importpath = "github.com/golangci/golangci-lint",
+    strip_prefix = "golangci-lint-4570c043a9b56ccf0372cb6ba7a8b645d92b3357",
+    urls = ["https://github.com/golangci/golangci-lint/archive/4570c043a9b56ccf0372cb6ba7a8b645d92b3357.zip"],
+)

build/run_in_workspace_with_goroot.bzl

@@ -0,0 +1,103 @@
+# Copyright 2018 The Kubernetes Authors.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# 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.
+
+# This is a modified version of the same rule from kubernetes/repo-infra
+# modified to add the GO SDK to the PATH environment variable.
+
+# Writes out a script which saves the runfiles directory,
+# changes to the workspace root, and then runs a command.
+
+def _workspace_binary_script_impl(ctx):
+    content = """#!/usr/bin/env bash
+set -o errexit
+set -o nounset
+set -o pipefail
+GOBINDIR=$(dirname $(readlink {go_bin}))
+export PATH=$GOBINDIR:$PATH
+BASE=$(pwd)
+cd $(dirname $(readlink {root_file}))
+"$BASE/{cmd}" $@
+""".format(
+        cmd = ctx.file.cmd.short_path,
+        root_file = ctx.file.root_file.short_path,
+        go_bin = ctx.file.go_bin.short_path,
+    )
+    ctx.actions.write(
+        output = ctx.outputs.executable,
+        content = content,
+        is_executable = True,
+    )
+    runfiles = ctx.runfiles(
+        files = [
+            ctx.file.cmd,
+            ctx.file.go_bin,
+            ctx.file.root_file,
+        ],
+    )
+    return [DefaultInfo(runfiles = runfiles)]
+
+_workspace_binary_script = rule(
+    attrs = {
+        "cmd": attr.label(
+            mandatory = True,
+            allow_files = True,
+            single_file = True,
+        ),
+        "root_file": attr.label(
+            mandatory = True,
+            allow_files = True,
+            single_file = True,
+        ),
+        "go_bin": attr.label(
+            mandatory = True,
+            allow_files = True,
+            single_file = True,
+        ),
+    },
+    executable = True,
+    implementation = _workspace_binary_script_impl,
+)
+
+# Wraps a binary to be run in the workspace root via bazel run.
+#
+# For example, one might do something like
+#
+# workspace_binary(
+#     name = "dep",
+#     cmd = "//vendor/github.com/golang/dep/cmd/dep",
+# )
+#
+# which would allow running dep with bazel run.
+def workspace_binary(
+        name,
+        cmd,
+        args = None,
+        visibility = None,
+        go_bin = "@go_sdk//:bin/go",
+        root_file = "//:WORKSPACE"):
+    script_name = name + "_script"
+    _workspace_binary_script(
+        name = script_name,
+        cmd = cmd,
+        root_file = root_file,
+        go_bin = go_bin,
+        tags = ["manual"],
+    )
+    native.sh_binary(
+        name = name,
+        srcs = [":" + script_name],
+        args = args,
+        visibility = visibility,
+        tags = ["manual"],
+    )

cmd/clusterctl/clientcmd/configutil.go

@@ -42,10 +42,10 @@ func NewCoreClientSetForDefaultSearchPath(kubeconfigPath string, overrides clien
 	return kubernetes.NewForConfig(config)
 }
 
-// NewClusterApiClientForDefaultSearchPath creates a Cluster API clientset. If the kubeconfigPath is specified then the configuration is loaded from that path.
+// NewClusterAPIClientForDefaultSearchPath creates a Cluster API clientset. If the kubeconfigPath is specified then the configuration is loaded from that path.
 // Otherwise the default kubeconfig search path is used.
 // The overrides parameter is used to select a specific context of the config, for example, select the context with a given cluster name or namespace.
-func NewClusterApiClientForDefaultSearchPath(kubeconfigPath string, overrides clientcmd.ConfigOverrides) (*clientset.Clientset, error) {
+func NewClusterAPIClientForDefaultSearchPath(kubeconfigPath string, overrides clientcmd.ConfigOverrides) (*clientset.Clientset, error) {
 	config, err := newRestConfigForDefaultSearchPath(kubeconfigPath, overrides)
 	if err != nil {
 		return nil, err
@@ -72,16 +72,16 @@ func newRestConfigForDefaultSearchPath(kubeconfigPath string, overrides clientcm
 			return config, nil
 		}
 	}
-	apiConfig, err := newApiConfigForDefaultSearchPath(kubeconfigPath)
+	apiConfig, err := newAPIConfigForDefaultSearchPath(kubeconfigPath)
 	if err != nil {
 		return nil, err
 	}
 	return newRestConfig(apiConfig, overrides)
 }
 
-// newApiConfigForDefaultSearchPath creates an api.Config by searching for the kubeconfig on the default search path. If an override 'kubeconfigPath' is
+// newAPIConfigForDefaultSearchPath creates an api.Config by searching for the kubeconfig on the default search path. If an override 'kubeconfigPath' is
 // given then that path is used instead of the default path.
-func newApiConfigForDefaultSearchPath(kubeconfigPath string) (*api.Config, error) {
+func newAPIConfigForDefaultSearchPath(kubeconfigPath string) (*api.Config, error) {
 	configLoader := clientcmd.NewDefaultClientConfigLoadingRules()
 	configLoader.ExplicitPath = kubeconfigPath
 	return configLoader.Load()

cmd/clusterctl/clusterdeployer/bootstrap/existing/existingcluster_test.go

@@ -61,6 +61,8 @@ func createTempFile(contents string) (string, error) {
 		return "", err
 	}
 	defer f.Close()
-	f.WriteString(contents)
+	if _, err := f.WriteString(contents); err != nil {
+		return "", err
+	}
 	return f.Name(), nil
 }

cmd/clusterctl/clusterdeployer/bootstrap/kind/BUILD.bazel

@@ -5,11 +5,15 @@ go_library(
     srcs = ["kind.go"],
     importpath = "sigs.k8s.io/cluster-api/cmd/clusterctl/clusterdeployer/bootstrap/kind",
     visibility = ["//visibility:public"],
-    deps = ["//vendor/k8s.io/klog:go_default_library"],
+    deps = [
+        "//vendor/github.com/pkg/errors:go_default_library",
+        "//vendor/k8s.io/klog:go_default_library",
+    ],
 )
 
 go_test(
     name = "go_default_test",
     srcs = ["kind_test.go"],
     embed = [":go_default_library"],
+    deps = ["//vendor/github.com/pkg/errors:go_default_library"],
 )

cmd/clusterctl/clusterdeployer/bootstrap/kind/kind.go

@@ -27,8 +27,7 @@ import (
 )
 
 const (
-	kubeconfigEnvVar = "KUBECONFIG"
-	kindClusterName  = "clusterapi"
+	kindClusterName = "clusterapi"
 )
 
 type Kind struct {

cmd/clusterctl/clusterdeployer/bootstrap/kind/kind_test.go

@@ -121,6 +121,8 @@ func createTempFile(contents string) (string, error) {
 		return "", err
 	}
 	defer f.Close()
-	f.WriteString(contents)
+	if _, err := f.WriteString(contents); err != nil {
+		return "", err
+	}
 	return f.Name(), nil
 }

cmd/clusterctl/clusterdeployer/bootstrap/minikube/minikube_test.go

@@ -147,6 +147,8 @@ func createTempFile(contents string) (string, error) {
 		return "", err
 	}
 	defer f.Close()
-	f.WriteString(contents)
+	if _, err := f.WriteString(contents); err != nil {
+		return "", err
+	}
 	return f.Name(), nil
 }

cmd/clusterctl/clusterdeployer/bootstrap/provisioner.go

@@ -41,6 +41,6 @@ func Get(o Options) (ClusterProvisioner, error) {
 			return existing.NewExistingCluster(o.KubeConfig)
 		}
 
-		return nil, errors.New("no bootstrap provisioner specified, you can specify `--bootstrap-cluster-kubeconfig` to use an existing Kubernetes cluster or `--bootstrap-type` to use a built-in ephemeral cluster.")
+		return nil, errors.New("no bootstrap provisioner specified, you can specify `--bootstrap-cluster-kubeconfig` to use an existing Kubernetes cluster or `--bootstrap-type` to use a built-in ephemeral cluster")
 	}
 }

cmd/clusterctl/clusterdeployer/clusterclient/clusterclient.go

@@ -143,7 +143,7 @@ func (c *client) DeleteNamespace(namespaceName string) error {
 // NewFromDefaultSearchPath creates and returns a Client.  The kubeconfigFile argument is expected to be the path to a
 // valid kubeconfig file.
 func NewFromDefaultSearchPath(kubeconfigFile string, overrides tcmd.ConfigOverrides) (*client, error) {
-	c, err := clientcmd.NewClusterApiClientForDefaultSearchPath(kubeconfigFile, overrides)
+	c, err := clientcmd.NewClusterAPIClientForDefaultSearchPath(kubeconfigFile, overrides)
 	if err != nil {
 		return nil, err
 	}

cmd/clusterctl/clusterdeployer/clusterdeployer_test.go

@@ -301,7 +301,7 @@ func (c *testClusterClient) DeleteNamespace(namespaceName string) error {
 		return nil
 	}
 
-	var ns []string
+	ns := make([]string, 0, len(c.namespaces))
 	for _, n := range c.namespaces {
 		if n == namespaceName {
 			continue
@@ -1210,15 +1210,15 @@ func generateTestNodeMachine(name string) *clusterv1.Machine {
 }
 
 func generateTestControlPlaneMachines(controlPlaneNames []string) []*clusterv1.Machine {
-	var controlPlanes []*clusterv1.Machine
+	controlPlanes := make([]*clusterv1.Machine, 0, len(controlPlaneNames))
 	for _, mn := range controlPlaneNames {
 		controlPlanes = append(controlPlanes, generateTestControlPlaneMachine(mn))
 	}
 	return controlPlanes
 }
 
 func generateTestNodeMachines(nodeNames []string) []*clusterv1.Machine {
-	var nodes []*clusterv1.Machine
+	nodes := make([]*clusterv1.Machine, 0, len(nodeNames))
 	for _, nn := range nodeNames {
 		nodes = append(nodes, generateTestNodeMachine(nn))
 	}
@@ -1247,8 +1247,8 @@ func generateMachines() []*clusterv1.Machine {
 
 func newMachineSetsFixture() []*clusterv1.MachineSet {
 	return []*clusterv1.MachineSet{
-		&clusterv1.MachineSet{ObjectMeta: metav1.ObjectMeta{Name: "machine-set-name-1"}},
-		&clusterv1.MachineSet{ObjectMeta: metav1.ObjectMeta{Name: "machine-set-name-2"}},
+		{ObjectMeta: metav1.ObjectMeta{Name: "machine-set-name-1"}},
+		{ObjectMeta: metav1.ObjectMeta{Name: "machine-set-name-2"}},
 	}
 }
 
@@ -1267,15 +1267,15 @@ func getClustersForNamespace(namespace string, count int) []*clusterv1.Cluster {
 
 func newMachineDeploymentsFixture() []*clusterv1.MachineDeployment {
 	return []*clusterv1.MachineDeployment{
-		&clusterv1.MachineDeployment{ObjectMeta: metav1.ObjectMeta{Name: "machine-deployment-name-1"}},
-		&clusterv1.MachineDeployment{ObjectMeta: metav1.ObjectMeta{Name: "machine-deployment-name-2"}},
+		{ObjectMeta: metav1.ObjectMeta{Name: "machine-deployment-name-1"}},
+		{ObjectMeta: metav1.ObjectMeta{Name: "machine-deployment-name-2"}},
 	}
 }
 
 func newClustersFixture() []*clusterv1.Cluster {
 	return []*clusterv1.Cluster{
-		&clusterv1.Cluster{ObjectMeta: metav1.ObjectMeta{Name: "cluster-name-1"}},
-		&clusterv1.Cluster{ObjectMeta: metav1.ObjectMeta{Name: "cluster-name-2"}},
+		{ObjectMeta: metav1.ObjectMeta{Name: "cluster-name-1"}},
+		{ObjectMeta: metav1.ObjectMeta{Name: "cluster-name-2"}},
 	}
 }
 

cmd/clusterctl/validation/validate_cluster_api_objects.go

@@ -50,7 +50,7 @@ func ValidateClusterAPIObjects(w io.Writer, c client.Client, clusterName string,
 	return validateMachineObjects(w, machines, c)
 }
 
-func getClusterObject(c client.Client, clusterName string, namespace string) (*v1alpha1.Cluster, error) {
+func getClusterObject(c client.Reader, clusterName string, namespace string) (*v1alpha1.Cluster, error) {
 	if clusterName != "" {
 		cluster := &clusterv1alpha1.Cluster{}
 		err := c.Get(context.TODO(), types.NamespacedName{Name: clusterName, Namespace: namespace}, cluster)
@@ -97,7 +97,7 @@ func validateMachineObjects(w io.Writer, machines *v1alpha1.MachineList, client
 func validateMachineObject(w io.Writer, machine v1alpha1.Machine, client client.Client) bool {
 	fmt.Fprintf(w, "Checking machine object %q... ", machine.Name)
 	if machine.Status.ErrorReason != nil || machine.Status.ErrorMessage != nil {
-		var reason common.MachineStatusError = ""
+		var reason common.MachineStatusError
 		if machine.Status.ErrorReason != nil {
 			reason = *machine.Status.ErrorReason
 		}

pkg/controller/machinedeployment/controller.go

@@ -112,8 +112,8 @@ func (r *ReconcileMachineDeployment) getMachineSetsForDeployment(d *v1alpha1.Mac
 
 	// TODO: flush out machine set adoption.
 
-	var filteredMS []*v1alpha1.MachineSet
-	for idx, _ := range machineSets.Items {
+	filteredMS := make([]*v1alpha1.MachineSet, 0, len(machineSets.Items))
+	for idx := range machineSets.Items {
 		ms := &machineSets.Items[idx]
 		if metav1.GetControllerOf(ms) == nil || (metav1.GetControllerOf(ms) != nil && !metav1.IsControlledBy(ms, d)) {
 			klog.V(4).Infof("%s not controlled by %v", ms.Name, d.Name)
@@ -220,7 +220,7 @@ func (r *ReconcileMachineDeployment) getMachineDeploymentsForMachineSet(ms *v1al
 		return nil
 	}
 
-	var deployments []*v1alpha1.MachineDeployment
+	deployments := make([]*v1alpha1.MachineDeployment, 0, len(dList.Items))
 	for idx, d := range dList.Items {
 		selector, err := metav1.LabelSelectorAsSelector(&d.Spec.Selector)
 		if err != nil {