diff --git a/staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/BUILD b/staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/BUILD index a4de1a6f93e..a98e4232620 100644 --- a/staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/BUILD +++ b/staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/BUILD @@ -3,17 +3,6 @@ package(default_visibility = ["//visibility:public"]) load( "@io_bazel_rules_go//go:def.bzl", "go_library", - "go_test", -) - -go_test( - name = "go_default_test", - srcs = ["converter_test.go"], - library = ":go_default_library", - deps = [ - "//vendor/k8s.io/apimachinery/pkg/util/diff:go_default_library", - "//vendor/k8s.io/apimachinery/pkg/util/json:go_default_library", - ], ) go_library( @@ -40,6 +29,9 @@ filegroup( filegroup( name = "all-srcs", - srcs = [":package-srcs"], + srcs = [ + ":package-srcs", + "//staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/testing:all-srcs", + ], tags = ["automanaged"], ) diff --git a/staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/converter.go b/staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/converter.go index e7021f1b952..84dca4ac7c2 100644 --- a/staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/converter.go +++ b/staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/converter.go @@ -426,17 +426,29 @@ var ( falseBytes = []byte("false") ) -func toUnstructured(sv, dv reflect.Value) error { - st, dt := sv.Type(), dv.Type() +func getMarshaler(v reflect.Value) (encodingjson.Marshaler, bool) { + // Check value receivers if v is not a pointer and pointer receivers if v is a pointer + if v.Type().Implements(marshalerType) { + return v.Interface().(encodingjson.Marshaler), true + } + // Check pointer receivers if v is not a pointer + if v.Kind() != reflect.Ptr && v.CanAddr() { + v = v.Addr() + if v.Type().Implements(marshalerType) { + return v.Interface().(encodingjson.Marshaler), true + } + } + return nil, false +} +func toUnstructured(sv, dv reflect.Value) error { // Check if the object has a custom JSON marshaller/unmarshaller. - if st.Implements(marshalerType) { + if marshaler, ok := getMarshaler(sv); ok { if sv.Kind() == reflect.Ptr && sv.IsNil() { // We're done - we don't need to store anything. return nil } - marshaler := sv.Interface().(encodingjson.Marshaler) data, err := marshaler.MarshalJSON() if err != nil { return err @@ -496,6 +508,7 @@ func toUnstructured(sv, dv reflect.Value) error { return nil } + st, dt := sv.Type(), dv.Type() switch st.Kind() { case reflect.String: if dt.Kind() == reflect.Interface && dv.NumMethod() == 0 { diff --git a/staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/testing/BUILD b/staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/testing/BUILD new file mode 100644 index 00000000000..77d4bd72e42 --- /dev/null +++ b/staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/testing/BUILD @@ -0,0 +1,28 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_test") + +go_test( + name = "go_default_test", + srcs = ["converter_test.go"], + deps = [ + "//vendor/github.com/stretchr/testify/assert:go_default_library", + "//vendor/github.com/stretchr/testify/require:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/conversion/unstructured:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/util/diff:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/util/json:go_default_library", + ], +) + +filegroup( + name = "package-srcs", + srcs = glob(["**"]), + tags = ["automanaged"], + visibility = ["//visibility:private"], +) + +filegroup( + name = "all-srcs", + srcs = [":package-srcs"], + tags = ["automanaged"], + visibility = ["//visibility:public"], +) diff --git a/staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/converter_test.go b/staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/testing/converter_test.go similarity index 80% rename from staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/converter_test.go rename to staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/testing/converter_test.go index 4c5264e18e5..1338e4e430b 100644 --- a/staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/converter_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/testing/converter_test.go @@ -14,15 +14,25 @@ See the License for the specific language governing permissions and limitations under the License. */ -package unstructured +// These tests are in a separate package to break cyclic dependency in tests. +// Unstructured type depends on unstructured converter package but we want to test how the converter handles +// the Unstructured type so we need to import both. + +package testing import ( "fmt" "reflect" + "strconv" "testing" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + conversionunstructured "k8s.io/apimachinery/pkg/conversion/unstructured" "k8s.io/apimachinery/pkg/util/diff" "k8s.io/apimachinery/pkg/util/json" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) // Definte a number of test types. @@ -72,14 +82,27 @@ type F struct { } type G struct { - Custom Custom `json:"custom"` + CustomValue1 CustomValue `json:"customValue1"` + CustomValue2 *CustomValue `json:"customValue2"` + CustomPointer1 CustomPointer `json:"customPointer1"` + CustomPointer2 *CustomPointer `json:"customPointer2"` } -type Custom struct { +type CustomValue struct { data []byte } -func (c Custom) MarshalJSON() ([]byte, error) { +// MarshalJSON has a value receiver on this type. +func (c CustomValue) MarshalJSON() ([]byte, error) { + return c.data, nil +} + +type CustomPointer struct { + data []byte +} + +// MarshalJSON has a pointer receiver on this type. +func (c *CustomPointer) MarshalJSON() ([]byte, error) { return c.data, nil } @@ -113,14 +136,14 @@ func doRoundTrip(t *testing.T, item interface{}) { return } - newUnstr, err := DefaultConverter.ToUnstructured(item) + newUnstr, err := conversionunstructured.DefaultConverter.ToUnstructured(item) if err != nil { t.Errorf("ToUnstructured failed: %v", err) return } newObj := reflect.New(reflect.TypeOf(item).Elem()).Interface() - err = DefaultConverter.FromUnstructured(newUnstr, newObj) + err = conversionunstructured.DefaultConverter.FromUnstructured(newUnstr, newObj) if err != nil { t.Errorf("FromUnstructured failed: %v", err) return @@ -136,6 +159,17 @@ func TestRoundTrip(t *testing.T) { testCases := []struct { obj interface{} }{ + { + obj: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "Foo", + "metadata": map[string]interface{}{ + "name": "foo1", + }, + }, + }, + }, { // This (among others) tests nil map, slice and pointer. obj: &C{ @@ -230,7 +264,7 @@ func doUnrecognized(t *testing.T, jsonData string, item interface{}, expectedErr return } newObj := reflect.New(reflect.TypeOf(item).Elem()).Interface() - err = DefaultConverter.FromUnstructured(unstr, newObj) + err = conversionunstructured.DefaultConverter.FromUnstructured(unstr, newObj) if (err != nil) != (expectedErr != nil) { t.Errorf("Unexpected error in FromUnstructured: %v, expected: %v", err, expectedErr) } @@ -434,7 +468,7 @@ func TestFloatIntConversion(t *testing.T) { unstr := map[string]interface{}{"fd": float64(3)} var obj F - if err := DefaultConverter.FromUnstructured(unstr, &obj); err != nil { + if err := conversionunstructured.DefaultConverter.FromUnstructured(unstr, &obj); err != nil { t.Errorf("Unexpected error in FromUnstructured: %v", err) } @@ -469,18 +503,37 @@ func TestCustomToUnstructured(t *testing.T) { } for _, tc := range testcases { - result, err := DefaultConverter.ToUnstructured(&G{Custom: Custom{data: []byte(tc.Data)}}) - if err != nil { - t.Errorf("%s: %v", tc.Data, err) - continue - } - - fieldResult := result["custom"] - if !reflect.DeepEqual(fieldResult, tc.Expected) { - t.Errorf("%s: expected %v, got %v", tc.Data, tc.Expected, fieldResult) - // t.Log("expected", spew.Sdump(tc.Expected)) - // t.Log("actual", spew.Sdump(fieldResult)) - continue - } + tc := tc + t.Run(tc.Data, func(t *testing.T) { + t.Parallel() + result, err := conversionunstructured.DefaultConverter.ToUnstructured(&G{ + CustomValue1: CustomValue{data: []byte(tc.Data)}, + CustomValue2: &CustomValue{data: []byte(tc.Data)}, + CustomPointer1: CustomPointer{data: []byte(tc.Data)}, + CustomPointer2: &CustomPointer{data: []byte(tc.Data)}, + }) + require.NoError(t, err) + for field, fieldResult := range result { + assert.Equal(t, tc.Expected, fieldResult, field) + } + }) + } +} + +func TestCustomToUnstructuredTopLevel(t *testing.T) { + // Only objects are supported at the top level + topLevelCases := []interface{}{ + &CustomValue{data: []byte(`{"a":1}`)}, + &CustomPointer{data: []byte(`{"a":1}`)}, + } + expected := map[string]interface{}{"a": int64(1)} + for i, obj := range topLevelCases { + obj := obj + t.Run(strconv.Itoa(i), func(t *testing.T) { + t.Parallel() + result, err := conversionunstructured.DefaultConverter.ToUnstructured(obj) + require.NoError(t, err) + assert.Equal(t, expected, result) + }) } }