Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Don't pass IAM role to ECS Service when there are more than one Target Groups #83

Merged
merged 14 commits into from
Dec 1, 2020
Merged

Don't pass IAM role to ECS Service when there are more than one Target Groups #83

merged 14 commits into from
Dec 1, 2020

Conversation

msmagoo87
Copy link
Contributor

@msmagoo87 msmagoo87 commented Nov 30, 2020 •
edited

what

  • In the case of registering more than one Target Group, don't create or pass the ECS service IAM role to the aws_ecs_service block.

why

  • If you are registering multiple target groups to the service, you must allow ECS to create a service-linked role rather than passing one that was manually created. Passing one results in an API error from ECS: Error: InvalidParameterException: You cannot specify an IAM role for services that require a service linked role.

references

@jamengual
Copy link
Contributor

/rebuild-readme

@jamengual
Copy link
Contributor

/test all

1 similar comment
@jamengual
Copy link
Contributor

/test all

jamengual
jamengual reviewed Nov 30, 2020
View reviewed changes
main.tf Outdated Show resolved Hide resolved
@jamengual
Copy link
Contributor

/terraform-fmt

@jamengual
Copy link
Contributor

/test all

@msmagoo87
Copy link
Contributor Author

/test all

@msmagoo87
Copy link
Contributor Author

/rebuild-readme

@jamengual
Copy link
Contributor

/test all

@jamengual
Copy link
Contributor

/rebuild-readme

@jamengual
Copy link
Contributor

/test all

@jamengual
Copy link
Contributor

/terraform-fmt

@jamengual
Copy link
Contributor

/test all

@jamengual
Copy link
Contributor

/test all

2 similar comments
@jamengual
Copy link
Contributor

/test all

@jamengual
Copy link
Contributor

/test all

aknysh
aknysh reviewed Dec 1, 2020
View reviewed changes
main.tf Outdated Show resolved Hide resolved
aknysh
aknysh reviewed Dec 1, 2020
View reviewed changes
main.tf Outdated Show resolved Hide resolved
aknysh
aknysh reviewed Dec 1, 2020
View reviewed changes
Copy link
Member

@aknysh aknysh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please see comments

@msmagoo87 @aknysh
Update main.tf with suggestion
4047cd3 
Co-authored-by: Andriy Knysh <aknysh@users.noreply.github.com>
@msmagoo87 @aknysh
Update main.tf with suggestion
03131db 
Co-authored-by: Andriy Knysh <aknysh@users.noreply.github.com>
@jamengual
Copy link
Contributor

/test all

1 similar comment
@jamengual
Copy link
Contributor

/test all

@jamengual
Copy link
Contributor

/test all

@jamengual jamengual merged commit e8f90ee into cloudposse:master Dec 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aknysh aknysh aknysh left review comments

@jamengual jamengual jamengual approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@msmagoo87 @jamengual @aknysh @actions-bot