-
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
r/aws_backup_vault_lock_configuration: New resource #21315
r/aws_backup_vault_lock_configuration: New resource #21315
Conversation
You've hit us right in the middle of our biggest change in years. Hopefully it will make like much easier but it is change. Please see #20000 for more guidance. |
Hi @YakDriver thanks for the update! I think I started working on this PR after the many changes made yesterday and referenced similar resources like |
Pull request #21306 has significantly refactored the AWS Provider codebase. As a result, most PRs opened prior to the refactor now have merge conflicts that must be resolved before proceeding. Specifically, PR #21306 relocated the code for all AWS resources and data sources from a single We recognize that many pull requests have been open for some time without yet being addressed by our maintainers. Therefore, we want to make it clear that resolving these conflicts in no way affects the prioritization of a particular pull request. Once a pull request has been prioritized for review, the necessary changes will be made by a maintainer -- either directly or in collaboration with the pull request author. For a more complete description of this refactor, including examples of how old filepaths and function names correspond to their new counterparts: please refer to issue #20000. For a quick guide on how to amend your pull request to resolve the merge conflicts resulting from this refactor and bring it in line with our new code patterns: please refer to our Service Package Refactor Pull Request Guide. |
follows the same approach used in vault_notifications
use DescribeBackupVault to verify properties since BackupVaultLockConfiguration currently does not have a GetBackupVaultLockConfiguration
d9c7762
to
f78e8b8
Compare
Acceptance test output: % make testacc TESTARGS='-run=TestAccBackupVaultLockConfiguration_' PKG_NAME=internal/service/backup ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./internal/service/backup/... -v -count 1 -parallel 20 -run=TestAccBackupVaultLockConfiguration_ -timeout 180m === RUN TestAccBackupVaultLockConfiguration_basic === PAUSE TestAccBackupVaultLockConfiguration_basic === RUN TestAccBackupVaultLockConfiguration_disappears === PAUSE TestAccBackupVaultLockConfiguration_disappears === CONT TestAccBackupVaultLockConfiguration_disappears === CONT TestAccBackupVaultLockConfiguration_basic --- PASS: TestAccBackupVaultLockConfiguration_disappears (18.28s) --- PASS: TestAccBackupVaultLockConfiguration_basic (20.16s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/backup 25.551s
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 🚀.
Commercial
% make testacc TESTARGS='-run=TestAccBackupVaultLockConfiguration_\|TestAccBackupVault_' PKG_NAME=internal/service/backup
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/backup/... -v -count 1 -parallel 20 -run=TestAccBackupVaultLockConfiguration_\|TestAccBackupVault_ -timeout 180m
=== RUN TestAccBackupVaultLockConfiguration_basic
=== PAUSE TestAccBackupVaultLockConfiguration_basic
=== RUN TestAccBackupVaultLockConfiguration_disappears
=== PAUSE TestAccBackupVaultLockConfiguration_disappears
=== RUN TestAccBackupVault_basic
=== PAUSE TestAccBackupVault_basic
=== RUN TestAccBackupVault_withKMSKey
=== PAUSE TestAccBackupVault_withKMSKey
=== RUN TestAccBackupVault_withTags
=== PAUSE TestAccBackupVault_withTags
=== RUN TestAccBackupVault_disappears
=== PAUSE TestAccBackupVault_disappears
=== CONT TestAccBackupVaultLockConfiguration_basic
=== CONT TestAccBackupVault_basic
=== CONT TestAccBackupVaultLockConfiguration_disappears
=== CONT TestAccBackupVault_withKMSKey
=== CONT TestAccBackupVault_disappears
=== CONT TestAccBackupVault_withTags
--- PASS: TestAccBackupVault_disappears (25.79s)
--- PASS: TestAccBackupVaultLockConfiguration_disappears (33.12s)
--- PASS: TestAccBackupVault_basic (33.38s)
--- PASS: TestAccBackupVaultLockConfiguration_basic (36.30s)
--- PASS: TestAccBackupVault_withKMSKey (37.76s)
--- PASS: TestAccBackupVault_withTags (60.20s)
PASS
ok github.com/hashicorp/terraform-provider-aws/internal/service/backup 67.282s
GovCloud
% make testacc TESTARGS='-run=TestAccBackupVaultLockConfiguration_\|TestAccBackupVault_' PKG_NAME=internal/service/backup
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/backup/... -v -count 1 -parallel 20 -run=TestAccBackupVaultLockConfiguration_\|TestAccBackupVault_ -timeout 180m
=== RUN TestAccBackupVaultLockConfiguration_basic
=== PAUSE TestAccBackupVaultLockConfiguration_basic
=== RUN TestAccBackupVaultLockConfiguration_disappears
=== PAUSE TestAccBackupVaultLockConfiguration_disappears
=== RUN TestAccBackupVault_basic
=== PAUSE TestAccBackupVault_basic
=== RUN TestAccBackupVault_withKMSKey
=== PAUSE TestAccBackupVault_withKMSKey
=== RUN TestAccBackupVault_withTags
=== PAUSE TestAccBackupVault_withTags
=== RUN TestAccBackupVault_disappears
=== PAUSE TestAccBackupVault_disappears
=== CONT TestAccBackupVaultLockConfiguration_disappears
=== CONT TestAccBackupVaultLockConfiguration_basic
=== CONT TestAccBackupVault_withKMSKey
=== CONT TestAccBackupVault_withTags
=== CONT TestAccBackupVault_disappears
=== CONT TestAccBackupVault_basic
--- PASS: TestAccBackupVault_disappears (29.20s)
--- PASS: TestAccBackupVaultLockConfiguration_disappears (34.85s)
--- PASS: TestAccBackupVault_basic (35.22s)
--- PASS: TestAccBackupVaultLockConfiguration_basic (38.28s)
--- PASS: TestAccBackupVault_withKMSKey (39.67s)
--- PASS: TestAccBackupVault_withTags (65.79s)
PASS
ok github.com/hashicorp/terraform-provider-aws/internal/service/backup 79.078s
@GlennChia Thanks for the contribution 🎉 👏. |
AWS Provided. |
This functionality has been released in v3.64.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. |
Community Note
Relates OR Closes #21235
Output from acceptance testing (Using the most up-to-date testing command with reference to #20000):
Requested guidance
aws_backup_vault_notifications
heavily (At the time I reference it, it was after the changes mentioned in Big Changes: AWS Provider Refactor #20000). Would appreciate PR feedback and guidanceDescribeBackupVault
which can verifyBackupVaultName
. Note thatchangeable_for_days
is not returned by the API and I useImportStateVerifyIgnore: []string{"changeable_for_days"},
to account for thisDescribeBackupVault
to set themax_retention_days
andmin_retention_days
in aresourceVaultLockConfigurationRead
function. Note that it doesn't have information aboutchangeable_for_days
so that one remains empty (Reference: https://pkg.go.dev/github.com/aws/aws-sdk-go-v2/service/backup). Is that the right approach or should I comment out theresourceVaultLockConfigurationRead
function?Importer: &schema.ResourceImporter{
and wanted to know if this will handle the imports somehow behind the scenes?terraform providers schema
andproviderlint
error messages? It seems like it should be fixed outside this PR? As forgo generate
, it suggestedUnexpected difference in directories after code generation. Run 'make gen' command and commit.
, should I follow this suggestion?Thanks for your help!