fix: unused config's create_before_destroy on resource change with no refresh (#2248)

Signed-off-by: ollevche <ollevche@gmail.com>

CHANGELOG.md

@@ -9,6 +9,8 @@ ENHANCEMENTS:
 BUG FIXES:
 * `tofu init` command does not attempt to read encryption keys when `-backend=false` flag is set. (https://github.com/opentofu/opentofu/pull/2293)
 
+* Changes in `create_before_destroy` for resources which require replacement are now properly handled when refresh is disabled. ([#2248](https://github.com/opentofu/opentofu/pull/2248))
+
 ## Previous Releases
 
 For information on prior major and minor releases, see their changelogs:

internal/command/apply_test.go

@@ -2394,6 +2394,83 @@ func TestApply_showSensitiveArg(t *testing.T) {
 	}
 }
 
+func TestApply_CreateBeforeDestroyWithRefreshFalse(t *testing.T) {
+	td := t.TempDir()
+	testCopyDir(t, testFixturePath("apply_cbd_with_refresh_false"), td)
+	defer testChdir(t, td)()
+
+	originalState := states.BuildState(func(s *states.SyncState) {
+		s.SetResourceInstanceCurrent(
+			addrs.Resource{
+				Mode: addrs.ManagedResourceMode,
+				Type: "test_instance",
+				Name: "a",
+			}.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance),
+			&states.ResourceInstanceObjectSrc{
+				AttrsJSON: []byte(`{"id":"foo"}`),
+				Status:    states.ObjectReady,
+				// Create before destroy is set in the state and
+				// omitted in the configuration.
+				CreateBeforeDestroy: true,
+			},
+			addrs.AbsProviderConfig{
+				Provider: addrs.NewDefaultProvider("test"),
+				Module:   addrs.RootModule,
+			},
+			addrs.NoKey,
+		)
+	})
+	statePath := testStateFile(t, originalState)
+
+	p := testProvider()
+	p.GetProviderSchemaResponse = &providers.GetProviderSchemaResponse{
+		ResourceTypes: map[string]providers.Schema{
+			"test_instance": {
+				Block: &configschema.Block{
+					Attributes: map[string]*configschema.Attribute{
+						"id": {Type: cty.String, Computed: true},
+					},
+				},
+			},
+		},
+	}
+	// Resource must be replaced.
+	p.PlanResourceChangeFn = func(req providers.PlanResourceChangeRequest) providers.PlanResourceChangeResponse {
+		return providers.PlanResourceChangeResponse{
+			PlannedState:    req.ProposedNewState,
+			RequiresReplace: []cty.Path{{cty.GetAttrStep{Name: "id"}}},
+		}
+	}
+
+	view, done := testView(t)
+	c := &ApplyCommand{
+		Meta: Meta{
+			testingOverrides: metaOverridesForProvider(p),
+			View:             view,
+		},
+	}
+
+	args := []string{
+		"-state", statePath,
+		"-auto-approve",
+		// We are disabling refresh to see if cbd will be updated.
+		"-refresh=false",
+	}
+
+	code := c.Run(args)
+	output := done(t)
+	if code != 0 {
+		t.Fatalf("Failed to run: \n%s", output.Stderr())
+	}
+
+	// If cbd is not updated, resource becomes detached and
+	// apply results in 1 added, 0 changed, 0 destroyed.
+	stdout := output.Stdout()
+	if !strings.Contains(stdout, "Apply complete! Resources: 1 added, 0 changed, 1 destroyed.") {
+		t.Fatalf("bad: output should contain 'notsensitive' output\n%s", stdout)
+	}
+}
+
 // applyFixtureSchema returns a schema suitable for processing the
 // configuration in testdata/apply . This schema should be
 // assigned to a mock provider named "test".

internal/command/testdata/apply_cbd_with_refresh_false/main.tf

@@ -0,0 +1,3 @@
+resource "test_instance" "a" {
+  id = "bar"
+}

internal/tofu/context_apply.go

@@ -57,7 +57,7 @@ func (c *Context) Apply(ctx context.Context, plan *plans.Plan, config *configs.C
 
 	providerFunctionTracker := make(ProviderFunctionMapping)
 
-	graph, operation, diags := c.applyGraph(plan, config, true, providerFunctionTracker)
+	graph, operation, diags := c.applyGraph(plan, config, providerFunctionTracker)
 	if diags.HasErrors() {
 		return nil, diags
 	}
@@ -123,8 +123,7 @@ Note that the -target and -exclude options are not suitable for routine use, and
 	return newState, diags
 }
 
-//nolint:revive,unparam // TODO remove validate bool as it's not used
-func (c *Context) applyGraph(plan *plans.Plan, config *configs.Config, validate bool, providerFunctionTracker ProviderFunctionMapping) (*Graph, walkOperation, tfdiags.Diagnostics) {
+func (c *Context) applyGraph(plan *plans.Plan, config *configs.Config, providerFunctionTracker ProviderFunctionMapping) (*Graph, walkOperation, tfdiags.Diagnostics) {
 	var diags tfdiags.Diagnostics
 
 	variables := InputValues{}
@@ -211,7 +210,7 @@ func (c *Context) ApplyGraphForUI(plan *plans.Plan, config *configs.Config) (*Gr
 
 	var diags tfdiags.Diagnostics
 
-	graph, _, moreDiags := c.applyGraph(plan, config, false, make(ProviderFunctionMapping))
+	graph, _, moreDiags := c.applyGraph(plan, config, make(ProviderFunctionMapping))
 	diags = diags.Append(moreDiags)
 	return graph, diags
 }

internal/tofu/context_plan.go

@@ -291,7 +291,7 @@ func (c *Context) checkApplyGraph(plan *plans.Plan, config *configs.Config) tfdi
 		return nil
 	}
 	log.Println("[DEBUG] building apply graph to check for errors")
-	_, _, diags := c.applyGraph(plan, config, true, make(ProviderFunctionMapping))
+	_, _, diags := c.applyGraph(plan, config, make(ProviderFunctionMapping))
 	return diags
 }
 

internal/tofu/graph_builder_apply.go

@@ -187,10 +187,7 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer {
 			Changes:   b.Changes,
 			Operation: b.Operation,
 		},
-		&CBDEdgeTransformer{
-			Config: b.Config,
-			State:  b.State,
-		},
+		&CBDEdgeTransformer{},
 
 		// We need to remove configuration nodes that are not used at all, as
 		// they may not be able to evaluate, especially during destroy.

internal/tofu/node_resource_plan_instance.go

@@ -237,7 +237,21 @@ func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext)
 		log.Printf("[WARN] managedResourceExecute: no Managed config value found in instance state for %q", n.Addr)
 	} else {
 		if instanceRefreshState != nil {
+			prevCreateBeforeDestroy := instanceRefreshState.CreateBeforeDestroy
+
+			// This change is usually written to the refreshState and then
+			// updated value used for further graph execution. However, with
+			// "refresh=false", refreshState is not being written and then
+			// some resources with updated configuration could be detached
+			// due to missaligned create_before_destroy in different graph nodes.
 			instanceRefreshState.CreateBeforeDestroy = n.Config.Managed.CreateBeforeDestroy || n.ForceCreateBeforeDestroy
+
+			if prevCreateBeforeDestroy != instanceRefreshState.CreateBeforeDestroy && n.skipRefresh {
+				diags = diags.Append(n.writeResourceInstanceState(ctx, instanceRefreshState, refreshState))
+				if diags.HasErrors() {
+					return diags
+				}
+			}
 		}
 	}
 

internal/tofu/transform_destroy_cbd.go

@@ -9,9 +9,7 @@ import (
 	"fmt"
 	"log"
 
-	"github.com/opentofu/opentofu/internal/configs"
 	"github.com/opentofu/opentofu/internal/dag"
-	"github.com/opentofu/opentofu/internal/states"
 )
 
 // GraphNodeDestroyerCBD must be implemented by nodes that might be
@@ -115,12 +113,7 @@ func (t *ForcedCBDTransformer) hasCBDDescendent(g *Graph, v dag.Vertex) bool {
 // will get here by recording the CBD-ness of each change in the plan during
 // the plan walk and then forcing the nodes into the appropriate setting during
 // DiffTransformer when building the apply graph.
-type CBDEdgeTransformer struct {
-	// Module and State are only needed to look up dependencies in
-	// any way possible. Either can be nil if not available.
-	Config *configs.Config
-	State  *states.State
-}
+type CBDEdgeTransformer struct{}
 
 func (t *CBDEdgeTransformer) Transform(g *Graph) error {
 	// Go through and reverse any destroy edges