diff --git a/builtin/providers/aws/resource_aws_security_group_rule.go b/builtin/providers/aws/resource_aws_security_group_rule.go index d1759dcaf..005163fba 100644 --- a/builtin/providers/aws/resource_aws_security_group_rule.go +++ b/builtin/providers/aws/resource_aws_security_group_rule.go @@ -149,7 +149,9 @@ information and instructions for recovery. Error message: %s`, awsErr.Message()) ruleType, autherr) } - d.SetId(ipPermissionIDHash(sg_id, ruleType, perm)) + id := ipPermissionIDHash(sg_id, ruleType, perm) + d.SetId(id) + log.Printf("[DEBUG] Security group rule ID set to %s", id) return resourceAwsSecurityGroupRuleRead(d, meta) } @@ -164,6 +166,8 @@ func resourceAwsSecurityGroupRuleRead(d *schema.ResourceData, meta interface{}) return nil } + isVPC := sg.VpcId != nil && *sg.VpcId != "" + var rule *ec2.IpPermission var rules []*ec2.IpPermission ruleType := d.Get("type").(string) @@ -215,8 +219,14 @@ func resourceAwsSecurityGroupRuleRead(d *schema.ResourceData, meta interface{}) remaining = len(p.UserIdGroupPairs) for _, ip := range p.UserIdGroupPairs { for _, rip := range r.UserIdGroupPairs { - if *ip.GroupId == *rip.GroupId { - remaining-- + if isVPC { + if *ip.GroupId == *rip.GroupId { + remaining-- + } + } else { + if *ip.GroupName == *rip.GroupName { + remaining-- + } } } } @@ -250,7 +260,11 @@ func resourceAwsSecurityGroupRuleRead(d *schema.ResourceData, meta interface{}) if len(p.UserIdGroupPairs) > 0 { s := p.UserIdGroupPairs[0] - d.Set("source_security_group_id", *s.GroupId) + if isVPC { + d.Set("source_security_group_id", *s.GroupId) + } else { + d.Set("source_security_group_id", *s.GroupName) + } } return nil @@ -406,6 +420,7 @@ func expandIPPerm(d *schema.ResourceData, sg *ec2.SecurityGroup) (*ec2.IpPermiss } if v, ok := d.GetOk("self"); ok && v.(bool) { + // if sg.GroupId != nil { if sg.VpcId != nil && *sg.VpcId != "" { groups[*sg.GroupId] = true } else { diff --git a/builtin/providers/aws/resource_aws_security_group_rule_test.go b/builtin/providers/aws/resource_aws_security_group_rule_test.go index 7f6fa6d6f..9f459e4ca 100644 --- a/builtin/providers/aws/resource_aws_security_group_rule_test.go +++ b/builtin/providers/aws/resource_aws_security_group_rule_test.go @@ -343,6 +343,24 @@ func TestAccAWSSecurityGroupRule_PartialMatching_Source(t *testing.T) { }) } +func TestAccAWSSecurityGroupRule_Issue5310(t *testing.T) { + var group ec2.SecurityGroup + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSSecurityGroupRuleDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: testAccAWSSecurityGroupRuleIssue5310, + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSSecurityGroupRuleExists("aws_security_group.issue_5310", &group), + ), + }, + }, + }) +} + func TestAccAWSSecurityGroupRule_Race(t *testing.T) { var group ec2.SecurityGroup @@ -527,6 +545,26 @@ resource "aws_security_group_rule" "ingress_1" { } ` +const testAccAWSSecurityGroupRuleIssue5310 = ` +provider "aws" { + region = "us-east-1" +} + +resource "aws_security_group" "issue_5310" { + name = "terraform-test-issue_5310" + description = "SG for test of issue 5310" +} + +resource "aws_security_group_rule" "issue_5310" { + type = "ingress" + from_port = 0 + to_port = 65535 + protocol = "tcp" + security_group_id = "${aws_security_group.issue_5310.id}" + self = true +} +` + const testAccAWSSecurityGroupRuleIngressClassicConfig = ` provider "aws" { region = "us-east-1"