Fix Alert .spec.eventMetadata behavior

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"`
 

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

docs/api/v1beta2/notification.md

@@ -134,10 +134,11 @@ map[string]string
 </td>
 <td>
 <em>(Optional)</em>
-<p>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.</p>
+<p>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&rsquo;t happen, i.e. the original value is preserved, and an error
+log is printed.</p>
 </td>
 </tr>
 <tr>
@@ -637,10 +638,11 @@ map[string]string
 </td>
 <td>
 <em>(Optional)</em>
-<p>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.</p>
+<p>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&rsquo;t happen, i.e. the original value is preserved, and an error
+log is printed.</p>
 </td>
 </tr>
 <tr>

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
 

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(&notification, 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
+	}
 }

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))
+		})
+	}
+}

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