resource/aws_dynamodb_table: Update and validate attributes

[radeksimko]

Fixes #1424

Test results

=== RUN   TestAccAWSDynamoDbTable_attributeUpdateValidation
--- PASS: TestAccAWSDynamoDbTable_attributeUpdateValidation (2.97s)
=== RUN   TestAccAWSDynamoDbTable_tags
--- PASS: TestAccAWSDynamoDbTable_tags (53.09s)
=== RUN   TestAccAWSDynamoDbTable_importTimeToLive
--- PASS: TestAccAWSDynamoDbTable_importTimeToLive (53.51s)
=== RUN   TestAccAWSDynamoDbTable_ttl
--- PASS: TestAccAWSDynamoDbTable_ttl (113.45s)
=== RUN   TestAccAWSDynamoDbTable_importBasic
--- PASS: TestAccAWSDynamoDbTable_importBasic (127.66s)
=== RUN   TestAccAWSDynamoDbTable_importTags
--- PASS: TestAccAWSDynamoDbTable_importTags (127.85s)
=== RUN   TestAccAWSDynamoDbTable_gsiUpdateNonKeyAttributes
--- PASS: TestAccAWSDynamoDbTable_gsiUpdateNonKeyAttributes (146.59s)
=== RUN   TestAccAWSDynamoDbTable_streamSpecification
--- PASS: TestAccAWSDynamoDbTable_streamSpecification (168.25s)
=== RUN   TestAccAWSDynamoDbTable_gsiUpdateCapacity
--- PASS: TestAccAWSDynamoDbTable_gsiUpdateCapacity (312.93s)
=== RUN   TestAccAWSDynamoDbTable_gsiUpdateOtherAttributes
--- PASS: TestAccAWSDynamoDbTable_gsiUpdateOtherAttributes (452.91s)
=== RUN   TestAccAWSDynamoDbTable_attributeUpdate
--- PASS: TestAccAWSDynamoDbTable_attributeUpdate (458.46s)
=== RUN   TestAccAWSDynamoDbTable_basic
--- PASS: TestAccAWSDynamoDbTable_basic (461.79s)

[bflad]

FYI this requires a rebase for aws/validators.go

Can you find documentation to support this? Indexes and attributes are tightly coupled (DynamoDB disallows unindexed attributes) I don't remember this being a limitation of the service nor does it explicitly state that in the DynamoDB documentation.

[radeksimko]

Can you find documentation to support this? Indexes and attributes are tightly coupled (DynamoDB disallows unindexed attributes) I don't remember this being a limitation of the service nor does it explicitly state that in the DynamoDB documentation.

I came to that conclusion after:

a. Getting this error message:

ValidationException: One or more parameter values were invalid: Some AttributeDefinitions are not used. AttributeDefinitions: [TestGSIRangeKey, TestTableHashKey, TestLSIRangeKey, TestTableRangeKey], keys used: [TestTableRangeKey, TestTableHashKey, TestLSIRangeKey]
	status code: 400, request id: 9A6QGJAD38PD94IV10GJ8G2DM3VV4KQNSO5AEMVJF66Q9ASUAAJG

b. When DynamoDB removed an attribute during UpdateTable which was removing the only index that was referencing that field.

I however agree it's not very clear and we should get a confirmation from AWS support and perhaps paste it here. I'll reach out to them.

[radeksimko]

AWS support just confirmed what I wrote above:

00:17:12 PM radek: right, that's what I thought, so AttributeDefinitions in the API is basically a schema for indexes. It obviously doesn't define schema for records (as DDB is schemaless), but any attribute which is used in KeySchema and/or in GSI and/or LSI has to be in that list (AttributeDefinitions)?
00:18:43 PM AWS: yup..

Admittedly my comment may be suggesting DynamoDB requires you to define schema for all attributes, but that's partially because of how much the resource schema is drifted from API's naming convention. Maybe if attribute was attribute_definition it would be slightly more obvious? I'm open to suggestions.

[bflad]

Ah okay - this makes more sense. Thank you for clarifying here!

I just wanted to be sure this configuration is still valid (you can have attribute definitions that are not indexed):

resource "aws_dynamodb_table" "basic-dynamodb-table" {
  name = "%s"
  read_capacity = 10
  write_capacity = 10
  hash_key = "staticHashKey"

  attribute {
    name = "staticHashKey"
    type = "S"
  }
 
  attribute {
    name = "indexed"
    type = "%s"
  }

  attribute {
    name = "notIndexed"
    type = "%s"
  }
 
   global_secondary_index {
     name = "gsiName"
     hash_key = "indexed"
     write_capacity = 10
     read_capacity = 10
     projection_type = "KEYS_ONLY"
   }
 }

Maybe some simple rewording of comments/variable names like this would help me personally (and potentially other future code readers) 😄

• s/DynamoDB disallows unindexed attributes/DynamoDB requires attribute definitions for all indexed attributes/
• s/Check if all attributes are indexed/Check if all indexed attributes have an attribute definition/
• s/unusedAttributes/missingAttributeDefintions/

What do you think? Otherwise, I think this is good to go.

[bflad]

Okay it turns out that I completely misunderstood AttributeDefinition under the hood that they cannot specify extra attributes (a limitation on the AWS side). Knowing that, this PR looks great once its rebased. 👍

[radeksimko]

Currently blocked by hashicorp/terraform#17261

[bflad]

Update: we're still waiting on upstream Terraform core 0.11.4 release (rather than vendoring a commit SHA)

[bflad]

@radeksimko upstream Terraform 0.11.4 has been vendored! 😄

[radeksimko]

Rebased & ran related acceptance tests - all green, so I'm going to merge this.

[bflad]

This has been released in version 1.12.0 of the AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

[Venkat2811]

We are using Terraform 0.10.6 version. Any idea on how to overcome this issue ?

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!