From 30db06d0df851f773fe01041bcca5a4fdaf95e33 Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Tue, 10 Oct 2023 11:44:53 +0200 Subject: [PATCH 1/6] misc: do not capitalize err strings Except for where names are being used (e.g. `Authorization` header, Nexus, etc.) Signed-off-by: Hidde Beydals --- internal/notifier/azure_eventhub.go | 6 +++--- internal/notifier/github_dispatch.go | 4 ++-- internal/server/receiver_handlers.go | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/notifier/azure_eventhub.go b/internal/notifier/azure_eventhub.go index 15d5f7c87..ed0bd1224 100644 --- a/internal/notifier/azure_eventhub.go +++ b/internal/notifier/azure_eventhub.go @@ -60,17 +60,17 @@ func (e *AzureEventHub) Post(ctx context.Context, event eventv1.Event) error { eventBytes, err := json.Marshal(event) if err != nil { - return fmt.Errorf("Unable to marshall event: %w", err) + return fmt.Errorf("unable to marshall event: %w", err) } err = e.Hub.Send(ctx, eventhub.NewEvent(eventBytes)) if err != nil { - return fmt.Errorf("Failed to send msg: %w", err) + return fmt.Errorf("failed to send msg: %w", err) } err = e.Hub.Close(ctx) if err != nil { - return fmt.Errorf("Unable to close connection: %w", err) + return fmt.Errorf("unable to close connection: %w", err) } return nil } diff --git a/internal/notifier/github_dispatch.go b/internal/notifier/github_dispatch.go index 10aa60f8a..620be483f 100644 --- a/internal/notifier/github_dispatch.go +++ b/internal/notifier/github_dispatch.go @@ -99,7 +99,7 @@ func (g *GitHubDispatch) Post(ctx context.Context, event eventv1.Event) error { eventData, err := json.Marshal(event) if err != nil { - return fmt.Errorf("Failed to marshal object into json: %w", err) + return fmt.Errorf("failed to marshal object into json: %w", err) } eventDataRaw := json.RawMessage(eventData) @@ -110,7 +110,7 @@ func (g *GitHubDispatch) Post(ctx context.Context, event eventv1.Event) error { _, _, err = g.Client.Repositories.Dispatch(ctx, g.Owner, g.Repo, opts) if err != nil { - return fmt.Errorf("Could not send github repository dispatch webhook: %v", err) + return fmt.Errorf("could not send github repository dispatch webhook: %v", err) } return nil diff --git a/internal/server/receiver_handlers.go b/internal/server/receiver_handlers.go index 2e0e19411..b3631153d 100644 --- a/internal/server/receiver_handlers.go +++ b/internal/server/receiver_handlers.go @@ -482,12 +482,12 @@ func authenticateGCRRequest(c *http.Client, bearer string, tokenIndex int) (err resp, err := c.Get(url) if err != nil { - return fmt.Errorf("Cannot verify authenticity of payload: %w", err) + return fmt.Errorf("cannot verify authenticity of payload: %w", err) } var p auth if err := json.NewDecoder(resp.Body).Decode(&p); err != nil { - return fmt.Errorf("Cannot decode auth payload: %w", err) + return fmt.Errorf("cannot decode auth payload: %w", err) } return nil From 23d57eb9d32d1034a6a3f31f7df9173993aafca0 Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Tue, 10 Oct 2023 11:47:04 +0200 Subject: [PATCH 2/6] misc: use `strings.EqualFold` Signed-off-by: Hidde Beydals --- internal/server/receiver_handlers.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/server/receiver_handlers.go b/internal/server/receiver_handlers.go index b3631153d..c55d0063b 100644 --- a/internal/server/receiver_handlers.go +++ b/internal/server/receiver_handlers.go @@ -164,7 +164,7 @@ func (s *ReceiverServer) validate(ctx context.Context, receiver apiv1.Receiver, if len(receiver.Spec.Events) > 0 { allowed := false for _, e := range receiver.Spec.Events { - if strings.ToLower(event) == strings.ToLower(e) { + if strings.EqualFold(event, e) { allowed = true break } @@ -185,7 +185,7 @@ func (s *ReceiverServer) validate(ctx context.Context, receiver apiv1.Receiver, if len(receiver.Spec.Events) > 0 { allowed := false for _, e := range receiver.Spec.Events { - if strings.ToLower(event) == strings.ToLower(e) { + if strings.EqualFold(event, e) { allowed = true break } @@ -207,7 +207,7 @@ func (s *ReceiverServer) validate(ctx context.Context, receiver apiv1.Receiver, if len(receiver.Spec.Events) > 0 { allowed := false for _, e := range receiver.Spec.Events { - if strings.ToLower(event) == strings.ToLower(e) { + if strings.EqualFold(event, e) { allowed = true break } From 16fb90ed8463ea2d8e7ab82b3e4889636bc54eb9 Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Tue, 10 Oct 2023 11:54:21 +0200 Subject: [PATCH 3/6] misc: remove redundant `break` statements Signed-off-by: Hidde Beydals --- internal/notifier/sentry.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/internal/notifier/sentry.go b/internal/notifier/sentry.go index 03e2189c3..6a0396d01 100644 --- a/internal/notifier/sentry.go +++ b/internal/notifier/sentry.go @@ -65,11 +65,9 @@ func (s *Sentry) Post(ctx context.Context, event eventv1.Event) error { case eventv1.EventSeverityInfo: // Info is sent as a trace sev = eventToSpan(event) - break case eventv1.EventSeverityError: // Errors are sent as normal events sev = toSentryEvent(event) - break } s.Client.CaptureEvent(sev, nil, nil) From bdda58062fedbd06ed4ea156f9484d946a2beb60 Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Tue, 10 Oct 2023 11:58:20 +0200 Subject: [PATCH 4/6] misc: handle `strings.Title` deprecation Signed-off-by: Hidde Beydals --- go.mod | 2 +- internal/notifier/alertmanager.go | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 2019f483f..d3b5fd3fa 100644 --- a/go.mod +++ b/go.mod @@ -34,6 +34,7 @@ require ( github.com/whilp/git-urls v1.0.0 github.com/xanzy/go-gitlab v0.90.0 golang.org/x/oauth2 v0.11.0 + golang.org/x/text v0.12.0 google.golang.org/api v0.138.0 k8s.io/api v0.27.4 k8s.io/apimachinery v0.27.4 @@ -140,7 +141,6 @@ require ( golang.org/x/sync v0.3.0 // indirect golang.org/x/sys v0.11.0 // indirect golang.org/x/term v0.11.0 // indirect - golang.org/x/text v0.12.0 // indirect golang.org/x/time v0.3.0 // indirect gomodules.xyz/jsonpatch/v2 v2.3.0 // indirect google.golang.org/appengine v1.6.7 // indirect diff --git a/internal/notifier/alertmanager.go b/internal/notifier/alertmanager.go index a762b2509..3bfa352d0 100644 --- a/internal/notifier/alertmanager.go +++ b/internal/notifier/alertmanager.go @@ -21,7 +21,9 @@ import ( "crypto/x509" "fmt" "net/url" - "strings" + + "golang.org/x/text/cases" + "golang.org/x/text/language" eventv1 "github.com/fluxcd/pkg/apis/event/v1beta1" ) @@ -70,7 +72,7 @@ func (s *Alertmanager) Post(ctx context.Context, event eventv1.Event) error { if event.Metadata != nil { labels = event.Metadata } - labels["alertname"] = "Flux" + event.InvolvedObject.Kind + strings.Title(event.Reason) + labels["alertname"] = "Flux" + event.InvolvedObject.Kind + cases.Title(language.Und).String(event.Reason) labels["severity"] = event.Severity labels["reason"] = event.Reason labels["timestamp"] = event.Timestamp.String() From edb6a54e23b9437f68e69fda3a1d95c1186c3fd9 Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Tue, 10 Oct 2023 11:59:28 +0200 Subject: [PATCH 5/6] misc: take errs into account Signed-off-by: Hidde Beydals --- internal/server/event_server.go | 6 +++++- internal/server/receiver_handler_test.go | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/internal/server/event_server.go b/internal/server/event_server.go index 8183a8312..aa9a0b38c 100644 --- a/internal/server/event_server.go +++ b/internal/server/event_server.go @@ -114,7 +114,11 @@ func (s *EventServer) eventMiddleware(h http.Handler) http.Handler { w.WriteHeader(http.StatusBadRequest) return } - r.Body.Close() + if err := r.Body.Close(); err != nil { + s.logger.Error(err, "closing the request body failed") + w.WriteHeader(http.StatusBadRequest) + return + } r.Body = io.NopCloser(bytes.NewBuffer(body)) event := &eventv1.Event{} diff --git a/internal/server/receiver_handler_test.go b/internal/server/receiver_handler_test.go index 35d317dce..c72ff0057 100644 --- a/internal/server/receiver_handler_test.go +++ b/internal/server/receiver_handler_test.go @@ -677,7 +677,7 @@ func Test_handlePayload(t *testing.T) { g.Expect(rr.Result().StatusCode).To(gomega.Equal(tt.expectedResponseCode)) var allReceivers apiv1.ReceiverList - err = client.List(context.TODO(), &allReceivers) + g.Expect(client.List(context.TODO(), &allReceivers)).To(gomega.Succeed()) var annotatedResources int for _, obj := range allReceivers.Items { From 3dce75a07ebbfd4995b52589b3b0e14e604f0085 Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Tue, 10 Oct 2023 12:02:33 +0200 Subject: [PATCH 6/6] misc: fix hypothetical implicit memory aliasing Signed-off-by: Hidde Beydals --- internal/controller/alert_controller.go | 4 ++-- internal/server/event_handlers.go | 10 +++++----- internal/server/receiver_handlers.go | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/internal/controller/alert_controller.go b/internal/controller/alert_controller.go index 0616f232a..e0afbe21b 100644 --- a/internal/controller/alert_controller.go +++ b/internal/controller/alert_controller.go @@ -206,8 +206,8 @@ func (r *AlertReconciler) requestsForProviderChange(ctx context.Context, o clien } var reqs []reconcile.Request - for _, i := range list.Items { - reqs = append(reqs, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(&i)}) + for i := range list.Items { + reqs = append(reqs, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(&list.Items[i])}) } return reqs diff --git a/internal/server/event_handlers.go b/internal/server/event_handlers.go index b0a57227f..1cd1adee1 100644 --- a/internal/server/event_handlers.go +++ b/internal/server/event_handlers.go @@ -61,12 +61,12 @@ func (s *EventServer) handleEvent() func(w http.ResponseWriter, r *http.Request) // find matching alerts alerts := make([]apiv1beta2.Alert, 0) each_alert: - for _, alert := range allAlerts.Items { - alertLogger := eventLogger.WithValues("alert", client.ObjectKeyFromObject(&alert)) + for i, alert := range allAlerts.Items { + alertLogger := eventLogger.WithValues("alert", client.ObjectKeyFromObject(&allAlerts.Items[i])) ctx := log.IntoContext(ctx, alertLogger) // skip suspended and not ready alerts - isReady := conditions.IsReady(&alert) + isReady := conditions.IsReady(&allAlerts.Items[i]) if alert.Spec.Suspend || !isReady { continue each_alert } @@ -123,8 +123,8 @@ func (s *EventServer) handleEvent() func(w http.ResponseWriter, r *http.Request) eventLogger.Info(fmt.Sprintf("Dispatching event: %s", event.Message)) // dispatch notifications - for _, alert := range alerts { - alertLogger := eventLogger.WithValues("alert", client.ObjectKeyFromObject(&alert)) + for i, alert := range alerts { + alertLogger := eventLogger.WithValues("alert", client.ObjectKeyFromObject(&alerts[i])) ctx := log.IntoContext(ctx, alertLogger) // verify if event comes from a different namespace diff --git a/internal/server/receiver_handlers.go b/internal/server/receiver_handlers.go index c55d0063b..28688107d 100644 --- a/internal/server/receiver_handlers.go +++ b/internal/server/receiver_handlers.go @@ -408,8 +408,8 @@ func (s *ReceiverServer) requestReconciliation(ctx context.Context, logger logr. return nil } - for _, resource := range resources.Items { - if err := s.annotate(ctx, &resource); err != nil { + for i, resource := range resources.Items { + if err := s.annotate(ctx, &resources.Items[i]); err != nil { return fmt.Errorf("failed to annotate resource: '%s/%s.%s': %w", resource.Kind, resource.Name, namespace, err) } else { logger.Info(fmt.Sprintf("resource '%s/%s.%s' annotated",