Fix linter errors, update and move lint to config file

mcristina422 · mcristina422 · commit fd19c6a64e7b · 2020-02-11T11:42:47.000-06:00
diff --git a/.golangci.yml b/.golangci.yml
@@ -1,5 +1,27 @@
+linters:
+  disable-all: true
+  enable:
+    - misspell
+    - structcheck
+    - golint
+    - deadcode
+    - errcheck
+    - varcheck
+    - goconst
+    - unparam
+    - ineffassign
+    - nakedret
+    - interfacer
+    - gocyclo
+    - lll
+    - dupl
+    - goimports
+
 linters-settings:
   lll:
     line-length: 170
   dupl:
     threshold: 400
+
+run:
+  timeout: 5m
diff --git a/hack/check-everything.sh b/hack/check-everything.sh
@@ -78,7 +78,7 @@ function golangci_lint_has_version {
 
 function fetch_go_tools {
   header_text "Checking for golangci-lint"
-  local golangci_lint_version="v1.21.0"
+  local golangci_lint_version="v1.23.6"
   if ! is_installed golangci-lint || ! golangci_lint_has_version "${golangci_lint_version}"; then
     header_text "Installing golangci-lint"
     curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh| sh -s -- -b $(go env GOPATH)/bin "${golangci_lint_version}"
diff --git a/hack/verify.sh b/hack/verify.sh
@@ -24,30 +24,6 @@ go vet ${MOD_OPT} ./...
 
 header_text "running golangci-lint"
 
-golangci-lint run --disable-all \
-    --deadline 5m \
-    --enable=misspell \
-    --enable=structcheck \
-    --enable=golint \
-    --enable=deadcode \
-    --enable=errcheck \
-    --enable=varcheck \
-    --enable=goconst \
-    --enable=unparam \
-    --enable=ineffassign \
-    --enable=nakedret \
-    --enable=interfacer \
-    --enable=misspell \
-    --enable=gocyclo \
-    --enable=lll \
-    --enable=dupl \
-    --enable=goimports \
-    --enable=bodyclose \
-    ./pkg/... ./examples/... .
-
-# TODO: Enable these as we fix them to make them pass
-#    --enable=gosec \
-#    --enable=maligned \
-#    --enable=safesql \
+golangci-lint run ./pkg/... ./examples/... .
 
 GO111MODULES=on go list -mod=readonly ./...
diff --git a/pkg/client/client_cache.go b/pkg/client/client_cache.go
@@ -124,10 +124,8 @@ type resourceMeta struct {
 
 // isNamespaced returns true if the type is namespaced
 func (r *resourceMeta) isNamespaced() bool {
-	if r.mapping.Scope.Name() == meta.RESTScopeNameRoot {
-		return false
-	}
-	return true
+	return r.mapping.Scope.Name() != meta.RESTScopeNameRoot
+
 }
 
 // resource returns the resource name of the type
diff --git a/pkg/client/fake/client_test.go b/pkg/client/fake/client_test.go
@@ -17,6 +17,7 @@ limitations under the License.
 package fake
 
 import (
+	"context"
 	"encoding/json"
 
 	. "github.com/onsi/ginkgo"
@@ -85,7 +86,7 @@ var _ = Describe("Fake client", func() {
 				Namespace: "ns1",
 			}
 			obj := &appsv1.Deployment{}
-			err := cl.Get(nil, namespacedName, obj)
+			err := cl.Get(context.Background(), namespacedName, obj)
 			Expect(err).To(BeNil())
 			Expect(obj).To(Equal(dep))
 		})
@@ -99,14 +100,14 @@ var _ = Describe("Fake client", func() {
 			obj := &unstructured.Unstructured{}
 			obj.SetAPIVersion("apps/v1")
 			obj.SetKind("Deployment")
-			err := cl.Get(nil, namespacedName, obj)
+			err := cl.Get(context.Background(), namespacedName, obj)
 			Expect(err).To(BeNil())
 		})
 
 		It("should be able to List", func() {
 			By("Listing all deployments in a namespace")
 			list := &appsv1.DeploymentList{}
-			err := cl.List(nil, list, client.InNamespace("ns1"))
+			err := cl.List(context.Background(), list, client.InNamespace("ns1"))
 			Expect(err).To(BeNil())
 			Expect(list.Items).To(HaveLen(2))
 			Expect(list.Items).To(ConsistOf(*dep, *dep2))
@@ -117,15 +118,15 @@ var _ = Describe("Fake client", func() {
 			list := &unstructured.UnstructuredList{}
 			list.SetAPIVersion("apps/v1")
 			list.SetKind("DeploymentList")
-			err := cl.List(nil, list, client.InNamespace("ns1"))
+			err := cl.List(context.Background(), list, client.InNamespace("ns1"))
 			Expect(err).To(BeNil())
 			Expect(list.Items).To(HaveLen(2))
 		})
 
 		It("should support filtering by labels and their values", func() {
 			By("Listing deployments with a particular label and value")
 			list := &appsv1.DeploymentList{}
-			err := cl.List(nil, list, client.InNamespace("ns1"),
+			err := cl.List(context.Background(), list, client.InNamespace("ns1"),
 				client.MatchingLabels(map[string]string{
 					"test-label": "label-value",
 				}))
@@ -156,7 +157,7 @@ var _ = Describe("Fake client", func() {
 					Namespace: "ns2",
 				},
 			}
-			err := cl.Create(nil, newcm)
+			err := cl.Create(context.Background(), newcm)
 			Expect(err).To(BeNil())
 
 			By("Getting the new configmap")
@@ -165,7 +166,7 @@ var _ = Describe("Fake client", func() {
 				Namespace: "ns2",
 			}
 			obj := &corev1.ConfigMap{}
-			err = cl.Get(nil, namespacedName, obj)
+			err = cl.Get(context.Background(), namespacedName, obj)
 			Expect(err).To(BeNil())
 			Expect(obj).To(Equal(newcm))
 			Expect(obj.ObjectMeta.ResourceVersion).To(Equal("1"))
@@ -187,7 +188,7 @@ var _ = Describe("Fake client", func() {
 					"test-key": "new-value",
 				},
 			}
-			err := cl.Update(nil, newcm)
+			err := cl.Update(context.Background(), newcm)
 			Expect(err).To(BeNil())
 
 			By("Getting the new configmap")
@@ -196,33 +197,33 @@ var _ = Describe("Fake client", func() {
 				Namespace: "ns2",
 			}
 			obj := &corev1.ConfigMap{}
-			err = cl.Get(nil, namespacedName, obj)
+			err = cl.Get(context.Background(), namespacedName, obj)
 			Expect(err).To(BeNil())
 			Expect(obj).To(Equal(newcm))
 			Expect(obj.ObjectMeta.ResourceVersion).To(Equal("2"))
 		})
 
 		It("should be able to Delete", func() {
 			By("Deleting a deployment")
-			err := cl.Delete(nil, dep)
+			err := cl.Delete(context.Background(), dep)
 			Expect(err).To(BeNil())
 
 			By("Listing all deployments in the namespace")
 			list := &appsv1.DeploymentList{}
-			err = cl.List(nil, list, client.InNamespace("ns1"))
+			err = cl.List(context.Background(), list, client.InNamespace("ns1"))
 			Expect(err).To(BeNil())
 			Expect(list.Items).To(HaveLen(1))
 			Expect(list.Items).To(ConsistOf(*dep2))
 		})
 
 		It("should be able to Delete a Collection", func() {
 			By("Deleting a deploymentList")
-			err := cl.DeleteAllOf(nil, &appsv1.Deployment{}, client.InNamespace("ns1"))
+			err := cl.DeleteAllOf(context.Background(), &appsv1.Deployment{}, client.InNamespace("ns1"))
 			Expect(err).To(BeNil())
 
 			By("Listing all deployments in the namespace")
 			list := &appsv1.DeploymentList{}
-			err = cl.List(nil, list, client.InNamespace("ns1"))
+			err = cl.List(context.Background(), list, client.InNamespace("ns1"))
 			Expect(err).To(BeNil())
 			Expect(list.Items).To(BeEmpty())
 		})
@@ -236,7 +237,7 @@ var _ = Describe("Fake client", func() {
 						Namespace: "ns2",
 					},
 				}
-				err := cl.Create(nil, newcm, client.CreateDryRunAll)
+				err := cl.Create(context.Background(), newcm, client.DryRunAll)
 				Expect(err).To(BeNil())
 
 				By("Getting the new configmap")
@@ -245,7 +246,7 @@ var _ = Describe("Fake client", func() {
 					Namespace: "ns2",
 				}
 				obj := &corev1.ConfigMap{}
-				err = cl.Get(nil, namespacedName, obj)
+				err = cl.Get(context.Background(), namespacedName, obj)
 				Expect(err).To(HaveOccurred())
 				Expect(errors.IsNotFound(err)).To(BeTrue())
 				Expect(obj).NotTo(Equal(newcm))
@@ -263,7 +264,7 @@ var _ = Describe("Fake client", func() {
 						"test-key": "new-value",
 					},
 				}
-				err := cl.Update(nil, newcm, client.UpdateDryRunAll)
+				err := cl.Update(context.Background(), newcm, client.DryRunAll)
 				Expect(err).To(BeNil())
 
 				By("Getting the new configmap")
@@ -272,7 +273,7 @@ var _ = Describe("Fake client", func() {
 					Namespace: "ns2",
 				}
 				obj := &corev1.ConfigMap{}
-				err = cl.Get(nil, namespacedName, obj)
+				err = cl.Get(context.Background(), namespacedName, obj)
 				Expect(err).To(BeNil())
 				Expect(obj).To(Equal(cm))
 				Expect(obj.ObjectMeta.ResourceVersion).To(Equal(""))
@@ -289,7 +290,7 @@ var _ = Describe("Fake client", func() {
 				},
 			})
 			Expect(err).NotTo(HaveOccurred())
-			err = cl.Patch(nil, dep, client.RawPatch(types.StrategicMergePatchType, mergePatch))
+			err = cl.Patch(context.Background(), dep, client.RawPatch(types.StrategicMergePatchType, mergePatch))
 			Expect(err).NotTo(HaveOccurred())
 
 			By("Getting the patched deployment")
@@ -298,7 +299,7 @@ var _ = Describe("Fake client", func() {
 				Namespace: "ns1",
 			}
 			obj := &appsv1.Deployment{}
-			err = cl.Get(nil, namespacedName, obj)
+			err = cl.Get(context.Background(), namespacedName, obj)
 			Expect(err).NotTo(HaveOccurred())
 			Expect(obj.Annotations["foo"]).To(Equal("bar"))
 			Expect(obj.ObjectMeta.ResourceVersion).To(Equal("1"))
diff --git a/pkg/controller/controller_integration_test.go b/pkg/controller/controller_integration_test.go
@@ -122,7 +122,7 @@ var _ = Describe("controller", func() {
 			By("Invoking Reconciling for Update")
 			newDeployment := deployment.DeepCopy()
 			newDeployment.Labels = map[string]string{"foo": "bar"}
-			newDeployment, err = clientset.AppsV1().Deployments("default").Update(newDeployment)
+			_, err = clientset.AppsV1().Deployments("default").Update(newDeployment)
 			Expect(err).NotTo(HaveOccurred())
 			Expect(<-reconciled).To(Equal(expectedReconcileRequest))
 
@@ -152,7 +152,7 @@ var _ = Describe("controller", func() {
 			By("Invoking Reconciling for an OwnedObject when it is updated")
 			newReplicaset := replicaset.DeepCopy()
 			newReplicaset.Labels = map[string]string{"foo": "bar"}
-			newReplicaset, err = clientset.AppsV1().ReplicaSets("default").Update(newReplicaset)
+			_, err = clientset.AppsV1().ReplicaSets("default").Update(newReplicaset)
 			Expect(err).NotTo(HaveOccurred())
 			Expect(<-reconciled).To(Equal(expectedReconcileRequest))
 
diff --git a/pkg/handler/eventhandler_test.go b/pkg/handler/eventhandler_test.go
@@ -42,7 +42,6 @@ var _ = Describe("Eventhandler", func() {
 	t := true
 	BeforeEach(func() {
 		q = controllertest.Queue{Interface: workqueue.New()}
-		instance = handler.EnqueueRequestForObject{}
 		pod = &corev1.Pod{
 			ObjectMeta: metav1.ObjectMeta{Namespace: "biz", Name: "baz"},
 		}
diff --git a/pkg/internal/controller/controller.go b/pkg/internal/controller/controller.go
@@ -236,7 +236,7 @@ func (c *Controller) reconcileHandler(obj interface{}) bool {
 	// Update metrics after processing each item
 	reconcileStartTS := time.Now()
 	defer func() {
-		c.updateMetrics(time.Now().Sub(reconcileStartTS))
+		c.updateMetrics(time.Since(reconcileStartTS))
 	}()
 
 	var req reconcile.Request
diff --git a/pkg/leaderelection/fake/leader_election.go b/pkg/leaderelection/fake/leader_election.go
@@ -81,7 +81,7 @@ func (f *ResourceLock) Update(ler resourcelock.LeaderElectionRecord) error {
 
 // RecordEvent implements the ResourceLockInterface.
 func (f *ResourceLock) RecordEvent(s string) {
-	return
+
 }
 
 // Identity implements the ResourceLockInterface.
diff --git a/pkg/log/zap/zap_test.go b/pkg/log/zap/zap_test.go
@@ -58,6 +58,8 @@ type fakeLoggerRoot struct {
 	messages []logInfo
 }
 
+var _ logr.Logger = &fakeLogger{}
+
 // fakeLogger is a fake implementation of logr.Logger that records
 // messages, tags, and names,
 // just records the name.
diff --git a/pkg/manager/internal.go b/pkg/manager/internal.go
@@ -360,11 +360,9 @@ func (cm *controllerManager) serveMetrics(stop <-chan struct{}) {
 	}()
 
 	// Shutdown the server when stop is closed
-	select {
-	case <-stop:
-		if err := server.Shutdown(context.Background()); err != nil {
-			cm.errSignal.SignalError(err)
-		}
+	<-stop
+	if err := server.Shutdown(context.Background()); err != nil {
+		cm.errSignal.SignalError(err)
 	}
 }
 
@@ -392,11 +390,9 @@ func (cm *controllerManager) serveHealthProbes(stop <-chan struct{}) {
 	cm.mu.Unlock()
 
 	// Shutdown the server when stop is closed
-	select {
-	case <-stop:
-		if err := server.Shutdown(context.Background()); err != nil {
-			cm.errSignal.SignalError(err)
-		}
+	<-stop
+	if err := server.Shutdown(context.Background()); err != nil {
+		cm.errSignal.SignalError(err)
 	}
 }
 
diff --git a/pkg/manager/signals/signal_test.go b/pkg/manager/signals/signal_test.go
@@ -36,7 +36,7 @@ var _ = Describe("runtime signal", func() {
 			task := &Task{
 				ticker: time.NewTicker(time.Second * 2),
 			}
-			c := make(chan os.Signal)
+			c := make(chan os.Signal, 1)
 			signal.Notify(c, os.Interrupt)
 			task.wg.Add(1)
 			go func(c chan os.Signal) {
diff --git a/pkg/reconcile/example_test.go b/pkg/reconcile/example_test.go
@@ -26,7 +26,6 @@ import (
 
 // This example implements a simple no-op reconcile function that prints the object to be Reconciled.
 func ExampleFunc() {
-	type Reconciler struct{}
 
 	r := reconcile.Func(func(o reconcile.Request) (reconcile.Result, error) {
 		// Create your business logic to create, update, delete objects here.
diff --git a/pkg/webhook/server.go b/pkg/webhook/server.go
@@ -119,7 +119,7 @@ func (s *Server) Register(path string, hook http.Handler) {
 func instrumentedHook(path string, hookRaw http.Handler) http.Handler {
 	return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) {
 		startTS := time.Now()
-		defer func() { metrics.RequestLatency.WithLabelValues(path).Observe(time.Now().Sub(startTS).Seconds()) }()
+		defer func() { metrics.RequestLatency.WithLabelValues(path).Observe(time.Since(startTS).Seconds()) }()
 		hookRaw.ServeHTTP(resp, req)
 
 		// TODO(directxman12): add back in metric about total requests broken down by result?

Original file line number	Diff line number	Diff line change
`@@ -42,7 +42,6 @@ var _ = Describe("Eventhandler", func() {`
`42`	`42`	`t := true`
`43`	`43`	`BeforeEach(func() {`
`44`	`44`	`q = controllertest.Queue{Interface: workqueue.New()}`
`45`		`- instance = handler.EnqueueRequestForObject{}`
`46`	`45`	`pod = &corev1.Pod{`
`47`	`46`	`ObjectMeta: metav1.ObjectMeta{Namespace: "biz", Name: "baz"},`
`48`	`47`	`}`
Original file line number	Diff line number	Diff line change
`@@ -81,7 +81,7 @@ func (f *ResourceLock) Update(ler resourcelock.LeaderElectionRecord) error {`
`81`	`81`
`82`	`82`	`// RecordEvent implements the ResourceLockInterface.`
`83`	`83`	`func (f *ResourceLock) RecordEvent(s string) {`
`84`		`- return`
	`84`	`+`
`85`	`85`	`}`
`86`	`86`
`87`	`87`	`// Identity implements the ResourceLockInterface.`
Original file line number	Diff line number	Diff line change
`@@ -58,6 +58,8 @@ type fakeLoggerRoot struct {`
`58`	`58`	`messages []logInfo`
`59`	`59`	`}`
`60`	`60`
	`61`	`+var _ logr.Logger = &fakeLogger{}`
	`62`	`+`
`61`	`63`	`// fakeLogger is a fake implementation of logr.Logger that records`
`62`	`64`	`// messages, tags, and names,`
`63`	`65`	`// just records the name.`