Merge pull request kubevirt#8058 from maiqueb/add-errcheck-linter

Add errcheck linter

BUILD.bazel

@@ -57,6 +57,7 @@ nogo(
         "//vendor/github.com/nunnatsa/ginkgolinter:go_default_library",
         "//tools/analyzers/banncheck:go_default_library",
         "//vendor/github.com/gordonklaus/ineffassign/pkg/ineffassign:go_default_library",
+        "//vendor/github.com/kisielk/errcheck/errcheck:go_default_library",
         "@org_golang_x_tools//go/analysis/passes/asmdecl:go_default_library",
         "@org_golang_x_tools//go/analysis/passes/assign:go_default_library",
         "@org_golang_x_tools//go/analysis/passes/atomic:go_default_library",

go.mod

@@ -31,6 +31,7 @@ require (
 	github.com/imdario/mergo v0.3.11
 	github.com/insomniacslk/dhcp v0.0.0-20201112113307-4de412bc85d8
 	github.com/k8snetworkplumbingwg/network-attachment-definition-client v1.3.0
+	github.com/kisielk/errcheck v1.6.2
 	github.com/krolaw/dhcp4 v0.0.0-20180925202202-7cead472c414
 	github.com/kubernetes-csi/external-snapshotter/client/v4 v4.2.0
 	github.com/mitchellh/go-ps v0.0.0-20190716172923-621e5597135b
@@ -138,6 +139,7 @@ require (
 	github.com/vishvananda/netns v0.0.0-20200728191858-db3c7e526aae // indirect
 	github.com/willf/bitset v1.1.11 // indirect
 	go.mongodb.org/mongo-driver v1.8.4 // indirect
+	golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4 // indirect
 	golang.org/x/oauth2 v0.0.0-20210819190943-2bc19b11175f // indirect
 	golang.org/x/text v0.3.7 // indirect
 	google.golang.org/appengine v1.6.7 // indirect

go.sum

@@ -871,6 +871,8 @@ github.com/kelseyhightower/envconfig v1.4.0/go.mod h1:cccZRl6mQpaq41TPp5QxidR+Sa
 github.com/kisielk/errcheck v1.1.0/go.mod h1:EZBBE59ingxPouuu3KfxchcWSUPOHkagtvWXihfKN4Q=
 github.com/kisielk/errcheck v1.2.0/go.mod h1:/BMXB+zMLi60iA8Vv6Ksmxu/1UDYcXs4uQLJ+jE2L00=
 github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI2bnpBCr8=
+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/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck=
 github.com/klauspost/compress v1.9.5/go.mod h1:RyIbtBH6LamlWaDj8nUwkbUhJ87Yi3uG0guNDohfE1A=
 github.com/klauspost/compress v1.10.7/go.mod h1:aoV0uJVorq1K+umq18yTdKaF57EivdYsUV+/s2qKfXs=
@@ -1494,6 +1496,7 @@ golang.org/x/mod v0.4.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
 golang.org/x/mod v0.4.1/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
 golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
 golang.org/x/mod v0.5.1/go.mod h1:5OXOZSfqPIIbmVBIIKWRFfZjPR0E5r58TLhUjH0a2Ro=
+golang.org/x/mod v0.6.0-dev.0.20220106191415-9b9b3d81d5e3/go.mod h1:3p9vT2HGsQu2K1YbXdKPJLVgG5VJdoTa1poYQBtP1AY=
 golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4 h1:6zppjxzCulZykYSLyVDYbneBfbaBIQPYMevg0bEwv2s=
 golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4=
 golang.org/x/net v0.0.0-20180530234432-1e491301e022/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
@@ -1829,6 +1832,7 @@ golang.org/x/tools v0.1.2/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk=
 golang.org/x/tools v0.1.5/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk=
 golang.org/x/tools v0.1.6-0.20210820212750-d4cc65f0b2ff/go.mod h1:YD9qOF0M9xpSpdWTBbzEl5e/RnCefISl8E5Noe10jFM=
 golang.org/x/tools v0.1.9/go.mod h1:nABZi5QlRsZVlzPpHl034qft6wpY4eDcsTt5AaioBiU=
+golang.org/x/tools v0.1.10/go.mod h1:Uh6Zz+xoGYZom868N8YTex3t7RhtHDBrE8Gzo9bV56E=
 golang.org/x/tools v0.1.11 h1:loJ25fNOEhSXfHrpoGj91eCUThwdNX6u24rO1xnNteY=
 golang.org/x/tools v0.1.11/go.mod h1:SgwaegtQh8clINPpECJMqnxLv9I09HLqnW3RMqW0CA4=
 golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=

nogo_config.json

@@ -139,5 +139,29 @@
       "vendor/": "vendor doesn't pass vet",
       "external/": "externaldoesn't pass vet"
     }
+  },
+  "errcheck": {
+    "exclude_files": {
+      "vendor/": "vendor doesn't pass errcheck",
+      "external/": "external doesn't pass errcheck",
+      "src/": "golang core code does not pass errcheck",
+      "bazel-out/": "this directory should be off-limits to errcheck",
+      "pkg/": "KubeVirt pkg does not pass errcheck yet",
+      "cmd/": "KubeVirt binaries do not pass errcheck yet",
+      "tools/": "KubeVirt tools do not pass errcheck yet",
+      "tests/console": "tests/console does not pass errcheck yet",
+      "tests/monitoring/": "tests/monitoring does not pass errcheck yet",
+      "tests/reporter/": "tests/reporter pkg does not pass errcheck yet",
+      "tests/canary_upgrade_test.go": "canary_upgrade_test does not pass errcheck yet",
+      "tests/infra_test.go": "infra_test.go does not pass errcheck yet",
+      "tests/kubectl_test.go": "kubectl_test does not pass errcheck yet",
+      "tests/migration_test.go": "migration_test does not pass errcheck yet",
+      "tests/replicaset_test.go": "replicaset_test does not pass errcheck yet",
+      "tests/usbredir_test.go": "kubectl_test does not pass errcheck yet",
+      "tests/utils.go": "utils.go does not pass errcheck yet",
+      "tests/vmi_configuration_test.go": "vmi_configuration_test does not pass errcheck yet",
+      "tests/vmi_lifecycle_test.go": "vmi_lifecycle_test does not pass errcheck yet",
+      "tests/vnc_test.go": "vnc_test does not pass errcheck yet"
+    }
   }
 }

tests/errorhandling/BUILD.bazel

@@ -0,0 +1,9 @@
+load("@io_bazel_rules_go//go:def.bzl", "go_library")
+
+go_library(
+    name = "go_default_library",
+    srcs = ["closing_files.go"],
+    importpath = "kubevirt.io/kubevirt/tests/errorhandling",
+    visibility = ["//visibility:public"],
+    deps = ["//vendor/golang.org/x/sys/unix:go_default_library"],
+)

tests/errorhandling/closing_files.go

@@ -0,0 +1,23 @@
+package errorhandling
+
+import (
+	"os"
+
+	"golang.org/x/sys/unix"
+)
+
+func SafelyCloseFile(f *os.File) {
+	if err := f.Close(); err != nil {
+		panicIfNotReadOnlyFile(f)
+	}
+}
+
+func panicIfNotReadOnlyFile(f *os.File) {
+	fileFlags, err := unix.FcntlInt(f.Fd(), unix.F_GETFD, 0)
+	if err != nil {
+		panic("could not access file's FD flags")
+	}
+	if fileFlags&(unix.O_WRONLY|unix.O_APPEND|unix.O_RDWR) != 0 {
+		panic("this is not a read-only fd")
+	}
+}

tests/libnode/node.go

@@ -68,9 +68,13 @@ func CleanNodes() {
 
 		k8sClient := clientcmd.GetK8sCmdClient()
 		if k8sClient == "oc" {
-			clientcmd.RunCommandWithNS("", k8sClient, "adm", "uncordon", node.Name)
+			if _, _, err := clientcmd.RunCommandWithNS("", k8sClient, "adm", "uncordon", node.Name); err != nil {
+				panic(fmt.Sprintf("could not uncordon the node %s", node.Name))
+			}
 		} else {
-			clientcmd.RunCommandWithNS("", k8sClient, "uncordon", node.Name)
+			if _, _, err := clientcmd.RunCommandWithNS("", k8sClient, "uncordon", node.Name); err != nil {
+				panic(fmt.Sprintf("could not uncordon the node %s", node.Name))
+			}
 		}
 
 		found := false

tests/libstorage/BUILD.bazel

@@ -14,6 +14,7 @@ go_library(
         "//pkg/util/net/ip:go_default_library",
         "//staging/src/kubevirt.io/api/core/v1:go_default_library",
         "//staging/src/kubevirt.io/client-go/kubecli:go_default_library",
+        "//tests/errorhandling:go_default_library",
         "//tests/flags:go_default_library",
         "//tests/framework/cleanup:go_default_library",
         "//tests/libnode:go_default_library",

tests/libstorage/config.go

@@ -24,6 +24,7 @@ import (
 	"io/ioutil"
 	"os"
 
+	"kubevirt.io/kubevirt/tests/errorhandling"
 	"kubevirt.io/kubevirt/tests/flags"
 )
 
@@ -56,7 +57,7 @@ func LoadConfig() (*KubeVirtTestsConfiguration, error) {
 		return nil, err
 	}
 
-	defer jsonFile.Close()
+	defer errorhandling.SafelyCloseFile(jsonFile)
 
 	// read the configuration file as a byte array
 	byteValue, _ := ioutil.ReadAll(jsonFile)

tests/network/port_forward.go

@@ -154,7 +154,9 @@ func killPortForwardCommand(portForwardCmd *exec.Cmd) error {
 		return nil
 	}
 
-	portForwardCmd.Process.Kill()
+	if err := portForwardCmd.Process.Kill(); err != nil {
+		return err
+	}
 	_, err := portForwardCmd.Process.Wait()
 	return err
 }

tests/network/vmi_passt.go

@@ -213,7 +213,6 @@ EOL`, inetSuffix, serverIP, serverPort)
 						Expect(err).ToNot(HaveOccurred())
 						clientVMI = tests.WaitForSuccessfulVMIStartIgnoreWarnings(clientVMI)
 						Expect(console.LoginToAlpine(clientVMI)).To(Succeed())
-						console.LoginToAlpine(clientVMI)
 
 						By("Starting and verifying UDP client")
 						// Due to a passt bug, at least one UDPv6 message has to be sent from a machine before it can receive UDPv6 messages

tests/portforward_test.go

@@ -81,7 +81,7 @@ var _ = Describe("[sig-compute]PortForward", func() {
 				Out: &out,
 			})
 			if err != nil {
-				fmt.Fprintln(GinkgoWriter, err)
+				_, _ = fmt.Fprintln(GinkgoWriter, err)
 			}
 			close(streamClosed)
 		}()

tests/storage/BUILD.bazel

@@ -38,6 +38,7 @@ go_library(
         "//tests/clientcmd:go_default_library",
         "//tests/console:go_default_library",
         "//tests/containerdisk:go_default_library",
+        "//tests/errorhandling:go_default_library",
         "//tests/flags:go_default_library",
         "//tests/framework/checks:go_default_library",
         "//tests/framework/matcher:go_default_library",

tests/storage/datavolume.go

@@ -899,7 +899,9 @@ var _ = SIGDescribe("DataVolume Integration", func() {
 				[]string{"ls", "-s", "/var/run/kubevirt-private/vmi-disks/disk0/disk.img"},
 			)
 			Expect(err).ToNot(HaveOccurred())
-			fmt.Sscanf(lsOutput, "%d %s", &imageSize, &unused)
+			if _, err := fmt.Sscanf(lsOutput, "%d %s", &imageSize, &unused); err != nil {
+				return 0
+			}
 			return imageSize
 		}
 

tests/storage/export.go

@@ -252,9 +252,9 @@ var _ = SIGDescribe("Export", func() {
 		}, 30*time.Second, 1*time.Second).Should(Equal(k8sv1.ClaimBound))
 		By("Deleting the trigger pod")
 		immediate := int64(0)
-		virtClient.CoreV1().Pods(triggerPod.Namespace).Delete(context.Background(), triggerPod.Name, metav1.DeleteOptions{
+		Expect(virtClient.CoreV1().Pods(triggerPod.Namespace).Delete(context.Background(), triggerPod.Name, metav1.DeleteOptions{
 			GracePeriodSeconds: &immediate,
-		})
+		})).To(Succeed())
 	}
 
 	createExportTokenSecret := func(name, namespace string) *k8sv1.Secret {
@@ -635,12 +635,12 @@ var _ = SIGDescribe("Export", func() {
 	}
 
 	matchesCNOrAlt := func(cert *x509.Certificate, hostName string) bool {
-		fmt.Fprintf(GinkgoWriter, "CN: %s, hostname: %s\n", cert.Subject.CommonName, hostName)
+		logToGinkgoWritter("CN: %s, hostname: %s\n", cert.Subject.CommonName, hostName)
 		if strings.Contains(cert.Subject.CommonName, hostName) {
 			return true
 		}
 		for _, extension := range cert.Extensions {
-			fmt.Fprintf(GinkgoWriter, "ExtensionID: %s, subjectAltNameId: %s, value: %s, hostname: %s\n", extension.Id.String(), subjectAltNameId, string(extension.Value), hostName)
+			logToGinkgoWritter("ExtensionID: %s, subjectAltNameId: %s, value: %s, hostname: %s\n", extension.Id.String(), subjectAltNameId, string(extension.Value), hostName)
 			if extension.Id.String() == subjectAltNameId && strings.Contains(string(extension.Value), hostName) {
 				return true
 			}
@@ -835,23 +835,33 @@ var _ = SIGDescribe("Export", func() {
 			}
 		})
 
-		generateTestCert := func(hostName string) string {
+		generateTestCert := func(hostName string) (string, error) {
 			key, err := certutil.NewPrivateKey()
-			Expect(err).ToNot(HaveOccurred())
+			if err != nil {
+				return "", err
+			}
 
 			config := certutil.Config{
 				CommonName: "blah blah",
 			}
 
 			cert, err := certutil.NewSelfSignedCACertWithAltNames(config, key, time.Hour, "hahaha.wwoo", hostName, "fgdgd.dfsgdf")
-			Expect(err).ToNot(HaveOccurred())
+			if err != nil {
+				return "", err
+			}
+
 			pemOut := strings.Builder{}
-			pem.Encode(&pemOut, &pem.Block{Type: "CERTIFICATE", Bytes: cert.Raw})
-			return strings.TrimSpace(pemOut.String())
+			if err := pem.Encode(&pemOut, &pem.Block{Type: "CERTIFICATE", Bytes: cert.Raw}); err != nil {
+				return "", err
+			}
+			return strings.TrimSpace(pemOut.String()), nil
 		}
 
-		createIngressTLSSecret := func(name string) string {
-			testCert := generateTestCert(testHostName)
+		createIngressTLSSecret := func(name string) (string, error) {
+			testCert, err := generateTestCert(testHostName)
+			if err != nil {
+				return "", err
+			}
 			secret := &k8sv1.Secret{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      name,
@@ -862,9 +872,11 @@ var _ = SIGDescribe("Export", func() {
 					tlsCert: testCert,
 				},
 			}
-			_, err := virtClient.CoreV1().Secrets(flags.KubeVirtInstallNamespace).Create(context.Background(), secret, metav1.CreateOptions{})
-			Expect(err).ToNot(HaveOccurred())
-			return testCert
+			_, err = virtClient.CoreV1().Secrets(flags.KubeVirtInstallNamespace).Create(context.Background(), secret, metav1.CreateOptions{})
+			if err != nil {
+				return "", err
+			}
+			return testCert, nil
 		}
 
 		createIngress := func(tlsSecretName string) *networkingv1.Ingress {
@@ -927,7 +939,8 @@ var _ = SIGDescribe("Export", func() {
 			if !exists {
 				Skip("Skip test when Filesystem storage is not present")
 			}
-			testCert := createIngressTLSSecret(tlsSecretName)
+			testCert, err := createIngressTLSSecret(tlsSecretName)
+			Expect(err).NotTo(HaveOccurred())
 			ingress := createIngress(tlsSecretName)
 			vmExport := createRunningExport(sc, k8sv1.PersistentVolumeFilesystem)
 			Expect(vmExport.Status.Links.External.Cert).To(Equal(testCert))
@@ -974,3 +987,7 @@ var _ = SIGDescribe("Export", func() {
 		})
 	})
 })
+
+func logToGinkgoWritter(format string, parameters ...interface{}) {
+	_, _ = fmt.Fprintf(GinkgoWriter, format, parameters...)
+}

tests/storage/imageupload.go

@@ -24,6 +24,7 @@ import (
 
 	"kubevirt.io/kubevirt/tests"
 	"kubevirt.io/kubevirt/tests/clientcmd"
+	"kubevirt.io/kubevirt/tests/errorhandling"
 	"kubevirt.io/kubevirt/tests/flags"
 	"kubevirt.io/kubevirt/tests/libstorage"
 )
@@ -291,8 +292,8 @@ var _ = SIGDescribe("[Serial]ImageUpload", func() {
 
 	AfterEach(func() {
 		if kubectlCmd != nil {
-			kubectlCmd.Process.Kill()
-			kubectlCmd.Wait()
+			Expect(kubectlCmd.Process.Kill()).To(Succeed())
+			Expect(kubectlCmd.Wait()).To(Succeed())
 		}
 
 		err := os.Remove(imagePath)
@@ -304,7 +305,7 @@ func createArchive(targetFile, tgtDir string, sourceFilesNames ...string) string
 	tgtPath := filepath.Join(tgtDir, filepath.Base(targetFile)+".tar")
 	tgtFile, err := os.Create(tgtPath)
 	Expect(err).ToNot(HaveOccurred())
-	defer tgtFile.Close()
+	defer errorhandling.SafelyCloseFile(tgtFile)
 
 	tests.ArchiveToFile(tgtFile, sourceFilesNames...)
 

tests/virtctl/BUILD.bazel

@@ -14,6 +14,7 @@ go_library(
         "//tests:go_default_library",
         "//tests/clientcmd:go_default_library",
         "//tests/console:go_default_library",
+        "//tests/errorhandling:go_default_library",
         "//tests/libvmi:go_default_library",
         "//tests/util:go_default_library",
         "//vendor/github.com/onsi/ginkgo/v2:go_default_library",

tests/virtctl/key.go

@@ -10,6 +10,8 @@ import (
 	"os"
 
 	"golang.org/x/crypto/ssh"
+
+	"kubevirt.io/kubevirt/tests/errorhandling"
 )
 
 func NewKeyPair() (*ecdsa.PrivateKey, ssh.PublicKey, error) {
@@ -37,7 +39,7 @@ func DumpPrivateKey(privateKey *ecdsa.PrivateKey, file string) error {
 	if err != nil {
 		return err
 	}
-	defer f.Close()
+	defer errorhandling.SafelyCloseFile(f)
 	if err = pem.Encode(f, privateKeyBlock); err != nil {
 		return fmt.Errorf("error when encode private pem: %s", err)
 

tools/analyzers/BUILD.bazel

@@ -7,6 +7,7 @@ go_library(
     visibility = ["//visibility:public"],
     deps = [
         "//vendor/github.com/gordonklaus/ineffassign/pkg/ineffassign:go_default_library",
+        "//vendor/github.com/kisielk/errcheck/errcheck:go_default_library",
         "//vendor/github.com/nunnatsa/ginkgolinter:go_default_library",
     ],
 )

tools/analyzers/analyzer.go

@@ -3,5 +3,6 @@ package analyzers
 // Keep a reference to analyzers
 import (
 	_ "github.com/gordonklaus/ineffassign/pkg/ineffassign"
+	_ "github.com/kisielk/errcheck/errcheck"
 	_ "github.com/nunnatsa/ginkgolinter"
 )