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 <example@email.com>. 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=[<example@email.com>]
This commit is contained in:
tonypowa
@@ -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 = "<example@email.com>"
|
||||
)
|
||||
|
||||
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,
|
||||
|
||||
@@ -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: "<example@email.com>",
|
||||
expected: true,
|
||||
},
|
||||
{
|
||||
name: "placeholder without angle brackets",
|
||||
email: "example@email.com",
|
||||
expected: true,
|
||||
},
|
||||
{
|
||||
name: "placeholder with spaces",
|
||||
email: " <example@email.com> ",
|
||||
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{"<example@email.com>"},
|
||||
expectSend: false,
|
||||
expectedRecips: nil,
|
||||
description: "Should skip sending when all recipients are placeholders",
|
||||
},
|
||||
{
|
||||
name: "mixed valid and placeholder addresses",
|
||||
recipients: []string{"<example@email.com>", "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", "<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)
|
||||
}
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user