Merge pull request #7669 from kwilczynski/fix/aws-network-acl-rule

Fix icmp_type and icmp_code in aws_network_acl_rule.

builtin/providers/aws/resource_aws_network_acl_rule.go

@@ -63,14 +63,16 @@ func resourceAwsNetworkAclRule() *schema.Resource {
 				ForceNew: true,
 			},
 			"icmp_type": &schema.Schema{
-				Type:     schema.TypeInt,
+				Type:         schema.TypeString,
-				Optional: true,
+				Optional:     true,
-				ForceNew: true,
+				ForceNew:     true,
+				ValidateFunc: validateICMPArgumentValue,
 			},
 			"icmp_code": &schema.Schema{
-				Type:     schema.TypeInt,
+				Type:         schema.TypeString,
-				Optional: true,
+				Optional:     true,
-				ForceNew: true,
+				ForceNew:     true,
+				ValidateFunc: validateICMPArgumentValue,
 			},
 		},
 	}
@@ -85,7 +87,7 @@ func resourceAwsNetworkAclRuleCreate(d *schema.ResourceData, meta interface{}) e
 		var ok bool
 		p, ok = protocolIntegers()[protocol]
 		if !ok {
-			return fmt.Errorf("Invalid Protocol %s for rule %#v", protocol, d.Get("rule_number").(int))
+			return fmt.Errorf("Invalid Protocol %s for rule %d", protocol, d.Get("rule_number").(int))
 		}
 	}
 	log.Printf("[INFO] Transformed Protocol %s into %d", protocol, p)
@@ -103,14 +105,25 @@ func resourceAwsNetworkAclRuleCreate(d *schema.ResourceData, meta interface{}) e
 		},
 	}
 
-	// Specify additional required fields for ICMP
+	// Specify additional required fields for ICMP. For the list
+	// of ICMP codes and types, see: http://www.nthelp.com/icmp.html
 	if p == 1 {
 		params.IcmpTypeCode = &ec2.IcmpTypeCode{}
-		if v, ok := d.GetOk("icmp_code"); ok {
-			params.IcmpTypeCode.Code = aws.Int64(int64(v.(int)))
-		}
 		if v, ok := d.GetOk("icmp_type"); ok {
-			params.IcmpTypeCode.Type = aws.Int64(int64(v.(int)))
+			icmpType, err := strconv.Atoi(v.(string))
+			if err != nil {
+				return fmt.Errorf("Unable to parse ICMP type %s for rule %d", v, d.Get("rule_number").(int))
+			}
+			params.IcmpTypeCode.Type = aws.Int64(int64(icmpType))
+			log.Printf("[DEBUG] Got ICMP type %d for rule %d", icmpType, d.Get("rule_number").(int))
+		}
+		if v, ok := d.GetOk("icmp_code"); ok {
+			icmpCode, err := strconv.Atoi(v.(string))
+			if err != nil {
+				return fmt.Errorf("Unable to parse ICMP code %s for rule %d", v, d.Get("rule_number").(int))
+			}
+			params.IcmpTypeCode.Code = aws.Int64(int64(icmpCode))
+			log.Printf("[DEBUG] Got ICMP code %d for rule %d", icmpCode, d.Get("rule_number").(int))
 		}
 	}
 
@@ -165,7 +178,7 @@ func resourceAwsNetworkAclRuleRead(d *schema.ResourceData, meta interface{}) err
 		var ok bool
 		protocol, ok := protocolStrings(protocolIntegers())[p]
 		if !ok {
-			return fmt.Errorf("Invalid Protocol %s for rule %#v", *resp.Protocol, d.Get("rule_number").(int))
+			return fmt.Errorf("Invalid Protocol %s for rule %d", *resp.Protocol, d.Get("rule_number").(int))
 		}
 		log.Printf("[INFO] Transformed Protocol %s back into %s", *resp.Protocol, protocol)
 		d.Set("protocol", protocol)
@@ -198,7 +211,7 @@ func findNetworkAclRule(d *schema.ResourceData, meta interface{}) (*ec2.NetworkA
 	filters := make([]*ec2.Filter, 0, 2)
 	ruleNumberFilter := &ec2.Filter{
 		Name:   aws.String("entry.rule-number"),
-		Values: []*string{aws.String(fmt.Sprintf("%v", d.Get("rule_number").(int)))},
+		Values: []*string{aws.String(fmt.Sprintf("%d", d.Get("rule_number").(int)))},
 	}
 	filters = append(filters, ruleNumberFilter)
 	egressFilter := &ec2.Filter{
@@ -245,3 +258,12 @@ func networkAclIdRuleNumberEgressHash(networkAclId string, ruleNumber int, egres
 	buf.WriteString(fmt.Sprintf("%s-", protocol))
 	return fmt.Sprintf("nacl-%d", hashcode.String(buf.String()))
 }
+
+func validateICMPArgumentValue(v interface{}, k string) (ws []string, errors []error) {
+	value := v.(string)
+	_, err := strconv.Atoi(value)
+	if len(value) == 0 || err != nil {
+		errors = append(errors, fmt.Errorf("%q must be an integer value: %q", k, value))
+	}
+	return
+}

builtin/providers/aws/resource_aws_network_acl_rule_test.go

@@ -23,7 +23,9 @@ func TestAccAWSNetworkAclRule_basic(t *testing.T) {
 			resource.TestStep{
 				Config: testAccAWSNetworkAclRuleBasicConfig,
 				Check: resource.ComposeTestCheckFunc(
-					testAccCheckAWSNetworkAclRuleExists("aws_network_acl_rule.bar", &networkAcl),
+					testAccCheckAWSNetworkAclRuleExists("aws_network_acl_rule.baz", &networkAcl),
+					testAccCheckAWSNetworkAclRuleExists("aws_network_acl_rule.qux", &networkAcl),
+					testAccCheckAWSNetworkAclRuleExists("aws_network_acl_rule.wibble", &networkAcl),
 				),
 			},
 		},
@@ -112,7 +114,7 @@ resource "aws_vpc" "foo" {
 resource "aws_network_acl" "bar" {
 	vpc_id = "${aws_vpc.foo.id}"
 }
-resource "aws_network_acl_rule" "bar" {
+resource "aws_network_acl_rule" "baz" {
 	network_acl_id = "${aws_network_acl.bar.id}"
 	rule_number = 200
 	egress = false
@@ -122,4 +124,22 @@ resource "aws_network_acl_rule" "bar" {
 	from_port = 22
 	to_port = 22
 }
+resource "aws_network_acl_rule" "qux" {
+	network_acl_id = "${aws_network_acl.bar.id}"
+	rule_number = 300
+	protocol = "icmp"
+	rule_action = "allow"
+	cidr_block = "0.0.0.0/0"
+	icmp_type = 0
+	icmp_code = -1
+}
+resource "aws_network_acl_rule" "wibble" {
+	network_acl_id = "${aws_network_acl.bar.id}"
+	rule_number = 400
+	protocol = "icmp"
+	rule_action = "allow"
+	cidr_block = "0.0.0.0/0"
+	icmp_type = -1
+	icmp_code = -1
+}
 `

website/source/docs/providers/aws/r/network_acl_rule.html.markdown

@@ -45,6 +45,8 @@ The following arguments are supported:
 
 ~> **NOTE:** If the value of `protocol` is `-1` or `all`, the `from_port` and `to_port` values will be ignored and the rule will apply to all ports.
 
+~> **NOTE:** If the value of `icmp_type` is `-1` (which results in a wildcard ICMP type), the `icmp_code` must also be set to `-1` (wildcard ICMP code).
+
 ~> Note: For more information on ICMP types and codes, see here: http://www.nthelp.com/icmp.html
 
 ## Attributes Reference
@@ -52,4 +54,3 @@ The following arguments are supported:
 The following attributes are exported:
 
 * `id` - The ID of the network ACL Rule
-