From b99c615ee67ff1c4f5c1e3c8d4cf5511e1f5b932 Mon Sep 17 00:00:00 2001 From: Chris Marchesi Date: Mon, 22 May 2017 21:32:59 -0700 Subject: [PATCH] helper/schema: New ResourceDiff object This adds a new object, ResourceDiff, to the schema package. This object, in conjunction with a function defined in CustomizeDiff in the resource schema, allows for the in-flight customization of a Terraform diff. This helps support use cases such as when there are necessary changes to a resource that cannot be detected in config, such as via computed fields (most of the utility in this object works on computed fields only). It also allows for a wholesale wipe of the diff to allow for diff logic completely offloaded to an external API, if it is a better use case for a specific provider. As part of this work, many internal diff functions have been moved to use a special resourceDiffer interface, to allow for shared functionality between ResourceDiff and ResourceData. This may be extended to the DiffSuppressFunc as well which would restrict use of ResourceData in DiffSuppressFunc to generally read-only fields. This work is not yet in its final state - CustomizeDiff is not yet implemented in the general diff workflow, new functions may be added (notably Clear() for a single key), and functionality may be altered. Tests will follow as well. --- helper/schema/resource.go | 13 ++ helper/schema/resource_diff.go | 233 +++++++++++++++++++++++++++++++++ helper/schema/schema.go | 34 +++-- 3 files changed, 270 insertions(+), 10 deletions(-) create mode 100644 helper/schema/resource_diff.go diff --git a/helper/schema/resource.go b/helper/schema/resource.go index 97b357706..5c2a4e0d7 100644 --- a/helper/schema/resource.go +++ b/helper/schema/resource.go @@ -85,6 +85,16 @@ type Resource struct { Delete DeleteFunc Exists ExistsFunc + // CustomizeDiff is a custom function for controlling diff logic after the + // initial diff is performed - it can be used to veto particular changes in + // the diff, customize the diff that has been created, or diff values not + // controlled by config. It is passed a *ResourceDiff, a structure similar to + // ResourceData but lacking most write functions, allowing the provider to + // customize the diff only. + // + // Only computed fields can be customized by this function. + CustomizeDiff CustomizeDiffFunc + // Importer is the ResourceImporter implementation for this resource. // If this is nil, then this resource does not support importing. If // this is non-nil, then it supports importing and ResourceImporter @@ -126,6 +136,9 @@ type ExistsFunc func(*ResourceData, interface{}) (bool, error) type StateMigrateFunc func( int, *terraform.InstanceState, interface{}) (*terraform.InstanceState, error) +// See Resource documentation. +type CustomizeDiffFunc func(*ResourceDiff, interface{}) error + // Apply creates, updates, and/or deletes a resource. func (r *Resource) Apply( s *terraform.InstanceState, diff --git a/helper/schema/resource_diff.go b/helper/schema/resource_diff.go new file mode 100644 index 000000000..66b72d906 --- /dev/null +++ b/helper/schema/resource_diff.go @@ -0,0 +1,233 @@ +package schema + +import ( + "fmt" + "reflect" + "strings" + + "github.com/hashicorp/terraform/terraform" +) + +// ResourceDiff is used to query and make custom changes to an in-flight diff. +// It can be used to veto particular changes in the diff, customize the diff +// that has been created, or diff values not controlled by config. +// +// The object functions similar to ResourceData, however most notably lacks +// Set, SetPartial, and Partial, as it should only be used to change diff +// values only. Most other frist-class ResourceData functions exist, namely +// Get, GetOk, HasChange, and GetChange exist. +// +// All functions in ResourceDiff, save for ForceNew, can only be used on +// computed fields. +type ResourceDiff struct { + // The underlying ResourceData object used as a refrence and diff storage. + data *ResourceData + + // A source "copy" of the ResourceData object, designed to preserve the + // original diff, schema, and state to allow for rollbacks. + originalData *ResourceData + + // A writer that holds overridden old fields. + oldWriter *MapFieldWriter + + // A reader that is tied to oldWriter's map. + oldReader *MapFieldReader + + // A writer that holds overridden new fields. + newWriter *MapFieldWriter + + // A reader that is tied to newWriter's map. + newReader *MapFieldReader + + // A map of keys that will be force-flagged as computed. + computedKeys map[string]bool + + // A catalog of top-level keys that are safely diffable. diffChange will + // panic if the key is not found here to guard against bugs and edge cases + // when processing diffs. + catalog map[string]bool +} + +// newResourceDiff creates a new ResourceDiff instance. +func newResourceDiff(diff *terraform.InstanceDiff, data *ResourceData) *ResourceDiff { + d := new(ResourceDiff) + d.originalData = &ResourceData{ + schema: data.schema, + state: data.state, + config: data.config, + diff: diff, + } + d.data = new(ResourceData) + d.data.config = data.config + d.data.state = d.originalData.state.DeepCopy() + d.data.diff = d.originalData.diff.DeepCopy() + for k, v := range d.originalData.schema { + newSchema := *v + d.data.schema[k] = &newSchema + } + + d.data.once.Do(d.data.init) + + d.oldWriter = &MapFieldWriter{Schema: data.schema} + d.oldReader = &MapFieldReader{ + Schema: data.schema, + Map: BasicMapReader(d.oldWriter.Map()), + } + + d.newWriter = &MapFieldWriter{Schema: data.schema} + d.newReader = &MapFieldReader{ + Schema: data.schema, + Map: BasicMapReader(d.newWriter.Map()), + } + + return d +} + +// ClearAll re-creates the ResourceDiff instance and drops the old one on the +// floor. The new instance starts off without a diff. +func (d *ResourceDiff) ClearAll() { + nd := newResourceDiff(d.originalData.diff, d.originalData) + nd.data.diff = new(terraform.InstanceDiff) + d = nd +} + +// Reset re-creates the ResourceDiff instance, similar to ClearAll, but with +// the original diff preserved. +func (d *ResourceDiff) Reset() { + nd := newResourceDiff(d.originalData.diff, d.originalData) + d = nd +} + +// getDiff returns the current diff as it is in the underlying ResourceData +// object. +func (d *ResourceDiff) getDiff() *terraform.InstanceDiff { + return d.data.diff +} + +// diffChange helps to implement resourceDiffer and derives its change values +// from ResourceDiff's own change data. +// +// Note that it's a currently unsupported operation to diff a field (and hence +// use this function) on a field that has not been explicitly operated on with +// SetNew, SetNewComputed, or SetDiff. The function will panic if you do. +func (d *ResourceDiff) diffChange(key string) (interface{}, interface{}, bool, bool) { + // Panic if key was never set by any of our diff functions. It's not a legit + // use case of this function to be used outside of very specific functions in + // ResourceDiff. + if _, ok := d.catalog[key]; !ok { + panic(fmt.Errorf("ResourceDiff.diffChange: %s was not found as a valid set key", key)) + } + + old, err := d.oldReader.ReadField(strings.Split(key, ".")) + if err != nil { + panic(fmt.Errorf("ResourceDiff.diffChange: Reading old value for %s failed: %s", key, err)) + } + new, err := d.newReader.ReadField(strings.Split(key, ".")) + if err != nil { + panic(fmt.Errorf("ResourceDiff.diffChange: Reading new value for %s failed: %s", key, err)) + } + + if !old.Exists { + old.Value = nil + } + if !new.Exists { + new.Value = nil + } + + var computed bool + if v, ok := d.computedKeys[strings.Split(key, ".")[0]]; ok { + computed = v + } + + return old.Value, new.Value, !reflect.DeepEqual(old.Value, new.Value), computed +} + +// SetNew is used to set a new diff value for the mentioned key. The value must +// be correct for the attribute's schema (mostly relevant for maps, lists, and +// sets). The original value from the state is used as the old value. +// +// This function is only allowed on computed attributes. +// +// It is an unsupported operation to set invalid values with this function - +// doing so will taint any existing diff for this key and will remove it from +// the catalog. +func (d *ResourceDiff) SetNew(key string, value interface{}) error { + return d.SetDiff(key, d.Get(key), value, false) +} + +// SetNewComputed functions like SetNew, except that it sets the new value to +// the zero value and flags the attribute's diff as computed. +// +// This function is only allowed on computed keys. +func (d *ResourceDiff) SetNewComputed(key string) error { + return d.SetDiff(key, d.Get(key), d.data.schema[key].ZeroValue(), true) +} + +// SetDiff allows the setting of both old and new values for the diff +// referenced by a given key. This can be used to completely override +// Terraform's own diff behaviour, and can be used in conjunction with Clear or +// ClearAll to construct a compleletely new diff based off of provider logic +// alone. +// +// This function is only allowed on computed keys. +func (d *ResourceDiff) SetDiff(key string, old, new interface{}, computed bool) error { + if !d.data.schema[key].Computed { + return fmt.Errorf("SetNew, SetNewComputed, and SetDiff are allowed on computed attributes only - %s is not one", key) + } + + if err := d.oldWriter.WriteField(strings.Split(key, "."), old); err != nil { + delete(d.catalog, key) + return fmt.Errorf("Cannot set old diff value for key %s: %s", key, err) + } + + if err := d.newWriter.WriteField(strings.Split(key, "."), new); err != nil { + delete(d.catalog, key) + return fmt.Errorf("Cannot set new diff value for key %s: %s", key, err) + } + + d.computedKeys[key] = computed + d.catalog[key] = true + + return schemaMap(d.data.schema).diff(key, d.data.schema[key], d.data.diff, d, false) +} + +// ForceNew force-flags ForceNew in the schema for a specific key, and +// re-calculates its diff. This function is a no-op/error if there is no diff. +// +// Note that the change to schema is permanent for the lifecycle of this +// specific ResourceDiff instance, until ClearAll or Reset is called to start +// anew. +func (d *ResourceDiff) ForceNew(key string) error { + if !d.HasChange(key) { + return fmt.Errorf("ResourceDiff.ForceNew: No changes for %s", key) + } + + old, new := d.GetChange(key) + d.data.schema[key].ForceNew = true + return d.SetDiff(key, old, new, false) +} + +// Get hands off to ResourceData.Get. +func (d *ResourceDiff) Get(key string) interface{} { + return d.data.Get(key) +} + +// GetChange hands off to ResourceData.GetChange. +func (d *ResourceDiff) GetChange(key string) (interface{}, interface{}) { + return d.data.getChange(key, getSourceState, getSourceDiff) +} + +// GetOk hands off to ResourceData.GetOk. +func (d *ResourceDiff) GetOk(key string) (interface{}, bool) { + return d.data.GetOk(key) +} + +// HasChange hands off to ResourceData.HasChange. +func (d *ResourceDiff) HasChange(key string) bool { + return d.data.HasChange(key) +} + +// Id hands off to ResourceData.Id. +func (d *ResourceDiff) Id() string { + return d.data.Id() +} diff --git a/helper/schema/schema.go b/helper/schema/schema.go index b1e0fc6bd..67bbe112a 100644 --- a/helper/schema/schema.go +++ b/helper/schema/schema.go @@ -684,11 +684,23 @@ func isValidFieldName(name string) bool { return re.MatchString(name) } +// resourceDiffer is an interface that is used by the private diff functions. +// This helps facilitate diff logic for both ResourceData and ResoureDiff with +// minimal divergence in code. +type resourceDiffer interface { + diffChange(string) (interface{}, interface{}, bool, bool) + Get(string) interface{} + GetChange(string) (interface{}, interface{}) + GetOk(string) (interface{}, bool) + HasChange(string) bool + Id() string +} + func (m schemaMap) diff( k string, schema *Schema, diff *terraform.InstanceDiff, - d *ResourceData, + d resourceDiffer, all bool) error { unsupressedDiff := new(terraform.InstanceDiff) @@ -709,12 +721,14 @@ func (m schemaMap) diff( } for attrK, attrV := range unsupressedDiff.Attributes { - if schema.DiffSuppressFunc != nil && - attrV != nil && - schema.DiffSuppressFunc(attrK, attrV.Old, attrV.New, d) { - continue + switch rd := d.(type) { + case *ResourceData: + if schema.DiffSuppressFunc != nil && + attrV != nil && + schema.DiffSuppressFunc(attrK, attrV.Old, attrV.New, rd) { + continue + } } - diff.Attributes[attrK] = attrV } @@ -725,7 +739,7 @@ func (m schemaMap) diffList( k string, schema *Schema, diff *terraform.InstanceDiff, - d *ResourceData, + d resourceDiffer, all bool) error { o, n, _, computedList := d.diffChange(k) if computedList { @@ -844,7 +858,7 @@ func (m schemaMap) diffMap( k string, schema *Schema, diff *terraform.InstanceDiff, - d *ResourceData, + d resourceDiffer, all bool) error { prefix := k + "." @@ -938,7 +952,7 @@ func (m schemaMap) diffSet( k string, schema *Schema, diff *terraform.InstanceDiff, - d *ResourceData, + d resourceDiffer, all bool) error { o, n, _, computedSet := d.diffChange(k) @@ -1059,7 +1073,7 @@ func (m schemaMap) diffString( k string, schema *Schema, diff *terraform.InstanceDiff, - d *ResourceData, + d resourceDiffer, all bool) error { var originalN interface{} var os, ns string