mattbnz commented 1 year ago

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 https://github.com/hashicorp/terraform-provider-aws/issues/26796#issuecomment-1644076699 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

github-actions[bot] commented 1 year ago

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.

kamilturek commented 10 months ago

Hey @jar-b, following the discussion here https://github.com/hashicorp/terraform-provider-aws/issues/26796#issuecomment-1644076699, 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 commented 10 months ago

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 commented 10 months ago

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.

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 commented 9 months ago

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:

https://github.com/hashicorp/terraform-provider-aws/blob/e3609fb6efb06f125e8e30078037db05b7ac7e02/internal/service/lakeformation/data_lake_settings.go#L268-L276

hashicorp / terraform-provider-aws