core: Allow downstream targeting of certain node types

The previous behavior of targets was that targeting a particular node
would implicitly target everything it depends on. This makes sense when
the dependencies in question are between resources, since we need to
make sure all of a resource's dependencies are in place before we can
create or update it.

However, it had the undesirable side-effect that targeting a resource
would _exclude_ any outputs referring to it, since the dependency edge
goes from output to resource. This then causes the output to be "stale",
which is problematic when outputs are being consumed by downstream
configs using terraform_remote_state.

GraphNodeTargetDownstream allows nodes to opt-in to a new behavior where
they can be targeted by _inverted_ dependency edges. That is, it allows
outputs to be considered targeted if anything they directly depend on
is targeted.

This is different than the implied targeting behavior in the other
direction because transitive dependencies are not considered unless the
intermediate nodes themselves have TargetDownstream. This means that
an output1→output2→resource chain can implicitly target both outputs, but
an output→resource1→resource2 chain _won't_ target the output if only
resource2 is targeted.

This behavior creates a scenario where an output can be visited before
all of its dependencies are ready, since it may have a mixture of both
targeted and untargeted dependencies. This is fine for outputs because
they silently ignore any errors encountered during interpolation anyway,
but other hypothetical future implementers of this interface may need to
be more careful.

This fixes #14186.

terraform/context_apply_test.go

@@ -7187,6 +7187,30 @@ func TestContext2Apply_targetedModuleUnrelatedOutputs(t *testing.T) {
 			"aws": testProviderFuncFixed(p),
 		},
 		Targets: []string{"module.child2"},
+		State: &State{
+			Modules: []*ModuleState{
+				{
+					Path:      []string{"root"},
+					Outputs:   map[string]*OutputState{},
+					Resources: map[string]*ResourceState{},
+				},
+				{
+					Path: []string{"root", "child1"},
+					Outputs: map[string]*OutputState{
+						"instance_id": {
+							Type:  "string",
+							Value: "foo-bar-baz",
+						},
+					},
+					Resources: map[string]*ResourceState{},
+				},
+				{
+					Path:      []string{"root", "child2"},
+					Outputs:   map[string]*OutputState{},
+					Resources: map[string]*ResourceState{},
+				},
+			},
+		},
 	})
 
 	if _, err := ctx.Plan(); err != nil {
@@ -7198,12 +7222,28 @@ func TestContext2Apply_targetedModuleUnrelatedOutputs(t *testing.T) {
 		t.Fatalf("err: %s", err)
 	}
 
+	// module.child1's instance_id output should be retained from state
+	// module.child2's instance_id is updated because its dependency is updated
+	// child2_id is updated because if its transitive dependency via module.child2
 	checkStateString(t, state, `
 <no state>
+Outputs:
+
+child2_id = foo
+
+module.child1:
+  <no state>
+  Outputs:
+
+  instance_id = foo-bar-baz
 module.child2:
   aws_instance.foo:
     ID = foo
-	`)
+
+  Outputs:
+
+  instance_id = foo
+`)
 }
 
 func TestContext2Apply_targetedModuleResource(t *testing.T) {

terraform/node_output.go

@@ -5,6 +5,7 @@ import (
 	"strings"
 
 	"github.com/hashicorp/terraform/config"
+	"github.com/hashicorp/terraform/dag"
 )
 
 // NodeApplyableOutput represents an output that is "applyable":
@@ -35,6 +36,14 @@ func (n *NodeApplyableOutput) RemoveIfNotTargeted() bool {
 	return true
 }
 
+// GraphNodeTargetDownstream
+func (n *NodeApplyableOutput) TargetDownstream(targetedDeps, untargetedDeps *dag.Set) bool {
+	// If any of the direct dependencies of an output are targeted then
+	// the output must always be targeted as well, so its value will always
+	// be up-to-date at the completion of an apply walk.
+	return true
+}
+
 // GraphNodeReferenceable
 func (n *NodeApplyableOutput) ReferenceableName() []string {
 	name := fmt.Sprintf("output.%s", n.Config.Name)

terraform/test-fixtures/apply-targeted-module-unrelated-outputs/child1/main.tf

@@ -2,6 +2,13 @@ variable "instance_id" {
 }
 
 output "instance_id" {
+  # The instance here isn't targeted, so this output shouldn't get updated.
+  # But it already has an existing value in state (specified within the
+  # test code) so we expect this to remain unchanged afterwards.
+  value = "${aws_instance.foo.id}"
+}
+
+output "given_instance_id" {
   value = "${var.instance_id}"
 }
 

terraform/test-fixtures/apply-targeted-module-unrelated-outputs/child2/main.tf

@@ -1,2 +1,9 @@
 resource "aws_instance" "foo" {
 }
+
+output "instance_id" {
+  # Even though we're targeting just the resource a bove, this should still
+  # be populated because outputs are implicitly targeted when their
+  # dependencies are
+  value = "${aws_instance.foo.id}"
+}

terraform/test-fixtures/apply-targeted-module-unrelated-outputs/main.tf

@@ -8,3 +8,30 @@ module "child1" {
 module "child2" {
   source = "./child2"
 }
+
+output "child1_id" {
+  value = "${module.child1.instance_id}"
+}
+
+output "child1_given_id" {
+  value = "${module.child1.given_instance_id}"
+}
+
+output "child2_id" {
+  # This should get updated even though we're targeting specifically
+  # module.child2, because outputs are implicitly targeted when their
+  # dependencies are.
+  value = "${module.child2.instance_id}"
+}
+
+output "all_ids" {
+  # Here we are intentionally referencing values covering three different scenarios:
+  # - not targeted and not already in state
+  # - not targeted and already in state
+  # - targeted
+  # This is important because this output must appear in the graph after
+  # target filtering in case the targeted node changes its value, but we must
+  # therefore silently ignore the failure that results from trying to
+  # interpolate the un-targeted, not-in-state node.
+  value = "${aws_instance.foo.id} ${module.child1.instance_id} ${module.child2.instance_id}"
+}

terraform/test-fixtures/transform-targets-downstream/child/child.tf

@@ -0,0 +1,14 @@
+resource "aws_instance" "foo" {
+}
+
+module "grandchild" {
+  source = "./grandchild"
+}
+
+output "id" {
+  value = "${aws_instance.foo.id}"
+}
+
+output "grandchild_id" {
+  value = "${module.grandchild.id}"
+}

terraform/test-fixtures/transform-targets-downstream/child/grandchild/grandchild.tf

@@ -0,0 +1,6 @@
+resource "aws_instance" "foo" {
+}
+
+output "id" {
+  value = "${aws_instance.foo.id}"
+}

terraform/test-fixtures/transform-targets-downstream/main.tf

@@ -0,0 +1,18 @@
+resource "aws_instance" "foo" {
+}
+
+module "child" {
+  source = "./child"
+}
+
+output "root_id" {
+  value = "${aws_instance.foo.id}"
+}
+
+output "child_id" {
+  value = "${module.child.id}"
+}
+
+output "grandchild_id" {
+  value = "${module.child.grandchild_id}"
+}

terraform/transform_targets.go

@@ -15,6 +15,21 @@ type GraphNodeTargetable interface {
 	SetTargets([]ResourceAddress)
 }
 
+// GraphNodeTargetDownstream is an interface for graph nodes that need to
+// be remain present under targeting if any of their dependencies are targeted.
+// TargetDownstream is called with the set of vertices that are direct
+// dependencies for the node, and it should return true if the node must remain
+// in the graph in support of those dependencies.
+//
+// This is used in situations where the dependency edges are representing an
+// ordering relationship but the dependency must still be visited if its
+// dependencies are visited. This is true for outputs, for example, since
+// they must get updated if any of their dependent resources get updated,
+// which would not normally be true if one of their dependencies were targeted.
+type GraphNodeTargetDownstream interface {
+	TargetDownstream(targeted, untargeted *dag.Set) bool
+}
+
 // TargetsTransformer is a GraphTransformer that, when the user specifies a
 // list of resources to target, limits the graph to only those resources and
 // their dependencies.
@@ -84,7 +99,10 @@ func (t *TargetsTransformer) parseTargetAddresses() ([]ResourceAddress, error) {
 func (t *TargetsTransformer) selectTargetedNodes(
 	g *Graph, addrs []ResourceAddress) (*dag.Set, error) {
 	targetedNodes := new(dag.Set)
-	for _, v := range g.Vertices() {
+
+	vertices := g.Vertices()
+
+	for _, v := range vertices {
 		if t.nodeIsTarget(v, addrs) {
 			targetedNodes.Add(v)
 
@@ -112,6 +130,63 @@ func (t *TargetsTransformer) selectTargetedNodes(
 		}
 	}
 
+	// Handle nodes that need to be included if their dependencies are included.
+	// This requires multiple passes since we need to catch transitive
+	// dependencies if and only if they are via other nodes that also
+	// support TargetDownstream. For example:
+	// output -> output -> targeted-resource: both outputs need to be targeted
+	// output -> non-targeted-resource -> targeted-resource: output not targeted
+	//
+	// We'll keep looping until we stop targeting more nodes.
+	queue := targetedNodes.List()
+	for len(queue) > 0 {
+		vertices := queue
+		queue = nil // ready to append for next iteration if neccessary
+		for _, v := range vertices {
+			dependers := g.UpEdges(v)
+			if dependers == nil {
+				// indicates that there are no up edges for this node, so
+				// we have nothing to do here.
+				continue
+			}
+
+			dependers = dependers.Filter(func(dv interface{}) bool {
+				// Can ignore nodes that are already targeted
+				/*if targetedNodes.Include(dv) {
+					return false
+				}*/
+
+				_, ok := dv.(GraphNodeTargetDownstream)
+				return ok
+			})
+
+			if dependers.Len() == 0 {
+				continue
+			}
+
+			for _, dv := range dependers.List() {
+				if targetedNodes.Include(dv) {
+					// Already present, so nothing to do
+					continue
+				}
+
+				// We'll give the node some information about what it's
+				// depending on in case that informs its decision about whether
+				// it is safe to be targeted.
+				deps := g.DownEdges(v)
+				depsTargeted := deps.Intersection(targetedNodes)
+				depsUntargeted := deps.Difference(depsTargeted)
+
+				if dv.(GraphNodeTargetDownstream).TargetDownstream(depsTargeted, depsUntargeted) {
+					targetedNodes.Add(dv)
+					// Need to visit this node on the next pass to see if it
+					// has any transitive dependers.
+					queue = append(queue, dv)
+				}
+			}
+		}
+	}
+
 	return targetedNodes, nil
 }
 

terraform/transform_targets_test.go

@@ -50,6 +50,69 @@ aws_vpc.me
 	}
 }
 
+func TestTargetsTransformer_downstream(t *testing.T) {
+	mod := testModule(t, "transform-targets-downstream")
+
+	g := Graph{Path: RootModulePath}
+	{
+		transform := &ConfigTransformer{Module: mod}
+		if err := transform.Transform(&g); err != nil {
+			t.Fatalf("%T failed: %s", transform, err)
+		}
+	}
+
+	{
+		transform := &AttachResourceConfigTransformer{Module: mod}
+		if err := transform.Transform(&g); err != nil {
+			t.Fatalf("%T failed: %s", transform, err)
+		}
+	}
+
+	{
+		transform := &AttachResourceConfigTransformer{Module: mod}
+		if err := transform.Transform(&g); err != nil {
+			t.Fatalf("%T failed: %s", transform, err)
+		}
+	}
+
+	{
+		transform := &OutputTransformer{Module: 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: []string{"module.child.module.grandchild.aws_instance.foo"}}
+		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 resource, 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")