diff --git a/plans/changes.go b/plans/changes.go index fac1fec748..69bfffaaa4 100644 --- a/plans/changes.go +++ b/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 []*ResourceInstanceChangeSrc - RootOutputs map[string]*OutputChangeSrc + // Resources tracks planned changes to resource instance objects. + Resources []*ResourceInstanceChangeSrc + + // 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 diff --git a/plans/changes_src.go b/plans/changes_src.go index 6c46f9b311..f2782ac3cc 100644 --- a/plans/changes_src.go +++ b/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. diff --git a/plans/changes_sync.go b/plans/changes_sync.go index 258869986d..e9305eaf94 100644 --- a/plans/changes_sync.go +++ b/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 + } +} diff --git a/plans/planfile/planfile_test.go b/plans/planfile/planfile_test.go index 7624b3b0e9..5b44e05b25 100644 --- a/plans/planfile/planfile_test.go +++ b/plans/planfile/planfile_test.go @@ -43,8 +43,8 @@ func TestRoundtrip(t *testing.T) { // file is tested more fully in tfplan_test.go . planIn := &plans.Plan{ Changes: &plans.Changes{ - Resources: []*plans.ResourceInstanceChangeSrc{}, - RootOutputs: map[string]*plans.OutputChangeSrc{}, + Resources: []*plans.ResourceInstanceChangeSrc{}, + Outputs: []*plans.OutputChangeSrc{}, }, ProviderSHA256s: map[string][]byte{}, VariableValues: map[string]plans.DynamicValue{ diff --git a/plans/planfile/tfplan.go b/plans/planfile/tfplan.go index 88dcf50d32..7875422251 100644 --- a/plans/planfile/tfplan.go +++ b/plans/planfile/tfplan.go @@ -52,8 +52,8 @@ func readTfplan(r io.Reader) (*plans.Plan, error) { plan := &plans.Plan{ VariableValues: map[string]plans.DynamicValue{}, Changes: &plans.Changes{ - RootOutputs: map[string]*plans.OutputChangeSrc{}, - Resources: []*plans.ResourceInstanceChangeSrc{}, + Outputs: []*plans.OutputChangeSrc{}, + Resources: []*plans.ResourceInstanceChangeSrc{}, }, ProviderSHA256s: map[string][]byte{}, @@ -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. diff --git a/plans/planfile/tfplan_test.go b/plans/planfile/tfplan_test.go index 7ab7377f05..dcb76530a3 100644 --- a/plans/planfile/tfplan_test.go +++ b/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"),