Updated iam_user force delete to include public ssh keys. fixes #4176

[nodefortytwo]

Fixes #4176

Changes proposed in this pull request:

• when force delete is specified delete any public ssh keys attached to user

Output from acceptance testing:
BEFORE change was applied:

-> dazn aws exec -p dazn-demo-one -- make testacc TEST=./aws/ TESTARGS='-run=TestAccAWSUser_ForceDestroy_SSHKey'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -parallel 20 -run=TestAccAWSUser_ForceDestroy_SSHKey -timeout 120m
=== RUN   TestAccAWSUser_ForceDestroy_SSHKey
=== PAUSE TestAccAWSUser_ForceDestroy_SSHKey
=== CONT  TestAccAWSUser_ForceDestroy_SSHKey
--- FAIL: TestAccAWSUser_ForceDestroy_SSHKey (17.93s)
    testing.go:599: Error destroying resource! WARNING: Dangling resources
        may exist. The full state and error is shown below.

        Error: Error applying: 1 error occurred:
                * aws_iam_user.test (destroy): 1 error occurred:
                * aws_iam_user.test: Error deleting IAM User tf-acc-test-1916028493612556295: DeleteConflict: Cannot delete entity, must remove referenced objectsfirst.
                status code: 409, request id: c393d746-e00e-11e8-9c3c-8dbf40989de3





        State: aws_iam_user.test:
          ID = tf-acc-test-1916028493612556295
          provider = provider.aws
          arn = arn:aws:iam::621041455573:user/tf-acc-test-1916028493612556295
          force_destroy = true
          name = tf-acc-test-1916028493612556295
          path = /
          unique_id = AIDAJHVSXXJ7EDMBLQLN6
FAIL
FAIL    github.com/nodefortytwo/terraform-provider-aws/aws      17.979s

AFTER changes

-> dazn aws exec -p dazn-demo-one -- make testacc TEST=./aws/ TESTARGS='-run=TestAccAWSUser_ForceDestroy_SSHKey'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -parallel 20 -run=TestAccAWSUser_ForceDestroy_SSHKey -timeout 120m
=== RUN   TestAccAWSUser_ForceDestroy_SSHKey
=== PAUSE TestAccAWSUser_ForceDestroy_SSHKey
=== CONT  TestAccAWSUser_ForceDestroy_SSHKey
--- PASS: TestAccAWSUser_ForceDestroy_SSHKey (19.99s)
PASS
ok      github.com/nodefortytwo/terraform-provider-aws/aws      20.032s

ignore the dazn bit, just generates temporary credentials to one of our ephemeral accounts

[tapatoo]

👍

[robertstettner]

👍

[bflad]

Output from acceptance testing: I cannot run without applying gofmt to loads of unchanged files?

Upgrading to Go 1.11 should make those disappear. 👍

[bflad]

It would be really nice to write up an acceptance test that actually covers this code path. We can set this up similar to how _disappears acceptance tests are written. e.g.

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: testAccAWSUserConfigForceDestory(rName),
				Check: resource.ComposeTestCheckFunc(
					testAccCheckAWSUserExists(resourceName, &user),
					testAccCheckAWSUserUploadsSSHKey(&user),
				),
			},
		},
	})
}

func testAccCheckAWSUserUploadsSSHKey(getUserOutput *iam.GetUserOutput) resource.TestCheckFunc {
	return func(s *terraform.State) error {
		iamconn := testAccProvider.Meta().(*AWSClient).iamconn

		input := &iam.UploadSSHPublicKeyInput{
			UserName:         getUserOutput.User.UserName,
			SSHPublicKeyBody: aws.String(/* can be hardcoded or preferably read from a file in aws/test-fixtures */),
		}

		_, err := iamconn.UploadSSHPublicKey(request)
		if err != nil {
			return fmt.Errorf("error uploading IAM User (%s) SSH key: %s", userName, err)
		}

		return nil
	}
}

func testAccAWSUserConfigForceDestroy(rName string) string {
	return fmt.Sprintf(`
resource "aws_iam_user" "test" {
  force_destroy = true
  name = %q
}
`, rName)
}

[nodefortytwo]

I'll address this over the weekend, thanks for the feedback

[nodefortytwo]

Tests added and results added to PR

[bflad]

Thanks for the updates, @nodefortytwo! Since the code surrounding the other force destroy options was also updated, I added acceptance testing coverage where I could as well in the followup commit (49bc1a740630b0b2aafbd67fc5f452f0ca457020).

🚀

--- PASS: TestAccAWSUser_disappears (7.15s)
--- PASS: TestAccAWSUser_ForceDestroy_SSHKey (9.38s)
--- PASS: TestAccAWSUser_importBasic (9.49s)
--- PASS: TestAccAWSUser_ForceDestroy_AccessKey (9.55s)
--- PASS: TestAccAWSUser_ForceDestroy_LoginProfile (9.78s)
--- PASS: TestAccAWSUser_pathChange (13.74s)
--- PASS: TestAccAWSUser_nameChange (13.80s)
--- PASS: TestAccAWSUser_basic (13.94s)
--- PASS: TestAccAWSUser_permissionsBoundary (31.21s)
--- SKIP: TestAccAWSUser_ForceDestroy_MFADevice (0.00s)
    resource_aws_iam_user_test.go:177: Virtual MFA device creation is not currently implemented

[bflad]

This has been released in version 1.43.0 of the AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

[ghost]

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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!