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

Migrate SES SMTP password to v4 for existing aws_iam_access_key #14529

Closed
gileri opened this issue Aug 8, 2020 · 8 comments
Closed

Migrate SES SMTP password to v4 for existing aws_iam_access_key #14529

gileri opened this issue Aug 8, 2020 · 8 comments
Labels
enhancement Requests to existing resources that expand the functionality or scope. service/iam Issues and PRs that pertain to the iam service.

Comments

@gileri
Copy link
Contributor

gileri commented Aug 8, 2020

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • 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
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Description

Historically aws_iam_access_key only had the ses_smtp_password attribute, which was complemented by ses_smtp_password_v4, then ses_smtp_password was deprecated.

ses_smtp_password_v4 is derived from the SecretAccessKey during resource creation, and currently isn't created during resource update. Users must taint+recreate aws_iam_access_key to be able to continue SES SMTP passwords.

Two possible reasons may explain that missing "migration" :

  • The SecretAccessKey isn't stored by AWS, so during refresh it isn't available from AWS API's response.
  • The secret (=SecretAccessKey) attribute may not be available if user chose to PGP-encrypt the key.

However, if the secret isn't PGP-encrypted, it is stored in state, so ses_smtp_password_v4 could be calculated during resource update.

Would it be interesting to generate ses_smtp_password_v4 during resource update ? I know that access key rotation shouldn't be a big deal, but this deprecation makes upgrading less easy.

I made a POC that seems to work, but would like input from maintainers to know if this is a good idea before polishing it up.

diff --git a/aws/resource_aws_iam_access_key.go b/aws/resource_aws_iam_access_key.go
index f6dfdc3c1..dcc1b65f7 100644
--- a/aws/resource_aws_iam_access_key.go
+++ b/aws/resource_aws_iam_access_key.go
@@ -147,7 +147,7 @@ func resourceAwsIamAccessKeyRead(d *schema.ResourceData, meta interface{}) error
        return nil
 }
 
-func resourceAwsIamAccessKeyReadResult(d *schema.ResourceData, key *iam.AccessKeyMetadata) error {
+func resourceAwsIamAccessKeyReadResult(d *schema.ResourceData, key *iam.AccessKeyMetadata, region *string) error {
        d.SetId(*key.AccessKeyId)
        if err := d.Set("user", key.UserName); err != nil {
                return err
@@ -155,6 +155,13 @@ func resourceAwsIamAccessKeyReadResult(d *schema.ResourceData, key *iam.AccessKe
        if err := d.Set("status", key.Status); err != nil {
                return err
        }
+    var secret string = d.Get("secret").(string)
+    sesSMTPPasswordV4, err := sesSmtpPasswordFromSecretKeySigV4(&secret, *region)
+    if err != nil {
+        return fmt.Errorf("error getting SES SigV4 SMTP Password from Secret Access Key: %s", err)
+    }
+    d.Set("ses_smtp_password_v4", sesSMTPPasswordV4)
+
        return nil
 }

New or Affected Resource(s)

  • aws_iam_access_key.

Potential Terraform Configuration

resource "aws_iam_access_key" "lb" {
  user    = aws_iam_user.lb.name
  # pgp_key = "keybase:some_person_that_exists"
}

resource "aws_iam_user" "lb" {
  name = "loadbalancer"
}

References

@gileri gileri added the enhancement Requests to existing resources that expand the functionality or scope. label Aug 8, 2020
@ghost ghost added the service/iam Issues and PRs that pertain to the iam service. label Aug 8, 2020
@github-actions github-actions bot added the needs-triage Waiting for first response or review from a maintainer. label Aug 8, 2020
@jdheyburn
Copy link
Contributor

It might be difficult to translate this to the end user; will they understand why a field (ses_smtp_password_v4) is missing a value given they have pgp_key set?

I think a sane alternative that doesn't involve changing the provider is creating a new aws_iam_access_key for the IAM user and then using that new key downstream then removing the old one. It's like a taint-recreate but this way there is no downtime.

@Porkepix
Copy link

I'm taking the occasion of this issue to ask (unfortunately, IRC have been abandoned, and the Matrix room connected to Gitter doesn't seems to have any success): how is one supposed to achieve the migration cleanly nowadays?
I guess this issue is here to improve things, but currently, if we try to use ses_smtp_password_v4 while previous terraform use is from a long time ago (before provider version 2.50), this attribute doesn't exist in state file, and this leads to errors as the v4 isn't known.

Sure a solution is to destroy the resource, but this doesn't seems very clean, besides the fact it can cause temporary issues.
Is there some kind of ways to force an update on iam_access_key so that it creates its ses_smtp_password_v4 attribute in the state file? Because without changing anything to the iam_access_key resource, terraform consider there's nothing to change there, and therefore doesn't update it at all.

@jdheyburn
Copy link
Contributor

FWIW, I have raised a PR on improving the upgrade documentation. Please feel free to add suggestions on there.

#14829

@gileri
Copy link
Contributor Author

gileri commented Aug 25, 2020

Either way with or without provider modification this change is not currently clear to the end user.

If modifying the provider is not the best avenue, maybe we should modify the documentation to clarify this migration tidbit for a few months.

Copy link

Marking this issue as stale due to inactivity. This helps our maintainers find and focus on the active issues. If this issue receives no comments in the next 30 days it will automatically be closed. Maintainers can also remove the stale label.

If this issue was automatically closed and you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thank you!

@github-actions github-actions bot added the stale Old or inactive issues managed by automation, if no further action taken these will get closed. label Apr 11, 2024
@gileri
Copy link
Contributor Author

gileri commented Apr 21, 2024

Since the migration to SigV4 was 4 years ago, it's a bit late now. Closing.

@gileri gileri closed this as not planned Won't fix, can't repro, duplicate, stale Apr 21, 2024
Copy link

Warning

This issue has been closed, meaning that any additional comments are hard for our team to see. Please assume that the maintainers will not see them.

Ongoing conversations amongst community members are welcome, however, the issue will be locked after 30 days. Moving conversations to another venue, such as the AWS Provider forum, is recommended. If you have additional concerns, please open a new issue, referencing this one where needed.

@github-actions github-actions bot removed the stale Old or inactive issues managed by automation, if no further action taken these will get closed. label Apr 21, 2024
Copy link

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Requests to existing resources that expand the functionality or scope. service/iam Issues and PRs that pertain to the iam service.
Projects
None yet
Development

No branches or pull requests

4 participants