From b88410984bbec27810a2bf70f06f5c93a1ddca4f Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 3 Aug 2018 12:17:42 -0400 Subject: [PATCH] legacy provider needs to handle StateUpgraders In order to not require state migrations to be supported in both MigrateState and StateUpgraders, the legacy provider codepath needs to handle the StateUpgraders transparently during Refresh. --- helper/schema/core_schema_test.go | 6 -- helper/schema/resource.go | 139 +++++++++++++++++++++++++-- helper/schema/resource_test.go | 154 ++++++++++++++++++++++++++++++ 3 files changed, 286 insertions(+), 13 deletions(-) diff --git a/helper/schema/core_schema_test.go b/helper/schema/core_schema_test.go index aedd72ea4..c9b0513aa 100644 --- a/helper/schema/core_schema_test.go +++ b/helper/schema/core_schema_test.go @@ -4,17 +4,11 @@ import ( "testing" "github.com/google/go-cmp/cmp" - "github.com/google/go-cmp/cmp/cmpopts" "github.com/zclconf/go-cty/cty" "github.com/hashicorp/terraform/configs/configschema" ) -var ( - equateEmpty = cmpopts.EquateEmpty() - typeComparer = cmp.Comparer(cty.Type.Equals) -) - // add the implicit "id" attribute for test resources func testResource(block *configschema.Block) *configschema.Block { if block.Attributes == nil { diff --git a/helper/schema/resource.go b/helper/schema/resource.go index 01ecd4cc3..9b45af427 100644 --- a/helper/schema/resource.go +++ b/helper/schema/resource.go @@ -340,6 +340,59 @@ func (r *Resource) ReadDataApply( return r.recordCurrentSchemaVersion(state), err } +// RefreshWithoutUpgrade reads the instance state, but does not call +// MigrateState or the StateUpgraders, since those are now invoked in a +// separate API call. +// RefreshWithoutUpgrade is part of the new plugin shims. +func (r *Resource) RefreshWithoutUpgrade( + s *terraform.InstanceState, + meta interface{}) (*terraform.InstanceState, error) { + // If the ID is already somehow blank, it doesn't exist + if s.ID == "" { + return nil, nil + } + + rt := ResourceTimeout{} + if _, ok := s.Meta[TimeoutKey]; ok { + if err := rt.StateDecode(s); err != nil { + log.Printf("[ERR] Error decoding ResourceTimeout: %s", err) + } + } + + if r.Exists != nil { + // Make a copy of data so that if it is modified it doesn't + // affect our Read later. + data, err := schemaMap(r.Schema).Data(s, nil) + data.timeouts = &rt + + if err != nil { + return s, err + } + + exists, err := r.Exists(data, meta) + if err != nil { + return s, err + } + if !exists { + return nil, nil + } + } + + data, err := schemaMap(r.Schema).Data(s, nil) + data.timeouts = &rt + if err != nil { + return s, err + } + + err = r.Read(data, meta) + state := data.State() + if state != nil && state.ID == "" { + state = nil + } + + return r.recordCurrentSchemaVersion(state), err +} + // Refresh refreshes the state of the resource. func (r *Resource) Refresh( s *terraform.InstanceState, @@ -375,12 +428,10 @@ func (r *Resource) Refresh( } } - needsMigration, stateSchemaVersion := r.checkSchemaVersion(s) - if needsMigration && r.MigrateState != nil { - s, err := r.MigrateState(stateSchemaVersion, s, meta) - if err != nil { - return s, err - } + // there may be new StateUpgraders that need to be run + s, err := r.upgradeState(s, meta) + if err != nil { + return s, err } data, err := schemaMap(r.Schema).Data(s, nil) @@ -398,6 +449,72 @@ func (r *Resource) Refresh( return r.recordCurrentSchemaVersion(state), err } +func (r *Resource) upgradeState(s *terraform.InstanceState, meta interface{}) (*terraform.InstanceState, error) { + var err error + + needsMigration, stateSchemaVersion := r.checkSchemaVersion(s) + migrate := needsMigration && r.MigrateState != nil + + if migrate { + s, err = r.MigrateState(stateSchemaVersion, s, meta) + if err != nil { + return s, err + } + } + + if len(r.StateUpgraders) == 0 { + return s, nil + } + + // If we ran MigrateState, then the stateSchemaVersion value is no longer + // correct. We can expect the first upgrade function to be the correct + // schema type version. + if migrate { + stateSchemaVersion = r.StateUpgraders[0].Version + } + + schemaType := r.CoreConfigSchema().ImpliedType() + // find the expected type to convert the state + for _, upgrader := range r.StateUpgraders { + if stateSchemaVersion == upgrader.Version { + schemaType = upgrader.Type + } + } + + // StateUpgraders only operate on the new JSON format state, so the state + // need to be converted. + stateVal, err := StateValueFromInstanceState(s, schemaType) + if err != nil { + return nil, err + } + + jsonState, err := StateValueToJSONMap(stateVal, schemaType) + if err != nil { + return nil, err + } + + for _, upgrader := range r.StateUpgraders { + if stateSchemaVersion != upgrader.Version { + continue + } + + jsonState, err = upgrader.Upgrade(jsonState, meta) + if err != nil { + return nil, err + } + stateSchemaVersion++ + } + + // now we need to re-flatmap the new state + stateVal, err = JSONMapToStateValue(jsonState, r.CoreConfigSchema()) + if err != nil { + return nil, err + } + + s = InstanceStateFromStateValue(stateVal, r.SchemaVersion) + return s, nil +} + // InternalValidate should be called to validate the structure // of the resource. // @@ -603,7 +720,15 @@ func (r *Resource) checkSchemaVersion(is *terraform.InstanceState) (bool, int) { } stateSchemaVersion, _ := strconv.Atoi(rawString) - return stateSchemaVersion < r.SchemaVersion, stateSchemaVersion + + // Don't run MigrateState if the version is handled by a StateUpgrader, + // since StateMigrateFuncs are not required to handle unknown versions + maxVersion := r.SchemaVersion + if len(r.StateUpgraders) > 0 { + maxVersion = r.StateUpgraders[0].Version + } + + return stateSchemaVersion < maxVersion, stateSchemaVersion } func (r *Resource) recordCurrentSchemaVersion( diff --git a/helper/schema/resource_test.go b/helper/schema/resource_test.go index d9f8820df..0b366d4d6 100644 --- a/helper/schema/resource_test.go +++ b/helper/schema/resource_test.go @@ -8,6 +8,7 @@ import ( "testing" "time" + "github.com/google/go-cmp/cmp" "github.com/hashicorp/terraform/config" "github.com/hashicorp/terraform/config/hcl2shim" "github.com/hashicorp/terraform/terraform" @@ -1540,3 +1541,156 @@ func TestResource_ValidateUpgradeState(t *testing.T) { t.Fatal("StateUpgraders cannot have a version >= current SchemaVersion") } } + +// The legacy provider will need to be able to handle both types of schema +// transformations, which has been retrofitted into the Refresh method. +func TestResource_migrateAndUpgrade(t *testing.T) { + r := &Resource{ + SchemaVersion: 4, + Schema: map[string]*Schema{ + "four": { + Type: TypeInt, + Required: true, + }, + }, + // this MigrateState will take the state to version 2 + MigrateState: func(v int, is *terraform.InstanceState, _ interface{}) (*terraform.InstanceState, error) { + switch v { + case 0: + _, ok := is.Attributes["zero"] + if !ok { + return nil, fmt.Errorf("zero not found in %#v", is.Attributes) + } + is.Attributes["one"] = "1" + delete(is.Attributes, "zero") + fallthrough + case 1: + _, ok := is.Attributes["one"] + if !ok { + return nil, fmt.Errorf("one not found in %#v", is.Attributes) + } + is.Attributes["two"] = "2" + delete(is.Attributes, "one") + default: + return nil, fmt.Errorf("invalid schema version %d", v) + } + return is, nil + }, + } + + r.Read = func(d *ResourceData, m interface{}) error { + return d.Set("four", 4) + } + + r.StateUpgraders = []StateUpgrader{ + { + Version: 2, + Type: cty.Object(map[string]cty.Type{ + "id": cty.String, + "two": cty.Number, + }), + Upgrade: func(m map[string]interface{}, meta interface{}) (map[string]interface{}, error) { + _, ok := m["two"].(float64) + if !ok { + return nil, fmt.Errorf("two not found in %#v", m) + } + m["three"] = float64(3) + delete(m, "two") + return m, nil + }, + }, + { + Version: 3, + Type: cty.Object(map[string]cty.Type{ + "id": cty.String, + "three": cty.Number, + }), + Upgrade: func(m map[string]interface{}, meta interface{}) (map[string]interface{}, error) { + _, ok := m["three"].(float64) + if !ok { + return nil, fmt.Errorf("three not found in %#v", m) + } + m["four"] = float64(4) + delete(m, "three") + return m, nil + }, + }, + } + + testStates := []*terraform.InstanceState{ + { + ID: "bar", + Attributes: map[string]string{ + "id": "bar", + "zero": "0", + }, + Meta: map[string]interface{}{ + "schema_version": "0", + }, + }, + { + ID: "bar", + Attributes: map[string]string{ + "id": "bar", + "one": "1", + }, + Meta: map[string]interface{}{ + "schema_version": "1", + }, + }, + { + ID: "bar", + Attributes: map[string]string{ + "id": "bar", + "two": "2", + }, + Meta: map[string]interface{}{ + "schema_version": "2", + }, + }, + { + ID: "bar", + Attributes: map[string]string{ + "id": "bar", + "three": "3", + }, + Meta: map[string]interface{}{ + "schema_version": "3", + }, + }, + { + ID: "bar", + Attributes: map[string]string{ + "id": "bar", + "four": "4", + }, + Meta: map[string]interface{}{ + "schema_version": "4", + }, + }, + } + + for i, s := range testStates { + t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { + newState, err := r.Refresh(s, nil) + if err != nil { + t.Fatal(err) + } + + expected := &terraform.InstanceState{ + ID: "bar", + Attributes: map[string]string{ + "id": "bar", + "four": "4", + }, + Meta: map[string]interface{}{ + "schema_version": "4", + }, + } + + if !cmp.Equal(expected, newState, equateEmpty) { + t.Fatal(cmp.Diff(expected, newState, equateEmpty)) + } + }) + } +}