Skip to content

Commit aeef92b

Browse files
committed
Preserve int/float distinction when decoding raw values
1 parent d911254 commit aeef92b

File tree

4 files changed

+190
-36
lines changed

4 files changed

+190
-36
lines changed

staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/goopenapi_test.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,9 @@ func TestStructuralRoundtrip(t *testing.T) {
3838
f.RandSource(rand.New(rand.NewSource(seed)))
3939
f.Funcs(
4040
func(s *JSON, c fuzz.Continue) {
41-
switch c.Intn(6) {
41+
switch c.Intn(7) {
4242
case 0:
43-
s.Object = float64(42.0)
43+
s.Object = float64(42.2)
4444
case 1:
4545
s.Object = map[string]interface{}{"foo": "bar"}
4646
case 2:
@@ -51,6 +51,8 @@ func TestStructuralRoundtrip(t *testing.T) {
5151
s.Object = map[string]interface{}{}
5252
case 5:
5353
s.Object = nil
54+
case 6:
55+
s.Object = int64(42)
5456
}
5557
},
5658
)

staging/src/k8s.io/apiextensions-apiserver/test/integration/defaulting_test.go

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,10 @@ var defaultingFixture = &apiextensionsv1.CustomResourceDefinition{
5353
Served: true,
5454
Subresources: &apiextensionsv1.CustomResourceSubresources{
5555
Status: &apiextensionsv1.CustomResourceSubresourceStatus{},
56+
Scale: &apiextensionsv1.CustomResourceSubresourceScale{
57+
SpecReplicasPath: ".spec.replicas",
58+
StatusReplicasPath: ".status.replicas",
59+
},
5660
},
5761
},
5862
{
@@ -61,6 +65,10 @@ var defaultingFixture = &apiextensionsv1.CustomResourceDefinition{
6165
Served: false,
6266
Subresources: &apiextensionsv1.CustomResourceSubresources{
6367
Status: &apiextensionsv1.CustomResourceSubresourceStatus{},
68+
Scale: &apiextensionsv1.CustomResourceSubresourceScale{
69+
SpecReplicasPath: ".spec.replicas",
70+
StatusReplicasPath: ".status.replicas",
71+
},
6472
},
6573
},
6674
},
@@ -94,6 +102,11 @@ properties:
94102
default: "v1beta1"
95103
v1beta2:
96104
type: string
105+
replicas:
106+
default: 1
107+
format: int32
108+
minimum: 0
109+
type: integer
97110
status:
98111
type: object
99112
properties:
@@ -110,6 +123,11 @@ properties:
110123
default: "v1beta1"
111124
v1beta2:
112125
type: string
126+
replicas:
127+
default: 0
128+
format: int32
129+
minimum: 0
130+
type: integer
113131
`
114132

115133
const defaultingFooV1beta2Schema = `
@@ -131,6 +149,11 @@ properties:
131149
v1beta2:
132150
type: string
133151
default: "v1beta2"
152+
replicas:
153+
default: 1
154+
format: int32
155+
minimum: 0
156+
type: integer
134157
status:
135158
type: object
136159
properties:
@@ -147,6 +170,11 @@ properties:
147170
v1beta2:
148171
type: string
149172
default: "v1beta2"
173+
replicas:
174+
default: 0
175+
format: int32
176+
minimum: 0
177+
type: integer
150178
`
151179

152180
const defaultingFooInstance = `
@@ -274,15 +302,15 @@ func testDefaulting(t *testing.T, watchCache bool) {
274302
// spec.a and spec.b are defaulted in both versions
275303
// spec.v1beta1 is defaulted when reading the incoming request
276304
// spec.v1beta2 is defaulted when reading the storage response
277-
mustExist(foo.Object, [][]string{{"spec", "a"}, {"spec", "b"}, {"spec", "v1beta1"}, {"spec", "v1beta2"}})
305+
mustExist(foo.Object, [][]string{{"spec", "a"}, {"spec", "b"}, {"spec", "v1beta1"}, {"spec", "v1beta2"}, {"spec", "replicas"}})
278306
mustNotExist(foo.Object, [][]string{{"status"}})
279307

280308
t.Logf("Updating status and expecting 'a' and 'b' to show up.")
281309
unstructured.SetNestedField(foo.Object, map[string]interface{}{}, "status")
282310
if foo, err = fooClient.UpdateStatus(context.TODO(), foo, metav1.UpdateOptions{}); err != nil {
283311
t.Fatal(err)
284312
}
285-
mustExist(foo.Object, [][]string{{"spec", "a"}, {"spec", "b"}, {"status", "a"}, {"status", "b"}})
313+
mustExist(foo.Object, [][]string{{"spec", "a"}, {"spec", "b"}, {"status", "a"}, {"status", "b"}, {"status", "replicas"}})
286314

287315
t.Logf("Add 'c' default to the storage version and wait until GET sees it in both status and spec")
288316
addDefault("v1beta2", "c", "C")

staging/src/k8s.io/apimachinery/pkg/util/json/json.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,11 +66,36 @@ func Unmarshal(data []byte, v interface{}) error {
6666
// If the decode succeeds, post-process the map to convert json.Number objects to int64 or float64
6767
return convertSliceNumbers(*v, 0)
6868

69+
case *interface{}:
70+
// Build a decoder from the given data
71+
decoder := json.NewDecoder(bytes.NewBuffer(data))
72+
// Preserve numbers, rather than casting to float64 automatically
73+
decoder.UseNumber()
74+
// Run the decode
75+
if err := decoder.Decode(v); err != nil {
76+
return err
77+
}
78+
// If the decode succeeds, post-process the map to convert json.Number objects to int64 or float64
79+
return convertInterfaceNumbers(v, 0)
80+
6981
default:
7082
return json.Unmarshal(data, v)
7183
}
7284
}
7385

86+
func convertInterfaceNumbers(v *interface{}, depth int) error {
87+
var err error
88+
switch v2 := (*v).(type) {
89+
case json.Number:
90+
*v, err = convertNumber(v2)
91+
case map[string]interface{}:
92+
err = convertMapNumbers(v2, depth+1)
93+
case []interface{}:
94+
err = convertSliceNumbers(v2, depth+1)
95+
}
96+
return err
97+
}
98+
7499
// convertMapNumbers traverses the map, converting any json.Number values to int64 or float64.
75100
// values which are map[string]interface{} or []interface{} are recursively visited
76101
func convertMapNumbers(m map[string]interface{}, depth int) error {

staging/src/k8s.io/apimachinery/pkg/util/json/json_test.go

Lines changed: 131 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ limitations under the License.
1919
package json
2020

2121
import (
22+
gojson "encoding/json"
23+
2224
"fmt"
2325
"math"
2426
"reflect"
@@ -278,42 +280,139 @@ func TestEvaluateTypes(t *testing.T) {
278280
},
279281
}
280282

281-
for _, tc := range testCases {
282-
inputJSON := fmt.Sprintf(`{"data":%s}`, tc.In)
283-
expectedJSON := fmt.Sprintf(`{"data":%s}`, tc.Out)
284-
m := map[string]interface{}{}
285-
err := Unmarshal([]byte(inputJSON), &m)
286-
if tc.Err && err != nil {
287-
// Expected error
288-
continue
289-
}
290-
if err != nil {
291-
t.Errorf("%s: error decoding: %v", tc.In, err)
292-
continue
293-
}
294-
if tc.Err {
295-
t.Errorf("%s: expected error, got none", tc.In)
296-
continue
297-
}
298-
data, ok := m["data"]
299-
if !ok {
300-
t.Errorf("%s: decoded object missing data key: %#v", tc.In, m)
301-
continue
302-
}
303-
if !reflect.DeepEqual(tc.Data, data) {
304-
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))
305-
continue
283+
for i, tc := range testCases {
284+
t.Run(fmt.Sprintf("%d_map", i), func(t *testing.T) {
285+
// decode the input as a map item
286+
inputJSON := fmt.Sprintf(`{"data":%s}`, tc.In)
287+
expectedJSON := fmt.Sprintf(`{"data":%s}`, tc.Out)
288+
m := map[string]interface{}{}
289+
err := Unmarshal([]byte(inputJSON), &m)
290+
if tc.Err && err != nil {
291+
// Expected error
292+
return
293+
}
294+
if err != nil {
295+
t.Fatalf("%s: error decoding: %v", tc.In, err)
296+
}
297+
if tc.Err {
298+
t.Fatalf("%s: expected error, got none", tc.In)
299+
}
300+
data, ok := m["data"]
301+
if !ok {
302+
t.Fatalf("%s: decoded object missing data key: %#v", tc.In, m)
303+
}
304+
if !reflect.DeepEqual(tc.Data, data) {
305+
t.Fatalf("%s: expected\n\t%#v (%v), got\n\t%#v (%v)", tc.In, tc.Data, reflect.TypeOf(tc.Data), data, reflect.TypeOf(data))
306+
}
307+
308+
outputJSON, err := Marshal(m)
309+
if err != nil {
310+
t.Fatalf("%s: error encoding: %v", tc.In, err)
311+
}
312+
313+
if expectedJSON != string(outputJSON) {
314+
t.Fatalf("%s: expected\n\t%s, got\n\t%s", tc.In, expectedJSON, string(outputJSON))
315+
}
316+
})
317+
318+
t.Run(fmt.Sprintf("%d_slice", i), func(t *testing.T) {
319+
// decode the input as an array item
320+
inputJSON := fmt.Sprintf(`[0,%s]`, tc.In)
321+
expectedJSON := fmt.Sprintf(`[0,%s]`, tc.Out)
322+
m := []interface{}{}
323+
err := Unmarshal([]byte(inputJSON), &m)
324+
if tc.Err && err != nil {
325+
// Expected error
326+
return
327+
}
328+
if err != nil {
329+
t.Fatalf("%s: error decoding: %v", tc.In, err)
330+
}
331+
if tc.Err {
332+
t.Fatalf("%s: expected error, got none", tc.In)
333+
}
334+
if len(m) != 2 {
335+
t.Fatalf("%s: decoded object wasn't the right length: %#v", tc.In, m)
336+
}
337+
data := m[1]
338+
if !reflect.DeepEqual(tc.Data, data) {
339+
t.Fatalf("%s: expected\n\t%#v (%v), got\n\t%#v (%v)", tc.In, tc.Data, reflect.TypeOf(tc.Data), data, reflect.TypeOf(data))
340+
}
341+
342+
outputJSON, err := Marshal(m)
343+
if err != nil {
344+
t.Fatalf("%s: error encoding: %v", tc.In, err)
345+
}
346+
347+
if expectedJSON != string(outputJSON) {
348+
t.Fatalf("%s: expected\n\t%s, got\n\t%s", tc.In, expectedJSON, string(outputJSON))
349+
}
350+
})
351+
352+
t.Run(fmt.Sprintf("%d_raw", i), func(t *testing.T) {
353+
// decode the input as a standalone object
354+
inputJSON := fmt.Sprintf(`%s`, tc.In)
355+
expectedJSON := fmt.Sprintf(`%s`, tc.Out)
356+
var m interface{}
357+
err := Unmarshal([]byte(inputJSON), &m)
358+
if tc.Err && err != nil {
359+
// Expected error
360+
return
361+
}
362+
if err != nil {
363+
t.Fatalf("%s: error decoding: %v", tc.In, err)
364+
}
365+
if tc.Err {
366+
t.Fatalf("%s: expected error, got none", tc.In)
367+
}
368+
data := m
369+
if !reflect.DeepEqual(tc.Data, data) {
370+
t.Fatalf("%s: expected\n\t%#v (%v), got\n\t%#v (%v)", tc.In, tc.Data, reflect.TypeOf(tc.Data), data, reflect.TypeOf(data))
371+
}
372+
373+
outputJSON, err := Marshal(m)
374+
if err != nil {
375+
t.Fatalf("%s: error encoding: %v", tc.In, err)
376+
}
377+
378+
if expectedJSON != string(outputJSON) {
379+
t.Fatalf("%s: expected\n\t%s, got\n\t%s", tc.In, expectedJSON, string(outputJSON))
380+
}
381+
})
382+
}
383+
}
384+
385+
func TestUnmarshalNil(t *testing.T) {
386+
{
387+
var v *interface{}
388+
err := Unmarshal([]byte(`0`), v)
389+
goerr := gojson.Unmarshal([]byte(`0`), v)
390+
if err == nil || goerr == nil || err.Error() != goerr.Error() {
391+
t.Fatalf("expected error matching stdlib, got %v, %v", err, goerr)
392+
} else {
393+
t.Log(err)
306394
}
395+
}
307396

308-
outputJSON, err := Marshal(m)
309-
if err != nil {
310-
t.Errorf("%s: error encoding: %v", tc.In, err)
311-
continue
397+
{
398+
var v *[]interface{}
399+
err := Unmarshal([]byte(`[]`), v)
400+
goerr := gojson.Unmarshal([]byte(`[]`), v)
401+
if err == nil || goerr == nil || err.Error() != goerr.Error() {
402+
t.Fatalf("expected error matching stdlib, got %v, %v", err, goerr)
403+
} else {
404+
t.Log(err)
312405
}
406+
}
313407

314-
if expectedJSON != string(outputJSON) {
315-
t.Errorf("%s: expected\n\t%s, got\n\t%s", tc.In, expectedJSON, string(outputJSON))
316-
continue
408+
{
409+
var v *map[string]interface{}
410+
err := Unmarshal([]byte(`{}`), v)
411+
goerr := gojson.Unmarshal([]byte(`{}`), v)
412+
if err == nil || goerr == nil || err.Error() != goerr.Error() {
413+
t.Fatalf("expected error matching stdlib, got %v, %v", err, goerr)
414+
} else {
415+
t.Log(err)
317416
}
318417
}
319418
}

0 commit comments

Comments
 (0)