Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Alert .spec.eventMetadata behavior #529

Merged
merged 1 commit into from
May 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 5 additions & 4 deletions api/v1beta2/alert_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`

Expand Down
8 changes: 5 additions & 3 deletions config/crd/bases/notification.toolkit.fluxcd.io_alerts.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 10 additions & 8 deletions docs/api/v1beta2/notification.md
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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>
Expand Down
9 changes: 5 additions & 4 deletions docs/spec/v1beta2/alerts.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
43 changes: 24 additions & 19 deletions internal/server/event_handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"fmt"
"net/http"
"regexp"
"strings"
"time"

"github.com/fluxcd/pkg/runtime/conditions"
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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
}
}
106 changes: 106 additions & 0 deletions internal/server/event_handlers_test.go
Original file line number Diff line number Diff line change
@@ -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))
})
}
}
9 changes: 9 additions & 0 deletions internal/server/event_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down