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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

Revert "Return error when TmplText errors in sns notifier" #3876

Merged
merged 1 commit into from
Jun 14, 2024
Merged

Revert "Return error when TmplText errors in sns notifier" #3876

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 8 additions & 31 deletions notify/sns/sns.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import (
"github.com/aws/aws-sdk-go/service/sns"
"github.com/go-kit/log"
"github.com/go-kit/log/level"
"github.com/pkg/errors"
commoncfg "github.com/prometheus/common/config"

"github.com/prometheus/alertmanager/config"
Expand Down Expand Up @@ -64,12 +63,12 @@ func New(c *config.SNSConfig, t *template.Template, l log.Logger, httpOpts ...co

func (n *Notifier) Notify(ctx context.Context, alert ...*types.Alert) (bool, error) {
var (
tmplErr error
data = notify.GetTemplateData(ctx, n.tmpl, alert, n.logger)
tmpl = notify.TmplText(n.tmpl, data, &tmplErr)
err error
data = notify.GetTemplateData(ctx, n.tmpl, alert, n.logger)
tmpl = notify.TmplText(n.tmpl, data, &err)
)

client, err := n.createSNSClient(tmpl, &tmplErr)
client, err := n.createSNSClient(tmpl)
if err != nil {
var e awserr.RequestFailure
if errors.As(err, &e) {
Expand All @@ -78,7 +77,7 @@ func (n *Notifier) Notify(ctx context.Context, alert ...*types.Alert) (bool, err
return true, err
}

publishInput, err := n.createPublishInput(ctx, tmpl, &tmplErr)
publishInput, err := n.createPublishInput(ctx, tmpl)
if err != nil {
return true, err
}
Expand All @@ -100,7 +99,7 @@ func (n *Notifier) Notify(ctx context.Context, alert ...*types.Alert) (bool, err
return false, nil
}

func (n *Notifier) createSNSClient(tmpl func(string) string, tmplErr *error) (*sns.SNS, error) {
func (n *Notifier) createSNSClient(tmpl func(string) string) (*sns.SNS, error) {
var creds *credentials.Credentials
// If there are provided sigV4 credentials we want to use those to create a session.
if n.conf.Sigv4.AccessKey != "" && n.conf.Sigv4.SecretKey != "" {
Expand All @@ -116,9 +115,6 @@ func (n *Notifier) createSNSClient(tmpl func(string) string, tmplErr *error) (*s
if err != nil {
return nil, err
}
if *tmplErr != nil {
return nil, notify.NewErrorWithReason(notify.ClientErrorReason, errors.Wrap(*tmplErr, "execute 'api_url' template"))
}

if n.conf.Sigv4.RoleARN != "" {
var stsSess *session.Session
Expand Down Expand Up @@ -148,19 +144,13 @@ func (n *Notifier) createSNSClient(tmpl func(string) string, tmplErr *error) (*s
return client, nil
}

func (n *Notifier) createPublishInput(ctx context.Context, tmpl func(string) string, tmplErr *error) (*sns.PublishInput, error) {
func (n *Notifier) createPublishInput(ctx context.Context, tmpl func(string) string) (*sns.PublishInput, error) {
publishInput := &sns.PublishInput{}
messageAttributes := n.createMessageAttributes(tmpl)
if *tmplErr != nil {
return nil, notify.NewErrorWithReason(notify.ClientErrorReason, errors.Wrap(*tmplErr, "execute 'attributes' template"))
}
// Max message size for a message in a SNS publish request is 256KB, except for SMS messages where the limit is 1600 characters/runes.
messageSizeLimit := 256 * 1024
if n.conf.TopicARN != "" {
topicARN := tmpl(n.conf.TopicARN)
if *tmplErr != nil {
return nil, notify.NewErrorWithReason(notify.ClientErrorReason, errors.Wrap(*tmplErr, "execute 'topic_arn' template"))
}
publishInput.SetTopicArn(topicARN)
// If we are using a topic ARN, it could be a FIFO topic specified by the topic's suffix ".fifo".
if strings.HasSuffix(topicARN, ".fifo") {
Expand All @@ -175,24 +165,14 @@ func (n *Notifier) createPublishInput(ctx context.Context, tmpl func(string) str
}
if n.conf.PhoneNumber != "" {
publishInput.SetPhoneNumber(tmpl(n.conf.PhoneNumber))
if *tmplErr != nil {
return nil, notify.NewErrorWithReason(notify.ClientErrorReason, errors.Wrap(*tmplErr, "execute 'phone_number' template"))
}
// If we have an SMS message, we need to truncate to 1600 characters/runes.
messageSizeLimit = 1600
}
if n.conf.TargetARN != "" {
publishInput.SetTargetArn(tmpl(n.conf.TargetARN))
if *tmplErr != nil {
return nil, notify.NewErrorWithReason(notify.ClientErrorReason, errors.Wrap(*tmplErr, "execute 'target_arn' template"))
}
}

tmplMessage := tmpl(n.conf.Message)
if *tmplErr != nil {
return nil, notify.NewErrorWithReason(notify.ClientErrorReason, errors.Wrap(*tmplErr, "execute 'message' template"))
}
messageToSend, isTrunc, err := validateAndTruncateMessage(tmplMessage, messageSizeLimit)
messageToSend, isTrunc, err := validateAndTruncateMessage(tmpl(n.conf.Message), messageSizeLimit)
if err != nil {
return nil, err
}
Expand All @@ -206,9 +186,6 @@ func (n *Notifier) createPublishInput(ctx context.Context, tmpl func(string) str

if n.conf.Subject != "" {
publishInput.SetSubject(tmpl(n.conf.Subject))
if *tmplErr != nil {
return nil, notify.NewErrorWithReason(notify.ClientErrorReason, errors.Wrap(*tmplErr, "execute 'subject' template"))
}
}

return publishInput, nil
Expand Down
105 changes: 0 additions & 105 deletions notify/sns/sns_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,23 +14,11 @@
package sns

import (
"context"
"net/url"
"strings"
"testing"

"github.com/go-kit/log"
commoncfg "github.com/prometheus/common/config"
"github.com/prometheus/common/sigv4"
"github.com/stretchr/testify/require"

"github.com/prometheus/alertmanager/config"
"github.com/prometheus/alertmanager/template"
"github.com/prometheus/alertmanager/types"
)

var logger = log.NewNopLogger()

func TestValidateAndTruncateMessage(t *testing.T) {
sBuff := make([]byte, 257*1024)
for i := range sBuff {
Expand All @@ -55,96 +43,3 @@ func TestValidateAndTruncateMessage(t *testing.T) {
_, _, err = validateAndTruncateMessage(invalidUtf8String, 100)
require.Error(t, err)
}

func TestNotifyWithInvalidTemplate(t *testing.T) {
for _, tc := range []struct {
title string
errMsg string
updateCfg func(*config.SNSConfig)
}{
{
title: "with invalid Attribute template",
errMsg: "execute 'attributes' template",
updateCfg: func(cfg *config.SNSConfig) {
cfg.Attributes = map[string]string{
"attribName1": "{{ template \"unknown_template\" . }}",
}
},
},
{
title: "with invalid TopicArn template",
errMsg: "execute 'topic_arn' template",
updateCfg: func(cfg *config.SNSConfig) {
cfg.TopicARN = "{{ template \"unknown_template\" . }}"
},
},
{
title: "with invalid PhoneNumber template",
errMsg: "execute 'phone_number' template",
updateCfg: func(cfg *config.SNSConfig) {
cfg.PhoneNumber = "{{ template \"unknown_template\" . }}"
},
},
{
title: "with invalid Message template",
errMsg: "execute 'message' template",
updateCfg: func(cfg *config.SNSConfig) {
cfg.Message = "{{ template \"unknown_template\" . }}"
},
},
{
title: "with invalid Subject template",
errMsg: "execute 'subject' template",
updateCfg: func(cfg *config.SNSConfig) {
cfg.Subject = "{{ template \"unknown_template\" . }}"
},
},
{
title: "with invalid APIUrl template",
errMsg: "execute 'api_url' template",
updateCfg: func(cfg *config.SNSConfig) {
cfg.APIUrl = "{{ template \"unknown_template\" . }}"
},
},
{
title: "with invalid TargetARN template",
errMsg: "execute 'target_arn' template",
updateCfg: func(cfg *config.SNSConfig) {
cfg.TargetARN = "{{ template \"unknown_template\" . }}"
},
},
} {
tc := tc
t.Run(tc.title, func(t *testing.T) {
snsCfg := &config.SNSConfig{
HTTPConfig: &commoncfg.HTTPClientConfig{},
TopicARN: "TestTopic",
Sigv4: sigv4.SigV4Config{
Region: "us-west-2",
},
}
if tc.updateCfg != nil {
tc.updateCfg(snsCfg)
}
notifier, err := New(
snsCfg,
createTmpl(t),
logger,
)
require.NoError(t, err)
var alerts []*types.Alert
_, err = notifier.Notify(context.Background(), alerts...)
require.Error(t, err)
require.True(t, strings.Contains(err.Error(), "template \"unknown_template\" not defined"))
require.True(t, strings.Contains(err.Error(), tc.errMsg))
})
}
}

// CreateTmpl returns a ready-to-use template.
func createTmpl(t *testing.T) *template.Template {
tmpl, err := template.FromGlobs([]string{})
require.NoError(t, err)
tmpl.ExternalURL, _ = url.Parse("http://am")
return tmpl
}
Loading