Skip to content

Commit c3d1c4b

Browse files
committed
cleanup: Handle errors
The unhandled errors were found by Goland code inspection. Signed-off-by: Andrej Krejcir <[email protected]>
1 parent d96d1bd commit c3d1c4b

File tree

15 files changed

+62
-45
lines changed

15 files changed

+62
-45
lines changed

hack/csv-generator.go

+4-2
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,8 @@ var (
5858
Run: func(cmd *cobra.Command, args []string) {
5959
err := runGenerator()
6060
if err != nil {
61-
fmt.Fprintf(os.Stderr, "%v\n", err)
61+
// Ignoring returned error: no reasonable way to handle it.
62+
_, _ = fmt.Fprintf(os.Stderr, "%v\n", err)
6263
os.Exit(1)
6364
}
6465
},
@@ -67,7 +68,8 @@ var (
6768

6869
func main() {
6970
if err := rootCmd.Execute(); err != nil {
70-
fmt.Fprintf(os.Stderr, "%v\n", err)
71+
// Ignoring returned error: no reasonable way to handle it.
72+
_, _ = fmt.Fprintf(os.Stderr, "%v\n", err)
7173
os.Exit(1)
7274
}
7375
}

internal/env/env_test.go

+3-2
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,11 @@ import (
1212

1313
var _ = Describe("environments", func() {
1414
It("should return correct value for OPERATOR_VERSION when variable is set", func() {
15-
os.Setenv(OperatorVersionKey, "v0.0.1")
15+
Expect(os.Setenv(OperatorVersionKey, "v0.0.1")).To(Succeed())
16+
defer Expect(os.Unsetenv(OperatorVersionKey)).To(Succeed())
17+
1618
res := GetOperatorVersion()
1719
Expect(res).To(Equal("v0.0.1"), "OPERATOR_VERSION should equal")
18-
os.Unsetenv(OperatorVersionKey)
1920
})
2021

2122
It("should return correct value for OPERATOR_VERSION when variable is not set", func() {

internal/operands/metrics/reconcile_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ var _ = Describe("Metrics operand", func() {
6363
})
6464

6565
AfterEach(func() {
66-
os.Unsetenv(runbookURLTemplateEnv)
66+
Expect(os.Unsetenv(runbookURLTemplateEnv)).To(Succeed())
6767
})
6868

6969
It("should create metrics resources", func() {
@@ -82,7 +82,7 @@ var _ = Describe("Metrics operand", func() {
8282
DescribeTable("runbook URL template",
8383
func(template string) {
8484
if template != defaultRunbookURLTemplate {
85-
os.Setenv(runbookURLTemplateEnv, template)
85+
Expect(os.Setenv(runbookURLTemplateEnv, template)).To(Succeed())
8686
}
8787

8888
err := rules.SetupRules()

internal/template-validator/validation/eval.go

+13-6
Original file line numberDiff line numberDiff line change
@@ -160,42 +160,49 @@ func (ev *Evaluator) Evaluate(rules []Rule, vm *k6tv1.VirtualMachine) *Result {
160160
// Otherwise, if we simply skip the malformed rule, the error can go unnoticed.
161161
// IOW, this is a policy decision
162162
if _, ok := uniqueNames[r.Name]; ok {
163-
fmt.Fprintf(ev.Sink, "%s failed: duplicate name\n", r.Name)
163+
// Ignoring returned error: This print is used only for logging
164+
_, _ = fmt.Fprintf(ev.Sink, "%s failed: duplicate name\n", r.Name)
164165
result.Fail(r, ErrDuplicateRuleName)
165166
continue
166167
}
167168
uniqueNames[r.Name] = struct{}{}
168169

169170
if err := validateRule(r); err != nil {
170-
fmt.Fprintf(ev.Sink, "%s failed: %v\n", r.Name, err)
171+
// Ignoring returned error: This print is used only for logging
172+
_, _ = fmt.Fprintf(ev.Sink, "%s failed: %v\n", r.Name, err)
171173
result.Fail(r, err)
172174
continue
173175
}
174176

175177
// Specialize() may be costly, so we do this before.
176178
if !r.IsAppliableOn(vm) {
177179
// Legit case. Nothing to do or to complain.
178-
fmt.Fprintf(ev.Sink, "%s SKIPPED: not appliable\n", r.Name)
180+
181+
// Ignoring returned error: This print is used only for logging
182+
_, _ = fmt.Fprintf(ev.Sink, "%s SKIPPED: not appliable\n", r.Name)
179183
result.Skip(r)
180184
continue
181185
}
182186

183187
ra, err := r.Specialize(vm, refVm)
184188
if err != nil {
185-
fmt.Fprintf(ev.Sink, "%s failed: cannot specialize: %v\n", r.Name, err)
189+
// Ignoring returned error: This print is used only for logging
190+
_, _ = fmt.Fprintf(ev.Sink, "%s failed: cannot specialize: %v\n", r.Name, err)
186191
result.Fail(r, err)
187192
continue
188193
}
189194

190195
satisfied, err := ra.Apply(vm, refVm)
191196
if err != nil {
192-
fmt.Fprintf(ev.Sink, "%s failed: cannot apply: %v\n", r.Name, err)
197+
// Ignoring returned error: This print is used only for logging
198+
_, _ = fmt.Fprintf(ev.Sink, "%s failed: cannot apply: %v\n", r.Name, err)
193199
result.Fail(r, err)
194200
continue
195201
}
196202

197203
applicationText := ra.String()
198-
fmt.Fprintf(ev.Sink, "%s applied: %v, %s\n", r.Name, boolAsStatus(satisfied), applicationText)
204+
// Ignoring returned error: This print is used only for logging
205+
_, _ = fmt.Fprintf(ev.Sink, "%s applied: %v, %s\n", r.Name, boolAsStatus(satisfied), applicationText)
199206
result.Applied(r, satisfied, applicationText)
200207
}
201208

internal/template-validator/validation/eval_test.go

+4-2
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,8 @@ var _ = Describe("Eval", func() {
285285
Expect(res.Succeeded()).To(BeTrue())
286286

287287
for ix := range res.Status {
288-
fmt.Fprintf(GinkgoWriter, "%+#v", res.Status[ix])
288+
_, err := fmt.Fprintf(GinkgoWriter, "%+#v", res.Status[ix])
289+
Expect(err).ToNot(HaveOccurred())
289290
}
290291

291292
Expect(res.Status).To(HaveLen(len(rules)))
@@ -342,7 +343,8 @@ var _ = Describe("Eval", func() {
342343
res := ev.Evaluate(rules, vmCirros)
343344

344345
for ix := range res.Status {
345-
fmt.Fprintf(GinkgoWriter, "%+#v", res.Status[ix])
346+
_, err := fmt.Fprintf(GinkgoWriter, "%+#v", res.Status[ix])
347+
Expect(err).ToNot(HaveOccurred())
346348
}
347349

348350
Expect(res.Succeeded()).To(BeFalse(), "succeeded")

internal/template-validator/validator/app.go

+3-2
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package validator
22

33
import (
44
"crypto/tls"
5-
"fmt"
65
"net/http"
76
"os"
87

@@ -130,7 +129,9 @@ func (app *App) Run() {
130129

131130
func registerReadinessProbe() {
132131
http.HandleFunc("/readyz", func(resp http.ResponseWriter, req *http.Request) {
133-
fmt.Fprintf(resp, "ok")
132+
if _, err := resp.Write([]byte("ok")); err != nil {
133+
logger.Log.Error(err, "Failed to write response to /readyz")
134+
}
134135
})
135136
}
136137

main.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package main
1919
import (
2020
"context"
2121
"crypto/tls"
22+
"errors"
2223
"flag"
2324
"fmt"
2425
"net/http"
@@ -164,7 +165,7 @@ func (s *prometheusServer) Start(ctx context.Context) error {
164165

165166
server.TLSConfig = s.getPrometheusTLSConfig(ctx, certWatcher)
166167

167-
if err := server.ListenAndServeTLS(s.certPath, s.keyPath); err != nil && err != http.ErrServerClosed {
168+
if err := server.ListenAndServeTLS(s.certPath, s.keyPath); err != nil && !errors.Is(err, http.ErrServerClosed) {
168169
setupLog.Error(err, "Failed to start Prometheus metrics endpoint server")
169170
return err
170171
}

tests/crypto_policy_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ func getCaCertificate() []byte {
204204
func tryToAccessEndpoint(pod core.Pod, serviceName string, subpath string, port uint16, tlsConfig clientTLSOptions, insecure bool) (attemptedUrl string, err error) {
205205
conn, err := portForwarder.Connect(&pod, port)
206206
Expect(err).ToNot(HaveOccurred())
207-
defer conn.Close()
207+
defer Expect(conn.Close()).To(Succeed())
208208

209209
certPool := x509.NewCertPool()
210210
certPool.AppendCertsFromPEM(getCaCertificate())

tests/labels_test.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,8 @@ func encodePatch(operations []jsonpatch.Operation) client.Patch {
5858
patchBytes, err := json.Marshal(operations)
5959
Expect(err).NotTo(HaveOccurred())
6060

61-
fmt.Fprintf(GinkgoWriter, "sending patch: %s", string(patchBytes))
61+
_, err = fmt.Fprintf(GinkgoWriter, "sending patch: %s", string(patchBytes))
62+
Expect(err).NotTo(HaveOccurred())
6263
return client.RawPatch(types.JSONPatchType, patchBytes)
6364
}
6465

tests/metrics_test_utils.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ func intMetricValue(metricName string, metricsPort uint16, pod *v1.Pod) int {
3434
}
3535
resp, err := client.Get(fmt.Sprintf("https://localhost:%d/metrics", metricsPort))
3636
Expect(err).ToNot(HaveOccurred(), "Can't get metrics from %s", pod.Name)
37-
defer resp.Body.Close()
37+
defer Expect(resp.Body.Close()).To(Succeed())
3838
body, _ := io.ReadAll(resp.Body)
3939
regex, ok := regexpForMetrics[metricName]
4040
if !ok {

tests/misc_test.go

+9-10
Original file line numberDiff line numberDiff line change
@@ -372,19 +372,18 @@ var _ = Describe("RHEL VM creation", func() {
372372

373373
func logObject(key client.ObjectKey, obj client.Object) {
374374
gvk, err := apiutil.GVKForObject(obj, testScheme)
375-
if err != nil {
376-
panic(err)
377-
}
375+
Expect(err).ToNot(HaveOccurred())
378376
obj.GetObjectKind().SetGroupVersionKind(gvk)
379377

380378
err = apiClient.Get(ctx, key, obj)
381379
if err != nil {
382-
fmt.Fprintf(GinkgoWriter, "Failed to get %s: %s\n", gvk.Kind, err)
383-
} else {
384-
objJson, err := json.MarshalIndent(obj, "", " ")
385-
if err != nil {
386-
panic(err)
387-
}
388-
fmt.Fprintf(GinkgoWriter, "Found %s:\n%s\n", gvk.Kind, objJson)
380+
_, printErr := fmt.Fprintf(GinkgoWriter, "Failed to get %s: %s\n", gvk.Kind, err)
381+
Expect(printErr).ToNot(HaveOccurred())
382+
return
389383
}
384+
385+
objJson, err := json.MarshalIndent(obj, "", " ")
386+
Expect(err).ToNot(HaveOccurred())
387+
_, printErr := fmt.Fprintf(GinkgoWriter, "Found %s:\n%s\n", gvk.Kind, objJson)
388+
Expect(printErr).ToNot(HaveOccurred())
390389
}

tests/port-forwarding.go

+6-5
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package tests
22

33
import (
4+
"errors"
45
"fmt"
56
"io"
67
"net"
@@ -43,17 +44,17 @@ func (p *portForwarderImpl) Connect(pod *core.Pod, remotePort uint16) (net.Conn,
4344
headers.Set(core.PortForwardRequestIDHeader, strconv.Itoa(int(requestId)))
4445
errorStream, err := streamConnection.CreateStream(headers)
4546
if err != nil {
46-
streamConnection.Close()
47-
return nil, err
47+
return nil, errors.Join(err, streamConnection.Close())
4848
}
4949
// We will not write to error stream
50-
errorStream.Close()
50+
if err := errorStream.Close(); err != nil {
51+
return nil, errors.Join(err, streamConnection.Close())
52+
}
5153

5254
headers.Set(core.StreamType, core.StreamTypeData)
5355
dataStream, err := streamConnection.CreateStream(headers)
5456
if err != nil {
55-
streamConnection.Close()
56-
return nil, err
57+
return nil, errors.Join(err, streamConnection.Close())
5758
}
5859

5960
pipeIn, pipeOut := net.Pipe()

tests/validator_test.go

+9-8
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package tests
33
import (
44
"crypto/tls"
55
"crypto/x509"
6+
"errors"
67
"fmt"
78
"reflect"
89
"strings"
@@ -17,7 +18,7 @@ import (
1718
apps "k8s.io/api/apps/v1"
1819
core "k8s.io/api/core/v1"
1920
rbac "k8s.io/api/rbac/v1"
20-
"k8s.io/apimachinery/pkg/api/errors"
21+
k8serrors "k8s.io/apimachinery/pkg/api/errors"
2122
"k8s.io/apimachinery/pkg/api/resource"
2223
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2324
"k8s.io/apimachinery/pkg/runtime"
@@ -446,7 +447,7 @@ var _ = Describe("Template validator webhooks", func() {
446447
AfterEach(func() {
447448
if vm != nil {
448449
err := apiClient.Delete(ctx, vm)
449-
if !errors.IsNotFound(err) {
450+
if !k8serrors.IsNotFound(err) {
450451
Expect(err).ToNot(HaveOccurred(), "Failed to Delete VM")
451452
}
452453
// Need to wait until VM is removed, otherwise the webhook may
@@ -456,7 +457,7 @@ var _ = Describe("Template validator webhooks", func() {
456457
if template != nil {
457458
Eventually(func(g Gomega) {
458459
if err := apiClient.Delete(ctx, template); err != nil {
459-
g.Expect(errors.ReasonForError(err)).To(Equal(metav1.StatusReasonNotFound))
460+
g.Expect(k8serrors.ReasonForError(err)).To(Equal(metav1.StatusReasonNotFound))
460461
}
461462
}, env.ShortTimeout(), time.Second).Should(Succeed(), "Template should be deleted")
462463
}
@@ -823,7 +824,7 @@ var _ = Describe("Template validator webhooks", func() {
823824

824825
Eventually(func() metav1.StatusReason {
825826
err := apiClient.Delete(ctx, template, client.DryRunAll)
826-
return errors.ReasonForError(err)
827+
return k8serrors.ReasonForError(err)
827828
}, env.ShortTimeout(), 1*time.Second).Should(Equal(metav1.StatusReasonForbidden), "Should have given forbidden error")
828829
})
829830

@@ -936,7 +937,7 @@ func eventuallyFailToCreateVm(vm *kubevirtv1.VirtualMachine) bool {
936937
}
937938
return metav1.StatusReasonUnknown, fmt.Errorf("VM was created")
938939
}
939-
return errors.ReasonForError(err), nil
940+
return k8serrors.ReasonForError(err), nil
940941
}, env.ShortTimeout()).Should(Equal(metav1.StatusReasonInvalid), "Should have given the invalid error")
941942
}
942943

@@ -953,7 +954,7 @@ func failVmCreationToIncreaseRejectedVmsMetrics(template *templatev1.Template) {
953954
eventuallyFailToCreateVm(vm)
954955
Expect(totalRejectedVmsMetricsValue() - rejectedCountBefore).To(Equal(1))
955956
err := apiClient.Delete(ctx, vm)
956-
if !errors.IsNotFound(err) {
957+
if !k8serrors.IsNotFound(err) {
957958
Expect(err).ToNot(HaveOccurred(), "Failed to Delete VM")
958959
}
959960
waitForDeletion(client.ObjectKeyFromObject(vm), vm)
@@ -1190,14 +1191,14 @@ func TemplateWithoutRules() *templatev1.Template {
11901191
return addObjectsToTemplates("test-fedora-desktop-small-without-rules", validations)
11911192
}
11921193

1193-
func getWebhookServerCertificates(validatorPod *core.Pod) ([]*x509.Certificate, error) {
1194+
func getWebhookServerCertificates(validatorPod *core.Pod) (certs []*x509.Certificate, err error) {
11941195
conn, err := portForwarder.Connect(validatorPod, validator.ContainerPort)
11951196
if err != nil {
11961197
return nil, err
11971198
}
11981199

11991200
tlsConn := tls.Client(conn, &tls.Config{InsecureSkipVerify: true})
1200-
defer tlsConn.Close()
1201+
defer func() { err = errors.Join(err, tlsConn.Close()) }()
12011202

12021203
err = tlsConn.Handshake()
12031204
if err != nil {

tests/watcher.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ func (s *sspWatch) startWatch() {
8585

8686
err = s.handleWatch(w)
8787
if err != nil {
88-
if err != errStopWatch {
88+
if errors.Is(err, errStopWatch) {
8989
s.atomicError.Store(err)
9090
}
9191
return

tools/test-rules-writer/test_rules_writer.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@ func main() {
2121
encoder := json.NewEncoder(os.Stdout)
2222
encoder.SetIndent("", " ")
2323
if err := encoder.Encode(pr.Spec); err != nil {
24-
fmt.Fprintf(os.Stderr, "Error encoding prometheus spec: %v", err)
24+
// Ignoring returned error: no reasonable way to handle it.
25+
_, _ = fmt.Fprintf(os.Stderr, "Error encoding prometheus spec: %v", err)
2526
os.Exit(1)
2627
}
2728
}

0 commit comments

Comments
 (0)