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

Removing ingress rules from aws_security_group is not detected #4399

Closed
ghost opened this issue Apr 30, 2018 · 15 comments · Fixed by #32424
Closed

Removing ingress rules from aws_security_group is not detected #4399

ghost opened this issue Apr 30, 2018 · 15 comments · Fixed by #32424
Labels
bug Addresses a defect in current functionality. service/ec2 Issues and PRs that pertain to the ec2 service.
Milestone

Comments

@ghost
Copy link

ghost commented Apr 30, 2018

This issue was originally opened by @BookOfGreg as hashicorp/terraform#17967. It was migrated here as a result of the provider split. The original body of the issue is below.


Terraform Version

Terraform v0.11.7
+ provider.aws v1.15.0

Terraform Configuration Files

Removing Ingress from a security group has no effect

Before:

resource "aws_security_group" "my_group" {
  vpc_id      = "${aws_vpc.my_vpc.id}"
  name        = "my_group"
  description = "App security group"

  ingress {
    from_port = 80
    to_port   = 80
    protocol  = "tcp"
    cidr_blocks = ["0.0.0.0/0"]
  }

  egress {
    from_port   = 0
    to_port     = 0
    protocol    = "-1"
    cidr_blocks = ["0.0.0.0/0"]
  }
}

After:

resource "aws_security_group" "my_group" {
  vpc_id      = "${aws_vpc.my_vpc.id}"
  name        = "my_group"
  description = "App security group"

  egress {
    from_port   = 0
    to_port     = 0
    protocol    = "-1"
    cidr_blocks = ["0.0.0.0/0"]
  }
}

Expected Behavior

My security group has no ingress on it

Actual Behavior

My security group still has port 80

References

I've seen issues with similar symptoms for tools written in Go, such as this K8s bug I found:
kubernetes/kubernetes#59482
Not sure if relevant or not, feel free to remove the link from this post if it's a red herring.

@bflad bflad added bug Addresses a defect in current functionality. service/ec2 Issues and PRs that pertain to the ec2 service. labels May 1, 2018
@mickeytheq
Copy link

mickeytheq commented Aug 3, 2018

I am also seeing this issue. Removing the last ingress or egress rule from the security group results in no plan being generated.

I have tested the following scenarios

  • Removing one or more Ingress/Egress rules but leaving at least one rule: Works correctly
  • Removing one or more Ingress/Egress rules and leaving zero remaining: Does not work correctly (no plan generated)

I think any change that removes the last rule of either type does not work correctly.

@mickeytheq
Copy link

Using separate aws_security_group_rule resources instead of inline rules works around this problem.

@craiggenner
Copy link
Contributor

craiggenner commented Sep 7, 2018

Also seeing this.

Terraform v0.11.7
+ provider.aws v1.21.0

@pib
Copy link

pib commented Oct 2, 2018

Using separate aws_security_group_rule resources instead of inline rules works around this problem.

This does work, unless you are trying to transition from inline ingress rules to separate aws_security_group_rule resources. Then you just get errors back from AWS telling you that you have duplicate rules.

A (somewhat ugly) solution that I'm using is to add a dummy ingress rule to force it to remove the real ingress rules as I move them to aws_security_group_rule resources. Then later the dummy rule can be removed (though the actual ingress rule won't be removed until/unless this bug is fixed at some point or someone manually fixes it).

Edit: Actually doing it this way will cause it to cycle back and forth between removing external rules and adding them back in, so there's no good non-manual solution I have found yet.

@bflad
Copy link
Contributor

bflad commented Oct 2, 2018

FYI, there's an open pull request to support aws_security_group_rule resource import: #6027 (👍 reactions on the PR can help prioritize)

@OpenSourceZombie
Copy link

Same problem

  • Terraform v0.11.11
  • provider.aws v1.56.0
  • provider.random v2.0.0

@danieladams456
Copy link
Contributor

I ran into this on egress rules as well. I feel like it is a pretty important bug since apply runs with no errors, but rules are not removed. This could potentially lead to security issues due to extra rules that are still there.

@BookOfGreg
Copy link
Contributor

It does make me wonder how many other list-removal-related bugs there are currently if it's systematic to both this, Kubernetes, and other programs written in Go.

@lorengordon
Copy link
Contributor

What happens if you set ingress/egress to an empty list, rather than removing it entirely?

ingress = []

@hdpe
Copy link

hdpe commented May 13, 2019

What happens if you set ingress/egress to an empty list, rather than removing it entirely?

This worked for me. Thanks!

@winglot
Copy link

winglot commented Nov 24, 2020

What happens if you set ingress/egress to an empty list, rather than removing it entirely?

ingress = []

This one works and can also be applied as workaroud when you tried to use the dynamic block.

Bugged with dynamic:

  dynamic "ingress" {
    for_each = var.security_group_ingress_rules

    content {
      from_port       = ingress.value.from_port
      to_port         = ingress.value.to_port
      protocol        = ingress.value.protocol
      cidr_blocks     = ingress.value.cidr_blocks
      security_groups = ingress.value.security_groups
      self            = ingress.value.self
    }
  }

Workaround using a for loop:

  ingress = [for _, rule in var.security_group_ingress_rules : {
    from_port       = rule.from_port
    to_port         = rule.to_port
    protocol        = rule.protocol
    cidr_blocks     = rule.cidr_blocks
    security_groups = rule.security_groups
    self            = rule.self

    description      = null
    ipv6_cidr_blocks = null
    prefix_list_ids  = null
  }]

the downside is that you have to provide all parameters (event the ones that are not required) and set unused to null.

It's ugly, but it works for now if you cannot use aws_security_group_rule resource.

@hwn123
Copy link

hwn123 commented Oct 25, 2021

Two years later, aws provider version 3.58.0 - issue is still here. It is disappointing that terraform can not handle crucial for security resources well.

@Turtwiggy
Copy link

Turtwiggy commented May 11, 2022

Still occurring for me as well on:

  • Terraform v1.1.9
  • on windows_386
  • provider registry.terraform.io/hashicorp/aws v4.11.0

@github-actions
Copy link

This functionality has been released in v5.8.0 of the Terraform AWS Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

@github-actions
Copy link

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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Addresses a defect in current functionality. service/ec2 Issues and PRs that pertain to the ec2 service.
Projects
None yet
Development

Successfully merging a pull request may close this issue.