Terraform-provider-aws: aws_iam_instance_profile resource can't be deleted without refresh

[mmdriley]

If an aws_iam_instance_profile resource is created with role set (vs. roles, which is deprecated) then the resulting resource is written to the state file with only role populated.

An IAM instance profile cannot be deleted as long as there is a role attached.

If an attempt is made to delete the instance profile resource without an intervening refresh, the provider will read the (empty) roles property, detach no roles, then try to delete the resource. The delete will fail.

Most users won't see this because apply and plan refresh first, and the code for Get always sets roles.

I think the solution here is for instanceProfileRemoveAllRoles to consider the value of role as well as roles, without going so far as to try to detach the same role twice.

[radeksimko]

Hi @mmdriley

If an attempt is made to delete the instance profile resource without an intervening refresh

generally speaking I would expect -refresh=false to be used in rare cases as it has other potentially negative side effects caused by the fact that state is drifted.

Do you mind explaining why do you do that?

[handlerbot]

@radeksimko I can speak to that some from my own experience, though I can't speak for @mmdriley, of course. :-)

Terraform with -refresh=true on some of our configurations takes enough time to be annoying and disruptive to flow. While it's definitely the safest default for making changes, when I know that I am the only operator working on our configurations, and I'm doing tight loops where a full refresh cycle is happening every 5 or 10 minutes or so, I'll often use -refresh=false to save time and keep me moving faster, especially for something (relatively) safe like destroying an IAM role and instance profile that is no longer in use.

(This is also the only Terraform AWS resource that I have found where it's impossible to delete it while setting -refresh=false, so there was definitely a "principle of least astonishment" violation here for me...)

[mmdriley]

Thanks @handlerbot. Our use case is similar -- we have a process that owns and serializes all changes to our environment, so it seemed sane to disable refresh. This is the only issue we've hit so far.

[radeksimko]

I understand the need to use it occasionally, but I'd discourage anyone from disabling refresh for routine operations. Working with an up-to-date state is quite essential for Terraform.

Most, if not all Read() implementations of resources are able to detect existence of the resource and remove it from state (if it's gone). Such operation may cost 1 extra API call though unless we're talking about calling service/Delete* method and ignoring a particular error code (e.g. NotFoundException). We don't intend to add any expensive checks there so we always mainly expect refresh to do the job and the resource to be already gone (if it's gone in reality) before it even has chance to reach Delete().

I hope the reasoning makes sense.

That said #2983 is fairly innocent and will be partially removed (or reworked) in the next major version (2.0.0) of the provider along with the removal of the deprecated field (roles), so I don't mind merging it. 😉

[bflad]

This has been released in terraform-provider-aws version 1.7.1. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

[hashibot[bot]]

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!