From 3f8fa8ddf455e86fe82406929188f1339a07a736 Mon Sep 17 00:00:00 2001 From: Jake Champlin Date: Wed, 15 Mar 2017 15:51:20 -0400 Subject: [PATCH 1/2] provider/aws: Correctly check if setting CidrBlock or IPv6CidrBlock in NetworkAcl Previously the check for if we are setting `CidrBlock` or `IPv6CidrBlock` during an `Update` of the `aws_network_acl` resource would populate the input struct with a nil string value `""`. This caused our acceptance tests to fail, and broke the resource's functionality if a user only set `CidrBlock` or `IPv6CidrBlock` for either an `ingress` or `egress` rule as the API would error out with an `Invalid CidrBlock` error. Previously: ``` aws_network_acl.bond: Error creating egress entry: InvalidParameterValue: CIDR block is malformed status code: 400, request id: 0620e0b7-4e30-4c14-9a7a-5d373cc9f33b ``` Currently: ``` $ make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSNetworkAcl' ==> Checking that code complies with gofmt requirements... go generate $(go list ./... | grep -v /terraform/vendor/) 2017/03/15 15:41:17 Generated command/internal_plugin_list.go TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSNetworkAcl -timeout 120m === RUN TestAccAWSNetworkAcl_importBasic --- PASS: TestAccAWSNetworkAcl_importBasic (26.96s) === RUN TestAccAWSNetworkAclRule_basic --- PASS: TestAccAWSNetworkAclRule_basic (23.08s) === RUN TestAccAWSNetworkAclRule_ipv6 --- PASS: TestAccAWSNetworkAclRule_ipv6 (26.24s) === RUN TestAccAWSNetworkAcl_EgressAndIngressRules --- PASS: TestAccAWSNetworkAcl_EgressAndIngressRules (25.11s) === RUN TestAccAWSNetworkAcl_OnlyIngressRules_basic --- PASS: TestAccAWSNetworkAcl_OnlyIngressRules_basic (31.82s) === RUN TestAccAWSNetworkAcl_OnlyIngressRules_update --- PASS: TestAccAWSNetworkAcl_OnlyIngressRules_update (48.59s) === RUN TestAccAWSNetworkAcl_OnlyEgressRules --- PASS: TestAccAWSNetworkAcl_OnlyEgressRules (25.48s) === RUN TestAccAWSNetworkAcl_SubnetChange --- PASS: TestAccAWSNetworkAcl_SubnetChange (57.12s) === RUN TestAccAWSNetworkAcl_Subnets --- PASS: TestAccAWSNetworkAcl_Subnets (67.55s) === RUN TestAccAWSNetworkAcl_ipv6Rules --- PASS: TestAccAWSNetworkAcl_ipv6Rules (31.52s) === RUN TestAccAWSNetworkAcl_espProtocol acc--- PASS: TestAccAWSNetworkAcl_espProtocol (24.37s) PASS ok github.com/hashicorp/terraform/builtin/providers/aws 387.855s ``` --- builtin/providers/aws/import_aws_network_acl_test.go | 4 ++-- builtin/providers/aws/resource_aws_network_acl.go | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/builtin/providers/aws/import_aws_network_acl_test.go b/builtin/providers/aws/import_aws_network_acl_test.go index 407d3e45e..6adf8a47d 100644 --- a/builtin/providers/aws/import_aws_network_acl_test.go +++ b/builtin/providers/aws/import_aws_network_acl_test.go @@ -23,11 +23,11 @@ func TestAccAWSNetworkAcl_importBasic(t *testing.T) { Providers: testAccProviders, CheckDestroy: testAccCheckAWSNetworkAclDestroy, Steps: []resource.TestStep{ - resource.TestStep{ + { Config: testAccAWSNetworkAclEgressNIngressConfig, }, - resource.TestStep{ + { ResourceName: "aws_network_acl.bar", ImportState: true, ImportStateVerify: true, diff --git a/builtin/providers/aws/resource_aws_network_acl.go b/builtin/providers/aws/resource_aws_network_acl.go index cd3c6d049..87e49db85 100644 --- a/builtin/providers/aws/resource_aws_network_acl.go +++ b/builtin/providers/aws/resource_aws_network_acl.go @@ -397,7 +397,7 @@ func updateNetworkAclEntries(d *schema.ResourceData, entryType string, conn *ec2 } } - if add.CidrBlock != nil { + if *add.CidrBlock != "" { // AWS mutates the CIDR block into a network implied by the IP and // mask provided. This results in hashing inconsistencies between // the local config file and the state returned by the API. Error @@ -417,11 +417,11 @@ func updateNetworkAclEntries(d *schema.ResourceData, entryType string, conn *ec2 IcmpTypeCode: add.IcmpTypeCode, } - if add.CidrBlock != nil { + if *add.CidrBlock != "" { createOpts.CidrBlock = add.CidrBlock } - if add.Ipv6CidrBlock != nil { + if *add.Ipv6CidrBlock != "" { createOpts.Ipv6CidrBlock = add.Ipv6CidrBlock } From f93848670ea0b04686e2097c0ddc189dacbcb0a7 Mon Sep 17 00:00:00 2001 From: Jake Champlin Date: Wed, 15 Mar 2017 18:21:40 -0400 Subject: [PATCH 2/2] protect against panics in nil checks --- builtin/providers/aws/resource_aws_network_acl.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/builtin/providers/aws/resource_aws_network_acl.go b/builtin/providers/aws/resource_aws_network_acl.go index 87e49db85..7a525e299 100644 --- a/builtin/providers/aws/resource_aws_network_acl.go +++ b/builtin/providers/aws/resource_aws_network_acl.go @@ -397,7 +397,7 @@ func updateNetworkAclEntries(d *schema.ResourceData, entryType string, conn *ec2 } } - if *add.CidrBlock != "" { + if add.CidrBlock != nil && *add.CidrBlock != "" { // AWS mutates the CIDR block into a network implied by the IP and // mask provided. This results in hashing inconsistencies between // the local config file and the state returned by the API. Error @@ -417,11 +417,11 @@ func updateNetworkAclEntries(d *schema.ResourceData, entryType string, conn *ec2 IcmpTypeCode: add.IcmpTypeCode, } - if *add.CidrBlock != "" { + if add.CidrBlock != nil && *add.CidrBlock != "" { createOpts.CidrBlock = add.CidrBlock } - if *add.Ipv6CidrBlock != "" { + if add.Ipv6CidrBlock != nil && *add.Ipv6CidrBlock != "" { createOpts.Ipv6CidrBlock = add.Ipv6CidrBlock }