hashicorp / terraform-provider-aws

The AWS Provider enables Terraform to manage AWS resources.
https://registry.terraform.io/providers/hashicorp/aws
Mozilla Public License 2.0
9.61k stars 8.99k forks source link

[Bug]: aws_default_security_group doesn't work as documented. #27177

Open prowlaiii opened 1 year ago

prowlaiii commented 1 year ago

Terraform Core Version

1.2.8

AWS Provider Version

4.34.0

Affected Resource(s)

Documentation (https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/default_security_group, version 4.34.0) states:

And gives an example where removing the egress rule from the Terraform resource deletes it and only leaves the ingres rule:

If I do that and do "terraform plan" it says "No changes".

So, simply omitting the "egress" block does nothing.

However, doing a change to it (eg. changing the protocol) does cause a change.

(My use-case is to remove both ingress and egress rules, but I limited the examples here to just the egress rule, to tally with what's in the documentation.)

Expected Behavior

Expected a change to the Security Group egress rule to delete the item.

Actual Behavior

No changes

Relevant Error/Panic Output Snippet

No response

Terraform Configuration Files

Using the sample from the documentaiton to re-create the AWS initial default SG config. Comment out the egress block to remove the egress rule:

resource "aws_default_security_group" "default" {
  vpc_id                 = aws_vpc.main.id
  tags                   = merge( local.tags, {Name = "default"} )

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

terraform plan gives:

No changes. Your infrastructure matches the configuration.

Steps to Reproduce

Initial definition with ingress and egress rules:

resource "aws_default_security_group" "default" {
  vpc_id                 = aws_vpc.main.id
  tags                   = merge( local.tags, {Name = "default"} )

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

terraform plan gives:

No changes. Your infrastructure matches the configuration.

Remove the egress rule:

resource "aws_default_security_group" "default" {
  vpc_id                 = aws_vpc.main.id
  tags                   = merge( local.tags, {Name = "default"} )

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

terraform plan gives:

No changes. Your infrastructure matches the configuration.

Make a deliberate change to the protocol to force a change:

resource "aws_default_security_group" "default" {
  vpc_id                 = aws_vpc.main.id
  tags                   = merge( local.tags, {Name = "default"} )

  ingress {
    from_port   = 0
    protocol    = -1
    self        = true
    to_port     = 0
  }
  egress {
    cidr_blocks = ["0.0.0.0/0"]
    from_port   = 0
    #protocol    = "-1"
    protocol    = 80
    to_port     = 0
  }
}

terraform plan gives:

  # module.main.module.base.module.network.module.network.aws_default_security_group.default will be updated in-place
  ~ resource "aws_default_security_group" "default" {
      ~ egress                 = [
          - {
              - cidr_blocks      = [
                  - "0.0.0.0/0",
                ]
              - description      = ""
              - from_port        = 0
              - ipv6_cidr_blocks = []
              - prefix_list_ids  = []
              - protocol         = "-1"
              - security_groups  = []
              - self             = false
              - to_port          = 0
            },
          + {
              + cidr_blocks      = [
                  + "0.0.0.0/0",
                ]
              + description      = ""
              + from_port        = 0
              + ipv6_cidr_blocks = []
              + prefix_list_ids  = []
              + protocol         = "80"
              + security_groups  = []
              + self             = false
              + to_port          = 0
            },
        ]
        id                     = "sg-09b47369b0a59b875"
        name                   = "default"
        tags                   = {
           ....
        }
        # (7 unchanged attributes hidden)
    }

Debug Output

No response

Panic Output

No response

Important Factoids

My use-case is to remove both ingress and egress rules, but I limited the examples here to just the egress rule, to tally with what's in the documentation.

References

No response

Would you like to implement a fix?

As a workaround, setting the cider_block to [] (egress) and self to false (ingress) seems to kick the vending machine in the right place... Terraform plan then offers the changes as they seemingly would be, but on apply the ingress and egress rules are gone in the AWS Console.

Code:

resource "aws_default_security_group" "default" {
  vpc_id                 = aws_vpc.main.id
  tags                   = merge( local.tags, { Name = "default" } )
  revoke_rules_on_delete = false

  dynamic "ingress" {
    for_each = var.default_sg_deny ? { deny = "deny" } : { allow = "allow" }
    content {
      description = "Default ingress"
      from_port = 0
      protocol  = -1
      self      = var.default_sg_deny ? false : true
      to_port   = 0
    }
  }
  dynamic "egress" {
    for_each = var.default_sg_deny ? { deny = "deny" } : { allow = "allow" }
    content {
      cidr_blocks = var.default_sg_deny ? [] : ["0.0.0.0/0"]
      description = "Default egress"
      from_port   = 0
      protocol    = "-1"
      to_port     = 0
    }
  }
}

terraform plan

  # module.main.module.base.module.network.module.network.aws_default_security_group.default will be updated in-place
  ~ resource "aws_default_security_group" "default" {
      ~ egress                 = [
          - {
              - cidr_blocks      = [
                  - "0.0.0.0/0",
                ]
              - description      = ""
              - from_port        = 0
              - ipv6_cidr_blocks = []
              - prefix_list_ids  = []
              - protocol         = "-1"
              - security_groups  = []
              - self             = false
              - to_port          = 0
            },
          + {
              + cidr_blocks      = []
              + description      = "Default egress"
              + from_port        = 0
              + ipv6_cidr_blocks = []
              + prefix_list_ids  = []
              + protocol         = "-1"
              + security_groups  = []
              + self             = false
              + to_port          = 0
            },
        ]
        id                     = "sg-00ec07ce319b4afbd"
      ~ ingress                = [
          - {
              - cidr_blocks      = []
              - description      = ""
              - from_port        = 0
              - ipv6_cidr_blocks = []
              - prefix_list_ids  = []
              - protocol         = "-1"
              - security_groups  = []
              - self             = true
              - to_port          = 0
            },
          + {
              + cidr_blocks      = []
              + description      = "Default ingress"
              + from_port        = 0
              + ipv6_cidr_blocks = []
              + prefix_list_ids  = []
              + protocol         = "-1"
              + security_groups  = []
              + self             = false
              + to_port          = 0
            },
        ]
        name                   = "default"
      + revoke_rules_on_delete = false
        tags                   = {
           ....
        }
        # (6 unchanged attributes hidden)
    }

UPDATE: That doesn't work; Terraform then thinks it needs to create the rule(s) every iteration thereafter. I've also tried creating separate SG rules to apply to the default SG and putting an ignore_changes clause against them in the SG, but that doesn't work, as the rules creation insists on values being set. My latest workaround is to amend the rules to just allow ICMP to self for both ingress and egress, which is about as near to a NOP as I can get.

resource "aws_default_security_group" "default" {
  vpc_id                 = aws_vpc.main.id
  tags                   = merge( local.tags, { Name = "default" } )
  revoke_rules_on_delete = false

  # NOTE: The egress & ingress rules can be locked down by simply allowing access to self;
  #       1. simply removing them doesn't do what the documentation says and simply leaves them in-place,
  #       2. setting the values to null does remove them but then Terraform thinks it needs to re-apply
  #          every execution.
    dynamic "egress" {
      for_each = var.default_sg_deny ? { deny = "deny" } : { allow = "allow" }
      content {
        cidr_blocks      = var.default_sg_deny ? [] : ["0.0.0.0/0"]
        description      = var.default_sg_deny ? "Minimal default rule" : null
        from_port        = 0
        ipv6_cidr_blocks = []
        prefix_list_ids  = []
        protocol         = var.default_sg_deny ? "icmp" : -1
        security_groups  = []
        self             = var.default_sg_deny ? true : false
        to_port          = 0
      }
    }
    dynamic "ingress" {
      for_each = var.default_sg_deny ? { deny = "deny" } : { allow = "allow" }
      content {
        cidr_blocks      = []
        description      = var.default_sg_deny ? "Minimal default rule" : null
        from_port        = 0
        ipv6_cidr_blocks = []
        prefix_list_ids  = []
        protocol         = var.default_sg_deny ? "icmp" : -1
        security_groups  = []
        self             = true
        to_port          = 0
      }
    }

  # The default rules are:
  #egress {
  #  cidr_blocks = ["0.0.0.0/0"]
  #  from_port   = 0
  #  protocol    = "-1"
  #  to_port     = 0
  #}
  #ingress {
  #  from_port   = 0
  #  protocol    = -1
  #  self        = true
  #  to_port     = 0
  #}
}
github-actions[bot] commented 1 year ago

Community Note

Voting for Prioritization

Volunteering to Work on This Issue

tolidano commented 1 year ago

I wanted to share the way the "official" module does it in case that helps: https://github.com/terraform-aws-modules/terraform-aws-vpc/blob/master/main.tf#L1210

I just had default rules and then removed them and it worked properly, fwiw.

fd-npelliccia commented 2 weeks ago

Just tested and experiencing the issue as OP.