Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

r/aws_dynamodb_table_item: fix to remove item elements on update #25326

Merged
merged 6 commits into from
Jun 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/25326.txt
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
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
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