From bb02c07ccd455c7c57846adb7d10da56c939cc7a Mon Sep 17 00:00:00 2001 From: Steve Zesch Date: Sat, 3 Feb 2024 11:51:28 -0500 Subject: [PATCH 01/16] Fix dynamodb make a DescribeTable call per replica --- internal/service/dynamodb/table.go | 45 +++++++++++++++--------------- 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/internal/service/dynamodb/table.go b/internal/service/dynamodb/table.go index 3c3746fc296..9ab5ee2b3a5 100644 --- a/internal/service/dynamodb/table.go +++ b/internal/service/dynamodb/table.go @@ -22,6 +22,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" + "github.com/hashicorp/terraform-provider-aws/internal/conns" "github.com/hashicorp/terraform-provider-aws/internal/create" "github.com/hashicorp/terraform-provider-aws/internal/errs/sdkdiag" @@ -1795,25 +1796,6 @@ func replicaPITR(ctx context.Context, conn *dynamodb.DynamoDB, tableName string, return enabled, nil } -func replicaStream(ctx context.Context, conn *dynamodb.DynamoDB, tableName string, region string, tfVersion string) (string, string) { - // This does not return an error because it is attempting to add "Computed"-only information to replica - tolerating errors. - session, err := conns.NewSessionForRegion(&conn.Config, region, tfVersion) - if err != nil { - log.Printf("[WARN] Attempting to get replica (%s) stream information, ignoring encountered error: %s", tableName, err) - return "", "" - } - - conn = dynamodb.New(session) - - table, err := FindTableByName(ctx, conn, tableName) - if err != nil { - log.Printf("[WARN] When attempting to get replica (%s) stream information, ignoring encountered error: %s", tableName, err) - return "", "" - } - - return aws.StringValue(table.LatestStreamArn), aws.StringValue(table.LatestStreamLabel) -} - func addReplicaPITRs(ctx context.Context, conn *dynamodb.DynamoDB, tableName string, tfVersion string, replicas []interface{}) ([]interface{}, error) { // This non-standard approach is needed because PITR info for a replica // must come from a region-specific connection. @@ -1844,9 +1826,28 @@ func enrichReplicas(ctx context.Context, conn *dynamodb.DynamoDB, arn, tableName } replica[names.AttrARN] = newARN - streamARN, streamLabel := replicaStream(ctx, conn, tableName, replica["region_name"].(string), tfVersion) - replica["stream_arn"] = streamARN - replica["stream_label"] = streamLabel + session, err := conns.NewSessionForRegion(&conn.Config, replica["region_name"].(string), tfVersion) + if err != nil { + log.Printf("[WARN] Attempting to get replica (%s) stream information, ignoring encountered error: %s", tableName, err) + return nil, fmt.Errorf("") + } + + conn = dynamodb.New(session) + + table, err := FindTableByName(ctx, conn, tableName) + if err != nil { + log.Printf("[WARN] When attempting to get replica (%s) stream information, ignoring encountered error: %s", tableName, err) + return nil, fmt.Errorf("") + } + + replica["stream_arn"] = aws.StringValue(table.LatestStreamArn) + replica["stream_label"] = aws.StringValue(table.LatestStreamLabel) + + if table.SSEDescription != nil { + log.Printf("[WARN] in enrichReplicas setting KMS key arn") + replica[names.AttrKMSKeyARN] = aws.StringValue(table.SSEDescription.KMSMasterKeyArn) + } + replicas[i] = replica } From 58e1bd7783359436444d97973ef2c35bf196fc7e Mon Sep 17 00:00:00 2001 From: Steve Zesch Date: Sat, 3 Feb 2024 12:03:47 -0500 Subject: [PATCH 02/16] Added changelog for 35630 --- .changelog/35630.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/35630.txt diff --git a/.changelog/35630.txt b/.changelog/35630.txt new file mode 100644 index 00000000000..36419b68564 --- /dev/null +++ b/.changelog/35630.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/aws_dynamodb_table: Make DescribeTable call per replica +``` From d65d7e486ce02bd16b0824bf66e91776df715059 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 6 Feb 2024 13:51:53 -0500 Subject: [PATCH 03/16] r/aws_dynamodb_kinesis_streaming_destination: Tidy up. --- internal/service/dynamodb/errors.go | 8 + internal/service/dynamodb/exports_test.go | 5 +- internal/service/dynamodb/find.go | 31 --- .../dynamodb/kinesis_streaming_destination.go | 201 ++++++++++++++---- .../kinesis_streaming_destination_test.go | 44 +--- .../service/dynamodb/service_package_gen.go | 3 +- internal/service/dynamodb/status.go | 16 -- internal/service/dynamodb/wait.go | 44 +--- 8 files changed, 186 insertions(+), 166 deletions(-) create mode 100644 internal/service/dynamodb/errors.go diff --git a/internal/service/dynamodb/errors.go b/internal/service/dynamodb/errors.go new file mode 100644 index 00000000000..016f4d15e7b --- /dev/null +++ b/internal/service/dynamodb/errors.go @@ -0,0 +1,8 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package dynamodb + +const ( + errCodeValidationException = "ValidationException" +) diff --git a/internal/service/dynamodb/exports_test.go b/internal/service/dynamodb/exports_test.go index b27e345cdb2..5a5693e643f 100644 --- a/internal/service/dynamodb/exports_test.go +++ b/internal/service/dynamodb/exports_test.go @@ -5,5 +5,8 @@ package dynamodb // Exports for use in tests only. var ( - ListTags = listTags + ResourceKinesisStreamingDestination = resourceKinesisStreamingDestination + + FindKinesisDataStreamDestinationByTwoPartKey = findKinesisDataStreamDestinationByTwoPartKey + ListTags = listTags ) diff --git a/internal/service/dynamodb/find.go b/internal/service/dynamodb/find.go index 0a8f99d13a7..a79a9bca91d 100644 --- a/internal/service/dynamodb/find.go +++ b/internal/service/dynamodb/find.go @@ -13,37 +13,6 @@ import ( "github.com/hashicorp/terraform-provider-aws/internal/tfresource" ) -func FindKinesisDataStreamDestination(ctx context.Context, conn *dynamodb.DynamoDB, streamArn, tableName string) (*dynamodb.KinesisDataStreamDestination, error) { - input := &dynamodb.DescribeKinesisStreamingDestinationInput{ - TableName: aws.String(tableName), - } - - output, err := conn.DescribeKinesisStreamingDestinationWithContext(ctx, input) - - if err != nil { - return nil, err - } - - if output == nil { - return nil, nil - } - - var result *dynamodb.KinesisDataStreamDestination - - for _, destination := range output.KinesisDataStreamDestinations { - if destination == nil { - continue - } - - if aws.StringValue(destination.StreamArn) == streamArn { - result = destination - break - } - } - - return result, nil -} - func FindTableByName(ctx context.Context, conn *dynamodb.DynamoDB, name string) (*dynamodb.TableDescription, error) { input := &dynamodb.DescribeTableInput{ TableName: aws.String(name), diff --git a/internal/service/dynamodb/kinesis_streaming_destination.go b/internal/service/dynamodb/kinesis_streaming_destination.go index 99d38d55ecf..27991cd7d66 100644 --- a/internal/service/dynamodb/kinesis_streaming_destination.go +++ b/internal/service/dynamodb/kinesis_streaming_destination.go @@ -5,22 +5,31 @@ package dynamodb import ( "context" - "fmt" + "errors" "log" - "strings" + "time" "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/diag" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-provider-aws/internal/conns" + "github.com/hashicorp/terraform-provider-aws/internal/errs" "github.com/hashicorp/terraform-provider-aws/internal/errs/sdkdiag" + "github.com/hashicorp/terraform-provider-aws/internal/flex" + tfslices "github.com/hashicorp/terraform-provider-aws/internal/slices" + "github.com/hashicorp/terraform-provider-aws/internal/tfresource" "github.com/hashicorp/terraform-provider-aws/internal/verify" ) -// @SDKResource("aws_dynamodb_kinesis_streaming_destination") -func ResourceKinesisStreamingDestination() *schema.Resource { +const ( + kinesisStreamingDestinationResourceIDPartCount = 2 +) + +// @SDKResource("aws_dynamodb_kinesis_streaming_destination", name="Kinesis Streaming Destination") +func resourceKinesisStreamingDestination() *schema.Resource { return &schema.Resource{ CreateWithoutTimeout: resourceKinesisStreamingDestinationCreate, ReadWithoutTimeout: resourceKinesisStreamingDestinationRead, @@ -51,63 +60,49 @@ func resourceKinesisStreamingDestinationCreate(ctx context.Context, d *schema.Re conn := meta.(*conns.AWSClient).DynamoDBConn(ctx) - streamArn := d.Get("stream_arn").(string) + streamARN := d.Get("stream_arn").(string) tableName := d.Get("table_name").(string) - + id := errs.Must(flex.FlattenResourceId([]string{tableName, streamARN}, kinesisStreamingDestinationResourceIDPartCount, false)) input := &dynamodb.EnableKinesisStreamingDestinationInput{ - StreamArn: aws.String(streamArn), + StreamArn: aws.String(streamARN), TableName: aws.String(tableName), } - output, err := conn.EnableKinesisStreamingDestinationWithContext(ctx, input) + _, err := conn.EnableKinesisStreamingDestinationWithContext(ctx, input) if err != nil { - return sdkdiag.AppendErrorf(diags, "enabling DynamoDB Kinesis streaming destination (stream: %s, table: %s): %s", streamArn, tableName, err) + return sdkdiag.AppendErrorf(diags, "enabling DynamoDB Kinesis Streaming Destination (%s): %s", id, err) } - if output == nil { - return sdkdiag.AppendErrorf(diags, "enabling DynamoDB Kinesis streaming destination (stream: %s, table: %s): empty output", streamArn, tableName) - } + d.SetId(id) - if err := waitKinesisStreamingDestinationActive(ctx, conn, streamArn, tableName); err != nil { - return sdkdiag.AppendErrorf(diags, "waiting for DynamoDB Kinesis streaming destination (stream: %s, table: %s) to be active: %s", streamArn, tableName, err) + if _, err := waitKinesisStreamingDestinationActive(ctx, conn, streamARN, tableName); err != nil { + return sdkdiag.AppendErrorf(diags, "waiting for DynamoDB Kinesis Streaming Destination (%s) create: %s", d.Id(), err) } - d.SetId(fmt.Sprintf("%s,%s", aws.StringValue(output.TableName), aws.StringValue(output.StreamArn))) - return append(diags, resourceKinesisStreamingDestinationRead(ctx, d, meta)...) } func resourceKinesisStreamingDestinationRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { var diags diag.Diagnostics - conn := meta.(*conns.AWSClient).DynamoDBConn(ctx) - tableName, streamArn, err := KinesisStreamingDestinationParseID(d.Id()) - + parts, err := flex.ExpandResourceId(d.Id(), kinesisStreamingDestinationResourceIDPartCount, false) if err != nil { return sdkdiag.AppendFromErr(diags, err) } - output, err := FindKinesisDataStreamDestination(ctx, conn, streamArn, tableName) + tableName, streamARN := parts[0], parts[1] + output, err := findKinesisDataStreamDestinationByTwoPartKey(ctx, conn, streamARN, tableName) - if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, dynamodb.ErrCodeResourceNotFoundException) { - log.Printf("[WARN] DynamoDB Kinesis Streaming Destination (stream: %s, table: %s) not found, removing from state", streamArn, tableName) + if !d.IsNewResource() && tfresource.NotFound(err) { + log.Printf("[WARN] DynamoDB Kinesis Streaming Destination (%s) not found, removing from state", d.Id()) d.SetId("") return diags } if err != nil { - return sdkdiag.AppendErrorf(diags, "retrieving DynamoDB Kinesis streaming destination (stream: %s, table: %s): %s", streamArn, tableName, err) - } - - if output == nil || aws.StringValue(output.DestinationStatus) == dynamodb.DestinationStatusDisabled { - if d.IsNewResource() { - return sdkdiag.AppendErrorf(diags, "retrieving DynamoDB Kinesis streaming destination (stream: %s, table: %s): empty output after creation", streamArn, tableName) - } - log.Printf("[WARN] DynamoDB Kinesis Streaming Destination (stream: %s, table: %s) not found, removing from state", streamArn, tableName) - d.SetId("") - return diags + return diag.Errorf("reading DynamoDB Kinesis Streaming Destination (%s): %s", d.Id(), err) } d.Set("stream_arn", output.StreamArn) @@ -121,36 +116,150 @@ func resourceKinesisStreamingDestinationDelete(ctx context.Context, d *schema.Re conn := meta.(*conns.AWSClient).DynamoDBConn(ctx) - tableName, streamArn, err := KinesisStreamingDestinationParseID(d.Id()) - + parts, err := flex.ExpandResourceId(d.Id(), kinesisStreamingDestinationResourceIDPartCount, false) if err != nil { return sdkdiag.AppendFromErr(diags, err) } - input := &dynamodb.DisableKinesisStreamingDestinationInput{ - TableName: aws.String(tableName), - StreamArn: aws.String(streamArn), + tableName, streamARN := parts[0], parts[1] + _, err = findKinesisDataStreamDestinationByTwoPartKey(ctx, conn, streamARN, tableName) + + if tfresource.NotFound(err) { + return diags } - _, err = conn.DisableKinesisStreamingDestinationWithContext(ctx, input) + log.Printf("[DEBUG] Deleting DynamoDB Kinesis Streaming Destination: %s", d.Id()) + _, err = conn.DisableKinesisStreamingDestinationWithContext(ctx, &dynamodb.DisableKinesisStreamingDestinationInput{ + TableName: aws.String(tableName), + StreamArn: aws.String(streamARN), + }) + + if tfawserr.ErrCodeEquals(err, dynamodb.ErrCodeResourceNotFoundException) { + return diags + } if err != nil { - return sdkdiag.AppendErrorf(diags, "disabling DynamoDB Kinesis streaming destination (stream: %s, table: %s): %s", streamArn, tableName, err) + return sdkdiag.AppendErrorf(diags, "disabling DynamoDB Kinesis Streaming Destination (%s): %s", d.Id(), err) } - if err := waitKinesisStreamingDestinationDisabled(ctx, conn, streamArn, tableName); err != nil { - return sdkdiag.AppendErrorf(diags, "waiting for DynamoDB Kinesis streaming destination (stream: %s, table: %s) to be disabled: %s", streamArn, tableName, err) + if _, err := waitKinesisStreamingDestinationDisabled(ctx, conn, streamARN, tableName); err != nil { + return sdkdiag.AppendErrorf(diags, "waiting for DynamoDB Kinesis Streaming Destination (%s) delete: %s", d.Id(), err) } return diags } -func KinesisStreamingDestinationParseID(id string) (string, string, error) { - parts := strings.SplitN(id, ",", 2) +func kinesisDataStreamDestinationForStream(arn string) tfslices.Predicate[*dynamodb.KinesisDataStreamDestination] { + return func(v *dynamodb.KinesisDataStreamDestination) bool { + return aws.StringValue(v.StreamArn) == arn + } +} + +func findKinesisDataStreamDestinationByTwoPartKey(ctx context.Context, conn *dynamodb.DynamoDB, streamARN, tableName string) (*dynamodb.KinesisDataStreamDestination, error) { + input := &dynamodb.DescribeKinesisStreamingDestinationInput{ + TableName: aws.String(tableName), + } + output, err := findKinesisDataStreamDestination(ctx, conn, input, kinesisDataStreamDestinationForStream(streamARN)) + + if err != nil { + return nil, err + } + + if aws.StringValue(output.DestinationStatus) == dynamodb.DestinationStatusDisabled { + return nil, &retry.NotFoundError{} + } + + return output, nil +} + +func findKinesisDataStreamDestination(ctx context.Context, conn *dynamodb.DynamoDB, input *dynamodb.DescribeKinesisStreamingDestinationInput, filter tfslices.Predicate[*dynamodb.KinesisDataStreamDestination]) (*dynamodb.KinesisDataStreamDestination, error) { + output, err := findKinesisDataStreamDestinations(ctx, conn, input, filter) + + if err != nil { + return nil, err + } + + return tfresource.AssertSinglePtrResult(output) +} + +func findKinesisDataStreamDestinations(ctx context.Context, conn *dynamodb.DynamoDB, input *dynamodb.DescribeKinesisStreamingDestinationInput, filter tfslices.Predicate[*dynamodb.KinesisDataStreamDestination]) ([]*dynamodb.KinesisDataStreamDestination, error) { + output, err := conn.DescribeKinesisStreamingDestinationWithContext(ctx, input) + + if tfawserr.ErrCodeEquals(err, dynamodb.ErrCodeResourceNotFoundException) { + return nil, &retry.NotFoundError{ + LastError: err, + LastRequest: input, + } + } + + if err != nil { + return nil, err + } + + if output == nil { + return nil, tfresource.NewEmptyResultError(input) + } + + return tfslices.Filter(output.KinesisDataStreamDestinations, filter), nil +} + +func statusKinesisStreamingDestination(ctx context.Context, conn *dynamodb.DynamoDB, streamARN, tableName string) retry.StateRefreshFunc { + return func() (interface{}, string, error) { + input := &dynamodb.DescribeKinesisStreamingDestinationInput{ + TableName: aws.String(tableName), + } + output, err := findKinesisDataStreamDestination(ctx, conn, input, kinesisDataStreamDestinationForStream(streamARN)) + + if tfresource.NotFound(err) { + return nil, "", nil + } + + if err != nil { + return nil, "", err + } + + return output, aws.StringValue(output.DestinationStatus), nil + } +} + +func waitKinesisStreamingDestinationActive(ctx context.Context, conn *dynamodb.DynamoDB, streamARN, tableName string) (*dynamodb.KinesisDataStreamDestination, error) { + const ( + timeout = 5 * time.Minute + ) + stateConf := &retry.StateChangeConf{ + Pending: []string{dynamodb.DestinationStatusDisabled, dynamodb.DestinationStatusEnabling}, + Target: []string{dynamodb.DestinationStatusActive}, + Timeout: timeout, + Refresh: statusKinesisStreamingDestination(ctx, conn, streamARN, tableName), + } + + outputRaw, err := stateConf.WaitForStateContext(ctx) + + if output, ok := outputRaw.(*dynamodb.KinesisDataStreamDestination); ok { + tfresource.SetLastError(err, errors.New(aws.StringValue(output.DestinationStatusDescription))) + return output, err + } + + return nil, err +} + +func waitKinesisStreamingDestinationDisabled(ctx context.Context, conn *dynamodb.DynamoDB, streamARN, tableName string) (*dynamodb.KinesisDataStreamDestination, error) { + const ( + timeout = 5 * time.Minute + ) + stateConf := &retry.StateChangeConf{ + Pending: []string{dynamodb.DestinationStatusActive, dynamodb.DestinationStatusDisabling}, + Target: []string{dynamodb.DestinationStatusDisabled}, + Timeout: timeout, + Refresh: statusKinesisStreamingDestination(ctx, conn, streamARN, tableName), + } + + outputRaw, err := stateConf.WaitForStateContext(ctx) - if len(parts) != 2 || parts[0] == "" || parts[1] == "" { - return "", "", fmt.Errorf("unexpected format of ID (%s), expected TABLE_NAME,STREAM_ARN", id) + if output, ok := outputRaw.(*dynamodb.KinesisDataStreamDestination); ok { + tfresource.SetLastError(err, errors.New(aws.StringValue(output.DestinationStatusDescription))) + return output, err } - return parts[0], parts[1], nil + return nil, err } diff --git a/internal/service/dynamodb/kinesis_streaming_destination_test.go b/internal/service/dynamodb/kinesis_streaming_destination_test.go index 3c083929c54..363c29acd7c 100644 --- a/internal/service/dynamodb/kinesis_streaming_destination_test.go +++ b/internal/service/dynamodb/kinesis_streaming_destination_test.go @@ -10,13 +10,13 @@ import ( "github.com/YakDriver/regexache" "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-testing/helper/acctest" "github.com/hashicorp/terraform-plugin-testing/helper/resource" "github.com/hashicorp/terraform-plugin-testing/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 TestAccDynamoDBKinesisStreamingDestination_basic(t *testing.T) { @@ -121,36 +121,18 @@ resource "aws_dynamodb_kinesis_streaming_destination" "test" { `, rName) } -func testAccCheckKinesisStreamingDestinationExists(ctx context.Context, resourceName string) resource.TestCheckFunc { +func testAccCheckKinesisStreamingDestinationExists(ctx context.Context, n string) resource.TestCheckFunc { return func(s *terraform.State) error { - rs, ok := s.RootModule().Resources[resourceName] + rs, ok := s.RootModule().Resources[n] if !ok { - return fmt.Errorf("not found: %s", resourceName) - } - - if rs.Primary.ID == "" { - return fmt.Errorf("no ID is set") - } - - tableName, streamArn, err := tfdynamodb.KinesisStreamingDestinationParseID(rs.Primary.ID) - - if err != nil { - return err + return fmt.Errorf("Not found: %s", n) } conn := acctest.Provider.Meta().(*conns.AWSClient).DynamoDBConn(ctx) - output, err := tfdynamodb.FindKinesisDataStreamDestination(ctx, conn, streamArn, tableName) - - if err != nil { - return err - } - - if output == nil { - return fmt.Errorf("DynamoDB Kinesis Streaming Destination (%s) not found", rs.Primary.ID) - } + _, err := tfdynamodb.FindKinesisDataStreamDestinationByTwoPartKey(ctx, conn, rs.Primary.Attributes["stream_arn"], rs.Primary.Attributes["table_name"]) - return nil + return err } } @@ -163,15 +145,9 @@ func testAccCheckKinesisStreamingDestinationDestroy(ctx context.Context) resourc continue } - tableName, streamArn, err := tfdynamodb.KinesisStreamingDestinationParseID(rs.Primary.ID) - - if err != nil { - return err - } - - output, err := tfdynamodb.FindKinesisDataStreamDestination(ctx, conn, streamArn, tableName) + _, err := tfdynamodb.FindKinesisDataStreamDestinationByTwoPartKey(ctx, conn, rs.Primary.Attributes["stream_arn"], rs.Primary.Attributes["table_name"]) - if tfawserr.ErrCodeEquals(err, dynamodb.ErrCodeResourceNotFoundException) { + if tfresource.NotFound(err) { continue } @@ -179,9 +155,7 @@ func testAccCheckKinesisStreamingDestinationDestroy(ctx context.Context) resourc return err } - if output != nil { - return fmt.Errorf("DynamoDB Kinesis Streaming Destination (%s) still exists", rs.Primary.ID) - } + return fmt.Errorf("DynamoDB Kinesis Streaming Destination %s still exists", rs.Primary.ID) } return nil diff --git a/internal/service/dynamodb/service_package_gen.go b/internal/service/dynamodb/service_package_gen.go index 59299d79838..cb86dba0df0 100644 --- a/internal/service/dynamodb/service_package_gen.go +++ b/internal/service/dynamodb/service_package_gen.go @@ -47,8 +47,9 @@ func (p *servicePackage) SDKResources(ctx context.Context) []*types.ServicePacka TypeName: "aws_dynamodb_global_table", }, { - Factory: ResourceKinesisStreamingDestination, + Factory: resourceKinesisStreamingDestination, TypeName: "aws_dynamodb_kinesis_streaming_destination", + Name: "Kinesis Streaming Destination", }, { Factory: ResourceTable, diff --git a/internal/service/dynamodb/status.go b/internal/service/dynamodb/status.go index ed06fafaf84..f40a295254a 100644 --- a/internal/service/dynamodb/status.go +++ b/internal/service/dynamodb/status.go @@ -13,22 +13,6 @@ import ( "github.com/hashicorp/terraform-provider-aws/internal/tfresource" ) -func statusKinesisStreamingDestination(ctx context.Context, conn *dynamodb.DynamoDB, streamArn, tableName string) retry.StateRefreshFunc { - return func() (interface{}, string, error) { - result, err := FindKinesisDataStreamDestination(ctx, conn, streamArn, tableName) - - if err != nil { - return nil, "", err - } - - if result == nil { - return nil, "", nil - } - - return result, aws.StringValue(result.DestinationStatus), nil - } -} - func statusTable(ctx context.Context, conn *dynamodb.DynamoDB, tableName string) retry.StateRefreshFunc { return func() (interface{}, string, error) { table, err := FindTableByName(ctx, conn, tableName) diff --git a/internal/service/dynamodb/wait.go b/internal/service/dynamodb/wait.go index 5a1b44a571f..0cd7c247ea7 100644 --- a/internal/service/dynamodb/wait.go +++ b/internal/service/dynamodb/wait.go @@ -14,16 +14,14 @@ import ( ) const ( - kinesisStreamingDestinationActiveTimeout = 5 * time.Minute - kinesisStreamingDestinationDisabledTimeout = 5 * time.Minute - createTableTimeout = 30 * time.Minute - updateTableTimeoutTotal = 60 * time.Minute - replicaUpdateTimeout = 30 * time.Minute - updateTableTimeout = 20 * time.Minute - updateTableContinuousBackupsTimeout = 20 * time.Minute - deleteTableTimeout = 10 * time.Minute - pitrUpdateTimeout = 30 * time.Second - ttlUpdateTimeout = 30 * time.Second + createTableTimeout = 30 * time.Minute + updateTableTimeoutTotal = 60 * time.Minute + replicaUpdateTimeout = 30 * time.Minute + updateTableTimeout = 20 * time.Minute + updateTableContinuousBackupsTimeout = 20 * time.Minute + deleteTableTimeout = 10 * time.Minute + pitrUpdateTimeout = 30 * time.Second + ttlUpdateTimeout = 30 * time.Second ) func maxDuration(a, b time.Duration) time.Duration { @@ -34,32 +32,6 @@ func maxDuration(a, b time.Duration) time.Duration { return b } -func waitKinesisStreamingDestinationActive(ctx context.Context, conn *dynamodb.DynamoDB, streamArn, tableName string) error { - stateConf := &retry.StateChangeConf{ - Pending: []string{dynamodb.DestinationStatusDisabled, dynamodb.DestinationStatusEnabling}, - Target: []string{dynamodb.DestinationStatusActive}, - Timeout: kinesisStreamingDestinationActiveTimeout, - Refresh: statusKinesisStreamingDestination(ctx, conn, streamArn, tableName), - } - - _, err := stateConf.WaitForStateContext(ctx) - - return err -} - -func waitKinesisStreamingDestinationDisabled(ctx context.Context, conn *dynamodb.DynamoDB, streamArn, tableName string) error { - stateConf := &retry.StateChangeConf{ - Pending: []string{dynamodb.DestinationStatusActive, dynamodb.DestinationStatusDisabling}, - Target: []string{dynamodb.DestinationStatusDisabled}, - Timeout: kinesisStreamingDestinationDisabledTimeout, - Refresh: statusKinesisStreamingDestination(ctx, conn, streamArn, tableName), - } - - _, err := stateConf.WaitForStateContext(ctx) - - return err -} - func waitTableActive(ctx context.Context, conn *dynamodb.DynamoDB, tableName string, timeout time.Duration) (*dynamodb.TableDescription, error) { stateConf := &retry.StateChangeConf{ Pending: []string{dynamodb.TableStatusCreating, dynamodb.TableStatusUpdating}, From 3de522d9d34d53638f117f4c1b25aeda800fe8a9 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 6 Feb 2024 13:52:16 -0500 Subject: [PATCH 04/16] Acceptance test output: % make testacc TESTARGS='-run=TestAccDynamoDBKinesisStreamingDestination_' PKG=dynamodb ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./internal/service/dynamodb/... -v -count 1 -parallel 20 -run=TestAccDynamoDBKinesisStreamingDestination_ -timeout 360m === RUN TestAccDynamoDBKinesisStreamingDestination_basic === PAUSE TestAccDynamoDBKinesisStreamingDestination_basic === RUN TestAccDynamoDBKinesisStreamingDestination_disappears === PAUSE TestAccDynamoDBKinesisStreamingDestination_disappears === RUN TestAccDynamoDBKinesisStreamingDestination_Disappears_dynamoDBTable === PAUSE TestAccDynamoDBKinesisStreamingDestination_Disappears_dynamoDBTable === CONT TestAccDynamoDBKinesisStreamingDestination_basic === CONT TestAccDynamoDBKinesisStreamingDestination_disappears === CONT TestAccDynamoDBKinesisStreamingDestination_Disappears_dynamoDBTable --- PASS: TestAccDynamoDBKinesisStreamingDestination_disappears (60.52s) --- PASS: TestAccDynamoDBKinesisStreamingDestination_basic (61.20s) --- PASS: TestAccDynamoDBKinesisStreamingDestination_Disappears_dynamoDBTable (66.23s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/dynamodb 76.104s From 0fa31a4107aac651a3c5fbe88d70f370bf1f0cd0 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 6 Feb 2024 14:03:01 -0500 Subject: [PATCH 05/16] r/aws_dynamodb_table_replica: Handle deletion of non-existant replica. --- internal/service/dynamodb/table_replica.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/internal/service/dynamodb/table_replica.go b/internal/service/dynamodb/table_replica.go index 23a2ad8782c..552b9722afc 100644 --- a/internal/service/dynamodb/table_replica.go +++ b/internal/service/dynamodb/table_replica.go @@ -503,6 +503,10 @@ func resourceTableReplicaDelete(ctx context.Context, d *schema.ResourceData, met _, err = conn.UpdateTableWithContext(ctx, input) } + if tfawserr.ErrMessageContains(err, errCodeValidationException, "Replica specified in the Replica Update or Replica Delete action of the request was not found") { + return diags + } + if err != nil { return create.AppendDiagError(diags, names.DynamoDB, create.ErrActionDeleting, ResNameTableReplica, d.Id(), err) } From a4ca585bfd3c60cd844f477e9230284f39eda35e Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 6 Feb 2024 14:27:37 -0500 Subject: [PATCH 06/16] Acceptance test output: % make testacc TESTARGS='-run=TestAccDynamoDBTableReplica_' PKG=dynamodb ACCTEST_PARALLELISM=2 ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./internal/service/dynamodb/... -v -count 1 -parallel 2 -run=TestAccDynamoDBTableReplica_ -timeout 360m === RUN TestAccDynamoDBTableReplica_basic === PAUSE TestAccDynamoDBTableReplica_basic === RUN TestAccDynamoDBTableReplica_disappears === PAUSE TestAccDynamoDBTableReplica_disappears === RUN TestAccDynamoDBTableReplica_pitr === PAUSE TestAccDynamoDBTableReplica_pitr === RUN TestAccDynamoDBTableReplica_pitrKMS === PAUSE TestAccDynamoDBTableReplica_pitrKMS === RUN TestAccDynamoDBTableReplica_pitrDefault === PAUSE TestAccDynamoDBTableReplica_pitrDefault === RUN TestAccDynamoDBTableReplica_tags === PAUSE TestAccDynamoDBTableReplica_tags === RUN TestAccDynamoDBTableReplica_tableClass === PAUSE TestAccDynamoDBTableReplica_tableClass === RUN TestAccDynamoDBTableReplica_keys === PAUSE TestAccDynamoDBTableReplica_keys === CONT TestAccDynamoDBTableReplica_basic === CONT TestAccDynamoDBTableReplica_pitrDefault --- PASS: TestAccDynamoDBTableReplica_basic (193.02s) === CONT TestAccDynamoDBTableReplica_tableClass --- PASS: TestAccDynamoDBTableReplica_pitrDefault (212.09s) === CONT TestAccDynamoDBTableReplica_keys --- PASS: TestAccDynamoDBTableReplica_tableClass (498.59s) === CONT TestAccDynamoDBTableReplica_pitr --- PASS: TestAccDynamoDBTableReplica_keys (494.96s) === CONT TestAccDynamoDBTableReplica_pitrKMS --- PASS: TestAccDynamoDBTableReplica_pitrKMS (255.85s) === CONT TestAccDynamoDBTableReplica_tags --- PASS: TestAccDynamoDBTableReplica_pitr (281.19s) === CONT TestAccDynamoDBTableReplica_disappears --- PASS: TestAccDynamoDBTableReplica_disappears (210.81s) --- PASS: TestAccDynamoDBTableReplica_tags (353.57s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/dynamodb 1327.115s From b7d84732ae8ab52d47efad721500d3b37e5bb04e Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 6 Feb 2024 14:30:36 -0500 Subject: [PATCH 07/16] Fix 'TestAccDynamoDBTable_BillingModeGSI_payPerRequestToProvisioned'. --- internal/service/dynamodb/table_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/internal/service/dynamodb/table_test.go b/internal/service/dynamodb/table_test.go index 48dbd842a7e..a113e3d8a08 100644 --- a/internal/service/dynamodb/table_test.go +++ b/internal/service/dynamodb/table_test.go @@ -784,9 +784,10 @@ func TestAccDynamoDBTable_BillingModeGSI_payPerRequestToProvisioned(t *testing.T ), }, { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"global_secondary_index"}, }, { Config: testAccTableConfig_billingProvisionedGSI(rName), From 573ecf925f5e854679cfe9f4d658e9ef368858a7 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 6 Feb 2024 14:35:40 -0500 Subject: [PATCH 08/16] Fix 'TestAccDynamoDBTable_Disappears_payPerRequestWithGSI'. --- internal/service/dynamodb/table_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/internal/service/dynamodb/table_test.go b/internal/service/dynamodb/table_test.go index a113e3d8a08..e1dfa75ee80 100644 --- a/internal/service/dynamodb/table_test.go +++ b/internal/service/dynamodb/table_test.go @@ -475,9 +475,10 @@ func TestAccDynamoDBTable_Disappears_payPerRequestWithGSI(t *testing.T) { ), }, { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"global_secondary_index"}, }, }, }) From 5276a3b38ccde84bfbf366200c75ad02d59b0e82 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 6 Feb 2024 14:49:49 -0500 Subject: [PATCH 09/16] r/aws_dynamodb_table: Correct 'acctest.PreCheckMultipleRegion'. --- internal/service/dynamodb/table_test.go | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/internal/service/dynamodb/table_test.go b/internal/service/dynamodb/table_test.go index e1dfa75ee80..5025be08cc4 100644 --- a/internal/service/dynamodb/table_test.go +++ b/internal/service/dynamodb/table_test.go @@ -1874,7 +1874,7 @@ func TestAccDynamoDBTable_Replica_pitr(t *testing.T) { resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(ctx, t) - acctest.PreCheckMultipleRegion(t, 2) + acctest.PreCheckMultipleRegion(t, 3) }, ErrorCheck: acctest.ErrorCheck(t, dynamodb.EndpointsID), ProtoV5ProviderFactories: acctest.ProtoV5FactoriesMultipleRegions(ctx, t, 3), // 3 due to shared test configuration @@ -1937,7 +1937,7 @@ func TestAccDynamoDBTable_Replica_pitrKMS(t *testing.T) { resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(ctx, t) - acctest.PreCheckMultipleRegion(t, 2) + acctest.PreCheckMultipleRegion(t, 3) }, ErrorCheck: acctest.ErrorCheck(t, dynamodb.EndpointsID), ProtoV5ProviderFactories: acctest.ProtoV5FactoriesMultipleRegions(ctx, t, 3), // 3 due to shared test configuration @@ -2063,7 +2063,7 @@ func TestAccDynamoDBTable_Replica_tagsOneOfTwo(t *testing.T) { resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(ctx, t) - acctest.PreCheckMultipleRegion(t, 2) + acctest.PreCheckMultipleRegion(t, 3) }, ErrorCheck: acctest.ErrorCheck(t, dynamodb.EndpointsID), ProtoV5ProviderFactories: acctest.ProtoV5FactoriesMultipleRegions(ctx, t, 3), @@ -2103,7 +2103,7 @@ func TestAccDynamoDBTable_Replica_tagsTwoOfTwo(t *testing.T) { resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(ctx, t) - acctest.PreCheckMultipleRegion(t, 2) + acctest.PreCheckMultipleRegion(t, 3) }, ErrorCheck: acctest.ErrorCheck(t, dynamodb.EndpointsID), ProtoV5ProviderFactories: acctest.ProtoV5FactoriesMultipleRegions(ctx, t, 3), @@ -2141,7 +2141,10 @@ func TestAccDynamoDBTable_Replica_tagsNext(t *testing.T) { rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(ctx, t); acctest.PreCheckMultipleRegion(t, 2) }, + PreCheck: func() { + acctest.PreCheck(ctx, t) + acctest.PreCheckMultipleRegion(t, 3) + }, ErrorCheck: acctest.ErrorCheck(t, dynamodb.EndpointsID), ProtoV5ProviderFactories: acctest.ProtoV5FactoriesMultipleRegions(ctx, t, 3), CheckDestroy: testAccCheckTableDestroy(ctx), @@ -2236,7 +2239,10 @@ func TestAccDynamoDBTable_Replica_tagsUpdate(t *testing.T) { rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(ctx, t); acctest.PreCheckMultipleRegion(t, 2) }, + PreCheck: func() { + acctest.PreCheck(ctx, t) + acctest.PreCheckMultipleRegion(t, 3) + }, ErrorCheck: acctest.ErrorCheck(t, dynamodb.EndpointsID), ProtoV5ProviderFactories: acctest.ProtoV5FactoriesMultipleRegions(ctx, t, 3), CheckDestroy: testAccCheckTableDestroy(ctx), From 203b79a61fce949e4c78f48d2953a0ce60a47c5c Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 6 Feb 2024 15:33:23 -0500 Subject: [PATCH 10/16] TestAccDynamoDBTable_Replica_singleAddCMK: Tidy up. --- internal/service/dynamodb/table_test.go | 96 +++++++++++-------------- 1 file changed, 42 insertions(+), 54 deletions(-) diff --git a/internal/service/dynamodb/table_test.go b/internal/service/dynamodb/table_test.go index 5025be08cc4..3894759789b 100644 --- a/internal/service/dynamodb/table_test.go +++ b/internal/service/dynamodb/table_test.go @@ -1640,7 +1640,7 @@ func TestAccDynamoDBTable_Replica_single(t *testing.T) { resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(ctx, t) - acctest.PreCheckMultipleRegion(t, 2) + acctest.PreCheckMultipleRegion(t, 3) }, ErrorCheck: acctest.ErrorCheck(t, dynamodb.EndpointsID), ProtoV5ProviderFactories: acctest.ProtoV5FactoriesMultipleRegions(ctx, t, 3), // 3 due to shared test configuration @@ -1698,7 +1698,7 @@ func TestAccDynamoDBTable_Replica_singleStreamSpecification(t *testing.T) { resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(ctx, t) - acctest.PreCheckMultipleRegion(t, 2) + acctest.PreCheckMultipleRegion(t, 3) }, ErrorCheck: acctest.ErrorCheck(t, dynamodb.EndpointsID), ProtoV5ProviderFactories: acctest.ProtoV5FactoriesMultipleRegions(ctx, t, 3), // 3 due to shared test configuration @@ -1734,7 +1734,7 @@ func TestAccDynamoDBTable_Replica_singleDefaultKeyEncrypted(t *testing.T) { resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(ctx, t) - acctest.PreCheckMultipleRegion(t, 2) + acctest.PreCheckMultipleRegion(t, 3) }, ErrorCheck: acctest.ErrorCheck(t, dynamodb.EndpointsID), ProtoV5ProviderFactories: acctest.ProtoV5FactoriesMultipleRegions(ctx, t, 3), // 3 due to shared test configuration @@ -1773,13 +1773,13 @@ func TestAccDynamoDBTable_Replica_singleCMK(t *testing.T) { var conf dynamodb.TableDescription resourceName := "aws_dynamodb_table.test" kmsKeyResourceName := "aws_kms_key.test" - kmsKeyReplicaResourceName := "aws_kms_key.replica" + kmsKeyReplicaResourceName := "aws_kms_key.awsalternate" rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(ctx, t) - acctest.PreCheckMultipleRegion(t, 2) + acctest.PreCheckMultipleRegion(t, 3) }, ErrorCheck: acctest.ErrorCheck(t, dynamodb.EndpointsID), ProtoV5ProviderFactories: acctest.ProtoV5FactoriesMultipleRegions(ctx, t, 3), // 3 due to shared test configuration @@ -1808,14 +1808,14 @@ func TestAccDynamoDBTable_Replica_singleAddCMK(t *testing.T) { var conf dynamodb.TableDescription resourceName := "aws_dynamodb_table.test" kmsKeyResourceName := "aws_kms_key.test" - kmsKeyReplicaResourceName := "aws_kms_key.replica" - kmsKeyReplica2ResourceName := "aws_kms_key.replica2" + kmsKey1Replica1ResourceName := "aws_kms_key.awsalternate1" + kmsKey2Replica1ResourceName := "aws_kms_key.awsalternate2" rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(ctx, t) - acctest.PreCheckMultipleRegion(t, 2) + acctest.PreCheckMultipleRegion(t, 3) }, ErrorCheck: acctest.ErrorCheck(t, dynamodb.EndpointsID), ProtoV5ProviderFactories: acctest.ProtoV5FactoriesMultipleRegions(ctx, t, 3), // 3 due to shared test configuration @@ -1826,37 +1826,31 @@ func TestAccDynamoDBTable_Replica_singleAddCMK(t *testing.T) { Check: resource.ComposeAggregateTestCheckFunc( testAccCheckInitialTableExists(ctx, resourceName, &conf), resource.TestCheckResourceAttr(resourceName, "replica.#", "1"), - resource.TestCheckResourceAttr(resourceName, "server_side_encryption.0.enabled", "true"), resource.TestCheckResourceAttr(resourceName, "replica.0.kms_key_arn", ""), + resource.TestCheckResourceAttr(resourceName, "server_side_encryption.0.enabled", "true"), resource.TestCheckResourceAttr(resourceName, "server_side_encryption.0.kms_key_arn", ""), ), }, { - Config: testAccTableConfig_replicaCMKUpdate(rName, "replica"), + Config: testAccTableConfig_replicaCMKUpdate(rName, "awsalternate1"), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckInitialTableExists(ctx, resourceName, &conf), resource.TestCheckResourceAttr(resourceName, "replica.#", "1"), - resource.TestCheckResourceAttrPair(resourceName, "replica.0.kms_key_arn", kmsKeyReplicaResourceName, names.AttrARN), + resource.TestCheckResourceAttrPair(resourceName, "replica.0.kms_key_arn", kmsKey1Replica1ResourceName, names.AttrARN), resource.TestCheckResourceAttr(resourceName, "server_side_encryption.0.enabled", "true"), resource.TestCheckResourceAttrPair(resourceName, "server_side_encryption.0.kms_key_arn", kmsKeyResourceName, names.AttrARN), ), }, { - Config: testAccTableConfig_replicaCMKUpdate(rName, "replica2"), + Config: testAccTableConfig_replicaCMKUpdate(rName, "awsalternate2"), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckInitialTableExists(ctx, resourceName, &conf), resource.TestCheckResourceAttr(resourceName, "replica.#", "1"), - resource.TestCheckResourceAttrPair(resourceName, "replica.0.kms_key_arn", kmsKeyReplica2ResourceName, names.AttrARN), + resource.TestCheckResourceAttrPair(resourceName, "replica.0.kms_key_arn", kmsKey2Replica1ResourceName, names.AttrARN), resource.TestCheckResourceAttr(resourceName, "server_side_encryption.0.enabled", "true"), resource.TestCheckResourceAttrPair(resourceName, "server_side_encryption.0.kms_key_arn", kmsKeyResourceName, names.AttrARN), ), }, - { - Config: testAccTableConfig_replicaCMKUpdate(rName, "replica2"), - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, - }, }, }) } @@ -3722,15 +3716,9 @@ resource "aws_kms_key" "test" { deletion_window_in_days = 7 } -resource "aws_kms_key" "replica" { - provider = "awsalternate" - description = "%[1]s-2" - deletion_window_in_days = 7 -} - -resource "aws_kms_key" "replica2" { +resource "aws_kms_key" "awsalternate" { provider = "awsalternate" - description = "%[1]s-3" + description = %[1]q deletion_window_in_days = 7 } @@ -3748,7 +3736,7 @@ resource "aws_dynamodb_table" "test" { replica { region_name = data.aws_region.alternate.name - kms_key_arn = aws_kms_key.replica.arn + kms_key_arn = aws_kms_key.awsalternate.arn } server_side_encryption { @@ -3765,7 +3753,7 @@ resource "aws_dynamodb_table" "test" { `, rName)) } -func testAccTableConfig_replicaCMKUpdate(rName, key string) string { +func testAccTableConfig_replicaAmazonManagedKey(rName string) string { return acctest.ConfigCompose( acctest.ConfigMultipleRegionProvider(3), // Prevent "Provider configuration not present" errors fmt.Sprintf(` @@ -3773,21 +3761,13 @@ data "aws_region" "alternate" { provider = "awsalternate" } -resource "aws_kms_key" "test" { - description = %[1]q - deletion_window_in_days = 7 -} - -resource "aws_kms_key" "replica" { - provider = "awsalternate" - description = "%[1]s-2" - deletion_window_in_days = 7 +data "aws_kms_alias" "dynamodb" { + name = "alias/aws/dynamodb" } -resource "aws_kms_key" "replica2" { - provider = "awsalternate" - description = "%[1]s-3" - deletion_window_in_days = 7 +data "aws_kms_alias" "awsalternate" { + name = "alias/aws/dynamodb" + provider = "awsalternate" } resource "aws_dynamodb_table" "test" { @@ -3804,12 +3784,10 @@ resource "aws_dynamodb_table" "test" { replica { region_name = data.aws_region.alternate.name - kms_key_arn = aws_kms_key.%[2]s.arn } server_side_encryption { - enabled = true - kms_key_arn = aws_kms_key.test.arn + enabled = true } timeouts { @@ -3818,10 +3796,10 @@ resource "aws_dynamodb_table" "test" { delete = "20m" } } -`, rName, key)) +`, rName)) } -func testAccTableConfig_replicaAmazonManagedKey(rName string) string { +func testAccTableConfig_replicaCMKUpdate(rName, key string) string { return acctest.ConfigCompose( acctest.ConfigMultipleRegionProvider(3), // Prevent "Provider configuration not present" errors fmt.Sprintf(` @@ -3829,13 +3807,21 @@ data "aws_region" "alternate" { provider = "awsalternate" } -data "aws_kms_alias" "dynamodb" { - name = "alias/aws/dynamodb" +resource "aws_kms_key" "test" { + description = %[1]q + deletion_window_in_days = 7 } -data "aws_kms_alias" "replica" { - name = "alias/aws/dynamodb" - provider = "awsalternate" +resource "aws_kms_key" "awsalternate1" { + provider = "awsalternate" + description = "%[1]s-1" + deletion_window_in_days = 7 +} + +resource "aws_kms_key" "awsalternate2" { + provider = "awsalternate" + description = "%[1]s-2" + deletion_window_in_days = 7 } resource "aws_dynamodb_table" "test" { @@ -3852,10 +3838,12 @@ resource "aws_dynamodb_table" "test" { replica { region_name = data.aws_region.alternate.name + kms_key_arn = aws_kms_key.%[2]s.arn } server_side_encryption { - enabled = true + enabled = true + kms_key_arn = aws_kms_key.test.arn } timeouts { @@ -3864,7 +3852,7 @@ resource "aws_dynamodb_table" "test" { delete = "20m" } } -`, rName)) +`, rName, key)) } func testAccTableConfig_replicaPITR(rName string, mainPITR, replica1, replica2 bool) string { From 6f85c75f2d584098e38b0e79134070f2366daa8d Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 6 Feb 2024 15:33:32 -0500 Subject: [PATCH 11/16] Acceptance test output: % make testacc TESTARGS='-run=TestAccDynamoDBTable_Replica_singleAddCMK\|TestAccDynamoDBTable_Replica_singleCMK' PKG=dynamodb ACCTEST_PARALLELISM=2 ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./internal/service/dynamodb/... -v -count 1 -parallel 2 -run=TestAccDynamoDBTable_Replica_singleAddCMK\|TestAccDynamoDBTable_Replica_singleCMK -timeout 360m === RUN TestAccDynamoDBTable_Replica_singleCMK === PAUSE TestAccDynamoDBTable_Replica_singleCMK === RUN TestAccDynamoDBTable_Replica_singleAddCMK === PAUSE TestAccDynamoDBTable_Replica_singleAddCMK === CONT TestAccDynamoDBTable_Replica_singleCMK === CONT TestAccDynamoDBTable_Replica_singleAddCMK --- PASS: TestAccDynamoDBTable_Replica_singleAddCMK (511.49s) --- PASS: TestAccDynamoDBTable_Replica_singleAddCMK (636.69s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/dynamodb 646.844s From 8af7bfa3472e23a5b743014c70165ec8f0c5834f Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 6 Feb 2024 16:07:44 -0500 Subject: [PATCH 12/16] 'TestAccDynamoDBTable_Replica_singleAddCMK' -> 'TestAccDynamoDBTable_Replica_doubleAddCMK'. --- internal/service/dynamodb/table_test.go | 55 ++++++++++++++++++------- 1 file changed, 40 insertions(+), 15 deletions(-) diff --git a/internal/service/dynamodb/table_test.go b/internal/service/dynamodb/table_test.go index 3894759789b..847cbcca013 100644 --- a/internal/service/dynamodb/table_test.go +++ b/internal/service/dynamodb/table_test.go @@ -1799,7 +1799,7 @@ func TestAccDynamoDBTable_Replica_singleCMK(t *testing.T) { }) } -func TestAccDynamoDBTable_Replica_singleAddCMK(t *testing.T) { +func TestAccDynamoDBTable_Replica_doubleAddCMK(t *testing.T) { ctx := acctest.Context(t) if testing.Short() { t.Skip("skipping long-running test in short mode") @@ -1810,6 +1810,8 @@ func TestAccDynamoDBTable_Replica_singleAddCMK(t *testing.T) { kmsKeyResourceName := "aws_kms_key.test" kmsKey1Replica1ResourceName := "aws_kms_key.awsalternate1" kmsKey2Replica1ResourceName := "aws_kms_key.awsalternate2" + kmsKey1Replica2ResourceName := "aws_kms_key.awsthird1" + kmsKey2Replica2ResourceName := "aws_kms_key.awsthird2" rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resource.ParallelTest(t, resource.TestCase{ @@ -1825,28 +1827,31 @@ func TestAccDynamoDBTable_Replica_singleAddCMK(t *testing.T) { Config: testAccTableConfig_replicaAmazonManagedKey(rName), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckInitialTableExists(ctx, resourceName, &conf), - resource.TestCheckResourceAttr(resourceName, "replica.#", "1"), + resource.TestCheckResourceAttr(resourceName, "replica.#", "2"), resource.TestCheckResourceAttr(resourceName, "replica.0.kms_key_arn", ""), + resource.TestCheckResourceAttr(resourceName, "replica.1.kms_key_arn", ""), resource.TestCheckResourceAttr(resourceName, "server_side_encryption.0.enabled", "true"), resource.TestCheckResourceAttr(resourceName, "server_side_encryption.0.kms_key_arn", ""), ), }, { - Config: testAccTableConfig_replicaCMKUpdate(rName, "awsalternate1"), + Config: testAccTableConfig_replicaCMKUpdate(rName, "awsalternate1", "awsthird1"), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckInitialTableExists(ctx, resourceName, &conf), - resource.TestCheckResourceAttr(resourceName, "replica.#", "1"), + resource.TestCheckResourceAttr(resourceName, "replica.#", "2"), resource.TestCheckResourceAttrPair(resourceName, "replica.0.kms_key_arn", kmsKey1Replica1ResourceName, names.AttrARN), + resource.TestCheckResourceAttrPair(resourceName, "replica.1.kms_key_arn", kmsKey1Replica2ResourceName, names.AttrARN), resource.TestCheckResourceAttr(resourceName, "server_side_encryption.0.enabled", "true"), resource.TestCheckResourceAttrPair(resourceName, "server_side_encryption.0.kms_key_arn", kmsKeyResourceName, names.AttrARN), ), }, { - Config: testAccTableConfig_replicaCMKUpdate(rName, "awsalternate2"), + Config: testAccTableConfig_replicaCMKUpdate(rName, "awsalternate2", "awsthird2"), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckInitialTableExists(ctx, resourceName, &conf), - resource.TestCheckResourceAttr(resourceName, "replica.#", "1"), + resource.TestCheckResourceAttr(resourceName, "replica.#", "2"), resource.TestCheckResourceAttrPair(resourceName, "replica.0.kms_key_arn", kmsKey2Replica1ResourceName, names.AttrARN), + resource.TestCheckResourceAttrPair(resourceName, "replica.1.kms_key_arn", kmsKey2Replica2ResourceName, names.AttrARN), resource.TestCheckResourceAttr(resourceName, "server_side_encryption.0.enabled", "true"), resource.TestCheckResourceAttrPair(resourceName, "server_side_encryption.0.kms_key_arn", kmsKeyResourceName, names.AttrARN), ), @@ -3761,13 +3766,8 @@ data "aws_region" "alternate" { provider = "awsalternate" } -data "aws_kms_alias" "dynamodb" { - name = "alias/aws/dynamodb" -} - -data "aws_kms_alias" "awsalternate" { - name = "alias/aws/dynamodb" - provider = "awsalternate" +data "aws_region" "third" { + provider = "awsthird" } resource "aws_dynamodb_table" "test" { @@ -3786,6 +3786,10 @@ resource "aws_dynamodb_table" "test" { region_name = data.aws_region.alternate.name } + replica { + region_name = data.aws_region.third.name + } + server_side_encryption { enabled = true } @@ -3799,7 +3803,7 @@ resource "aws_dynamodb_table" "test" { `, rName)) } -func testAccTableConfig_replicaCMKUpdate(rName, key string) string { +func testAccTableConfig_replicaCMKUpdate(rName, keyReplica1, keyReplica2 string) string { return acctest.ConfigCompose( acctest.ConfigMultipleRegionProvider(3), // Prevent "Provider configuration not present" errors fmt.Sprintf(` @@ -3807,6 +3811,10 @@ data "aws_region" "alternate" { provider = "awsalternate" } +data "aws_region" "third" { + provider = "awsthird" +} + resource "aws_kms_key" "test" { description = %[1]q deletion_window_in_days = 7 @@ -3824,6 +3832,18 @@ resource "aws_kms_key" "awsalternate2" { deletion_window_in_days = 7 } +resource "aws_kms_key" "awsthird1" { + provider = "awsthird" + description = "%[1]s-1" + deletion_window_in_days = 7 +} + +resource "aws_kms_key" "awsthird2" { + provider = "awsthird" + description = "%[1]s-2" + deletion_window_in_days = 7 +} + resource "aws_dynamodb_table" "test" { name = %[1]q hash_key = "TestTableHashKey" @@ -3841,6 +3861,11 @@ resource "aws_dynamodb_table" "test" { kms_key_arn = aws_kms_key.%[2]s.arn } + replica { + region_name = data.aws_region.third.name + kms_key_arn = aws_kms_key.%[3]s.arn + } + server_side_encryption { enabled = true kms_key_arn = aws_kms_key.test.arn @@ -3852,7 +3877,7 @@ resource "aws_dynamodb_table" "test" { delete = "20m" } } -`, rName, key)) +`, rName, keyReplica1, keyReplica2)) } func testAccTableConfig_replicaPITR(rName string, mainPITR, replica1, replica2 bool) string { From f97b51ee8ffb7bc8e7344dfdfccf2c1647faaae1 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 6 Feb 2024 16:36:22 -0500 Subject: [PATCH 13/16] Fix importlint error. --- internal/service/dynamodb/table.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/service/dynamodb/table.go b/internal/service/dynamodb/table.go index 9ab5ee2b3a5..a2696abb477 100644 --- a/internal/service/dynamodb/table.go +++ b/internal/service/dynamodb/table.go @@ -22,7 +22,6 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" - "github.com/hashicorp/terraform-provider-aws/internal/conns" "github.com/hashicorp/terraform-provider-aws/internal/create" "github.com/hashicorp/terraform-provider-aws/internal/errs/sdkdiag" From e08ac6f40dd1ac397495c959e8640662c6a4827d Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 6 Feb 2024 16:36:32 -0500 Subject: [PATCH 14/16] Tweak CHANGELOG entry. --- .changelog/35630.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changelog/35630.txt b/.changelog/35630.txt index 36419b68564..d4a893be622 100644 --- a/.changelog/35630.txt +++ b/.changelog/35630.txt @@ -1,3 +1,3 @@ ```release-note:bug -resource/aws_dynamodb_table: Make DescribeTable call per replica +resource/aws_dynamodb_table: Ensure that `replica`s are always set on Read ``` From a4765e76acd00ea44423ca98cade2def3254c6d5 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 6 Feb 2024 16:42:01 -0500 Subject: [PATCH 15/16] enrichReplicas: Tweak error handling. --- internal/service/dynamodb/table.go | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/internal/service/dynamodb/table.go b/internal/service/dynamodb/table.go index a2696abb477..16a4b12fb7a 100644 --- a/internal/service/dynamodb/table.go +++ b/internal/service/dynamodb/table.go @@ -1813,22 +1813,25 @@ func addReplicaPITRs(ctx context.Context, conn *dynamodb.DynamoDB, tableName str return replicas, nil } -func enrichReplicas(ctx context.Context, conn *dynamodb.DynamoDB, arn, tableName, tfVersion string, replicas []interface{}) ([]interface{}, error) { +func enrichReplicas(ctx context.Context, conn *dynamodb.DynamoDB, arn, tableName, tfVersion string, tfList []interface{}) ([]interface{}, error) { // This non-standard approach is needed because PITR info for a replica // must come from a region-specific connection. - for i, replicaRaw := range replicas { - replica := replicaRaw.(map[string]interface{}) + for i, tfMapRaw := range tfList { + tfMap, ok := tfMapRaw.(map[string]interface{}) + if !ok { + continue + } - newARN, err := ARNForNewRegion(arn, replica["region_name"].(string)) + newARN, err := ARNForNewRegion(arn, tfMap["region_name"].(string)) if err != nil { return nil, fmt.Errorf("creating new-region ARN: %s", err) } - replica[names.AttrARN] = newARN + tfMap[names.AttrARN] = newARN - session, err := conns.NewSessionForRegion(&conn.Config, replica["region_name"].(string), tfVersion) + session, err := conns.NewSessionForRegion(&conn.Config, tfMap["region_name"].(string), tfVersion) if err != nil { log.Printf("[WARN] Attempting to get replica (%s) stream information, ignoring encountered error: %s", tableName, err) - return nil, fmt.Errorf("") + continue } conn = dynamodb.New(session) @@ -1836,21 +1839,20 @@ func enrichReplicas(ctx context.Context, conn *dynamodb.DynamoDB, arn, tableName table, err := FindTableByName(ctx, conn, tableName) if err != nil { log.Printf("[WARN] When attempting to get replica (%s) stream information, ignoring encountered error: %s", tableName, err) - return nil, fmt.Errorf("") + continue } - replica["stream_arn"] = aws.StringValue(table.LatestStreamArn) - replica["stream_label"] = aws.StringValue(table.LatestStreamLabel) + tfMap["stream_arn"] = aws.StringValue(table.LatestStreamArn) + tfMap["stream_label"] = aws.StringValue(table.LatestStreamLabel) if table.SSEDescription != nil { - log.Printf("[WARN] in enrichReplicas setting KMS key arn") - replica[names.AttrKMSKeyARN] = aws.StringValue(table.SSEDescription.KMSMasterKeyArn) + tfMap[names.AttrKMSKeyARN] = aws.StringValue(table.SSEDescription.KMSMasterKeyArn) } - replicas[i] = replica + tfList[i] = tfMap } - return replicas, nil + return tfList, nil } func addReplicaTagPropagates(configReplicas *schema.Set, replicas []interface{}) []interface{} { From 92c825739517803153f0c3dad85f57c593c9b0f1 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 6 Feb 2024 17:26:08 -0500 Subject: [PATCH 16/16] Fix semgrep 'ci.semgrep.pluginsdk.avoid-diag_Errorf'. --- internal/service/dynamodb/kinesis_streaming_destination.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/service/dynamodb/kinesis_streaming_destination.go b/internal/service/dynamodb/kinesis_streaming_destination.go index 27991cd7d66..a57901e1cf8 100644 --- a/internal/service/dynamodb/kinesis_streaming_destination.go +++ b/internal/service/dynamodb/kinesis_streaming_destination.go @@ -102,7 +102,7 @@ func resourceKinesisStreamingDestinationRead(ctx context.Context, d *schema.Reso } if err != nil { - return diag.Errorf("reading DynamoDB Kinesis Streaming Destination (%s): %s", d.Id(), err) + return sdkdiag.AppendErrorf(diags, "reading DynamoDB Kinesis Streaming Destination (%s): %s", d.Id(), err) } d.Set("stream_arn", output.StreamArn)