Merge pull request #39752 from hashicorp/b-dynamodb_table_payperreque…

…st_update r/dynamodb_table: fix crash when table is updated without GSI updates
hashicorp · Oct 16, 2024 · ecb8002 · ecb8002
2 parents 14689f2 + a136312
commit ecb8002
Show file tree

Hide file tree

Showing 3 changed files with 55 additions and 11 deletions.
diff --git a/.changelog/39752.txt b/.changelog/39752.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+resource/aws_dynamodb_table: Fix crash when `billing_mode` is set to `PAY_PER_REQUEST` without `global_secondary_index` updates
+```
diff --git a/internal/service/dynamodb/table.go b/internal/service/dynamodb/table.go
@@ -272,9 +272,10 @@ func resourceTable() *schema.Resource {
 				ForceNew: true,
 			},
 			"read_capacity": {
-				Type:     schema.TypeInt,
-				Optional: true,
-				Computed: true,
+				Type:          schema.TypeInt,
+				Optional:      true,
+				Computed:      true,
+				ConflictsWith: []string{"on_demand_throughput"},
 			},
 			"replica": {
 				Type:     schema.TypeSet,
@@ -483,9 +484,10 @@ func resourceTable() *schema.Resource {
 				DiffSuppressFunc: verify.SuppressMissingOptionalConfigurationBlock,
 			},
 			"write_capacity": {
-				Type:     schema.TypeInt,
-				Computed: true,
-				Optional: true,
+				Type:          schema.TypeInt,
+				Computed:      true,
+				Optional:      true,
+				ConflictsWith: []string{"on_demand_throughput"},
 			},
 		},
 	}
@@ -1079,7 +1081,7 @@ func resourceTableUpdate(ctx context.Context, d *schema.ResourceData, meta inter
 	// update only on-demand throughput indexes when switching to PAY_PER_REQUEST
 	if newBillingMode == awstypes.BillingModePayPerRequest {
 		for _, gsiUpdate := range gsiUpdates {
-			if gsiUpdate.Update.OnDemandThroughput == nil {
+			if gsiUpdate.Update == nil || (gsiUpdate.Update != nil && gsiUpdate.Update.OnDemandThroughput == nil) {
 				continue
 			}
 
@@ -1736,15 +1738,15 @@ func updateDiffGSI(oldGsi, newGsi []interface{}, billingMode awstypes.BillingMod
 			}
 			otherAttributesChanged := nonKeyAttributesChanged || !reflect.DeepEqual(oldAttributes, newAttributes)
 
-			if capacityChanged && !otherAttributesChanged {
+			if capacityChanged && !otherAttributesChanged && billingMode == awstypes.BillingModeProvisioned {
 				update := awstypes.GlobalSecondaryIndexUpdate{
 					Update: &awstypes.UpdateGlobalSecondaryIndexAction{
 						IndexName:             aws.String(idxName),
 						ProvisionedThroughput: expandProvisionedThroughput(newMap, billingMode),
 					},
 				}
 				ops = append(ops, update)
-			} else if onDemandThroughputChanged && !otherAttributesChanged {
+			} else if onDemandThroughputChanged && !otherAttributesChanged && billingMode == awstypes.BillingModePayPerRequest {
 				update := awstypes.GlobalSecondaryIndexUpdate{
 					Update: &awstypes.UpdateGlobalSecondaryIndexAction{
 						IndexName:          aws.String(idxName),

diff --git a/internal/service/dynamodb/table_test.go b/internal/service/dynamodb/table_test.go
@@ -846,6 +846,45 @@ func TestAccDynamoDBTable_BillingModeGSI_provisionedToPayPerRequest(t *testing.T
 	})
 }
 
+func TestAccDynamoDBTable_BillingMode_payPerRequestBasic(t *testing.T) {
+	ctx := acctest.Context(t)
+	var conf awstypes.TableDescription
+	resourceName := "aws_dynamodb_table.test"
+	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:                 func() { acctest.PreCheck(ctx, t) },
+		ErrorCheck:               acctest.ErrorCheck(t, names.DynamoDBServiceID),
+		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
+		CheckDestroy:             testAccCheckTableDestroy(ctx),
+		Steps: []resource.TestStep{
+			{
+				Config: testAccTableConfig_billingPayPerRequest(rName),
+				Check: resource.ComposeAggregateTestCheckFunc(
+					testAccCheckInitialTableExists(ctx, resourceName, &conf),
+					resource.TestCheckResourceAttr(resourceName, "billing_mode", string(awstypes.BillingModePayPerRequest)),
+					resource.TestCheckResourceAttr(resourceName, "read_capacity", acctest.Ct0),
+					resource.TestCheckResourceAttr(resourceName, "write_capacity", acctest.Ct0),
+				),
+			},
+			{
+				ResourceName:      resourceName,
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
+			{
+				Config: testAccTableConfig_billingPayPerRequestGSI(rName),
+				Check: resource.ComposeAggregateTestCheckFunc(
+					testAccCheckInitialTableExists(ctx, resourceName, &conf),
+					resource.TestCheckResourceAttr(resourceName, "billing_mode", string(awstypes.BillingModePayPerRequest)),
+					resource.TestCheckResourceAttr(resourceName, "read_capacity", acctest.Ct0),
+					resource.TestCheckResourceAttr(resourceName, "write_capacity", acctest.Ct0),
+				),
+			},
+		},
+	})
+}
+
 func TestAccDynamoDBTable_onDemandThroughput(t *testing.T) {
 	ctx := acctest.Context(t)
 	var conf awstypes.TableDescription
@@ -1781,8 +1820,8 @@ func TestAccDynamoDBTable_restoreCrossRegion(t *testing.T) {
 					testAccCheckInitialTableExists(ctx, resourceName, &conf),
 					resource.TestCheckResourceAttr(resourceName, names.AttrName, rName),
 					resource.TestCheckResourceAttr(resourceNameRestore, names.AttrName, rNameRestore),
-					acctest.MatchResourceAttrRegionalARNRegion(resourceName, names.AttrARN, "dynamodb", acctest.Region(), regexache.MustCompile(`table/XXX`)),
-					acctest.MatchResourceAttrRegionalARNRegion(resourceNameRestore, names.AttrARN, "dynamodb", acctest.AlternateRegion(), regexache.MustCompile(`table/YYY`)),
+					acctest.MatchResourceAttrRegionalARNRegion(resourceName, names.AttrARN, "dynamodb", acctest.Region(), regexache.MustCompile(`table/.+$`)),
+					acctest.MatchResourceAttrRegionalARNRegion(resourceNameRestore, names.AttrARN, "dynamodb", acctest.AlternateRegion(), regexache.MustCompile(`table/.+$`)),
 				),
 			},
 			{