From e6a877aadb6f1fdb0c55a4eb6a4d4c95a5d81a55 Mon Sep 17 00:00:00 2001 From: Brittan DeYoung <32572259+brittandeyoung@users.noreply.github.com> Date: Thu, 4 Apr 2024 10:27:03 -0400 Subject: [PATCH 1/4] cloudfront: Amend key_value_store, Add mutex on update and delete --- internal/service/cloudfront/key_value_store.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/internal/service/cloudfront/key_value_store.go b/internal/service/cloudfront/key_value_store.go index 9618734e5de9..63b41fe746bd 100644 --- a/internal/service/cloudfront/key_value_store.go +++ b/internal/service/cloudfront/key_value_store.go @@ -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" @@ -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() { @@ -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), From 681c394048de856e28f3ee59e11ace45fe3e26c4 Mon Sep 17 00:00:00 2001 From: Brittan DeYoung <32572259+brittandeyoung@users.noreply.github.com> Date: Thu, 4 Apr 2024 10:27:50 -0400 Subject: [PATCH 2/4] cloudfrontkeyvaluestore: Amend key, Add mutex on update, create, and delete --- .../service/cloudfrontkeyvaluestore/key.go | 41 +++++++++++++++---- 1 file changed, 33 insertions(+), 8 deletions(-) diff --git a/internal/service/cloudfrontkeyvaluestore/key.go b/internal/service/cloudfrontkeyvaluestore/key.go index 8dacfb9e68fd..64d25ccb551b 100644 --- a/internal/service/cloudfrontkeyvaluestore/key.go +++ b/internal/service/cloudfrontkeyvaluestore/key.go @@ -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" @@ -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 } @@ -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 } @@ -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 } @@ -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 } @@ -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 } From ca59da6a8face2bccf15cf3d41a63fe1ac3bda10 Mon Sep 17 00:00:00 2001 From: Brittan DeYoung <32572259+brittandeyoung@users.noreply.github.com> Date: Thu, 4 Apr 2024 10:29:29 -0400 Subject: [PATCH 3/4] cloudfrontkeyvaluestore: Amend key_test, Add test to validate mutex --- .../cloudfrontkeyvaluestore/key_test.go | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/internal/service/cloudfrontkeyvaluestore/key_test.go b/internal/service/cloudfrontkeyvaluestore/key_test.go index 4e4da7288cf2..f9eeaa6b4a1e 100644 --- a/internal/service/cloudfrontkeyvaluestore/key_test.go +++ b/internal/service/cloudfrontkeyvaluestore/key_test.go @@ -5,6 +5,7 @@ package cloudfrontkeyvaluestore_test import ( "context" + "encoding/json" "fmt" "testing" @@ -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) @@ -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) +} From 8f86e1feb73bb8d8be2a392c847fbcc28cd24600 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Thu, 4 Apr 2024 11:39:15 -0400 Subject: [PATCH 4/4] Add CHANGELOG entries. --- .changelog/36734.txt | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changelog/36734.txt diff --git a/.changelog/36734.txt b/.changelog/36734.txt new file mode 100644 index 000000000000..ba9989d251fe --- /dev/null +++ b/.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 +``` \ No newline at end of file