From eb1496354d663c22b177b8ac2b66561f16dbc179 Mon Sep 17 00:00:00 2001 From: AMeng Date: Thu, 13 Apr 2017 10:40:25 -0600 Subject: [PATCH] provider/aws: Fix security group rule import --- .../aws/import_aws_security_group.go | 69 ++++++++++----- .../aws/import_aws_security_group_test.go | 56 ++++++++++++ .../aws/resource_aws_security_group_test.go | 85 +++++++++++++++++++ 3 files changed, 191 insertions(+), 19 deletions(-) diff --git a/builtin/providers/aws/import_aws_security_group.go b/builtin/providers/aws/import_aws_security_group.go index d802c75e2326..d76529058849 100644 --- 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 index 4b0597670f9c..d91b1027a514 100644 --- 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 index f1fe67ca9aba..f5a4f8d16936 100644 --- 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"