-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
DDB update replica Amazon Owned SSE #31284
DDB update replica Amazon Owned SSE #31284
Conversation
Community NoteVoting for Prioritization
For Submitters
|
2e4e32f
to
484c978
Compare
@ewbankkit Hows this looking? I can't see the test failures. |
@ewbankkit @justinretzolk Bump. |
@justinretzolk any movement on this? |
Hey @joshmyers 👋 Thank you for checking in on this! Unfortunately I can't provide an ETA on when this will be reviewed/merged due to the potential of shifting priorities. We prioritize by count of 👍 reactions and a few other things (more information on our prioritization guide if you're interested). |
@justinretzolk any updates ? |
Hey guys, looking for updates about this issue as well. |
Any update on this issue? |
This is still affecting and need to priotised. Any work around on this till fix is released? |
Some time ago Amazon changed the default DDB encryption behaviour to use a key 'Managed by DynamoDB' (it lives in the DDB internal account) if setting `server_side_encryption.enabled = false` The current update table logic, if using replicas, assumes you are using a CMK. If this is not the case, an update table request is triggered with an empty `kMSMasterKeyId`[1] and AWS API throws the below error. This commit fixes the logic to only update replicas if there is a CMK used. [1] ``` │ Error: updating DynamoDB Table (BADGERS) SSE: ValidationException: 4 validation errors detected: Value '' at 'replicaUpdates.1.member.update.kMSMasterKeyId' failed to satisfy constraint: Member must have length greater than or equal to 1; Value '' at 'replicaUpdates.2.member.update.kMSMasterKeyId' failed to satisfy constraint: Member must have length greater than or equal to 1; Value '' at 'replicaUpdates.3.member.update.kMSMasterKeyId' failed to satisfy constraint: Member must have length greater than or equal to 1; Value '' at 'replicaUpdates.4.member.update.kMSMasterKeyId' failed to satisfy constraint: Member must have length greater than or equal to 1 ``` Fixes hashicorp#31153
Adds test case for changing DDB `server_side_encryption.enabled` from true to false on a table with replicas and no CMK.
72c127b
to
ec4203d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your work on this!
(Failing test is pre-existing.)
% make t T=TestAccDynamoDBTable_ K=dynamodb P=4
make: Verifying source code with gofmt...
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go1.22.6 test ./internal/service/dynamodb/... -v -count 1 -parallel 4 -run='TestAccDynamoDBTable_' -timeout 360m
=== RUN TestAccDynamoDBTable_basic
=== PAUSE TestAccDynamoDBTable_basic
=== RUN TestAccDynamoDBTable_deletion_protection
=== PAUSE TestAccDynamoDBTable_deletion_protection
=== RUN TestAccDynamoDBTable_disappears
=== PAUSE TestAccDynamoDBTable_disappears
=== RUN TestAccDynamoDBTable_Disappears_payPerRequestWithGSI
=== PAUSE TestAccDynamoDBTable_Disappears_payPerRequestWithGSI
=== RUN TestAccDynamoDBTable_extended
=== PAUSE TestAccDynamoDBTable_extended
=== RUN TestAccDynamoDBTable_enablePITR
=== PAUSE TestAccDynamoDBTable_enablePITR
=== RUN TestAccDynamoDBTable_BillingMode_payPerRequestToProvisioned
=== PAUSE TestAccDynamoDBTable_BillingMode_payPerRequestToProvisioned
=== RUN TestAccDynamoDBTable_BillingMode_payPerRequestToProvisionedIgnoreChanges
=== PAUSE TestAccDynamoDBTable_BillingMode_payPerRequestToProvisionedIgnoreChanges
=== RUN TestAccDynamoDBTable_BillingMode_provisionedToPayPerRequest
=== PAUSE TestAccDynamoDBTable_BillingMode_provisionedToPayPerRequest
=== RUN TestAccDynamoDBTable_BillingMode_provisionedToPayPerRequestIgnoreChanges
=== PAUSE TestAccDynamoDBTable_BillingMode_provisionedToPayPerRequestIgnoreChanges
=== RUN TestAccDynamoDBTable_BillingModeGSI_payPerRequestToProvisioned
=== PAUSE TestAccDynamoDBTable_BillingModeGSI_payPerRequestToProvisioned
=== RUN TestAccDynamoDBTable_BillingModeGSI_provisionedToPayPerRequest
=== PAUSE TestAccDynamoDBTable_BillingModeGSI_provisionedToPayPerRequest
=== RUN TestAccDynamoDBTable_streamSpecification
=== PAUSE TestAccDynamoDBTable_streamSpecification
=== RUN TestAccDynamoDBTable_streamSpecificationDiffs
=== PAUSE TestAccDynamoDBTable_streamSpecificationDiffs
=== RUN TestAccDynamoDBTable_streamSpecificationValidation
=== PAUSE TestAccDynamoDBTable_streamSpecificationValidation
=== RUN TestAccDynamoDBTable_tags
=== PAUSE TestAccDynamoDBTable_tags
=== RUN TestAccDynamoDBTable_gsiUpdateCapacity
=== PAUSE TestAccDynamoDBTable_gsiUpdateCapacity
=== RUN TestAccDynamoDBTable_gsiUpdateOtherAttributes
=== PAUSE TestAccDynamoDBTable_gsiUpdateOtherAttributes
=== RUN TestAccDynamoDBTable_lsiNonKeyAttributes
=== PAUSE TestAccDynamoDBTable_lsiNonKeyAttributes
=== RUN TestAccDynamoDBTable_gsiUpdateNonKeyAttributes
=== PAUSE TestAccDynamoDBTable_gsiUpdateNonKeyAttributes
=== RUN TestAccDynamoDBTable_GsiUpdateNonKeyAttributes_emptyPlan
=== PAUSE TestAccDynamoDBTable_GsiUpdateNonKeyAttributes_emptyPlan
=== RUN TestAccDynamoDBTable_TTL_enabled
=== PAUSE TestAccDynamoDBTable_TTL_enabled
=== RUN TestAccDynamoDBTable_TTL_disabled
=== PAUSE TestAccDynamoDBTable_TTL_disabled
=== RUN TestAccDynamoDBTable_TTL_update
=== PAUSE TestAccDynamoDBTable_TTL_update
=== RUN TestAccDynamoDBTable_TTL_validate
=== PAUSE TestAccDynamoDBTable_TTL_validate
=== RUN TestAccDynamoDBTable_attributeUpdate
=== PAUSE TestAccDynamoDBTable_attributeUpdate
=== RUN TestAccDynamoDBTable_lsiUpdate
=== PAUSE TestAccDynamoDBTable_lsiUpdate
=== RUN TestAccDynamoDBTable_attributeUpdateValidation
=== PAUSE TestAccDynamoDBTable_attributeUpdateValidation
=== RUN TestAccDynamoDBTable_encryption
=== PAUSE TestAccDynamoDBTable_encryption
=== RUN TestAccDynamoDBTable_restoreCrossRegion
=== PAUSE TestAccDynamoDBTable_restoreCrossRegion
=== RUN TestAccDynamoDBTable_Replica_multiple
=== PAUSE TestAccDynamoDBTable_Replica_multiple
=== RUN TestAccDynamoDBTable_Replica_single
=== PAUSE TestAccDynamoDBTable_Replica_single
=== RUN TestAccDynamoDBTable_Replica_singleStreamSpecification
=== PAUSE TestAccDynamoDBTable_Replica_singleStreamSpecification
=== RUN TestAccDynamoDBTable_Replica_singleDefaultKeyEncrypted
=== PAUSE TestAccDynamoDBTable_Replica_singleDefaultKeyEncrypted
=== RUN TestAccDynamoDBTable_Replica_singleDefaultKeyEncryptedAmazonOwned
=== PAUSE TestAccDynamoDBTable_Replica_singleDefaultKeyEncryptedAmazonOwned
=== RUN TestAccDynamoDBTable_Replica_singleCMK
=== PAUSE TestAccDynamoDBTable_Replica_singleCMK
=== RUN TestAccDynamoDBTable_Replica_doubleAddCMK
=== PAUSE TestAccDynamoDBTable_Replica_doubleAddCMK
=== RUN TestAccDynamoDBTable_Replica_pitr
=== PAUSE TestAccDynamoDBTable_Replica_pitr
=== RUN TestAccDynamoDBTable_Replica_pitrKMS
=== PAUSE TestAccDynamoDBTable_Replica_pitrKMS
=== RUN TestAccDynamoDBTable_Replica_tagsOneOfTwo
=== PAUSE TestAccDynamoDBTable_Replica_tagsOneOfTwo
=== RUN TestAccDynamoDBTable_Replica_tagsTwoOfTwo
=== PAUSE TestAccDynamoDBTable_Replica_tagsTwoOfTwo
=== RUN TestAccDynamoDBTable_Replica_tagsNext
=== PAUSE TestAccDynamoDBTable_Replica_tagsNext
=== RUN TestAccDynamoDBTable_Replica_tagsUpdate
=== PAUSE TestAccDynamoDBTable_Replica_tagsUpdate
=== RUN TestAccDynamoDBTable_tableClassInfrequentAccess
=== PAUSE TestAccDynamoDBTable_tableClassInfrequentAccess
=== RUN TestAccDynamoDBTable_tableClassExplicitDefault
=== PAUSE TestAccDynamoDBTable_tableClassExplicitDefault
=== RUN TestAccDynamoDBTable_tableClass_ConcurrentModification
=== PAUSE TestAccDynamoDBTable_tableClass_ConcurrentModification
=== RUN TestAccDynamoDBTable_tableClass_migrate
=== PAUSE TestAccDynamoDBTable_tableClass_migrate
=== RUN TestAccDynamoDBTable_backupEncryption
=== PAUSE TestAccDynamoDBTable_backupEncryption
=== RUN TestAccDynamoDBTable_backup_overrideEncryption
=== PAUSE TestAccDynamoDBTable_backup_overrideEncryption
=== RUN TestAccDynamoDBTable_importTable
=== PAUSE TestAccDynamoDBTable_importTable
=== CONT TestAccDynamoDBTable_basic
=== CONT TestAccDynamoDBTable_attributeUpdate
=== CONT TestAccDynamoDBTable_streamSpecificationDiffs
=== CONT TestAccDynamoDBTable_Replica_pitrKMS
--- PASS: TestAccDynamoDBTable_basic (34.07s)
=== CONT TestAccDynamoDBTable_tableClassExplicitDefault
--- PASS: TestAccDynamoDBTable_tableClassExplicitDefault (33.23s)
=== CONT TestAccDynamoDBTable_importTable
--- PASS: TestAccDynamoDBTable_streamSpecificationDiffs (155.09s)
=== CONT TestAccDynamoDBTable_backup_overrideEncryption
--- PASS: TestAccDynamoDBTable_importTable (127.46s)
=== CONT TestAccDynamoDBTable_backupEncryption
--- PASS: TestAccDynamoDBTable_Replica_pitrKMS (388.93s)
=== CONT TestAccDynamoDBTable_tableClass_migrate
--- PASS: TestAccDynamoDBTable_tableClass_migrate (35.02s)
=== CONT TestAccDynamoDBTable_tableClass_ConcurrentModification
--- PASS: TestAccDynamoDBTable_backup_overrideEncryption (269.57s)
=== CONT TestAccDynamoDBTable_gsiUpdateNonKeyAttributes
--- PASS: TestAccDynamoDBTable_tableClass_ConcurrentModification (51.06s)
=== CONT TestAccDynamoDBTable_TTL_validate
--- PASS: TestAccDynamoDBTable_TTL_validate (5.60s)
=== CONT TestAccDynamoDBTable_TTL_update
--- PASS: TestAccDynamoDBTable_TTL_update (38.52s)
=== CONT TestAccDynamoDBTable_TTL_disabled
--- PASS: TestAccDynamoDBTable_TTL_disabled (33.44s)
=== CONT TestAccDynamoDBTable_TTL_enabled
--- PASS: TestAccDynamoDBTable_attributeUpdate (583.49s)
=== CONT TestAccDynamoDBTable_GsiUpdateNonKeyAttributes_emptyPlan
--- PASS: TestAccDynamoDBTable_gsiUpdateNonKeyAttributes (160.28s)
=== CONT TestAccDynamoDBTable_gsiUpdateCapacity
--- PASS: TestAccDynamoDBTable_TTL_enabled (34.02s)
=== CONT TestAccDynamoDBTable_lsiNonKeyAttributes
--- PASS: TestAccDynamoDBTable_lsiNonKeyAttributes (32.74s)
=== CONT TestAccDynamoDBTable_gsiUpdateOtherAttributes
--- PASS: TestAccDynamoDBTable_GsiUpdateNonKeyAttributes_emptyPlan (39.86s)
=== CONT TestAccDynamoDBTable_Replica_tagsTwoOfTwo
--- PASS: TestAccDynamoDBTable_gsiUpdateCapacity (61.29s)
=== CONT TestAccDynamoDBTable_Replica_singleStreamSpecification
--- PASS: TestAccDynamoDBTable_Replica_singleStreamSpecification (212.13s)
=== CONT TestAccDynamoDBTable_Replica_pitr
--- PASS: TestAccDynamoDBTable_backupEncryption (799.68s)
=== CONT TestAccDynamoDBTable_Replica_doubleAddCMK
--- PASS: TestAccDynamoDBTable_Replica_tagsTwoOfTwo (395.13s)
=== CONT TestAccDynamoDBTable_Replica_tagsNext
--- PASS: TestAccDynamoDBTable_Replica_pitr (311.69s)
=== CONT TestAccDynamoDBTable_Replica_singleCMK
--- PASS: TestAccDynamoDBTable_gsiUpdateOtherAttributes (632.52s)
=== CONT TestAccDynamoDBTable_Replica_singleDefaultKeyEncryptedAmazonOwned
--- PASS: TestAccDynamoDBTable_Replica_singleCMK (192.99s)
=== CONT TestAccDynamoDBTable_tableClassInfrequentAccess
--- PASS: TestAccDynamoDBTable_tableClassInfrequentAccess (45.45s)
=== CONT TestAccDynamoDBTable_Replica_singleDefaultKeyEncrypted
--- PASS: TestAccDynamoDBTable_Replica_tagsNext (581.58s)
=== CONT TestAccDynamoDBTable_Replica_tagsUpdate
--- PASS: TestAccDynamoDBTable_Replica_singleDefaultKeyEncrypted (193.01s)
=== CONT TestAccDynamoDBTable_restoreCrossRegion
--- PASS: TestAccDynamoDBTable_Replica_singleDefaultKeyEncryptedAmazonOwned (350.23s)
=== CONT TestAccDynamoDBTable_attributeUpdateValidation
--- PASS: TestAccDynamoDBTable_attributeUpdateValidation (5.99s)
=== CONT TestAccDynamoDBTable_Replica_single
=== NAME TestAccDynamoDBTable_Replica_tagsUpdate
table_test.go:2420: Step 2/5 error: After applying this test step, the non-refresh plan was not empty.
stdout:
Terraform used the selected providers to generate the following execution
plan. Resource actions are indicated with the following symbols:
~ update in-place
Terraform will perform the following actions:
# aws_dynamodb_table.test will be updated in-place
~ resource "aws_dynamodb_table" "test" {
id = "tf-acc-test-4517083279634296887"
name = "tf-acc-test-4517083279634296887"
~ tags = {
"Name" = "tf-acc-test-4517083279634296887"
"Pozo" = "Amargo"
+ "Thrill" = "Seekers"
+ "tyDi" = "Lullaby"
}
~ tags_all = {
+ "Thrill" = "Seekers"
+ "tyDi" = "Lullaby"
# (2 unchanged elements hidden)
}
# (11 unchanged attributes hidden)
# (4 unchanged blocks hidden)
}
Plan: 0 to add, 1 to change, 0 to destroy.
--- FAIL: TestAccDynamoDBTable_Replica_tagsUpdate (210.47s)
=== CONT TestAccDynamoDBTable_encryption
--- PASS: TestAccDynamoDBTable_Replica_doubleAddCMK (885.44s)
=== CONT TestAccDynamoDBTable_Replica_multiple
--- PASS: TestAccDynamoDBTable_encryption (117.00s)
=== CONT TestAccDynamoDBTable_lsiUpdate
--- PASS: TestAccDynamoDBTable_lsiUpdate (55.88s)
=== CONT TestAccDynamoDBTable_Replica_tagsOneOfTwo
--- PASS: TestAccDynamoDBTable_Replica_single (455.89s)
=== CONT TestAccDynamoDBTable_BillingMode_payPerRequestToProvisionedIgnoreChanges
--- PASS: TestAccDynamoDBTable_BillingMode_payPerRequestToProvisionedIgnoreChanges (45.88s)
=== CONT TestAccDynamoDBTable_streamSpecification
--- PASS: TestAccDynamoDBTable_streamSpecification (72.57s)
=== CONT TestAccDynamoDBTable_streamSpecificationValidation
--- PASS: TestAccDynamoDBTable_streamSpecificationValidation (1.99s)
=== CONT TestAccDynamoDBTable_BillingModeGSI_provisionedToPayPerRequest
--- PASS: TestAccDynamoDBTable_Replica_tagsOneOfTwo (317.22s)
=== CONT TestAccDynamoDBTable_BillingModeGSI_payPerRequestToProvisioned
--- PASS: TestAccDynamoDBTable_BillingModeGSI_payPerRequestToProvisioned (65.79s)
=== CONT TestAccDynamoDBTable_tags
--- PASS: TestAccDynamoDBTable_restoreCrossRegion (797.57s)
=== CONT TestAccDynamoDBTable_extended
--- PASS: TestAccDynamoDBTable_tags (47.80s)
=== CONT TestAccDynamoDBTable_BillingMode_provisionedToPayPerRequestIgnoreChanges
--- PASS: TestAccDynamoDBTable_Replica_multiple (677.78s)
=== CONT TestAccDynamoDBTable_BillingMode_payPerRequestToProvisioned
--- PASS: TestAccDynamoDBTable_extended (189.05s)
=== CONT TestAccDynamoDBTable_disappears
--- PASS: TestAccDynamoDBTable_BillingMode_payPerRequestToProvisioned (42.86s)
=== CONT TestAccDynamoDBTable_enablePITR
--- PASS: TestAccDynamoDBTable_disappears (23.51s)
=== CONT TestAccDynamoDBTable_Disappears_payPerRequestWithGSI
--- PASS: TestAccDynamoDBTable_enablePITR (58.06s)
=== CONT TestAccDynamoDBTable_deletion_protection
--- PASS: TestAccDynamoDBTable_deletion_protection (37.31s)
=== CONT TestAccDynamoDBTable_BillingMode_provisionedToPayPerRequest
--- PASS: TestAccDynamoDBTable_Disappears_payPerRequestWithGSI (86.35s)
--- PASS: TestAccDynamoDBTable_BillingMode_provisionedToPayPerRequestIgnoreChanges (607.72s)
--- PASS: TestAccDynamoDBTable_BillingMode_provisionedToPayPerRequest (598.13s)
--- PASS: TestAccDynamoDBTable_BillingModeGSI_provisionedToPayPerRequest (1133.05s)
FAIL
FAIL github.com/hashicorp/terraform-provider-aws/internal/service/dynamodb 3322.659s
This functionality has been released in v5.68.0 of the Terraform AWS Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you! |
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. |
Description
Fixes update DDB table action to only try and update replicas CMK if actually using a CMK.
Relations
Closes #31153
References
Built and tested this locally. Ran what was failing on the linked bug report which applied clean.
Output from Acceptance Testing
Before change:
After change: