plans: Retain output value changes for all outputs in memory

During the plan operation we need to retain _somewhere_ the planned
changes for all outputs so we can refer to them during expression
evaluation. For consistency with how we handle resource instance changes,
we'll keep them in the plan so we can properly retain unknown values,
which cannot be written to state.

As with output values in the state, only root output plans are retained
in a round-trip through the on-disk plan file format, but that's okay
because we can trivially re-calculate all of these during apply. We
include the _root_ outputs in the plan file only because they are
externally-visible side effects that ought to be included in any rendering
of the plan made from the plan file for user inspection.

plans/changes.go

@@ -12,19 +12,27 @@ import (
 // A Changes object can be rendered into a visual diff (by the caller, using
 // code in another package) for display to the user.
 type Changes struct {
+	// Resources tracks planned changes to resource instance objects.
 	Resources []*ResourceInstanceChangeSrc
-	RootOutputs map[string]*OutputChangeSrc
+
+	// Outputs tracks planned changes output values.
+	//
+	// Note that although an in-memory plan contains planned changes for
+	// outputs throughout the configuration, a plan serialized
+	// to disk retains only the root outputs because they are
+	// externally-visible, while other outputs are implementation details and
+	// can be easily re-calculated during the apply phase. Therefore only root
+	// module outputs will survive a round-trip through a plan file.
+	Outputs []*OutputChangeSrc
 }
 
 // NewChanges returns a valid Changes object that describes no changes.
 func NewChanges() *Changes {
-	return &Changes{
-		RootOutputs: make(map[string]*OutputChangeSrc),
-	}
+	return &Changes{}
 }
 
 func (c *Changes) Empty() bool {
-	return (len(c.Resources) + len(c.RootOutputs)) == 0
+	return (len(c.Resources) + len(c.Outputs)) == 0
 }
 
 // ResourceInstance returns the planned change for the current object of the
@@ -55,6 +63,19 @@ func (c *Changes) ResourceInstanceDeposed(addr addrs.AbsResourceInstance, key st
 	return nil
 }
 
+// OutputValue returns the planned change for the output value with the
+//  given address, if any. Returns nil if no change is planned.
+func (c *Changes) OutputValue(addr addrs.AbsOutputValue) *OutputChangeSrc {
+	addrStr := addr.String()
+	for _, oc := range c.Outputs {
+		if oc.Addr.String() == addrStr {
+			return oc
+		}
+	}
+
+	return nil
+}
+
 // SyncWrapper returns a wrapper object around the receiver that can be used
 // to make certain changes to the receiver in a concurrency-safe way, as long
 // as all callers share the same wrapper object.
@@ -203,6 +224,10 @@ func (rc *ResourceInstanceChange) Simplify(destroying bool) *ResourceInstanceCha
 
 // OutputChange describes a change to an output value.
 type OutputChange struct {
+	// Addr is the absolute address of the output value that the change
+	// will apply to.
+	Addr addrs.AbsOutputValue
+
 	// Change is an embedded description of the change.
 	//
 	// For output value changes, the type constraint for the DynamicValue

plans/changes_src.go

@@ -94,8 +94,12 @@ func (rcs *ResourceInstanceChangeSrc) DeepCopy() *ResourceInstanceChangeSrc {
 	return &ret
 }
 
-// OutputChange describes a change to an output value.
+// OutputChangeSrc describes a change to an output value.
 type OutputChangeSrc struct {
+	// Addr is the absolute address of the output value that the change
+	// will apply to.
+	Addr addrs.AbsOutputValue
+
 	// ChangeSrc is an embedded description of the not-yet-decoded change.
 	//
 	// For output value changes, the type constraint for the DynamicValue
@@ -117,11 +121,32 @@ func (ocs *OutputChangeSrc) Decode() (*OutputChange, error) {
 		return nil, err
 	}
 	return &OutputChange{
+		Addr:      ocs.Addr,
 		Change:    *change,
 		Sensitive: ocs.Sensitive,
 	}, nil
 }
 
+// DeepCopy creates a copy of the receiver where any pointers to nested mutable
+// values are also copied, thus ensuring that future mutations of the receiver
+// will not affect the copy.
+//
+// Some types used within a resource change are immutable by convention even
+// though the Go language allows them to be mutated, such as the types from
+// the addrs package. These are _not_ copied by this method, under the
+// assumption that callers will behave themselves.
+func (ocs *OutputChangeSrc) DeepCopy() *OutputChangeSrc {
+	if ocs == nil {
+		return nil
+	}
+	ret := *ocs
+
+	ret.ChangeSrc.Before = ret.ChangeSrc.Before.Copy()
+	ret.ChangeSrc.After = ret.ChangeSrc.After.Copy()
+
+	return &ret
+}
+
 // ChangeSrc is a not-yet-decoded Change.
 type ChangeSrc struct {
 	// Action defines what kind of change is being made.

plans/changes_sync.go

@@ -87,3 +87,58 @@ func (cs *ChangesSync) RemoveResourceInstanceChange(addr addrs.AbsResourceInstan
 		return
 	}
 }
+
+// AppendOutputChange records the given output value change in the set of
+// planned value changes.
+//
+// The caller must ensure that there are no concurrent writes to the given
+// change while this method is running, but it is safe to resume mutating
+// it after this method returns without affecting the saved change.
+func (cs *ChangesSync) AppendOutputChange(changeSrc *OutputChangeSrc) {
+	if cs == nil {
+		panic("AppendOutputChange on nil ChangesSync")
+	}
+	cs.lock.Lock()
+	defer cs.lock.Unlock()
+
+	s := changeSrc.DeepCopy()
+	cs.changes.Outputs = append(cs.changes.Outputs, s)
+}
+
+// GetOutputChange searches the set of output value changes for one matching
+// the given address, returning it if it exists.
+//
+// If no such change exists, nil is returned.
+//
+// The returned object is a deep copy of the change recorded in the plan, so
+// callers may mutate it although it's generally better (less confusing) to
+// treat planned changes as immutable after they've been initially constructed.
+func (cs *ChangesSync) GetOutputChange(addr addrs.AbsOutputValue) *OutputChangeSrc {
+	if cs == nil {
+		panic("GetOutputChange on nil ChangesSync")
+	}
+	cs.lock.Lock()
+	defer cs.lock.Unlock()
+
+	return cs.changes.OutputValue(addr)
+}
+
+// RemoveOutputChange searches the set of output value changes for one matching
+// the given address, and removes it from the set if it exists.
+func (cs *ChangesSync) RemoveOutputChange(addr addrs.AbsOutputValue) {
+	if cs == nil {
+		panic("RemoveOutputChange on nil ChangesSync")
+	}
+	cs.lock.Lock()
+	defer cs.lock.Unlock()
+
+	addrStr := addr.String()
+	for i, r := range cs.changes.Resources {
+		if r.Addr.String() != addrStr {
+			continue
+		}
+		copy(cs.changes.Outputs[i:], cs.changes.Outputs[i+1:])
+		cs.changes.Outputs = cs.changes.Outputs[:len(cs.changes.Outputs)-1]
+		return
+	}
+}

plans/planfile/planfile_test.go

@@ -44,7 +44,7 @@ func TestRoundtrip(t *testing.T) {
 	planIn := &plans.Plan{
 		Changes: &plans.Changes{
 			Resources: []*plans.ResourceInstanceChangeSrc{},
-			RootOutputs: map[string]*plans.OutputChangeSrc{},
+			Outputs:   []*plans.OutputChangeSrc{},
 		},
 		ProviderSHA256s: map[string][]byte{},
 		VariableValues: map[string]plans.DynamicValue{

plans/planfile/tfplan.go

@@ -52,7 +52,7 @@ func readTfplan(r io.Reader) (*plans.Plan, error) {
 	plan := &plans.Plan{
 		VariableValues: map[string]plans.DynamicValue{},
 		Changes: &plans.Changes{
-			RootOutputs: map[string]*plans.OutputChangeSrc{},
+			Outputs:   []*plans.OutputChangeSrc{},
 			Resources: []*plans.ResourceInstanceChangeSrc{},
 		},
 
@@ -66,10 +66,14 @@ func readTfplan(r io.Reader) (*plans.Plan, error) {
 			return nil, fmt.Errorf("invalid plan for output %q: %s", name, err)
 		}
 
-		plan.Changes.RootOutputs[name] = &plans.OutputChangeSrc{
+		plan.Changes.Outputs = append(plan.Changes.Outputs, &plans.OutputChangeSrc{
+			// All output values saved in the plan file are root module outputs,
+			// since we don't retain others. (They can be easily recomputed
+			// during apply).
+			Addr:      addrs.OutputValue{Name: name}.Absolute(addrs.RootModuleInstance),
 			ChangeSrc: *change,
 			Sensitive: rawOC.Sensitive,
-		}
+		})
 	}
 
 	for _, rawRC := range rawPlan.ResourceChanges {
@@ -288,7 +292,16 @@ func writeTfplan(plan *plans.Plan, w io.Writer) error {
 		ResourceChanges: []*planproto.ResourceInstanceChange{},
 	}
 
-	for name, oc := range plan.Changes.RootOutputs {
+	for _, oc := range plan.Changes.Outputs {
+		// When serializing a plan we only retain the root outputs, since
+		// changes to these are externally-visible side effects (e.g. via
+		// terraform_remote_state).
+		if !oc.Addr.Module.IsRoot() {
+			continue
+		}
+
+		name := oc.Addr.OutputValue.Name
+
 		// Writing outputs as cty.DynamicPseudoType forces the stored values
 		// to also contain dynamic type information, so we can recover the
 		// original type when we read the values back in readTFPlan.

plans/planfile/tfplan_test.go

@@ -21,15 +21,17 @@ func TestTFPlanRoundTrip(t *testing.T) {
 			"foo": mustNewDynamicValueStr("foo value"),
 		},
 		Changes: &plans.Changes{
-			RootOutputs: map[string]*plans.OutputChangeSrc{
-				"bar": {
+			Outputs: []*plans.OutputChangeSrc{
+				{
+					Addr: addrs.OutputValue{Name: "bar"}.Absolute(addrs.RootModuleInstance),
 					ChangeSrc: plans.ChangeSrc{
 						Action: plans.Create,
 						After:  mustNewDynamicValueStr("bar value"),
 					},
 					Sensitive: false,
 				},
-				"baz": {
+				{
+					Addr: addrs.OutputValue{Name: "baz"}.Absolute(addrs.RootModuleInstance),
 					ChangeSrc: plans.ChangeSrc{
 						Action: plans.NoOp,
 						Before: mustNewDynamicValueStr("baz value"),
@@ -37,7 +39,8 @@ func TestTFPlanRoundTrip(t *testing.T) {
 					},
 					Sensitive: false,
 				},
-				"secret": {
+				{
+					Addr: addrs.OutputValue{Name: "secret"}.Absolute(addrs.RootModuleInstance),
 					ChangeSrc: plans.ChangeSrc{
 						Action: plans.Update,
 						Before: mustNewDynamicValueStr("old secret value"),