Skip to content

Commit

Permalink
Merge pull request #36734 from brittandeyoung/b-aws_cloudfrontkeyvalu…
Browse files Browse the repository at this point in the history
…estore_key-mutex

BugFix:  `aws_cloudfrontkeyvaluestore_key` mutex
  • Loading branch information
ewbankkit committed Apr 4, 2024
2 parents 467a5c3 + 8f86e1f commit 604d3c6
Show file tree
Hide file tree
Showing 4 changed files with 111 additions and 8 deletions.
7 changes: 7 additions & 0 deletions .changelog/36734.txt
@@ -0,0 +1,7 @@
```release-note:bug
resource/aws_cloudfrontkeyvaluestore_key: Serialize CloudFront KeyValueStore access
```

```release-note:bug
resource/aws_cloudfront_key_value_store: Serialize CloudFront KeyValueStore access
```
16 changes: 16 additions & 0 deletions internal/service/cloudfront/key_value_store.go
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/hashicorp/terraform-plugin-framework/schema/validator"
"github.com/hashicorp/terraform-plugin-framework/types"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry"
"github.com/hashicorp/terraform-provider-aws/internal/conns"
"github.com/hashicorp/terraform-provider-aws/internal/enum"
"github.com/hashicorp/terraform-provider-aws/internal/errs"
"github.com/hashicorp/terraform-provider-aws/internal/errs/fwdiag"
Expand Down Expand Up @@ -188,6 +189,14 @@ func (r *keyValueStoreResource) Update(ctx context.Context, request resource.Upd

conn := r.Meta().CloudFrontClient(ctx)

kvsARN := old.ARN.ValueString()

// Updating changes the etag of the key value store.
// Use a mutex serialize actions
mutexKey := kvsARN
conns.GlobalMutexKV.Lock(mutexKey)
defer conns.GlobalMutexKV.Unlock(mutexKey)

input := &cloudfront.UpdateKeyValueStoreInput{}
response.Diagnostics.Append(fwflex.Expand(ctx, new, input)...)
if response.Diagnostics.HasError() {
Expand Down Expand Up @@ -224,6 +233,13 @@ func (r *keyValueStoreResource) Delete(ctx context.Context, request resource.Del

conn := r.Meta().CloudFrontClient(ctx)

kvsARN := data.ARN.ValueString()

// Use a mutex serialize actions
mutexKey := kvsARN
conns.GlobalMutexKV.Lock(mutexKey)
defer conns.GlobalMutexKV.Unlock(mutexKey)

input := &cloudfront.DeleteKeyValueStoreInput{
IfMatch: fwflex.StringFromFramework(ctx, data.ETag),
Name: fwflex.StringFromFramework(ctx, data.ID),
Expand Down
41 changes: 33 additions & 8 deletions internal/service/cloudfrontkeyvaluestore/key.go
Expand Up @@ -16,6 +16,7 @@ import (
"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier"
"github.com/hashicorp/terraform-plugin-framework/types"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry"
"github.com/hashicorp/terraform-provider-aws/internal/conns"
"github.com/hashicorp/terraform-provider-aws/internal/errs"
"github.com/hashicorp/terraform-provider-aws/internal/errs/fwdiag"
"github.com/hashicorp/terraform-provider-aws/internal/flex"
Expand Down Expand Up @@ -82,10 +83,18 @@ func (r *keyResource) Create(ctx context.Context, request resource.CreateRequest

conn := r.Meta().CloudFrontKeyValueStoreClient(ctx)

etag, err := findETagByARN(ctx, conn, data.KvsARN.ValueString())
kvsARN := data.KvsARN.ValueString()

// Adding a key changes the etag of the key value store.
// Use a mutex serialize actions
mutexKey := kvsARN
conns.GlobalMutexKV.Lock(mutexKey)
defer conns.GlobalMutexKV.Unlock(mutexKey)

etag, err := findETagByARN(ctx, conn, kvsARN)

if err != nil {
response.Diagnostics.AddError(fmt.Sprintf("reading CloudFront KeyValueStore ETag (%s)", data.KvsARN.ValueString()), err.Error())
response.Diagnostics.AddError(fmt.Sprintf("reading CloudFront KeyValueStore ETag (%s)", kvsARN), err.Error())

return
}
Expand All @@ -102,7 +111,7 @@ func (r *keyResource) Create(ctx context.Context, request resource.CreateRequest
output, err := conn.PutKey(ctx, input)

if err != nil {
response.Diagnostics.AddError(fmt.Sprintf("creating CloudFront KeyValueStore (%s) Key (%s)", data.KvsARN.ValueString(), data.Key.ValueString()), err.Error())
response.Diagnostics.AddError(fmt.Sprintf("creating CloudFront KeyValueStore (%s) Key (%s)", kvsARN, data.Key.ValueString()), err.Error())

return
}
Expand Down Expand Up @@ -167,10 +176,18 @@ func (r *keyResource) Update(ctx context.Context, request resource.UpdateRequest
conn := r.Meta().CloudFrontKeyValueStoreClient(ctx)

if !new.Value.Equal(old.Value) {
etag, err := findETagByARN(ctx, conn, new.KvsARN.ValueString())
kvsARN := new.KvsARN.ValueString()

// Updating a key changes the etag of the key value store.
// Use a mutex serialize actions
mutexKey := kvsARN
conns.GlobalMutexKV.Lock(mutexKey)
defer conns.GlobalMutexKV.Unlock(mutexKey)

etag, err := findETagByARN(ctx, conn, kvsARN)

if err != nil {
response.Diagnostics.AddError(fmt.Sprintf("reading CloudFront KeyValueStore ETag (%s)", new.KvsARN.ValueString()), err.Error())
response.Diagnostics.AddError(fmt.Sprintf("reading CloudFront KeyValueStore ETag (%s)", kvsARN), err.Error())

return
}
Expand All @@ -187,7 +204,7 @@ func (r *keyResource) Update(ctx context.Context, request resource.UpdateRequest
output, err := conn.PutKey(ctx, input)

if err != nil {
response.Diagnostics.AddError(fmt.Sprintf("updating CloudFront KeyValueStore (%s) Key (%s)", new.KvsARN.ValueString(), new.Key.ValueString()), err.Error())
response.Diagnostics.AddError(fmt.Sprintf("updating CloudFront KeyValueStore (%s) Key (%s)", kvsARN, new.Key.ValueString()), err.Error())

return
}
Expand All @@ -208,10 +225,18 @@ func (r *keyResource) Delete(ctx context.Context, request resource.DeleteRequest

conn := r.Meta().CloudFrontKeyValueStoreClient(ctx)

etag, err := findETagByARN(ctx, conn, data.KvsARN.ValueString())
kvsARN := data.KvsARN.ValueString()

// Deleting a key changes the etag of the key value store.
// Use a mutex serialize actions
mutexKey := kvsARN
conns.GlobalMutexKV.Lock(mutexKey)
defer conns.GlobalMutexKV.Unlock(mutexKey)

etag, err := findETagByARN(ctx, conn, kvsARN)

if err != nil {
response.Diagnostics.AddError(fmt.Sprintf("reading CloudFront KeyValueStore ETag (%s)", data.KvsARN.ValueString()), err.Error())
response.Diagnostics.AddError(fmt.Sprintf("reading CloudFront KeyValueStore ETag (%s)", kvsARN), err.Error())

return
}
Expand Down
55 changes: 55 additions & 0 deletions internal/service/cloudfrontkeyvaluestore/key_test.go
Expand Up @@ -5,6 +5,7 @@ package cloudfrontkeyvaluestore_test

import (
"context"
"encoding/json"
"fmt"
"testing"

Expand Down Expand Up @@ -53,6 +54,39 @@ func TestAccCloudFrontKeyValueStoreKey_basic(t *testing.T) {
})
}

// This test is to verify the mutex lock is working correctly to allow serializing multiple keys being changed
func TestAccCloudFrontKeyValueStoreKey_mutex(t *testing.T) {
ctx := acctest.Context(t)
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
var rNames []string
for i := 1; i < 6; i++ {
rNames = append(rNames, sdkacctest.RandomWithPrefix(acctest.ResourcePrefix))
}
value := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() {
acctest.PreCheck(ctx, t)
acctest.PreCheckPartitionHasService(t, names.CloudFront)
},
ErrorCheck: acctest.ErrorCheck(t, names.CloudFront),
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
CheckDestroy: testAccCheckKeyDestroy(ctx),
Steps: []resource.TestStep{
{
Config: testAccKeyConfig_mutex(rNames, rName, value),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("aws_cloudfrontkeyvaluestore_key.test.0", "key", rNames[0]),
resource.TestCheckResourceAttr("aws_cloudfrontkeyvaluestore_key.test.1", "key", rNames[1]),
resource.TestCheckResourceAttr("aws_cloudfrontkeyvaluestore_key.test.2", "key", rNames[2]),
resource.TestCheckResourceAttr("aws_cloudfrontkeyvaluestore_key.test.3", "key", rNames[3]),
resource.TestCheckResourceAttr("aws_cloudfrontkeyvaluestore_key.test.4", "key", rNames[4]),
),
},
},
})
}

func TestAccCloudFrontKeyValueStoreKey_value(t *testing.T) {
ctx := acctest.Context(t)
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
Expand Down Expand Up @@ -182,3 +216,24 @@ resource "aws_cloudfrontkeyvaluestore_key" "test" {
}
`, rName, value)
}

func testAccKeyConfig_mutex(rNames []string, rName, value string) string {
rNameJson, _ := json.Marshal(rNames)
rNameString := string(rNameJson)
return fmt.Sprintf(`
locals {
key_list = %[1]s
}
resource "aws_cloudfront_key_value_store" "test" {
name = %[2]q
}
resource "aws_cloudfrontkeyvaluestore_key" "test" {
count = length(local.key_list)
key = local.key_list[count.index]
key_value_store_arn = aws_cloudfront_key_value_store.test.arn
value = %[3]q
}
`, rNameString, rName, value)
}

0 comments on commit 604d3c6

Please sign in to comment.