From b78b8ce3dbf5cdadb2dfb4a2d7de831e5377fd29 Mon Sep 17 00:00:00 2001 From: Paddy Carver Date: Tue, 15 Dec 2020 20:55:38 -0800 Subject: [PATCH] Allow opting into using json.Number in state. Go's JSON decoder, by default, uses a lossy conversion of JSON integers to float64s. For sufficiently large integers, this yields a loss of precision, and that causes problems with diffs and plans not matching. See hashicorp/terraform-plugin-sdk#655 for more details. This PR proposes a solution: keeping the lossless json.Number representation of integers in state files. This will ensure that no precision is lost, but it comes at a cost. json.Number is a string type, not a float64 type. That means that existing state upgraders that (correctly) cast a value to float64 will break, as the value will now be surfaced to them as a json.Number, which cannot be cast to a float64. To handle this, the schema.Resource type gains a `UseJSONNumber` property. When set to new, users are opted into the new json.Number values. When left false, the default, users get the existing float64 behavior. If a resource with state upgraders wants to use the new json.Number behavior, it must update all the state upgraders for that resource to use json.Number instead of float64 or users with old state files will panic during upgrades. Note: the backwards compatibility properties of this commit are unknown at this time, and there may be a way for us to avoid using schema.Resource.UseJSONNumber, instead preserving the choice until we get to the point where TypeInt casts the number to an int. Further investigation needed. --- go.mod | 4 +- go.sum | 14 +- helper/resource/json.go | 12 ++ helper/resource/state_shim.go | 26 ++-- helper/resource/testing_new_test.go | 18 +-- helper/schema/field_writer_map_test.go | 9 ++ helper/schema/grpc_provider.go | 14 +- helper/schema/grpc_provider_test.go | 189 ++++++++++++++++++++++++ helper/schema/json.go | 12 ++ helper/schema/provider_go115_test.go | 5 + helper/schema/provider_prego115_test.go | 5 + helper/schema/provider_test.go | 5 +- helper/schema/resource.go | 10 ++ helper/schema/shims.go | 14 +- 14 files changed, 302 insertions(+), 35 deletions(-) create mode 100644 helper/resource/json.go create mode 100644 helper/schema/json.go create mode 100644 helper/schema/provider_go115_test.go create mode 100644 helper/schema/provider_prego115_test.go diff --git a/go.mod b/go.mod index d027499ef20..dc872d47a9b 100644 --- a/go.mod +++ b/go.mod @@ -25,8 +25,8 @@ require ( github.com/hashicorp/go-version v1.2.1 github.com/hashicorp/hcl/v2 v2.3.0 github.com/hashicorp/logutils v1.0.0 - github.com/hashicorp/terraform-exec v0.10.0 - github.com/hashicorp/terraform-json v0.5.0 + github.com/hashicorp/terraform-exec v0.11.1-0.20201216194308-834cb4cce1d3 + github.com/hashicorp/terraform-json v0.7.1-0.20201216193920-93fe74c2941e github.com/hashicorp/terraform-plugin-go v0.1.0 github.com/hashicorp/yamux v0.0.0-20181012175058-2f1d1f20f75d // indirect github.com/keybase/go-crypto v0.0.0-20161004153544-93f5b35093ba diff --git a/go.sum b/go.sum index 0f50b20113a..25b2b6cbaea 100644 --- a/go.sum +++ b/go.sum @@ -42,6 +42,7 @@ github.com/agl/ed25519 v0.0.0-20170116200512-5312a6153412 h1:w1UutsfOrms1J05zt7I github.com/agl/ed25519 v0.0.0-20170116200512-5312a6153412/go.mod h1:WPjqKcmVOxf0XSf3YxCJs6N6AOSrOx3obionmG7T0y0= github.com/alcortesm/tgz v0.0.0-20161220082320-9c5fe88206d7 h1:uSoVVbwJiQipAclBbw+8quDsfcvFjOpI5iCf4p/cqCs= github.com/alcortesm/tgz v0.0.0-20161220082320-9c5fe88206d7/go.mod h1:6zEj6s6u/ghQa61ZWa/C2Aw3RkjiTBOix7dkqa1VLIs= +github.com/andybalholm/crlf v0.0.0-20171020200849-670099aa064f/go.mod h1:k8feO4+kXDxro6ErPXBRTJ/ro2mf0SsFG8s7doP9kJE= github.com/anmitsu/go-shlex v0.0.0-20161002113705-648efa622239 h1:kFOfPq6dUM1hTo4JG6LR5AXSUEsOjtdm0kw0FtQtMJA= github.com/anmitsu/go-shlex v0.0.0-20161002113705-648efa622239/go.mod h1:2FmKhYUyUczH0OGQWaF5ceTx0UBShxjsH6f8oGKYe2c= github.com/apparentlymart/go-cidr v1.0.1 h1:NmIwLZ/KdsjIUlhf+/Np40atNXm/+lZ5txfTJ/SpF+U= @@ -193,10 +194,10 @@ github.com/hashicorp/hcl/v2 v2.3.0 h1:iRly8YaMwTBAKhn1Ybk7VSdzbnopghktCD031P8ggU github.com/hashicorp/hcl/v2 v2.3.0/go.mod h1:d+FwDBbOLvpAM3Z6J7gPj/VoAGkNe/gm352ZhjJ/Zv8= github.com/hashicorp/logutils v1.0.0 h1:dLEQVugN8vlakKOUE3ihGLTZJRB4j+M2cdTm/ORI65Y= github.com/hashicorp/logutils v1.0.0/go.mod h1:QIAnNjmIWmVIIkWDTG1z5v++HQmx9WQRO+LraFDTW64= -github.com/hashicorp/terraform-exec v0.10.0 h1:3nh/1e3u9gYRUQGOKWp/8wPR7ABlL2F14sZMZBrp+dM= -github.com/hashicorp/terraform-exec v0.10.0/go.mod h1:tOT8j1J8rP05bZBGWXfMyU3HkLi1LWyqL3Bzsc3CJjo= -github.com/hashicorp/terraform-json v0.5.0 h1:7TV3/F3y7QVSuN4r9BEXqnWqrAyeOtON8f0wvREtyzs= -github.com/hashicorp/terraform-json v0.5.0/go.mod h1:eAbqb4w0pSlRmdvl8fOyHAi/+8jnkVYN28gJkSJrLhU= +github.com/hashicorp/terraform-exec v0.11.1-0.20201216194308-834cb4cce1d3 h1:WJyc/gjJl+313zgUw2XIKUg0Rpn7MuZEyuHHTHoLp20= +github.com/hashicorp/terraform-exec v0.11.1-0.20201216194308-834cb4cce1d3/go.mod h1:HXWkZcOyyEcYWZzed3S5ZvBVO2AqamHV+31KMV1fjog= +github.com/hashicorp/terraform-json v0.7.1-0.20201216193920-93fe74c2941e h1:eRFeH76HcGTdZ0+Y/tG4BoVUKEg52rXPf9aPDd1DGHU= +github.com/hashicorp/terraform-json v0.7.1-0.20201216193920-93fe74c2941e/go.mod h1:3defM4kkMfttwiE7VakJDwCd4R+umhSQnvJwORXbprE= github.com/hashicorp/terraform-plugin-go v0.1.0 h1:kyXZ0nkHxiRev/q18N40IbRRk4AV0zE/MDJkDM3u8dY= github.com/hashicorp/terraform-plugin-go v0.1.0/go.mod h1:10V6F3taeDWVAoLlkmArKttR3IULlRWFAGtQIQTIDr4= github.com/hashicorp/yamux v0.0.0-20180604194846-3520598351bb/go.mod h1:+NfK9FKeTrX5uv1uIXGdwYDTeHna2qgaIlx54MXqjAM= @@ -280,6 +281,8 @@ github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0 github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJyk= github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= +github.com/stretchr/testify v1.6.1 h1:hDPOHmpOpP40lSULcqw7IrRb/u7w6RpDC9399XyoNd0= +github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/ulikunitz/xz v0.5.5 h1:pFrO0lVpTBXLpYw+pnLj6TbvHuyjXMfjGeCwSqCVwok= github.com/ulikunitz/xz v0.5.5/go.mod h1:2bypXElzHzzJZwzH67Y6wb67pO62Rzfn7BSiF4ABRW8= github.com/ulikunitz/xz v0.5.8 h1:ERv8V6GKqVi23rgu5cj9pVfVzJbOqAY2Ntl88O6c2nQ= @@ -295,6 +298,7 @@ github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9de github.com/zclconf/go-cty v1.2.0/go.mod h1:hOPWgoHbaTUnI5k4D2ld+GRpFJSCe6bCM7m1q/N4PQ8= github.com/zclconf/go-cty v1.2.1 h1:vGMsygfmeCl4Xb6OA5U5XVAaQZ69FvoG7X2jUtQujb8= github.com/zclconf/go-cty v1.2.1/go.mod h1:hOPWgoHbaTUnI5k4D2ld+GRpFJSCe6bCM7m1q/N4PQ8= +github.com/zclconf/go-cty-debug v0.0.0-20191215020915-b22d67c1ba0b/go.mod h1:ZRKQfBXbGkpdV6QMzT3rU1kSTAnfu1dO8dPKjYprgj8= go.opencensus.io v0.21.0/go.mod h1:mSImk1erAIZhrmZN+AvHh14ztQfjbGwt4TtuofqLduU= go.opencensus.io v0.22.0 h1:C9hSCOW830chIVkdja34wa6Ky+IzWllkUinR+BtRZd4= go.opencensus.io v0.22.0/go.mod h1:+kGneAE2xo2IficOXnaByMWTGM9T73dGwxeWcUqIpI8= @@ -565,6 +569,8 @@ gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.4 h1:/eiJrUcujPVeJ3xlSWaiNi3uSVmDGBK1pDHUHAnao1I= gopkg.in/yaml.v2 v2.2.4/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.0-20190106161140-3f1c8253044a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.0-20190418001031-e561f6794a2a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= diff --git a/helper/resource/json.go b/helper/resource/json.go new file mode 100644 index 00000000000..345abf71995 --- /dev/null +++ b/helper/resource/json.go @@ -0,0 +1,12 @@ +package resource + +import ( + "bytes" + "encoding/json" +) + +func unmarshalJSON(data []byte, v interface{}) error { + dec := json.NewDecoder(bytes.NewReader(data)) + dec.UseNumber() + return dec.Decode(v) +} diff --git a/helper/resource/state_shim.go b/helper/resource/state_shim.go index 42c7b64081c..7f8c225291e 100644 --- a/helper/resource/state_shim.go +++ b/helper/resource/state_shim.go @@ -1,6 +1,7 @@ package resource import ( + "encoding/json" "fmt" "strconv" @@ -70,11 +71,11 @@ func shimOutputState(so *tfjson.StateOutput) (*terraform.OutputState, error) { elements[i] = el.(bool) } os.Value = elements - // unmarshalled number from JSON will always be float64 - case float64: + // unmarshalled number from JSON will always be json.Number + case json.Number: elements := make([]interface{}, len(v)) for i, el := range v { - elements[i] = el.(float64) + elements[i] = el.(json.Number) } os.Value = elements case []interface{}: @@ -93,10 +94,10 @@ func shimOutputState(so *tfjson.StateOutput) (*terraform.OutputState, error) { os.Type = "string" os.Value = strconv.FormatBool(v) return os, nil - // unmarshalled number from JSON will always be float64 - case float64: + // unmarshalled number from JSON will always be json.Number + case json.Number: os.Type = "string" - os.Value = strconv.FormatFloat(v, 'f', -1, 64) + os.Value = v.String() return os, nil } @@ -155,8 +156,13 @@ func shimResourceStateKey(res *tfjson.StateResource) (string, error) { var index int switch idx := res.Index.(type) { - case float64: - index = int(idx) + case json.Number: + i, err := idx.Int64() + if err != nil { + return "", fmt.Errorf("unexpected index value (%q) for %q, ", + idx, res.Address) + } + index = int(i) default: return "", fmt.Errorf("unexpected index type (%T) for %q, "+ "for_each is not supported", res.Index, res.Address) @@ -256,8 +262,8 @@ func (sf *shimmedFlatmap) AddEntry(key string, value interface{}) error { return nil case bool: sf.m[key] = strconv.FormatBool(el) - case float64: - sf.m[key] = strconv.FormatFloat(el, 'f', -1, 64) + case json.Number: + sf.m[key] = el.String() case string: sf.m[key] = el case map[string]interface{}: diff --git a/helper/resource/testing_new_test.go b/helper/resource/testing_new_test.go index d12dace5640..97fa2633e39 100644 --- a/helper/resource/testing_new_test.go +++ b/helper/resource/testing_new_test.go @@ -171,18 +171,18 @@ func TestShimState(t *testing.T) { "list_of_int": { Type: "list", Value: []interface{}{ - // TODO: Not sure if this is expectable - // but we have no way of distinguishing between int and float - // due outputs being schema-less and numbers - // always being unmarshaled into float64 - 1.0, 4.0, 9.0, + json.Number("1"), + json.Number("4"), + json.Number("9"), }, Sensitive: false, }, "list_of_float": { Type: "list", Value: []interface{}{ - 1.2, 4.2, 9.8, + json.Number("1.2"), + json.Number("4.2"), + json.Number("9.8"), }, Sensitive: false, }, @@ -280,12 +280,12 @@ func TestShimState(t *testing.T) { Value: []interface{}{ map[string]interface{}{ "allow_bool": true, - "port": float64(443), + "port": json.Number("443"), "rule": "allow", }, map[string]interface{}{ "allow_bool": false, - "port": float64(80), + "port": json.Number("80"), "rule": "deny", }, }, @@ -1087,7 +1087,7 @@ func TestShimState(t *testing.T) { t.Run(fmt.Sprintf("%d-%s", i, tc.Name), func(t *testing.T) { var rawState tfjson.State - err := json.Unmarshal([]byte(tc.RawState), &rawState) + err := unmarshalJSON([]byte(tc.RawState), &rawState) if err != nil { t.Fatal(err) } diff --git a/helper/schema/field_writer_map_test.go b/helper/schema/field_writer_map_test.go index 4480b41fc1b..5692e9cf5e5 100644 --- a/helper/schema/field_writer_map_test.go +++ b/helper/schema/field_writer_map_test.go @@ -88,6 +88,15 @@ func TestMapFieldWriter(t *testing.T) { }, }, + "bigint": { + []string{"int"}, + 7227701560655103598, + false, + map[string]string{ + "int": "7227701560655103598", + }, + }, + "string": { []string{"string"}, "42", diff --git a/helper/schema/grpc_provider.go b/helper/schema/grpc_provider.go index 354c94e0716..0082d29cf0d 100644 --- a/helper/schema/grpc_provider.go +++ b/helper/schema/grpc_provider.go @@ -282,7 +282,11 @@ func (s *GRPCProviderServer) UpgradeResourceState(ctx context.Context, req *tfpr } // if there's a JSON state, we need to decode it. case len(req.RawState.JSON) > 0: - err = json.Unmarshal(req.RawState.JSON, &jsonMap) + if res.UseJSONNumber { + err = unmarshalJSON(req.RawState.JSON, &jsonMap) + } else { + err = json.Unmarshal(req.RawState.JSON, &jsonMap) + } if err != nil { resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) return resp, nil @@ -405,7 +409,7 @@ func (s *GRPCProviderServer) upgradeFlatmapState(ctx context.Context, version in return nil, 0, err } - jsonMap, err := StateValueToJSONMap(newConfigVal, schemaType) + jsonMap, err := stateValueToJSONMap(newConfigVal, schemaType, res.UseJSONNumber) return jsonMap, upgradedVersion, err } @@ -551,7 +555,7 @@ func (s *GRPCProviderServer) ReadResource(ctx context.Context, req *tfprotov5.Re private := make(map[string]interface{}) if len(req.Private) > 0 { - if err := json.Unmarshal(req.Private, &private); err != nil { + if err := unmarshalJSON(req.Private, &private); err != nil { resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) return resp, nil } @@ -659,7 +663,7 @@ func (s *GRPCProviderServer) PlanResourceChange(ctx context.Context, req *tfprot } priorPrivate := make(map[string]interface{}) if len(req.PriorPrivate) > 0 { - if err := json.Unmarshal(req.PriorPrivate, &priorPrivate); err != nil { + if err := unmarshalJSON(req.PriorPrivate, &priorPrivate); err != nil { resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) return resp, nil } @@ -873,7 +877,7 @@ func (s *GRPCProviderServer) ApplyResourceChange(ctx context.Context, req *tfpro private := make(map[string]interface{}) if len(req.PlannedPrivate) > 0 { - if err := json.Unmarshal(req.PlannedPrivate, &private); err != nil { + if err := unmarshalJSON(req.PlannedPrivate, &private); err != nil { resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) return resp, nil } diff --git a/helper/schema/grpc_provider_test.go b/helper/schema/grpc_provider_test.go index cdc000e9560..ec12f4f7a20 100644 --- a/helper/schema/grpc_provider_test.go +++ b/helper/schema/grpc_provider_test.go @@ -3,6 +3,7 @@ package schema import ( "context" "fmt" + "math/big" "strconv" "strings" "testing" @@ -108,6 +109,59 @@ func TestUpgradeState_jsonState(t *testing.T) { } } +func TestUpgradeState_jsonStateBigInt(t *testing.T) { + r := &Resource{ + UseJSONNumber: true, + SchemaVersion: 2, + Schema: map[string]*Schema{ + "int": { + Type: TypeInt, + Required: true, + }, + }, + } + + server := NewGRPCProviderServer(&Provider{ + ResourcesMap: map[string]*Resource{ + "test": r, + }, + }) + + req := &tfprotov5.UpgradeResourceStateRequest{ + TypeName: "test", + Version: 0, + RawState: &tfprotov5.RawState{ + JSON: []byte(`{"id":"bar","int":7227701560655103598}`), + }, + } + + 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"), + "int": cty.NumberIntVal(7227701560655103598), + }) + + if !cmp.Equal(expected, val, valueComparer, equateEmpty) { + t.Fatal(cmp.Diff(expected, val, valueComparer, equateEmpty)) + } +} + func TestUpgradeState_removedAttr(t *testing.T) { r1 := &Resource{ Schema: map[string]*Schema{ @@ -577,6 +631,71 @@ func TestPlanResourceChange(t *testing.T) { } } +func TestPlanResourceChange_bigint(t *testing.T) { + r := &Resource{ + UseJSONNumber: true, + Schema: map[string]*Schema{ + "foo": { + Type: TypeInt, + Required: true, + }, + }, + } + + server := NewGRPCProviderServer(&Provider{ + ResourcesMap: map[string]*Resource{ + "test": r, + }, + }) + + schema := r.CoreConfigSchema() + priorState, err := msgpack.Marshal(cty.NullVal(schema.ImpliedType()), schema.ImpliedType()) + if err != nil { + t.Fatal(err) + } + + proposedVal := cty.ObjectVal(map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + "foo": cty.MustParseNumberVal("7227701560655103598"), + }) + proposedState, err := msgpack.Marshal(proposedVal, schema.ImpliedType()) + if err != nil { + t.Fatal(err) + } + + testReq := &tfprotov5.PlanResourceChangeRequest{ + TypeName: "test", + PriorState: &tfprotov5.DynamicValue{ + MsgPack: priorState, + }, + ProposedNewState: &tfprotov5.DynamicValue{ + MsgPack: proposedState, + }, + } + + resp, err := server.PlanResourceChange(context.Background(), testReq) + if err != nil { + t.Fatal(err) + } + + plannedStateVal, err := msgpack.Unmarshal(resp.PlannedState.MsgPack, schema.ImpliedType()) + if err != nil { + t.Fatal(err) + } + + if !cmp.Equal(proposedVal, plannedStateVal, valueComparer) { + t.Fatal(cmp.Diff(proposedVal, plannedStateVal, valueComparer)) + } + + plannedStateFoo, acc := plannedStateVal.GetAttr("foo").AsBigFloat().Int64() + if acc != big.Exact { + t.Fatalf("Expected exact accuracy, got %s", acc) + } + if plannedStateFoo != 7227701560655103598 { + t.Fatalf("Expected %d, got %d, this represents a loss of precision in planning large numbers", 7227701560655103598, plannedStateFoo) + } +} + func TestApplyResourceChange(t *testing.T) { r := &Resource{ SchemaVersion: 4, @@ -643,6 +762,76 @@ func TestApplyResourceChange(t *testing.T) { } } +func TestApplyResourceChange_bigint(t *testing.T) { + r := &Resource{ + UseJSONNumber: true, + Schema: map[string]*Schema{ + "foo": { + Type: TypeInt, + Required: true, + }, + }, + CreateContext: func(_ context.Context, rd *ResourceData, _ interface{}) diag.Diagnostics { + rd.SetId("bar") + return nil + }, + } + + server := NewGRPCProviderServer(&Provider{ + ResourcesMap: map[string]*Resource{ + "test": r, + }, + }) + + schema := r.CoreConfigSchema() + priorState, err := msgpack.Marshal(cty.NullVal(schema.ImpliedType()), schema.ImpliedType()) + if err != nil { + t.Fatal(err) + } + + plannedVal := cty.ObjectVal(map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + "foo": cty.MustParseNumberVal("7227701560655103598"), + }) + plannedState, err := msgpack.Marshal(plannedVal, schema.ImpliedType()) + if err != nil { + t.Fatal(err) + } + + testReq := &tfprotov5.ApplyResourceChangeRequest{ + TypeName: "test", + PriorState: &tfprotov5.DynamicValue{ + MsgPack: priorState, + }, + PlannedState: &tfprotov5.DynamicValue{ + MsgPack: plannedState, + }, + } + + resp, err := server.ApplyResourceChange(context.Background(), testReq) + if err != nil { + t.Fatal(err) + } + + newStateVal, err := msgpack.Unmarshal(resp.NewState.MsgPack, schema.ImpliedType()) + if err != nil { + t.Fatal(err) + } + + id := newStateVal.GetAttr("id").AsString() + if id != "bar" { + t.Fatalf("incorrect final state: %#v\n", newStateVal) + } + + foo, acc := newStateVal.GetAttr("foo").AsBigFloat().Int64() + if acc != big.Exact { + t.Fatalf("Expected exact accuracy, got %s", acc) + } + if foo != 7227701560655103598 { + t.Fatalf("Expected %d, got %d, this represents a loss of precision in applying large numbers", 7227701560655103598, foo) + } +} + func TestPrepareProviderConfig(t *testing.T) { for _, tc := range []struct { Name string diff --git a/helper/schema/json.go b/helper/schema/json.go new file mode 100644 index 00000000000..265099a6b6f --- /dev/null +++ b/helper/schema/json.go @@ -0,0 +1,12 @@ +package schema + +import ( + "bytes" + "encoding/json" +) + +func unmarshalJSON(data []byte, v interface{}) error { + dec := json.NewDecoder(bytes.NewReader(data)) + dec.UseNumber() + return dec.Decode(v) +} diff --git a/helper/schema/provider_go115_test.go b/helper/schema/provider_go115_test.go new file mode 100644 index 00000000000..9f9a08634ea --- /dev/null +++ b/helper/schema/provider_go115_test.go @@ -0,0 +1,5 @@ +// +build go1.15 + +package schema + +const invalidDurationErrMsg = "time: invalid duration \"invalid\"" diff --git a/helper/schema/provider_prego115_test.go b/helper/schema/provider_prego115_test.go new file mode 100644 index 00000000000..50d9d3d0648 --- /dev/null +++ b/helper/schema/provider_prego115_test.go @@ -0,0 +1,5 @@ +// +build !go1.15 + +package schema + +const invalidDurationErrMsg = "time: invalid duration invalid" diff --git a/helper/schema/provider_test.go b/helper/schema/provider_test.go index 090d66cc9d8..3279ee2f9bf 100644 --- a/helper/schema/provider_test.go +++ b/helper/schema/provider_test.go @@ -566,11 +566,10 @@ func TestProviderDiff_timeoutInvalidValue(t *testing.T) { if err == nil { t.Fatal("Expected provider.Diff to fail with invalid timeout value") } - expectedErrMsg := "time: invalid duration invalid" - if !strings.Contains(err.Error(), expectedErrMsg) { + if !strings.Contains(err.Error(), invalidDurationErrMsg) { t.Fatalf("Unexpected error message: %q\nExpected message to contain %q", err.Error(), - expectedErrMsg) + invalidDurationErrMsg) } } diff --git a/helper/schema/resource.go b/helper/schema/resource.go index f41964a5890..2f5663d357e 100644 --- a/helper/schema/resource.go +++ b/helper/schema/resource.go @@ -191,6 +191,16 @@ type Resource struct { // other user facing usage. It can be plain-text or markdown depending on the // global DescriptionKind setting. Description string + + // UseJSONNumber should be set when state upgraders will expect + // json.Numbers instead of float64s for numbers. This is added as a + // toggle for backwards compatibility for type assertions, but should + // be used in all new resources to avoid bugs with sufficiently large + // user input. + // + // See github.com/hashicorp/terraform-plugin-sdk/issues/655 for more + // details. + UseJSONNumber bool } // ShimInstanceStateFromValue converts a cty.Value to a diff --git a/helper/schema/shims.go b/helper/schema/shims.go index 40a5abc0db8..e1575a0f891 100644 --- a/helper/schema/shims.go +++ b/helper/schema/shims.go @@ -77,14 +77,24 @@ func ApplyDiff(base cty.Value, d *terraform.InstanceDiff, schema *configschema.B // StateValueToJSONMap converts a cty.Value to generic JSON map via the cty JSON // encoding. func StateValueToJSONMap(val cty.Value, ty cty.Type) (map[string]interface{}, error) { + return stateValueToJSONMap(val, ty, false) +} + +func stateValueToJSONMap(val cty.Value, ty cty.Type, useJSONNumber bool) (map[string]interface{}, error) { js, err := ctyjson.Marshal(val, ty) if err != nil { return nil, err } var m map[string]interface{} - if err := json.Unmarshal(js, &m); err != nil { - return nil, err + if useJSONNumber { + if err := unmarshalJSON(js, &m); err != nil { + return nil, err + } + } else { + if err := json.Unmarshal(js, &m); err != nil { + return nil, err + } } return m, nil