From 94078f90294161d211aed9ef2bd70dc4656fb3a3 Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Wed, 5 Jun 2019 23:23:45 -0400 Subject: [PATCH] helper/plugin: Allow missing MigrateState for provider flatmap state upgrades Reference: https://github.com/terraform-providers/terraform-provider-aws/issues/8828 Prior to Terraform 0.12, providers were not required to implement the `MigrateState` function when increasing the `SchemaVersion` above `0`. Effectively there is no flatmap state difference between version 0 and defined `SchemaVersion` or lowest `StateUpgrader` `Version` (whichever is lowest) when `MigrateState` is undefined, so here we remove the error and increase the schema version automatically. --- helper/plugin/grpc_provider.go | 13 +++-- helper/plugin/grpc_provider_test.go | 80 +++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 5 deletions(-) diff --git a/helper/plugin/grpc_provider.go b/helper/plugin/grpc_provider.go index 3bdc98898..161af4d46 100644 --- a/helper/plugin/grpc_provider.go +++ b/helper/plugin/grpc_provider.go @@ -2,7 +2,6 @@ package plugin import ( "encoding/json" - "errors" "fmt" "log" "strconv" @@ -316,11 +315,15 @@ func (s *GRPCProviderServer) upgradeFlatmapState(version int, m map[string]strin requiresMigrate = version < res.StateUpgraders[0].Version } - if requiresMigrate { - if res.MigrateState == nil { - return nil, 0, errors.New("cannot upgrade state, missing MigrateState function") + if requiresMigrate && res.MigrateState == nil { + // Providers were previously allowed to bump the version + // without declaring MigrateState. + // If there are further upgraders, then we've only updated that far. + if len(res.StateUpgraders) > 0 { + schemaType = res.StateUpgraders[0].Type + upgradedVersion = res.StateUpgraders[0].Version } - + } else if requiresMigrate { is := &terraform.InstanceState{ ID: m["id"], Attributes: m, diff --git a/helper/plugin/grpc_provider_test.go b/helper/plugin/grpc_provider_test.go index 6186cd411..89858913f 100644 --- a/helper/plugin/grpc_provider_test.go +++ b/helper/plugin/grpc_provider_test.go @@ -437,6 +437,86 @@ func TestUpgradeState_flatmapState(t *testing.T) { } } +func TestUpgradeState_flatmapStateMissingMigrateState(t *testing.T) { + r := &schema.Resource{ + SchemaVersion: 1, + Schema: map[string]*schema.Schema{ + "one": { + Type: schema.TypeInt, + Required: true, + }, + }, + } + + server := &GRPCProviderServer{ + provider: &schema.Provider{ + ResourcesMap: map[string]*schema.Resource{ + "test": r, + }, + }, + } + + testReqs := []*proto.UpgradeResourceState_Request{ + { + TypeName: "test", + Version: 0, + RawState: &proto.RawState{ + Flatmap: map[string]string{ + "id": "bar", + "one": "1", + }, + }, + }, + { + TypeName: "test", + Version: 1, + RawState: &proto.RawState{ + Flatmap: map[string]string{ + "id": "bar", + "one": "1", + }, + }, + }, + { + TypeName: "test", + Version: 1, + RawState: &proto.RawState{ + Json: []byte(`{"id":"bar","one":1}`), + }, + }, + } + + for i, req := range testReqs { + t.Run(fmt.Sprintf("%d-%d", i, req.Version), func(t *testing.T) { + resp, err := server.UpgradeResourceState(nil, req) + if err != nil { + t.Fatal(err) + } + + if len(resp.Diagnostics) > 0 { + for _, d := range resp.Diagnostics { + t.Errorf("%#v", d) + } + t.Fatal("error") + } + + val, err := msgpack.Unmarshal(resp.UpgradedState.Msgpack, r.CoreConfigSchema().ImpliedType()) + if err != nil { + t.Fatal(err) + } + + expected := cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("bar"), + "one": cty.NumberIntVal(1), + }) + + if !cmp.Equal(expected, val, valueComparer, equateEmpty) { + t.Fatal(cmp.Diff(expected, val, valueComparer, equateEmpty)) + } + }) + } +} + func TestPlanResourceChange(t *testing.T) { r := &schema.Resource{ SchemaVersion: 4,