Add errcheck to test suite to ensure all errors produced are checked

kubernetes · Aug 10, 2022 · 195555f · 195555f
1 parent c577834
commit 195555f
Show file tree

Hide file tree

Showing 14 changed files with 126 additions and 9 deletions.
diff --git a/build/test.sh b/build/test.sh
@@ -50,3 +50,25 @@ if [ -n "${ERRS}" ]; then
 fi
 echo "PASS"
 echo
+
+# Set gobin here, install dependencies after this
+export GOBIN=$PWD/hack/tools/bin
+export PATH=$GOBIN:$PATH
+
+# Install errcheck
+echo 'Installing errcheck'
+cd "hack/tools"
+go install github.com/kisielk/errcheck 2>/dev/null
+cd "../.."
+
+echo -n "Checking errcheck: "
+ERRS=$(errcheck -exclude errcheck_excludes.txt -ignoretests ${TARGETS} 2>&1 || true)
+if [ -n "${ERRS}" ]; then
+    echo "FAIL"
+    echo "${ERRS}"
+    echo
+    exit 1
+fi
+echo "PASS"
+echo
+
diff --git a/cmd/echo/app/handlers.go b/cmd/echo/app/handlers.go
@@ -136,7 +136,9 @@ func process(w http.ResponseWriter, r *http.Request, b []byte) ([]byte, error) {
 		if err != nil {
 			return nil, fmt.Errorf("failed to compress data: %v", err)
 		}
-		zw.Close()
+		if err := zw.Close(); err != nil {
+			return nil, fmt.Errorf("failed to close gzip writer: %v", err)
+		}
 		w.Header().Set("Content-Encoding", "gzip")
 		return compressed.Bytes(), nil
 	}

diff --git a/cmd/echo/app/tls.go b/cmd/echo/app/tls.go
@@ -93,11 +93,15 @@ func generateInsecureCertAndKey(organization string, validFrom time.Time, validF
 		klog.Fatalf("Failed to create certificate: %s", err)
 	}
 	var certBytes bytes.Buffer
-	pem.Encode(&certBytes, &pem.Block{Type: "CERTIFICATE", Bytes: derBytes})
+	if err := pem.Encode(&certBytes, &pem.Block{Type: "CERTIFICATE", Bytes: derBytes}); err != nil {
+		klog.Fatalf("Failed to encode certificate: %v", err)
+	}
 
 	var keyBytes bytes.Buffer
 	pb := &pem.Block{Type: "RSA PRIVATE KEY", Bytes: x509.MarshalPKCS1PrivateKey(priv)}
-	pem.Encode(&keyBytes, pb)
+	if err := pem.Encode(&keyBytes, pb); err != nil {
+		klog.Fatalf("Failed to encode RSA Key: %v", err)
+	}
 
 	return certBytes.Bytes(), keyBytes.Bytes(), nil
 }
diff --git a/cmd/glbc/app/handlers.go b/cmd/glbc/app/handlers.go
@@ -141,7 +141,9 @@ func getFlagPage(w http.ResponseWriter, r *http.Request) {
 		Version:   version.Version,
 		Verbosity: flag.Lookup("v").Value.String(),
 	}
-	flagPageTemplate.Execute(w, s)
+	if err := flagPageTemplate.Execute(w, s); err != nil {
+		klog.Errorf("Unable to apply flag page template: %v", err)
+	}
 }
 
 var flagPageTemplate = template.Must(template.New("").Parse(`GCE Ingress Controller "GLBC"

diff --git a/cmd/glbc/main.go b/cmd/glbc/main.go
@@ -270,7 +270,10 @@ func runControllers(ctx *ingctx.ControllerContext) {
 	lbc := controller.NewLoadBalancerController(ctx, stopCh)
 	if ctx.EnableASMConfigMap {
 		ctx.ASMConfigController.RegisterInformer(ctx.ConfigMapInformer, func() {
-			lbc.Stop(false) // We want to trigger a restart, don't have to clean up all the resources.
+			// We want to trigger a restart, don't have to clean up all the resources.
+			if err := lbc.Stop(false); err != nil {
+				klog.Errorf("Failed to stop the load balancer controller: %v", err)
+			}
 		})
 	}
 

diff --git a/errcheck_excludes.txt b/errcheck_excludes.txt
@@ -0,0 +1,11 @@
+fmt.Fprintf
+fmt.Fprint
+(net/http.ResponseWriter).Write
+(*net/http.Server).Shutdown
+(*flag.FlagSet).Parse
+(*os.File).Close
+(io.Closer).Close
+(flag.Value).Set
+
+// We sometimes call this with a function that has no chance to error
+k8s.io/apimachinery/pkg/util/wait.PollUntil
diff --git a/hack/tools/.gitignore b/hack/tools/.gitignore
@@ -0,0 +1 @@
+/bin/
diff --git a/hack/tools/go.mod b/hack/tools/go.mod
@@ -0,0 +1,12 @@
+module k8s.io/ingress-gce/hack/tools
+
+go 1.19
+
+require github.com/kisielk/errcheck v1.6.2
+
+require (
+	golang.org/x/mod v0.6.0-dev.0.20220106191415-9b9b3d81d5e3 // indirect
+	golang.org/x/sys v0.0.0-20211019181941-9d821ace8654 // indirect
+	golang.org/x/tools v0.1.10 // indirect
+	golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 // indirect
+)
diff --git a/hack/tools/go.sum b/hack/tools/go.sum
@@ -0,0 +1,31 @@
+github.com/kisielk/errcheck v1.6.2 h1:uGQ9xI8/pgc9iOoCe7kWQgRE6SBTrCGmTSf0LrEtY7c=
+github.com/kisielk/errcheck v1.6.2/go.mod h1:nXw/i/MfnvRHqXa7XXmQMUB0oNFGuBrNI8d8NLy0LPw=
+github.com/yuin/goldmark v1.4.1/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k=
+golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
+golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc=
+golang.org/x/mod v0.6.0-dev.0.20220106191415-9b9b3d81d5e3 h1:kQgndtyPBW/JIYERgdxfwMYh3AVStj88WQTlNDi2a+o=
+golang.org/x/mod v0.6.0-dev.0.20220106191415-9b9b3d81d5e3/go.mod h1:3p9vT2HGsQu2K1YbXdKPJLVgG5VJdoTa1poYQBtP1AY=
+golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
+golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg=
+golang.org/x/net v0.0.0-20211015210444-4f30a5c0130f/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y=
+golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
+golang.org/x/sync v0.0.0-20210220032951-036812b2e83c/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
+golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
+golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
+golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
+golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/sys v0.0.0-20211019181941-9d821ace8654 h1:id054HUawV2/6IGm2IV8KZQjqtwAOo2CYlOToYqa0d0=
+golang.org/x/sys v0.0.0-20211019181941-9d821ace8654/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
+golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
+golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
+golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
+golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ=
+golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
+golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
+golang.org/x/tools v0.1.10 h1:QjFRCZxdOhBJ/UNgnBZLbNV13DlbnK0quyivTnXJM20=
+golang.org/x/tools v0.1.10/go.mod h1:Uh6Zz+xoGYZom868N8YTex3t7RhtHDBrE8Gzo9bV56E=
+golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
+golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
+golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE=
+golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
diff --git a/hack/tools/tools.go b/hack/tools/tools.go
@@ -0,0 +1,21 @@
+/*
+Copyright 2019 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.
+*/
+
+// Package tools is used to track binary dependencies with go modules
+// https://github.com/golang/go/wiki/Modules#how-can-i-track-tool-dependencies-for-a-module
+package tools
+
+import (
+	// linting tools
+	_ "github.com/kisielk/errcheck"
+)
diff --git a/pkg/composite/meta/meta.go b/pkg/composite/meta/meta.go
@@ -221,7 +221,9 @@ func populateApiServices() {
 	}
 
 	var result map[string]interface{}
-	json.Unmarshal([]byte(byteValue), &result)
+	if err := json.Unmarshal([]byte(byteValue), &result); err != nil {
+		panic(err)
+	}
 
 	// Queue of ApiService names for BFS
 	typesQueue := []string{}

diff --git a/pkg/e2e/helpers.go b/pkg/e2e/helpers.go
@@ -101,7 +101,9 @@ var Scheme = runtime.NewScheme()
 
 func init() {
 	// Register external types for Scheme
-	v1.AddToScheme(Scheme)
+	if err := v1.AddToScheme(Scheme); err != nil {
+		panic(err)
+	}
 }
 
 // IsRfc1918Addr returns true if the address supplied is an RFC1918 address

diff --git a/pkg/firewalls/controller.go b/pkg/firewalls/controller.go
@@ -158,7 +158,9 @@ func (fwc *FirewallController) sync(key string) error {
 
 	// If there are no more ingresses, then delete the firewall rule.
 	if len(gceIngresses) == 0 {
-		fwc.firewallPool.GC()
+		if err := fwc.firewallPool.GC(); err != nil {
+			klog.Errorf("Could not garbage collect firewall pool, got error: %v", err)
+		}
 		return nil
 	}
 

diff --git a/pkg/loadbalancers/address_manager.go b/pkg/loadbalancers/address_manager.go
@@ -254,7 +254,9 @@ func (am *addressManager) TearDownAddressIPIfNetworkTierMismatch() error {
 			return utils.NewNetworkTierErr(fmt.Sprintf("User specific address IP (%v)", am.name), string(am.networkTier), addr.NetworkTier)
 		}
 		klog.V(3).Infof("Deleting IP address %v because has wrong network tier", am.targetIP)
-		am.svc.DeleteRegionAddress(addr.Name, am.targetIP)
+		if err := am.svc.DeleteRegionAddress(addr.Name, am.targetIP); err != nil {
+			klog.Errorf("Unable to delete region address %s on target ip %s, err: %v", addr.Name, am.targetIP, err)
+		}
 	}
 	return nil
 }