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.

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

Merged
merged 15 commits into from
Oct 26, 2023
Merged

Conversation

beschoenen
Copy link
Contributor

Summary

Added support for SES.

Checklist

  • Tested and/or added tests to validate that the changes work as intended, if applicable.
  • Updated documentation in README.md, if applicable.

@beschoenen beschoenen changed the title Add SES Provider Add SES Alerting Provider Sep 26, 2023
Copy link
Owner

@TwiN TwiN left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beautiful work!

My only suggestion would be to rename ses to aws-ses as ses alone is a tad bit too ambiguous

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
alerting/config.go Outdated Show resolved Hide resolved
alerting/provider/ses/ses.go Outdated Show resolved Hide resolved
alerting/provider/ses/ses.go Outdated Show resolved Hide resolved
alerting/provider/ses/ses_test.go Outdated Show resolved Hide resolved
config/config.go Outdated Show resolved Hide resolved
@TwiN TwiN changed the title Add SES Alerting Provider feat(alerting): Add AWS SES Alerting Provider Sep 27, 2023
@TwiN TwiN added feature New feature or request area/alerting Related to alerting labels Sep 27, 2023
@beschoenen
Copy link
Contributor Author

@TwiN Updated.

alerting/config.go Outdated Show resolved Hide resolved
config/config.go Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
alerting/config.go Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Sep 28, 2023

Codecov Report

Attention: 56 lines in your changes are missing coverage. Please review.

Comparison is base (619b69f) 80.26% compared to head (27c7f7e) 79.52%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #579      +/-   ##
==========================================
- Coverage   80.26%   79.52%   -0.74%     
==========================================
  Files          56       57       +1     
  Lines        4494     4592      +98     
==========================================
+ Hits         3607     3652      +45     
- Misses        705      759      +54     
+ Partials      182      181       -1     
Files Coverage Δ
alerting/provider/provider.go 100.00% <ø> (ø)
config/config.go 75.10% <100.00%> (+0.10%) ⬆️
alerting/provider/awsses/awsses.go 42.26% <42.26%> (ø)

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

alerting/provider/aws-ses/aws-ses.go Outdated Show resolved Hide resolved
@TwiN
Copy link
Owner

TwiN commented Oct 3, 2023

@beschoenen Can you resolve the conflict? Once it's resolved, I'll merge.

@beschoenen
Copy link
Contributor Author

@TwiN Should be all good now!

README.md Outdated Show resolved Hide resolved
}

return len(provider.From) > 0 && len(provider.To) > 0 &&
((len(provider.AccessKeyID) == 0 && len(provider.SecretAccessKey) == 0) || (len(provider.AccessKeyID) > 0 && len(provider.SecretAccessKey) > 0))
Copy link
Contributor

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 if len(provider.From) > 0 && len(provider.To) > 0?

Copy link
Contributor Author

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.

Copy link
Contributor

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?

Copy link
Contributor Author

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.

Copy link
Contributor

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.

Copy link
Owner

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

alerting/provider/awsses/awsses.go Outdated Show resolved Hide resolved
alerting/provider/awsses/awsses.go Outdated Show resolved Hide resolved
alerting/provider/awsses/awsses.go Outdated Show resolved Hide resolved
alerting/provider/awsses/awsses.go Outdated Show resolved Hide resolved
}

return len(provider.From) > 0 && len(provider.To) > 0 &&
((len(provider.AccessKeyID) == 0 && len(provider.SecretAccessKey) == 0) || (len(provider.AccessKeyID) > 0 && len(provider.SecretAccessKey) > 0))
Copy link
Owner

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

README.md Outdated Show resolved Hide resolved
@TwiN
Copy link
Owner

TwiN commented Oct 26, 2023

Thank you for the contribution!

@TwiN TwiN merged commit 802ad7f into TwiN:master Oct 26, 2023
Kloox pushed a commit to Kloox/gatus that referenced this pull request Nov 1, 2023
* Add SES Provider

* Formatting

* Rename ses to aws-ses

* Typo

* Parse tag instead of type name

* Use aws.slice to convert string array & rename awsses -> aws-ses

* Rename type

* Update README.md

* Update alerting/config.go

* Rename package aws-ses to awsses

* Update README.md

* PR comments

---------

Co-authored-by: TwiN <[email protected]>
MelvinTo added a commit to MelvinTo/gatus that referenced this pull request Nov 10, 2023
* 'master' of https://github.com/TwiN/gatus: (97 commits)
  docs: add instruction to install as binary (TwiN#615)
  feat(alerting): make authentication optional for email provider (TwiN#608)
  feat(alerting): Add gotify provider (TwiN#605)
  chore(deps): bump github.com/coreos/go-oidc/v3 from 3.6.0 to 3.7.0 (TwiN#604)
  chore(deps): bump github.com/valyala/fasthttp from 1.49.0 to 1.50.0 (TwiN#594)
  Feat/modify discord title (TwiN#602)
  ui: Fix issue back button appearing over title when logo is too small
  feat(alerting): Add AWS SES Alerting Provider (TwiN#579)
  chore(deps): bump github.com/wcharczuk/go-chart/v2 from 2.1.0 to 2.1.1 (TwiN#591)
  ci: Increase timeout-minutes for test workflow to 10 minutes
  ui: Use localStorage instead of sessionStorage for refresh interval + collapsed groups
  ui(settings): Fix refresh interval padding
  chore(deps): bump github.com/prometheus/client_golang from 1.16.0 to 1.17.0 (TwiN#586)
  chore(deps): bump modernc.org/sqlite from 1.24.0 to 1.26.0 (TwiN#585)
  fix: Support hexadecimal integers in conditions (TwiN#563)
  chore(deps): Bump github.com/TwiN/whois to v1.1.7
  chore(deps): Bump github.com/TwiN/whois to v1.1.7
  fix(alerting): Add support for client.insecure in email alerting provider (TwiN#583)
  chore(deps): bump github.com/gofiber/fiber/v2 from 2.46.0 to 2.49.2 (TwiN#584)
  docs: Clean up list of sponsors
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/alerting Related to alerting feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants