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

r/aws_s3_bucket_lifecycle_configuration: remove expired_object_delete_marker plan modifier #41462

Merged
merged 4 commits into from
Feb 20, 2025

Conversation

jar-b
Copy link
Member

@jar-b jar-b commented Feb 19, 2025

Description

This change should enable configurations with the rule.exipration.expired_object_delete_maker set alongside one of the other nested arguments (date or days) to remove the configured argument in order to resolve inconsistent result after apply errors. Previously, expired_object_delete_marker must have been explicitly set to false in order to resolve the issue.

Also introduces a warning diagnostic to provide additional context when arguments are provided together that may produce an inconsistent result after apply error. In practice the diagnostic will appear as follows.

│ Warning: Invalid Attribute Combination
│
│   with aws_s3_bucket_lifecycle_configuration.test,
│   on main.tf line 33, in resource "aws_s3_bucket_lifecycle_configuration" "test":
│   33:       expired_object_delete_marker = true
│
│ Attribute "rule[0].expiration[0].expired_object_delete_marker" should not be specified when "rule[0].expiration[0].days" is also specified

Relations

Relates #41126
Relates #41159
Relates #41277

References

Output from Acceptance Testing

% make testacc PKG=s3 TESTS=TestAccS3BucketLifecycleConfiguration_ ACCTEST_PARALLELISM=10
make: Verifying source code with gofmt...
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go1.23.5 test ./internal/service/s3/... -v -count 1 -parallel 10 -run='TestAccS3BucketLifecycleConfiguration_'  -timeout 360m -vet=off
2025/02/19 09:46:39 Initializing Terraform AWS Provider...

--- PASS: TestAccS3BucketLifecycleConfiguration_Filter_Tag (45.32s)
=== CONT  TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_RuleExpiration_expireMarkerOnly
--- PASS: TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_RulePrefix (98.53s)
=== CONT  TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_Filter_And_Tags
--- PASS: TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_nonCurrentVersionExpiration (107.94s)
=== CONT  TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_TransitionDate
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_TransitionStorageClassOnly_intelligentTiering (110.64s)
=== CONT  TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_EmptyFilter_NonCurrentVersions_WithChange
--- PASS: TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_FilterWithPrefix (117.73s)
=== CONT  TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_RulePrefix
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_ruleAbortIncompleteMultipartUpload (126.12s)
=== CONT  TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_RuleExpiration_emptyBlock
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_basic (126.67s)
=== CONT  TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_EmptyFilter_NonCurrentVersions
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_EmptyFilter_NonCurrentVersions_WithChange (136.66s)
=== CONT  TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_Filter_Tag
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_nonCurrentVersionExpiration (136.81s)
=== CONT  TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_TransitionStorageClassOnly_intelligentTiering
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_EmptyFilter_NonCurrentVersions (147.88s)
=== CONT  TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_Filter_ObjectSizeRange
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_RuleExpiration_expireMarkerOnly (107.25s)
=== CONT  TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_TransitionZeroDays_intelligentTiering
--- PASS: TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_Filter_And_Tags (71.87s)
=== CONT  TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_Filter_ObjectSizeRangeAndPrefix
--- PASS: TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_EmptyFilter_NonCurrentVersions (74.22s)
=== CONT  TestAccS3BucketLifecycleConfiguration_Filter_ObjectSizeRange
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_TransitionDate (93.15s)
=== CONT  TestAccS3BucketLifecycleConfiguration_EmptyFilter_NonCurrentVersions
--- PASS: TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_EmptyFilter_NonCurrentVersions_WithChange (96.42s)
=== CONT  TestAccS3BucketLifecycleConfiguration_RulePrefix
--- PASS: TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_TransitionStorageClassOnly_intelligentTiering (77.10s)
=== CONT  TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_basic
--- PASS: TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_Filter_Tag (80.41s)
=== CONT  TestAccS3BucketLifecycleConfiguration_nonCurrentVersionTransition
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_RuleExpiration_emptyBlock (92.54s)
=== CONT  TestAccS3BucketLifecycleConfiguration_transitionDefaultMinimumObjectSize_remove
--- PASS: TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_Filter_ObjectSizeRange (70.97s)
=== CONT  TestAccS3BucketLifecycleConfiguration_nonCurrentVersionExpiration
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_RulePrefix (102.19s)
=== CONT  TestAccS3BucketLifecycleConfiguration_transitionDefaultMinimumObjectSize_update
--- PASS: TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_TransitionZeroDays_intelligentTiering (71.37s)
=== CONT  TestAccS3BucketLifecycleConfiguration_directoryBucket
--- PASS: TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_Filter_ObjectSizeRangeAndPrefix (69.08s)
=== CONT  TestAccS3BucketLifecycleConfiguration_multipleRules_noFilterOrPrefix
--- PASS: TestAccS3BucketLifecycleConfiguration_Filter_ObjectSizeRange (41.24s)
=== CONT  TestAccS3BucketLifecycleConfiguration_Update_filterWithAndToFilterWithPrefix
--- PASS: TestAccS3BucketLifecycleConfiguration_RulePrefix (51.12s)
=== CONT  TestAccS3BucketLifecycleConfiguration_migrate_withChange
--- PASS: TestAccS3BucketLifecycleConfiguration_nonCurrentVersionTransition (41.39s)
=== CONT  TestAccS3BucketLifecycleConfiguration_migrate_noChange
--- PASS: TestAccS3BucketLifecycleConfiguration_nonCurrentVersionExpiration (41.18s)
=== CONT  TestAccS3BucketLifecycleConfiguration_multipleRules
--- PASS: TestAccS3BucketLifecycleConfiguration_directoryBucket (42.99s)
=== CONT  TestAccS3BucketLifecycleConfiguration_TransitionDate_intelligentTiering
--- PASS: TestAccS3BucketLifecycleConfiguration_EmptyFilter_NonCurrentVersions (78.21s)
=== CONT  TestAccS3BucketLifecycleConfiguration_disableRule
--- PASS: TestAccS3BucketLifecycleConfiguration_multipleRules_noFilterOrPrefix (40.65s)
=== CONT  TestAccS3BucketLifecycleConfiguration_TransitionUpdateBetweenDaysAndDate_intelligentTiering
--- PASS: TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_basic (69.08s)
=== CONT  TestAccS3BucketLifecycleConfiguration_Filter_ObjectSizeRangeAndPrefix
--- PASS: TestAccS3BucketLifecycleConfiguration_transitionDefaultMinimumObjectSize_update (77.73s)
=== CONT  TestAccS3BucketLifecycleConfiguration_TransitionZeroDays_intelligentTiering
--- PASS: TestAccS3BucketLifecycleConfiguration_transitionDefaultMinimumObjectSize_remove (79.19s)
=== CONT  TestAccS3BucketLifecycleConfiguration_ruleAbortIncompleteMultipartUpload
--- PASS: TestAccS3BucketLifecycleConfiguration_multipleRules (40.64s)
=== CONT  TestAccS3BucketLifecycleConfiguration_TransitionStorageClassOnly_intelligentTiering
--- PASS: TestAccS3BucketLifecycleConfiguration_migrate_withChange (48.76s)
=== CONT  TestAccS3BucketLifecycleConfiguration_TransitionDate_standardIa
--- PASS: TestAccS3BucketLifecycleConfiguration_migrate_noChange (49.11s)
=== CONT  TestAccS3BucketLifecycleConfiguration_RuleExpiration_emptyBlock
--- PASS: TestAccS3BucketLifecycleConfiguration_TransitionDate_intelligentTiering (40.86s)
=== CONT  TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_Filter_ObjectSizeRange
--- PASS: TestAccS3BucketLifecycleConfiguration_Update_filterWithAndToFilterWithPrefix (75.07s)
=== CONT  TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_TransitionDate
--- PASS: TestAccS3BucketLifecycleConfiguration_Filter_ObjectSizeRangeAndPrefix (43.37s)
=== CONT  TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_Filter_Tag
--- PASS: TestAccS3BucketLifecycleConfiguration_TransitionZeroDays_intelligentTiering (42.70s)
=== CONT  TestAccS3BucketLifecycleConfiguration_filterWithEmptyPrefix
--- PASS: TestAccS3BucketLifecycleConfiguration_TransitionStorageClassOnly_intelligentTiering (42.59s)
=== CONT  TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_Filter_And_Tags
--- PASS: TestAccS3BucketLifecycleConfiguration_TransitionDate_standardIa (42.19s)
=== CONT  TestAccS3BucketLifecycleConfiguration_Filter_ObjectSizeLessThan
--- PASS: TestAccS3BucketLifecycleConfiguration_RuleExpiration_emptyBlock (43.35s)
=== CONT  TestAccS3BucketLifecycleConfiguration_Filter_ObjectSizeGreaterThanZero
--- PASS: TestAccS3BucketLifecycleConfiguration_ruleAbortIncompleteMultipartUpload (78.46s)
=== CONT  TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_Filter_ObjectSizeRangeAndPrefix
--- PASS: TestAccS3BucketLifecycleConfiguration_filterWithEmptyPrefix (41.83s)
=== CONT  TestAccS3BucketLifecycleConfiguration_Filter_ObjectSizeGreaterThan
--- PASS: TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_TransitionDate (69.40s)
=== CONT  TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_RuleExpiration_expireMarkerOnly
--- PASS: TestAccS3BucketLifecycleConfiguration_Filter_ObjectSizeLessThan (41.97s)
=== CONT  TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_Filter_ObjectSizeGreaterThan
--- PASS: TestAccS3BucketLifecycleConfiguration_Filter_ObjectSizeGreaterThanZero (42.92s)
=== CONT  TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_RuleExpiration_emptyBlock
--- PASS: TestAccS3BucketLifecycleConfiguration_TransitionUpdateBetweenDaysAndDate_intelligentTiering (117.15s)
=== CONT  TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_Filter_ObjectSizeLessThan
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_Filter_ObjectSizeRange (96.53s)
=== CONT  TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_filterWithEmptyPrefix
--- PASS: TestAccS3BucketLifecycleConfiguration_disableRule (129.69s)
=== CONT  TestAccS3BucketLifecycleConfiguration_disappears
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_Filter_Tag (93.47s)
=== CONT  TestAccS3BucketLifecycleConfiguration_filterWithPrefix
--- PASS: TestAccS3BucketLifecycleConfiguration_Filter_ObjectSizeGreaterThan (41.83s)
=== CONT  TestAccS3BucketLifecycleConfiguration_RuleExpiration_expireMarkerOnly
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_Filter_And_Tags (91.97s)
=== CONT  TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_FilterWithPrefix
--- PASS: TestAccS3BucketLifecycleConfiguration_disappears (39.17s)
=== CONT  TestAccS3BucketLifecycleConfiguration_basic
--- PASS: TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_RuleExpiration_expireMarkerOnly (72.50s)
=== CONT  TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_ruleAbortIncompleteMultipartUpload
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_Filter_ObjectSizeRangeAndPrefix (91.95s)
=== CONT  TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_Filter_ObjectSizeLessThan
--- PASS: TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_RuleExpiration_emptyBlock (74.66s)
=== CONT  TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_Filter_ObjectSizeGreaterThan
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_Filter_ObjectSizeGreaterThan (95.97s)
=== CONT  TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_TransitionZeroDays_intelligentTiering
--- PASS: TestAccS3BucketLifecycleConfiguration_basic (43.20s)
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_Filter_ObjectSizeLessThan (100.62s)
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_filterWithEmptyPrefix (93.93s)
--- PASS: TestAccS3BucketLifecycleConfiguration_filterWithPrefix (81.38s)
--- PASS: TestAccS3BucketLifecycleConfiguration_RuleExpiration_expireMarkerOnly (79.60s)
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_FilterWithPrefix (89.24s)
--- PASS: TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_ruleAbortIncompleteMultipartUpload (68.55s)
--- PASS: TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_Filter_ObjectSizeLessThan (70.30s)
--- PASS: TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_Filter_ObjectSizeGreaterThan (74.09s)
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_TransitionZeroDays_intelligentTiering (87.45s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/s3 581.313s

…e_marker` plan modifier

This change should enable configurations with the `rule.exipration.expired_object_delete_maker` set alongside one of the other nested arguments (`date` or `days`) to remove the configured argument in order to resolve inconsistent result after apply errors. Previously, `expired_object_delete_marker` must have been explicitly set to `false` in order to resolve the issue.

```console
% make testacc PKG=s3 TESTS=TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_ ACCTEST_PARALLELISM=10
make: Verifying source code with gofmt...
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go1.23.5 test ./internal/service/s3/... -v -count 1 -parallel 10 -run='TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_'  -timeout 360m -vet=off
2025/02/19 09:27:36 Initializing Terraform AWS Provider...

--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_RulePrefix (101.47s)
=== CONT  TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_Filter_ObjectSizeRangeAndPrefix
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_EmptyFilter_NonCurrentVersions (110.59s)
=== CONT  TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_ruleAbortIncompleteMultipartUpload
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_TransitionDate (115.76s)
=== CONT  TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_nonCurrentVersionExpiration
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_RuleExpiration_expireMarkerOnly (122.87s)
=== CONT  TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_Filter_ObjectSizeGreaterThan
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_Filter_ObjectSizeRange (128.07s)
=== CONT  TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_Filter_ObjectSizeLessThan
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_basic (128.56s)
=== CONT  TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_Filter_And_Tags
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_RuleExpiration_emptyBlock (134.26s)
=== CONT  TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_Filter_Tag
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_TransitionZeroDays_intelligentTiering (143.71s)
=== CONT  TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_filterWithEmptyPrefix
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_TransitionStorageClassOnly_intelligentTiering (149.18s)
=== CONT  TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_FilterWithPrefix
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_EmptyFilter_NonCurrentVersions_WithChange (158.83s)
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_Filter_ObjectSizeRangeAndPrefix (88.74s)
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_ruleAbortIncompleteMultipartUpload (90.50s)
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_nonCurrentVersionExpiration (88.80s)
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_Filter_ObjectSizeGreaterThan (88.46s)
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_Filter_ObjectSizeLessThan (91.46s)
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_Filter_And_Tags (95.90s)
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_Filter_Tag (95.31s)
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_filterWithEmptyPrefix (90.50s)
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_FilterWithPrefix (99.94s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/s3 255.916s
```
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 tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. service/s3 Issues and PRs that pertain to the s3 service. prioritized Part of the maintainer teams immediate focus. To be addressed within the current quarter. labels Feb 19, 2025
This validator will be used with the `rule.exipration.expired_object_delete_marker` attribute to provide practitioners with additional context to prevent `inconsistent result after apply` errors.

For example, an invalid configuration which was previously allowed in
versions `<5.85.0` by the legacy Plugin-SDK data handling will now
result in an output like the following after upgrade.

```
│ Warning: Invalid Attribute Combination
│
│   with aws_s3_bucket_lifecycle_configuration.test,
│   on main.tf line 33, in resource "aws_s3_bucket_lifecycle_configuration" "test":
│   33:       expired_object_delete_marker = true
│
│ Attribute "rule[0].expiration[0].expired_object_delete_marker" should not be specified when "rule[0].expiration[0].days" is also specified
│
│ (and 2 more similar warnings elsewhere)
╵
╷
│ Error: Provider produced inconsistent result after apply
│
│ When applying changes to aws_s3_bucket_lifecycle_configuration.test, provider "provider[\"registry.terraform.io/hashicorp/aws\"]" produced an unexpected new value:
│ .rule[0].expiration[0].expired_object_delete_marker: was cty.True, but now cty.False.
│
│ This is a bug in the provider, which should be reported in the provider's own issue tracker.
```

The warning diagnostic supplements the hard `inconsistent result` error
with information the user can utilize to resolve the issue.
@jar-b jar-b marked this pull request as ready for review February 19, 2025 21:04
@jar-b jar-b requested a review from a team as a code owner February 19, 2025 21:04
@gdavison gdavison self-assigned this Feb 19, 2025
@@ -1964,6 +1964,18 @@ func checkExpiration_Days(days int32) knownvalue.Check {
})
}

func checkExpiration_DaysAfterMigration(days int32) knownvalue.Check {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should expirationDefaults be updated to set expired_object_delete_marker to knownvalue.Null() instead of knownvalue.Bool(false)? That would be more consistent, but change the behaviour from the SDK version

Copy link
Member Author

Choose a reason for hiding this comment

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

I initially went down the path of changing expirationDefaults, but the value still seems to becomes a known false boolean due to the legacy AutoFlex flattening. Here is the failure I kept running into during acceptance testing.

% make testacc PKG=s3 TESTS=TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_basic
make: Verifying source code with gofmt...
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go1.23.5 test ./internal/service/s3/... -v -count 1 -parallel 20 -run='TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_basic'  -timeout 360m -vet=off
2025/02/19 20:30:23 Initializing Terraform AWS Provider...

    bucket_lifecycle_configuration_migrate_v0_test.go:52: Step 2/2 error: Pre-apply plan check(s) failed:
        checking old value for attribute at path: aws_s3_bucket_lifecycle_configuration.test.rule.0.expiration, err: list element index 0: expired_object_delete_marker object attribute: expected nil value for Null check, got: bool
    panic.go:629: Error retrieving state, there may be dangling resources: exit status 1
        Failed to marshal state to json: schema version 0 for aws_s3_bucket_lifecycle_configuration.test in state does not match version 1 from the provider
--- FAIL: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_basic (77.57s)
FAIL
FAIL    github.com/hashicorp/terraform-provider-aws/internal/service/s3 84.229s

I very well may be misunderstanding the timing of these checks, but all of the variations I tried which modified expirationDefaults seemed to have some other knock on effect like this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, we'd have to remove legacy handling

Comment on lines +1100 to +1102
// Validation logic is adapted from the standard ConflictsWith validator
// available for all types
func (v warnIfSetWithValidator) ValidateBool(ctx context.Context, req validator.BoolRequest, resp *validator.BoolResponse) {
Copy link
Member Author

Choose a reason for hiding this comment

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

I've left this alongside the resource definition for now in case further customization is required. Assuming this generic implementation remains, we can migrate this in the future to a shared location (internal/framework) and extend it to support all types, similar to how the standard ConflictsWith validator works.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is a good approach

Copy link
Contributor

@gdavison gdavison left a comment

Choose a reason for hiding this comment

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

Looks good! 🚀

--- PASS: TestAccS3BucketLifecycleConfiguration_Filter_Tag (68.23s)
--- PASS: TestAccS3BucketLifecycleConfiguration_multipleRules_noFilterOrPrefix (68.40s)
--- PASS: TestAccS3BucketLifecycleConfiguration_RulePrefix (70.79s)
--- PASS: TestAccS3BucketLifecycleConfiguration_nonCurrentVersionExpiration (73.21s)
--- PASS: TestAccS3BucketLifecycleConfiguration_TransitionZeroDays_intelligentTiering (73.39s)
--- PASS: TestAccS3BucketLifecycleConfiguration_nonCurrentVersionTransition (73.42s)
--- PASS: TestAccS3BucketLifecycleConfiguration_multipleRules (73.79s)
--- PASS: TestAccS3BucketLifecycleConfiguration_TransitionStorageClassOnly_intelligentTiering (73.99s)
--- PASS: TestAccS3BucketLifecycleConfiguration_Update_filterWithAndToFilterWithPrefix (160.86s)
--- PASS: TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_basic (174.96s)
--- PASS: TestAccS3BucketLifecycleConfiguration_transitionDefaultMinimumObjectSize_remove (206.30s)
--- PASS: TestAccS3BucketLifecycleConfiguration_TransitionDate_standardIa (138.78s)
--- PASS: TestAccS3BucketLifecycleConfiguration_TransitionDate_intelligentTiering (150.23s)
--- PASS: TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_nonCurrentVersionExpiration (218.51s)
--- PASS: TestAccS3BucketLifecycleConfiguration_RuleExpiration_emptyBlock (163.16s)
--- PASS: TestAccS3BucketLifecycleConfiguration_migrate_withChange (167.22s)
--- PASS: TestAccS3BucketLifecycleConfiguration_migrate_noChange (169.93s)
--- PASS: TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_EmptyFilter_NonCurrentVersions (245.42s)
--- PASS: TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_FilterWithPrefix (250.33s)
--- PASS: TestAccS3BucketLifecycleConfiguration_disableRule (252.13s)
--- PASS: TestAccS3BucketLifecycleConfiguration_TransitionUpdateBetweenDaysAndDate_intelligentTiering (253.29s)
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_TransitionStorageClassOnly_intelligentTiering (254.85s)
--- PASS: TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_Filter_And_Tags (255.37s)
--- PASS: TestAccS3BucketLifecycleConfiguration_ruleAbortIncompleteMultipartUpload (207.02s)
--- PASS: TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_Filter_ObjectSizeRange (203.89s)
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_basic (304.84s)
--- PASS: TestAccS3BucketLifecycleConfiguration_RuleExpiration_expireMarkerOnly (231.45s)
--- PASS: TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_EmptyFilter_NonCurrentVersions_WithChange (306.88s)
--- PASS: TestAccS3BucketLifecycleConfiguration_Filter_ObjectSizeRangeAndPrefix (100.31s)
--- PASS: TestAccS3BucketLifecycleConfiguration_Filter_ObjectSizeGreaterThan (103.72s)
--- PASS: TestAccS3BucketLifecycleConfiguration_Filter_ObjectSizeRange (97.13s)
--- PASS: TestAccS3BucketLifecycleConfiguration_Filter_ObjectSizeGreaterThanZero (79.35s)
--- PASS: TestAccS3BucketLifecycleConfiguration_Filter_ObjectSizeLessThan (97.29s)
--- PASS: TestAccS3BucketLifecycleConfiguration_EmptyFilter_NonCurrentVersions (147.28s)
--- PASS: TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_Filter_ObjectSizeRangeAndPrefix (171.85s)
--- PASS: TestAccS3BucketLifecycleConfiguration_disappears (72.29s)
--- PASS: TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_Filter_ObjectSizeLessThan (133.49s)
--- PASS: TestAccS3BucketLifecycleConfiguration_filterWithEmptyPrefix (83.55s)
--- PASS: TestAccS3BucketLifecycleConfiguration_transitionDefaultMinimumObjectSize_update (139.41s)
--- PASS: TestAccS3BucketLifecycleConfiguration_basic (74.74s)
--- PASS: TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_Filter_ObjectSizeGreaterThan (151.63s)
--- PASS: TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_Filter_Tag (159.44s)
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_Filter_And_Tags (173.01s)
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_TransitionDate (177.46s)
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_Filter_Tag (193.50s)
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_RulePrefix (167.88s)
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_EmptyFilter_NonCurrentVersions (203.00s)
--- PASS: TestAccS3BucketLifecycleConfiguration_filterWithPrefix (148.63s)
--- PASS: TestAccS3BucketLifecycleConfiguration_directoryBucket (76.39s)
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_Filter_ObjectSizeRangeAndPrefix (164.30s)
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_RuleExpiration_expireMarkerOnly (163.78s)
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_Filter_ObjectSizeRange (197.37s)
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_RuleExpiration_emptyBlock (197.98s)
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_Filter_ObjectSizeLessThan (192.83s)
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_Filter_ObjectSizeGreaterThan (176.18s)
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_nonCurrentVersionExpiration (159.73s)
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_ruleAbortIncompleteMultipartUpload (195.29s)
--- PASS: TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_RulePrefix (127.29s)
--- PASS: TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_TransitionZeroDays_intelligentTiering (123.48s)
--- PASS: TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_RuleExpiration_expireMarkerOnly (101.69s)
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_EmptyFilter_NonCurrentVersions_WithChange (159.83s)
--- PASS: TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_TransitionStorageClassOnly_intelligentTiering (95.38s)
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_filterWithEmptyPrefix (149.12s)
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_FilterWithPrefix (146.91s)
--- PASS: TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_TransitionZeroDays_intelligentTiering (115.07s)
--- PASS: TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_RuleExpiration_emptyBlock (104.18s)
--- PASS: TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_TransitionDate (99.30s)
--- PASS: TestAccS3BucketLifecycleConfiguration_upgradeV5_86_0_ruleAbortIncompleteMultipartUpload (94.51s)

@gdavison gdavison merged commit afee73b into main Feb 20, 2025
40 checks passed
@gdavison gdavison deleted the f-bucket_lifecycle-expiration-validation branch February 20, 2025 18:01
@github-actions github-actions bot added this to the v5.88.0 milestone Feb 20, 2025
terraform-aws-provider bot pushed a commit that referenced this pull request Feb 20, 2025
Copy link

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

@github-actions github-actions bot removed the prioritized Part of the maintainer teams immediate focus. To be addressed within the current quarter. label Feb 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
service/s3 Issues and PRs that pertain to the s3 service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants