Skip to content

Commit

Permalink
Allow opting into using json.Number in state.
Browse files Browse the repository at this point in the history
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 #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.
  • Loading branch information
paddycarver committed Dec 18, 2020
1 parent d56de1c commit 272df8b
Show file tree
Hide file tree
Showing 14 changed files with 302 additions and 35 deletions.
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 10 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down Expand Up @@ -199,10 +200,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=
Expand Down Expand Up @@ -291,6 +292,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=
Expand All @@ -306,6 +309,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=
Expand Down Expand Up @@ -578,6 +582,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=
Expand Down
12 changes: 12 additions & 0 deletions helper/resource/json.go
Original file line number Diff line number Diff line change
@@ -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)
}
26 changes: 16 additions & 10 deletions helper/resource/state_shim.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package resource

import (
"encoding/json"
"fmt"
"strconv"

Expand Down Expand Up @@ -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{}:
Expand All @@ -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
}

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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{}:
Expand Down
18 changes: 9 additions & 9 deletions helper/resource/testing_new_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down Expand Up @@ -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",
},
},
Expand Down Expand Up @@ -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)
}
Expand Down
9 changes: 9 additions & 0 deletions helper/schema/field_writer_map_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,15 @@ func TestMapFieldWriter(t *testing.T) {
},
},

"bigint": {
[]string{"int"},
7227701560655103598,
false,
map[string]string{
"int": "7227701560655103598",
},
},

"string": {
[]string{"string"},
"42",
Expand Down
14 changes: 9 additions & 5 deletions helper/schema/grpc_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down
Loading

0 comments on commit 272df8b

Please sign in to comment.