aws_security_group_rule loses track of certain out-of-band changes

[johnrobisoncr]

Terraform Version

Terraform v0.11.1

• provider.aws v1.6.0

Affected Resource(s)

aws_security_group_rule

Terraform Configuration Files

module.security_group_tftest.aws_security_group_rule.ingress_cidr_blocks
  cidr_blocks.#:            "" => "1"
  cidr_blocks.0:            "" => "10.0.0.1/32"
  description:              "" => "blah blah blah"
  from_port:                "" => "0"
  protocol:                 "" => "tcp"
  security_group_id:        "" => "sg-e8815083"
  self:                     "" => "false"
  source_security_group_id: "" => "<computed>"
  to_port:                  "" => "0"
  type:                     "" => "ingress"

The specifics of the rule aren't super important, just any aws_security_group_rule with an IP address as the source will work.

Expected Behavior

Terraform should keep track of all aspects of the aws_security_group_rule, and revert all out of band changes back to the state described in Terraform code.

Actual Behavior

Terraform does keep track of a rule's "description" and does revert all out of band changes to it. But Terraform loses track of OOB changes to a rule's IP address, then creates a new rule with the desired state.

Steps to Reproduce

1. Create an aws_security_group_rule like the above using Terraform.
2. Go into the AWS console and make an out of band change to the rule description
3. Run terraform apply, and terraform will notice the OOB description change and fix it. This is the desired behavior.
4. Go back to the AWS console and change the source IP of the rule
5. Run terraform apply again. Terraform no longer detects the OOB change, and it will create the desired rule all over again. Now you have 2 rules, one with the OOB IP change and one with the state as described in Terraform code.

[bflad]

This resource does not have a great way to track out of band changes because AWS does not provide a stable identifier to tie to managing it. Changing IPs is one way to fool the security group rule lookup logic into thinking the rule has disappeared, due to how the EC2 API is structured.

Currently, if you want to manage all rules of an EC2 security group in holistic sense, the only option is to use aws_security_group with the ingress and egress blocks. Please be extremely careful if you do decide to use this resource as it will destroy security group rules not managed by it.

For reference, check out the EC2 SDK documentation where many of the SecurityGroup operations such as AuthorizeSecurityGroupIngress and UpdateSecurityGroupRuleDescriptions work off of a type called IpPermission, which is a combination of ports, IPs, protocols, etc. instead of a proper rule ID.

[radeksimko]

Just closed a few duplicate issues to direct the debate here.

@pmoust just to answer your question from #220 (comment)
We currently don't have any clean way to address this on provider level, hence I labelled this appropriately.

Resources are currently isolated from each other in terms of state and view of the world (API) and no resource can tell another via any special field that it holds the single point of truth, which is mostly for the better (esp. when needing to run operations in parallel), but sometimes not (like here).

We need to address this on core/schema level first and then implement it in AWS provider in those two resources.

[ze42]

If a security rule is defined by protocol/port/target, the current behaviour is supposed to be the correct one.

/Merely changing the IP/ is just like deleting a rule, and adding a new one.

Terraform tries to create what is missing, but terraform is NOT supposed to remove other unrelated rules.

aws_security_group_rule allows for rules to be added from different context.

To be sure to have only the one you want, you must either have them all listed within the security-group itself (but might have problems with cyclic dependecies), or create a (not yet supported) aws_security_group_rules that would manage them all.

Changing the behaviour of aws_security_group_rule would break existing use cases.

ie:

• Project 1 creating the security group with basic rules
• Projects 2-X adding rules to the security group from project 1.

[rabidscorpio]

Terraform tries to create what is missing, but terraform is NOT supposed to remove other unrelated rules.

Since when is terraform not supposed to remove other unrelated rules? That's the entire point of being declarative, you specify the rules you want and those are the rules you get. We're not running a procedural tool here where you specify what you want and then you can add things later.

Almost everything in terraform follows the paradigm of removing anything that's not in the current configuration. If you remove an instance or an ELB from your terraform configuration, guess what, that gets terminated on AWS. Why would security rules be any different?

I don't get why it would be hard to map of all the security rules a security group is supposed to have and then get rid of all the ones that aren't supposed to be there.

For example, I have a bastion host that has a security group with a list of allowed IP addresses, if I make any changes to that list, I want the old IP addresses to be removed instead of having to manually audit every single IP address in the security group. That's not how terraform is supposed to work.

[rabidscorpio]

This comment: hashicorp/terraform#11011 (comment) actually makes sense but there should be a way to opt-in or opt-out of this without having to do security group rules inline.

[ze42]

The mentionned comment is nice, and makes alot of sense, but does not address the cyclic rules that gets along with having rules defined within the security_group itself. Which is the only reason why I still consider aws_security_group_rules as a MUST have.

[github-actions]

Marking this issue as stale due to inactivity. This helps our maintainers find and focus on the active issues. If this issue receives no comments in the next 30 days it will automatically be closed. Maintainers can also remove the stale label.

If this issue was automatically closed and you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thank you!

[ghost]

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!