Skip to content

Commit

Permalink
Merge pull request #25326 from hashicorp/b-aws_dynamodb_table_item_up…
Browse files Browse the repository at this point in the history 
…date

r/aws_dynamodb_table_item: fix to remove item elements on update
  • Loading branch information
@johnsonaj
johnsonaj committed Jun 14, 2022
2 parents 3cc8e54 + 807cfa5 commit 9b3b6f2
Show file tree
Hide file tree
Showing 3 changed files with 132 additions and 71 deletions.
3 changes: 3 additions & 0 deletions .changelog/25326.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
resource/aws_dynamodb_table_item: Fix to remove attribute from table item on update
```
86 changes: 60 additions & 26 deletions internal/service/dynamodb/table_item.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -120,19 +122,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
Expand Down Expand Up @@ -169,29 +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),
ProjectionExpression: BuildProjectionExpression(attributes),
ExpressionAttributeNames: BuildExpressionAttributeNames(attributes),
})
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)
Expand Down Expand Up @@ -221,11 +223,43 @@ func resourceTableItemDelete(d *schema.ResourceData, meta interface{}) error {
Key: queryKey,
TableName: aws.String(d.Get("table_name").(string)),
})
return err

if err != nil {
return fmt.Errorf("error deleting DynamoDB Table Item (%s): %w", d.Id(), err)
}

return nil
}

// 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{}

Expand Down
114 changes: 69 additions & 45 deletions internal/service/dynamodb/table_item_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,19 @@ 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) {
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"},
Expand All @@ -32,10 +32,10 @@ func TestAccDynamoDBTableItem_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),
Expand All @@ -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 := `{
Expand All @@ -67,7 +67,7 @@ func TestAccDynamoDBTableItem_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),
Expand All @@ -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 := `{
Expand All @@ -111,7 +111,7 @@ func TestAccDynamoDBTableItem_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),
Expand All @@ -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)
Expand All @@ -154,7 +154,7 @@ func TestAccDynamoDBTableItem_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),
Expand All @@ -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 := `{
Expand All @@ -196,10 +196,10 @@ func TestAccDynamoDBTableItem_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),
Expand All @@ -209,7 +209,7 @@ func TestAccDynamoDBTableItem_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),
Expand All @@ -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"

Expand All @@ -244,7 +244,7 @@ func TestAccDynamoDBTableItem_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),
Expand Down Expand Up @@ -272,7 +272,39 @@ func TestAccDynamoDBTableItem_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 {
Expand All @@ -286,21 +318,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)
Expand Down Expand Up @@ -328,15 +355,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
Expand Down Expand Up @@ -364,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"
Expand Down

0 comments on commit 9b3b6f2

Please sign in to comment.