Skip to content

Commit

Permalink
Merge pull request #41542 from hashicorp/b-s3-lifecycle-rule-filter-and
Browse files Browse the repository at this point in the history 
resource/aws_s3_bucket_lifecycle_configuration: Prevents sending 0 for `rule.and.object_size_less_than`
  • Loading branch information
@gdavison
gdavison authored Feb 25, 2025
2 parents 40c6514 + c251e97 commit 08b986e
Show file tree
Hide file tree
Showing 6 changed files with 254 additions and 4 deletions.
3 changes: 3 additions & 0 deletions .changelog/41542.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
resource/aws_s3_bucket_lifecycle_configuration: Prevents `InvalidRequest` error when `rule.and.object_size_less_than` not set.
```
26 changes: 26 additions & 0 deletions internal/framework/flex/int.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,19 @@ func Int64ValueFromFramework(ctx context.Context, v basetypes.Int64Valuable) int
return output
}

func Int64FromFrameworkLegacy(_ context.Context, v types.Int64) *int64 {
if v.IsNull() || v.IsUnknown() {
return nil
}

i := v.ValueInt64()
if i == 0 {
return nil
}

return aws.Int64(i)
}

// Int64ToFramework converts an int64 pointer to a Framework Int64 value.
// A nil int64 pointer is converted to a null Int64.
func Int64ToFramework(ctx context.Context, v *int64) types.Int64 {
Expand Down Expand Up @@ -88,6 +101,19 @@ func Int32FromFrameworkInt32(ctx context.Context, v basetypes.Int32Valuable) *in
return output
}

func Int32FromFrameworkLegacy(_ context.Context, v types.Int32) *int32 {
if v.IsNull() || v.IsUnknown() {
return nil
}

i := v.ValueInt32()
if i == 0 {
return nil
}

return aws.Int32(i)
}

// Int32ValueFromFrameworkInt64 coverts a Framework Int64 value to an int32 value.
// A null Int64 is converted to a nil int32 pointer.
func Int32ValueFromFrameworkInt64(ctx context.Context, v basetypes.Int64Valuable) int32 {
Expand Down
39 changes: 39 additions & 0 deletions internal/framework/flex/int_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -230,3 +230,42 @@ func TestInt32FromFramework(t *testing.T) {
})
}
}

func TestInt32FromFrameworkLegacy(t *testing.T) {
t.Parallel()

type testCase struct {
input types.Int32
expected *int32
}
tests := map[string]testCase{
"valid int32": {
input: types.Int32Value(42),
expected: aws.Int32(42),
},
"zero int32": {
input: types.Int32Value(0),
expected: nil,
},
"null int32": {
input: types.Int32Null(),
expected: nil,
},
"unknown int32": {
input: types.Int32Unknown(),
expected: nil,
},
}

for name, test := range tests {
t.Run(name, func(t *testing.T) {
t.Parallel()

got := flex.Int32FromFrameworkLegacy(context.Background(), test.input)

if diff := cmp.Diff(got, test.expected); diff != "" {
t.Errorf("unexpected diff (+wanted, -got): %s", diff)
}
})
}
}
13 changes: 13 additions & 0 deletions internal/framework/flex/string.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,19 @@ func StringToFrameworkValuable[T basetypes.StringValuable](ctx context.Context,
return output
}

func StringFromFrameworkLegacy(_ context.Context, v types.String) *string {
if v.IsNull() || v.IsUnknown() {
return nil
}

s := v.ValueString()
if s == "" {
return nil
}

return aws.String(s)
}

func EmptyStringAsNull(v types.String) types.String {
if v.IsNull() || v.IsUnknown() {
return v
Expand Down
8 changes: 4 additions & 4 deletions internal/service/s3/bucket_lifecycle_configuration.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -991,9 +991,9 @@ var (
)

type lifecycleRuleAndOperatorModel struct {
ObjectSizeGreaterThan types.Int64 `tfsdk:"object_size_greater_than" autoflex:",legacy"`
ObjectSizeLessThan types.Int64 `tfsdk:"object_size_less_than" autoflex:",legacy"`
Prefix types.String `tfsdk:"prefix" autoflex:",legacy"`
ObjectSizeGreaterThan types.Int64 `tfsdk:"object_size_greater_than"`
ObjectSizeLessThan types.Int64 `tfsdk:"object_size_less_than"`
Prefix types.String `tfsdk:"prefix"`
Tags tftags.Map `tfsdk:"tags"`
}

Expand All @@ -1002,7 +1002,7 @@ func (m lifecycleRuleAndOperatorModel) Expand(ctx context.Context) (result any,

r.ObjectSizeGreaterThan = fwflex.Int64FromFramework(ctx, m.ObjectSizeGreaterThan)

r.ObjectSizeLessThan = fwflex.Int64FromFramework(ctx, m.ObjectSizeLessThan)
r.ObjectSizeLessThan = fwflex.Int64FromFrameworkLegacy(ctx, m.ObjectSizeLessThan)

r.Prefix = fwflex.StringFromFramework(ctx, m.Prefix)

Expand Down
169 changes: 169 additions & 0 deletions internal/service/s3/bucket_lifecycle_configuration_migrate_v0_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -903,6 +903,137 @@ func TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_Filter_And_Tags(
})
}

// Simulate a change in default value for `transition_default_minimum_object_size`
func TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_Filter_And_ZeroLessThan_ChangeOnUpdate(t *testing.T) {
ctx := acctest.Context(t)
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
resourceName := "aws_s3_bucket_lifecycle_configuration.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(ctx, t) },
ErrorCheck: acctest.ErrorCheck(t, names.S3ServiceID),
CheckDestroy: testAccCheckBucketLifecycleConfigurationDestroy(ctx),
Steps: []resource.TestStep{
{
ExternalProviders: map[string]resource.ExternalProvider{
"aws": {
Source: "hashicorp/aws",
VersionConstraint: providerVersionSchemaV0,
},
},
Config: testAccBucketLifecycleConfigurationConfig_Migrate_Filter_And_ZeroLessThan(rName, false),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckBucketLifecycleConfigurationExists(ctx, resourceName),
),
ConfigStateChecks: []statecheck.StateCheck{
statecheck.CompareValuePairs(resourceName, tfjsonpath.New(names.AttrBucket), "aws_s3_bucket.test", tfjsonpath.New(names.AttrBucket), compare.ValuesSame()),
statecheck.ExpectKnownValue(resourceName, tfjsonpath.New(names.AttrExpectedBucketOwner), knownvalue.StringExact("")),
statecheck.CompareValuePairs(resourceName, tfjsonpath.New(names.AttrID), resourceName, tfjsonpath.New(names.AttrBucket), compare.ValuesSame()),
statecheck.ExpectKnownValue(resourceName, tfjsonpath.New(names.AttrRule), knownvalue.ListExact([]knownvalue.Check{
knownvalue.ObjectExact(map[string]knownvalue.Check{
"abort_incomplete_multipart_upload": checkAbortIncompleteMultipartUpload_None(),
"expiration": checkSchemaV0Expiration_Days(1),
names.AttrFilter: checkSchemaV0Filter_And(
knownvalue.ObjectExact(map[string]knownvalue.Check{
"object_size_greater_than": knownvalue.Int64Exact(0),
"object_size_less_than": knownvalue.Int64Exact(0),
names.AttrPrefix: knownvalue.StringExact("baseline/"),
names.AttrTags: knownvalue.MapExact(map[string]knownvalue.Check{
"Key": knownvalue.StringExact("data-lifecycle-action"),
"Value": knownvalue.StringExact("delete"),
}),
}),
),
names.AttrID: knownvalue.StringExact(rName),
"noncurrent_version_expiration": checkNoncurrentVersionExpiration_None(),
"noncurrent_version_transition": checkNoncurrentVersionTransitions(),
names.AttrPrefix: knownvalue.StringExact(""),
names.AttrStatus: knownvalue.StringExact(tfs3.LifecycleRuleStatusEnabled),
"transition": checkTransitions(),
}),
})),
statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("transition_default_minimum_object_size"), knownvalue.StringExact("varies_by_storage_class")),
},
},
{
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
Config: testAccBucketLifecycleConfigurationConfig_Migrate_Filter_And_ZeroLessThan(rName, true),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckBucketLifecycleConfigurationExists(ctx, resourceName),
),
ConfigStateChecks: []statecheck.StateCheck{
statecheck.CompareValuePairs(resourceName, tfjsonpath.New(names.AttrBucket), "aws_s3_bucket.test", tfjsonpath.New(names.AttrBucket), compare.ValuesSame()),
statecheck.ExpectKnownValue(resourceName, tfjsonpath.New(names.AttrExpectedBucketOwner), knownvalue.StringExact("")),
statecheck.CompareValuePairs(resourceName, tfjsonpath.New(names.AttrID), resourceName, tfjsonpath.New(names.AttrBucket), compare.ValuesSame()),
statecheck.ExpectKnownValue(resourceName, tfjsonpath.New(names.AttrRule), knownvalue.ListExact([]knownvalue.Check{
knownvalue.ObjectExact(map[string]knownvalue.Check{
"abort_incomplete_multipart_upload": checkAbortIncompleteMultipartUpload_None(),
"expiration": checkExpiration_Days(1),
names.AttrFilter: checkFilter_And(
knownvalue.ObjectExact(map[string]knownvalue.Check{
"object_size_greater_than": knownvalue.Int64Exact(0),
"object_size_less_than": knownvalue.Int64Exact(0),
names.AttrPrefix: knownvalue.StringExact("baseline/"),
names.AttrTags: knownvalue.MapExact(map[string]knownvalue.Check{
"Key": knownvalue.StringExact("data-lifecycle-action"),
"Value": knownvalue.StringExact("delete"),
}),
}),
),
names.AttrID: knownvalue.StringExact(rName),
"noncurrent_version_expiration": checkNoncurrentVersionExpiration_None(),
"noncurrent_version_transition": checkNoncurrentVersionTransitions(),
names.AttrPrefix: knownvalue.StringExact(""),
names.AttrStatus: knownvalue.StringExact(tfs3.LifecycleRuleStatusEnabled),
"transition": checkTransitions(),
}),
})),
statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("transition_default_minimum_object_size"), knownvalue.StringExact("all_storage_classes_128K")),
},
ConfigPlanChecks: resource.ConfigPlanChecks{
PreApply: []plancheck.PlanCheck{
// This is checking the change _after_ the state migration step happens
plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionUpdate),
plancheck.ExpectKnownValue(resourceName, tfjsonpath.New(names.AttrRule), knownvalue.ListExact([]knownvalue.Check{
knownvalue.ObjectExact(map[string]knownvalue.Check{
"abort_incomplete_multipart_upload": checkAbortIncompleteMultipartUpload_None(),
"expiration": checkExpiration_Days(1),
names.AttrFilter: checkFilter_And(
knownvalue.ObjectExact(map[string]knownvalue.Check{
"object_size_greater_than": knownvalue.Int64Exact(0),
"object_size_less_than": knownvalue.Int64Exact(0),
names.AttrPrefix: knownvalue.StringExact("baseline/"),
names.AttrTags: knownvalue.MapExact(map[string]knownvalue.Check{
"Key": knownvalue.StringExact("data-lifecycle-action"),
"Value": knownvalue.StringExact("delete"),
}),
}),
),
names.AttrID: knownvalue.StringExact(rName),
"noncurrent_version_expiration": checkNoncurrentVersionExpiration_None(),
"noncurrent_version_transition": checkNoncurrentVersionTransitions(),
names.AttrPrefix: knownvalue.StringExact(""),
names.AttrStatus: knownvalue.StringExact(tfs3.LifecycleRuleStatusEnabled),
"transition": checkTransitions(),
}),
})),
tfplancheck.ExpectKnownValueChange(resourceName, tfjsonpath.New("transition_default_minimum_object_size"),
knownvalue.StringExact("varies_by_storage_class"),
knownvalue.StringExact("all_storage_classes_128K"),
),
},
PostApplyPreRefresh: []plancheck.PlanCheck{
plancheck.ExpectEmptyPlan(),
},
PostApplyPostRefresh: []plancheck.PlanCheck{
plancheck.ExpectEmptyPlan(),
},
},
},
},
})
}

func TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_Filter_Tag(t *testing.T) {
ctx := acctest.Context(t)
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
Expand Down Expand Up @@ -2247,3 +2378,41 @@ resource "aws_s3_bucket" "test" {
}
`, rName)
}

// Set `migrated` to `false` before migration and `true` after
func testAccBucketLifecycleConfigurationConfig_Migrate_Filter_And_ZeroLessThan(rName string, migrated bool) string {
var transition string
if !migrated {
transition = "transition_default_minimum_object_size = \"varies_by_storage_class\""
}
return fmt.Sprintf(`
resource "aws_s3_bucket" "test" {
bucket = %[1]q
}
resource "aws_s3_bucket_lifecycle_configuration" "test" {
bucket = aws_s3_bucket.test.id
rule {
status = "Enabled"
id = %[1]q
expiration {
days = 1
}
filter {
and {
prefix = "baseline/"
tags = {
Key = "data-lifecycle-action"
Value = "delete"
}
}
}
}
%[2]s
}`, rName, transition)
}

0 comments on commit 08b986e

Please sign in to comment.