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.

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

Remove CreateLogGroup permission from service role #8

Merged
merged 2 commits into from
Apr 5, 2021

Conversation

navaati
Copy link
Contributor

@navaati navaati commented Jan 6, 2021

This permission is not needed because we create the log group with Terraform so the VPC Flow Logs service doesn’t need to do it. On the other hand having this permission causes a bug where, on terraform destroy the log group will be destroyed, but then if there is still a few messages in a VPC Flow Logs queue the managed service will see that the log group does not exist and create it again using.

You’ll then have the log group lingering after the tf destroy, which can cause trouble if you try to terraform apply again with the same name: the log group will be already existing and your apply will fail. Not having the permission prevents that as the managed service will not be able to recreate the log group after the tf destroy.

This permission is not needed because we create the log group with Terraform so the VPC Flow Logs service doesn’t need to do it. On the other hand having this permission causes a bug where, on `terraform destroy` the log group will be destroyed, but then if there is still a few messages in a VPC Flow Logs queue the managed service will see that the log group does not exist and create it again using.

You’ll then have the log group lingering after the tf destroy, which can cause trouble if you try to `terraform apply` again with the same name: the log group will be already existing and your apply will fail. Not having the permission prevents that as the managed service will not be able to recreate the log group after the tf destroy.
Copy link
Contributor

@mdrummerboy09 mdrummerboy09 left a comment

Choose a reason for hiding this comment

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

👍

@mdrummerboy09 mdrummerboy09 merged commit a0bc6c6 into trussworks:master Apr 5, 2021
@navaati navaati deleted the patch-1 branch April 7, 2021 15:35
@navaati
Copy link
Contributor Author

navaati commented Apr 8, 2021

Hey, thanks for merging. Will you push the new 2.2.0 release to Terraform Registry soon ? It’s not available yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants