Catch AccessDenied permission errors when creating aws_iam_user_login…

[fbreckle]

…_profile resources.

Fixes #7518

Changes proposed in this pull request:

• AccessDenied errors are no longer silently ignored

I dont understand the rationale of the "error != NoSuchEntity" part below my changes.
My fix merely catches the AccessDenied error issue.

If someone knows why the error handling is written as-is, please explain.

[bflad]

Hi @fbreckle 👋 Thanks for submitting this.

It appears this resource was initially written in a way that contradicts how Terraform is designed to work for existing infrastructure. Terraform resources should require terraform import to begin managing existing infrastructure while this resource is instead automatically assuming control during resource creation. In a future major version release, we should expect this behavior to change and I have created a tracking issue: #7536

That said, we can still also fix the behavior for the current way this resource works. The resource creation logic in this case should be returning all errors (including networking issues, etc.) and not silently ignoring them. To accomplish this, I would suggest updating the code like below:

// DEPRECATED: Automatic import will be removed in a future major version update
// https://github.com/terraform-providers/terraform-provider-aws/issues/7536
_, err = iamconn.GetLoginProfile(&iam.GetLoginProfileInput{
	UserName: aws.String(username),
})

// If there is already a login profile, bring it under management (to prevent
// resource creation diffs) - we will never modify it, but obviously cannot
// set the password.
if err == nil {
  d.SetId(username)
  d.Set("key_fingerprint", "")
  d.Set("encrypted_password", "")
  return nil
}

if !isAWSErr(err, iam.ErrCodeNoSuchEntityException, "") {
  return fmt.Errorf("error checking for existing IAM User Login Profile: %s", err)
}

With the above change, we can get the updated error handling in before this resource is rewritten to appropriately behave like other Terraform resources in the next major version.

[fbreckle]

Hi @bflad,

thanks for your explanation. I figured already that the implementation on this method is weird/outdated, so I went with a "minimally invasive" implementation for my initial MR and just caught the AccessDenied error.
I changed the code to your snippet now, if it is still to be merged.

[bflad]

LGTM, thanks so much @fbreckle 🚀

--- PASS: TestAccAWSUserLoginProfile_notAKey (7.70s)
--- PASS: TestAccAWSUserLoginProfile_keybaseDoesntExist (7.76s)
--- PASS: TestAccAWSUserLoginProfile_keybase (13.44s)
--- PASS: TestAccAWSUserLoginProfile_PasswordLength (13.46s)
--- PASS: TestAccAWSUserLoginProfile_basic (25.08s)

[bflad]

This has been released in version 1.59.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!