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

network acl: change in one rule triggers updates for all #10611

Open
giner opened this issue Oct 23, 2019 · 10 comments
Open

network acl: change in one rule triggers updates for all #10611

giner opened this issue Oct 23, 2019 · 10 comments
Labels
bug Addresses a defect in current functionality. service/ec2 Issues and PRs that pertain to the ec2 service.

Comments

@giner
Copy link

giner commented Oct 23, 2019

If icmp_code and icmp_type are not provided in the config (as these are optional) their values are sent to aws API as nulls however retrieved later as zeroes. This leads to reapplying all of the rules every time there is a small change.

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Terraform Version

0.12.9 with the latest version of the aws provider plugin

Affected Resource(s)

  • aws_network_acl

Terraform Configuration Files

          - {
              - action          = "allow"
              - cidr_block      = "0.0.0.0/0"
              - from_port       = 1024
              - icmp_code       = 0
              - icmp_type       = 0
              - ipv6_cidr_block = ""
              - protocol        = "6"
              - rule_no         = 300
              - to_port         = 65535
            },
          + {
              + action          = "allow"
              + cidr_block      = "0.0.0.0/0"
              + from_port       = 1024
              + icmp_code       = null
              + icmp_type       = null
              + ipv6_cidr_block = null
              + protocol        = "tcp"
              + rule_no         = 300
              + to_port         = 65535
            },

Expected Behavior

Change in a single rule updates only the related record.

Actual Behavior

Change in a single rule triggers updates for all rules.

Steps to Reproduce

  1. Add a couple of rules allowing TCP traffic to a network acl and run terraform apply
  2. Add one more rule and run terraform plan or terraform apply

Workaround

Specifying icmp_type and icmp_code explicitly as '0' helps to work around the issue.

@ghost ghost added the service/ec2 Issues and PRs that pertain to the ec2 service. label Oct 23, 2019
@github-actions github-actions bot added the needs-triage Waiting for first response or review from a maintainer. label Oct 23, 2019
@github-actions
Copy link

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!

@github-actions github-actions bot added the stale Old or inactive issues managed by automation, if no further action taken these will get closed. label Oct 13, 2021
@github-actions github-actions bot removed the stale Old or inactive issues managed by automation, if no further action taken these will get closed. label Oct 26, 2021
@justinretzolk
Copy link
Member

Hey @giner 👋 Thank you for taking the time to file this issue. Given that there's been a number of Terraform and AWS provider releases since you initially filed it, can you confirm whether you're still experiencing this behavior?

@justinretzolk justinretzolk added waiting-response Maintainers are waiting on response from community or contributor. and removed needs-triage Waiting for first response or review from a maintainer. labels Nov 18, 2021
@giner
Copy link
Author

giner commented Nov 19, 2021

Hi @justinretzolk,

The behaviour is a bit different now but still in place. Before the changes were mistakenly recognized in icmp_type and icmp_code. Now the changes are recognized in protocol:

Configuration:

        {
          rule_no    = 300
          action     = "allow"
          protocol   = "tcp"
          from_port  = 1024
          to_port    = 65535
          cidr_block = "0.0.0.0/0"
        },

Plan:

          - {
              - action          = "allow"
              - cidr_block      = "0.0.0.0/0"
              - from_port       = 1024
              - icmp_code       = 0
              - icmp_type       = 0
              - ipv6_cidr_block = ""
              - protocol        = "6"
              - rule_no         = 300
              - to_port         = 65535
            },
          + {
              + action          = "allow"
              + cidr_block      = "0.0.0.0/0"
              + from_port       = 1024
              + icmp_code       = 0
              + icmp_type       = 0
              + ipv6_cidr_block = ""
              + protocol        = "tcp"
              + rule_no         = 300
              + to_port         = 65535
            },
$ terraform --version
Terraform v1.0.11
on linux_amd64
...
+ provider registry.terraform.io/hashicorp/aws v3.66.0
...

@github-actions github-actions bot removed the waiting-response Maintainers are waiting on response from community or contributor. label Nov 19, 2021
@leondkr
Copy link

leondkr commented Sep 19, 2022

I am facing the same issue when adding a new inline rule.

Here is the existing rule which requires update in-place.

          - {
              - action          = "allow"
              - cidr_block      = "0.0.0.0/0"
              - from_port       = 443
              - icmp_code       = 0
              - icmp_type       = 0
              - ipv6_cidr_block = ""
              - protocol        = "6"
              - rule_no         = 102
              - to_port         = 443
            },
          + {
              + action          = "allow"
              + cidr_block      = "0.0.0.0/0"
              + from_port       = 443
              + icmp_code       = null
              + icmp_type       = null
              + ipv6_cidr_block = null
              + protocol        = "tcp"
              + rule_no         = 102
              + to_port         = 443
            }

This is my new rule that I would like to add

          + {
              + action          = "allow"
              + cidr_block      = "0.0.0.0/0"
              + from_port       = 4444
              + icmp_code       = null
              + icmp_type       = null
              + ipv6_cidr_block = ""
              + protocol        = "tcp"
              + rule_no         = 103
              + to_port         = 4444
            },

Here is my Terraform version:

❯ terraform version
Terraform v1.2.6
on darwin_amd64
+ provider registry.terraform.io/hashicorp/aws v4.31.0

Is there any workarounds for this as it has to destroy the old rules and re-create them which is not reasonable for me, especially on Production environment. Appreciate!!!

@justinretzolk
Copy link
Member

Hey y'all 👋 Can someone who is experiencing this behavior supply a sample configuration so we have the information we need to look into this?

@justinretzolk justinretzolk added the waiting-response Maintainers are waiting on response from community or contributor. label Sep 21, 2022
@leondkr
Copy link

leondkr commented Sep 22, 2022

@justinretzolk Hi Justin, here is my simple Terraform resource that I use for testing this one.

resource "aws_network_acl" "nacl" {
  vpc_id = "vpc-1234"
  subnet_ids = [
    "subnet-1111",
    "subnet-2222"
  ]

  ingress {
    from_port  = 0
    to_port    = 0
    rule_no    = 100
    action     = "allow"
    protocol   = "-1"
    cidr_block = "0.0.0.0/0"
  }

  egress {
    from_port  = 0
    to_port    = 0
    rule_no    = 100
    action     = "allow"
    protocol   = "-1"
    cidr_block = "10.0.0.0/8"
  }

  egress {
    from_port  = 80
    to_port    = 80
    rule_no    = 101
    action     = "allow"
    protocol   = "tcp"
    cidr_block = "0.0.0.0/0"
  }

  ## Try adding a new simple rule cause this behavior
  # egress {
  #   from_port  = 8080
  #   to_port    = 8080
  #   rule_no    = 102
  #   action     = "allow"
  #   protocol   = "tcp"
  #   cidr_block = "0.0.0.0/0"
  # }

  tags = {
    Name = "nacl-troubleshoot"
  }
}

Also, here is my provider version as well:

terraform {
  required_providers {
    aws = {
      source  = "hashicorp/aws"
      version = "4.31.0"
    }
  }
}

@github-actions github-actions bot removed the waiting-response Maintainers are waiting on response from community or contributor. label Sep 22, 2022
@olfway
Copy link

olfway commented Oct 7, 2022

Same with ipv6_cidr_block:

If you pass ipv6_cidr_block = null to resource then terraform show changes:

          - {
              - action          = "allow"
              - cidr_block      = "10.14.6.0/24"
              - from_port       = 10250
              - icmp_code       = 0
              - icmp_type       = 0
              - ipv6_cidr_block = ""
              - protocol        = "6"
              - rule_no         = 102
              - to_port         = 10250
            },
          + {
              + action          = "allow"
              + cidr_block      = "10.14.6.0/24"
              + from_port       = 10250
              + icmp_code       = 0
              + icmp_type       = 0
              + ipv6_cidr_block = null
              + protocol        = "6"
              + rule_no         = 102
              + to_port         = 10250

@gamidirajesh
Copy link

gamidirajesh commented Oct 31, 2022

{
rule_number = 140
rule_action = "allow"
from_port = 80
to_port = 80
protocol = "tcp"
cidr_block = "0.0.0.0/0"
},
{
rule_number = 130
rule_action = "allow"
from_port = 3389
to_port = 3389
protocol = "tcp"
cidr_block = "0.0.0.0/0"
},

I just changed the order from inbound rule 140 to inbound rule 130 and inbound rule 130 to inbound rule 140 . Nothing was added or removed.
But terraform plan shows 2 to add and 2 to destroy.
There is no issue with the default network ACL rule, bcoz it was created using for_each.

@justinretzolk justinretzolk added the bug Addresses a defect in current functionality. label Nov 3, 2022
@jin-ahn
Copy link

jin-ahn commented Jan 9, 2023

I also am facing this issue from cdktf. Below is portion of my config This makes using terraform for production NACL highly unusable.

  egress: [
    {
      ruleNo: 1,
      protocol: "-1",
      action: "allow",
      cidrBlock: "x.x.x.x/x",
      fromPort: 0,
      toPort: 0,
    },
    {
      ruleNo: 2,
      protocol: "-1",
      action: "allow",
      cidrBlock: "x.x.x.x/x",
      fromPort: 0,
      toPort: 0,
    },
    {
      ruleNo: 3,
      protocol: "-1",
      action: "allow",
      cidrBlock: "x.x.x.x/x",
      fromPort: 0,
      toPort: 0,
    },
    {
      ruleNo: 4,
      protocol: "tcp",
      action: "allow",
      cidrBlock: "x.x.x.x/x",
      fromPort: 0,
      toPort: 65535,
    },
    {
      ruleNo: 5,
      protocol: "tcp",
      action: "allow",
      cidrBlock: "x.x.x.x/x",
      fromPort: 0,
      toPort: 65535,
    },

@ardichoke
Copy link

I'm having this issue as well, when managing the default network acl for a VPC.

resource "aws_default_network_acl" "euc1_main_acl-default" {
  default_network_acl_id = aws_vpc.euc1-main.default_network_acl_id

  lifecycle {
    ignore_changes = [
      subnet_ids
    ]
  }

  # SSH Ingress Rules
  ingress {
    rule_no    = 10
    protocol   = "tcp"
    from_port  = 22
    to_port    = 22
    cidr_block = "10.0.0.0/8"
    action     = "allow"
  }
  ingress {
    rule_no    = 11
    protocol   = "tcp"
    from_port  = 22
    to_port    = 22
    cidr_block = "172.16.0.0/12"
    action     = "allow"
  }
  ingress {
    rule_no    = 20
    protocol   = "tcp"
    from_port  = 22
    to_port    = 22
    cidr_block = "0.0.0.0/0"
    action     = "deny"
  }
  ingress {
    rule_no         = 21
    protocol        = "tcp"
    from_port       = 22
    to_port         = 22
    ipv6_cidr_block = "::0/0"
    action          = "deny"
  }

  # Default Egress Rules
  egress {
    protocol   = -1
    rule_no    = 1000
    cidr_block = "0.0.0.0/0"
    from_port  = 0
    to_port    = 0
    action     = "allow"
  }
  egress {
    protocol        = -1
    rule_no         = 1001
    ipv6_cidr_block = "::/0"
    to_port         = 0
    from_port       = 0
    action          = "allow"
  }

  # Default Ingress Rules
  ingress {
    rule_no    = 1002
    protocol   = -1
    cidr_block = "0.0.0.0/0"
    from_port  = 0
    to_port    = 0
    action     = "allow"
  }
  ingress {
    rule_no         = 1003
    protocol        = -1
    ipv6_cidr_block = "::/0"
    from_port       = 0
    to_port         = 0
    action          = "allow"
  }
}

This plan has already been applied, when I run terraform plan on this code again, I get the following output:

  # aws_default_network_acl.euc1_main_acl-default will be updated in-place
  ~ resource "aws_default_network_acl" "euc1_main_acl-default" {
        id                     = "acl-0361f10f89046a2b7"
        tags                   = {}
        # (6 unchanged attributes hidden)

      - ingress {
          - action          = "allow" -> null
          - from_port       = 0 -> null
          - icmp_code       = 0 -> null
          - icmp_type       = 0 -> null
          - ipv6_cidr_block = "::/0" -> null
          - protocol        = "-1" -> null
          - rule_no         = 1003 -> null
          - to_port         = 0 -> null
        }
      - ingress {
          - action     = "allow" -> null
          - cidr_block = "0.0.0.0/0" -> null
          - from_port  = 0 -> null
          - icmp_code  = 0 -> null
          - icmp_type  = 0 -> null
          - protocol   = "-1" -> null
          - rule_no    = 1002 -> null
          - to_port    = 0 -> null
        }
      + ingress {
          + action     = "allow"
          + cidr_block = "0.0.0.0/0"
          + from_port  = 0
          + protocol   = "-1"
          + rule_no    = 1002
          + to_port    = 0
        }
      - ingress {
          - action     = "allow" -> null
          - cidr_block = "10.0.0.0/8" -> null
          - from_port  = 22 -> null
          - icmp_code  = 0 -> null
          - icmp_type  = 0 -> null
          - protocol   = "6" -> null
          - rule_no    = 10 -> null
          - to_port    = 22 -> null
        }
      + ingress {
          + action     = "allow"
          + cidr_block = "10.0.0.0/8"
          + from_port  = 22
          + protocol   = "tcp"
          + rule_no    = 10
          + to_port    = 22
        }
      - ingress {
          - action     = "allow" -> null
          - cidr_block = "172.16.0.0/12" -> null
          - from_port  = 22 -> null
          - icmp_code  = 0 -> null
          - icmp_type  = 0 -> null
          - protocol   = "6" -> null
          - rule_no    = 11 -> null
          - to_port    = 22 -> null
        }
      + ingress {
          + action     = "allow"
          + cidr_block = "172.16.0.0/12"
          + from_port  = 22
          + protocol   = "tcp"
          + rule_no    = 11
          + to_port    = 22
        }
      + ingress {
          + action          = "allow"
          + from_port       = 0
          + ipv6_cidr_block = "::/0"
          + protocol        = "-1"
          + rule_no         = 1003
          + to_port         = 0
        }
      - ingress {
          - action          = "deny" -> null
          - from_port       = 22 -> null
          - icmp_code       = 0 -> null
          - icmp_type       = 0 -> null
          - ipv6_cidr_block = "::/0" -> null
          - protocol        = "6" -> null
          - rule_no         = 21 -> null
          - to_port         = 22 -> null
        }
      + ingress {
          + action          = "deny"
          + from_port       = 22
          + ipv6_cidr_block = "::0/0"
          + protocol        = "tcp"
          + rule_no         = 21
          + to_port         = 22
        }
      - ingress {
          - action     = "deny" -> null
          - cidr_block = "0.0.0.0/0" -> null
          - from_port  = 22 -> null
          - icmp_code  = 0 -> null
          - icmp_type  = 0 -> null
          - protocol   = "6" -> null
          - rule_no    = 20 -> null
          - to_port    = 22 -> null
        }
      + ingress {
          + action     = "deny"
          + cidr_block = "0.0.0.0/0"
          + from_port  = 22
          + protocol   = "tcp"
          + rule_no    = 20
          + to_port    = 22
        }

        # (2 unchanged blocks hidden)
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Addresses a defect in current functionality. service/ec2 Issues and PRs that pertain to the ec2 service.
Projects
None yet
Development

No branches or pull requests

7 participants