Add a receiver for Amazon SNS #173

robbierolin · 2024-03-20T20:55:32Z

This PR adds a receiver that will send notifications to Amazon SNS. Fixes #172

I have a corresponding change in the grafana code locally to add this receiver to the available channels that can be selected for contact points. I am guessing the process is to PR that one after this change gets merged.

Here are some example screenshots

I tested by subscribing my email to the sns topic. And verifying that I receive the emails when I send test requests.

Let me know if there's any other changes I should add or testing I should show.

yuri-tceretian · 2024-03-20T21:09:36Z

receivers/sns/config.go

+		Topic         string `json:"topic,omitempty" yaml:"topic,omitempty"`
+		AuthProvider  string `json:"authProvider,omitempty" yaml:"authProvider,omitempty"`
+		Subject       string `json:"subject,omitempty" yaml:"subject,omitempty"`
+		Body          string `json:"body,omitempty" yaml:"body,omitempty"`
+		MessageFormat string `json:"messageFormat,omitempty" yaml:"messageFormat,omitempty"`
+		Credentials   string `json:"credentials,omitempty" yaml:"credentials,omitempty"`
+		AccessKey     string `json:"accessKey,omitempty" yaml:"accessKey,omitempty"`
+		SecretKey     string `json:"secretKey,omitempty" yaml:"secretKey,omitempty"`
+		AssumeRoleARN string `json:"assumeRoleARN,omitempty" yaml:"assumeRoleARN,omitempty"`
+		ExternalID    string `json:"externalId,omitempty" yaml:"externalId,omitempty"`


First, thank you so much for opening the PR!

I have not reviewed it yet but the first thing that catches my eye is the config. We're trying to maintain as little drift as possible with integration in the upstream. Please make sure that json\yaml fields match the upstream integration
https://github.com/prometheus/alertmanager/blob/f82574a3763c21ca9bf2ada7baf69bd09e01d977/config/notifiers.go#L731-L744

Thanks for sharing, I will look at that. I think I will need to use this SubformOptions to provide the complex types like sigv4.SigV4Config?

Yes, complex types can be presented via subform options. You can see https://github.com/grafana/grafana/blob/cf601fab0906f1d811f1aa2fe18a67bc5421e3db/pkg/services/ngalert/notifier/channels_config/available_channels.go#L1312-L1338 as an example

Ok I updated the config to match the upstream and pulled in some of the functionality from there too, like message deduplication and truncation.

Also I noticed that the Grafana channel config UIs doesn't seem to support ShowWhen or secure fields for subforms. I can try to address that in my PR to grafana.

JohnnyQQQQ · 2024-05-22T09:20:30Z

@robbierolin could you rebase your PR please and @yuri-tceretian could you give it a final review?

robbierolin · 2024-05-25T00:19:41Z

Yup, should be good now.

yuri-tceretian

It looks like the logic pretty much matches already merged in vanilla Alertmanager. The code LGTM but I think it needs some more tests.

yuri-tceretian · 2024-05-28T14:38:44Z

receivers/sns/config.go

+		settings.Sigv4.AccessKey = decryptFn("accessKey", settings.Sigv4.AccessKey)
+		settings.Sigv4.SecretKey = decryptFn("secretKey", settings.Sigv4.SecretKey)
+		at = awsds.AuthTypeKeys
+	}


Can we add a validation here to check for situations when either of keys is not specified. Currently, it silently falls back to default authentication type

Yes, will do.

yuri-tceretian · 2024-05-28T14:41:01Z

receivers/sns/config_test.go

+			expectedMessage:  "message",
+			expectedAuthType: awsds.AuthTypeSharedCreds.String(),
+		},
+	}


Can you add tests for invalid configurations like

settings: `{ "topic_arn": "arn:aws:sns:region:0123456789:SNSTopicName", "sigv4": { "secret_key": "secret-key" } }`,

and

settings: `{ "topic_arn": "arn:aws:sns:region:0123456789:SNSTopicName", "sigv4": { "access-key": "access-key" } }`,

yuri-tceretian · 2024-05-28T14:50:41Z

receivers/sns/config.go

+		settings.APIUrl = fmt.Sprintf("https://sns.%s.amazonaws.com", settings.Sigv4.Region)
+	}
+
+	at := awsds.AuthTypeDefault


Do I understand correctly that in the case when auth type is "default", the credentials will be taken from a file ~/.aws/credentials or env vars?

That setting comes from here - https://github.com/grafana/grafana-aws-sdk/blob/main/pkg/awsds/settings.go#L16
The behaviour should be to use the default credential provider chain which looks for creds in multiple places. It's explained more here - https://docs.aws.amazon.com/sdk-for-go/v1/developer-guide/configuring-sdk.html#specifying-credentials

yuri-tceretian · 2024-05-28T14:54:38Z

receivers/sns/sns.go

+		return message, false, nil
+	}
+	// If the message is larger than our specified size we have to truncate.
+	truncated := make([]byte, maxMessageSizeInBytes)


We have a standard way for truncating messages notify.TruncateInBytes. However, this matches the logic in the vanilla Alertmanager. So, I think this is fine.

yuri-tceretian · 2024-05-28T14:57:35Z

receivers/sns/sns_test.go

+	"github.com/grafana/alerting/templates"
+)
+
+func TestCreatePublishInput(t *testing.T) {


This needs some more tests for truncation of messages

yuri-tceretian · 2024-05-29T18:33:44Z

CI fails due to linter

golangci-lint run

receivers/sns/sns.go:6: File is not `goimports`-ed (goimports)

	"github.com/prometheus/alertmanager/notify"

receivers/sns/config_test.go:4: File is not `goimports`-ed (goimports)

	"encoding/json"

yuri-tceretian

LGTM. Thanks for the contribution!
Once this is merged, we need to update this module in Grafana and introduce a new configuration in https://github.com/grafana/grafana/blob/fdaa091a4d1874e78040a46d2c0d15d29d0064ae/pkg/services/ngalert/notifier/channels_config/available_channels.go#L14
and update Terraform provider https://github.com/grafana/terraform-provider-grafana/blob/aaa20a20f309172c0b88071f80b9e9e5e52c75ae/internal/resources/grafana/resource_alerting_contact_point_notifiers.go#L2306-L2305
I would appreciate your help with this

This reverts commit 43d4caf.

robbierolin force-pushed the add-sns-receiver branch from e30675d to 98b7b19 Compare March 20, 2024 21:01

yuri-tceretian reviewed Mar 20, 2024

View reviewed changes

robbierolin force-pushed the add-sns-receiver branch 2 times, most recently from 726c586 to 5984351 Compare March 21, 2024 00:58

yuri-tceretian added the enhancement New feature or request label Mar 21, 2024

Add sns receiver

ec9c84a

robbierolin force-pushed the add-sns-receiver branch from 5984351 to ec9c84a Compare May 25, 2024 00:17

yuri-tceretian reviewed May 28, 2024

View reviewed changes

robbierolin added 2 commits May 29, 2024 11:49

Add additional validation, tests

2cc8b15

Linting updates

236b8aa

yuri-tceretian approved these changes May 30, 2024

View reviewed changes

yuri-tceretian merged commit 43d4caf into grafana:main May 30, 2024
2 checks passed

santihernandezc mentioned this pull request Jun 5, 2024

Remove grafana-aws-sdk dependency #197

Closed

santihernandezc added a commit that referenced this pull request Jun 5, 2024

Revert "Add a receiver for Amazon SNS (#173)"

5d695b8

This reverts commit 43d4caf.

This was referenced Jun 5, 2024

Update AWS SNS integration to not use grafana\aws-sdk-go #201

Merged

Alerting: Support AWS SNS integration in Grafana grafana/grafana#88867

Merged

yuri-tceretian mentioned this pull request Aug 21, 2024

Support Grafana implementation of alerting integration AWS SNS grafana/terraform-provider-grafana#1775

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a receiver for Amazon SNS #173

Add a receiver for Amazon SNS #173

robbierolin commented Mar 20, 2024 •

edited

Loading

yuri-tceretian Mar 20, 2024

robbierolin Mar 20, 2024

yuri-tceretian Mar 20, 2024

robbierolin Mar 21, 2024

JohnnyQQQQ commented May 22, 2024

robbierolin commented May 25, 2024

yuri-tceretian left a comment

yuri-tceretian May 28, 2024

robbierolin May 29, 2024

yuri-tceretian May 28, 2024

robbierolin May 29, 2024

yuri-tceretian May 28, 2024

robbierolin May 29, 2024

yuri-tceretian May 28, 2024

yuri-tceretian May 28, 2024

robbierolin May 29, 2024

yuri-tceretian commented May 29, 2024

yuri-tceretian left a comment

Add a receiver for Amazon SNS #173

Add a receiver for Amazon SNS #173

Conversation

robbierolin commented Mar 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JohnnyQQQQ commented May 22, 2024

robbierolin commented May 25, 2024

yuri-tceretian left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yuri-tceretian commented May 29, 2024

yuri-tceretian left a comment

Choose a reason for hiding this comment

robbierolin commented Mar 20, 2024 •

edited

Loading