Merge pull request #22937 from hashicorp/jbardin/cbd-modules

create_before_destroy across modules

terraform/context_apply_test.go

@@ -10602,3 +10602,225 @@ func TestContext2Apply_invalidIndexRef(t *testing.T) {
 		t.Fatalf("missing expected error\ngot: %s\n\nwant: error containing %q", gotErr, wantErr)
 	}
 }
+
+func TestContext2Apply_moduleReplaceCycle(t *testing.T) {
+	for _, mode := range []string{"normal", "cbd"} {
+		var m *configs.Config
+
+		switch mode {
+		case "normal":
+			m = testModule(t, "apply-module-replace-cycle")
+		case "cbd":
+			m = testModule(t, "apply-module-replace-cycle-cbd")
+		}
+
+		p := testProvider("aws")
+		p.DiffFn = testDiffFn
+		p.ApplyFn = testApplyFn
+
+		instanceSchema := &configschema.Block{
+			Attributes: map[string]*configschema.Attribute{
+				"id":          {Type: cty.String, Computed: true},
+				"require_new": {Type: cty.String, Optional: true},
+			},
+		}
+
+		p.GetSchemaReturn = &ProviderSchema{
+			ResourceTypes: map[string]*configschema.Block{
+				"aws_instance": instanceSchema,
+			},
+		}
+
+		state := states.NewState()
+		modA := state.EnsureModule(addrs.RootModuleInstance.Child("a", addrs.NoKey))
+		modA.SetResourceInstanceCurrent(
+			addrs.Resource{
+				Mode: addrs.ManagedResourceMode,
+				Type: "aws_instance",
+				Name: "a",
+			}.Instance(addrs.NoKey),
+			&states.ResourceInstanceObjectSrc{
+				Status:    states.ObjectReady,
+				AttrsJSON: []byte(`{"id":"a","require_new":"old"}`),
+			},
+			addrs.ProviderConfig{
+				Type: "aws",
+			}.Absolute(addrs.RootModuleInstance),
+		)
+
+		modB := state.EnsureModule(addrs.RootModuleInstance.Child("b", addrs.NoKey))
+		modB.SetResourceInstanceCurrent(
+			addrs.Resource{
+				Mode: addrs.ManagedResourceMode,
+				Type: "aws_instance",
+				Name: "b",
+			}.Instance(addrs.IntKey(0)),
+			&states.ResourceInstanceObjectSrc{
+				Status:    states.ObjectReady,
+				AttrsJSON: []byte(`{"id":"b","require_new":"old"}`),
+			},
+			addrs.ProviderConfig{
+				Type: "aws",
+			}.Absolute(addrs.RootModuleInstance),
+		)
+
+		aBefore, _ := plans.NewDynamicValue(
+			cty.ObjectVal(map[string]cty.Value{
+				"id":          cty.StringVal("a"),
+				"require_new": cty.StringVal("old"),
+			}), instanceSchema.ImpliedType())
+		aAfter, _ := plans.NewDynamicValue(
+			cty.ObjectVal(map[string]cty.Value{
+				"id":          cty.UnknownVal(cty.String),
+				"require_new": cty.StringVal("new"),
+			}), instanceSchema.ImpliedType())
+		bBefore, _ := plans.NewDynamicValue(
+			cty.ObjectVal(map[string]cty.Value{
+				"id":          cty.StringVal("b"),
+				"require_new": cty.StringVal("old"),
+			}), instanceSchema.ImpliedType())
+		bAfter, _ := plans.NewDynamicValue(
+			cty.ObjectVal(map[string]cty.Value{
+				"id":          cty.UnknownVal(cty.String),
+				"require_new": cty.UnknownVal(cty.String),
+			}), instanceSchema.ImpliedType())
+
+		var aAction plans.Action
+		switch mode {
+		case "normal":
+			aAction = plans.DeleteThenCreate
+		case "cbd":
+			aAction = plans.CreateThenDelete
+		}
+
+		changes := &plans.Changes{
+			Resources: []*plans.ResourceInstanceChangeSrc{
+				{
+					Addr: addrs.Resource{
+						Mode: addrs.ManagedResourceMode,
+						Type: "aws_instance",
+						Name: "a",
+					}.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance.Child("a", addrs.NoKey)),
+					ProviderAddr: addrs.ProviderConfig{
+						Type: "aws",
+					}.Absolute(addrs.RootModuleInstance),
+					ChangeSrc: plans.ChangeSrc{
+						Action: aAction,
+						Before: aBefore,
+						After:  aAfter,
+					},
+				},
+				{
+					Addr: addrs.Resource{
+						Mode: addrs.ManagedResourceMode,
+						Type: "aws_instance",
+						Name: "b",
+					}.Instance(addrs.IntKey(0)).Absolute(addrs.RootModuleInstance.Child("b", addrs.NoKey)),
+					ProviderAddr: addrs.ProviderConfig{
+						Type: "aws",
+					}.Absolute(addrs.RootModuleInstance),
+					ChangeSrc: plans.ChangeSrc{
+						Action: plans.DeleteThenCreate,
+						Before: bBefore,
+						After:  bAfter,
+					},
+				},
+			},
+		}
+
+		ctx := testContext2(t, &ContextOpts{
+			Config: m,
+			ProviderResolver: providers.ResolverFixed(
+				map[string]providers.Factory{
+					"aws": testProviderFuncFixed(p),
+				},
+			),
+			State:   state,
+			Changes: changes,
+		})
+
+		t.Run(mode, func(t *testing.T) {
+			_, diags := ctx.Apply()
+			if diags.HasErrors() {
+				t.Fatal(diags.Err())
+			}
+		})
+	}
+}
+
+func TestContext2Apply_destroyDataCycle(t *testing.T) {
+	m, snap := testModuleWithSnapshot(t, "apply-destroy-data-cycle")
+	p := testProvider("null")
+	p.ApplyFn = testApplyFn
+	p.DiffFn = testDiffFn
+
+	state := states.NewState()
+	root := state.EnsureModule(addrs.RootModuleInstance)
+	root.SetResourceInstanceCurrent(
+		addrs.Resource{
+			Mode: addrs.ManagedResourceMode,
+			Type: "null_resource",
+			Name: "a",
+		}.Instance(addrs.IntKey(0)),
+		&states.ResourceInstanceObjectSrc{
+			Status:    states.ObjectReady,
+			AttrsJSON: []byte(`{"id":"a"}`),
+		},
+		addrs.ProviderConfig{
+			Type: "null",
+		}.Absolute(addrs.RootModuleInstance),
+	)
+	root.SetResourceInstanceCurrent(
+		addrs.Resource{
+			Mode: addrs.DataResourceMode,
+			Type: "null_data_source",
+			Name: "d",
+		}.Instance(addrs.NoKey),
+		&states.ResourceInstanceObjectSrc{
+			Status:    states.ObjectReady,
+			AttrsJSON: []byte(`{"id":"data"}`),
+		},
+		addrs.ProviderConfig{
+			Type: "null",
+		}.Absolute(addrs.RootModuleInstance),
+	)
+
+	providerResolver := providers.ResolverFixed(
+		map[string]providers.Factory{
+			"null": testProviderFuncFixed(p),
+		},
+	)
+
+	hook := &testHook{}
+	ctx := testContext2(t, &ContextOpts{
+		Config:           m,
+		ProviderResolver: providerResolver,
+		State:            state,
+		Destroy:          true,
+		Hooks:            []Hook{hook},
+	})
+
+	plan, diags := ctx.Plan()
+	diags.HasErrors()
+	if diags.HasErrors() {
+		t.Fatalf("diags: %s", diags.Err())
+	}
+
+	// We'll marshal and unmarshal the plan here, to ensure that we have
+	// a clean new context as would be created if we separately ran
+	// terraform plan -out=tfplan && terraform apply tfplan
+	ctxOpts, err := contextOptsForPlanViaFile(snap, state, plan)
+	if err != nil {
+		t.Fatal(err)
+	}
+	ctxOpts.ProviderResolver = providerResolver
+	ctx, diags = NewContext(ctxOpts)
+	if diags.HasErrors() {
+		t.Fatalf("failed to create context for plan: %s", diags.Err())
+	}
+
+	_, diags = ctx.Apply()
+	if diags.HasErrors() {
+		t.Fatalf("diags: %s", diags.Err())
+	}
+}

terraform/graph_builder_apply.go

@@ -127,21 +127,6 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer {
 		// Attach the state
 		&AttachStateTransformer{State: b.State},
 
-		// Destruction ordering
-		&DestroyEdgeTransformer{
-			Config:  b.Config,
-			State:   b.State,
-			Schemas: b.Schemas,
-		},
-		GraphTransformIf(
-			func() bool { return !b.Destroy },
-			&CBDEdgeTransformer{
-				Config:  b.Config,
-				State:   b.State,
-				Schemas: b.Schemas,
-			},
-		),
-
 		// Provisioner-related transformations
 		&MissingProvisionerTransformer{Provisioners: b.Components.ResourceProvisioners()},
 		&ProvisionerTransformer{},
@@ -171,6 +156,20 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer {
 		// Connect references so ordering is correct
 		&ReferenceTransformer{},
 
+		// Destruction ordering
+		&DestroyEdgeTransformer{
+			Config:  b.Config,
+			State:   b.State,
+			Schemas: b.Schemas,
+		},
+
+		&CBDEdgeTransformer{
+			Config:  b.Config,
+			State:   b.State,
+			Schemas: b.Schemas,
+			Destroy: b.Destroy,
+		},
+
 		// Handle destroy time transformations for output and local values.
 		// Reverse the edges from outputs and locals, so that
 		// interpolations don't fail during destroy.

terraform/graph_builder_apply_test.go

@@ -522,8 +522,9 @@ root
 test_object.A (prepare state)
   provider.test
 test_object.A[1] (destroy)
-  test_object.A (prepare state)
+  provider.test
 test_object.B
+  test_object.A (prepare state)
   test_object.A[1] (destroy)
   test_object.B (prepare state)
 test_object.B (prepare state)

terraform/testdata/apply-destroy-data-cycle/main.tf

@@ -0,0 +1,10 @@
+locals {
+    l = data.null_data_source.d.id
+}
+
+data "null_data_source" "d" {
+}
+
+resource "null_resource" "a" {
+    count = local.l == "NONE" ? 1 : 0
+}

terraform/testdata/apply-module-replace-cycle-cbd/main.tf

@@ -0,0 +1,8 @@
+module "a" {
+  source = "./mod1"
+}
+
+module "b" {
+  source = "./mod2"
+  ids = module.a.ids
+}

terraform/testdata/apply-module-replace-cycle-cbd/mod1/main.tf

@@ -0,0 +1,10 @@
+resource "aws_instance" "a" {
+  require_new = "new"
+  lifecycle {
+    create_before_destroy = true
+  }
+}
+
+output "ids" {
+  value = [aws_instance.a.id]
+}

terraform/testdata/apply-module-replace-cycle-cbd/mod2/main.tf

@@ -0,0 +1,8 @@
+resource "aws_instance" "b" {
+  count    = length(var.ids)
+  require_new = var.ids[count.index]
+}
+
+variable "ids" {
+  type = list(string)
+}

terraform/testdata/apply-module-replace-cycle/main.tf

@@ -0,0 +1,8 @@
+module "a" {
+  source = "./mod1"
+}
+
+module "b" {
+  source = "./mod2"
+  ids = module.a.ids
+}

terraform/testdata/apply-module-replace-cycle/mod1/main.tf

@@ -0,0 +1,10 @@
+resource "aws_instance" "a" {
+  require_new = "new"
+  lifecycle {
+    create_before_destroy = true
+  }
+}
+
+output "ids" {
+  value = [aws_instance.a.id]
+}

terraform/testdata/apply-module-replace-cycle/mod2/main.tf

@@ -0,0 +1,8 @@
+resource "aws_instance" "b" {
+  count    = length(var.ids)
+  require_new = var.ids[count.index]
+}
+
+variable "ids" {
+  type = list(string)
+}

terraform/testdata/transform-cbd-destroy-edge-both-count/main.tf

@@ -0,0 +1,11 @@
+resource "test_object" "A" {
+  count = 2
+  lifecycle {
+    create_before_destroy = true
+  }
+}
+
+resource "test_object" "B" {
+  count       = 2
+  test_string = test_object.A[*].test_string[count.index]
+}

terraform/testdata/transform-cbd-destroy-edge-count/main.tf

@@ -0,0 +1,10 @@
+resource "test_object" "A" {
+  lifecycle {
+    create_before_destroy = true
+  }
+}
+
+resource "test_object" "B" {
+  count       = 2
+  test_string = test_object.A.test_string
+}

terraform/testdata/transform-destroy-cbd-edge-basic/main.tf

@@ -0,0 +1,9 @@
+resource "test_object" "A" {
+  lifecycle {
+    create_before_destroy = true
+  }
+}
+
+resource "test_object" "B" {
+  test_string = "${test_object.A.id}"
+}