hashicorp / terraform-provider-aws

The AWS Provider enables Terraform to manage AWS resources.
https://registry.terraform.io/providers/hashicorp/aws
Mozilla Public License 2.0
9.85k stars 9.2k forks source link

[Enhancement]: aws_iam_service_linked_role adopt existing resource #29318

Open sidekick-eimantas opened 1 year ago

sidekick-eimantas commented 1 year ago

Description

Multiple aws_iam_service_linked_role's are created automatically by AWS when an account is opened. Namely the "organizations.amazonaws.com", "support.amazonaws.com" and "trustedadvisor.amazonaws.com". As part of an account setup, we try to create those service linked roles, and a few others. Those three however fail as they already exist. Importing them is too inconvenient in fully CI/CD'd setups. Would be great if the aws_iam_service_linked_role resource could automatically adopt an existing service linked role.

Affected Resource(s) and/or Data Source(s)

Potential Terraform Configuration

No response

References

No response

Would you like to implement a fix?

None

github-actions[bot] commented 1 year ago

Community Note

Voting for Prioritization

Volunteering to Work on This Issue

gdavison commented 1 year ago

Thanks for filing this, @sidekick-eimantas. I was about to do the same.

timsutton commented 1 year ago

Would be great if the aws_iam_service_linked_role resource could automatically adopt an existing service linked role.

I had a similar thought. Is there a workaround that might be usable today? As in to maybe try importing an existing IAM role with the expected name, and then invoke this role conditionally if a resource could be imported?

WolverineFan commented 7 months ago

NOTE: If someone implements this, the adopted resource should NOT be deleted with a terraform destroy. It would be quite possible that multiple modules/state files would end up referencing the same adopted resource, and if you ran a destroy on just one of them it would break them all. An example of that is the AWSServiceRoleForAutoScaling role, which might be "created" alongside every autoscaling group, but shouldn't be destroyed or it will impact ALL autoscaling groups.

WolverineFan commented 7 months ago

Would be great if the aws_iam_service_linked_role resource could automatically adopt an existing service linked role.

I had a similar thought. Is there a workaround that might be usable today? As in to maybe try importing an existing IAM role with the expected name, and then invoke this role conditionally if a resource could be imported?

You could maybe use an import block in Terraform 1.5+ but you would hit the problem I describe in my comment above where the imported resource would be destroyed and that may not be what you want.

alexbacchin commented 2 months ago

As IAM Service Linked roles can be created either by the CreateServiceLinkedRole action via IAM API or AWS itself when a service is invoked. It makes difficult to deploy/maintain modules that include the aws_iam_service_linked_role resource. If another module or AWS creates the role, the resource throws an error if the role exists.
The import is a viable option is some scenarios, but not practical when 2 modules need this to ensure this role is present.

I can work on a PR to implement this, but I am not sure if there is support from community to get this changed

onefifth commented 2 months ago

This is especially problematic for service linked roles that are globally only-one-per-account and do not support a custom_suffix argument. (e.g. ecs.amazonaws.com/AWSServiceRoleForECS)

At the very least, creating a Data Source for these feels like it'd help significantly. This would at least allow multiple terraform modules managing a single aws account to reference the globally shared role while avoiding ownership issues. It would be the responsibility of the user to create the resource manually, or ensure they have a singular account-wide resource managed by terraform outside of any modules.

If you wanted to take the Data Source a step further, it could neat to give them some kind of create_if_missing argument (similar to the ideas pitched in https://github.com/hashicorp/terraform/issues/33633). Ownership becomes a bit muddier in this scenario, but I think it's a pretty reasonable pattern for these account wide "singleton" style roles. Considering many of them have minimal configurable options and are often created automatically by other aws tooling, as a user I'd be totally okay if a create_if_missing feature resulted in a resource that was completely unmanaged by terraform.

alexbacchin commented 2 months ago

@onefifth @gdavison @timsutton @WolverineFan I raise a new PR #39441 for this. If you could please vote for it, it would be great!