diff --git a/builtin/providers/aws/mutex.go b/builtin/providers/aws/mutex.go new file mode 100644 index 000000000000..757af34d7de5 --- /dev/null +++ b/builtin/providers/aws/mutex.go @@ -0,0 +1,51 @@ +package aws + +import ( + "log" + "sync" +) + +// MutexKV is a simple key/value store for arbitrary mutexes. It can be used to +// serialize changes across arbitrary collaborators that share knowledge of the +// keys they must serialize on. +// +// The initial use case is to let aws_security_group_rule resources serialize +// their access to individual security groups based on SG ID. +type MutexKV struct { + lock sync.Mutex + store map[string]*sync.Mutex +} + +// Locks the mutex for the given key. Caller is responsible for calling Unlock +// for the same key +func (m *MutexKV) Lock(key string) { + log.Printf("[DEBUG] Locking %q", key) + m.get(key).Lock() +} + +// Unlock the mutex for the given key. Caller must have called Lock for the same key first +func (m *MutexKV) Unlock(key string) { + log.Printf("[DEBUG] Unlocking %q", key) + m.get(key).Unlock() +} + +// Returns a mutex for the given key, no guarantee of its lock status +func (m *MutexKV) get(key string) *sync.Mutex { + m.lock.Lock() + defer m.lock.Unlock() + mutex, ok := m.store[key] + if !ok { + mutex = &sync.Mutex{} + m.store[key] = mutex + } + return mutex +} + +func NewMutexKV() *MutexKV { + return &MutexKV{ + store: make(map[string]*sync.Mutex), + } +} + +// This is a global MutexKV for use within this plugin. +var awsMutexKV = NewMutexKV() diff --git a/builtin/providers/aws/resource_aws_security_group_rule.go b/builtin/providers/aws/resource_aws_security_group_rule.go index 55499cfd5855..6d61b425deac 100644 --- a/builtin/providers/aws/resource_aws_security_group_rule.go +++ b/builtin/providers/aws/resource_aws_security_group_rule.go @@ -84,8 +84,11 @@ func resourceAwsSecurityGroupRule() *schema.Resource { func resourceAwsSecurityGroupRuleCreate(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).ec2conn sg_id := d.Get("security_group_id").(string) - sg, err := findResourceSecurityGroup(conn, sg_id) + awsMutexKV.Lock(sg_id) + defer awsMutexKV.Unlock(sg_id) + + sg, err := findResourceSecurityGroup(conn, sg_id) if err != nil { return err } @@ -249,8 +252,11 @@ func resourceAwsSecurityGroupRuleRead(d *schema.ResourceData, meta interface{}) func resourceAwsSecurityGroupRuleDelete(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).ec2conn sg_id := d.Get("security_group_id").(string) - sg, err := findResourceSecurityGroup(conn, sg_id) + awsMutexKV.Lock(sg_id) + defer awsMutexKV.Unlock(sg_id) + + sg, err := findResourceSecurityGroup(conn, sg_id) if err != nil { return err } 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 f06dd3e137ab..5cf96dace2d7 100644 --- a/builtin/providers/aws/resource_aws_security_group_rule_test.go +++ b/builtin/providers/aws/resource_aws_security_group_rule_test.go @@ -1,6 +1,7 @@ package aws import ( + "bytes" "fmt" "log" "testing" @@ -339,7 +340,24 @@ func TestAccAWSSecurityGroupRule_PartialMatching_Source(t *testing.T) { }, }, }) +} + +func TestAccAWSSecurityGroupRule_Race(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: testAccAWSSecurityGroupRuleRace, + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSSecurityGroupRuleExists("aws_security_group.race", &group), + ), + }, + }, + }) } func testAccCheckAWSSecurityGroupRuleDestroy(s *terraform.State) error { @@ -718,3 +736,41 @@ resource "aws_security_group_rule" "other_ingress" { security_group_id = "${aws_security_group.web.id}" } ` + +var testAccAWSSecurityGroupRuleRace = func() string { + var b bytes.Buffer + iterations := 50 + b.WriteString(fmt.Sprintf(` + resource "aws_vpc" "default" { + cidr_block = "10.0.0.0/16" + tags { Name = "tf-sg-rule-race" } + } + + resource "aws_security_group" "race" { + name = "tf-sg-rule-race-group-%d" + vpc_id = "${aws_vpc.default.id}" + } + `, genRandInt())) + for i := 1; i < iterations; i++ { + b.WriteString(fmt.Sprintf(` + resource "aws_security_group_rule" "ingress%d" { + security_group_id = "${aws_security_group.race.id}" + type = "ingress" + from_port = %d + to_port = %d + protocol = "tcp" + cidr_blocks = ["10.0.0.%d/32"] + } + + resource "aws_security_group_rule" "egress%d" { + security_group_id = "${aws_security_group.race.id}" + type = "egress" + from_port = %d + to_port = %d + protocol = "tcp" + cidr_blocks = ["10.0.0.%d/32"] + } + `, i, i, i, i, i, i, i, i)) + } + return b.String() +}()