Merge pull request #6407 from terraform-providers/b-aws_security_rule…

…_rule-all-update-description resource/aws_security_group_rule: Properly handle updating description when protocol is -1/ALL
hashicorp · Nov 9, 2018 · fb4e02b · fb4e02b
2 parents 212e91f + 6ffadc9
commit fb4e02b
Show file tree

Hide file tree

Showing 2 changed files with 85 additions and 2 deletions.
diff --git a/aws/resource_aws_security_group_rule.go b/aws/resource_aws_security_group_rule.go
@@ -577,11 +577,15 @@ func ipPermissionIDHash(sg_id, ruleType string, ip *ec2.IpPermission) string {
 func expandIPPerm(d *schema.ResourceData, sg *ec2.SecurityGroup) (*ec2.IpPermission, error) {
 	var perm ec2.IpPermission
 
-	perm.FromPort = aws.Int64(int64(d.Get("from_port").(int)))
-	perm.ToPort = aws.Int64(int64(d.Get("to_port").(int)))
 	protocol := protocolForValue(d.Get("protocol").(string))
 	perm.IpProtocol = aws.String(protocol)
 
+	// InvalidParameterValue: When protocol is ALL, you cannot specify from-port.
+	if protocol != "-1" {
+		perm.FromPort = aws.Int64(int64(d.Get("from_port").(int)))
+		perm.ToPort = aws.Int64(int64(d.Get("to_port").(int)))
+	}
+
 	// build a group map that behaves like a set
 	groups := make(map[string]bool)
 	if raw, ok := d.GetOk("source_security_group_id"); ok {

diff --git a/aws/resource_aws_security_group_rule_test.go b/aws/resource_aws_security_group_rule_test.go
@@ -786,6 +786,63 @@ func TestAccAWSSecurityGroupRule_EgressDescription_updates(t *testing.T) {
 	})
 }
 
+func TestAccAWSSecurityGroupRule_Description_AllPorts(t *testing.T) {
+	var group ec2.SecurityGroup
+	rName := acctest.RandomWithPrefix("tf-acc-test")
+	securityGroupResourceName := "aws_security_group.test"
+	resourceName := "aws_security_group_rule.test"
+
+	rule1 := ec2.IpPermission{
+		IpProtocol: aws.String("-1"),
+		IpRanges: []*ec2.IpRange{
+			{CidrIp: aws.String("0.0.0.0/0"), Description: aws.String("description1")},
+		},
+	}
+
+	rule2 := ec2.IpPermission{
+		IpProtocol: aws.String("-1"),
+		IpRanges: []*ec2.IpRange{
+			{CidrIp: aws.String("0.0.0.0/0"), Description: aws.String("description2")},
+		},
+	}
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckAWSSecurityGroupRuleDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccAWSSecurityGroupRuleConfigDescriptionAllPorts(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, "protocol", "-1"),
+					resource.TestCheckResourceAttr(resourceName, "to_port", "0"),
+				),
+			},
+			{
+				ResourceName:      resourceName,
+				ImportState:       true,
+				ImportStateIdFunc: testAccAWSSecurityGroupRuleImportStateIdFunc(resourceName),
+				ImportStateVerify: true,
+			},
+			{
+				Config: testAccAWSSecurityGroupRuleConfigDescriptionAllPorts(rName, "description2"),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckAWSSecurityGroupRuleExists(securityGroupResourceName, &group),
+					testAccCheckAWSSecurityGroupRuleAttributes(resourceName, &group, &rule2, "ingress"),
+					resource.TestCheckResourceAttr(resourceName, "description", "description2"),
+					resource.TestCheckResourceAttr(resourceName, "from_port", "0"),
+					resource.TestCheckResourceAttr(resourceName, "protocol", "-1"),
+					resource.TestCheckResourceAttr(resourceName, "to_port", "0"),
+				),
+			},
+		},
+	})
+}
+
 func TestAccAWSSecurityGroupRule_MultiDescription(t *testing.T) {
 	var group ec2.SecurityGroup
 	var nat ec2.SecurityGroup
@@ -1726,6 +1783,28 @@ resource "aws_security_group_rule" "egress_1" {
     `, rInt)
 }
 
+func testAccAWSSecurityGroupRuleConfigDescriptionAllPorts(rName, description string) string {
+	return fmt.Sprintf(`
+resource "aws_security_group" "test" {
+  name = %q
+
+  tags {
+    Name = "tf-acc-test-ec2-security-group-rule"
+  }
+}
+
+resource "aws_security_group_rule" "test" {
+  cidr_blocks       = ["0.0.0.0/0"]
+  description       = %q
+  from_port         = 0
+  protocol          = -1
+  security_group_id = "${aws_security_group.test.id}"
+  to_port           = 0
+  type              = "ingress"
+}
+`, rName, description)
+}
+
 var testAccAWSSecurityGroupRuleRace = func() string {
 	var b bytes.Buffer
 	iterations := 50