-
Notifications
You must be signed in to change notification settings - Fork 8.9k
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_s3_bucket: Prevent various panics with empty configuration blocks #12614
Conversation
…on blocks Reference: #11420 Reference: #12480 This does not contain a fully reproducible configuration for #12480 after a few timeboxed attempts, but left a test that adds `access_control_translation` since that most closely mimics what was reported and was previously untested. The addition of an empty configuration block in the plan difference appears to be a bug in the Terraform Plugin SDK or Terraform core logic. If/when these various S3 configurations are potentially moved to their own resources, we should try to remove the Set hashing functions then. Generally they are unnecessary except in specific situations. Previously: ``` === CONT TestAccAWSS3Bucket_LifecycleRule_Expiration_EmptyConfigurationBlock panic: interface conversion: interface {} is nil, not map[string]interface {} goroutine 228 [running]: github.com/terraform-providers/terraform-provider-aws/aws.expirationHash(0x0, 0x0, 0xc000e4bf10) /Users/bflad/src/github.com/terraform-providers/terraform-provider-aws/aws/resource_aws_s3_bucket.go:2503 +0x3d6 github.com/hashicorp/terraform-plugin-sdk/helper/schema.(*Set).hash(0xc000e4bf00, 0x0, 0x0, 0x746172697078652e, 0x2e302e6e6f69) /Users/bflad/go/pkg/mod/github.com/hashicorp/terraform-plugin-sdk@v1.8.0/helper/schema/set.go:221 +0x3d ``` Output from acceptance testing: ``` --- PASS: TestAccAWSS3Bucket_acceleration (62.39s) --- PASS: TestAccAWSS3Bucket_AclToGrant (62.44s) --- PASS: TestAccAWSS3Bucket_basic (37.44s) --- PASS: TestAccAWSS3Bucket_Bucket_EmptyString (35.25s) --- PASS: TestAccAWSS3Bucket_Cors_Delete (30.46s) --- PASS: TestAccAWSS3Bucket_Cors_EmptyOrigin (37.46s) --- PASS: TestAccAWSS3Bucket_Cors_Update (64.62s) --- PASS: TestAccAWSS3Bucket_disableDefaultEncryption_whenDefaultEncryptionIsEnabled (62.63s) --- PASS: TestAccAWSS3Bucket_enableDefaultEncryption_whenAES256IsUsed (36.77s) --- PASS: TestAccAWSS3Bucket_enableDefaultEncryption_whenTypical (64.54s) --- PASS: TestAccAWSS3Bucket_forceDestroy (34.28s) --- PASS: TestAccAWSS3Bucket_forceDestroyWithEmptyPrefixes (33.82s) --- PASS: TestAccAWSS3Bucket_forceDestroyWithObjectLockEnabled (40.74s) --- PASS: TestAccAWSS3Bucket_generatedName (34.28s) --- PASS: TestAccAWSS3Bucket_GrantToAcl (55.10s) --- PASS: TestAccAWSS3Bucket_LifecycleBasic (85.90s) --- PASS: TestAccAWSS3Bucket_LifecycleExpireMarkerOnly (60.69s) --- PASS: TestAccAWSS3Bucket_LifecycleRule_Expiration_EmptyConfigurationBlock (29.75s) --- PASS: TestAccAWSS3Bucket_Logging (53.50s) --- PASS: TestAccAWSS3Bucket_namePrefix (34.09s) --- PASS: TestAccAWSS3Bucket_objectLock (61.56s) --- PASS: TestAccAWSS3Bucket_Policy (88.91s) --- PASS: TestAccAWSS3Bucket_region (35.91s) --- PASS: TestAccAWSS3Bucket_Replication (173.40s) --- PASS: TestAccAWSS3Bucket_ReplicationConfiguration_Rule_Destination_AccessControlTranslation (109.21s) --- PASS: TestAccAWSS3Bucket_ReplicationConfiguration_Rule_Destination_AddAccessControlTranslation (87.98s) --- PASS: TestAccAWSS3Bucket_ReplicationExpectVersioningValidationError (27.38s) --- PASS: TestAccAWSS3Bucket_ReplicationSchemaV2 (150.14s) --- PASS: TestAccAWSS3Bucket_ReplicationWithoutPrefix (51.20s) --- PASS: TestAccAWSS3Bucket_ReplicationWithoutStorageClass (51.04s) --- PASS: TestAccAWSS3Bucket_RequestPayer (61.18s) --- PASS: TestAccAWSS3Bucket_shouldFailNotFound (17.00s) --- PASS: TestAccAWSS3Bucket_tagsWithNoSystemTags (114.98s) --- PASS: TestAccAWSS3Bucket_tagsWithSystemTags (148.05s) --- PASS: TestAccAWSS3Bucket_UpdateAcl (60.53s) --- PASS: TestAccAWSS3Bucket_UpdateGrant (90.89s) --- PASS: TestAccAWSS3Bucket_Versioning (89.97s) --- PASS: TestAccAWSS3Bucket_Website_Simple (89.17s) --- PASS: TestAccAWSS3Bucket_WebsiteRedirect (89.72s) --- PASS: TestAccAWSS3Bucket_WebsiteRoutingRules (63.15s) ```
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.
LGTM 🚀
--- PASS: TestAccAWSS3Bucket_generatedName (16.79s)
--- PASS: TestAccAWSS3Bucket_region (17.14s)
--- PASS: TestAccAWSS3Bucket_Bucket_EmptyString (17.36s)
--- PASS: TestAccAWSS3Bucket_basic (17.59s)
--- PASS: TestAccAWSS3Bucket_namePrefix (18.07s)
--- PASS: TestAccAWSS3Bucket_enableDefaultEncryption_whenAES256IsUsed (18.71s)
--- PASS: TestAccAWSS3Bucket_AclToGrant (22.97s)
--- PASS: TestAccAWSS3Bucket_GrantToAcl (24.41s)
--- PASS: TestAccAWSS3Bucket_UpdateAcl (25.39s)
--- PASS: TestAccAWSS3Bucket_shouldFailNotFound (9.04s)
--- PASS: TestAccAWSS3Bucket_WebsiteRoutingRules (26.96s)
--- PASS: TestAccAWSS3Bucket_RequestPayer (27.44s)
--- PASS: TestAccAWSS3Bucket_Cors_Delete (12.84s)
--- PASS: TestAccAWSS3Bucket_acceleration (31.16s)
--- PASS: TestAccAWSS3Bucket_WebsiteRedirect (33.48s)
--- PASS: TestAccAWSS3Bucket_Cors_EmptyOrigin (14.90s)
--- PASS: TestAccAWSS3Bucket_Policy (34.30s)
--- PASS: TestAccAWSS3Bucket_Website_Simple (34.95s)
--- PASS: TestAccAWSS3Bucket_enableDefaultEncryption_whenTypical (36.80s)
--- PASS: TestAccAWSS3Bucket_Logging (17.68s)
--- PASS: TestAccAWSS3Bucket_LifecycleRule_Expiration_EmptyConfigurationBlock (13.72s)
--- PASS: TestAccAWSS3Bucket_UpdateGrant (40.04s)
--- PASS: TestAccAWSS3Bucket_disableDefaultEncryption_whenDefaultEncryptionIsEnabled (23.18s)
--- PASS: TestAccAWSS3Bucket_tagsWithNoSystemTags (41.39s)
--- PASS: TestAccAWSS3Bucket_Cors_Update (24.71s)
--- FAIL: TestAccAWSS3Bucket_LifecycleBasic (20.75s)
--- FAIL: TestAccAWSS3Bucket_LifecycleExpireMarkerOnly (21.44s)
--- PASS: TestAccAWSS3Bucket_forceDestroy (11.00s)
--- PASS: TestAccAWSS3Bucket_Versioning (28.88s)
--- PASS: TestAccAWSS3Bucket_forceDestroyWithEmptyPrefixes (10.80s)
--- PASS: TestAccAWSS3Bucket_forceDestroyWithObjectLockEnabled (12.35s)
--- PASS: TestAccAWSS3Bucket_objectLock (21.46s)
--- PASS: TestAccAWSS3Bucket_ReplicationExpectVersioningValidationError (26.47s)
--- PASS: TestAccAWSS3Bucket_ReplicationWithoutStorageClass (40.57s)
--- PASS: TestAccAWSS3Bucket_ReplicationWithoutPrefix (41.11s)
--- PASS: TestAccAWSS3Bucket_tagsWithSystemTags (79.52s)
--- PASS: TestAccAWSS3Bucket_ReplicationConfiguration_Rule_Destination_AddAccessControlTranslation (63.90s)
--- FAIL: TestAccAWSS3Bucket_ReplicationConfiguration_Rule_Destination_AccessControlTranslation (74.43s)
--- PASS: TestAccAWSS3Bucket_ReplicationSchemaV2 (107.28s)
--- PASS: TestAccAWSS3Bucket_Replication (130.23s)
Test failures are not related to this change:
TestAccAWSS3Bucket_LifecycleBasic
TestAccAWSS3Bucket_LifecycleExpireMarkerOnly
TestAccAWSS3Bucket_ReplicationConfiguration_Rule_Destination_AccessControlTranslation
This has been released in version 2.56.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 for triage. Thanks! |
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! |
Community Note
Closes #11420
Closes #12480
Release note for CHANGELOG:
This does not contain a fully reproducible configuration for #12480 after a few timeboxed attempts, but left a test that adds
access_control_translation
since that most closely mimics what was reported and was previously untested. The addition of an empty configuration block in the plan difference appears to be a bug in the Terraform Plugin SDK or Terraform core logic.If/when these various S3 configurations are potentially moved to their own resources, we should try to remove the Set hashing functions then. Generally they are unnecessary except in specific situations.
Previously:
Output from acceptance testing: