Skip to content

Commit

Permalink
resource/aws_security_group_rule: Support all non-zero from_port an…
Browse files Browse the repository at this point in the history
…d `to_port` configurations with `protocol` ALL/-1

This completely suppresses `from_port` and `to_port` differences when `protocol` is `ALL` or `-1`. The API ignores them in this scenario, so previously it was possible to specify whatever non-zero values you wished (-1 for both, 0 and 65535, etc), but previously the logic was not showing the difference.

Previously:

```
--- FAIL: TestAccAWSSecurityGroupRule_Description_AllPorts_NonZeroPorts (16.96s)
    testing.go:538: Step 0 error: After applying this step and refreshing, the plan was not empty:

        DIFF:

        DESTROY/CREATE: aws_security_group_rule.test
          cidr_blocks.#:            "1" => "1"
          cidr_blocks.0:            "0.0.0.0/0" => "0.0.0.0/0"
          description:              "description1" => "description1"
          from_port:                "0" => "-1" (forces new resource)
          protocol:                 "-1" => "-1"
          security_group_id:        "sg-04722579708a472f8" => "sg-04722579708a472f8"
          self:                     "false" => "false"
          source_security_group_id: "" => "<computed>"
          to_port:                  "0" => "-1" (forces new resource)
          type:                     "ingress" => "ingress"
```

Output from acceptance testing:

```
--- PASS: TestAccAWSSecurityGroupRule_ExpectInvalidTypeError (1.70s)
--- PASS: TestAccAWSSecurityGroupRule_ExpectInvalidCIDR (1.84s)
--- PASS: TestAccAWSSecurityGroupRule_MultiIngress (23.08s)
--- PASS: TestAccAWSSecurityGroupRule_EgressDescription (23.84s)
--- PASS: TestAccAWSSecurityGroupRule_Issue5310 (24.41s)
--- PASS: TestAccAWSSecurityGroupRule_Ingress_Classic (24.04s)
--- PASS: TestAccAWSSecurityGroupRule_MultipleRuleSearching_AllProtocolCrash (25.85s)
--- PASS: TestAccAWSSecurityGroupRule_Ingress_VPC (27.91s)
--- PASS: TestAccAWSSecurityGroupRule_IngressDescription (28.45s)
--- PASS: TestAccAWSSecurityGroupRule_Description_AllPorts_NonZeroPorts (30.69s)
--- PASS: TestAccAWSSecurityGroupRule_Egress (36.77s)
--- PASS: TestAccAWSSecurityGroupRule_IngressDescription_updates (37.11s)
--- PASS: TestAccAWSSecurityGroupRule_SelfSource (42.98s)
--- PASS: TestAccAWSSecurityGroupRule_EgressDescription_updates (43.95s)
--- PASS: TestAccAWSSecurityGroupRule_SelfReference (44.43s)
--- PASS: TestAccAWSSecurityGroupRule_Ingress_Ipv6 (45.83s)
--- PASS: TestAccAWSSecurityGroupRule_PartialMatching_Source (50.10s)
--- PASS: TestAccAWSSecurityGroupRule_PartialMatching_basic (51.93s)
--- PASS: TestAccAWSSecurityGroupRule_Description_AllPorts (52.00s)
--- PASS: TestAccAWSSecurityGroupRule_PrefixListEgress (57.10s)
--- PASS: TestAccAWSSecurityGroupRule_Ingress_Protocol (37.05s)
--- PASS: TestAccAWSSecurityGroupRule_MultiDescription (89.38s)
--- PASS: TestAccAWSSecurityGroupRule_Race (275.19s)
```
  • Loading branch information
bflad committed Nov 10, 2018
1 parent bb30267 commit a1a56a9
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 10 deletions.
11 changes: 10 additions & 1 deletion aws/resource_aws_security_group_rule.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,15 +56,24 @@ func resourceAwsSecurityGroupRule() *schema.Resource {
Type: schema.TypeInt,
Required: true,
ForceNew: true,
// Support existing configurations that have non-zero from_port and to_port defined with all protocols
DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool {
protocol := protocolForValue(d.Get("protocol").(string))
if protocol == "-1" && old == "0" {
return true
}
return false
},
},

"to_port": {
Type: schema.TypeInt,
Required: true,
ForceNew: true,
// Support existing configurations that have non-zero from_port and to_port defined with all protocols
DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool {
protocol := protocolForValue(d.Get("protocol").(string))
if protocol == "-1" && old == "0" && new == "65535" {
if protocol == "-1" && old == "0" {
return true
}
return false
Expand Down
17 changes: 8 additions & 9 deletions aws/resource_aws_security_group_rule_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -843,8 +843,7 @@ func TestAccAWSSecurityGroupRule_Description_AllPorts(t *testing.T) {
})
}

// Reference: https://github.com/terraform-providers/terraform-provider-aws/issues/6416
func TestAccAWSSecurityGroupRule_Description_AllPorts_ToPort65535(t *testing.T) {
func TestAccAWSSecurityGroupRule_Description_AllPorts_NonZeroPorts(t *testing.T) {
var group ec2.SecurityGroup
rName := acctest.RandomWithPrefix("tf-acc-test")
securityGroupResourceName := "aws_security_group.test"
Expand All @@ -870,14 +869,14 @@ func TestAccAWSSecurityGroupRule_Description_AllPorts_ToPort65535(t *testing.T)
CheckDestroy: testAccCheckAWSSecurityGroupRuleDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSSecurityGroupRuleConfigDescriptionAllPortsToPort65535(rName, "description1"),
Config: testAccAWSSecurityGroupRuleConfigDescriptionAllPortsNonZeroPorts(rName, "description1"),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSSecurityGroupRuleExists(securityGroupResourceName, &group),
testAccCheckAWSSecurityGroupRuleAttributes(resourceName, &group, &rule1, "ingress"),
resource.TestCheckResourceAttr(resourceName, "description", "description1"),
resource.TestCheckResourceAttr(resourceName, "from_port", "0"),
resource.TestCheckResourceAttr(resourceName, "from_port", "-1"),
resource.TestCheckResourceAttr(resourceName, "protocol", "-1"),
resource.TestCheckResourceAttr(resourceName, "to_port", "65535"),
resource.TestCheckResourceAttr(resourceName, "to_port", "-1"),
),
},
{
Expand All @@ -887,7 +886,7 @@ func TestAccAWSSecurityGroupRule_Description_AllPorts_ToPort65535(t *testing.T)
ImportStateVerify: true,
},
{
Config: testAccAWSSecurityGroupRuleConfigDescriptionAllPorts(rName, "description2"),
Config: testAccAWSSecurityGroupRuleConfigDescriptionAllPortsNonZeroPorts(rName, "description2"),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSSecurityGroupRuleExists(securityGroupResourceName, &group),
testAccCheckAWSSecurityGroupRuleAttributes(resourceName, &group, &rule2, "ingress"),
Expand Down Expand Up @@ -1922,7 +1921,7 @@ resource "aws_security_group_rule" "test" {
`, rName, description)
}

func testAccAWSSecurityGroupRuleConfigDescriptionAllPortsToPort65535(rName, description string) string {
func testAccAWSSecurityGroupRuleConfigDescriptionAllPortsNonZeroPorts(rName, description string) string {
return fmt.Sprintf(`
resource "aws_security_group" "test" {
name = %q
Expand All @@ -1935,10 +1934,10 @@ resource "aws_security_group" "test" {
resource "aws_security_group_rule" "test" {
cidr_blocks = ["0.0.0.0/0"]
description = %q
from_port = 0
from_port = -1
protocol = -1
security_group_id = "${aws_security_group.test.id}"
to_port = 65535
to_port = -1
type = "ingress"
}
`, rName, description)
Expand Down

0 comments on commit a1a56a9

Please sign in to comment.