From 08f58b2346574009a71787030dea2ed8931368e4 Mon Sep 17 00:00:00 2001 From: tonypowa Date: Mon, 3 Nov 2025 11:42:03 +0100 Subject: [PATCH] Alerting: Handle placeholder email addresses gracefully in default contact point This change prevents errors when using the default grafana-default-email contact point with its placeholder email address . Changes include: - Skip sending emails to placeholder addresses without throwing errors - Show warning message in UI when testing contact points with placeholder emails - Filter out placeholder addresses when mixed with valid email addresses - Add unit tests for placeholder email detection and handling - Handle empty recipient lists gracefully in SMTP client The UI now displays a yellow warning box with a helpful message prompting users to configure a valid email address. Backend logs now show an informative message when skipping placeholder emails: INFO Skipping email notification to placeholder address(es). Please configure a valid email address in your contact point to receive alerts. logger=ngalert.notifier.sender addresses=[] --- pkg/services/ngalert/notifier/sender.go | 35 +++- pkg/services/ngalert/notifier/sender_test.go | 194 ++++++++++++++++++ pkg/services/notifications/smtp.go | 5 + .../alerting/unified/state/actions.ts | 67 +++++- 4 files changed, 298 insertions(+), 3 deletions(-) create mode 100644 pkg/services/ngalert/notifier/sender_test.go diff --git a/pkg/services/ngalert/notifier/sender.go b/pkg/services/ngalert/notifier/sender.go index 7b8710778d7..aa8bc76051c 100644 --- a/pkg/services/ngalert/notifier/sender.go +++ b/pkg/services/ngalert/notifier/sender.go @@ -2,19 +2,52 @@ package notifier import ( "context" + "strings" "github.com/grafana/alerting/receivers" + "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/notifications" ) +const ( + // placeholderEmailAddress is the default placeholder email address used when no real email is configured + placeholderEmailAddress = "" +) + +var logger = log.New("ngalert.notifier.sender") + type emailSender struct { ns notifications.Service } +// isPlaceholderEmail checks if the given email address is a placeholder that should not be sent +func isPlaceholderEmail(email string) bool { + trimmed := strings.TrimSpace(email) + return trimmed == placeholderEmailAddress || trimmed == "example@email.com" +} + func (s emailSender) SendEmail(ctx context.Context, cmd *receivers.SendEmailSettings) error { + // Filter out placeholder addresses from the recipient list (single loop) + validRecipients := make([]string, 0, len(cmd.To)) + for _, addr := range cmd.To { + if !isPlaceholderEmail(addr) { + validRecipients = append(validRecipients, addr) + } else { + logger.Warn("Filtering out placeholder email address from recipients", "address", addr) + } + } + + // If no valid recipients remain, skip sending + if len(validRecipients) == 0 { + if len(cmd.To) > 0 { + logger.Info("Skipping email notification to placeholder address(es). Please configure a valid email address in your contact point to receive alerts.", "addresses", cmd.To) + } + return nil + } + sendEmailCommand := notifications.SendEmailCommand{ - To: cmd.To, + To: validRecipients, SingleEmail: cmd.SingleEmail, Template: cmd.Template, Subject: cmd.Subject, diff --git a/pkg/services/ngalert/notifier/sender_test.go b/pkg/services/ngalert/notifier/sender_test.go new file mode 100644 index 00000000000..450c9da37c8 --- /dev/null +++ b/pkg/services/ngalert/notifier/sender_test.go @@ -0,0 +1,194 @@ +package notifier + +import ( + "context" + "testing" + + "github.com/grafana/alerting/receivers" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/grafana/grafana/pkg/services/notifications" + "github.com/grafana/grafana/pkg/services/user" +) + +func TestIsPlaceholderEmail(t *testing.T) { + tests := []struct { + name string + email string + expected bool + }{ + { + name: "placeholder with angle brackets", + email: "", + expected: true, + }, + { + name: "placeholder without angle brackets", + email: "example@email.com", + expected: true, + }, + { + name: "placeholder with spaces", + email: " ", + expected: true, + }, + { + name: "valid email", + email: "user@example.com", + expected: false, + }, + { + name: "another valid email", + email: "admin@grafana.com", + expected: false, + }, + { + name: "empty string", + email: "", + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := isPlaceholderEmail(tt.email) + assert.Equal(t, tt.expected, result) + }) + } +} + +type mockNotificationService struct { + sendEmailCalled bool + lastCommand *notifications.SendEmailCommandSync +} + +func (m *mockNotificationService) SendEmailCommandHandlerSync(ctx context.Context, cmd *notifications.SendEmailCommandSync) error { + m.sendEmailCalled = true + m.lastCommand = cmd + return nil +} + +func (m *mockNotificationService) SendEmailCommandHandler(ctx context.Context, cmd *notifications.SendEmailCommand) error { + return nil +} + +func (m *mockNotificationService) SendWebhookSync(ctx context.Context, cmd *notifications.SendWebhookSync) error { + return nil +} + +func (m *mockNotificationService) SendResetPasswordEmail(ctx context.Context, cmd *notifications.SendResetPasswordEmailCommand) error { + return nil +} + +func (m *mockNotificationService) ValidateResetPasswordCode(ctx context.Context, query *notifications.ValidateResetPasswordCodeQuery, userByLogin notifications.GetUserByLoginFunc) (*user.User, error) { + return nil, nil +} + +func (m *mockNotificationService) SendVerificationEmail(ctx context.Context, cmd *notifications.SendVerifyEmailCommand) error { + return nil +} + +func TestEmailSender_SendEmail_PlaceholderHandling(t *testing.T) { + tests := []struct { + name string + recipients []string + expectSend bool + expectedRecips []string + description string + }{ + { + name: "all placeholder addresses - skip gracefully", + recipients: []string{""}, + expectSend: false, + expectedRecips: nil, + description: "Should skip sending when all recipients are placeholders", + }, + { + name: "mixed valid and placeholder addresses", + recipients: []string{"", "user@example.com"}, + expectSend: true, + expectedRecips: []string{"user@example.com"}, + description: "Should filter out placeholders and send to valid addresses", + }, + { + name: "all valid addresses", + recipients: []string{"user@example.com", "admin@grafana.com"}, + expectSend: true, + expectedRecips: []string{"user@example.com", "admin@grafana.com"}, + description: "Should send to all valid addresses", + }, + { + name: "multiple placeholders with one valid", + recipients: []string{"example@email.com", "", "valid@example.com"}, + expectSend: true, + expectedRecips: []string{"valid@example.com"}, + description: "Should filter out all placeholder variations and send to valid address", + }, + { + name: "only placeholders without angle brackets", + recipients: []string{"example@email.com"}, + expectSend: false, + expectedRecips: nil, + description: "Should skip sending when placeholder is without angle brackets", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockNS := &mockNotificationService{} + sender := emailSender{ns: mockNS} + + cmd := &receivers.SendEmailSettings{ + To: tt.recipients, + Subject: "Test Subject", + SingleEmail: true, + } + + err := sender.SendEmail(context.Background(), cmd) + require.NoError(t, err, tt.description) + + if tt.expectSend { + assert.True(t, mockNS.sendEmailCalled, "Expected email to be sent but it was not") + assert.Equal(t, tt.expectedRecips, mockNS.lastCommand.SendEmailCommand.To, "Recipients mismatch") + } else { + assert.False(t, mockNS.sendEmailCalled, "Expected email not to be sent but it was") + } + }) + } +} + +func TestEmailSender_SendEmail_EmptyRecipients(t *testing.T) { + mockNS := &mockNotificationService{} + sender := emailSender{ns: mockNS} + + cmd := &receivers.SendEmailSettings{ + To: []string{}, + Subject: "Test Subject", + SingleEmail: true, + } + + err := sender.SendEmail(context.Background(), cmd) + require.NoError(t, err) + assert.False(t, mockNS.sendEmailCalled, "Should not send email with empty recipient list") +} + +func TestEmailSender_SendEmail_EmbeddedContents(t *testing.T) { + mockNS := &mockNotificationService{} + sender := emailSender{ns: mockNS} + + cmd := &receivers.SendEmailSettings{ + To: []string{"user@example.com"}, + Subject: "Test with embedded content", + SingleEmail: true, + EmbeddedContents: []receivers.EmbeddedContent{ + {Name: "image.png", Content: []byte("fake image data")}, + }, + } + + err := sender.SendEmail(context.Background(), cmd) + require.NoError(t, err) + assert.True(t, mockNS.sendEmailCalled, "Email should be sent") + assert.Len(t, mockNS.lastCommand.SendEmailCommand.EmbeddedContents, 1, "Embedded content should be passed through") + assert.Equal(t, "image.png", mockNS.lastCommand.SendEmailCommand.EmbeddedContents[0].Name) +} diff --git a/pkg/services/notifications/smtp.go b/pkg/services/notifications/smtp.go index 20b1dc24222..babc525a92a 100644 --- a/pkg/services/notifications/smtp.go +++ b/pkg/services/notifications/smtp.go @@ -74,6 +74,11 @@ func (sc *SmtpClient) sendMessage(ctx context.Context, dialer *gomail.Dialer, ms )) defer span.End() + // Skip sending if there are no recipients + if len(msg.To) == 0 { + return nil + } + m := sc.buildEmail(ctx, msg) err := dialer.DialAndSend(m) diff --git a/public/app/features/alerting/unified/state/actions.ts b/public/app/features/alerting/unified/state/actions.ts index 0ac481fdf83..ebc7e17f13b 100644 --- a/public/app/features/alerting/unified/state/actions.ts +++ b/public/app/features/alerting/unified/state/actions.ts @@ -1,14 +1,27 @@ import { createAsyncThunk } from '@reduxjs/toolkit'; import { isEmpty } from 'lodash'; +import { AppEvents } from '@grafana/data'; import { locationService, logMeasurement } from '@grafana/runtime'; -import { AlertManagerCortexConfig, AlertmanagerGroup, Matcher } from 'app/plugins/datasource/alertmanager/types'; +import { appEvents } from 'app/core/core'; +import { + AlertManagerCortexConfig, + AlertmanagerGroup, + Matcher, + Receiver, + TestReceiversAlert, +} from 'app/plugins/datasource/alertmanager/types'; import { ThunkResult } from 'app/types/store'; import { RuleIdentifier, RuleNamespace, StateHistoryItem } from 'app/types/unified-alerting'; import { RulerRuleDTO, RulerRulesConfigDTO } from 'app/types/unified-alerting-dto'; import { withPromRulesMetadataLogging, withRulerRulesMetadataLogging } from '../Analytics'; -import { deleteAlertManagerConfig, fetchAlertGroups, updateAlertManagerConfig } from '../api/alertmanager'; +import { + deleteAlertManagerConfig, + fetchAlertGroups, + testReceivers, + updateAlertManagerConfig, +} from '../api/alertmanager'; import { alertmanagerApi } from '../api/alertmanagerApi'; import { fetchAnnotations } from '../api/annotations'; import { featureDiscoveryApi } from '../api/featureDiscoveryApi'; @@ -253,6 +266,56 @@ export const deleteAlertManagerConfigAction = createAsyncThunk( } ); +interface TestReceiversOptions { + alertManagerSourceName: string; + receivers: Receiver[]; + alert?: TestReceiversAlert; +} + +// Check if a receiver uses placeholder email addresses +function hasPlaceholderEmail(receivers: Receiver[]): boolean { + const placeholderEmails = ['', 'example@email.com']; + + return receivers.some((receiver) => + receiver.grafana_managed_receiver_configs?.some((config) => { + if (config.type === 'email' && config.settings?.addresses) { + const addresses = config.settings.addresses; + // addresses can be a string or array + const addressList = typeof addresses === 'string' ? [addresses] : addresses; + return addressList.some((addr: string) => placeholderEmails.includes(addr.trim())); + } + return false; + }) + ); +} + +export const testReceiversAction = createAsyncThunk( + 'unifiedalerting/testReceivers', + async ({ alertManagerSourceName, receivers, alert }: TestReceiversOptions): Promise => { + const usesPlaceholder = hasPlaceholderEmail(receivers); + + if (usesPlaceholder) { + // Handle placeholder email case with custom warning message + try { + await withSerializedError(testReceivers(alertManagerSourceName, receivers, alert)); + appEvents.emit(AppEvents.alertWarning, [ + 'Test completed, but no email was sent because a placeholder email address is configured. Please update your contact point with a valid email address to receive alerts.', + ]); + } catch (e) { + const msg = e instanceof Error ? e.message : 'Unknown error'; + appEvents.emit(AppEvents.alertError, [`Failed to send test alert: ${msg}`]); + throw e; + } + return; + } + + return withAppEvents(withSerializedError(testReceivers(alertManagerSourceName, receivers, alert)), { + errorMessage: 'Failed to send test alert.', + successMessage: 'Test alert sent.', + }); + } +); + export const rulesInSameGroupHaveInvalidFor = (rules: RulerRuleDTO[], everyDuration: string) => { return rules.filter((rule: RulerRuleDTO) => { const { forDuration } = getAlertInfo(rule, everyDuration);