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

resource/aws_dynamodb_table: Fixes perpetual diff when ttl.attribute_name is set when not enabled #37991

Merged
merged 7 commits into from
Jun 17, 2024
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
7 changes: 7 additions & 0 deletions .changelog/37991.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
```release-note:bug
resource/aws_dynamodb_table: Fixes perpetual diff when `ttl.attribute_name` is set when `ttl.enabled` is not set.
```

```release-note:enhancement
resource/aws_dynamodb_table: Adds validation for `ttl` values.
```
65 changes: 64 additions & 1 deletion internal/service/dynamodb/table.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/aws/aws-sdk-go-v2/service/dynamodb"
awstypes "github.com/aws/aws-sdk-go-v2/service/dynamodb/types"
"github.com/hashicorp/aws-sdk-go-base/v2/tfawserr"
"github.com/hashicorp/go-cty/cty"
"github.com/hashicorp/go-multierror"
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff"
Expand Down Expand Up @@ -116,6 +117,7 @@ func resourceTable() *schema.Resource {
// https://github.com/hashicorp/terraform-provider-aws/issues/25214
return old.(string) != new.(string) && new.(string) != ""
}),
validateTTLCustomDiff,
verify.SetTagsDiff,
),

Expand Down Expand Up @@ -450,7 +452,7 @@ func resourceTable() *schema.Resource {
Schema: map[string]*schema.Schema{
"attribute_name": {
Type: schema.TypeString,
Required: true,
Optional: true,
},
names.AttrEnabled: {
Type: schema.TypeBool,
Expand Down Expand Up @@ -2389,3 +2391,64 @@ func validateProvisionedThroughputField(diff *schema.ResourceDiff, key string) e
}
return nil
}

func validateTTLCustomDiff(ctx context.Context, d *schema.ResourceDiff, meta any) error {
var diags diag.Diagnostics

configRaw := d.GetRawConfig()
if !configRaw.IsKnown() || configRaw.IsNull() {
return nil
}

ttlPath := cty.GetAttrPath("ttl")
ttl := configRaw.GetAttr("ttl")
if ttl.IsKnown() && !ttl.IsNull() {
if ttl.LengthInt() == 1 {
idx := cty.NumberIntVal(0)
ttl := ttl.Index(idx)
ttlPath := ttlPath.Index(idx)
ttlPlantimeValidate(ttlPath, ttl, &diags)
}
}

return sdkdiag.DiagnosticsError(diags)
}

func ttlPlantimeValidate(ttlPath cty.Path, ttl cty.Value, diags *diag.Diagnostics) {
attribute := ttl.GetAttr("attribute_name")
if !attribute.IsKnown() {
return
}

enabled := ttl.GetAttr(names.AttrEnabled)
if !enabled.IsKnown() {
return
}
if enabled.IsNull() {
return
}

if enabled.True() {
if attribute.IsNull() {
*diags = append(*diags, errs.NewAttributeRequiredWhenError(
ttlPath.GetAttr("attribute_name"),
ttlPath.GetAttr(names.AttrEnabled),
"true",
))
} else if attribute.AsString() == "" {
*diags = append(*diags, errs.NewInvalidValueAttributeErrorf(
ttlPath.GetAttr("attribute_name"),
"Attribute %q cannot have an empty value",
errs.PathString(ttlPath.GetAttr("attribute_name")),
))
}
} else {
if !(attribute.IsNull() || attribute.AsString() == "") {
*diags = append(*diags, errs.NewAttributeConflictsWhenError(
ttlPath.GetAttr("attribute_name"),
ttlPath.GetAttr(names.AttrEnabled),
"false",
))
}
}
}
163 changes: 150 additions & 13 deletions internal/service/dynamodb/table_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@ import (
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
sdkacctest "github.com/hashicorp/terraform-plugin-testing/helper/acctest"
"github.com/hashicorp/terraform-plugin-testing/helper/resource"
"github.com/hashicorp/terraform-plugin-testing/knownvalue"
"github.com/hashicorp/terraform-plugin-testing/statecheck"
"github.com/hashicorp/terraform-plugin-testing/terraform"
"github.com/hashicorp/terraform-plugin-testing/tfjsonpath"
"github.com/hashicorp/terraform-provider-aws/internal/acctest"
"github.com/hashicorp/terraform-provider-aws/internal/conns"
"github.com/hashicorp/terraform-provider-aws/internal/create"
Expand Down Expand Up @@ -372,10 +375,16 @@ func TestAccDynamoDBTable_basic(t *testing.T) {
resource.TestCheckResourceAttr(resourceName, "table_class", "STANDARD"),
resource.TestCheckResourceAttr(resourceName, acctest.CtTagsPercent, acctest.Ct0),
resource.TestCheckResourceAttr(resourceName, acctest.CtTagsAllPercent, acctest.Ct0),
resource.TestCheckResourceAttr(resourceName, "ttl.#", acctest.Ct1),
resource.TestCheckResourceAttr(resourceName, "ttl.0.enabled", acctest.CtFalse),
resource.TestCheckResourceAttr(resourceName, "write_capacity", acctest.Ct1),
),
ConfigStateChecks: []statecheck.StateCheck{
statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("ttl"), knownvalue.ListExact([]knownvalue.Check{
knownvalue.ObjectExact(map[string]knownvalue.Check{
"attribute_name": knownvalue.StringExact(""),
names.AttrEnabled: knownvalue.Bool(false),
}),
})),
},
},
{
ResourceName: resourceName,
Expand Down Expand Up @@ -1362,18 +1371,29 @@ func TestAccDynamoDBTable_TTL_enabled(t *testing.T) {
CheckDestroy: testAccCheckTableDestroy(ctx),
Steps: []resource.TestStep{
{
Config: testAccTableConfig_timeToLive(rName, true),
Config: testAccTableConfig_timeToLive(rName, rName, true),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckInitialTableExists(ctx, resourceName, &table),
resource.TestCheckResourceAttr(resourceName, "ttl.#", acctest.Ct1),
resource.TestCheckResourceAttr(resourceName, "ttl.0.enabled", acctest.CtTrue),
),
ConfigStateChecks: []statecheck.StateCheck{
statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("ttl"), knownvalue.ListExact([]knownvalue.Check{
knownvalue.ObjectExact(map[string]knownvalue.Check{
"attribute_name": knownvalue.StringExact(rName),
names.AttrEnabled: knownvalue.Bool(true),
}),
})),
},
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
{
Config: testAccTableConfig_timeToLive_unset(rName),
PlanOnly: true,
ExpectNonEmptyPlan: false,
},
},
})
}
Expand All @@ -1393,25 +1413,105 @@ func TestAccDynamoDBTable_TTL_disabled(t *testing.T) {
CheckDestroy: testAccCheckTableDestroy(ctx),
Steps: []resource.TestStep{
{
Config: testAccTableConfig_timeToLive(rName, false),
Config: testAccTableConfig_timeToLive(rName, "", false),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckInitialTableExists(ctx, resourceName, &table),
),
ConfigStateChecks: []statecheck.StateCheck{
statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("ttl"), knownvalue.ListExact([]knownvalue.Check{
knownvalue.ObjectExact(map[string]knownvalue.Check{
"attribute_name": knownvalue.StringExact(""),
names.AttrEnabled: knownvalue.Bool(false),
}),
})),
},
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
{
Config: testAccTableConfig_timeToLive_unset(rName),
PlanOnly: true,
ExpectNonEmptyPlan: false,
},
},
})
}

// TTL tests must be split since it can only be updated once per hour
// ValidationException: Time to live has been modified multiple times within a fixed interval
func TestAccDynamoDBTable_TTL_update(t *testing.T) {
ctx := acctest.Context(t)
var table awstypes.TableDescription
resourceName := "aws_dynamodb_table.test"
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(ctx, t) },
ErrorCheck: acctest.ErrorCheck(t, names.DynamoDBServiceID),
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
CheckDestroy: testAccCheckTableDestroy(ctx),
Steps: []resource.TestStep{
{
Config: testAccTableConfig_timeToLive(rName, "", false),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckInitialTableExists(ctx, resourceName, &table),
resource.TestCheckResourceAttr(resourceName, "ttl.#", acctest.Ct1),
resource.TestCheckResourceAttr(resourceName, "ttl.0.enabled", acctest.CtFalse),
),
ConfigStateChecks: []statecheck.StateCheck{
statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("ttl"), knownvalue.ListExact([]knownvalue.Check{
knownvalue.ObjectExact(map[string]knownvalue.Check{
"attribute_name": knownvalue.StringExact(""),
names.AttrEnabled: knownvalue.Bool(false),
}),
})),
},
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
{
Config: testAccTableConfig_timeToLive(rName, true),
Config: testAccTableConfig_timeToLive(rName, rName, true),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckInitialTableExists(ctx, resourceName, &table),
resource.TestCheckResourceAttr(resourceName, "ttl.#", acctest.Ct1),
resource.TestCheckResourceAttr(resourceName, "ttl.0.enabled", acctest.CtTrue),
),
ConfigStateChecks: []statecheck.StateCheck{
statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("ttl"), knownvalue.ListExact([]knownvalue.Check{
knownvalue.ObjectExact(map[string]knownvalue.Check{
"attribute_name": knownvalue.StringExact(rName),
names.AttrEnabled: knownvalue.Bool(true),
}),
})),
},
},
},
})
}

func TestAccDynamoDBTable_TTL_validate(t *testing.T) {
ctx := acctest.Context(t)
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(ctx, t) },
ErrorCheck: acctest.ErrorCheck(t, names.DynamoDBServiceID),
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
CheckDestroy: testAccCheckTableDestroy(ctx),
Steps: []resource.TestStep{
{
Config: testAccTableConfig_timeToLive(rName, "TestTTL", false),
ExpectError: regexache.MustCompile(regexp.QuoteMeta(`Attribute "ttl[0].attribute_name" cannot be specified when "ttl[0].enabled" is "false"`)),
},
{
Config: testAccTableConfig_timeToLive(rName, "", true),
ExpectError: regexache.MustCompile(regexp.QuoteMeta(`Attribute "ttl[0].attribute_name" cannot have an empty value`)),
},
{
Config: testAccTableConfig_TTL_missingAttributeName(rName, true),
ExpectError: regexache.MustCompile(regexp.QuoteMeta(`Attribute "ttl[0].attribute_name" cannot have an empty value`)),
},
},
})
Expand Down Expand Up @@ -3496,7 +3596,44 @@ resource "aws_dynamodb_table" "test" {
`, rName)
}

func testAccTableConfig_timeToLive(rName string, ttlEnabled bool) string {
func testAccTableConfig_timeToLive(rName, ttlAttribute string, ttlEnabled bool) string {
return fmt.Sprintf(`
resource "aws_dynamodb_table" "test" {
hash_key = "TestTableHashKey"
name = %[1]q
read_capacity = 1
write_capacity = 1

attribute {
name = "TestTableHashKey"
type = "S"
}

ttl {
attribute_name = %[2]q
enabled = %[3]t
}
}
`, rName, ttlAttribute, ttlEnabled)
}

func testAccTableConfig_timeToLive_unset(rName string) string {
return fmt.Sprintf(`
resource "aws_dynamodb_table" "test" {
hash_key = "TestTableHashKey"
name = %[1]q
read_capacity = 1
write_capacity = 1

attribute {
name = "TestTableHashKey"
type = "S"
}
}
`, rName)
}

func testAccTableConfig_TTL_missingAttributeName(rName string, ttlEnabled bool) string {
return fmt.Sprintf(`
resource "aws_dynamodb_table" "test" {
hash_key = "TestTableHashKey"
Expand All @@ -3510,7 +3647,7 @@ resource "aws_dynamodb_table" "test" {
}

ttl {
attribute_name = %[2]t ? "TestTTL" : ""
attribute_name = ""
enabled = %[2]t
}
}
Expand Down
8 changes: 5 additions & 3 deletions website/docs/r/dynamodb_table.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ resource "aws_dynamodb_table" "basic-dynamodb-table" {

ttl {
attribute_name = "TimeToExist"
enabled = false
enabled = true
}

global_secondary_index {
Expand Down Expand Up @@ -261,8 +261,10 @@ Optional arguments:

### `ttl`

* `enabled` - (Required) Whether TTL is enabled.
* `attribute_name` - (Required) Name of the table attribute to store the TTL timestamp in.
* `attribute_name` - (Optional) Name of the table attribute to store the TTL timestamp in.
Required if `enabled` is `true`, must not be set otherwise.
* `enabled` - (Optional) Whether TTL is enabled.
Default value is `false`.

## Attribute Reference

Expand Down
Loading