From 3342aa580c7cab26c2b047f21388d13c7f3ad095 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 23 Feb 2017 10:44:05 -0800 Subject: [PATCH 1/2] terraform: InstanceState.Meta is value type interface{} This changes the type of values in Meta for InstanceState to `interface{}`. They were `string` before. This will allow richer structures to be persisted to this without flatmapping them (down with flatmap!). The documentation clearly states that only primitives/collections are allowed here. The only thing using this was helper/schema for schema versioning. Appropriate type checking was added to make this change safe. The timeout work @catsby is doing will use this for a richer structure. --- helper/schema/resource.go | 21 ++++++++++++++++++--- helper/schema/resource_data.go | 2 +- helper/schema/resource_test.go | 18 +++++++++--------- terraform/state.go | 7 ++++--- terraform/state_test.go | 12 ++++++------ terraform/state_upgrade_v1_to_v2.go | 8 +++++++- 6 files changed, 45 insertions(+), 23 deletions(-) diff --git a/helper/schema/resource.go b/helper/schema/resource.go index 9087c6932..51ddb14dc 100644 --- a/helper/schema/resource.go +++ b/helper/schema/resource.go @@ -353,7 +353,7 @@ func (r *Resource) Data(s *terraform.InstanceState) *ResourceData { } // Set the schema version to latest by default - result.meta = map[string]string{ + result.meta = map[string]interface{}{ "schema_version": strconv.Itoa(r.SchemaVersion), } @@ -378,7 +378,22 @@ func (r *Resource) isTopLevel() bool { // Determines if a given InstanceState needs to be migrated by checking the // stored version number with the current SchemaVersion func (r *Resource) checkSchemaVersion(is *terraform.InstanceState) (bool, int) { - stateSchemaVersion, _ := strconv.Atoi(is.Meta["schema_version"]) + // Get the raw interface{} value for the schema version. If it doesn't + // exist or is nil then set it to zero. + raw := is.Meta["schema_version"] + if raw == nil { + raw = "0" + } + + // Try to convert it to a string. If it isn't a string then we pretend + // that it isn't set at all. It should never not be a string unless it + // was manually tampered with. + rawString, ok := raw.(string) + if !ok { + rawString = "0" + } + + stateSchemaVersion, _ := strconv.Atoi(rawString) return stateSchemaVersion < r.SchemaVersion, stateSchemaVersion } @@ -386,7 +401,7 @@ func (r *Resource) recordCurrentSchemaVersion( state *terraform.InstanceState) *terraform.InstanceState { if state != nil && r.SchemaVersion > 0 { if state.Meta == nil { - state.Meta = make(map[string]string) + state.Meta = make(map[string]interface{}) } state.Meta["schema_version"] = strconv.Itoa(r.SchemaVersion) } diff --git a/helper/schema/resource_data.go b/helper/schema/resource_data.go index b040b63ee..f20a6b386 100644 --- a/helper/schema/resource_data.go +++ b/helper/schema/resource_data.go @@ -23,7 +23,7 @@ type ResourceData struct { config *terraform.ResourceConfig state *terraform.InstanceState diff *terraform.InstanceDiff - meta map[string]string + meta map[string]interface{} // Don't set multiReader *MultiLevelFieldReader diff --git a/helper/schema/resource_test.go b/helper/schema/resource_test.go index 14704ea7b..89cd8a46a 100644 --- a/helper/schema/resource_test.go +++ b/helper/schema/resource_test.go @@ -52,7 +52,7 @@ func TestResourceApply_create(t *testing.T) { "id": "foo", "foo": "42", }, - Meta: map[string]string{ + Meta: map[string]interface{}{ "schema_version": "2", }, } @@ -210,7 +210,7 @@ func TestResourceApply_destroyPartial(t *testing.T) { "id": "bar", "foo": "42", }, - Meta: map[string]string{ + Meta: map[string]interface{}{ "schema_version": "3", }, } @@ -558,7 +558,7 @@ func TestResourceRefresh(t *testing.T) { "id": "bar", "foo": "13", }, - Meta: map[string]string{ + Meta: map[string]interface{}{ "schema_version": "2", }, } @@ -749,7 +749,7 @@ func TestResourceRefresh_needsMigration(t *testing.T) { Attributes: map[string]string{ "oldfoo": "1.2", }, - Meta: map[string]string{ + Meta: map[string]interface{}{ "schema_version": "1", }, } @@ -765,7 +765,7 @@ func TestResourceRefresh_needsMigration(t *testing.T) { "id": "bar", "newfoo": "13", }, - Meta: map[string]string{ + Meta: map[string]interface{}{ "schema_version": "2", }, } @@ -803,7 +803,7 @@ func TestResourceRefresh_noMigrationNeeded(t *testing.T) { Attributes: map[string]string{ "newfoo": "12", }, - Meta: map[string]string{ + Meta: map[string]interface{}{ "schema_version": "2", }, } @@ -819,7 +819,7 @@ func TestResourceRefresh_noMigrationNeeded(t *testing.T) { "id": "bar", "newfoo": "13", }, - Meta: map[string]string{ + Meta: map[string]interface{}{ "schema_version": "2", }, } @@ -871,7 +871,7 @@ func TestResourceRefresh_stateSchemaVersionUnset(t *testing.T) { "id": "bar", "newfoo": "13", }, - Meta: map[string]string{ + Meta: map[string]interface{}{ "schema_version": "1", }, } @@ -945,7 +945,7 @@ func TestResourceData(t *testing.T) { } // Set expectations - state.Meta = map[string]string{ + state.Meta = map[string]interface{}{ "schema_version": "2", } diff --git a/terraform/state.go b/terraform/state.go index 1965c0105..f9cc5602c 100644 --- a/terraform/state.go +++ b/terraform/state.go @@ -1541,8 +1541,9 @@ type InstanceState struct { // Meta is a simple K/V map that is persisted to the State but otherwise // ignored by Terraform core. It's meant to be used for accounting by - // external client code. - Meta map[string]string `json:"meta"` + // external client code. The value here must only contain Go primitives + // and collections. + Meta map[string]interface{} `json:"meta"` // Tainted is used to mark a resource for recreation. Tainted bool `json:"tainted"` @@ -1561,7 +1562,7 @@ func (s *InstanceState) init() { s.Attributes = make(map[string]string) } if s.Meta == nil { - s.Meta = make(map[string]string) + s.Meta = make(map[string]interface{}) } s.Ephemeral.init() } diff --git a/terraform/state_test.go b/terraform/state_test.go index 694c9f751..331bcf437 100644 --- a/terraform/state_test.go +++ b/terraform/state_test.go @@ -278,7 +278,7 @@ func TestStateDeepCopy(t *testing.T) { Resources: map[string]*ResourceState{ "test_instance.foo": &ResourceState{ Primary: &InstanceState{ - Meta: map[string]string{}, + Meta: map[string]interface{}{}, }, }, }, @@ -298,7 +298,7 @@ func TestStateDeepCopy(t *testing.T) { Resources: map[string]*ResourceState{ "test_instance.foo": &ResourceState{ Primary: &InstanceState{ - Meta: map[string]string{}, + Meta: map[string]interface{}{}, }, Deposed: []*InstanceState{ {ID: "test"}, @@ -389,7 +389,7 @@ func TestStateEqual(t *testing.T) { Resources: map[string]*ResourceState{ "test_instance.foo": &ResourceState{ Primary: &InstanceState{ - Meta: map[string]string{ + Meta: map[string]interface{}{ "schema_version": "1", }, }, @@ -405,7 +405,7 @@ func TestStateEqual(t *testing.T) { Resources: map[string]*ResourceState{ "test_instance.foo": &ResourceState{ Primary: &InstanceState{ - Meta: map[string]string{ + Meta: map[string]interface{}{ "schema_version": "2", }, }, @@ -610,7 +610,7 @@ func TestStateIncrementSerialMaybe(t *testing.T) { Resources: map[string]*ResourceState{ "test_instance.foo": &ResourceState{ Primary: &InstanceState{ - Meta: map[string]string{}, + Meta: map[string]interface{}{}, }, }, }, @@ -625,7 +625,7 @@ func TestStateIncrementSerialMaybe(t *testing.T) { Resources: map[string]*ResourceState{ "test_instance.foo": &ResourceState{ Primary: &InstanceState{ - Meta: map[string]string{ + Meta: map[string]interface{}{ "schema_version": "1", }, }, diff --git a/terraform/state_upgrade_v1_to_v2.go b/terraform/state_upgrade_v1_to_v2.go index 038615336..928cdba11 100644 --- a/terraform/state_upgrade_v1_to_v2.go +++ b/terraform/state_upgrade_v1_to_v2.go @@ -150,16 +150,22 @@ func (old *instanceStateV1) upgradeToV2() (*InstanceState, error) { if err != nil { return nil, fmt.Errorf("Error upgrading InstanceState V1: %v", err) } + meta, err := copystructure.Copy(old.Meta) if err != nil { return nil, fmt.Errorf("Error upgrading InstanceState V1: %v", err) } + newMeta := make(map[string]interface{}) + for k, v := range meta.(map[string]string) { + newMeta[k] = v + } + return &InstanceState{ ID: old.ID, Attributes: attributes.(map[string]string), Ephemeral: *ephemeral, - Meta: meta.(map[string]string), + Meta: newMeta, }, nil } From 4c7c46bf40d9c98d92e0ad5cf4c5ec0c14b60abf Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 23 Feb 2017 10:51:29 -0800 Subject: [PATCH 2/2] command: fix test for new Meta type --- command/refresh_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command/refresh_test.go b/command/refresh_test.go index c196db394..2defaff45 100644 --- a/command/refresh_test.go +++ b/command/refresh_test.go @@ -794,7 +794,7 @@ func newInstanceState(id string) *terraform.InstanceState { Ephemeral: terraform.EphemeralState{ ConnInfo: make(map[string]string), }, - Meta: make(map[string]string), + Meta: make(map[string]interface{}), } }