Skip to content

Commit

Permalink
only add NoOp nodes with conditions
Browse files Browse the repository at this point in the history
ONly add NoOp changes to the apply graph if they have conditions which
need to be evaluated.
  • Loading branch information
jbardin committed Oct 28, 2022
1 parent 297148e commit fc299c3
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 1 deletion.
1 change: 1 addition & 0 deletions internal/terraform/graph_builder_apply.go
Expand Up @@ -107,6 +107,7 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer {
Concrete: concreteResourceInstance,
State: b.State,
Changes: b.Changes,
Config: b.Config,
},

// Attach the state
Expand Down
26 changes: 25 additions & 1 deletion internal/terraform/transform_diff.go
Expand Up @@ -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"
Expand All @@ -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 {
Expand Down Expand Up @@ -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:
Expand Down
39 changes: 39 additions & 0 deletions internal/terraform/transform_diff_test.go
Expand Up @@ -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)
Expand All @@ -89,6 +109,7 @@ func TestDiffTransformer_noOpChange(t *testing.T) {
}

tf := &DiffTransformer{
Config: m,
Changes: &plans.Changes{
Resources: []*plans.ResourceInstanceChangeSrc{
{
Expand All @@ -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,
},
},
},
},
}
Expand Down

0 comments on commit fc299c3

Please sign in to comment.