Merge pull request kubernetes-retired#169 from nobbs/refactor/tools-staticcheck

Add `staticcheck` to the make build process

Author: k8s-ci-robot

Makefile

@@ -80,6 +80,8 @@ endif
 
 CONTROLLER_GEN ?= ${CURDIR}/bin/controller-gen
 
+STATICCHECK ?= ${CURDIR}/bin/staticcheck
+
 # Get the currently used golang install path (in GOPATH/bin, unless GOBIN is set)
 GOBIN ?= $(shell go env GOPATH)/bin
 
@@ -106,7 +108,7 @@ test-only:
 	go test ${DIRS} -coverprofile cover.out
 
 # Builds all binaries (manager and kubectl) and manifests
-build: generate fmt vet manifests
+build: generate fmt vet staticcheck manifests
 	go build -o bin/manager ./cmd/manager/main.go
 	GOOS=linux GOARCH=amd64 go build \
 	     -o bin/kubectl/kubectl-hns_linux_amd64 \
@@ -199,6 +201,13 @@ check-fmt:
 vet:
 	go vet ${DIRS}
 
+# Run staticcheck against code
+staticcheck: build-staticcheck
+	$(STATICCHECK) ${DIRS}
+
+build-staticcheck:
+	go build -o $(STATICCHECK) honnef.co/go/tools/cmd/staticcheck
+
 # Generate code
 generate: controller-gen
 	$(CONTROLLER_GEN) object:headerFile=./hack/boilerplate.go.txt paths=./api/...
@@ -265,21 +274,21 @@ else
 endif
 
 # Build the docker image
-docker-build: generate fmt vet
+docker-build: generate fmt vet staticcheck
 	@echo "Warning: this does not run tests. Run 'make test' to ensure tests are passing."
 	docker build . -t ${HNC_IMG}
 
 
 buildx-setup:
 	## This script needs to be run to start the emulator for multiarch builds
-	# This driver translates the instruction set to different platforms 
+	# This driver translates the instruction set to different platforms
 	docker run --rm --privileged linuxkit/binfmt:v0.8
 	docker buildx create --use --name=qemu
 	docker buildx inspect --bootstrap
 
 
 # Build and push multi-arch image
-docker-push-multi: buildx-setup generate fmt vet
+docker-push-multi: buildx-setup generate fmt vet staticcheck
 	@echo "Warning: this does not run tests. Run 'make test' to ensure tests are passing."
 	docker buildx build \
 	--push \
@@ -466,4 +475,3 @@ krew-install: krew-build
 # Uninstall kubectl plugin locally using krew.
 krew-uninstall:
 	-kubectl krew uninstall hns
-

go.mod

@@ -14,6 +14,7 @@ require (
 	github.com/spf13/cobra v1.2.1
 	go.opencensus.io v0.23.0
 	go.uber.org/zap v1.19.1
+	honnef.co/go/tools v0.2.2
 	k8s.io/api v0.23.2
 	k8s.io/apiextensions-apiserver v0.23.2
 	k8s.io/apimachinery v0.23.2
@@ -31,6 +32,7 @@ require (
 	github.com/Azure/go-autorest/autorest/date v0.3.0 // indirect
 	github.com/Azure/go-autorest/logger v0.2.1 // indirect
 	github.com/Azure/go-autorest/tracing v0.6.0 // indirect
+	github.com/BurntSushi/toml v0.3.1 // indirect
 	github.com/PuerkitoBio/purell v1.1.1 // indirect
 	github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578 // indirect
 	github.com/aws/aws-sdk-go v1.23.20 // indirect

go.sum

@@ -58,6 +58,7 @@ github.com/Azure/go-autorest/logger v0.2.1 h1:IG7i4p/mDa2Ce4TRyAO8IHnVhAVF3RFU+Z
 github.com/Azure/go-autorest/logger v0.2.1/go.mod h1:T9E3cAhj2VqvPOtCYAvby9aBXkZmbF5NWuPV8+WeEW8=
 github.com/Azure/go-autorest/tracing v0.6.0 h1:TYi4+3m5t6K48TGI9AUdb+IzbnSxvnvUMfuitfgcfuo=
 github.com/Azure/go-autorest/tracing v0.6.0/go.mod h1:+vhtPC754Xsa23ID7GlGsrdKBpUA79WCAKPPZVC2DeU=
+github.com/BurntSushi/toml v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ=
 github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU=
 github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym/WlBOVXweHU+Q+/VP0lqqI8lqeDx9IjBqo=
 github.com/NYTimes/gziphandler v0.0.0-20170623195520-56545f4a5d46/go.mod h1:3wb06e3pkSAbeQ52E9H9iFoQsEEwGN64994WTCIhntQ=
@@ -1007,6 +1008,8 @@ honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWh
 honnef.co/go/tools v0.0.1-2019.2.3/go.mod h1:a3bituU0lyd329TUQxRnasdCoJDkEUEAqEt0JzvZhAg=
 honnef.co/go/tools v0.0.1-2020.1.3/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k=
 honnef.co/go/tools v0.0.1-2020.1.4/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k=
+honnef.co/go/tools v0.2.2 h1:MNh1AVMyVX23VUHE2O27jm6lNj3vjO5DexS4A1xvnzk=
+honnef.co/go/tools v0.2.2/go.mod h1:lPVVZ2BS5TfnjLyizF7o7hv7j9/L+8cZY2hLyjP9cGY=
 k8s.io/api v0.23.0/go.mod h1:8wmDdLBHBNxtOIytwLstXt5E9PddnZb0GaMcqsvDBpg=
 k8s.io/api v0.23.2 h1:62cpzreV3dCuj0hqPi8r4dyWh48ogMcyh+ga9jEGij4=
 k8s.io/api v0.23.2/go.mod h1:sYuDb3flCtRPI8ghn6qFrcK5ZBu2mhbElxRE95qpwlI=

hack/tools.go

@@ -7,5 +7,6 @@
 package hack
 
 import (
+	_ "honnef.co/go/tools/cmd/staticcheck"
 	_ "sigs.k8s.io/controller-tools/cmd/controller-gen"
 )

internal/anchor/reconciler.go

@@ -245,7 +245,7 @@ func (r *Reconciler) shouldDeleteSubns(log logr.Logger, inst *api.SubnamespaceAn
 		return false
 
 	default:
-		log.Error(errors.New("Bad anchor state"), "state", inst.Status.State)
+		log.Error(errors.New("bad anchor state"), "state", inst.Status.State)
 		// Stay on the safe side - don't delete
 		return false
 	}
@@ -295,7 +295,7 @@ func (r *Reconciler) shouldFinalizeAnchor(log logr.Logger, inst *api.Subnamespac
 
 	default:
 		// Should never happen, so log an error and let it be deleted.
-		log.Error(errors.New("Illegal state"), "Unknown state", "state", inst.Status.State)
+		log.Error(errors.New("illegal state"), "Unknown state", "state", inst.Status.State)
 		return true
 	}
 }
@@ -362,6 +362,7 @@ func (r *Reconciler) writeInstance(ctx context.Context, log logr.Logger, inst *a
 
 // deleteInstance deletes the anchor instance. Note: Make sure there's no
 // finalizers on the instance before calling this function.
+//lint:ignore U1000 Ignore for now, as it may be used again in the future
 func (r *Reconciler) deleteInstance(ctx context.Context, inst *api.SubnamespaceAnchor) error {
 	if r.ReadOnly {
 		return nil

internal/config/namespace.go

@@ -98,10 +98,10 @@ func setManagedMeta(patterns []string, option string, regexes *[]*regexp.Regexp)
 	for _, p := range patterns {
 		r, err := regexp.Compile("^" + p + "$")
 		if err != nil {
-			return fmt.Errorf("Illegal value for %s %q: %w", option, p, err)
+			return fmt.Errorf("illegal value for %s %q: %w", option, p, err)
 		}
 		if r.MatchString(api.MetaGroup) {
-			return fmt.Errorf("Illegal value for %s %q: cannot specify a pattern that matches %q", option, p, api.MetaGroup)
+			return fmt.Errorf("illegal value for %s %q: cannot specify a pattern that matches %q", option, p, api.MetaGroup)
 		}
 		*regexes = append(*regexes, r)
 	}

internal/forest/namespace.go

@@ -148,7 +148,7 @@ func (ns *Namespace) UpdateAllowCascadingDeletion(acd bool) bool {
 // AllowsCascadingDeletion returns true if the namespace's or any of the ancestors'
 // allowCascadingDeletion field is set to true.
 func (ns *Namespace) AllowsCascadingDeletion() bool {
-	if ns.allowCascadingDeletion == true {
+	if ns.allowCascadingDeletion {
 		return true
 	}
 	if ns.parent == nil || ns.CycleNames() != nil {

internal/hncconfig/reconciler_test.go

@@ -2,7 +2,6 @@ package hncconfig_test
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"testing"
 	"time"
@@ -454,6 +453,7 @@ func setTypeConfig(ctx context.Context, group, resource string, mode api.Synchro
 
 // updateTypeConfig is like setTypeConfig but it doesn't wait to confirm that the change was
 // successful.
+//lint:ignore U1000 Ignore for now, as it may be used again in the future
 func updateTypeConfig(ctx context.Context, group, resource string, mode api.SynchronizationMode) {
 	updateTypeConfigWithOffset(ctx, 1, group, resource, mode)
 }
@@ -483,6 +483,7 @@ func updateTypeConfigWithOffset(ctx context.Context, offset int, group, resource
 	}).Should(Succeed(), "While updating type config for %s/%s to %s", group, resource, mode)
 }
 
+//lint:ignore U1000 Ignore for now, as it may be used again in the future
 func unsetTypeConfig(ctx context.Context, group, resource string) {
 	removeTypeConfigWithOffset(1, ctx, group, resource)
 	Eventually(typeStatusMode(ctx, group, resource)).Should(Equal(testModeMisssing))
@@ -580,11 +581,11 @@ func getNumPropagatedObjects(ctx context.Context, group, resource string) func()
 				if t.NumPropagatedObjects != nil {
 					return *t.NumPropagatedObjects, nil
 				}
-				return -1, errors.New(fmt.Sprintf("NumPropagatedObjects field is not set for "+
-					"group %s, resource %s", group, resource))
+				return -1, fmt.Errorf("NumPropagatedObjects field is not set for "+
+					"group %s, resource %s", group, resource)
 			}
 		}
-		return -1, errors.New(fmt.Sprintf("group %s, resource %s is not found in status", group, resource))
+		return -1, fmt.Errorf("group %s, resource %s is not found in status", group, resource)
 	}
 }
 
@@ -601,7 +602,7 @@ func hasNumSourceObjects(ctx context.Context, group, resource string) func() (bo
 				return t.NumSourceObjects != nil, nil
 			}
 		}
-		return false, errors.New(fmt.Sprintf("group %s, resource %s is not found in status", group, resource))
+		return false, fmt.Errorf("group %s, resource %s is not found in status", group, resource)
 	}
 }
 
@@ -618,10 +619,10 @@ func getNumSourceObjects(ctx context.Context, group, resource string) func() (in
 				if t.NumSourceObjects != nil {
 					return *t.NumSourceObjects, nil
 				}
-				return -1, errors.New(fmt.Sprintf("NumSourceObjects field is not set for "+
-					"group %s, resource %s", group, resource))
+				return -1, fmt.Errorf("NumSourceObjects field is not set for "+
+					"group %s, resource %s", group, resource)
 			}
 		}
-		return -1, errors.New(fmt.Sprintf("group %s, resource %s is not found in status", group, resource))
+		return -1, fmt.Errorf("group %s, resource %s is not found in status", group, resource)
 	}
 }

internal/integtest/helpers.go

@@ -6,8 +6,8 @@ import (
 	"fmt"
 	"strings"
 
-	. "github.com/onsi/ginkgo"
-	. "github.com/onsi/gomega"
+	. "github.com/onsi/ginkgo" //lint:ignore ST1001 Ignoring this for now
+	. "github.com/onsi/gomega" //lint:ignore ST1001 Ignoring this for now
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -82,15 +82,15 @@ func TryUpdateHierarchy(ctx context.Context, h *api.HierarchyConfiguration) erro
 func GetLabel(ctx context.Context, from, label string) func() string {
 	return func() string {
 		ns := GetNamespace(ctx, from)
-		val, _ := ns.GetLabels()[label]
+		val := ns.GetLabels()[label]
 		return val
 	}
 }
 
 func GetAnnotation(ctx context.Context, from, key string) func() string {
 	return func() string {
 		ns := GetNamespace(ctx, from)
-		val, _ := ns.GetAnnotations()[key]
+		val := ns.GetAnnotations()[key]
 		return val
 	}
 }
@@ -378,7 +378,7 @@ func ObjectInheritedFrom(ctx context.Context, resource string, nsName, name stri
 	if inst.GetLabels() == nil {
 		return ""
 	}
-	lif, _ := inst.GetLabels()[api.LabelInheritedFrom]
+	lif := inst.GetLabels()[api.LabelInheritedFrom]
 	return lif
 }
 

internal/integtest/setup.go

@@ -21,8 +21,8 @@ import (
 	"testing"
 	"time"
 
-	. "github.com/onsi/ginkgo"
-	. "github.com/onsi/gomega"
+	. "github.com/onsi/ginkgo" //lint:ignore ST1001 Ignoring this for now
+	. "github.com/onsi/gomega" //lint:ignore ST1001 Ignoring this for now
 	corev1 "k8s.io/api/core/v1"
 	apiextensions "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	"k8s.io/client-go/kubernetes/scheme"