Merge pull request #41462 from hashicorp/f-bucket_lifecycle-expiratio…

…n-validation

r/aws_s3_bucket_lifecycle_configuration: remove `expired_object_delete_marker` plan modifier

.changelog/41462.txt

@@ -0,0 +1,3 @@
+```release-note:note
+resource/aws_service_thing: A warning diagnostic has been added for configurations where `rule.exipration.expired_object_delete_marker` is set with either `rule.exipration.date` or `rule.exipration.days`. While historically the provider allowed this invalid configuration, the migration of this resource to the Terraform Plugin Framework in `v5.86.0` resulted in this misconfiguration surfacing as a hard `inconsistent result after apply` error. This diagnostic aims to direct users how to resolve the issue at plan time. See [this issue comment](https://github.com/hashicorp/terraform-provider-aws/issues/41277#issuecomment-2654728812) for additional context.
+```

internal/service/s3/bucket_lifecycle_configuration.go

@@ -21,11 +21,11 @@ import (
 	"github.com/hashicorp/terraform-plugin-framework-validators/int64validator"
 	"github.com/hashicorp/terraform-plugin-framework-validators/listvalidator"
 	"github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator"
+	"github.com/hashicorp/terraform-plugin-framework/attr"
 	"github.com/hashicorp/terraform-plugin-framework/diag"
 	"github.com/hashicorp/terraform-plugin-framework/path"
 	"github.com/hashicorp/terraform-plugin-framework/resource"
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema"
-	"github.com/hashicorp/terraform-plugin-framework/resource/schema/boolplanmodifier"
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema/int32planmodifier"
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema/int64planmodifier"
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier"
@@ -166,8 +166,11 @@ func (r *resourceBucketLifecycleConfiguration) Schema(ctx context.Context, reque
 									"expired_object_delete_marker": schema.BoolAttribute{
 										Optional: true,
 										Computed: true,
-										PlanModifiers: []planmodifier.Bool{
-											boolplanmodifier.UseStateForUnknown(),
+										Validators: []validator.Bool{
+											warnIfSetWith(
+												path.MatchRelative().AtParent().AtName("date"),
+												path.MatchRelative().AtParent().AtName("days"),
+											),
 										},
 									},
 								},
@@ -1071,3 +1074,78 @@ func (m transitionDefaultMinimumObjectSizeDefaultModifier) PlanModifyString(ctx
 	}
 	resp.PlanValue = v
 }
+
+var (
+	_ validator.Bool = warnIfSetWithValidator{}
+)
+
+func warnIfSetWith(expressions ...path.Expression) validator.Bool {
+	return warnIfSetWithValidator{
+		PathExpressions: expressions,
+	}
+}
+
+type warnIfSetWithValidator struct {
+	PathExpressions path.Expressions
+}
+
+func (v warnIfSetWithValidator) Description(ctx context.Context) string {
+	return v.MarkdownDescription(ctx)
+}
+
+func (v warnIfSetWithValidator) MarkdownDescription(_ context.Context) string {
+	return fmt.Sprintf("Ensure that if an attribute is set, a warning is emitted if any of these are also set: %q", v.PathExpressions)
+}
+
+// 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) {
+	// If attribute configuration is null, it cannot conflict with others
+	// If attribute configuration is unknown, delay the validation until it is known.
+	if req.ConfigValue.IsNull() || req.ConfigValue.IsUnknown() {
+		return
+	}
+
+	expressions := req.PathExpression.MergeExpressions(v.PathExpressions...)
+
+	for _, expression := range expressions {
+		matchedPaths, diags := req.Config.PathMatches(ctx, expression)
+
+		resp.Diagnostics.Append(diags...)
+
+		// Collect all errors
+		if diags.HasError() {
+			continue
+		}
+
+		for _, mp := range matchedPaths {
+			// If the user specifies the same attribute this validator is applied to,
+			// also as part of the input, skip it
+			if mp.Equal(req.Path) {
+				continue
+			}
+
+			var mpVal attr.Value
+			diags := req.Config.GetAttribute(ctx, mp, &mpVal)
+			resp.Diagnostics.Append(diags...)
+
+			// Collect all errors
+			if diags.HasError() {
+				continue
+			}
+
+			// Delay validation until all involved attribute have a known value
+			if mpVal.IsUnknown() {
+				return
+			}
+
+			if !mpVal.IsNull() {
+				resp.Diagnostics.Append(diag.NewAttributeWarningDiagnostic(
+					req.Path,
+					"Invalid Attribute Combination",
+					fmt.Sprintf("Attribute %q should not be specified when %q is also specified", req.Path, mp),
+				))
+			}
+		}
+	}
+}

internal/service/s3/bucket_lifecycle_configuration_migrate_v0_test.go

@@ -116,17 +116,19 @@ func TestAccS3BucketLifecycleConfiguration_frameworkMigrationV0_basic(t *testing
 						plancheck.ExpectKnownValue(resourceName, tfjsonpath.New(names.AttrRule), knownvalue.ListExact([]knownvalue.Check{
 							knownvalue.ObjectPartial(map[string]knownvalue.Check{
 								"abort_incomplete_multipart_upload": checkAbortIncompleteMultipartUpload_None(),
-								"expiration":                        checkExpiration_Days(365),
-								// names.AttrFilter:
-								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(),
+								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(),
 							}),
 						})),
 						// This is checking the change _after_ the state migration step happens
+						tfplancheck.ExpectKnownValueChange(resourceName, tfjsonpath.New(names.AttrRule).AtSliceIndex(0).AtMapKey("expiration"),
+							checkExpiration_Days(365),
+							checkExpiration_DaysAfterMigration(365),
+						),
 						tfplancheck.ExpectKnownValueChange(resourceName, tfjsonpath.New(names.AttrRule).AtSliceIndex(0).AtMapKey(names.AttrFilter),
 							checkFilter_Prefix(""),
 							checkFilter_None(),

internal/service/s3/bucket_lifecycle_configuration_test.go

@@ -1964,6 +1964,18 @@ func checkExpiration_Days(days int32) knownvalue.Check {
 	})
 }
 
+func checkExpiration_DaysAfterMigration(days int32) knownvalue.Check {
+	return knownvalue.ListExact([]knownvalue.Check{
+		knownvalue.ObjectExact(
+			map[string]knownvalue.Check{
+				"date": knownvalue.Null(),
+				"days": knownvalue.Int32Exact(days),
+				// "expired_object_delete_marker": knownvalue.Bool(false),
+			},
+		),
+	})
+}
+
 func checkExpiration_DeleteMarker(marker bool) knownvalue.Check {
 	checks := expirationDefaults()
 	maps.Copy(checks, map[string]knownvalue.Check{