Merge pull request #24296 from hashicorp/pselle/module-targetable

Make modules targetable

addrs/module.go

@@ -44,6 +44,49 @@ func (m Module) Equal(other Module) bool {
 	return m.String() == other.String()
 }
 
+func (m Module) targetableSigil() {
+	// Module is targetable
+}
+
+// TargetContains implements Targetable for Module by returning true if the given other
+// address either matches the receiver, is a sub-module-instance of the
+// receiver, or is a targetable absolute address within a module that
+// is contained within the receiver.
+func (m Module) TargetContains(other Targetable) bool {
+	switch to := other.(type) {
+
+	case Module:
+		if len(to) < len(m) {
+			// Can't be contained if the path is shorter
+			return false
+		}
+		// Other is contained if its steps match for the length of our own path.
+		for i, ourStep := range m {
+			otherStep := to[i]
+			if ourStep != otherStep {
+				return false
+			}
+		}
+		// If we fall out here then the prefixed matched, so it's contained.
+		return true
+
+	case ModuleInstance:
+		return m.TargetContains(to.Module())
+
+	case ConfigResource:
+		return m.TargetContains(to.Module)
+
+	case AbsResource:
+		return m.TargetContains(to.Module)
+
+	case AbsResourceInstance:
+		return m.TargetContains(to.Module)
+
+	default:
+		return false
+	}
+}
+
 // Child returns the address of a child call in the receiver, identified by the
 // given name.
 func (m Module) Child(name string) Module {

addrs/module_instance.go

@@ -383,8 +383,7 @@ func (m ModuleInstance) CallInstance() (ModuleInstance, ModuleCallInstance) {
 // is contained within the reciever.
 func (m ModuleInstance) TargetContains(other Targetable) bool {
 	switch to := other.(type) {
-
+	case Module:
-	case ModuleInstance:
 		if len(to) < len(m) {
 			// Can't be contained if the path is shorter
 			return false
@@ -392,13 +391,38 @@ func (m ModuleInstance) TargetContains(other Targetable) bool {
 		// Other is contained if its steps match for the length of our own path.
 		for i, ourStep := range m {
 			otherStep := to[i]
-			if ourStep != otherStep {
+
+			// We can't contain an entire module if we have a specific instance
+			// key. The case of NoKey is OK because this address is either
+			// meant to address an unexpanded module, or a single instance of
+			// that module, and both of those are a covered in-full by the
+			// Module address.
+			if ourStep.InstanceKey != NoKey {
+				return false
+			}
+
+			if ourStep.Name != otherStep {
 				return false
 			}
 		}
 		// If we fall out here then the prefixed matched, so it's contained.
 		return true
 
+	case ModuleInstance:
+		if len(to) < len(m) {
+			return false
+		}
+		for i, ourStep := range m {
+			otherStep := to[i]
+			if ourStep != otherStep {
+				return false
+			}
+		}
+		return true
+
+	case ConfigResource:
+		return m.TargetContains(to.Module)
+
 	case AbsResource:
 		return m.TargetContains(to.Module)
 

addrs/resource.go

@@ -145,6 +145,11 @@ func (r AbsResource) TargetContains(other Targetable) bool {
 		// We'll use our stringification as a cheat-ish way to test for equality.
 		return to.String() == r.String()
 
+	case ConfigResource:
+		// if an absolute resource from parsing a target address contains a
+		// ConfigResource, the string representation will match
+		return to.String() == r.String()
+
 	case AbsResourceInstance:
 		return r.TargetContains(to.ContainingResource())
 
@@ -203,9 +208,15 @@ func (r AbsResourceInstance) ContainingResource() AbsResource {
 func (r AbsResourceInstance) TargetContains(other Targetable) bool {
 	switch to := other.(type) {
 
+	// while we currently don't start with an AbsResourceInstance as a target
+	// address, check all resource types for consistency.
 	case AbsResourceInstance:
 		// We'll use our stringification as a cheat-ish way to test for equality.
 		return to.String() == r.String()
+	case ConfigResource:
+		return to.String() == r.String()
+	case AbsResource:
+		return to.String() == r.String()
 
 	default:
 		return false

addrs/target_test.go

@@ -94,8 +94,14 @@ func TestTargetContains(t *testing.T) {
 			mustParseTarget("module.bar[0].test_resource.foo[2]"),
 			false,
 		},
+		{
+			mustParseTarget("module.bar.test_resource.foo"),
+			mustParseTarget("module.bar.test_resource.foo[0]"),
+			true,
+		},
 
-		// Config paths, while never returned from parsing a target, must still be targetable
+		// Config paths, while never returned from parsing a target, must still
+		// be targetable
 		{
 			ConfigResource{
 				Module: []string{"bar"},
@@ -108,6 +114,30 @@ func TestTargetContains(t *testing.T) {
 			mustParseTarget("module.bar.test_resource.foo[2]"),
 			true,
 		},
+		{
+			mustParseTarget("module.bar"),
+			ConfigResource{
+				Module: []string{"bar"},
+				Resource: Resource{
+					Mode: ManagedResourceMode,
+					Type: "test_resource",
+					Name: "foo",
+				},
+			},
+			true,
+		},
+		{
+			mustParseTarget("module.bar.test_resource.foo"),
+			ConfigResource{
+				Module: []string{"bar"},
+				Resource: Resource{
+					Mode: ManagedResourceMode,
+					Type: "test_resource",
+					Name: "foo",
+				},
+			},
+			true,
+		},
 		{
 			ConfigResource{
 				Resource: Resource{
@@ -131,6 +161,45 @@ func TestTargetContains(t *testing.T) {
 			mustParseTarget("module.bar[0].test_resource.foo"),
 			true,
 		},
+
+		// Modules are also never the result of parsing a target, but also need
+		// to be targetable
+		{
+			Module{"bar"},
+			Module{"bar", "baz"},
+			true,
+		},
+		{
+			Module{"bar"},
+			mustParseTarget("module.bar[0]"),
+			true,
+		},
+		{
+			// Parsing an ambiguous module path needs to ensure the
+			// ModuleInstance could contain the Module. This is safe because if
+			// the module could be expanded, it must have an index, meaning no
+			// index indicates that the module instance and module are
+			// functionally equivalent.
+			mustParseTarget("module.bar"),
+			Module{"bar"},
+			true,
+		},
+		{
+			// A specific ModuleInstance cannot contain a module
+			mustParseTarget("module.bar[0]"),
+			Module{"bar"},
+			false,
+		},
+		{
+			Module{"bar", "baz"},
+			mustParseTarget("module.bar[0].module.baz.test_resource.foo[1]"),
+			true,
+		},
+		{
+			mustParseTarget("module.bar[0].module.baz"),
+			Module{"bar", "baz"},
+			false,
+		},
 	} {
 		t.Run(fmt.Sprintf("%s-in-%s", test.other, test.addr), func(t *testing.T) {
 			got := test.addr.TargetContains(test.other)

terraform/transform_targets_test.go

@@ -130,6 +130,77 @@ output.grandchild_id
 	}
 }
 
+// This tests the TargetsTransformer targeting a whole module,
+// rather than a resource within a module instance.
+func TestTargetsTransformer_wholeModule(t *testing.T) {
+	mod := testModule(t, "transform-targets-downstream")
+
+	g := Graph{Path: addrs.RootModuleInstance}
+	{
+		transform := &ConfigTransformer{Config: mod}
+		if err := transform.Transform(&g); err != nil {
+			t.Fatalf("%T failed: %s", transform, err)
+		}
+	}
+
+	{
+		transform := &AttachResourceConfigTransformer{Config: mod}
+		if err := transform.Transform(&g); err != nil {
+			t.Fatalf("%T failed: %s", transform, err)
+		}
+	}
+
+	{
+		transform := &AttachResourceConfigTransformer{Config: mod}
+		if err := transform.Transform(&g); err != nil {
+			t.Fatalf("%T failed: %s", transform, err)
+		}
+	}
+
+	{
+		transform := &OutputTransformer{Config: mod}
+		if err := transform.Transform(&g); err != nil {
+			t.Fatalf("%T failed: %s", transform, err)
+		}
+	}
+
+	{
+		transform := &ReferenceTransformer{}
+		if err := transform.Transform(&g); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+	}
+
+	{
+		transform := &TargetsTransformer{
+			Targets: []addrs.Targetable{
+				addrs.RootModule.
+					Child("child").
+					Child("grandchild"),
+			},
+		}
+		if err := transform.Transform(&g); err != nil {
+			t.Fatalf("%T failed: %s", transform, err)
+		}
+	}
+
+	actual := strings.TrimSpace(g.String())
+	// Even though we only asked to target the grandchild module, all of the
+	// outputs that descend from it are also targeted.
+	expected := strings.TrimSpace(`
+module.child.module.grandchild.aws_instance.foo
+module.child.module.grandchild.output.id
+  module.child.module.grandchild.aws_instance.foo
+module.child.output.grandchild_id
+  module.child.module.grandchild.output.id
+output.grandchild_id
+  module.child.output.grandchild_id
+	`)
+	if actual != expected {
+		t.Fatalf("bad:\n\nexpected:\n%s\n\ngot:\n%s\n", expected, actual)
+	}
+}
+
 func TestTargetsTransformer_destroy(t *testing.T) {
 	mod := testModule(t, "transform-targets-destroy")