create a SimpleDiff for the new provider shims

Terraform now handles any actual "diffing" of resource, and the existing
Diff functions are only used to shim the schema.Provider to the new
methods. Since terraform is handling what used to be the Diff, the
provider now should not modify the diff based on RequiresNew due to it
interfering with the ignore_changes handling.
This commit is contained in:
James Bardin 2018-10-15 19:29:37 -04:00 committed by Martin Atkins
parent 0d4d572c39
commit 46b4c27dbe
9 changed files with 138 additions and 106 deletions

View File

@ -6,8 +6,8 @@ import (
"github.com/hashicorp/terraform/tfdiags" "github.com/hashicorp/terraform/tfdiags"
"github.com/zclconf/go-cty/cty" "github.com/zclconf/go-cty/cty"
"github.com/hashicorp/terraform/configs/configschema"
"github.com/hashicorp/terraform/config/hcl2shim" "github.com/hashicorp/terraform/config/hcl2shim"
"github.com/hashicorp/terraform/configs/configschema"
"github.com/hashicorp/terraform/terraform" "github.com/hashicorp/terraform/terraform"
) )
@ -77,7 +77,7 @@ func (b *Backend) Configure(obj cty.Value) tfdiags.Diagnostics {
// Get a ResourceData for this configuration. To do this, we actually // Get a ResourceData for this configuration. To do this, we actually
// generate an intermediary "diff" although that is never exposed. // generate an intermediary "diff" although that is never exposed.
diff, err := sm.Diff(nil, shimRC, nil, nil) diff, err := sm.Diff(nil, shimRC, nil, nil, true)
if err != nil { if err != nil {
diags = diags.Append(err) diags = diags.Append(err)
return diags return diags

View File

@ -251,7 +251,7 @@ func (p *Provider) Configure(c *terraform.ResourceConfig) error {
// Get a ResourceData for this configuration. To do this, we actually // Get a ResourceData for this configuration. To do this, we actually
// generate an intermediary "diff" although that is never exposed. // generate an intermediary "diff" although that is never exposed.
diff, err := sm.Diff(nil, c, nil, p.meta) diff, err := sm.Diff(nil, c, nil, p.meta, true)
if err != nil { if err != nil {
return err return err
} }
@ -296,6 +296,20 @@ func (p *Provider) Diff(
return r.Diff(s, c, p.meta) return r.Diff(s, c, p.meta)
} }
// SimpleDiff is used by the new protocol wrappers to get a diff that doesn't
// attempt to calculate ignore_changes.
func (p *Provider) SimpleDiff(
info *terraform.InstanceInfo,
s *terraform.InstanceState,
c *terraform.ResourceConfig) (*terraform.InstanceDiff, error) {
r, ok := p.ResourcesMap[info.Type]
if !ok {
return nil, fmt.Errorf("unknown resource type: %s", info.Type)
}
return r.simpleDiff(s, c, p.meta)
}
// Refresh implementation of terraform.ResourceProvider interface. // Refresh implementation of terraform.ResourceProvider interface.
func (p *Provider) Refresh( func (p *Provider) Refresh(
info *terraform.InstanceInfo, info *terraform.InstanceInfo,
@ -311,7 +325,7 @@ func (p *Provider) Refresh(
// Resources implementation of terraform.ResourceProvider interface. // Resources implementation of terraform.ResourceProvider interface.
func (p *Provider) Resources() []terraform.ResourceType { func (p *Provider) Resources() []terraform.ResourceType {
keys := make([]string, 0, len(p.ResourcesMap)) keys := make([]string, 0, len(p.ResourcesMap))
for k, _ := range p.ResourcesMap { for k := range p.ResourcesMap {
keys = append(keys, k) keys = append(keys, k)
} }
sort.Strings(keys) sort.Strings(keys)

View File

@ -152,7 +152,7 @@ func (p *Provisioner) Apply(
} }
sm := schemaMap(p.ConnSchema) sm := schemaMap(p.ConnSchema)
diff, err := sm.Diff(nil, terraform.NewResourceConfig(c), nil, nil) diff, err := sm.Diff(nil, terraform.NewResourceConfig(c), nil, nil, true)
if err != nil { if err != nil {
return err return err
} }
@ -166,7 +166,7 @@ func (p *Provisioner) Apply(
// Build the configuration data. Doing this requires making a "diff" // Build the configuration data. Doing this requires making a "diff"
// even though that's never used. We use that just to get the correct types. // even though that's never used. We use that just to get the correct types.
configMap := schemaMap(p.Schema) configMap := schemaMap(p.Schema)
diff, err := configMap.Diff(nil, c, nil, nil) diff, err := configMap.Diff(nil, c, nil, nil, true)
if err != nil { if err != nil {
return err return err
} }

View File

@ -287,7 +287,35 @@ func (r *Resource) Diff(
return nil, fmt.Errorf("[ERR] Error decoding timeout: %s", err) return nil, fmt.Errorf("[ERR] Error decoding timeout: %s", err)
} }
instanceDiff, err := schemaMap(r.Schema).Diff(s, c, r.CustomizeDiff, meta) instanceDiff, err := schemaMap(r.Schema).Diff(s, c, r.CustomizeDiff, meta, true)
if err != nil {
return instanceDiff, err
}
if instanceDiff != nil {
if err := t.DiffEncode(instanceDiff); err != nil {
log.Printf("[ERR] Error encoding timeout to instance diff: %s", err)
}
} else {
log.Printf("[DEBUG] Instance Diff is nil in Diff()")
}
return instanceDiff, err
}
func (r *Resource) simpleDiff(
s *terraform.InstanceState,
c *terraform.ResourceConfig,
meta interface{}) (*terraform.InstanceDiff, error) {
t := &ResourceTimeout{}
err := t.ConfigDecode(r, c)
if err != nil {
return nil, fmt.Errorf("[ERR] Error decoding timeout: %s", err)
}
instanceDiff, err := schemaMap(r.Schema).Diff(s, c, r.CustomizeDiff, meta, false)
if err != nil { if err != nil {
return instanceDiff, err return instanceDiff, err
} }

View File

@ -410,7 +410,8 @@ func (m schemaMap) Diff(
s *terraform.InstanceState, s *terraform.InstanceState,
c *terraform.ResourceConfig, c *terraform.ResourceConfig,
customizeDiff CustomizeDiffFunc, customizeDiff CustomizeDiffFunc,
meta interface{}) (*terraform.InstanceDiff, error) { meta interface{},
handleRequiresNew bool) (*terraform.InstanceDiff, error) {
result := new(terraform.InstanceDiff) result := new(terraform.InstanceDiff)
result.Attributes = make(map[string]*terraform.ResourceAttrDiff) result.Attributes = make(map[string]*terraform.ResourceAttrDiff)
@ -456,82 +457,85 @@ func (m schemaMap) Diff(
} }
} }
// If the diff requires a new resource, then we recompute the diff if handleRequiresNew {
// so we have the complete new resource diff, and preserve the // If the diff requires a new resource, then we recompute the diff
// RequiresNew fields where necessary so the user knows exactly what // so we have the complete new resource diff, and preserve the
// caused that. // RequiresNew fields where necessary so the user knows exactly what
if result.RequiresNew() { // caused that.
// Create the new diff if result.RequiresNew() {
result2 := new(terraform.InstanceDiff) // Create the new diff
result2.Attributes = make(map[string]*terraform.ResourceAttrDiff) result2 := new(terraform.InstanceDiff)
result2.Attributes = make(map[string]*terraform.ResourceAttrDiff)
// Preserve the DestroyTainted flag // Preserve the DestroyTainted flag
result2.DestroyTainted = result.DestroyTainted result2.DestroyTainted = result.DestroyTainted
// Reset the data to not contain state. We have to call init() // Reset the data to not contain state. We have to call init()
// again in order to reset the FieldReaders. // again in order to reset the FieldReaders.
d.state = nil d.state = nil
d.init() d.init()
// Perform the diff again // Perform the diff again
for k, schema := range m { for k, schema := range m {
err := m.diff(k, schema, result2, d, false) err := m.diff(k, schema, result2, d, false)
if err != nil {
return nil, err
}
}
// Re-run customization
if !result2.DestroyTainted && customizeDiff != nil {
mc := m.DeepCopy()
rd := newResourceDiff(mc, c, d.state, result2)
if err := customizeDiff(rd, meta); err != nil {
return nil, err
}
for _, k := range rd.UpdatedKeys() {
err := m.diff(k, mc[k], result2, rd, false)
if err != nil { if err != nil {
return nil, err return nil, err
} }
} }
// Re-run customization
if !result2.DestroyTainted && customizeDiff != nil {
mc := m.DeepCopy()
rd := newResourceDiff(mc, c, d.state, result2)
if err := customizeDiff(rd, meta); err != nil {
return nil, err
}
for _, k := range rd.UpdatedKeys() {
err := m.diff(k, mc[k], result2, rd, false)
if err != nil {
return nil, err
}
}
}
// Force all the fields to not force a new since we know what we
// want to force new.
for k, attr := range result2.Attributes {
if attr == nil {
continue
}
if attr.RequiresNew {
attr.RequiresNew = false
}
if s != nil {
attr.Old = s.Attributes[k]
}
}
// Now copy in all the requires new diffs...
for k, attr := range result.Attributes {
if attr == nil {
continue
}
newAttr, ok := result2.Attributes[k]
if !ok {
newAttr = attr
}
if attr.RequiresNew {
newAttr.RequiresNew = true
}
result2.Attributes[k] = newAttr
}
// And set the diff!
result = result2
} }
// Force all the fields to not force a new since we know what we
// want to force new.
for k, attr := range result2.Attributes {
if attr == nil {
continue
}
if attr.RequiresNew {
attr.RequiresNew = false
}
if s != nil {
attr.Old = s.Attributes[k]
}
}
// Now copy in all the requires new diffs...
for k, attr := range result.Attributes {
if attr == nil {
continue
}
newAttr, ok := result2.Attributes[k]
if !ok {
newAttr = attr
}
if attr.RequiresNew {
newAttr.RequiresNew = true
}
result2.Attributes[k] = newAttr
}
// And set the diff!
result = result2
} }
// Go through and detect all of the ComputedWhens now that we've // Go through and detect all of the ComputedWhens now that we've

View File

@ -3246,7 +3246,7 @@ func TestSchemaMap_Diff(t *testing.T) {
} }
} }
d, err := schemaMap(tc.Schema).Diff(tc.State, terraform.NewResourceConfig(c), tc.CustomizeDiff, nil) d, err := schemaMap(tc.Schema).Diff(tc.State, terraform.NewResourceConfig(c), tc.CustomizeDiff, nil, true)
if err != nil != tc.Err { if err != nil != tc.Err {
t.Fatalf("err: %s", err) t.Fatalf("err: %s", err)
} }
@ -4096,7 +4096,7 @@ func TestSchemaMap_DiffSuppress(t *testing.T) {
} }
} }
d, err := schemaMap(tc.Schema).Diff(tc.State, terraform.NewResourceConfig(c), nil, nil) d, err := schemaMap(tc.Schema).Diff(tc.State, terraform.NewResourceConfig(c), nil, nil, true)
if err != nil != tc.Err { if err != nil != tc.Err {
t.Fatalf("#%q err: %s", tn, err) t.Fatalf("#%q err: %s", tn, err)
} }

View File

@ -29,7 +29,7 @@ func diffFromValues(prior, planned cty.Value, res *Resource, cust CustomizeDiffF
cfg := terraform.NewResourceConfigShimmed(planned, configSchema) cfg := terraform.NewResourceConfigShimmed(planned, configSchema)
diff, err := schemaMap(res.Schema).Diff(instanceState, cfg, cust, nil) diff, err := schemaMap(res.Schema).Diff(instanceState, cfg, cust, nil, false)
if err != nil { if err != nil {
return nil, err return nil, err
} }
@ -41,8 +41,8 @@ func diffFromValues(prior, planned cty.Value, res *Resource, cust CustomizeDiffF
// get a new cty.Value state. This is used to convert the diff returned from // get a new cty.Value state. This is used to convert the diff returned from
// the legacy provider Diff method to the state required for the new // the legacy provider Diff method to the state required for the new
// PlanResourceChange method. // PlanResourceChange method.
func ApplyDiff(state cty.Value, d *terraform.InstanceDiff, schemaBlock *configschema.Block) (cty.Value, error) { func ApplyDiff(base cty.Value, d *terraform.InstanceDiff, schema *configschema.Block) (cty.Value, error) {
return d.ApplyToValue(state, schemaBlock) return d.ApplyToValue(base, schema)
} }
// StateValueToJSONMap converts a cty.Value to generic JSON map via the cty JSON // StateValueToJSONMap converts a cty.Value to generic JSON map via the cty JSON

View File

@ -2024,11 +2024,11 @@ func TestShimSchemaMap_Diff(t *testing.T) {
RequiresNew: true, RequiresNew: true,
}, },
"address": &terraform.ResourceAttrDiff{ // "address": &terraform.ResourceAttrDiff{
Old: "foo", // Old: "foo",
New: "", // New: "",
NewComputed: true, // NewComputed: true,
}, // },
}, },
}, },
@ -2075,11 +2075,11 @@ func TestShimSchemaMap_Diff(t *testing.T) {
RequiresNew: true, RequiresNew: true,
}, },
"ports.#": &terraform.ResourceAttrDiff{ // "ports.#": &terraform.ResourceAttrDiff{
Old: "1", // Old: "1",
New: "", // New: "",
NewComputed: true, // NewComputed: true,
}, // },
}, },
}, },
@ -2849,10 +2849,6 @@ func TestShimSchemaMap_Diff(t *testing.T) {
Diff: &terraform.InstanceDiff{ Diff: &terraform.InstanceDiff{
Attributes: map[string]*terraform.ResourceAttrDiff{ Attributes: map[string]*terraform.ResourceAttrDiff{
"instances.#": &terraform.ResourceAttrDiff{
Old: "2",
New: "2",
},
"instances.2": &terraform.ResourceAttrDiff{ "instances.2": &terraform.ResourceAttrDiff{
Old: "22", Old: "22",
New: "", New: "",
@ -2973,10 +2969,6 @@ func TestShimSchemaMap_Diff(t *testing.T) {
Diff: &terraform.InstanceDiff{ Diff: &terraform.InstanceDiff{
Attributes: map[string]*terraform.ResourceAttrDiff{ Attributes: map[string]*terraform.ResourceAttrDiff{
"ports.#": &terraform.ResourceAttrDiff{
Old: "3",
New: "3",
},
"ports.1": &terraform.ResourceAttrDiff{ "ports.1": &terraform.ResourceAttrDiff{
Old: "1", Old: "1",
New: "1", New: "1",
@ -3103,8 +3095,6 @@ func TestShimSchemaMap_Diff(t *testing.T) {
Diff: &terraform.InstanceDiff{ Diff: &terraform.InstanceDiff{
Attributes: map[string]*terraform.ResourceAttrDiff{ Attributes: map[string]*terraform.ResourceAttrDiff{
"ports.#": &terraform.ResourceAttrDiff{ "ports.#": &terraform.ResourceAttrDiff{
Old: "3",
New: "",
NewComputed: true, NewComputed: true,
RequiresNew: true, RequiresNew: true,
}, },
@ -3357,10 +3347,6 @@ func TestShimSchemaMap_Diff(t *testing.T) {
Diff: &terraform.InstanceDiff{ Diff: &terraform.InstanceDiff{
Attributes: map[string]*terraform.ResourceAttrDiff{ Attributes: map[string]*terraform.ResourceAttrDiff{
"ports.#": &terraform.ResourceAttrDiff{
Old: "3",
New: "3",
},
"ports.1": &terraform.ResourceAttrDiff{ "ports.1": &terraform.ResourceAttrDiff{
Old: "1", Old: "1",
New: "1", New: "1",
@ -3576,7 +3562,7 @@ func TestShimSchemaMap_Diff(t *testing.T) {
} }
{ {
d, err := InternalMap(tc.Schema).Diff(tc.State, terraform.NewResourceConfig(c), tc.CustomizeDiff, nil) d, err := InternalMap(tc.Schema).Diff(tc.State, terraform.NewResourceConfig(c), tc.CustomizeDiff, nil, false)
if err != nil != tc.Err { if err != nil != tc.Err {
t.Fatalf("err: %s", err) t.Fatalf("err: %s", err)
} }

View File

@ -18,7 +18,7 @@ func TestResourceDataRaw(
} }
sm := schemaMap(schema) sm := schemaMap(schema)
diff, err := sm.Diff(nil, terraform.NewResourceConfig(c), nil, nil) diff, err := sm.Diff(nil, terraform.NewResourceConfig(c), nil, nil, true)
if err != nil { if err != nil {
t.Fatalf("err: %s", err) t.Fatalf("err: %s", err)
} }