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

Use separate TF resources for security group ingress/egress rules #1782

Merged
merged 3 commits into from
Sep 25, 2024

Conversation

markdboyd
Copy link
Contributor

Changes proposed in this pull request:

Part of https://github.com/cloud-gov/private/issues/1439

We have a problem where Terraform is constantly detecting changes to our aws_security_group resources, when in fact nothing is changing except that Terraform thinks the order of the cidr_blocks values have changed.

After looking for an easy fix in the Terraform AWS provider GitHub, I struck out and then went to the documentation page for the aws_security_group resource which has this note:

Avoid using the ingress and egress arguments of the aws_security_group resource to configure in-line rules, as they struggle with managing multiple CIDR blocks, and, due to the historical lack of unique IDs, tags and descriptions. To avoid these problems, use the current best practice of the aws_vpc_security_group_egress_rule and aws_vpc_security_group_ingress_rule resources with one CIDR block per rule.

In line with those suggestions, I am refactoring the code to use separate aws_vpc_security_group_ingress_rule and aws_vpc_security_group_egress_rule resources to have a separate ingress/egress rule for each CIDR block. I think this refactoring will resolve the issue with Terraform always detecting changes because each IP will effectively create a different resource in Terraform and thus ordering should not matter.

security considerations

There should be no material changes to security group ingress/egress, just refactoring how those resources are declared in Terraform.

@markdboyd markdboyd requested a review from a team as a code owner September 25, 2024 19:23
@bengerman13
Copy link
Contributor

I'm guessing this might force recreates on some of these resources. Do we have a strategy to validate there's no downtime applying this in dev/stage before applying it to prod?

@markdboyd markdboyd merged commit fa0fb06 into main Sep 25, 2024
2 checks passed
@markdboyd markdboyd deleted the update-security-group-rules branch September 25, 2024 19:43
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.

2 participants