From c2facd05e07c773495a44074fde7ba4f17db5ae3 Mon Sep 17 00:00:00 2001 From: Adrian Johnson Date: Mon, 13 Jun 2022 16:19:19 -0500 Subject: [PATCH 1/6] r/aws_dynamodb_table_item: remove items no longer is schema --- internal/service/dynamodb/table_item.go | 34 +++++++++++++++---------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/internal/service/dynamodb/table_item.go b/internal/service/dynamodb/table_item.go index ad43fda9e5b..6612c288624 100644 --- a/internal/service/dynamodb/table_item.go +++ b/internal/service/dynamodb/table_item.go @@ -120,19 +120,29 @@ func resourceTableItemUpdate(d *schema.ResourceData, meta interface{}) error { } } + oldAttributes, err := ExpandTableItemAttributes(oldItem.(string)) + if err != nil { + return err + } + + for k := range oldAttributes { + if k == hashKey || k == rangeKey { + continue + } + if _, ok := attributes[k]; !ok { + updates[k] = &dynamodb.AttributeValueUpdate{ + Action: aws.String(dynamodb.AttributeActionDelete), + } + } + } + _, err = conn.UpdateItem(&dynamodb.UpdateItemInput{ AttributeUpdates: updates, TableName: aws.String(tableName), Key: newQueryKey, }) if err != nil { - return err - } - - oItem := oldItem.(string) - oldAttributes, err := ExpandTableItemAttributes(oItem) - if err != nil { - return err + return fmt.Errorf("error updating DynamoDB Table Item (%s): %w", d.Id(), err) } // New record is created via UpdateItem in case we're changing hash key @@ -170,11 +180,9 @@ func resourceTableItemRead(d *schema.ResourceData, meta interface{}) error { } result, err := conn.GetItem(&dynamodb.GetItemInput{ - TableName: aws.String(tableName), - ConsistentRead: aws.Bool(true), - Key: BuildTableItemqueryKey(attributes, hashKey, rangeKey), - ProjectionExpression: BuildProjectionExpression(attributes), - ExpressionAttributeNames: BuildExpressionAttributeNames(attributes), + TableName: aws.String(tableName), + ConsistentRead: aws.Bool(true), + Key: BuildTableItemqueryKey(attributes, hashKey, rangeKey), }) if err != nil { if tfawserr.ErrCodeEquals(err, dynamodb.ErrCodeResourceNotFoundException) { @@ -221,7 +229,7 @@ func resourceTableItemDelete(d *schema.ResourceData, meta interface{}) error { Key: queryKey, TableName: aws.String(d.Get("table_name").(string)), }) - return err + return fmt.Errorf("error deleting DynamoDB Table Item (%s): %w", d.Id(), err) } // Helpers From a37b19d6d3c434509db00f1f339594668dacf8f3 Mon Sep 17 00:00:00 2001 From: Adrian Johnson Date: Mon, 13 Jun 2022 16:30:38 -0500 Subject: [PATCH 2/6] r/aws_dynamodb_table_item: fix error message --- internal/service/dynamodb/table_item.go | 7 ++++++- internal/service/dynamodb/table_item_test.go | 10 +++++----- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/internal/service/dynamodb/table_item.go b/internal/service/dynamodb/table_item.go index 6612c288624..841c2612444 100644 --- a/internal/service/dynamodb/table_item.go +++ b/internal/service/dynamodb/table_item.go @@ -229,7 +229,12 @@ func resourceTableItemDelete(d *schema.ResourceData, meta interface{}) error { Key: queryKey, TableName: aws.String(d.Get("table_name").(string)), }) - return fmt.Errorf("error deleting DynamoDB Table Item (%s): %w", d.Id(), err) + + if err != nil { + return fmt.Errorf("error deleting DynamoDB Table Item (%s): %w", d.Id(), err) + } + + return nil } // Helpers diff --git a/internal/service/dynamodb/table_item_test.go b/internal/service/dynamodb/table_item_test.go index c6b0876a30a..cace5f48f1d 100644 --- a/internal/service/dynamodb/table_item_test.go +++ b/internal/service/dynamodb/table_item_test.go @@ -287,11 +287,11 @@ func testAccCheckItemDestroy(s *terraform.State) error { } result, err := conn.GetItem(&dynamodb.GetItemInput{ - TableName: aws.String(attrs["table_name"]), - ConsistentRead: aws.Bool(true), - Key: tfdynamodb.BuildTableItemqueryKey(attributes, attrs["hash_key"], attrs["range_key"]), - ProjectionExpression: tfdynamodb.BuildProjectionExpression(attributes), - ExpressionAttributeNames: tfdynamodb.BuildExpressionAttributeNames(attributes), + TableName: aws.String(attrs["table_name"]), + ConsistentRead: aws.Bool(true), + Key: tfdynamodb.BuildTableItemqueryKey(attributes, attrs["hash_key"], attrs["range_key"]), + //ProjectionExpression: tfdynamodb.BuildProjectionExpression(attributes), + //ExpressionAttributeNames: tfdynamodb.BuildExpressionAttributeNames(attributes), }) if err != nil { if tfawserr.ErrCodeEquals(err, dynamodb.ErrCodeResourceNotFoundException) { From 5d4477d9bfa4b0642eab11b514aafe32d8571bbe Mon Sep 17 00:00:00 2001 From: Adrian Johnson Date: Tue, 14 Jun 2022 10:48:10 -0500 Subject: [PATCH 3/6] r/aws_dynamodb_table_item: add finder function --- internal/service/dynamodb/table_item.go | 53 ++++++++++++++------ internal/service/dynamodb/table_item_test.go | 38 ++++++-------- 2 files changed, 52 insertions(+), 39 deletions(-) diff --git a/internal/service/dynamodb/table_item.go b/internal/service/dynamodb/table_item.go index 841c2612444..ac98fd9a55b 100644 --- a/internal/service/dynamodb/table_item.go +++ b/internal/service/dynamodb/table_item.go @@ -10,8 +10,10 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/dynamodb" "github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-provider-aws/internal/conns" + "github.com/hashicorp/terraform-provider-aws/internal/tfresource" "github.com/hashicorp/terraform-provider-aws/internal/verify" ) @@ -179,27 +181,19 @@ func resourceTableItemRead(d *schema.ResourceData, meta interface{}) error { return err } - result, err := conn.GetItem(&dynamodb.GetItemInput{ - TableName: aws.String(tableName), - ConsistentRead: aws.Bool(true), - Key: BuildTableItemqueryKey(attributes, hashKey, rangeKey), - }) - if err != nil { - if tfawserr.ErrCodeEquals(err, dynamodb.ErrCodeResourceNotFoundException) { - log.Printf("[WARN] Dynamodb Table Item (%s) not found, error code (404)", d.Id()) - d.SetId("") - return nil - } - - return fmt.Errorf("Error retrieving DynamoDB table item: %s", err) - } + key := BuildTableItemqueryKey(attributes, hashKey, rangeKey) + result, err := FindTableItem(conn, tableName, key) - if result.Item == nil { - log.Printf("[WARN] Dynamodb Table Item (%s) not found", d.Id()) + if !d.IsNewResource() && tfresource.NotFound(err) { + log.Printf("[WARN] Dynamodb Table Item (%s) not found, removing from state", d.Id()) d.SetId("") return nil } + if err != nil { + return fmt.Errorf("error reading DynamoDB Table Item (%s): %w", d.Id(), err) + } + // The record exists, now test if it differs from what is desired if !reflect.DeepEqual(result.Item, attributes) { itemAttrs, err := flattenTableItemAttributes(result.Item) @@ -239,6 +233,33 @@ func resourceTableItemDelete(d *schema.ResourceData, meta interface{}) error { // Helpers +func FindTableItem(conn *dynamodb.DynamoDB, tableName string, key map[string]*dynamodb.AttributeValue) (*dynamodb.GetItemOutput, error) { + in := &dynamodb.GetItemInput{ + TableName: aws.String(tableName), + ConsistentRead: aws.Bool(true), + Key: key, + } + + out, err := conn.GetItem(in) + + if tfawserr.ErrCodeEquals(err, dynamodb.ErrCodeResourceNotFoundException) { + return nil, &resource.NotFoundError{ + LastError: err, + LastRequest: in, + } + } + + if err != nil { + return nil, err + } + + if out == nil || out.Item == nil { + return nil, tfresource.NewEmptyResultError(in) + } + + return out, nil +} + func BuildExpressionAttributeNames(attrs map[string]*dynamodb.AttributeValue) map[string]*string { names := map[string]*string{} diff --git a/internal/service/dynamodb/table_item_test.go b/internal/service/dynamodb/table_item_test.go index cace5f48f1d..f9adb7c8553 100644 --- a/internal/service/dynamodb/table_item_test.go +++ b/internal/service/dynamodb/table_item_test.go @@ -6,13 +6,13 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/dynamodb" - "github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr" sdkacctest "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" "github.com/hashicorp/terraform-provider-aws/internal/acctest" "github.com/hashicorp/terraform-provider-aws/internal/conns" tfdynamodb "github.com/hashicorp/terraform-provider-aws/internal/service/dynamodb" + "github.com/hashicorp/terraform-provider-aws/internal/tfresource" ) func TestAccDynamoDBTableItem_basic(t *testing.T) { @@ -286,21 +286,16 @@ func testAccCheckItemDestroy(s *terraform.State) error { return err } - result, err := conn.GetItem(&dynamodb.GetItemInput{ - TableName: aws.String(attrs["table_name"]), - ConsistentRead: aws.Bool(true), - Key: tfdynamodb.BuildTableItemqueryKey(attributes, attrs["hash_key"], attrs["range_key"]), - //ProjectionExpression: tfdynamodb.BuildProjectionExpression(attributes), - //ExpressionAttributeNames: tfdynamodb.BuildExpressionAttributeNames(attributes), - }) - if err != nil { - if tfawserr.ErrCodeEquals(err, dynamodb.ErrCodeResourceNotFoundException) { - return nil - } - return fmt.Errorf("Error retrieving DynamoDB table item: %s", err) + key := tfdynamodb.BuildTableItemqueryKey(attributes, attrs["hash_key"], attrs["range_key"]) + + _, err = tfdynamodb.FindTableItem(conn, attrs["table_name"], key) + + if tfresource.NotFound(err) { + continue } - if result.Item == nil { - return nil + + if err != nil { + return err } return fmt.Errorf("DynamoDB table item %s still exists.", rs.Primary.ID) @@ -328,15 +323,12 @@ func testAccCheckTableItemExists(n string, item *dynamodb.GetItemOutput) resourc return err } - result, err := conn.GetItem(&dynamodb.GetItemInput{ - TableName: aws.String(attrs["table_name"]), - ConsistentRead: aws.Bool(true), - Key: tfdynamodb.BuildTableItemqueryKey(attributes, attrs["hash_key"], attrs["range_key"]), - ProjectionExpression: tfdynamodb.BuildProjectionExpression(attributes), - ExpressionAttributeNames: tfdynamodb.BuildExpressionAttributeNames(attributes), - }) + key := tfdynamodb.BuildTableItemqueryKey(attributes, attrs["hash_key"], attrs["range_key"]) + + result, err := tfdynamodb.FindTableItem(conn, attrs["table_name"], key) + if err != nil { - return fmt.Errorf("Problem getting table item '%s': %s", rs.Primary.ID, err) + return err } *item = *result From 2e7da8d3ffeacd46f811abc100bbca00d8a54429 Mon Sep 17 00:00:00 2001 From: Adrian Johnson Date: Tue, 14 Jun 2022 10:59:27 -0500 Subject: [PATCH 4/6] r/aws_dynamodb_table_item: cleanup test func names --- internal/service/dynamodb/table_item_test.go | 22 ++++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/internal/service/dynamodb/table_item_test.go b/internal/service/dynamodb/table_item_test.go index f9adb7c8553..e043fa666bd 100644 --- a/internal/service/dynamodb/table_item_test.go +++ b/internal/service/dynamodb/table_item_test.go @@ -15,10 +15,10 @@ import ( "github.com/hashicorp/terraform-provider-aws/internal/tfresource" ) -func TestAccDynamoDBTableItem_basic(t *testing.T) { +func TestAccTableItem_basic(t *testing.T) { var conf dynamodb.GetItemOutput - tableName := fmt.Sprintf("tf-acc-test-%s", sdkacctest.RandString(8)) + tableName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) hashKey := "hashKey" itemContent := `{ "hashKey": {"S": "something"}, @@ -48,10 +48,10 @@ func TestAccDynamoDBTableItem_basic(t *testing.T) { }) } -func TestAccDynamoDBTableItem_rangeKey(t *testing.T) { +func TestAccTableItem_rangeKey(t *testing.T) { var conf dynamodb.GetItemOutput - tableName := fmt.Sprintf("tf-acc-test-%s", sdkacctest.RandString(8)) + tableName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) hashKey := "hashKey" rangeKey := "rangeKey" itemContent := `{ @@ -84,11 +84,11 @@ func TestAccDynamoDBTableItem_rangeKey(t *testing.T) { }) } -func TestAccDynamoDBTableItem_withMultipleItems(t *testing.T) { +func TestAccTableItem_withMultipleItems(t *testing.T) { var conf1 dynamodb.GetItemOutput var conf2 dynamodb.GetItemOutput - tableName := fmt.Sprintf("tf-acc-test-%s", sdkacctest.RandString(8)) + tableName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) hashKey := "hashKey" rangeKey := "rangeKey" firstItem := `{ @@ -135,7 +135,7 @@ func TestAccDynamoDBTableItem_withMultipleItems(t *testing.T) { }) } -func TestAccDynamoDBTableItem_wonkyItems(t *testing.T) { +func TestAccTableItem_wonkyItems(t *testing.T) { var conf1 dynamodb.GetItemOutput rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) @@ -172,10 +172,10 @@ func TestAccDynamoDBTableItem_wonkyItems(t *testing.T) { }) } -func TestAccDynamoDBTableItem_update(t *testing.T) { +func TestAccTableItem_update(t *testing.T) { var conf dynamodb.GetItemOutput - tableName := fmt.Sprintf("tf-acc-test-%s", sdkacctest.RandString(8)) + tableName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) hashKey := "hashKey" itemBefore := `{ @@ -222,10 +222,10 @@ func TestAccDynamoDBTableItem_update(t *testing.T) { }) } -func TestAccDynamoDBTableItem_updateWithRangeKey(t *testing.T) { +func TestAccTableItem_updateWithRangeKey(t *testing.T) { var conf dynamodb.GetItemOutput - tableName := fmt.Sprintf("tf-acc-test-%s", sdkacctest.RandString(8)) + tableName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) hashKey := "hashKey" rangeKey := "rangeKey" From a5f25c8c4e37fd2bec27dd8c408b9ac5926e623d Mon Sep 17 00:00:00 2001 From: Adrian Johnson Date: Tue, 14 Jun 2022 11:09:22 -0500 Subject: [PATCH 5/6] r/aws_dynamodb_table_item: add disappears test --- internal/service/dynamodb/table_item_test.go | 54 ++++++++++++++++---- 1 file changed, 43 insertions(+), 11 deletions(-) diff --git a/internal/service/dynamodb/table_item_test.go b/internal/service/dynamodb/table_item_test.go index e043fa666bd..0073efad661 100644 --- a/internal/service/dynamodb/table_item_test.go +++ b/internal/service/dynamodb/table_item_test.go @@ -32,10 +32,10 @@ func TestAccTableItem_basic(t *testing.T) { PreCheck: func() { acctest.PreCheck(t) }, ErrorCheck: acctest.ErrorCheck(t, dynamodb.EndpointsID), ProviderFactories: acctest.ProviderFactories, - CheckDestroy: testAccCheckItemDestroy, + CheckDestroy: testAccCheckTableItemDestroy, Steps: []resource.TestStep{ { - Config: testAccItemBasicConfig(tableName, hashKey, itemContent), + Config: testAccTableItemBasicConfig(tableName, hashKey, itemContent), Check: resource.ComposeTestCheckFunc( testAccCheckTableItemExists("aws_dynamodb_table_item.test", &conf), testAccCheckTableItemCount(tableName, 1), @@ -67,7 +67,7 @@ func TestAccTableItem_rangeKey(t *testing.T) { PreCheck: func() { acctest.PreCheck(t) }, ErrorCheck: acctest.ErrorCheck(t, dynamodb.EndpointsID), ProviderFactories: acctest.ProviderFactories, - CheckDestroy: testAccCheckItemDestroy, + CheckDestroy: testAccCheckTableItemDestroy, Steps: []resource.TestStep{ { Config: testAccItemWithRangeKeyConfig(tableName, hashKey, rangeKey, itemContent), @@ -111,7 +111,7 @@ func TestAccTableItem_withMultipleItems(t *testing.T) { PreCheck: func() { acctest.PreCheck(t) }, ErrorCheck: acctest.ErrorCheck(t, dynamodb.EndpointsID), ProviderFactories: acctest.ProviderFactories, - CheckDestroy: testAccCheckItemDestroy, + CheckDestroy: testAccCheckTableItemDestroy, Steps: []resource.TestStep{ { Config: testAccItemWithMultipleItemsConfig(tableName, hashKey, rangeKey, firstItem, secondItem), @@ -154,7 +154,7 @@ func TestAccTableItem_wonkyItems(t *testing.T) { PreCheck: func() { acctest.PreCheck(t) }, ErrorCheck: acctest.ErrorCheck(t, dynamodb.EndpointsID), ProviderFactories: acctest.ProviderFactories, - CheckDestroy: testAccCheckItemDestroy, + CheckDestroy: testAccCheckTableItemDestroy, Steps: []resource.TestStep{ { Config: testAccItemWithWonkyItemsConfig(rName, hashKey, rangeKey, item), @@ -196,10 +196,10 @@ func TestAccTableItem_update(t *testing.T) { PreCheck: func() { acctest.PreCheck(t) }, ErrorCheck: acctest.ErrorCheck(t, dynamodb.EndpointsID), ProviderFactories: acctest.ProviderFactories, - CheckDestroy: testAccCheckItemDestroy, + CheckDestroy: testAccCheckTableItemDestroy, Steps: []resource.TestStep{ { - Config: testAccItemBasicConfig(tableName, hashKey, itemBefore), + Config: testAccTableItemBasicConfig(tableName, hashKey, itemBefore), Check: resource.ComposeTestCheckFunc( testAccCheckTableItemExists("aws_dynamodb_table_item.test", &conf), testAccCheckTableItemCount(tableName, 1), @@ -209,7 +209,7 @@ func TestAccTableItem_update(t *testing.T) { ), }, { - Config: testAccItemBasicConfig(tableName, hashKey, itemAfter), + Config: testAccTableItemBasicConfig(tableName, hashKey, itemAfter), Check: resource.ComposeTestCheckFunc( testAccCheckTableItemExists("aws_dynamodb_table_item.test", &conf), testAccCheckTableItemCount(tableName, 1), @@ -244,7 +244,7 @@ func TestAccTableItem_updateWithRangeKey(t *testing.T) { PreCheck: func() { acctest.PreCheck(t) }, ErrorCheck: acctest.ErrorCheck(t, dynamodb.EndpointsID), ProviderFactories: acctest.ProviderFactories, - CheckDestroy: testAccCheckItemDestroy, + CheckDestroy: testAccCheckTableItemDestroy, Steps: []resource.TestStep{ { Config: testAccItemWithRangeKeyConfig(tableName, hashKey, rangeKey, itemBefore), @@ -272,7 +272,39 @@ func TestAccTableItem_updateWithRangeKey(t *testing.T) { }) } -func testAccCheckItemDestroy(s *terraform.State) error { +func TestAccTableItem_disappears(t *testing.T) { + var conf dynamodb.GetItemOutput + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_dynamodb_table_item.test" + + hashKey := "hashKey" + itemContent := `{ + "hashKey": {"S": "something"}, + "one": {"N": "11111"}, + "two": {"N": "22222"}, + "three": {"N": "33333"}, + "four": {"N": "44444"} +}` + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, dynamodb.EndpointsID), + ProviderFactories: acctest.ProviderFactories, + CheckDestroy: testAccCheckTableItemDestroy, + Steps: []resource.TestStep{ + { + Config: testAccTableItemBasicConfig(rName, hashKey, itemContent), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckTableItemExists(resourceName, &conf), + acctest.CheckResourceDisappears(acctest.Provider, tfdynamodb.ResourceTableItem(), resourceName), + ), + ExpectNonEmptyPlan: true, + }, + }, + }) +} + +func testAccCheckTableItemDestroy(s *terraform.State) error { conn := acctest.Provider.Meta().(*conns.AWSClient).DynamoDBConn for _, rs := range s.RootModule().Resources { @@ -356,7 +388,7 @@ func testAccCheckTableItemCount(tableName string, count int64) resource.TestChec } } -func testAccItemBasicConfig(tableName, hashKey, item string) string { +func testAccTableItemBasicConfig(tableName, hashKey, item string) string { return fmt.Sprintf(` resource "aws_dynamodb_table" "test" { name = "%s" From 807cfa598760fbc029969fcdd1a78dd939a3fc1e Mon Sep 17 00:00:00 2001 From: Adrian Johnson Date: Tue, 14 Jun 2022 11:49:39 -0500 Subject: [PATCH 6/6] update CHANGELOG for #25326 --- .changelog/25326.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/25326.txt diff --git a/.changelog/25326.txt b/.changelog/25326.txt new file mode 100644 index 00000000000..a20b264d19e --- /dev/null +++ b/.changelog/25326.txt @@ -0,0 +1,3 @@ +```release-note:bug + resource/aws_dynamodb_table_item: Fix to remove attribute from table item on update + ``` \ No newline at end of file