From 9a656796cd7d9914c9dfbb57c4a7d0b33455d9fb Mon Sep 17 00:00:00 2001 From: Rick Burgess Date: Fri, 2 Nov 2018 15:02:32 +0100 Subject: [PATCH 1/4] Updated iam_user force delete to include public ssh keys. fixes #4176 --- aws/resource_aws_iam_user.go | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/aws/resource_aws_iam_user.go b/aws/resource_aws_iam_user.go index 7501bb0b589..0ba12dea5fc 100644 --- a/aws/resource_aws_iam_user.go +++ b/aws/resource_aws_iam_user.go @@ -229,6 +229,30 @@ func resourceAwsIamUserDelete(d *schema.ResourceData, meta interface{}) error { } } + var publicKeys []string + listSSHPublicKeys := &iam.ListSSHPublicKeysInput{ + UserName: aws.String(d.Id()), + } + pageOfListSSHPublicKeys := func(page *iam.ListSSHPublicKeysOutput, lastPage bool) (shouldContinue bool) { + for _, k := range page.SSHPublicKeys { + publicKeys = append(publicKeys, *k.SSHPublicKeyId) + } + return !lastPage + } + err = iamconn.ListSSHPublicKeysPages(listSSHPublicKeys, pageOfListSSHPublicKeys) + if err != nil { + return fmt.Errorf("Error removing public ssh keys of user %s: %s", d.Id(), err) + } + for _, k := range publicKeys { + _, err := iamconn.DeleteSSHPublicKey(&iam.DeleteSSHPublicKeyInput{ + UserName: aws.String(d.Id()), + SSHPublicKeyId: aws.String(k), + }) + if err != nil { + return fmt.Errorf("Error deleting access key %s: %s", k, err) + } + } + var MFADevices []string listMFADevices := &iam.ListMFADevicesInput{ UserName: aws.String(d.Id()), From e225279c8692497cb87b399a0a25aeb55fa89e6e Mon Sep 17 00:00:00 2001 From: Rick Burgess Date: Fri, 2 Nov 2018 15:32:10 +0100 Subject: [PATCH 2/4] fixed a copy paste error --- aws/resource_aws_iam_user.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/aws/resource_aws_iam_user.go b/aws/resource_aws_iam_user.go index 0ba12dea5fc..98fff704305 100644 --- a/aws/resource_aws_iam_user.go +++ b/aws/resource_aws_iam_user.go @@ -241,7 +241,7 @@ func resourceAwsIamUserDelete(d *schema.ResourceData, meta interface{}) error { } err = iamconn.ListSSHPublicKeysPages(listSSHPublicKeys, pageOfListSSHPublicKeys) if err != nil { - return fmt.Errorf("Error removing public ssh keys of user %s: %s", d.Id(), err) + return fmt.Errorf("Error removing public SSH keys of user %s: %s", d.Id(), err) } for _, k := range publicKeys { _, err := iamconn.DeleteSSHPublicKey(&iam.DeleteSSHPublicKeyInput{ @@ -249,7 +249,7 @@ func resourceAwsIamUserDelete(d *schema.ResourceData, meta interface{}) error { SSHPublicKeyId: aws.String(k), }) if err != nil { - return fmt.Errorf("Error deleting access key %s: %s", k, err) + return fmt.Errorf("Error deleting public SSH key %s: %s", k, err) } } From 51191b66cfa908c8aa1ac373c77b440503cb1264 Mon Sep 17 00:00:00 2001 From: Rick Burgess Date: Sat, 3 Nov 2018 08:29:42 +0100 Subject: [PATCH 3/4] added tests for force delete path --- aws/resource_aws_iam_user_test.go | 58 ++++++++++++++++++++++++++++ aws/test-fixtures/public-ssh-key.pub | 1 + 2 files changed, 59 insertions(+) create mode 100644 aws/test-fixtures/public-ssh-key.pub diff --git a/aws/resource_aws_iam_user_test.go b/aws/resource_aws_iam_user_test.go index 2a38e81f32d..c49c9ebe074 100644 --- a/aws/resource_aws_iam_user_test.go +++ b/aws/resource_aws_iam_user_test.go @@ -4,6 +4,8 @@ import ( "fmt" "testing" + "io/ioutil" + "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/iam" "github.com/hashicorp/terraform/helper/acctest" @@ -368,3 +370,59 @@ resource "aws_iam_user" "user" { } `, rName, permissionsBoundary) } + +func TestAccAWSUser_ForceDestroy_SSHKey(t *testing.T) { + var user iam.GetUserOutput + + rName := acctest.RandomWithPrefix("tf-acc-test") + resourceName := "aws_iam_user.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSUserDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSUserConfigForceDestroy(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSUserExists(resourceName, &user), + testAccCheckAWSUserUploadsSSHKey(&user), + ), + }, + }, + }) +} + +func testAccCheckAWSUserUploadsSSHKey(getUserOutput *iam.GetUserOutput) resource.TestCheckFunc { + + return func(s *terraform.State) error { + + sshKey, err := ioutil.ReadFile("./test-fixtures/public-ssh-key.pub") + if err != nil { + return fmt.Errorf("error reading SSH fixture: %s", err) + } + + iamconn := testAccProvider.Meta().(*AWSClient).iamconn + + input := &iam.UploadSSHPublicKeyInput{ + UserName: getUserOutput.User.UserName, + SSHPublicKeyBody: aws.String(string(sshKey)), + } + + _, err = iamconn.UploadSSHPublicKey(input) + if err != nil { + return fmt.Errorf("error uploading IAM User (%s) SSH key: %s", *getUserOutput.User.UserName, err) + } + + return nil + } +} + +func testAccAWSUserConfigForceDestroy(rName string) string { + return fmt.Sprintf(` +resource "aws_iam_user" "test" { + force_destroy = true + name = %q +} +`, rName) +} diff --git a/aws/test-fixtures/public-ssh-key.pub b/aws/test-fixtures/public-ssh-key.pub new file mode 100644 index 00000000000..70f18ecfd04 --- /dev/null +++ b/aws/test-fixtures/public-ssh-key.pub @@ -0,0 +1 @@ +ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQCtCk0lzMj1gPEOjdfQ37AIxCyETqJBubaMWuB4bgvGHp8LvEghr2YDl2bml1JrE1EOcZhPnIwgyucryXKA959sTUlgbvaFN7vmpVze56Q9tVU6BJQxOdaRoy5FcQMET9LB6SdbXk+V4CkDMsQNaFXezpg98HgCj+V7+bBWsfI6U63IESlWKK7kraCom8EWxkQk4mk9fizE2I+KrtiqN4xcah02LFG6IMnS+Xy3CDhcpZeYzWOV6zhcf675UJOdg/pLgQbUhhiwTOJFgRo8IcvE3iBrRMz508ppx6vLLr8J+3B8ujykc+/3ZSGfQfx6rO+OuSskhG5FLI6icbQBtBzf terraform-provider-aws@hashicorp.com From e4631a3b12372cd384415e02ae21d16e6b7d9bb1 Mon Sep 17 00:00:00 2001 From: Rick Burgess Date: Sun, 4 Nov 2018 10:23:14 +0100 Subject: [PATCH 4/4] Refactor: moved all delete behaviours into seperate functions This makes the force delete flow more obvious and allows for easier reordering or parallelisation of delete behaviours --- aws/resource_aws_iam_user.go | 205 +++++++++++++++++++++-------------- 1 file changed, 124 insertions(+), 81 deletions(-) diff --git a/aws/resource_aws_iam_user.go b/aws/resource_aws_iam_user.go index 98fff704305..830674ebe05 100644 --- a/aws/resource_aws_iam_user.go +++ b/aws/resource_aws_iam_user.go @@ -205,97 +205,25 @@ func resourceAwsIamUserDelete(d *schema.ResourceData, meta interface{}) error { // All access keys, MFA devices and login profile for the user must be removed if d.Get("force_destroy").(bool) { - var accessKeys []string - listAccessKeys := &iam.ListAccessKeysInput{ - UserName: aws.String(d.Id()), - } - pageOfAccessKeys := func(page *iam.ListAccessKeysOutput, lastPage bool) (shouldContinue bool) { - for _, k := range page.AccessKeyMetadata { - accessKeys = append(accessKeys, *k.AccessKeyId) - } - return !lastPage - } - err = iamconn.ListAccessKeysPages(listAccessKeys, pageOfAccessKeys) + + err = deleteAwsIamUserAccessKeys(iamconn, d.Id()) if err != nil { - return fmt.Errorf("Error removing access keys of user %s: %s", d.Id(), err) - } - for _, k := range accessKeys { - _, err := iamconn.DeleteAccessKey(&iam.DeleteAccessKeyInput{ - UserName: aws.String(d.Id()), - AccessKeyId: aws.String(k), - }) - if err != nil { - return fmt.Errorf("Error deleting access key %s: %s", k, err) - } + return err } - var publicKeys []string - listSSHPublicKeys := &iam.ListSSHPublicKeysInput{ - UserName: aws.String(d.Id()), - } - pageOfListSSHPublicKeys := func(page *iam.ListSSHPublicKeysOutput, lastPage bool) (shouldContinue bool) { - for _, k := range page.SSHPublicKeys { - publicKeys = append(publicKeys, *k.SSHPublicKeyId) - } - return !lastPage - } - err = iamconn.ListSSHPublicKeysPages(listSSHPublicKeys, pageOfListSSHPublicKeys) + err = deleteAwsIamUserSSHKeys(iamconn, d.Id()) if err != nil { - return fmt.Errorf("Error removing public SSH keys of user %s: %s", d.Id(), err) - } - for _, k := range publicKeys { - _, err := iamconn.DeleteSSHPublicKey(&iam.DeleteSSHPublicKeyInput{ - UserName: aws.String(d.Id()), - SSHPublicKeyId: aws.String(k), - }) - if err != nil { - return fmt.Errorf("Error deleting public SSH key %s: %s", k, err) - } + return err } - var MFADevices []string - listMFADevices := &iam.ListMFADevicesInput{ - UserName: aws.String(d.Id()), - } - pageOfMFADevices := func(page *iam.ListMFADevicesOutput, lastPage bool) (shouldContinue bool) { - for _, m := range page.MFADevices { - MFADevices = append(MFADevices, *m.SerialNumber) - } - return !lastPage - } - err = iamconn.ListMFADevicesPages(listMFADevices, pageOfMFADevices) + err = deleteAwsIamUserMFADevices(iamconn, d.Id()) if err != nil { - return fmt.Errorf("Error removing MFA devices of user %s: %s", d.Id(), err) - } - for _, m := range MFADevices { - _, err := iamconn.DeactivateMFADevice(&iam.DeactivateMFADeviceInput{ - UserName: aws.String(d.Id()), - SerialNumber: aws.String(m), - }) - if err != nil { - return fmt.Errorf("Error deactivating MFA device %s: %s", m, err) - } + return err } - err = resource.Retry(1*time.Minute, func() *resource.RetryError { - _, err = iamconn.DeleteLoginProfile(&iam.DeleteLoginProfileInput{ - UserName: aws.String(d.Id()), - }) - if err != nil { - if isAWSErr(err, iam.ErrCodeNoSuchEntityException, "") { - return nil - } - // EntityTemporarilyUnmodifiable: Login Profile for User XXX cannot be modified while login profile is being created. - if isAWSErr(err, iam.ErrCodeEntityTemporarilyUnmodifiableException, "") { - return resource.RetryableError(err) - } - return resource.NonRetryableError(err) - } - return nil - }) - + err = deleteAwsIamUserLoginProfile(iamconn, d.Id()) if err != nil { - return fmt.Errorf("Error deleting Account Login Profile: %s", err) + return err } } @@ -324,3 +252,118 @@ func validateAwsIamUserName(v interface{}, k string) (ws []string, errors []erro } return } + +func deleteAwsIamUserSSHKeys(svc *iam.IAM, username string) error { + var publicKeys []string + var err error + + listSSHPublicKeys := &iam.ListSSHPublicKeysInput{ + UserName: aws.String(username), + } + pageOfListSSHPublicKeys := func(page *iam.ListSSHPublicKeysOutput, lastPage bool) (shouldContinue bool) { + for _, k := range page.SSHPublicKeys { + publicKeys = append(publicKeys, *k.SSHPublicKeyId) + } + return !lastPage + } + err = svc.ListSSHPublicKeysPages(listSSHPublicKeys, pageOfListSSHPublicKeys) + if err != nil { + return fmt.Errorf("Error removing public SSH keys of user %s: %s", username, err) + } + for _, k := range publicKeys { + _, err := svc.DeleteSSHPublicKey(&iam.DeleteSSHPublicKeyInput{ + UserName: aws.String(username), + SSHPublicKeyId: aws.String(k), + }) + if err != nil { + return fmt.Errorf("Error deleting public SSH key %s: %s", k, err) + } + } + + return nil +} + +func deleteAwsIamUserMFADevices(svc *iam.IAM, username string) error { + var MFADevices []string + var err error + + listMFADevices := &iam.ListMFADevicesInput{ + UserName: aws.String(username), + } + pageOfMFADevices := func(page *iam.ListMFADevicesOutput, lastPage bool) (shouldContinue bool) { + for _, m := range page.MFADevices { + MFADevices = append(MFADevices, *m.SerialNumber) + } + return !lastPage + } + err = svc.ListMFADevicesPages(listMFADevices, pageOfMFADevices) + if err != nil { + return fmt.Errorf("Error removing MFA devices of user %s: %s", username, err) + } + for _, m := range MFADevices { + _, err := svc.DeactivateMFADevice(&iam.DeactivateMFADeviceInput{ + UserName: aws.String(username), + SerialNumber: aws.String(m), + }) + if err != nil { + return fmt.Errorf("Error deactivating MFA device %s: %s", m, err) + } + } + + return nil +} + +func deleteAwsIamUserLoginProfile(svc *iam.IAM, username string) error { + var err error + err = resource.Retry(1*time.Minute, func() *resource.RetryError { + _, err = svc.DeleteLoginProfile(&iam.DeleteLoginProfileInput{ + UserName: aws.String(username), + }) + if err != nil { + if isAWSErr(err, iam.ErrCodeNoSuchEntityException, "") { + return nil + } + // EntityTemporarilyUnmodifiable: Login Profile for User XXX cannot be modified while login profile is being created. + if isAWSErr(err, iam.ErrCodeEntityTemporarilyUnmodifiableException, "") { + return resource.RetryableError(err) + } + return resource.NonRetryableError(err) + } + return nil + }) + + if err != nil { + return fmt.Errorf("Error deleting Account Login Profile: %s", err) + } + + return nil +} + +func deleteAwsIamUserAccessKeys(svc *iam.IAM, username string) error { + var accessKeys []string + var err error + listAccessKeys := &iam.ListAccessKeysInput{ + UserName: aws.String(username), + } + pageOfAccessKeys := func(page *iam.ListAccessKeysOutput, lastPage bool) (shouldContinue bool) { + for _, k := range page.AccessKeyMetadata { + accessKeys = append(accessKeys, *k.AccessKeyId) + } + return !lastPage + } + err = svc.ListAccessKeysPages(listAccessKeys, pageOfAccessKeys) + if err != nil { + return fmt.Errorf("Error removing access keys of user %s: %s", username, err) + } + for _, k := range accessKeys { + _, err := svc.DeleteAccessKey(&iam.DeleteAccessKeyInput{ + UserName: aws.String(username), + AccessKeyId: aws.String(k), + }) + if err != nil { + return fmt.Errorf("Error deleting access key %s: %s", k, err) + } + } + + return nil +}