diff --git a/plans/changes.go b/plans/changes.go index cc6e58263..d7e0dcdb8 100644 --- a/plans/changes.go +++ b/plans/changes.go @@ -254,6 +254,7 @@ func (oc *OutputChange) Encode() (*OutputChangeSrc, error) { return nil, err } return &OutputChangeSrc{ + Addr: oc.Addr, ChangeSrc: *cs, Sensitive: oc.Sensitive, }, err diff --git a/plans/changes_sync.go b/plans/changes_sync.go index e9305eaf9..6b4ff98ff 100644 --- a/plans/changes_sync.go +++ b/plans/changes_sync.go @@ -133,8 +133,8 @@ func (cs *ChangesSync) RemoveOutputChange(addr addrs.AbsOutputValue) { defer cs.lock.Unlock() addrStr := addr.String() - for i, r := range cs.changes.Resources { - if r.Addr.String() != addrStr { + for i, o := range cs.changes.Outputs { + if o.Addr.String() != addrStr { continue } copy(cs.changes.Outputs[i:], cs.changes.Outputs[i+1:]) diff --git a/terraform/context_plan_test.go b/terraform/context_plan_test.go index 581a0f429..e7f0e7ab2 100644 --- a/terraform/context_plan_test.go +++ b/terraform/context_plan_test.go @@ -5513,3 +5513,137 @@ func objectVal(t *testing.T, schema *configschema.Block, m map[string]cty.Value) } return v } + +func TestContext2Plan_requiredModuleOutput(t *testing.T) { + m := testModule(t, "plan-required-output") + p := testProvider("test") + p.GetSchemaReturn = &ProviderSchema{ + ResourceTypes: map[string]*configschema.Block{ + "test_resource": { + Attributes: map[string]*configschema.Attribute{ + "id": {Type: cty.String, Computed: true}, + "required": {Type: cty.String, Required: true}, + }, + }, + }, + } + p.DiffFn = testDiffFn + + ctx := testContext2(t, &ContextOpts{ + Config: m, + ProviderResolver: providers.ResolverFixed( + map[string]providers.Factory{ + "test": testProviderFuncFixed(p), + }, + ), + }) + + plan, diags := ctx.Plan() + if diags.HasErrors() { + t.Fatalf("unexpected errors: %s", diags.Err()) + } + + schema := p.GetSchemaReturn.ResourceTypes["test_resource"] + ty := schema.ImpliedType() + + if len(plan.Changes.Resources) != 2 { + t.Fatal("expected 2 changes, got", len(plan.Changes.Resources)) + } + + for _, res := range plan.Changes.Resources { + t.Run(fmt.Sprintf("%s %s", res.Action, res.Addr), func(t *testing.T) { + if res.Action != plans.Create { + t.Fatalf("expected resource creation, got %s", res.Action) + } + ric, err := res.Decode(ty) + if err != nil { + t.Fatal(err) + } + + var expected cty.Value + switch i := ric.Addr.String(); i { + case "test_resource.root": + expected = objectVal(t, schema, map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + "required": cty.UnknownVal(cty.String), + }) + case "module.mod.test_resource.for_output": + expected = objectVal(t, schema, map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + "required": cty.StringVal("val"), + }) + default: + t.Fatal("unknown instance:", i) + } + + checkVals(t, expected, ric.After) + }) + } +} + +func TestContext2Plan_requiredModuleObject(t *testing.T) { + m := testModule(t, "plan-required-whole-mod") + p := testProvider("test") + p.GetSchemaReturn = &ProviderSchema{ + ResourceTypes: map[string]*configschema.Block{ + "test_resource": { + Attributes: map[string]*configschema.Attribute{ + "id": {Type: cty.String, Computed: true}, + "required": {Type: cty.String, Required: true}, + }, + }, + }, + } + p.DiffFn = testDiffFn + + ctx := testContext2(t, &ContextOpts{ + Config: m, + ProviderResolver: providers.ResolverFixed( + map[string]providers.Factory{ + "test": testProviderFuncFixed(p), + }, + ), + }) + + plan, diags := ctx.Plan() + if diags.HasErrors() { + t.Fatalf("unexpected errors: %s", diags.Err()) + } + + schema := p.GetSchemaReturn.ResourceTypes["test_resource"] + ty := schema.ImpliedType() + + if len(plan.Changes.Resources) != 2 { + t.Fatal("expected 2 changes, got", len(plan.Changes.Resources)) + } + + for _, res := range plan.Changes.Resources { + t.Run(fmt.Sprintf("%s %s", res.Action, res.Addr), func(t *testing.T) { + if res.Action != plans.Create { + t.Fatalf("expected resource creation, got %s", res.Action) + } + ric, err := res.Decode(ty) + if err != nil { + t.Fatal(err) + } + + var expected cty.Value + switch i := ric.Addr.String(); i { + case "test_resource.root": + expected = objectVal(t, schema, map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + "required": cty.UnknownVal(cty.String), + }) + case "module.mod.test_resource.for_output": + expected = objectVal(t, schema, map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + "required": cty.StringVal("val"), + }) + default: + t.Fatal("unknown instance:", i) + } + + checkVals(t, expected, ric.After) + }) + } +} diff --git a/terraform/eval_output.go b/terraform/eval_output.go index 6829934f0..10573971f 100644 --- a/terraform/eval_output.go +++ b/terraform/eval_output.go @@ -9,6 +9,7 @@ import ( "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/plans" + "github.com/hashicorp/terraform/states" ) // EvalDeleteOutput is an EvalNode implementation that deletes an output @@ -61,22 +62,33 @@ func (n *EvalWriteOutput) Eval(ctx EvalContext) (interface{}, error) { // if we're continuing, make sure the output is included, and // marked as unknown. If the evaluator was able to find a type // for the value in spite of the error then we'll use it. - state.SetOutputValue(addr, cty.UnknownVal(val.Type()), n.Sensitive) + n.setValue(addr, state, changes, cty.UnknownVal(val.Type())) return nil, EvalEarlyExitError{} } return nil, diags.Err() } + n.setValue(addr, state, changes, val) + + return nil, nil +} + +func (n *EvalWriteOutput) setValue(addr addrs.AbsOutputValue, state *states.SyncState, changes *plans.ChangesSync, val cty.Value) { if val.IsKnown() && !val.IsNull() { // The state itself doesn't represent unknown values, so we null them // out here and then we'll save the real unknown value in the planned // changeset below, if we have one on this graph walk. + log.Printf("[TRACE] EvalWriteOutput: Saving value for %s in state", addr) stateVal := cty.UnknownAsNull(val) state.SetOutputValue(addr, stateVal, n.Sensitive) } else { + log.Printf("[TRACE] EvalWriteOutput: Removing %s from state (it is now null)", addr) state.RemoveOutputValue(addr) } + // If we also have an active changeset then we'll replicate the value in + // there. This is used in preference to the state where present, since it + // *is* able to represent unknowns, while the state cannot. if changes != nil { // For the moment we are not properly tracking changes to output // values, and just marking them always as "Create" or "Destroy" @@ -116,8 +128,8 @@ func (n *EvalWriteOutput) Eval(ctx EvalContext) (interface{}, error) { // Should never happen, since we just constructed this right above panic(fmt.Sprintf("planned change for %s could not be encoded: %s", addr, err)) } - changes.AppendOutputChange(cs) + log.Printf("[TRACE] EvalWriteOutput: Saving %s change for %s in changeset", change.Action, addr) + changes.RemoveOutputChange(addr) // remove any existing planned change, if present + changes.AppendOutputChange(cs) // add the new planned change } - - return nil, nil } diff --git a/terraform/evaluate.go b/terraform/evaluate.go index bed3a6cc8..918df2e87 100644 --- a/terraform/evaluate.go +++ b/terraform/evaluate.go @@ -305,14 +305,31 @@ func (d *evaluationStateData) GetModuleInstance(addr addrs.ModuleCallInstance, r vals := map[string]cty.Value{} for n := range outputConfigs { addr := addrs.OutputValue{Name: n}.Absolute(moduleAddr) - os := d.Evaluator.State.OutputValue(addr) - if os == nil { - // Not evaluated yet? - vals[n] = cty.DynamicVal - continue - } - vals[n] = os.Value + // If a pending change is present in our current changeset then its value + // takes priority over what's in state. (It will usually be the same but + // will differ if the new value is unknown during planning.) + if changeSrc := d.Evaluator.Changes.GetOutputChange(addr); changeSrc != nil { + change, err := changeSrc.Decode() + if err != nil { + // This should happen only if someone has tampered with a plan + // file, so we won't bother with a pretty error for it. + diags = diags.Append(fmt.Errorf("planned change for %s could not be decoded: %s", addr, err)) + vals[n] = cty.DynamicVal + continue + } + // We care only about the "after" value, which is the value this output + // will take on after the plan is applied. + vals[n] = change.After + } else { + os := d.Evaluator.State.OutputValue(addr) + if os == nil { + // Not evaluated yet? + vals[n] = cty.DynamicVal + continue + } + vals[n] = os.Value + } } return cty.ObjectVal(vals), diags } diff --git a/terraform/test-fixtures/plan-required-output/main.tf b/terraform/test-fixtures/plan-required-output/main.tf new file mode 100644 index 000000000..227b5c153 --- /dev/null +++ b/terraform/test-fixtures/plan-required-output/main.tf @@ -0,0 +1,7 @@ +resource "test_resource" "root" { + required = module.mod.object.id +} + +module "mod" { + source = "./mod" +} diff --git a/terraform/test-fixtures/plan-required-output/mod/main.tf b/terraform/test-fixtures/plan-required-output/mod/main.tf new file mode 100644 index 000000000..772f1645f --- /dev/null +++ b/terraform/test-fixtures/plan-required-output/mod/main.tf @@ -0,0 +1,7 @@ +resource "test_resource" "for_output" { + required = "val" +} + +output "object" { + value = test_resource.for_output +} diff --git a/terraform/test-fixtures/plan-required-whole-mod/main.tf b/terraform/test-fixtures/plan-required-whole-mod/main.tf new file mode 100644 index 000000000..9deb3c5a1 --- /dev/null +++ b/terraform/test-fixtures/plan-required-whole-mod/main.tf @@ -0,0 +1,17 @@ +resource "test_resource" "root" { + required = local.object.id +} + +locals { + # This indirection is here to force the evaluator to produce the whole + # module object here rather than just fetching the single "object" output. + # This makes this fixture different than plan-required-output, which just + # accesses module.mod.object.id directly and thus visits a different + # codepath in the evaluator. + mod = module.mod + object = local.mod.object +} + +module "mod" { + source = "./mod" +} diff --git a/terraform/test-fixtures/plan-required-whole-mod/mod/main.tf b/terraform/test-fixtures/plan-required-whole-mod/mod/main.tf new file mode 100644 index 000000000..772f1645f --- /dev/null +++ b/terraform/test-fixtures/plan-required-whole-mod/mod/main.tf @@ -0,0 +1,7 @@ +resource "test_resource" "for_output" { + required = "val" +} + +output "object" { + value = test_resource.for_output +}