Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
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
feat(alerting): Add AWS SES Alerting Provider #579
feat(alerting): Add AWS SES Alerting Provider #579
Changes from 11 commits
8c94494
fb0dec6
504bfb4
19010f8
5aadae7
09b8b41
905e5d2
6d651e8
d84932c
e1572bb
0b24d79
0b1c744
623c64e
2fb5cd3
27c7f7e
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When would
(len(provider.AccessKeyID) == 0 && len(provider.SecretAccessKey) == 0)
be acceptable iflen(provider.From) > 0 && len(provider.To) > 0
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AWS access key and secret are optional variables if you use IAM authentication. So they both should either be empty or filled in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In that case, should the condition check for IAM auth if the keys aren't provided?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IAM authentication is implicit on resources within AWS, and doesn't really need to be checked, if that's what you mean.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I see, I was interpretting it as the aws profile config but I suppose it's implicit if you deploy to aws.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a comment that clarifies this?
i.e. `if both AccessKeyID and SecretAccessKey are specified, we'll use these to authenticate, otherwise if neither are specified, then we'll fall back on IAM auth