Skip to content

Commit

Permalink
existing outputs can only be Updated
Browse files Browse the repository at this point in the history
The planning logic here was inspired by resources, but unlike resources
a null value is valid for an output and does not necessarily indicate it
is being removed. If both before and after are null, the change should
be NoOp. When an output is removed from the configuration, we have a
separate node to create a Delete action to remove it from the state.
  • Loading branch information
jbardin committed Jan 14, 2021
1 parent 0a08603 commit 57f004e
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 6 deletions.
29 changes: 29 additions & 0 deletions terraform/context_plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6685,3 +6685,32 @@ func TestContext2Plan_variableCustomValidationsSensitive(t *testing.T) {
t.Fatalf("wrong error:\ngot: %s\nwant: message containing %q", got, want)
}
}

func TestContext2Plan_nullOutputNoOp(t *testing.T) {
// this should always plan a NoOp change for the output
m := testModuleInline(t, map[string]string{
"main.tf": `
output "planned" {
value = false ? 1 : null
}
`,
})

ctx := testContext2(t, &ContextOpts{
Config: m,
State: states.BuildState(func(s *states.SyncState) {
r := s.Module(addrs.RootModuleInstance)
r.SetOutputValue("planned", cty.NullVal(cty.DynamicPseudoType), false)
}),
})
plan, diags := ctx.Plan()
if diags.HasErrors() {
t.Fatal(diags.Err())
}

for _, c := range plan.Changes.Outputs {
if c.Action != plans.NoOp {
t.Fatalf("expected no changes, got %s for %q", c.Action, c.Addr)
}
}
}
11 changes: 5 additions & 6 deletions terraform/node_output.go
Original file line number Diff line number Diff line change
Expand Up @@ -451,10 +451,12 @@ func (n *NodeApplyableOutput) setValue(state *states.SyncState, changes *plans.C
// strip any marks here just to be sure we don't panic on the True comparison
val, _ = val.UnmarkDeep()

var action plans.Action
action := plans.Update
switch {
case val.IsNull():
action = plans.Delete
case val.IsNull() && before.IsNull():
// This is separate from the NoOp case below, since we can ignore
// sensitivity here if there are only null values.
action = plans.NoOp

case before.IsNull():
action = plans.Create
Expand All @@ -467,9 +469,6 @@ func (n *NodeApplyableOutput) setValue(state *states.SyncState, changes *plans.C
// only one we can act on, and the state will have been loaded
// without any marks to consider.
action = plans.NoOp

default:
action = plans.Update
}

change := &plans.OutputChange{
Expand Down

0 comments on commit 57f004e

Please sign in to comment.