provider/aws: Fix security group rule import (#13630)

hashicorp · Apr 21, 2017 · 7cc4018 · 7cc4018
1 parent 358e1c2
commit 7cc4018
Show file tree

Hide file tree

Showing 3 changed files with 191 additions and 19 deletions.
diff --git a/builtin/providers/aws/import_aws_security_group.go b/builtin/providers/aws/import_aws_security_group.go
@@ -50,34 +50,65 @@ func resourceAwsSecurityGroupImportState(
 }
 
 func resourceAwsSecurityGroupImportStatePerm(sg *ec2.SecurityGroup, ruleType string, perm *ec2.IpPermission) ([]*schema.ResourceData, error) {
+	/*
+	   Create a seperate Security Group Rule for:
+	   * The collection of IpRanges (cidr_blocks)
+	   * The collection of Ipv6Ranges (ipv6_cidr_blocks)
+	   * Each individual UserIdGroupPair (source_security_group_id)
+
+	   If, for example, a security group has rules for:
+	   * 2 IpRanges
+	   * 2 Ipv6Ranges
+	   * 2 UserIdGroupPairs
+
+	   This would generate 4 security group rules:
+	   * 1 for the collection of IpRanges
+	   * 1 for the collection of Ipv6Ranges
+	   * 1 for the first UserIdGroupPair
+	   * 1 for the second UserIdGroupPair
+	*/
 	var result []*schema.ResourceData
 
-	if len(perm.UserIdGroupPairs) == 0 {
-		r, err := resourceAwsSecurityGroupImportStatePermPair(sg, ruleType, perm)
+	if perm.IpRanges != nil {
+		p := &ec2.IpPermission{
+			FromPort:      perm.FromPort,
+			IpProtocol:    perm.IpProtocol,
+			PrefixListIds: perm.PrefixListIds,
+			ToPort:        perm.ToPort,
+			IpRanges:      perm.IpRanges,
+		}
+
+		r, err := resourceAwsSecurityGroupImportStatePermPair(sg, ruleType, p)
 		if err != nil {
 			return nil, err
 		}
 		result = append(result, r)
-	} else {
-		// If the rule contained more than one source security group, this
-		// will iterate over them and create one rule for each
-		// source security group.
-		for _, pair := range perm.UserIdGroupPairs {
-			p := &ec2.IpPermission{
-				FromPort:      perm.FromPort,
-				IpProtocol:    perm.IpProtocol,
-				PrefixListIds: perm.PrefixListIds,
-				ToPort:        perm.ToPort,
+	}
 
-				UserIdGroupPairs: []*ec2.UserIdGroupPair{pair},
-			}
+	if perm.Ipv6Ranges != nil {
+		p := &ec2.IpPermission{
+			FromPort:      perm.FromPort,
+			IpProtocol:    perm.IpProtocol,
+			PrefixListIds: perm.PrefixListIds,
+			ToPort:        perm.ToPort,
+			Ipv6Ranges:    perm.Ipv6Ranges,
+		}
 
-			if perm.Ipv6Ranges != nil {
-				p.Ipv6Ranges = perm.Ipv6Ranges
-			}
+		r, err := resourceAwsSecurityGroupImportStatePermPair(sg, ruleType, p)
+		if err != nil {
+			return nil, err
+		}
+		result = append(result, r)
+	}
 
-			if perm.IpRanges != nil {
-				p.IpRanges = perm.IpRanges
+	if len(perm.UserIdGroupPairs) > 0 {
+		for _, pair := range perm.UserIdGroupPairs {
+			p := &ec2.IpPermission{
+				FromPort:         perm.FromPort,
+				IpProtocol:       perm.IpProtocol,
+				PrefixListIds:    perm.PrefixListIds,
+				ToPort:           perm.ToPort,
+				UserIdGroupPairs: []*ec2.UserIdGroupPair{pair},
 			}
 
 			r, err := resourceAwsSecurityGroupImportStatePermPair(sg, ruleType, p)

diff --git a/builtin/providers/aws/import_aws_security_group_test.go b/builtin/providers/aws/import_aws_security_group_test.go
@@ -101,3 +101,59 @@ func TestAccAWSSecurityGroup_importSourceSecurityGroup(t *testing.T) {
 		},
 	})
 }
+
+func TestAccAWSSecurityGroup_importIPRangeAndSecurityGroupWithSameRules(t *testing.T) {
+	checkFn := func(s []*terraform.InstanceState) error {
+		// Expect 4: group, 3 rules
+		if len(s) != 4 {
+			return fmt.Errorf("expected 4 states: %#v", s)
+		}
+
+		return nil
+	}
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckAWSSecurityGroupDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccAWSSecurityGroupConfig_importIPRangeAndSecurityGroupWithSameRules,
+			},
+
+			{
+				ResourceName:     "aws_security_group.test_group_1",
+				ImportState:      true,
+				ImportStateCheck: checkFn,
+			},
+		},
+	})
+}
+
+func TestAccAWSSecurityGroup_importIPRangesWithSameRules(t *testing.T) {
+	checkFn := func(s []*terraform.InstanceState) error {
+		// Expect 4: group, 2 rules
+		if len(s) != 3 {
+			return fmt.Errorf("expected 3 states: %#v", s)
+		}
+
+		return nil
+	}
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckAWSSecurityGroupDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccAWSSecurityGroupConfig_importIPRangesWithSameRules,
+			},
+
+			{
+				ResourceName:     "aws_security_group.test_group_1",
+				ImportState:      true,
+				ImportStateCheck: checkFn,
+			},
+		},
+	})
+}
diff --git a/builtin/providers/aws/resource_aws_security_group_test.go b/builtin/providers/aws/resource_aws_security_group_test.go
@@ -1995,6 +1995,91 @@ resource "aws_security_group_rule" "allow_test_group_3" {
 }
 `
 
+const testAccAWSSecurityGroupConfig_importIPRangeAndSecurityGroupWithSameRules = `
+resource "aws_vpc" "foo" {
+  cidr_block = "10.1.0.0/16"
+
+  tags {
+    Name = "tf_sg_import_test"
+  }
+}
+
+resource "aws_security_group" "test_group_1" {
+  name        = "test group 1"
+  vpc_id      = "${aws_vpc.foo.id}"
+}
+
+resource "aws_security_group" "test_group_2" {
+  name        = "test group 2"
+  vpc_id      = "${aws_vpc.foo.id}"
+}
+
+resource "aws_security_group_rule" "allow_security_group" {
+  type      = "ingress"
+  from_port = 0
+  to_port   = 0
+  protocol  = "tcp"
+
+  source_security_group_id = "${aws_security_group.test_group_2.id}"
+  security_group_id = "${aws_security_group.test_group_1.id}"
+}
+
+resource "aws_security_group_rule" "allow_cidr_block" {
+  type      = "ingress"
+  from_port = 0
+  to_port   = 0
+  protocol  = "tcp"
+
+  cidr_blocks = ["10.0.0.0/32"]
+  security_group_id = "${aws_security_group.test_group_1.id}"
+}
+
+resource "aws_security_group_rule" "allow_ipv6_cidr_block" {
+  type      = "ingress"
+  from_port = 0
+  to_port   = 0
+  protocol  = "tcp"
+
+  ipv6_cidr_blocks = ["::/0"]
+  security_group_id = "${aws_security_group.test_group_1.id}"
+}
+`
+
+const testAccAWSSecurityGroupConfig_importIPRangesWithSameRules = `
+resource "aws_vpc" "foo" {
+  cidr_block = "10.1.0.0/16"
+
+  tags {
+    Name = "tf_sg_import_test"
+  }
+}
+
+resource "aws_security_group" "test_group_1" {
+  name        = "test group 1"
+  vpc_id      = "${aws_vpc.foo.id}"
+}
+
+resource "aws_security_group_rule" "allow_cidr_block" {
+  type      = "ingress"
+  from_port = 0
+  to_port   = 0
+  protocol  = "tcp"
+
+  cidr_blocks = ["10.0.0.0/32"]
+  security_group_id = "${aws_security_group.test_group_1.id}"
+}
+
+resource "aws_security_group_rule" "allow_ipv6_cidr_block" {
+  type      = "ingress"
+  from_port = 0
+  to_port   = 0
+  protocol  = "tcp"
+
+  ipv6_cidr_blocks = ["::/0"]
+  security_group_id = "${aws_security_group.test_group_1.id}"
+}
+`
+
 const testAccAWSSecurityGroupConfigPrefixListEgress = `
 resource "aws_vpc" "tf_sg_prefix_list_egress_test" {
     cidr_block = "10.0.0.0/16"