Skip to content

Commit

Permalink
Revert "Return error when TmplText errors in sns notifier (prometheus…
Browse files Browse the repository at this point in the history
…#3531)" (prometheus#3876)

This reverts commit 603a728.
  • Loading branch information
gotjosh authored and TheMeier committed Sep 29, 2024
1 parent 79ea1fc commit 7fc1516
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 136 deletions.
39 changes: 8 additions & 31 deletions notify/sns/sns.go
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
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
}

0 comments on commit 7fc1516

Please sign in to comment.