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

fix: Fix crash on update when cache_attributes block is removed for aws_storagegateway_smb_file_share #37611

Conversation

acwwat
Copy link
Contributor

@acwwat acwwat commented May 20, 2024

Description

This issue is to fix a crash when the cache_attributes block is removed from an aws_storagegateway_smb_file_share resource during an update.

While this PR will fix the crash, it will not address the issue of actually removing the settings due to limitations of the AWS API. Whether I don't pass in the object or pass in an object with without CacheStaleTimeoutInSeconds to the update API, AWS does not remove the previously set value. So it seems that once the attribute is set, you are always stuck with having to provide some value.

Relations

Closes #36385

References

n/a

Output from Acceptance Testing

$ TF_ACC=1 go1.22.2 test ./internal/service/storagegateway/... -v -count 1 -parallel 3 -run='TestAccStorageGatewaySMBFileShare_'  -timeout 360m
=== RUN   TestAccStorageGatewaySMBFileShare_Authentication_activeDirectory
=== PAUSE TestAccStorageGatewaySMBFileShare_Authentication_activeDirectory
=== RUN   TestAccStorageGatewaySMBFileShare_Authentication_guestAccess
=== PAUSE TestAccStorageGatewaySMBFileShare_Authentication_guestAccess
=== RUN   TestAccStorageGatewaySMBFileShare_accessBasedEnumeration
=== PAUSE TestAccStorageGatewaySMBFileShare_accessBasedEnumeration
=== RUN   TestAccStorageGatewaySMBFileShare_notificationPolicy
=== PAUSE TestAccStorageGatewaySMBFileShare_notificationPolicy
=== RUN   TestAccStorageGatewaySMBFileShare_defaultStorageClass
=== PAUSE TestAccStorageGatewaySMBFileShare_defaultStorageClass
=== RUN   TestAccStorageGatewaySMBFileShare_encryptedUpdate
=== PAUSE TestAccStorageGatewaySMBFileShare_encryptedUpdate
=== RUN   TestAccStorageGatewaySMBFileShare_fileShareName
=== PAUSE TestAccStorageGatewaySMBFileShare_fileShareName
=== RUN   TestAccStorageGatewaySMBFileShare_tags
=== PAUSE TestAccStorageGatewaySMBFileShare_tags
=== RUN   TestAccStorageGatewaySMBFileShare_guessMIMETypeEnabled
=== PAUSE TestAccStorageGatewaySMBFileShare_guessMIMETypeEnabled
=== RUN   TestAccStorageGatewaySMBFileShare_invalidUserList
=== PAUSE TestAccStorageGatewaySMBFileShare_invalidUserList
=== RUN   TestAccStorageGatewaySMBFileShare_kmsEncrypted
=== PAUSE TestAccStorageGatewaySMBFileShare_kmsEncrypted
=== RUN   TestAccStorageGatewaySMBFileShare_kmsKeyARN
=== PAUSE TestAccStorageGatewaySMBFileShare_kmsKeyARN
=== RUN   TestAccStorageGatewaySMBFileShare_objectACL
=== PAUSE TestAccStorageGatewaySMBFileShare_objectACL
=== RUN   TestAccStorageGatewaySMBFileShare_readOnly
=== PAUSE TestAccStorageGatewaySMBFileShare_readOnly
=== RUN   TestAccStorageGatewaySMBFileShare_requesterPays
=== PAUSE TestAccStorageGatewaySMBFileShare_requesterPays
=== RUN   TestAccStorageGatewaySMBFileShare_validUserList
=== PAUSE TestAccStorageGatewaySMBFileShare_validUserList
=== RUN   TestAccStorageGatewaySMBFileShare_SMB_acl
=== PAUSE TestAccStorageGatewaySMBFileShare_SMB_acl
=== RUN   TestAccStorageGatewaySMBFileShare_audit
=== PAUSE TestAccStorageGatewaySMBFileShare_audit
=== RUN   TestAccStorageGatewaySMBFileShare_cacheAttributes
=== PAUSE TestAccStorageGatewaySMBFileShare_cacheAttributes
=== RUN   TestAccStorageGatewaySMBFileShare_caseSensitivity
=== PAUSE TestAccStorageGatewaySMBFileShare_caseSensitivity
=== RUN   TestAccStorageGatewaySMBFileShare_disappears
=== PAUSE TestAccStorageGatewaySMBFileShare_disappears
=== RUN   TestAccStorageGatewaySMBFileShare_adminUserList
=== PAUSE TestAccStorageGatewaySMBFileShare_adminUserList
=== CONT  TestAccStorageGatewaySMBFileShare_Authentication_activeDirectory
=== CONT  TestAccStorageGatewaySMBFileShare_kmsKeyARN
=== CONT  TestAccStorageGatewaySMBFileShare_audit
--- PASS: TestAccStorageGatewaySMBFileShare_audit (269.05s)
=== CONT  TestAccStorageGatewaySMBFileShare_adminUserList
--- PASS: TestAccStorageGatewaySMBFileShare_kmsKeyARN (364.77s)
=== CONT  TestAccStorageGatewaySMBFileShare_disappears
--- PASS: TestAccStorageGatewaySMBFileShare_disappears (229.41s)
=== CONT  TestAccStorageGatewaySMBFileShare_caseSensitivity
--- PASS: TestAccStorageGatewaySMBFileShare_Authentication_activeDirectory (841.00s)
=== CONT  TestAccStorageGatewaySMBFileShare_cacheAttributes
--- PASS: TestAccStorageGatewaySMBFileShare_caseSensitivity (361.36s)
=== CONT  TestAccStorageGatewaySMBFileShare_fileShareName
--- PASS: TestAccStorageGatewaySMBFileShare_cacheAttributes (370.80s)
=== CONT  TestAccStorageGatewaySMBFileShare_kmsEncrypted
--- PASS: TestAccStorageGatewaySMBFileShare_fileShareName (300.34s)
=== CONT  TestAccStorageGatewaySMBFileShare_invalidUserList
--- PASS: TestAccStorageGatewaySMBFileShare_adminUserList (999.71s)
=== CONT  TestAccStorageGatewaySMBFileShare_guessMIMETypeEnabled
--- PASS: TestAccStorageGatewaySMBFileShare_kmsEncrypted (225.69s)
=== CONT  TestAccStorageGatewaySMBFileShare_tags
--- PASS: TestAccStorageGatewaySMBFileShare_guessMIMETypeEnabled (274.25s)
=== CONT  TestAccStorageGatewaySMBFileShare_notificationPolicy
--- PASS: TestAccStorageGatewaySMBFileShare_tags (272.12s)
=== CONT  TestAccStorageGatewaySMBFileShare_encryptedUpdate
--- PASS: TestAccStorageGatewaySMBFileShare_notificationPolicy (366.22s)
=== CONT  TestAccStorageGatewaySMBFileShare_defaultStorageClass
--- PASS: TestAccStorageGatewaySMBFileShare_encryptedUpdate (394.95s)
=== CONT  TestAccStorageGatewaySMBFileShare_accessBasedEnumeration
--- PASS: TestAccStorageGatewaySMBFileShare_defaultStorageClass (335.65s)
=== CONT  TestAccStorageGatewaySMBFileShare_requesterPays
--- PASS: TestAccStorageGatewaySMBFileShare_invalidUserList (1079.55s)
=== CONT  TestAccStorageGatewaySMBFileShare_SMB_acl
--- PASS: TestAccStorageGatewaySMBFileShare_accessBasedEnumeration (370.57s)
=== CONT  TestAccStorageGatewaySMBFileShare_validUserList
--- PASS: TestAccStorageGatewaySMBFileShare_requesterPays (332.82s)
=== CONT  TestAccStorageGatewaySMBFileShare_Authentication_guestAccess
--- PASS: TestAccStorageGatewaySMBFileShare_Authentication_guestAccess (228.15s)
=== CONT  TestAccStorageGatewaySMBFileShare_readOnly
--- PASS: TestAccStorageGatewaySMBFileShare_readOnly (314.57s)
=== CONT  TestAccStorageGatewaySMBFileShare_objectACL
--- PASS: TestAccStorageGatewaySMBFileShare_SMB_acl (1035.48s)
--- PASS: TestAccStorageGatewaySMBFileShare_validUserList (949.47s)
--- PASS: TestAccStorageGatewaySMBFileShare_objectACL (324.91s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/storagegateway     3445.537s

Copy link

Community Note

Voting for Prioritization

  • Please vote on this pull request by adding a 👍 reaction to the original post to help the community and maintainers prioritize this pull request.
  • Please see our prioritization guide for information on how we prioritize.
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.

For Submitters

  • Review the contribution guide relating to the type of change you are making to ensure all of the necessary steps have been taken.
  • For new resources and data sources, use skaff to generate scaffolding with comments detailing common expectations.
  • Whether or not the branch has been rebased will not impact prioritization, but doing so is always a welcome surprise.

@github-actions github-actions bot added size/XS Managed by automation to categorize the size of a PR. service/storagegateway Issues and PRs that pertain to the storagegateway service. labels May 20, 2024
@terraform-aws-provider terraform-aws-provider bot added needs-triage Waiting for first response or review from a maintainer. external-maintainer Contribution from a trusted external contributor. labels May 20, 2024
@acwwat acwwat force-pushed the b-aws_storagegateway_smb_file_share-fix_cache_attributes_crash branch from 649da52 to f8fdcc9 Compare May 20, 2024 17:30
@acwwat acwwat changed the title [WIP] fix: Fix crash on updated when cache_attributes block is removed for aws_storagegateway_smb_file_share fix: Fix crash on updated when cache_attributes block is removed for aws_storagegateway_smb_file_share May 20, 2024
@acwwat acwwat force-pushed the b-aws_storagegateway_smb_file_share-fix_cache_attributes_crash branch from f8fdcc9 to 2278868 Compare May 20, 2024 18:37
@justinretzolk justinretzolk added bug Addresses a defect in current functionality. crash Results from or addresses a Terraform crash or kernel panic. and removed needs-triage Waiting for first response or review from a maintainer. labels May 20, 2024
@acwwat acwwat changed the title fix: Fix crash on updated when cache_attributes block is removed for aws_storagegateway_smb_file_share fix: Fix crash on update when cache_attributes block is removed for aws_storagegateway_smb_file_share May 20, 2024
@github-actions github-actions bot added the documentation Introduces or discusses updates to documentation. label May 20, 2024
Copy link
Contributor

@ewbankkit ewbankkit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚀.

% ACCTEST_TIMEOUT=720m make testacc TESTARGS='-run=TestAccStorageGatewayCache_\|TestAccStorageGatewaySMBFileShare_' PKG=storagegateway ACCTEST_PARALLELISM=3
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go1.22.2 test ./internal/service/storagegateway/... -v -count 1 -parallel 3  -run=TestAccStorageGatewayCache_\|TestAccStorageGatewaySMBFileShare_ -timeout 720m
=== RUN   TestAccStorageGatewayCache_fileGateway
=== PAUSE TestAccStorageGatewayCache_fileGateway
=== RUN   TestAccStorageGatewayCache_tapeAndVolumeGateway
=== PAUSE TestAccStorageGatewayCache_tapeAndVolumeGateway
=== RUN   TestAccStorageGatewaySMBFileShare_Authentication_activeDirectory
=== PAUSE TestAccStorageGatewaySMBFileShare_Authentication_activeDirectory
=== RUN   TestAccStorageGatewaySMBFileShare_Authentication_guestAccess
=== PAUSE TestAccStorageGatewaySMBFileShare_Authentication_guestAccess
=== RUN   TestAccStorageGatewaySMBFileShare_accessBasedEnumeration
=== PAUSE TestAccStorageGatewaySMBFileShare_accessBasedEnumeration
=== RUN   TestAccStorageGatewaySMBFileShare_notificationPolicy
=== PAUSE TestAccStorageGatewaySMBFileShare_notificationPolicy
=== RUN   TestAccStorageGatewaySMBFileShare_defaultStorageClass
=== PAUSE TestAccStorageGatewaySMBFileShare_defaultStorageClass
=== RUN   TestAccStorageGatewaySMBFileShare_encryptedUpdate
=== PAUSE TestAccStorageGatewaySMBFileShare_encryptedUpdate
=== RUN   TestAccStorageGatewaySMBFileShare_fileShareName
=== PAUSE TestAccStorageGatewaySMBFileShare_fileShareName
=== RUN   TestAccStorageGatewaySMBFileShare_tags
=== PAUSE TestAccStorageGatewaySMBFileShare_tags
=== RUN   TestAccStorageGatewaySMBFileShare_guessMIMETypeEnabled
=== PAUSE TestAccStorageGatewaySMBFileShare_guessMIMETypeEnabled
=== RUN   TestAccStorageGatewaySMBFileShare_invalidUserList
=== PAUSE TestAccStorageGatewaySMBFileShare_invalidUserList
=== RUN   TestAccStorageGatewaySMBFileShare_kmsEncrypted
=== PAUSE TestAccStorageGatewaySMBFileShare_kmsEncrypted
=== RUN   TestAccStorageGatewaySMBFileShare_kmsKeyARN
=== PAUSE TestAccStorageGatewaySMBFileShare_kmsKeyARN
=== RUN   TestAccStorageGatewaySMBFileShare_objectACL
=== PAUSE TestAccStorageGatewaySMBFileShare_objectACL
=== RUN   TestAccStorageGatewaySMBFileShare_readOnly
=== PAUSE TestAccStorageGatewaySMBFileShare_readOnly
=== RUN   TestAccStorageGatewaySMBFileShare_requesterPays
=== PAUSE TestAccStorageGatewaySMBFileShare_requesterPays
=== RUN   TestAccStorageGatewaySMBFileShare_validUserList
=== PAUSE TestAccStorageGatewaySMBFileShare_validUserList
=== RUN   TestAccStorageGatewaySMBFileShare_SMB_acl
=== PAUSE TestAccStorageGatewaySMBFileShare_SMB_acl
=== RUN   TestAccStorageGatewaySMBFileShare_audit
=== PAUSE TestAccStorageGatewaySMBFileShare_audit
=== RUN   TestAccStorageGatewaySMBFileShare_cacheAttributes
=== PAUSE TestAccStorageGatewaySMBFileShare_cacheAttributes
=== RUN   TestAccStorageGatewaySMBFileShare_caseSensitivity
=== PAUSE TestAccStorageGatewaySMBFileShare_caseSensitivity
=== RUN   TestAccStorageGatewaySMBFileShare_disappears
=== PAUSE TestAccStorageGatewaySMBFileShare_disappears
=== RUN   TestAccStorageGatewaySMBFileShare_adminUserList
=== PAUSE TestAccStorageGatewaySMBFileShare_adminUserList
=== CONT  TestAccStorageGatewayCache_fileGateway
=== CONT  TestAccStorageGatewaySMBFileShare_kmsEncrypted
=== CONT  TestAccStorageGatewaySMBFileShare_defaultStorageClass
--- PASS: TestAccStorageGatewayCache_fileGateway (241.64s)
=== CONT  TestAccStorageGatewaySMBFileShare_tags
--- PASS: TestAccStorageGatewaySMBFileShare_kmsEncrypted (249.25s)
=== CONT  TestAccStorageGatewaySMBFileShare_invalidUserList
--- PASS: TestAccStorageGatewaySMBFileShare_defaultStorageClass (308.85s)
=== CONT  TestAccStorageGatewaySMBFileShare_guessMIMETypeEnabled
--- PASS: TestAccStorageGatewaySMBFileShare_tags (290.50s)
=== CONT  TestAccStorageGatewaySMBFileShare_fileShareName
--- PASS: TestAccStorageGatewaySMBFileShare_guessMIMETypeEnabled (298.78s)
=== CONT  TestAccStorageGatewaySMBFileShare_SMB_acl
--- PASS: TestAccStorageGatewaySMBFileShare_fileShareName (326.50s)
=== CONT  TestAccStorageGatewaySMBFileShare_adminUserList
--- PASS: TestAccStorageGatewaySMBFileShare_invalidUserList (1057.16s)
=== CONT  TestAccStorageGatewaySMBFileShare_disappears
--- PASS: TestAccStorageGatewaySMBFileShare_disappears (303.32s)
=== CONT  TestAccStorageGatewaySMBFileShare_caseSensitivity
--- PASS: TestAccStorageGatewaySMBFileShare_SMB_acl (1075.68s)
=== CONT  TestAccStorageGatewaySMBFileShare_cacheAttributes
--- PASS: TestAccStorageGatewaySMBFileShare_adminUserList (1033.58s)
=== CONT  TestAccStorageGatewaySMBFileShare_audit
--- PASS: TestAccStorageGatewaySMBFileShare_caseSensitivity (373.32s)
=== CONT  TestAccStorageGatewaySMBFileShare_readOnly
--- PASS: TestAccStorageGatewaySMBFileShare_cacheAttributes (342.50s)
=== CONT  TestAccStorageGatewaySMBFileShare_validUserList
--- PASS: TestAccStorageGatewaySMBFileShare_audit (306.74s)
=== CONT  TestAccStorageGatewaySMBFileShare_requesterPays
--- PASS: TestAccStorageGatewaySMBFileShare_readOnly (249.68s)
=== CONT  TestAccStorageGatewaySMBFileShare_objectACL
--- PASS: TestAccStorageGatewaySMBFileShare_requesterPays (336.83s)
=== CONT  TestAccStorageGatewaySMBFileShare_kmsKeyARN
--- PASS: TestAccStorageGatewaySMBFileShare_objectACL (303.08s)
=== CONT  TestAccStorageGatewaySMBFileShare_Authentication_guestAccess
--- PASS: TestAccStorageGatewaySMBFileShare_Authentication_guestAccess (249.97s)
=== CONT  TestAccStorageGatewaySMBFileShare_notificationPolicy
--- PASS: TestAccStorageGatewaySMBFileShare_kmsKeyARN (352.58s)
=== CONT  TestAccStorageGatewaySMBFileShare_accessBasedEnumeration
--- PASS: TestAccStorageGatewaySMBFileShare_validUserList (1074.64s)
=== CONT  TestAccStorageGatewaySMBFileShare_encryptedUpdate
--- PASS: TestAccStorageGatewaySMBFileShare_notificationPolicy (393.02s)
=== CONT  TestAccStorageGatewaySMBFileShare_Authentication_activeDirectory
--- PASS: TestAccStorageGatewaySMBFileShare_accessBasedEnumeration (394.31s)
=== CONT  TestAccStorageGatewayCache_tapeAndVolumeGateway
--- PASS: TestAccStorageGatewaySMBFileShare_encryptedUpdate (317.78s)
--- PASS: TestAccStorageGatewayCache_tapeAndVolumeGateway (332.54s)
--- PASS: TestAccStorageGatewaySMBFileShare_Authentication_activeDirectory (932.44s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/storagegateway	4115.908s

@ewbankkit
Copy link
Contributor

@acwwat Thanks for the contribution 🎉 👏.

@ewbankkit ewbankkit merged commit b15dbc9 into hashicorp:main May 22, 2024
43 checks passed
@github-actions github-actions bot added this to the v5.51.0 milestone May 22, 2024
Copy link

This functionality has been released in v5.51.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!

@acwwat acwwat deleted the b-aws_storagegateway_smb_file_share-fix_cache_attributes_crash branch May 25, 2024 02:13
Copy link

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.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Addresses a defect in current functionality. crash Results from or addresses a Terraform crash or kernel panic. documentation Introduces or discusses updates to documentation. external-maintainer Contribution from a trusted external contributor. service/storagegateway Issues and PRs that pertain to the storagegateway service. size/XS Managed by automation to categorize the size of a PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Unable to update storage gateway SMB acl_enabled flag
3 participants