Skip to content

Commit

Permalink
cleanup: Handle errors
Browse files Browse the repository at this point in the history
The unhandled errors were found by Goland code inspection.

Signed-off-by: Andrej Krejcir <[email protected]>
  • Loading branch information
akrejcir committed Feb 17, 2025
1 parent d96d1bd commit 45c1ccc
Show file tree
Hide file tree
Showing 15 changed files with 62 additions and 45 deletions.
6 changes: 4 additions & 2 deletions hack/csv-generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
},
Expand All @@ -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)
}
}
Expand Down
5 changes: 3 additions & 2 deletions internal/env/env_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
4 changes: 2 additions & 2 deletions internal/operands/metrics/reconcile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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()
Expand Down
19 changes: 13 additions & 6 deletions internal/template-validator/validation/eval.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
6 changes: 4 additions & 2 deletions internal/template-validator/validation/eval_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)))
Expand Down Expand Up @@ -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")
Expand Down
5 changes: 3 additions & 2 deletions internal/template-validator/validator/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package validator

import (
"crypto/tls"
"fmt"
"net/http"
"os"

Expand Down Expand Up @@ -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")
}
})
}

Expand Down
3 changes: 2 additions & 1 deletion main.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package main
import (
"context"
"crypto/tls"
"errors"
"flag"
"fmt"
"net/http"
Expand Down Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion tests/crypto_policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
3 changes: 2 additions & 1 deletion tests/labels_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
2 changes: 1 addition & 1 deletion tests/metrics_test_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func intMetricValue(metricName string, metricsPort uint16, pod *v1.Pod) int {
}
resp, err := client.Get(fmt.Sprintf("https://localhost:%d/metrics", metricsPort))
Expect(err).ToNot(HaveOccurred(), "Can't get metrics from %s", pod.Name)
defer resp.Body.Close()
defer func() { Expect(resp.Body.Close()).To(Succeed()) }()
body, _ := io.ReadAll(resp.Body)
regex, ok := regexpForMetrics[metricName]
if !ok {
Expand Down
19 changes: 9 additions & 10 deletions tests/misc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
11 changes: 6 additions & 5 deletions tests/port-forwarding.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package tests

import (
"errors"
"fmt"
"io"
"net"
Expand Down Expand Up @@ -43,17 +44,17 @@ 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, err
return nil, errors.Join(err, streamConnection.Close())
}
// We will not write to error stream
errorStream.Close()
if err := errorStream.Close(); err != nil {
return nil, errors.Join(err, streamConnection.Close())
}

headers.Set(core.StreamType, core.StreamTypeData)
dataStream, err := streamConnection.CreateStream(headers)
if err != nil {
streamConnection.Close()
return nil, err
return nil, errors.Join(err, streamConnection.Close())
}

pipeIn, pipeOut := net.Pipe()
Expand Down
17 changes: 9 additions & 8 deletions tests/validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package tests
import (
"crypto/tls"
"crypto/x509"
"errors"
"fmt"
"reflect"
"strings"
Expand All @@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -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")
}
Expand Down Expand Up @@ -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")
})

Expand Down Expand Up @@ -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")
}

Expand All @@ -953,7 +954,7 @@ func failVmCreationToIncreaseRejectedVmsMetrics(template *templatev1.Template) {
eventuallyFailToCreateVm(vm)
Expect(totalRejectedVmsMetricsValue() - 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)
Expand Down Expand Up @@ -1190,14 +1191,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 {
Expand Down
2 changes: 1 addition & 1 deletion tests/watcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion tools/test-rules-writer/test_rules_writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

0 comments on commit 45c1ccc

Please sign in to comment.