cleanup: Handle errors

The unhandled errors were found by Goland code inspection.

Signed-off-by: Andrej Krejcir <[email protected]>

Author: akrejcir

hack/csv-generator.go

@@ -58,7 +58,8 @@ var (
 		Run: func(cmd *cobra.Command, args []string) {
 			err := runGenerator()
 			if err != nil {
-				fmt.Fprintf(os.Stderr, "%v\n", err)
+				// Ignoring returned error: no reasonable way to handle it.
+				_, _ = fmt.Fprintf(os.Stderr, "%v\n", err)
 				os.Exit(1)
 			}
 		},
@@ -67,7 +68,8 @@ var (
 
 func main() {
 	if err := rootCmd.Execute(); err != nil {
-		fmt.Fprintf(os.Stderr, "%v\n", err)
+		// Ignoring returned error: no reasonable way to handle it.
+		_, _ = fmt.Fprintf(os.Stderr, "%v\n", err)
 		os.Exit(1)
 	}
 }

internal/env/env_test.go

@@ -12,10 +12,11 @@ import (
 
 var _ = Describe("environments", func() {
 	It("should return correct value for OPERATOR_VERSION when variable is set", func() {
-		os.Setenv(OperatorVersionKey, "v0.0.1")
+		Expect(os.Setenv(OperatorVersionKey, "v0.0.1")).To(Succeed())
+		defer func() { Expect(os.Unsetenv(OperatorVersionKey)).To(Succeed()) }()
+
 		res := GetOperatorVersion()
 		Expect(res).To(Equal("v0.0.1"), "OPERATOR_VERSION should equal")
-		os.Unsetenv(OperatorVersionKey)
 	})
 
 	It("should return correct value for OPERATOR_VERSION when variable is not set", func() {

internal/operands/metrics/reconcile_test.go

@@ -63,7 +63,7 @@ var _ = Describe("Metrics operand", func() {
 	})
 
 	AfterEach(func() {
-		os.Unsetenv(runbookURLTemplateEnv)
+		Expect(os.Unsetenv(runbookURLTemplateEnv)).To(Succeed())
 	})
 
 	It("should create metrics resources", func() {
@@ -82,7 +82,7 @@ var _ = Describe("Metrics operand", func() {
 	DescribeTable("runbook URL template",
 		func(template string) {
 			if template != defaultRunbookURLTemplate {
-				os.Setenv(runbookURLTemplateEnv, template)
+				Expect(os.Setenv(runbookURLTemplateEnv, template)).To(Succeed())
 			}
 
 			err := rules.SetupRules()

internal/template-validator/validation/eval.go

@@ -160,42 +160,49 @@ func (ev *Evaluator) Evaluate(rules []Rule, vm *k6tv1.VirtualMachine) *Result {
 		// Otherwise, if we simply skip the malformed rule, the error can go unnoticed.
 		// IOW, this is a policy decision
 		if _, ok := uniqueNames[r.Name]; ok {
-			fmt.Fprintf(ev.Sink, "%s failed: duplicate name\n", r.Name)
+			// Ignoring returned error: This print is used only for logging
+			_, _ = fmt.Fprintf(ev.Sink, "%s failed: duplicate name\n", r.Name)
 			result.Fail(r, ErrDuplicateRuleName)
 			continue
 		}
 		uniqueNames[r.Name] = struct{}{}
 
 		if err := validateRule(r); err != nil {
-			fmt.Fprintf(ev.Sink, "%s failed: %v\n", r.Name, err)
+			// Ignoring returned error: This print is used only for logging
+			_, _ = fmt.Fprintf(ev.Sink, "%s failed: %v\n", r.Name, err)
 			result.Fail(r, err)
 			continue
 		}
 
 		// Specialize() may be costly, so we do this before.
 		if !r.IsAppliableOn(vm) {
 			// Legit case. Nothing to do or to complain.
-			fmt.Fprintf(ev.Sink, "%s SKIPPED: not appliable\n", r.Name)
+
+			// Ignoring returned error: This print is used only for logging
+			_, _ = fmt.Fprintf(ev.Sink, "%s SKIPPED: not appliable\n", r.Name)
 			result.Skip(r)
 			continue
 		}
 
 		ra, err := r.Specialize(vm, refVm)
 		if err != nil {
-			fmt.Fprintf(ev.Sink, "%s failed: cannot specialize: %v\n", r.Name, err)
+			// Ignoring returned error: This print is used only for logging
+			_, _ = fmt.Fprintf(ev.Sink, "%s failed: cannot specialize: %v\n", r.Name, err)
 			result.Fail(r, err)
 			continue
 		}
 
 		satisfied, err := ra.Apply(vm, refVm)
 		if err != nil {
-			fmt.Fprintf(ev.Sink, "%s failed: cannot apply: %v\n", r.Name, err)
+			// Ignoring returned error: This print is used only for logging
+			_, _ = fmt.Fprintf(ev.Sink, "%s failed: cannot apply: %v\n", r.Name, err)
 			result.Fail(r, err)
 			continue
 		}
 
 		applicationText := ra.String()
-		fmt.Fprintf(ev.Sink, "%s applied: %v, %s\n", r.Name, boolAsStatus(satisfied), applicationText)
+		// Ignoring returned error: This print is used only for logging
+		_, _ = fmt.Fprintf(ev.Sink, "%s applied: %v, %s\n", r.Name, boolAsStatus(satisfied), applicationText)
 		result.Applied(r, satisfied, applicationText)
 	}
 

internal/template-validator/validation/eval_test.go

@@ -285,7 +285,8 @@ var _ = Describe("Eval", func() {
 			Expect(res.Succeeded()).To(BeTrue())
 
 			for ix := range res.Status {
-				fmt.Fprintf(GinkgoWriter, "%+#v", res.Status[ix])
+				_, err := fmt.Fprintf(GinkgoWriter, "%+#v", res.Status[ix])
+				Expect(err).ToNot(HaveOccurred())
 			}
 
 			Expect(res.Status).To(HaveLen(len(rules)))
@@ -342,7 +343,8 @@ var _ = Describe("Eval", func() {
 			res := ev.Evaluate(rules, vmCirros)
 
 			for ix := range res.Status {
-				fmt.Fprintf(GinkgoWriter, "%+#v", res.Status[ix])
+				_, err := fmt.Fprintf(GinkgoWriter, "%+#v", res.Status[ix])
+				Expect(err).ToNot(HaveOccurred())
 			}
 
 			Expect(res.Succeeded()).To(BeFalse(), "succeeded")

internal/template-validator/validator/app.go

@@ -2,7 +2,6 @@ package validator
 
 import (
 	"crypto/tls"
-	"fmt"
 	"net/http"
 	"os"
 
@@ -130,7 +129,9 @@ func (app *App) Run() {
 
 func registerReadinessProbe() {
 	http.HandleFunc("/readyz", func(resp http.ResponseWriter, req *http.Request) {
-		fmt.Fprintf(resp, "ok")
+		if _, err := resp.Write([]byte("ok")); err != nil {
+			logger.Log.Error(err, "Failed to write response to /readyz")
+		}
 	})
 }
 

main.go

@@ -19,6 +19,7 @@ package main
 import (
 	"context"
 	"crypto/tls"
+	"errors"
 	"flag"
 	"fmt"
 	"net/http"
@@ -164,7 +165,7 @@ func (s *prometheusServer) Start(ctx context.Context) error {
 
 	server.TLSConfig = s.getPrometheusTLSConfig(ctx, certWatcher)
 
-	if err := server.ListenAndServeTLS(s.certPath, s.keyPath); err != nil && err != http.ErrServerClosed {
+	if err := server.ListenAndServeTLS(s.certPath, s.keyPath); err != nil && !errors.Is(err, http.ErrServerClosed) {
 		setupLog.Error(err, "Failed to start Prometheus metrics endpoint server")
 		return err
 	}

tests/crypto_policy_test.go

@@ -204,7 +204,7 @@ func getCaCertificate() []byte {
 func tryToAccessEndpoint(pod core.Pod, serviceName string, subpath string, port uint16, tlsConfig clientTLSOptions, insecure bool) (attemptedUrl string, err error) {
 	conn, err := portForwarder.Connect(&pod, port)
 	Expect(err).ToNot(HaveOccurred())
-	defer conn.Close()
+	defer func() { Expect(conn.Close()).To(Succeed()) }()
 
 	certPool := x509.NewCertPool()
 	certPool.AppendCertsFromPEM(getCaCertificate())

tests/labels_test.go

@@ -58,7 +58,8 @@ func encodePatch(operations []jsonpatch.Operation) client.Patch {
 	patchBytes, err := json.Marshal(operations)
 	Expect(err).NotTo(HaveOccurred())
 
-	fmt.Fprintf(GinkgoWriter, "sending patch: %s", string(patchBytes))
+	_, err = fmt.Fprintf(GinkgoWriter, "sending patch: %s", string(patchBytes))
+	Expect(err).NotTo(HaveOccurred())
 	return client.RawPatch(types.JSONPatchType, patchBytes)
 }
 

tests/misc_test.go

@@ -372,19 +372,18 @@ var _ = Describe("RHEL VM creation", func() {
 
 func logObject(key client.ObjectKey, obj client.Object) {
 	gvk, err := apiutil.GVKForObject(obj, testScheme)
-	if err != nil {
-		panic(err)
-	}
+	Expect(err).ToNot(HaveOccurred())
 	obj.GetObjectKind().SetGroupVersionKind(gvk)
 
 	err = apiClient.Get(ctx, key, obj)
 	if err != nil {
-		fmt.Fprintf(GinkgoWriter, "Failed to get %s: %s\n", gvk.Kind, err)
-	} else {
-		objJson, err := json.MarshalIndent(obj, "", "    ")
-		if err != nil {
-			panic(err)
-		}
-		fmt.Fprintf(GinkgoWriter, "Found %s:\n%s\n", gvk.Kind, objJson)
+		_, printErr := fmt.Fprintf(GinkgoWriter, "Failed to get %s: %s\n", gvk.Kind, err)
+		Expect(printErr).ToNot(HaveOccurred())
+		return
 	}
+
+	objJson, err := json.MarshalIndent(obj, "", "    ")
+	Expect(err).ToNot(HaveOccurred())
+	_, printErr := fmt.Fprintf(GinkgoWriter, "Found %s:\n%s\n", gvk.Kind, objJson)
+	Expect(printErr).ToNot(HaveOccurred())
 }

tests/port-forwarding.go

@@ -37,12 +37,22 @@ func (p *portForwardConn) Close() error {
 	return errors.Join(p.Conn.Close(), p.streamConnCloser.Close())
 }
 
-func (p *portForwarderImpl) Connect(pod *core.Pod, remotePort uint16) (net.Conn, error) {
+func (p *portForwarderImpl) Connect(pod *core.Pod, remotePort uint16) (conn net.Conn, err error) {
 	streamConnection, err := p.createStreamConnection(pod)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create stream connection: %w", err)
 	}
 
+	runCleanup := true
+	defer func() {
+		if !runCleanup {
+			return
+		}
+		if closeErr := streamConnection.Close(); closeErr != nil {
+			err = errors.Join(err, fmt.Errorf("failed to close streamConnection: %w", closeErr))
+		}
+	}()
+
 	requestId := atomic.AddInt32(&p.requestId, 1)
 
 	// Error stream is needed, otherwise port-forwarding will not work
@@ -52,21 +62,21 @@ func (p *portForwarderImpl) Connect(pod *core.Pod, remotePort uint16) (net.Conn,
 	headers.Set(core.PortForwardRequestIDHeader, strconv.Itoa(int(requestId)))
 	errorStream, err := streamConnection.CreateStream(headers)
 	if err != nil {
-		streamConnection.Close()
 		return nil, fmt.Errorf("failed to create error stream: %w", err)
 	}
+
 	// We will not write to error stream
-	if err := errorStream.Close(); err != nil {
+	if err = errorStream.Close(); err != nil {
 		return nil, fmt.Errorf("failed to close error stream: %w", err)
 	}
 
 	headers.Set(core.StreamType, core.StreamTypeData)
 	dataStream, err := streamConnection.CreateStream(headers)
 	if err != nil {
-		streamConnection.Close()
 		return nil, fmt.Errorf("failed to create data stream: %w", err)
 	}
 
+	runCleanup = false
 	return &portForwardConn{
 		Conn:             dataStream.(net.Conn),
 		streamConnCloser: streamConnection,

tests/validator_test.go

@@ -3,6 +3,7 @@ package tests
 import (
 	"crypto/tls"
 	"crypto/x509"
+	"errors"
 	"fmt"
 	"reflect"
 	"strings"
@@ -17,7 +18,7 @@ import (
 	apps "k8s.io/api/apps/v1"
 	core "k8s.io/api/core/v1"
 	rbac "k8s.io/api/rbac/v1"
-	"k8s.io/apimachinery/pkg/api/errors"
+	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -446,7 +447,7 @@ var _ = Describe("Template validator webhooks", func() {
 	AfterEach(func() {
 		if vm != nil {
 			err := apiClient.Delete(ctx, vm)
-			if !errors.IsNotFound(err) {
+			if !k8serrors.IsNotFound(err) {
 				Expect(err).ToNot(HaveOccurred(), "Failed to Delete VM")
 			}
 			// Need to wait until VM is removed, otherwise the webhook may
@@ -456,7 +457,7 @@ var _ = Describe("Template validator webhooks", func() {
 		if template != nil {
 			Eventually(func(g Gomega) {
 				if err := apiClient.Delete(ctx, template); err != nil {
-					g.Expect(errors.ReasonForError(err)).To(Equal(metav1.StatusReasonNotFound))
+					g.Expect(k8serrors.ReasonForError(err)).To(Equal(metav1.StatusReasonNotFound))
 				}
 			}, env.ShortTimeout(), time.Second).Should(Succeed(), "Template should be deleted")
 		}
@@ -823,7 +824,7 @@ var _ = Describe("Template validator webhooks", func() {
 
 			Eventually(func() metav1.StatusReason {
 				err := apiClient.Delete(ctx, template, client.DryRunAll)
-				return errors.ReasonForError(err)
+				return k8serrors.ReasonForError(err)
 			}, env.ShortTimeout(), 1*time.Second).Should(Equal(metav1.StatusReasonForbidden), "Should have given forbidden error")
 		})
 
@@ -936,7 +937,7 @@ func eventuallyFailToCreateVm(vm *kubevirtv1.VirtualMachine) bool {
 			}
 			return metav1.StatusReasonUnknown, fmt.Errorf("VM was created")
 		}
-		return errors.ReasonForError(err), nil
+		return k8serrors.ReasonForError(err), nil
 	}, env.ShortTimeout()).Should(Equal(metav1.StatusReasonInvalid), "Should have given the invalid error")
 }
 
@@ -968,7 +969,7 @@ func failVmCreationToIncreaseRejectedVmsMetrics(template *templatev1.Template) {
 	Expect(rejectedCountAfter - rejectedCountBefore).To(Equal(1))
 
 	err := apiClient.Delete(ctx, vm)
-	if !errors.IsNotFound(err) {
+	if !k8serrors.IsNotFound(err) {
 		Expect(err).ToNot(HaveOccurred(), "Failed to Delete VM")
 	}
 	waitForDeletion(client.ObjectKeyFromObject(vm), vm)
@@ -1212,14 +1213,14 @@ func TemplateWithoutRules() *templatev1.Template {
 	return addObjectsToTemplates("test-fedora-desktop-small-without-rules", validations)
 }
 
-func getWebhookServerCertificates(validatorPod *core.Pod) ([]*x509.Certificate, error) {
+func getWebhookServerCertificates(validatorPod *core.Pod) (certs []*x509.Certificate, err error) {
 	conn, err := portForwarder.Connect(validatorPod, validator.ContainerPort)
 	if err != nil {
 		return nil, err
 	}
 
 	tlsConn := tls.Client(conn, &tls.Config{InsecureSkipVerify: true})
-	defer tlsConn.Close()
+	defer func() { err = errors.Join(err, tlsConn.Close()) }()
 
 	err = tlsConn.Handshake()
 	if err != nil {

tests/watcher.go

@@ -85,7 +85,7 @@ func (s *sspWatch) startWatch() {
 
 		err = s.handleWatch(w)
 		if err != nil {
-			if err != errStopWatch {
+			if errors.Is(err, errStopWatch) {
 				s.atomicError.Store(err)
 			}
 			return

tools/test-rules-writer/test_rules_writer.go

@@ -21,7 +21,8 @@ func main() {
 	encoder := json.NewEncoder(os.Stdout)
 	encoder.SetIndent("", "  ")
 	if err := encoder.Encode(pr.Spec); err != nil {
-		fmt.Fprintf(os.Stderr, "Error encoding prometheus spec: %v", err)
+		// Ignoring returned error: no reasonable way to handle it.
+		_, _ = fmt.Fprintf(os.Stderr, "Error encoding prometheus spec: %v", err)
 		os.Exit(1)
 	}
 }