Merge c9857aa818 into eba25e2fed

CHANGELOG.md

@@ -36,6 +36,7 @@ BUG FIXES:
 - `pg` backend doesn't fail on workspace creation for paralel runs, when the database is shared across multiple projects. ([#2411](https://github.com/opentofu/opentofu/pull/2411))
 - Generating an OpenTofu configuration from an `import` block that is referencing a resource with nested attributes now works correctly, instead of giving an error that the nested computed attribute is required. ([#2372](https://github.com/opentofu/opentofu/issues/2372)) 
 - `base64gunzip` now doesn't expose sensitive values if it fails during the base64 decoding. ([#2503](https://github.com/opentofu/opentofu/pull/2503))
+- Fix the issue with unexpected `create_before_destroy` (CBD) behavior, when CBD resource was depending on a non-CBD resource. ([#2508](https://github.com/opentofu/opentofu/pull/2508))
 
 ## Previous Releases
 

internal/tofu/context_apply_test.go

@@ -1124,6 +1124,98 @@ aws_instance.foo:
   require_new = yes
   type = aws_instance
 	`)
+
+	// Check that create_before_destroy was set on the foo resource
+	foo := state.RootModule().Resources["aws_instance.foo"].Instances[addrs.NoKey].Current
+	if !foo.CreateBeforeDestroy {
+		t.Fatalf("foo resource should have create_before_destroy set")
+	}
+}
+
+// This tests that when a CBD (C) resource depends on a non-CBD (B) resource that depends on another CBD resource (A)
+// Check that create_before_destroy is still set on the B resource after only the B resource is updated
+func TestContext2Apply_createBeforeDestroy_dependsNonCBD2(t *testing.T) {
+	m := testModule(t, "apply-cbd-depends-non-cbd2")
+	p := testProviderBuiltin()
+
+	state := states.NewState()
+	root := state.EnsureModule(addrs.RootModuleInstance)
+	root.SetResourceInstanceCurrent(
+		mustResourceInstanceAddr("terraform_data.A").Resource,
+		&states.ResourceInstanceObjectSrc{
+			Status: states.ObjectReady,
+			AttrsJSON: []byte(`
+			{
+				"id": "a1",
+				"triggers_replace": {
+				  "value": { "version": 0 },
+				  "type": ["object", { "version": "number" }]
+				}
+			}`),
+			CreateBeforeDestroy: true,
+		},
+		mustProviderConfig(`provider["terraform.io/builtin/terraform"]`),
+		addrs.NoKey,
+	)
+	root.SetResourceInstanceCurrent(
+		mustResourceInstanceAddr("terraform_data.B").Resource,
+		&states.ResourceInstanceObjectSrc{
+			Status: states.ObjectReady,
+			AttrsJSON: []byte(`
+			{
+				"id": "b1",
+				"triggers_replace": {
+				  "value": {
+					"base": "a1",
+					"version": 0
+				  },
+				  "type": ["object", { "base": "string", "version": "number" }]
+				}
+			}`),
+			Dependencies: []addrs.ConfigResource{mustConfigResourceAddr("terraform_data.A")},
+		},
+		mustProviderConfig(`provider["terraform.io/builtin/terraform"]`),
+		addrs.NoKey,
+	)
+	root.SetResourceInstanceCurrent(
+		mustResourceInstanceAddr("terraform_data.C").Resource,
+		&states.ResourceInstanceObjectSrc{
+			Status: states.ObjectReady,
+			AttrsJSON: []byte(`
+			{
+				"id": "c1",
+				"triggers_replace": null
+			}`),
+			Dependencies:        []addrs.ConfigResource{mustConfigResourceAddr("terraform_data.A"), mustConfigResourceAddr("terraform_data.B")},
+			CreateBeforeDestroy: true,
+		},
+		mustProviderConfig(`provider["terraform.io/builtin/terraform"]`),
+		addrs.NoKey,
+	)
+
+	ctx := testContext2(t, &ContextOpts{
+		Providers: map[addrs.Provider]providers.Factory{
+			addrs.NewBuiltInProvider("terraform"): testProviderFuncFixed(p),
+		},
+	})
+
+	plan, diags := ctx.Plan(context.Background(), m, state, DefaultPlanOpts)
+	if diags.HasErrors() {
+		t.Fatalf("diags: %s", diags.Err())
+	} else {
+		t.Log(legacyDiffComparisonString(plan.Changes))
+	}
+
+	state, diags = ctx.Apply(context.Background(), plan, m)
+	if diags.HasErrors() {
+		t.Fatalf("diags: %s", diags.Err())
+	}
+
+	// Check that create_before_destroy was set on the foo resource
+	foo := state.RootModule().Resources["terraform_data.B"].Instances[addrs.NoKey].Current
+	if !foo.CreateBeforeDestroy {
+		t.Fatalf("foo resource should have create_before_destroy set")
+	}
 }
 
 func TestContext2Apply_createBeforeDestroy_hook(t *testing.T) {

internal/tofu/context_test.go

@@ -18,6 +18,7 @@ import (
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
+	"github.com/hashicorp/go-uuid"
 	"github.com/hashicorp/go-version"
 	"github.com/opentofu/opentofu/internal/configs"
 	"github.com/opentofu/opentofu/internal/configs/configload"
@@ -369,6 +370,86 @@ func testDiffFn(req providers.PlanResourceChangeRequest) (resp providers.PlanRes
 	return
 }
 
+// testProviderBuiltin returns a mock provider that implements the relevant parts of builtin provider schema for testing.
+// Used to test with scenarios that are used for actual configs. Currently, ignores input and output attr handling.
+func testProviderBuiltin() *MockProvider {
+	p := new(MockProvider)
+	p.GetProviderSchemaResponse = &providers.GetProviderSchemaResponse{
+		ResourceTypes: map[string]providers.Schema{
+			"terraform_data": {
+				Block: &configschema.Block{
+					Attributes: map[string]*configschema.Attribute{
+						"triggers_replace": {Type: cty.DynamicPseudoType, Optional: true},
+						"id":               {Type: cty.String, Computed: true},
+					},
+				},
+			},
+		},
+	}
+	p.PlanResourceChangeFn = func(req providers.PlanResourceChangeRequest) providers.PlanResourceChangeResponse {
+		var resp providers.PlanResourceChangeResponse
+		if req.ProposedNewState.IsNull() {
+			// destroy op
+			resp.PlannedState = req.ProposedNewState
+			return resp
+		}
+
+		planned := req.ProposedNewState.AsValueMap()
+		trigger := req.ProposedNewState.GetAttr("triggers_replace")
+
+		switch {
+		case req.PriorState.IsNull():
+			// Create
+			// Set the id value to unknown.
+			planned["id"] = cty.UnknownVal(cty.String).RefineNotNull()
+
+			resp.PlannedState = cty.ObjectVal(planned)
+			return resp
+
+		case !req.PriorState.GetAttr("triggers_replace").RawEquals(trigger):
+			// trigger changed, so we need to replace the entire instance
+			resp.RequiresReplace = append(resp.RequiresReplace, cty.GetAttrPath("triggers_replace"))
+			planned["id"] = cty.UnknownVal(cty.String).RefineNotNull()
+		}
+
+		resp.PlannedState = cty.ObjectVal(planned)
+		return resp
+	}
+
+	p.ApplyResourceChangeFn = func(req providers.ApplyResourceChangeRequest) providers.ApplyResourceChangeResponse {
+		var resp providers.ApplyResourceChangeResponse
+		if req.PlannedState.IsNull() {
+			resp.NewState = req.PlannedState
+			return resp
+		}
+
+		newState := req.PlannedState.AsValueMap()
+
+		if !req.PlannedState.GetAttr("id").IsKnown() {
+			idString, err := uuid.GenerateUUID()
+			// OpenTofu would probably never get this far without a good random
+			// source, but catch the error anyway.
+			if err != nil {
+				diag := tfdiags.AttributeValue(
+					tfdiags.Error,
+					"Error generating id",
+					err.Error(),
+					cty.GetAttrPath("id"),
+				)
+
+				resp.Diagnostics = resp.Diagnostics.Append(diag)
+			}
+
+			newState["id"] = cty.StringVal(idString)
+		}
+
+		resp.NewState = cty.ObjectVal(newState)
+
+		return resp
+	}
+	return p
+}
+
 func testProvider(prefix string) *MockProvider {
 	p := new(MockProvider)
 	p.GetProviderSchemaResponse = testProviderSchema(prefix)

internal/tofu/node_resource_apply.go

@@ -19,6 +19,7 @@ type nodeExpandApplyableResource struct {
 }
 
 var (
+	_ GraphNodeDestroyerCBD         = (*nodeExpandApplyableResource)(nil)
 	_ GraphNodeReferenceable        = (*nodeExpandApplyableResource)(nil)
 	_ GraphNodeReferencer           = (*nodeExpandApplyableResource)(nil)
 	_ GraphNodeConfigResource       = (*nodeExpandApplyableResource)(nil)
@@ -30,6 +31,21 @@ var (
 func (n *nodeExpandApplyableResource) expandsInstances() {
 }
 
+// CreateBeforeDestroy implementation for GraphNodeDestroyerCBD
+func (n *nodeExpandApplyableResource) CreateBeforeDestroy() bool {
+	// If we have no config, we just assume no
+	if n.Config == nil || n.Config.Managed == nil {
+		return false
+	}
+
+	return n.Config.Managed.CreateBeforeDestroy
+}
+
+// ModifyCreateBeforeDestroy implementation for GraphNodeDestroyerCBD actually does nothing for this node type.
+func (n *nodeExpandApplyableResource) ModifyCreateBeforeDestroy(_ bool) error {
+	return nil
+}
+
 func (n *nodeExpandApplyableResource) References() []*addrs.Reference {
 	refs := n.NodeAbstractResource.References()
 

internal/tofu/testdata/apply-cbd-depends-non-cbd2/main.tf

@@ -0,0 +1,24 @@
+resource "terraform_data" "A" {
+  triggers_replace = {
+    version = 0
+  }
+
+  lifecycle {
+    create_before_destroy = true
+  }
+}
+
+resource "terraform_data" "B" {
+  triggers_replace = {
+    base    = terraform_data.A.id
+    version = 1
+  }
+}
+
+resource "terraform_data" "C" {
+  depends_on = [terraform_data.B]
+
+  lifecycle {
+    create_before_destroy = true
+  }
+}