From eb88ccbc7b5dbd223f2e8df6c1a571b42fca633d Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 26 Oct 2022 12:04:08 -0400 Subject: [PATCH] only add NoOp nodes with conditions ONly add NoOp changes to the apply graph if they have conditions which need to be evaluated. --- internal/terraform/graph_builder_apply.go | 1 + internal/terraform/transform_diff.go | 26 ++++++++++++++- internal/terraform/transform_diff_test.go | 39 +++++++++++++++++++++++ 3 files changed, 65 insertions(+), 1 deletion(-) diff --git a/internal/terraform/graph_builder_apply.go b/internal/terraform/graph_builder_apply.go index 4d7763d2a5..22f1b8c3bc 100644 --- a/internal/terraform/graph_builder_apply.go +++ b/internal/terraform/graph_builder_apply.go @@ -107,6 +107,7 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer { Concrete: concreteResourceInstance, State: b.State, Changes: b.Changes, + Config: b.Config, }, // Attach the state diff --git a/internal/terraform/transform_diff.go b/internal/terraform/transform_diff.go index bfabf60e9f..3bce1131f5 100644 --- a/internal/terraform/transform_diff.go +++ b/internal/terraform/transform_diff.go @@ -4,6 +4,8 @@ import ( "fmt" "log" + "github.com/hashicorp/terraform/internal/addrs" + "github.com/hashicorp/terraform/internal/configs" "github.com/hashicorp/terraform/internal/dag" "github.com/hashicorp/terraform/internal/plans" "github.com/hashicorp/terraform/internal/states" @@ -16,6 +18,28 @@ type DiffTransformer struct { Concrete ConcreteResourceInstanceNodeFunc State *states.State Changes *plans.Changes + Config *configs.Config +} + +// return true if the given resource instance has either Preconditions or +// Postconditions defined in the configuration. +func (t *DiffTransformer) hasConfigConditions(addr addrs.AbsResourceInstance) bool { + // unit tests may have no config + if t.Config == nil { + return false + } + + cfg := t.Config.DescendentForInstance(addr.Module) + if cfg == nil { + return false + } + + res := cfg.Module.ResourceByAddr(addr.ConfigResource().Resource) + if res == nil { + return false + } + + return len(res.Preconditions) > 0 || len(res.Postconditions) > 0 } func (t *DiffTransformer) Transform(g *Graph) error { @@ -69,7 +93,7 @@ func (t *DiffTransformer) Transform(g *Graph) error { // run any condition checks associated with the object, to // make sure that they still hold when considering the // results of other changes. - update = true + update = t.hasConfigConditions(addr) case plans.Delete: delete = true case plans.DeleteThenCreate, plans.CreateThenDelete: diff --git a/internal/terraform/transform_diff_test.go b/internal/terraform/transform_diff_test.go index c7cddc452f..fa1feae34d 100644 --- a/internal/terraform/transform_diff_test.go +++ b/internal/terraform/transform_diff_test.go @@ -81,6 +81,26 @@ func TestDiffTransformer_noOpChange(t *testing.T) { // results changed even if the resource instance they are attached to // didn't actually change directly itself. + // aws_instance.foo has a precondition, so should be included in the final + // graph. aws_instance.bar has no conditions, so there is nothing to + // execute during apply and it should not be included in the graph. + m := testModuleInline(t, map[string]string{ + "main.tf": ` +resource "aws_instance" "bar" { +} + +resource "aws_instance" "foo" { + test_string = "ok" + + lifecycle { + precondition { + condition = self.test_string != "" + error_message = "resource error" + } + } +} +`}) + g := Graph{Path: addrs.RootModuleInstance} beforeVal, err := plans.NewDynamicValue(cty.StringVal(""), cty.String) @@ -89,6 +109,7 @@ func TestDiffTransformer_noOpChange(t *testing.T) { } tf := &DiffTransformer{ + Config: m, Changes: &plans.Changes{ Resources: []*plans.ResourceInstanceChangeSrc{ { @@ -109,6 +130,24 @@ func TestDiffTransformer_noOpChange(t *testing.T) { After: beforeVal, }, }, + { + Addr: addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "aws_instance", + Name: "bar", + }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance), + ProviderAddr: addrs.AbsProviderConfig{ + Provider: addrs.NewDefaultProvider("aws"), + Module: addrs.RootModule, + }, + ChangeSrc: plans.ChangeSrc{ + // A "no-op" change has the no-op action and has the + // same object as both Before and After. + Action: plans.NoOp, + Before: beforeVal, + After: beforeVal, + }, + }, }, }, }