Return error when TmplText errors in sns notifier (#3531)

Signed-off-by: Emmanuel Lodovice <[email protected]>

notify/sns/sns.go

@@ -29,6 +29,7 @@ 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"
@@ -63,12 +64,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 (
-		err  error
-		data = notify.GetTemplateData(ctx, n.tmpl, alert, n.logger)
-		tmpl = notify.TmplText(n.tmpl, data, &err)
+		tmplErr error
+		data    = notify.GetTemplateData(ctx, n.tmpl, alert, n.logger)
+		tmpl    = notify.TmplText(n.tmpl, data, &tmplErr)
 	)
 
-	client, err := n.createSNSClient(tmpl)
+	client, err := n.createSNSClient(tmpl, &tmplErr)
 	if err != nil {
 		var e awserr.RequestFailure
 		if errors.As(err, &e) {
@@ -77,7 +78,7 @@ func (n *Notifier) Notify(ctx context.Context, alert ...*types.Alert) (bool, err
 		return true, err
 	}
 
-	publishInput, err := n.createPublishInput(ctx, tmpl)
+	publishInput, err := n.createPublishInput(ctx, tmpl, &tmplErr)
 	if err != nil {
 		return true, err
 	}
@@ -99,7 +100,7 @@ func (n *Notifier) Notify(ctx context.Context, alert ...*types.Alert) (bool, err
 	return false, nil
 }
 
-func (n *Notifier) createSNSClient(tmpl func(string) string) (*sns.SNS, error) {
+func (n *Notifier) createSNSClient(tmpl func(string) string, tmplErr *error) (*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 != "" {
@@ -115,6 +116,9 @@ func (n *Notifier) createSNSClient(tmpl func(string) string) (*sns.SNS, error) {
 	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
@@ -144,13 +148,19 @@ func (n *Notifier) createSNSClient(tmpl func(string) string) (*sns.SNS, error) {
 	return client, nil
 }
 
-func (n *Notifier) createPublishInput(ctx context.Context, tmpl func(string) string) (*sns.PublishInput, error) {
+func (n *Notifier) createPublishInput(ctx context.Context, tmpl func(string) string, tmplErr *error) (*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") {
@@ -165,14 +175,24 @@ 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"))
+		}
 	}
 
-	messageToSend, isTrunc, err := validateAndTruncateMessage(tmpl(n.conf.Message), messageSizeLimit)
+	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)
 	if err != nil {
 		return nil, err
 	}
@@ -186,6 +206,9 @@ 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

notify/sns/sns_test.go

@@ -14,11 +14,23 @@
 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 {
@@ -43,3 +55,96 @@ 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
+}