From e5f8adfc1a329adb0bb4855ba340e1e92ccb2f0a Mon Sep 17 00:00:00 2001 From: James Bardin Date: Mon, 19 Mar 2018 20:32:37 -0400 Subject: [PATCH 1/2] add failing test for invalid output with targets Outputs that are missing references aren't always removed from the graph, due to being filtered before their dependents are removed. --- terraform/context_plan_test.go | 22 +++++++++++++++++++ .../plan-untargeted-resource-output/main.tf | 8 +++++++ .../mod/main.tf | 15 +++++++++++++ 3 files changed, 45 insertions(+) create mode 100644 terraform/test-fixtures/plan-untargeted-resource-output/main.tf create mode 100644 terraform/test-fixtures/plan-untargeted-resource-output/mod/main.tf diff --git a/terraform/context_plan_test.go b/terraform/context_plan_test.go index 8b680879d..8afb4c320 100644 --- a/terraform/context_plan_test.go +++ b/terraform/context_plan_test.go @@ -2969,6 +2969,28 @@ STATE: } } +// ensure that outputs missing references due to targetting are removed from +// the graph. +func TestContext2Plan_outputContainsTargetedResource(t *testing.T) { + m := testModule(t, "plan-untargeted-resource-output") + p := testProvider("aws") + p.DiffFn = testDiffFn + ctx := testContext2(t, &ContextOpts{ + Module: m, + ProviderResolver: ResourceProviderResolverFixed( + map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + }, + ), + Targets: []string{"module.mod.aws_instance.a"}, + }) + + _, err := ctx.Plan() + if err != nil { + t.Fatalf("err: %s", err) + } +} + // https://github.com/hashicorp/terraform/issues/4515 func TestContext2Plan_targetedOverTen(t *testing.T) { m := testModule(t, "plan-targeted-over-ten") diff --git a/terraform/test-fixtures/plan-untargeted-resource-output/main.tf b/terraform/test-fixtures/plan-untargeted-resource-output/main.tf new file mode 100644 index 000000000..9d4a1c882 --- /dev/null +++ b/terraform/test-fixtures/plan-untargeted-resource-output/main.tf @@ -0,0 +1,8 @@ +module "mod" { + source = "./mod" +} + + +resource "aws_instance" "c" { + name = "${module.mod.output}" +} diff --git a/terraform/test-fixtures/plan-untargeted-resource-output/mod/main.tf b/terraform/test-fixtures/plan-untargeted-resource-output/mod/main.tf new file mode 100644 index 000000000..d7ee9a7fa --- /dev/null +++ b/terraform/test-fixtures/plan-untargeted-resource-output/mod/main.tf @@ -0,0 +1,15 @@ +locals { + "one" = 1 +} + +resource "aws_instance" "a" { + count = "${local.one}" +} + +resource "aws_instance" "b" { + count = "${local.one}" +} + +output "output" { + value = "${join("", coalescelist(aws_instance.a.*.id, aws_instance.b.*.id))}" +} From a5c4f7e08e80ee50e1def527bff780a9e44fd191 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Mon, 19 Mar 2018 21:20:06 -0400 Subject: [PATCH 2/2] remove unneeded partial outputs filterPartialOutputs was not taking into account that some dependent resources might yet be removed from the graph. Check that they are not in the targeted set before declaring that the output remain. --- terraform/transform_targets.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/terraform/transform_targets.go b/terraform/transform_targets.go index 0cfcb0ac0..af6defe36 100644 --- a/terraform/transform_targets.go +++ b/terraform/transform_targets.go @@ -217,6 +217,12 @@ func filterPartialOutputs(v interface{}, targetedNodes *dag.Set, g *Graph) bool if _, ok := d.(*NodeCountBoundary); ok { continue } + + if !targetedNodes.Include(d) { + // this one is going to be removed, so it doesn't count + continue + } + // as soon as we see a real dependency, we mark this as // non-removable return true