Merge pull request #2133 from vincepri/add-more-linters2

🌱 Add more linters

.golangci.yml

@@ -1,12 +1,17 @@
 linters:
   disable-all: true
   enable:
+  - asasalint
   - asciicheck
+  - bidichk
   - bodyclose
   - depguard
   - dogsled
+  - dupl
   - errcheck
+  - errchkjson
   - errorlint
+  - exhaustive
   - exportloopref
   - goconst
   - gocritic
@@ -19,15 +24,16 @@ linters:
   - govet
   - importas
   - ineffassign
+  - makezero
   - misspell
   - nakedret
   - nilerr
   - nolintlint
   - prealloc
   - revive
-  - rowserrcheck
   - staticcheck
   - stylecheck
+  - tagliatelle
   - typecheck
   - unconvert
   - unparam
@@ -127,6 +133,9 @@ issues:
   - linters:
     - revive
     text: "package-comments: should have a package comment"
+  - linters:
+    - dupl
+    path: _test\.go
 
 run:
   timeout: 10m

example_test.go

@@ -138,7 +138,7 @@ func (a *ReplicaSetReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 
 	// Update the ReplicaSet
 	rs.Labels["pod-count"] = fmt.Sprintf("%v", len(pods.Items))
-	err = a.Update(context.TODO(), rs)
+	err = a.Update(ctx, rs)
 	if err != nil {
 		return ctrl.Result{}, err
 	}

pkg/builder/controller_test.go

@@ -634,7 +634,7 @@ func doReconcileTest(ctx context.Context, nameSuffix string, mgr manager.Manager
 			},
 		},
 	}
-	err := mgr.GetClient().Create(context.TODO(), dep)
+	err := mgr.GetClient().Create(ctx, dep)
 	Expect(err).NotTo(HaveOccurred())
 
 	By("Waiting for the Deployment Reconcile")
@@ -664,7 +664,7 @@ func doReconcileTest(ctx context.Context, nameSuffix string, mgr manager.Manager
 			Template: dep.Spec.Template,
 		},
 	}
-	err = mgr.GetClient().Create(context.TODO(), rs)
+	err = mgr.GetClient().Create(ctx, rs)
 	Expect(err).NotTo(HaveOccurred())
 
 	By("Waiting for the ReplicaSet Reconcile")

pkg/certwatcher/example_test.go

@@ -64,7 +64,9 @@ func Example() {
 	// Start goroutine for handling server shutdown.
 	go func() {
 		<-ctx.Done()
-		if err := srv.Shutdown(context.Background()); err != nil {
+		ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+		defer cancel()
+		if err := srv.Shutdown(ctx); err != nil {
 			panic(err)
 		}
 	}()

pkg/predicate/predicate_test.go

@@ -413,7 +413,6 @@ var _ = Describe("Predicate", func() {
 
 	// AnnotationChangedPredicate has almost identical test cases as LabelChangedPredicates,
 	// so the duplication linter should be muted on both two test suites.
-	//nolint:dupl
 	Describe("When checking an AnnotationChangedPredicate", func() {
 		instance := predicate.AnnotationChangedPredicate{}
 		Context("Where the old object is missing", func() {
@@ -612,7 +611,6 @@ var _ = Describe("Predicate", func() {
 
 	// LabelChangedPredicates has almost identical test cases as AnnotationChangedPredicates,
 	// so the duplication linter should be muted on both two test suites.
-	//nolint:dupl
 	Describe("When checking a LabelChangedPredicate", func() {
 		instance := predicate.LabelChangedPredicate{}
 		Context("Where the old object is missing", func() {

pkg/webhook/admission/admissiontest/util.go

@@ -27,7 +27,7 @@ import (
 // or passes if ErrorToReturn is nil.
 type FakeValidator struct {
 	// ErrorToReturn is the error for which the FakeValidator rejects all requests
-	ErrorToReturn error `json:"ErrorToReturn,omitempty"`
+	ErrorToReturn error `json:"errorToReturn,omitempty"`
 	// GVKToReturn is the GroupVersionKind that the webhook operates on
 	GVKToReturn schema.GroupVersionKind
 }

pkg/webhook/admission/validator_custom.go

@@ -71,6 +71,9 @@ func (h *validatorForType) Handle(ctx context.Context, req Request) Response {
 
 	var err error
 	switch req.Operation {
+	case v1.Connect:
+		// No validation for connect requests.
+		// TODO(vincepri): Should we validate CONNECT requests? In what cases?
 	case v1.Create:
 		if err := h.decoder.Decode(req, obj); err != nil {
 			return Errored(http.StatusBadRequest, err)

pkg/webhook/server.go

@@ -275,10 +275,11 @@ func (s *Server) Start(ctx context.Context) error {
 	idleConnsClosed := make(chan struct{})
 	go func() {
 		<-ctx.Done()
-		log.Info("shutting down webhook server")
+		log.Info("Shutting down webhook server with timeout of 1 minute")
 
-		// TODO: use a context with reasonable timeout
-		if err := srv.Shutdown(context.Background()); err != nil {
+		ctx, cancel := context.WithTimeout(context.Background(), 1*time.Minute)
+		defer cancel()
+		if err := srv.Shutdown(ctx); err != nil {
 			// Error from closing listeners, or context timeout
 			log.Error(err, "error shutting down the HTTP server")
 		}