diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index f2f84a5f22..2b99ba59cd 100644 --- a/terraform/context_apply_test.go +++ b/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, ` +Outputs: + +child2_id = foo + +module.child1: + + Outputs: + + instance_id = foo-bar-baz module.child2: aws_instance.foo: ID = foo - `) + + Outputs: + + instance_id = foo +`) } func TestContext2Apply_targetedModuleResource(t *testing.T) { diff --git a/terraform/node_output.go b/terraform/node_output.go index e28e6f02fd..9017a63c40 100644 --- a/terraform/node_output.go +++ b/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) diff --git a/terraform/test-fixtures/apply-targeted-module-unrelated-outputs/child1/main.tf b/terraform/test-fixtures/apply-targeted-module-unrelated-outputs/child1/main.tf index b6de394c90..cffe3829e7 100644 --- a/terraform/test-fixtures/apply-targeted-module-unrelated-outputs/child1/main.tf +++ b/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}" } diff --git a/terraform/test-fixtures/apply-targeted-module-unrelated-outputs/child2/main.tf b/terraform/test-fixtures/apply-targeted-module-unrelated-outputs/child2/main.tf index b626e60c82..dce2d167be 100644 --- a/terraform/test-fixtures/apply-targeted-module-unrelated-outputs/child2/main.tf +++ b/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}" +} diff --git a/terraform/test-fixtures/apply-targeted-module-unrelated-outputs/main.tf b/terraform/test-fixtures/apply-targeted-module-unrelated-outputs/main.tf index 236f0c4953..1170072376 100644 --- a/terraform/test-fixtures/apply-targeted-module-unrelated-outputs/main.tf +++ b/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}" +} diff --git a/terraform/test-fixtures/transform-targets-downstream/child/child.tf b/terraform/test-fixtures/transform-targets-downstream/child/child.tf new file mode 100644 index 0000000000..6548b79493 --- /dev/null +++ b/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}" +} diff --git a/terraform/test-fixtures/transform-targets-downstream/child/grandchild/grandchild.tf b/terraform/test-fixtures/transform-targets-downstream/child/grandchild/grandchild.tf new file mode 100644 index 0000000000..3ad8fd0770 --- /dev/null +++ b/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}" +} diff --git a/terraform/test-fixtures/transform-targets-downstream/main.tf b/terraform/test-fixtures/transform-targets-downstream/main.tf new file mode 100644 index 0000000000..b732fdad7e --- /dev/null +++ b/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}" +} diff --git a/terraform/transform_targets.go b/terraform/transform_targets.go index 225ac4b4ae..125f9e3021 100644 --- a/terraform/transform_targets.go +++ b/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 } diff --git a/terraform/transform_targets_test.go b/terraform/transform_targets_test.go index 9418860e2a..c5c97cd22e 100644 --- a/terraform/transform_targets_test.go +++ b/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")