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

[New Resource]: SESv2 PutAccountDetails for sandbox removal requests #32655

Open
mattbnz opened this issue Jul 24, 2023 · 5 comments
Open

[New Resource]: SESv2 PutAccountDetails for sandbox removal requests #32655

mattbnz opened this issue Jul 24, 2023 · 5 comments
Labels
new-resource Introduces a new resource. service/sesv2 Issues and PRs that pertain to the sesv2 service.

Comments

@mattbnz
Copy link

mattbnz commented Jul 24, 2023

Description

Addition of support for the SESv2 PutAccountDetails API call that enables submission of requests to move an account out of the SES sandbox.

Requested Resource(s) and/or Data Source(s)

Guessing:

  • aws_sesv2_account_details

Potential Terraform Configuration

resource "aws_sesv2_account_details" "example" {
  additional_contact_emails = [ "someone@example.com" ]
  mail_type = "TRANSACTIONAL"
  production_access_enabled = true
  use_case_description = "Transactional email to registered and signed in users for the XXX app"
  website_url = "https://example.com"
}

References

This request was suggested at #26796 (comment) following the question in the prior comment.

The AWS API docs are at: https://docs.aws.amazon.com/ses/latest/APIReference-V2/API_PutAccountDetails.html

Would you like to implement a fix?

No

@mattbnz mattbnz added the needs-triage Waiting for first response or review from a maintainer. label Jul 24, 2023
@github-actions
Copy link

Community Note

Voting for Prioritization

  • Please vote on this issue by adding a 👍 reaction to the original post to help the community and maintainers prioritize this request.
  • Please see our prioritization guide for information on how we prioritize.
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.

Volunteering to Work on This Issue

  • If you are interested in working on this issue, please leave a comment.
  • If this would be your first contribution, please review the contribution guide.

@github-actions github-actions bot added the service/sesv2 Issues and PRs that pertain to the sesv2 service. label Jul 24, 2023
@justinretzolk justinretzolk added new-resource Introduces a new resource. and removed needs-triage Waiting for first response or review from a maintainer. labels Jul 24, 2023
@kamilturek
Copy link
Collaborator

Hey @jar-b, following the discussion here #26796 (comment), I'd be happy to work on implementing this resource.

My only question would be about acceptance tests. I believe the PutAccountDetails API action submits a request for production access which is going to be reviewed and evaluated by AWS. Acceptance tests probably shouldn't submit such "fake" requests. Is there any way to work it around?

@jar-b
Copy link
Member

jar-b commented Oct 5, 2023

Hey @kamilturek - thanks for your offer!

Skimming the linked API docs, one potential testing option could be to focus on the attributes which don't move the account out of the sandbox environment. For example, toggling the mail_type enum value, or updating the use_case_description or website_url strings. In these tests the production_access_enabled argument would remain false.

This is a suggestion based on the assumption that modifying these additional attributes can be done without enabling a "production" environment. If this isn't the case, we'd likely either omit tests and add a comment to the resource explaining why, or gate the tests behind an environment variable so that are skipped unless intentionally being run to exercise this behavior. Hope this helps!

@kamilturek
Copy link
Collaborator

Thanks @jar-b.

I tried calling PutAccountDetails with ProductionAccessEnabled set to false but it turns out it creates an AWS support case anyway. The case is immediately resolved though with such message.

image

While it shouldn't be a big problem for tests, I think it'd be a weird and unexpected side effect for users of the planned aws_sesv2_put_account_details resource.

Moreover, I realized that there is no sensible way to destroy the resource. Many of the PutAccountDetails relevant fields are required so it seems impossible to bring the account details to the original state.

Before calling the API for the first time, fields like UseCaseDescription, MailType, and WebsiteURL do not show up in account details at all but once they're set for the first time, there is no way to "erase" them.

@jar-b
Copy link
Member

jar-b commented Oct 30, 2023

Thanks for all this research @kamilturek.

While it shouldn't be a big problem for tests, I think it'd be a weird and unexpected side effect for users of the planned aws_sesv2_put_account_details resource.

If the same side effect happens from the console or AWS CLI, I don't think its a blocker, especially if the case automatically resolves. This would be a good point to mention in the documentation so users are aware that even changes not involving production access will trigger a support case.

Moreover, I realized that there is no sensible way to destroy the resource. Many of the PutAccountDetails relevant fields are required so it seems impossible to bring the account details to the original state.

Before calling the API for the first time, fields like UseCaseDescription, MailType, and WebsiteURL do not show up in account details at all but once they're set for the first time, there is no way to "erase" them.

If there is no way to modify them we could document that destroy will only remove the resource from state, but leave the account settings as they are. Or if it is possible to set the string fields to empty values (""), this could also be a reasonable alternative similar to other resources that are "zeroed out" on destroy. aws_lakeformation_data_lake_settings for example does this:

input := &lakeformation.PutDataLakeSettingsInput{
DataLakeSettings: &lakeformation.DataLakeSettings{
CreateDatabaseDefaultPermissions: make([]*lakeformation.PrincipalPermissions, 0),
CreateTableDefaultPermissions: make([]*lakeformation.PrincipalPermissions, 0),
DataLakeAdmins: make([]*lakeformation.DataLakePrincipal, 0),
ReadOnlyAdmins: make([]*lakeformation.DataLakePrincipal, 0),
TrustedResourceOwners: make([]*string, 0),
},
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-resource Introduces a new resource. service/sesv2 Issues and PRs that pertain to the sesv2 service.
Projects
None yet
Development

No branches or pull requests

4 participants