From 6dd0e3f6dba21148abfc0d51928f5e7897aee829 Mon Sep 17 00:00:00 2001 From: Krzysztof Wilczynski Date: Sun, 17 Jul 2016 03:24:48 +0900 Subject: [PATCH] Add validation for icmp_type and icmp_code. Also, change order of processing to parse icmp_type first. Change wording of the debug messages, and change format string type for rule_number where appropriate. Signed-off-by: Krzysztof Wilczynski --- .../aws/resource_aws_network_acl_rule.go | 49 ++++++++++++------- .../aws/resource_aws_network_acl_rule_test.go | 14 +++++- 2 files changed, 42 insertions(+), 21 deletions(-) diff --git a/builtin/providers/aws/resource_aws_network_acl_rule.go b/builtin/providers/aws/resource_aws_network_acl_rule.go index be347daf3..ad3f302a3 100644 --- a/builtin/providers/aws/resource_aws_network_acl_rule.go +++ b/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.TypeString, - Optional: true, - ForceNew: true, + Type: schema.TypeString, + Optional: true, + ForceNew: true, + ValidateFunc: validateICMPArgumentValue, }, "icmp_code": &schema.Schema{ - Type: schema.TypeString, - Optional: true, - ForceNew: true, + Type: schema.TypeString, + Optional: 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) @@ -107,21 +109,21 @@ func resourceAwsNetworkAclRuleCreate(d *schema.ResourceData, meta interface{}) e // 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 { - icmpCode, err := strconv.Atoi(v.(string)) - if err != nil { - return fmt.Errorf("Unable to parse ICMP code %s for rule %#v", v, d.Get("rule_number").(int)) - } - params.IcmpTypeCode.Code = aws.Int64(int64(icmpCode)) - log.Printf("[DEBUG] Transformed ICMP code %s into %d", v, icmpCode) - } if v, ok := d.GetOk("icmp_type"); ok { icmpType, err := strconv.Atoi(v.(string)) if err != nil { - return fmt.Errorf("Unable to parse ICMP type %s for rule %#v", v, d.Get("rule_number").(int)) + 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] Transformed ICMP type %s into %d", v, 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)) } } @@ -176,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) @@ -209,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{ @@ -256,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 +} diff --git a/builtin/providers/aws/resource_aws_network_acl_rule_test.go b/builtin/providers/aws/resource_aws_network_acl_rule_test.go index 95682bf41..b7559d7c9 100644 --- a/builtin/providers/aws/resource_aws_network_acl_rule_test.go +++ b/builtin/providers/aws/resource_aws_network_acl_rule_test.go @@ -24,7 +24,8 @@ func TestAccAWSNetworkAclRule_basic(t *testing.T) { Config: testAccAWSNetworkAclRuleBasicConfig, Check: resource.ComposeTestCheckFunc( testAccCheckAWSNetworkAclRuleExists("aws_network_acl_rule.baz", &networkAcl), - testAccCheckAWSNetworkAclRuleExists("aws_network_acl_rule.quux", &networkAcl), + testAccCheckAWSNetworkAclRuleExists("aws_network_acl_rule.qux", &networkAcl), + testAccCheckAWSNetworkAclRuleExists("aws_network_acl_rule.wibble", &networkAcl), ), }, }, @@ -123,7 +124,7 @@ resource "aws_network_acl_rule" "baz" { from_port = 22 to_port = 22 } -resource "aws_network_acl_rule" "quux" { +resource "aws_network_acl_rule" "qux" { network_acl_id = "${aws_network_acl.bar.id}" rule_number = 300 protocol = "icmp" @@ -132,4 +133,13 @@ resource "aws_network_acl_rule" "quux" { 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 +} `