Merge pull request #30189 from hashicorp/jbardin/validate-moves

Ignore unexpanded paths when validating move statements.

internal/instances/expander.go

@@ -99,6 +99,13 @@ func (e *Expander) SetResourceForEach(moduleAddr addrs.ModuleInstance, resourceA
 // had their expansion registered using one of the SetModule* methods before
 // calling, or this method will panic.
 func (e *Expander) ExpandModule(addr addrs.Module) []addrs.ModuleInstance {
+	return e.expandModule(addr, false)
+}
+
+// expandModule allows skipping unexpanded module addresses by setting skipUnknown to true.
+// This is used by instances.Set, which is only concerned with the expanded
+// instances, and should not panic when looking up unknown addresses.
+func (e *Expander) expandModule(addr addrs.Module, skipUnknown bool) []addrs.ModuleInstance {
 	if len(addr) == 0 {
 		// Root module is always a singleton.
 		return singletonRootModule
@@ -113,7 +120,7 @@ func (e *Expander) ExpandModule(addr addrs.Module) []addrs.ModuleInstance {
 	// (moduleInstances does plenty of allocations itself, so the benefit of
 	// pre-allocating this is marginal but it's not hard to do.)
 	parentAddr := make(addrs.ModuleInstance, 0, 4)
-	ret := e.exps.moduleInstances(addr, parentAddr)
+	ret := e.exps.moduleInstances(addr, parentAddr, skipUnknown)
 	sort.SliceStable(ret, func(i, j int) bool {
 		return ret[i].Less(ret[j])
 	})
@@ -344,10 +351,16 @@ func newExpanderModule() *expanderModule {
 
 var singletonRootModule = []addrs.ModuleInstance{addrs.RootModuleInstance}
 
-func (m *expanderModule) moduleInstances(addr addrs.Module, parentAddr addrs.ModuleInstance) []addrs.ModuleInstance {
+// if moduleInstances is being used to lookup known instances after all
+// expansions have been done, set skipUnknown to true which allows addrs which
+// may not have been seen to return with no instances rather than panicking.
+func (m *expanderModule) moduleInstances(addr addrs.Module, parentAddr addrs.ModuleInstance, skipUnknown bool) []addrs.ModuleInstance {
 	callName := addr[0]
 	exp, ok := m.moduleCalls[addrs.ModuleCall{Name: callName}]
 	if !ok {
+		if skipUnknown {
+			return nil
+		}
 		// This is a bug in the caller, because it should always register
 		// expansions for an object and all of its ancestors before requesting
 		// expansion of it.
@@ -363,7 +376,7 @@ func (m *expanderModule) moduleInstances(addr addrs.Module, parentAddr addrs.Mod
 				continue
 			}
 			instAddr := append(parentAddr, step)
-			ret = append(ret, inst.moduleInstances(addr[1:], instAddr)...)
+			ret = append(ret, inst.moduleInstances(addr[1:], instAddr, skipUnknown)...)
 		}
 		return ret
 	}

internal/instances/set.go

@@ -47,5 +47,5 @@ func (s Set) HasResource(want addrs.AbsResource) bool {
 // then the result is the full expansion of all combinations of all of their
 // declared instance keys.
 func (s Set) InstancesForModule(modAddr addrs.Module) []addrs.ModuleInstance {
-	return s.exp.ExpandModule(modAddr)
+	return s.exp.expandModule(modAddr, true)
 }

internal/instances/set_test.go

@@ -204,4 +204,8 @@ func TestSet(t *testing.T) {
 		t.Errorf("unexpected %T %s", input, input.String())
 	}
 
+	// ensure we can lookup non-existent addrs in a set without panic
+	if set.InstancesForModule(addrs.RootModule.Child("missing")) != nil {
+		t.Error("unexpected instances from missing module")
+	}
 }

internal/terraform/context_apply2_test.go

@@ -629,3 +629,52 @@ func TestContext2Apply_nullableVariables(t *testing.T) {
 		t.Fatalf("incorrect 'non_nullable_no_default' output value: %#v\n", v)
 	}
 }
+
+func TestContext2Apply_targetedDestroyWithMoved(t *testing.T) {
+	m := testModuleInline(t, map[string]string{
+		"main.tf": `
+module "modb" {
+  source = "./mod"
+  for_each = toset(["a", "b"])
+}
+`,
+		"./mod/main.tf": `
+resource "test_object" "a" {
+}
+
+module "sub" {
+  for_each = toset(["a", "b"])
+  source = "./sub"
+}
+
+moved {
+  from = module.old
+  to = module.sub
+}
+`,
+		"./mod/sub/main.tf": `
+resource "test_object" "s" {
+}
+`})
+
+	p := simpleMockProvider()
+
+	ctx := testContext2(t, &ContextOpts{
+		Providers: map[addrs.Provider]providers.Factory{
+			addrs.NewDefaultProvider("test"): testProviderFuncFixed(p),
+		},
+	})
+
+	plan, diags := ctx.Plan(m, states.NewState(), DefaultPlanOpts)
+	assertNoErrors(t, diags)
+
+	state, diags := ctx.Apply(plan, m)
+	assertNoErrors(t, diags)
+
+	// destroy only a single instance not included in the moved statements
+	_, diags = ctx.Plan(m, state, &PlanOpts{
+		Mode:    plans.DestroyMode,
+		Targets: []addrs.Targetable{mustResourceInstanceAddr(`module.modb["a"].test_object.a`)},
+	})
+	assertNoErrors(t, diags)
+}