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

aws_network_acl with icmp rule always recreates network acl #4423

Closed
slshen opened this issue Dec 22, 2015 · 6 comments
Closed

aws_network_acl with icmp rule always recreates network acl #4423

slshen opened this issue Dec 22, 2015 · 6 comments

Comments

@slshen
Copy link

slshen commented Dec 22, 2015

A rule like:

egress {
    rule_no = "100"
    protocol = "icmp"
    from_port = -1
    to_port = -1
    icmp_type = -1
    icmp_code = -1
    cidr_block = "0.0.0.0/0"
    action = "allow"
}

Creates an icmp any rule, but terraform always thinks it's different. plan says:

egress.2826807916.action:     "allow" => ""
egress.2826807916.cidr_block: "0.0.0.0/0" => ""
egress.2826807916.from_port:  "0" => "0"
egress.2826807916.icmp_code:  "-1" => "0"
egress.2826807916.icmp_type:  "-1" => "0"
egress.2826807916.protocol:   "1" => ""
egress.2826807916.rule_no:    "100" => "0"
egress.2826807916.to_port:    "0" => "0"

Changing the egress rule so that from_port = 0 and to_port = 0 eliminates the diff.

@felnne
Copy link

felnne commented Jan 15, 2016

Just been bitten by this too - glad i'm not going mad!

@nekinie
Copy link

nekinie commented Feb 9, 2016

Also seeing this issue

@fazy
Copy link

fazy commented Feb 21, 2016

Confirmed here too, terraform 0.6.11.

Affects VPC network ACLs (aws_network_acl) and security groups (aws_security_group).

@ericwestfall
Copy link
Contributor

Confirmed this impacts version 0.6.12; the issue does not occur if you describe the ICMP rule as a standalone aws_network_acl_rule resource rather than defining the rule inline with the ACL.

This is broken:

resource "aws_network_acl" "test" {
  vpc_id = "${aws_vpc.main.id}"
  tags {
    Name = "test-acl"
  }

  # Authorize all inbound traffic.
  ingress {
    protocol = "icmp"
    rule_no = 100
    action = "allow"
    cidr_block = "0.0.0.0/0"
    from_port = -1
    to_port = -1
    icmp_type = -1
    icmp_code = -1
  }

  # Authorize all outbound traffic.
  egress = {
    protocol = "icmp"
    rule_no = 100
    action = "allow"
    cidr_block = "0.0.0.0/0"
    from_port = -1
    to_port = -1
    icmp_type = -1
    icmp_code = -1
  }
}

This works just fine:

resource "aws_network_acl" "test" {
  vpc_id = "${aws_vpc.main.id}"
  tags {
    Name = "test-acl"
  }
}

resource "aws_network_acl_rule" "allow_ingress_icmp_test" {
    network_acl_id = "${aws_network_acl.test.id}"
    rule_number = 100
    egress = false
    protocol = "icmp"
    rule_action = "allow"
    cidr_block = "0.0.0.0/0"
    from_port = -1
    to_port = -1
    icmp_type = -1
    icmp_code = -1
}

resource "aws_network_acl_rule" "allow_egress_icmp_test" {
    network_acl_id = "${aws_network_acl.test.id}"
    rule_number = 100
    egress = true
    protocol = "icmp"
    rule_action = "allow"
    cidr_block = "0.0.0.0/0"
    from_port = -1
    to_port = -1
    icmp_type = -1
    icmp_code = -1
}

fazy pushed a commit to fazy/terraform that referenced this issue Mar 12, 2016
When ICMP rules are present and from_port or to_port are set to non-zero
values, terraform always sees the rules as changed and recreates them
(issue: hashicorp#4423).

This change forces the affected values to zero for ICMP rules when
creating a hash of the ACL values to detect if a change is needed.

Values are hashed in the same order as before, so users should not see
any change unless they were affected by this issue already; affected
users should see only one change, after which the new hash values will
be stored in the .tfstate file.

Documentation updated to clarify that a 0 value is needed (as it
is possible to guess "-1" after looking at the protocol "all" value).
fazy pushed a commit to fazy/terraform that referenced this issue Mar 12, 2016
When ICMP rules are present and from_port or to_port are set to non-zero
values, terraform always sees the rules as changed and recreates them
(issue: hashicorp#4423).

This change forces the affected values to zero for ICMP rules when
creating a hash of the ACL values to detect if a change is needed.

Values are hashed in the same order as before, so users should not see
any change unless they were affected by this issue already; affected
users should see only one change, after which the new hash values will
be stored in the .tfstate file.

Documentation updated to clarify that a 0 value is needed (as it
is possible to guess "-1" after looking at the protocol "all" value).
@fazy
Copy link

fazy commented Mar 12, 2016

I've had a go at fixing this in PR #5602 - it's a bit hacky, basically I changed the code that computes the hash of the rule properties to set from/to port to always be counted as zero for ICMP rules.

Any feedback/testing appreciated...

@ghost
Copy link

ghost commented Apr 11, 2020

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.

@hashicorp hashicorp locked and limited conversation to collaborators Apr 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants