From 8c11d8a19baa0bbd019865fb9d748cf625fa2147 Mon Sep 17 00:00:00 2001 From: Matheus Pimenta Date: Thu, 18 May 2023 12:26:49 +0200 Subject: [PATCH] Fix Alert .spec.eventMetadata behavior Signed-off-by: Matheus Pimenta --- api/v1beta2/alert_types.go | 9 +- ...notification.toolkit.fluxcd.io_alerts.yaml | 8 +- docs/api/v1beta2/notification.md | 18 +-- docs/spec/v1beta2/alerts.md | 9 +- internal/server/event_handlers.go | 43 +++---- internal/server/event_handlers_test.go | 106 ++++++++++++++++++ internal/server/event_server.go | 9 ++ 7 files changed, 164 insertions(+), 38 deletions(-) create mode 100644 internal/server/event_handlers_test.go diff --git a/api/v1beta2/alert_types.go b/api/v1beta2/alert_types.go index 3a9e2f7c3..f1ea5a105 100644 --- a/api/v1beta2/alert_types.go +++ b/api/v1beta2/alert_types.go @@ -50,10 +50,11 @@ type AlertSpec struct { // +optional InclusionList []string `json:"inclusionList,omitempty"` - // EventMetadata is an optional field for adding metadata to events emitted by the - // controller. Metadata fields added by the controller have priority over the fields - // added here, and the fields added here have priority over fields originally present - // in the event. + // EventMetadata is an optional field for adding metadata to events dispatched by the + // controller. This can be used for enhancing the context of the event. If a field + // would override one already present on the original event as generated by the emitter, + // then the override doesn't happen, i.e. the original value is preserved, and an error + // log is printed. // +optional EventMetadata map[string]string `json:"eventMetadata,omitempty"` diff --git a/config/crd/bases/notification.toolkit.fluxcd.io_alerts.yaml b/config/crd/bases/notification.toolkit.fluxcd.io_alerts.yaml index 6a0e6c1bf..a899e18dd 100644 --- a/config/crd/bases/notification.toolkit.fluxcd.io_alerts.yaml +++ b/config/crd/bases/notification.toolkit.fluxcd.io_alerts.yaml @@ -243,9 +243,11 @@ spec: additionalProperties: type: string description: EventMetadata is an optional field for adding metadata - to events emitted by the controller. Metadata fields added by the - controller have priority over the fields added here, and the fields - added here have priority over fields originally present in the event. + to events dispatched by the controller. This can be used for enhancing + the context of the event. If a field would override one already + present on the original event as generated by the emitter, then + the override doesn't happen, i.e. the original value is preserved, + and an error log is printed. type: object eventSeverity: default: info diff --git a/docs/api/v1beta2/notification.md b/docs/api/v1beta2/notification.md index d097025a5..e48ac49a7 100644 --- a/docs/api/v1beta2/notification.md +++ b/docs/api/v1beta2/notification.md @@ -134,10 +134,11 @@ map[string]string (Optional) -

EventMetadata is an optional field for adding metadata to events emitted by the -controller. Metadata fields added by the controller have priority over the fields -added here, and the fields added here have priority over fields originally present -in the event.

+

EventMetadata is an optional field for adding metadata to events dispatched by the +controller. This can be used for enhancing the context of the event. If a field +would override one already present on the original event as generated by the emitter, +then the override doesn’t happen, i.e. the original value is preserved, and an error +log is printed.

@@ -637,10 +638,11 @@ map[string]string (Optional) -

EventMetadata is an optional field for adding metadata to events emitted by the -controller. Metadata fields added by the controller have priority over the fields -added here, and the fields added here have priority over fields originally present -in the event.

+

EventMetadata is an optional field for adding metadata to events dispatched by the +controller. This can be used for enhancing the context of the event. If a field +would override one already present on the original event as generated by the emitter, +then the override doesn’t happen, i.e. the original value is preserved, and an error +log is printed.

diff --git a/docs/spec/v1beta2/alerts.md b/docs/spec/v1beta2/alerts.md index a8aba153e..b03ab4d17 100644 --- a/docs/spec/v1beta2/alerts.md +++ b/docs/spec/v1beta2/alerts.md @@ -164,10 +164,11 @@ preventing tenants from subscribing to another tenant's events. ### Event metadata -`.spec.eventMetadata` is an optional field for adding metadata to events emitted by the -controller. Metadata fields added by the controller have priority over the fields -added here, and the fields added here have priority over fields originally present -in the event. +`.spec.eventMetadata` is an optional field for adding metadata to events dispatched by +the controller. This can be used for enhancing the context of the event. If a field +would override one already present on the original event as generated by the emitter, +then the override doesn't happen, i.e. the original value is preserved, and an error +log is printed. #### Example diff --git a/internal/server/event_handlers.go b/internal/server/event_handlers.go index 553ebfaa9..c49081221 100644 --- a/internal/server/event_handlers.go +++ b/internal/server/event_handlers.go @@ -23,7 +23,6 @@ import ( "fmt" "net/http" "regexp" - "strings" "time" "github.com/fluxcd/pkg/runtime/conditions" @@ -254,19 +253,7 @@ func (s *EventServer) handleEvent() func(w http.ResponseWriter, r *http.Request) } notification := *event.DeepCopy() - meta := notification.Metadata - if meta == nil { - meta = make(map[string]string) - } - for key, value := range alert.Spec.EventMetadata { - meta[key] = value - } - if alert.Spec.Summary != "" { - meta["summary"] = alert.Spec.Summary - } - if len(meta) > 0 { - notification.Metadata = meta - } + s.enhanceEventWithAlertMetadata(¬ification, alert) go func(n notifier.Interface, e eventv1.Event) { ctx, cancel := context.WithTimeout(context.Background(), provider.GetTimeout()) @@ -327,11 +314,29 @@ func (s *EventServer) eventMatchesAlert(ctx context.Context, event *eventv1.Even return false } -func inList(l []string, i string) bool { - for _, v := range l { - if strings.EqualFold(v, i) { - return true +func (s *EventServer) enhanceEventWithAlertMetadata(event *eventv1.Event, alert apiv1beta2.Alert) { + meta := event.Metadata + if meta == nil { + meta = make(map[string]string) + } + + for key, value := range alert.Spec.EventMetadata { + if _, alreadyPresent := meta[key]; !alreadyPresent { + meta[key] = value + } else { + s.logger.Info("metadata key found in the existing set of metadata", + "reconciler kind", apiv1beta2.AlertKind, + "name", alert.Name, + "namespace", alert.Namespace, + "key", key) } } - return false + + if alert.Spec.Summary != "" { + meta["summary"] = alert.Spec.Summary + } + + if len(meta) > 0 { + event.Metadata = meta + } } diff --git a/internal/server/event_handlers_test.go b/internal/server/event_handlers_test.go new file mode 100644 index 000000000..668802e0a --- /dev/null +++ b/internal/server/event_handlers_test.go @@ -0,0 +1,106 @@ +/* +Copyright 2023 The Flux authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package server + +import ( + "testing" + + "github.com/go-logr/logr" + . "github.com/onsi/gomega" + + apiv1beta2 "github.com/fluxcd/notification-controller/api/v1beta2" + eventv1 "github.com/fluxcd/pkg/apis/event/v1beta1" +) + +func TestEnhanceEventWithAlertMetadata(t *testing.T) { + s := &EventServer{logger: logr.New(nil)} + + for name, tt := range map[string]struct { + event eventv1.Event + alert apiv1beta2.Alert + expectedMetadata map[string]string + }{ + "empty metadata": { + event: eventv1.Event{}, + alert: apiv1beta2.Alert{}, + expectedMetadata: nil, + }, + "enhanced with summary": { + event: eventv1.Event{}, + alert: apiv1beta2.Alert{ + Spec: apiv1beta2.AlertSpec{ + Summary: "summary", + }, + }, + expectedMetadata: map[string]string{ + "summary": "summary", + }, + }, + "overriden with summary": { + event: eventv1.Event{ + Metadata: map[string]string{ + "summary": "original summary", + }, + }, + alert: apiv1beta2.Alert{ + Spec: apiv1beta2.AlertSpec{ + Summary: "summary", + }, + }, + expectedMetadata: map[string]string{ + "summary": "summary", + }, + }, + "enhanced with metadata": { + event: eventv1.Event{}, + alert: apiv1beta2.Alert{ + Spec: apiv1beta2.AlertSpec{ + EventMetadata: map[string]string{ + "foo": "bar", + }, + }, + }, + expectedMetadata: map[string]string{ + "foo": "bar", + }, + }, + "skipped override with metadata": { + event: eventv1.Event{ + Metadata: map[string]string{ + "foo": "baz", + }, + }, + alert: apiv1beta2.Alert{ + Spec: apiv1beta2.AlertSpec{ + EventMetadata: map[string]string{ + "foo": "bar", + }, + }, + }, + expectedMetadata: map[string]string{ + "foo": "baz", + }, + }, + } { + t.Run(name, func(t *testing.T) { + g := NewGomegaWithT(t) + + s.enhanceEventWithAlertMetadata(&tt.event, tt.alert) + g.Expect(tt.event.Metadata).To(BeEquivalentTo(tt.expectedMetadata)) + }) + } +} diff --git a/internal/server/event_server.go b/internal/server/event_server.go index ab61f9d3f..e7fb8e06b 100644 --- a/internal/server/event_server.go +++ b/internal/server/event_server.go @@ -153,6 +153,15 @@ func cleanupMetadata(event *eventv1.Event) { event.Metadata = meta } +func inList(l []string, i string) bool { + for _, v := range l { + if strings.EqualFold(v, i) { + return true + } + } + return false +} + type statusRecorder struct { http.ResponseWriter Status int