From 195555f42b6765ca23383d799bfb0dbaf2024c0e Mon Sep 17 00:00:00 2001 From: Michael Aspinwall Date: Fri, 5 Aug 2022 16:24:45 +0000 Subject: [PATCH] Add errcheck to test suite to ensure all errors produced are checked --- build/test.sh | 22 ++++++++++++++++++++ cmd/echo/app/handlers.go | 4 +++- cmd/echo/app/tls.go | 8 +++++-- cmd/glbc/app/handlers.go | 4 +++- cmd/glbc/main.go | 5 ++++- errcheck_excludes.txt | 11 ++++++++++ hack/tools/.gitignore | 1 + hack/tools/go.mod | 12 +++++++++++ hack/tools/go.sum | 31 ++++++++++++++++++++++++++++ hack/tools/tools.go | 21 +++++++++++++++++++ pkg/composite/meta/meta.go | 4 +++- pkg/e2e/helpers.go | 4 +++- pkg/firewalls/controller.go | 4 +++- pkg/loadbalancers/address_manager.go | 4 +++- 14 files changed, 126 insertions(+), 9 deletions(-) create mode 100644 errcheck_excludes.txt create mode 100644 hack/tools/.gitignore create mode 100644 hack/tools/go.mod create mode 100644 hack/tools/go.sum create mode 100644 hack/tools/tools.go diff --git a/build/test.sh b/build/test.sh index 23b567944a..e8cd123969 100755 --- 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 index d0488968f9..d082bf7b8f 100644 --- 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 index cb622d346b..40142b94db 100644 --- 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 index 6fb916b32a..94305149f7 100644 --- 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 index 61957f91b9..3d067e472c 100644 --- 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 new file mode 100644 index 0000000000..fbc05f78f1 --- /dev/null +++ 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 \ No newline at end of file diff --git a/hack/tools/.gitignore b/hack/tools/.gitignore new file mode 100644 index 0000000000..65776c32fc --- /dev/null +++ b/hack/tools/.gitignore @@ -0,0 +1 @@ +/bin/ \ No newline at end of file diff --git a/hack/tools/go.mod b/hack/tools/go.mod new file mode 100644 index 0000000000..332b6bff70 --- /dev/null +++ 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 new file mode 100644 index 0000000000..83f745f237 --- /dev/null +++ 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 new file mode 100644 index 0000000000..c46d42d2f8 --- /dev/null +++ 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 index fca4b383de..12addd5c6a 100644 --- 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 index 8b916c501a..f8fd103f9b 100644 --- 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 index 8b55a39334..af4b894eb1 100644 --- 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 index 991abce43a..655dfa3da2 100644 --- 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 }