Merge pull request #16964 from liggitt/json_precision

Automatic merge from submit-queue

Preserve int data when unmarshaling

There are several places we use `json.Unmarshal` into an unstructured map (StrategicMergePatch, UnstructuredJSONScheme, many others).

In this scenario, the json package converts all numbers to float64. This exposes many of the int64 fields in our API types to corruption when the unstructured map is marshalled back to json.

A simple example is a pod with an `"activeDeadlineSeconds": 1000000`. Trying to use `kubectl label`, `annotate`, `patch`, etc results in that int64 being converted to a float64, submitted to the server, and the server rejecting it with an error about "cannot unmarshal number 1e+6 into Go value of type int64"

The json package provides a way to defer conversion of numbers (`json.Decoder#UseNumber`), but does not actually do conversions to int or float. This PR makes use of that feature, and post-processes the unmarshalled map to convert json.Number objects to either int64 or float64 values
This commit is contained in:
k8s-merge-robot 2016-04-12 07:09:32 -07:00
commit fcddb9cba5
8 changed files with 626 additions and 4 deletions

View File

@ -474,6 +474,30 @@ runTests() {
# Post-condition: valid-pod POD doesn't exist
kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" ''
### Create pod-with-precision POD
# Pre-condition: no POD is running
kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" ''
# Command
kubectl create -f hack/testdata/pod-with-precision.json "${kube_flags[@]}"
# Post-condition: valid-pod POD is running
kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" 'pod-with-precision:'
## Patch preserves precision
# Command
kubectl patch "${kube_flags[@]}" pod pod-with-precision -p='{"metadata":{"annotations":{"patchkey": "patchvalue"}}}'
# Post-condition: pod-with-precision POD has patched annotation
kube::test::get_object_assert 'pod pod-with-precision' "{{${annotations_field}.patchkey}}" 'patchvalue'
# Command
kubectl label pods pod-with-precision labelkey=labelvalue "${kube_flags[@]}"
# Post-condition: pod-with-precision POD has label
kube::test::get_object_assert 'pod pod-with-precision' "{{${labels_field}.labelkey}}" 'labelvalue'
# Command
kubectl annotate pods pod-with-precision annotatekey=annotatevalue "${kube_flags[@]}"
# Post-condition: pod-with-precision POD has annotation
kube::test::get_object_assert 'pod pod-with-precision' "{{${annotations_field}.annotatekey}}" 'annotatevalue'
# Cleanup
kubectl delete pod pod-with-precision "${kube_flags[@]}"
### Create valid-pod POD
# Pre-condition: no POD exists
create_and_use_new_namespace

24
hack/testdata/pod-with-precision.json vendored Normal file
View File

@ -0,0 +1,24 @@
{
"apiVersion": "v1",
"kind": "Pod",
"metadata": {
"name": "pod-with-precision"
},
"spec": {
"activeDeadlineSeconds": 2147483647,
"containers": [
{
"name": "kubernetes-pause",
"image": "kubernetes/pause"
}
],
"restartPolicy": "Never",
"securityContext": {
"supplementalGroups": [
0,
1000030003,
2147483647
]
}
}
}

View File

@ -17,10 +17,11 @@ limitations under the License.
package runtime
import (
"encoding/json"
gojson "encoding/json"
"io"
"k8s.io/kubernetes/pkg/api/unversioned"
"k8s.io/kubernetes/pkg/util/json"
)
// UnstructuredJSONScheme is capable of converting JSON data into the Unstructured
@ -77,7 +78,7 @@ func (unstructuredJSONScheme) EncodeToStream(obj Object, w io.Writer, overrides
func (s unstructuredJSONScheme) decode(data []byte) (Object, error) {
type detector struct {
Items json.RawMessage
Items gojson.RawMessage
}
var det detector
if err := json.Unmarshal(data, &det); err != nil {
@ -139,7 +140,7 @@ func (unstructuredJSONScheme) decodeToUnstructured(data []byte, unstruct *Unstru
func (s unstructuredJSONScheme) decodeToList(data []byte, list *UnstructuredList) error {
type decodeList struct {
TypeMeta `json:",inline"`
Items []json.RawMessage
Items []gojson.RawMessage
}
var dList decodeList

View File

@ -19,10 +19,12 @@ package runtime_test
import (
"fmt"
"reflect"
"strings"
"testing"
"k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/api/testapi"
"k8s.io/kubernetes/pkg/api/validation"
"k8s.io/kubernetes/pkg/runtime"
)
@ -129,3 +131,66 @@ func TestDecode(t *testing.T) {
}
}
}
func TestDecodeNumbers(t *testing.T) {
// Start with a valid pod
originalJSON := []byte(`{
"kind":"Pod",
"apiVersion":"v1",
"metadata":{"name":"pod","namespace":"foo"},
"spec":{
"containers":[{"name":"container","image":"container"}],
"activeDeadlineSeconds":1000030003
}
}`)
pod := &api.Pod{}
// Decode with structured codec
codec, err := testapi.GetCodecForObject(pod)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
err = runtime.DecodeInto(codec, originalJSON, pod)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
// ensure pod is valid
if errs := validation.ValidatePod(pod); len(errs) > 0 {
t.Fatalf("pod should be valid: %v", errs)
}
// Round-trip with unstructured codec
unstructuredObj, err := runtime.Decode(runtime.UnstructuredJSONScheme, originalJSON)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
roundtripJSON, err := runtime.Encode(runtime.UnstructuredJSONScheme, unstructuredObj)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
// Make sure we serialize back out in int form
if !strings.Contains(string(roundtripJSON), `"activeDeadlineSeconds":1000030003`) {
t.Errorf("Expected %s, got %s", `"activeDeadlineSeconds":1000030003`, string(roundtripJSON))
}
// Decode with structured codec again
obj2, err := runtime.Decode(codec, roundtripJSON)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
// ensure pod is still valid
pod2, ok := obj2.(*api.Pod)
if !ok {
t.Fatalf("expected an *api.Pod, got %#v", obj2)
}
if errs := validation.ValidatePod(pod2); len(errs) > 0 {
t.Fatalf("pod should be valid: %v", errs)
}
// ensure round-trip preserved large integers
if !reflect.DeepEqual(pod, pod2) {
t.Fatalf("Expected\n\t%#v, got \n\t%#v", pod, pod2)
}
}

107
pkg/util/json/json.go Normal file
View File

@ -0,0 +1,107 @@
/*
Copyright 2015 The Kubernetes Authors All rights reserved.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package json
import (
"bytes"
"encoding/json"
"io"
)
// NewEncoder delegates to json.NewEncoder
// It is only here so this package can be a drop-in for common encoding/json uses
func NewEncoder(w io.Writer) *json.Encoder {
return json.NewEncoder(w)
}
// Marshal delegates to json.Marshal
// It is only here so this package can be a drop-in for common encoding/json uses
func Marshal(v interface{}) ([]byte, error) {
return json.Marshal(v)
}
// Unmarshal unmarshals the given data
// If v is a *map[string]interface{}, numbers are converted to int64 or float64
func Unmarshal(data []byte, v interface{}) error {
switch v := v.(type) {
case *map[string]interface{}:
// Build a decoder from the given data
decoder := json.NewDecoder(bytes.NewBuffer(data))
// Preserve numbers, rather than casting to float64 automatically
decoder.UseNumber()
// Run the decode
if err := decoder.Decode(v); err != nil {
return err
}
// If the decode succeeds, post-process the map to convert json.Number objects to int64 or float64
return convertMapNumbers(*v)
default:
return json.Unmarshal(data, v)
}
}
// convertMapNumbers traverses the map, converting any json.Number values to int64 or float64.
// values which are map[string]interface{} or []interface{} are recursively visited
func convertMapNumbers(m map[string]interface{}) error {
var err error
for k, v := range m {
switch v := v.(type) {
case json.Number:
m[k], err = convertNumber(v)
case map[string]interface{}:
err = convertMapNumbers(v)
case []interface{}:
err = convertSliceNumbers(v)
}
if err != nil {
return err
}
}
return nil
}
// convertSliceNumbers traverses the slice, converting any json.Number values to int64 or float64.
// values which are map[string]interface{} or []interface{} are recursively visited
func convertSliceNumbers(s []interface{}) error {
var err error
for i, v := range s {
switch v := v.(type) {
case json.Number:
s[i], err = convertNumber(v)
case map[string]interface{}:
err = convertMapNumbers(v)
case []interface{}:
err = convertSliceNumbers(v)
}
if err != nil {
return err
}
}
return nil
}
// convertNumber converts a json.Number to an int64 or float64, or returns an error
func convertNumber(n json.Number) (interface{}, error) {
// Attempt to convert to an int64 first
if i, err := n.Int64(); err == nil {
return i, nil
}
// Return a float64 (default json.Decode() behavior)
// An overflow will return an error
return n.Float64()
}

317
pkg/util/json/json_test.go Normal file
View File

@ -0,0 +1,317 @@
/*
Copyright 2015 The Kubernetes Authors All rights reserved.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package json
import (
"fmt"
"math"
"reflect"
"strconv"
"strings"
"testing"
)
func TestEvaluateTypes(t *testing.T) {
testCases := []struct {
In string
Data interface{}
Out string
Err bool
}{
// Invalid syntaxes
{
In: `x`,
Err: true,
},
{
In: ``,
Err: true,
},
// Null
{
In: `null`,
Data: nil,
Out: `null`,
},
// Booleans
{
In: `true`,
Data: true,
Out: `true`,
},
{
In: `false`,
Data: false,
Out: `false`,
},
// Integers
{
In: `0`,
Data: int64(0),
Out: `0`,
},
{
In: `-0`,
Data: int64(-0),
Out: `0`,
},
{
In: `1`,
Data: int64(1),
Out: `1`,
},
{
In: `2147483647`,
Data: int64(math.MaxInt32),
Out: `2147483647`,
},
{
In: `-2147483648`,
Data: int64(math.MinInt32),
Out: `-2147483648`,
},
{
In: `9223372036854775807`,
Data: int64(math.MaxInt64),
Out: `9223372036854775807`,
},
{
In: `-9223372036854775808`,
Data: int64(math.MinInt64),
Out: `-9223372036854775808`,
},
// Int overflow
{
In: `9223372036854775808`, // MaxInt64 + 1
Data: float64(9223372036854775808),
Out: strconv.FormatFloat(9223372036854775808, 'g', -1, 64),
},
{
In: `-9223372036854775809`, // MinInt64 - 1
Data: float64(math.MinInt64),
Out: strconv.FormatFloat(-9223372036854775809, 'g', -1, 64),
},
// Floats
{
In: `0.0`,
Data: float64(0),
Out: `0`,
},
{
In: `-0.0`,
Data: float64(-0.0),
Out: `-0`,
},
{
In: `0.5`,
Data: float64(0.5),
Out: `0.5`,
},
{
In: `1e3`,
Data: float64(1e3),
Out: `1000`,
},
{
In: `1.5`,
Data: float64(1.5),
Out: `1.5`,
},
{
In: `-0.3`,
Data: float64(-.3),
Out: `-0.3`,
},
{
// Largest representable float32
In: `3.40282346638528859811704183484516925440e+38`,
Data: float64(math.MaxFloat32),
Out: strconv.FormatFloat(math.MaxFloat32, 'g', -1, 64),
},
{
// Smallest float32 without losing precision
In: `1.175494351e-38`,
Data: float64(1.175494351e-38),
Out: `1.175494351e-38`,
},
{
// float32 closest to zero
In: `1.401298464324817070923729583289916131280e-45`,
Data: float64(math.SmallestNonzeroFloat32),
Out: strconv.FormatFloat(math.SmallestNonzeroFloat32, 'g', -1, 64),
},
{
// Largest representable float64
In: `1.797693134862315708145274237317043567981e+308`,
Data: float64(math.MaxFloat64),
Out: strconv.FormatFloat(math.MaxFloat64, 'g', -1, 64),
},
{
// Closest to zero without losing precision
In: `2.2250738585072014e-308`,
Data: float64(2.2250738585072014e-308),
Out: `2.2250738585072014e-308`,
},
{
// float64 closest to zero
In: `4.940656458412465441765687928682213723651e-324`,
Data: float64(math.SmallestNonzeroFloat64),
Out: strconv.FormatFloat(math.SmallestNonzeroFloat64, 'g', -1, 64),
},
{
// math.MaxFloat64 + 2 overflow
In: `1.7976931348623159e+308`,
Err: true,
},
// Strings
{
In: `""`,
Data: string(""),
Out: `""`,
},
{
In: `"0"`,
Data: string("0"),
Out: `"0"`,
},
{
In: `"A"`,
Data: string("A"),
Out: `"A"`,
},
{
In: `"Iñtërnâtiônàlizætiøn"`,
Data: string("Iñtërnâtiônàlizætiøn"),
Out: `"Iñtërnâtiônàlizætiøn"`,
},
// Arrays
{
In: `[]`,
Data: []interface{}{},
Out: `[]`,
},
{
In: `[` + strings.Join([]string{
`null`,
`true`,
`false`,
`0`,
`9223372036854775807`,
`0.0`,
`0.5`,
`1.0`,
`1.797693134862315708145274237317043567981e+308`,
`"0"`,
`"A"`,
`"Iñtërnâtiônàlizætiøn"`,
`[null,true,1,1.0,1.5]`,
`{"boolkey":true,"floatkey":1.0,"intkey":1,"nullkey":null}`,
}, ",") + `]`,
Data: []interface{}{
nil,
true,
false,
int64(0),
int64(math.MaxInt64),
float64(0.0),
float64(0.5),
float64(1.0),
float64(math.MaxFloat64),
string("0"),
string("A"),
string("Iñtërnâtiônàlizætiøn"),
[]interface{}{nil, true, int64(1), float64(1.0), float64(1.5)},
map[string]interface{}{"nullkey": nil, "boolkey": true, "intkey": int64(1), "floatkey": float64(1.0)},
},
Out: `[` + strings.Join([]string{
`null`,
`true`,
`false`,
`0`,
`9223372036854775807`,
`0`,
`0.5`,
`1`,
strconv.FormatFloat(math.MaxFloat64, 'g', -1, 64),
`"0"`,
`"A"`,
`"Iñtërnâtiônàlizætiøn"`,
`[null,true,1,1,1.5]`,
`{"boolkey":true,"floatkey":1,"intkey":1,"nullkey":null}`, // gets alphabetized by Marshal
}, ",") + `]`,
},
// Maps
{
In: `{}`,
Data: map[string]interface{}{},
Out: `{}`,
},
{
In: `{"boolkey":true,"floatkey":1.0,"intkey":1,"nullkey":null}`,
Data: map[string]interface{}{"nullkey": nil, "boolkey": true, "intkey": int64(1), "floatkey": float64(1.0)},
Out: `{"boolkey":true,"floatkey":1,"intkey":1,"nullkey":null}`, // gets alphabetized by Marshal
},
}
for _, tc := range testCases {
inputJSON := fmt.Sprintf(`{"data":%s}`, tc.In)
expectedJSON := fmt.Sprintf(`{"data":%s}`, tc.Out)
m := map[string]interface{}{}
err := Unmarshal([]byte(inputJSON), &m)
if tc.Err && err != nil {
// Expected error
continue
}
if err != nil {
t.Errorf("%s: error decoding: %v", tc.In, err)
continue
}
if tc.Err {
t.Errorf("%s: expected error, got none", tc.In)
continue
}
data, ok := m["data"]
if !ok {
t.Errorf("%s: decoded object missing data key: %#v", tc.In, m)
continue
}
if !reflect.DeepEqual(tc.Data, data) {
t.Errorf("%s: expected\n\t%#v (%v), got\n\t%#v (%v)", tc.In, tc.Data, reflect.TypeOf(tc.Data), data, reflect.TypeOf(data))
continue
}
outputJSON, err := Marshal(m)
if err != nil {
t.Errorf("%s: error encoding: %v", tc.In, err)
continue
}
if expectedJSON != string(outputJSON) {
t.Errorf("%s: expected\n\t%s, got\n\t%s", tc.In, expectedJSON, string(outputJSON))
continue
}
}
}

View File

@ -17,11 +17,11 @@ limitations under the License.
package strategicpatch
import (
"encoding/json"
"fmt"
"reflect"
"sort"
"k8s.io/kubernetes/pkg/util/json"
forkedjson "k8s.io/kubernetes/third_party/forked/json"
"github.com/davecgh/go-spew/spew"

View File

@ -2024,3 +2024,87 @@ func TestHasConflicts(t *testing.T) {
}
}
}
type PrecisionItem struct {
Name string
Int32 int32
Int64 int64
Float32 float32
Float64 float64
}
var precisionItem PrecisionItem
func TestNumberConversion(t *testing.T) {
testcases := map[string]struct {
Old string
New string
ExpectedPatch string
ExpectedResult string
}{
"empty": {
Old: `{}`,
New: `{}`,
ExpectedPatch: `{}`,
ExpectedResult: `{}`,
},
"int32 medium": {
Old: `{"int32":1000000}`,
New: `{"int32":1000000,"name":"newname"}`,
ExpectedPatch: `{"name":"newname"}`,
ExpectedResult: `{"int32":1000000,"name":"newname"}`,
},
"int32 max": {
Old: `{"int32":2147483647}`,
New: `{"int32":2147483647,"name":"newname"}`,
ExpectedPatch: `{"name":"newname"}`,
ExpectedResult: `{"int32":2147483647,"name":"newname"}`,
},
"int64 medium": {
Old: `{"int64":1000000}`,
New: `{"int64":1000000,"name":"newname"}`,
ExpectedPatch: `{"name":"newname"}`,
ExpectedResult: `{"int64":1000000,"name":"newname"}`,
},
"int64 max": {
Old: `{"int64":9223372036854775807}`,
New: `{"int64":9223372036854775807,"name":"newname"}`,
ExpectedPatch: `{"name":"newname"}`,
ExpectedResult: `{"int64":9223372036854775807,"name":"newname"}`,
},
"float32 max": {
Old: `{"float32":3.4028234663852886e+38}`,
New: `{"float32":3.4028234663852886e+38,"name":"newname"}`,
ExpectedPatch: `{"name":"newname"}`,
ExpectedResult: `{"float32":3.4028234663852886e+38,"name":"newname"}`,
},
"float64 max": {
Old: `{"float64":1.7976931348623157e+308}`,
New: `{"float64":1.7976931348623157e+308,"name":"newname"}`,
ExpectedPatch: `{"name":"newname"}`,
ExpectedResult: `{"float64":1.7976931348623157e+308,"name":"newname"}`,
},
}
for k, tc := range testcases {
patch, err := CreateTwoWayMergePatch([]byte(tc.Old), []byte(tc.New), precisionItem)
if err != nil {
t.Errorf("%s: unexpected error %v", k, err)
continue
}
if tc.ExpectedPatch != string(patch) {
t.Errorf("%s: expected %s, got %s", k, tc.ExpectedPatch, string(patch))
continue
}
result, err := StrategicMergePatch([]byte(tc.Old), patch, precisionItem)
if err != nil {
t.Errorf("%s: unexpected error %v", k, err)
continue
}
if tc.ExpectedResult != string(result) {
t.Errorf("%s: expected %s, got %s", k, tc.ExpectedResult, string(result))
continue
}
}
}