From 12ee83ed94268bcd62343cedf83a74ca53ed500b Mon Sep 17 00:00:00 2001 From: wojtekt Date: Thu, 28 Nov 2019 14:41:07 +0100 Subject: [PATCH 1/3] Use new-style conversions in default conversions --- .../apimachinery/pkg/conversion/converter.go | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/staging/src/k8s.io/apimachinery/pkg/conversion/converter.go b/staging/src/k8s.io/apimachinery/pkg/conversion/converter.go index bc615dc3ace..eaed33fc367 100644 --- a/staging/src/k8s.io/apimachinery/pkg/conversion/converter.go +++ b/staging/src/k8s.io/apimachinery/pkg/conversion/converter.go @@ -546,6 +546,22 @@ func (c *Converter) callCustom(sv, dv, custom reflect.Value, scope *scope) error return ret.(error) } +// callUntyped calls predefined conversion func. +func (c *Converter) callUntyped(sv, dv reflect.Value, f ConversionFunc, scope *scope) error { + if !dv.CanAddr() { + return scope.errorf("cant addr dest") + } + var svPointer reflect.Value + if sv.CanAddr() { + svPointer = sv.Addr() + } else { + svPointer = reflect.New(sv.Type()) + svPointer.Elem().Set(sv) + } + dvPointer := dv.Addr() + return f(svPointer.Interface(), dvPointer.Interface(), scope) +} + // convert recursively copies sv into dv, calling an appropriate conversion function if // one is registered. func (c *Converter) convert(sv, dv reflect.Value, scope *scope) error { @@ -574,6 +590,14 @@ func (c *Converter) convert(sv, dv reflect.Value, scope *scope) error { return c.callCustom(sv, dv, fv, scope) } + pair = typePair{reflect.PtrTo(sv.Type()), reflect.PtrTo(dv.Type())} + if f, ok := c.conversionFuncs.untyped[pair]; ok { + return c.callUntyped(sv, dv, f, scope) + } + if f, ok := c.generatedConversionFuncs.untyped[pair]; ok { + return c.callUntyped(sv, dv, f, scope) + } + return c.defaultConvert(sv, dv, scope) } From d6969d2c554d15a2efb468517980b71e46f0f85f Mon Sep 17 00:00:00 2001 From: wojtekt Date: Thu, 28 Nov 2019 14:37:08 +0100 Subject: [PATCH 2/3] Cleanup metav1 conversions --- .../pkg/apis/meta/internalversion/register.go | 2 +- .../apimachinery/pkg/apis/meta/v1/BUILD | 1 - .../pkg/apis/meta/v1/conversion.go | 59 ------------------- .../pkg/apis/meta/v1/conversion_test.go | 37 +++++------- .../apimachinery/pkg/apis/meta/v1/register.go | 13 +--- 5 files changed, 16 insertions(+), 96 deletions(-) diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/internalversion/register.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/internalversion/register.go index 19f21099873..c64fa4f0b9a 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/internalversion/register.go +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/internalversion/register.go @@ -87,6 +87,7 @@ func addToGroupVersion(scheme *runtime.Scheme) error { &metav1.DeleteOptions{}, &metav1.CreateOptions{}, &metav1.UpdateOptions{}) + metav1.AddToGroupVersion(scheme, metav1.SchemeGroupVersion) return nil } @@ -95,5 +96,4 @@ func addToGroupVersion(scheme *runtime.Scheme) error { // the logic for conversion private. func init() { localSchemeBuilder.Register(addToGroupVersion) - localSchemeBuilder.Register(metav1.RegisterConversions) } diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/BUILD b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/BUILD index 970e1c88f0c..39faf5ee5df 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/BUILD +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/BUILD @@ -24,7 +24,6 @@ go_test( deps = [ "//staging/src/k8s.io/apimachinery/pkg/api/equality:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library", - "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime/serializer/json:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library", diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/conversion.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/conversion.go index 285a41a4228..b937398cd34 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/conversion.go +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/conversion.go @@ -26,69 +26,10 @@ import ( "k8s.io/apimachinery/pkg/conversion" "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/labels" - "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" ) -func AddConversionFuncs(scheme *runtime.Scheme) error { - return scheme.AddConversionFuncs( - Convert_v1_TypeMeta_To_v1_TypeMeta, - - Convert_v1_ListMeta_To_v1_ListMeta, - - Convert_v1_DeleteOptions_To_v1_DeleteOptions, - - Convert_intstr_IntOrString_To_intstr_IntOrString, - Convert_Pointer_intstr_IntOrString_To_intstr_IntOrString, - Convert_intstr_IntOrString_To_Pointer_intstr_IntOrString, - - Convert_Pointer_v1_Duration_To_v1_Duration, - Convert_v1_Duration_To_Pointer_v1_Duration, - - Convert_Slice_string_To_v1_Time, - Convert_Slice_string_To_Pointer_v1_Time, - - Convert_v1_Time_To_v1_Time, - Convert_v1_MicroTime_To_v1_MicroTime, - - Convert_resource_Quantity_To_resource_Quantity, - - Convert_string_To_labels_Selector, - Convert_labels_Selector_To_string, - - Convert_string_To_fields_Selector, - Convert_fields_Selector_To_string, - - Convert_Pointer_bool_To_bool, - Convert_bool_To_Pointer_bool, - - Convert_Pointer_string_To_string, - Convert_string_To_Pointer_string, - - Convert_Pointer_int64_To_int, - Convert_int_To_Pointer_int64, - - Convert_Pointer_int32_To_int32, - Convert_int32_To_Pointer_int32, - - Convert_Pointer_int64_To_int64, - Convert_int64_To_Pointer_int64, - - Convert_Pointer_float64_To_float64, - Convert_float64_To_Pointer_float64, - - Convert_Map_string_To_string_To_v1_LabelSelector, - Convert_v1_LabelSelector_To_Map_string_To_string, - - Convert_Slice_string_To_Slice_int32, - - Convert_Slice_string_To_Pointer_v1_DeletionPropagation, - - Convert_Slice_string_To_v1_IncludeObjectPolicy, - ) -} - func Convert_Pointer_float64_To_float64(in **float64, out *float64, s conversion.Scope) error { if *in == nil { *out = 0 diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/conversion_test.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/conversion_test.go index 45753f7ca96..10637599728 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/conversion_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/conversion_test.go @@ -17,13 +17,11 @@ limitations under the License. package v1_test import ( - "net/url" "testing" "time" apiequality "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" ) func TestMapToLabelSelectorRoundTrip(t *testing.T) { @@ -86,49 +84,42 @@ func TestConvertSliceStringToDeletionPropagation(t *testing.T) { } } -func TestUrlValuesToPointerTime(t *testing.T) { - scheme := runtime.NewScheme() - v1.AddConversionFuncs(scheme) - - type testType struct { - Time *v1.Time `json:"time"` - } - +func TestConvertSliceStringToPointerTime(t *testing.T) { t1 := v1.Date(1998, time.May, 5, 5, 5, 5, 0, time.UTC) t1String := t1.Format(time.RFC3339) t2 := v1.Date(2000, time.June, 6, 6, 6, 6, 0, time.UTC) t2String := t2.Format(time.RFC3339) tcs := []struct { - Input url.Values + Input []string Output *v1.Time }{ { - Input: url.Values{}, - Output: nil, - }, - { - Input: url.Values{"time": []string{}}, + Input: []string{}, Output: &v1.Time{}, }, { - Input: url.Values{"time": []string{""}}, + Input: []string{""}, Output: &v1.Time{}, }, { - Input: url.Values{"time": []string{t1String, t2String}}, + Input: []string{t1String}, + Output: &t1, + }, + { + Input: []string{t1String, t2String}, Output: &t1, }, } for _, tc := range tcs { - result := &testType{} - if err := scheme.Convert(&tc.Input, &result, nil); err != nil { - t.Errorf("Failed to convert []string to *metav1.Time %#v: %v", tc.Input, err) + var timePtr *v1.Time + if err := v1.Convert_Slice_string_To_Pointer_v1_Time(&tc.Input, &timePtr, nil); err != nil { + t.Errorf("Convert_Slice_string_To_Pointer_v1_Time(%#v): %v", tc.Input, err) continue } - if !apiequality.Semantic.DeepEqual(result.Time, tc.Output) { - t.Errorf("Unexpected output: %v, expected: %v", result.Time, tc.Output) + if !apiequality.Semantic.DeepEqual(timePtr, tc.Output) { + t.Errorf("slice string to *v1.Time conversion failed: got %#v; want %#v", timePtr, tc.Output) } } } diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/register.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/register.go index a7b8aa34f9e..31f3fe11425 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/register.go +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/register.go @@ -53,15 +53,6 @@ var scheme = runtime.NewScheme() // ParameterCodec knows about query parameters used with the meta v1 API spec. var ParameterCodec = runtime.NewParameterCodec(scheme) -func addEventConversionFuncs(scheme *runtime.Scheme) error { - return scheme.AddConversionFuncs( - Convert_v1_WatchEvent_To_watch_Event, - Convert_v1_InternalEvent_To_v1_WatchEvent, - Convert_watch_Event_To_v1_WatchEvent, - Convert_v1_WatchEvent_To_v1_InternalEvent, - ) -} - var optionsTypes = []runtime.Object{ &ListOptions{}, &ExportOptions{}, @@ -90,10 +81,8 @@ func AddToGroupVersion(scheme *runtime.Scheme, groupVersion schema.GroupVersion) &APIResourceList{}, ) - utilruntime.Must(addEventConversionFuncs(scheme)) - // register manually. This usually goes through the SchemeBuilder, which we cannot use here. - utilruntime.Must(AddConversionFuncs(scheme)) + utilruntime.Must(RegisterConversions(scheme)) utilruntime.Must(RegisterDefaults(scheme)) } From 2df8ad1b1c8571c8cb98d05967db3c054d76c46e Mon Sep 17 00:00:00 2001 From: wojtekt Date: Thu, 28 Nov 2019 16:10:36 +0100 Subject: [PATCH 3/3] Cleanup default conversions --- .../apimachinery/pkg/runtime/conversion.go | 32 ++++++++++++++----- .../apimachinery/pkg/runtime/embedded.go | 15 ++++++--- .../k8s.io/apimachinery/pkg/runtime/scheme.go | 6 ++-- 3 files changed, 38 insertions(+), 15 deletions(-) diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/conversion.go b/staging/src/k8s.io/apimachinery/pkg/runtime/conversion.go index 0947dce7356..d04d701f376 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/conversion.go +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/conversion.go @@ -53,14 +53,6 @@ func JSONKeyMapper(key string, sourceTag, destTag reflect.StructTag) (string, st return key, key } -// DefaultStringConversions are helpers for converting []string and string to real values. -var DefaultStringConversions = []interface{}{ - Convert_Slice_string_To_string, - Convert_Slice_string_To_int, - Convert_Slice_string_To_bool, - Convert_Slice_string_To_int64, -} - func Convert_Slice_string_To_string(in *[]string, out *string, s conversion.Scope) error { if len(*in) == 0 { *out = "" @@ -178,3 +170,27 @@ func Convert_Slice_string_To_Pointer_int64(in *[]string, out **int64, s conversi *out = &i return nil } + +func RegisterStringConversions(s *Scheme) error { + if err := s.AddConversionFunc((*[]string)(nil), (*string)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_Slice_string_To_string(a.(*[]string), b.(*string), scope) + }); err != nil { + return err + } + if err := s.AddConversionFunc((*[]string)(nil), (*int)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_Slice_string_To_int(a.(*[]string), b.(*int), scope) + }); err != nil { + return err + } + if err := s.AddConversionFunc((*[]string)(nil), (*bool)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_Slice_string_To_bool(a.(*[]string), b.(*bool), scope) + }); err != nil { + return err + } + if err := s.AddConversionFunc((*[]string)(nil), (*int64)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_Slice_string_To_int64(a.(*[]string), b.(*int64), scope) + }); err != nil { + return err + } + return nil +} diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/embedded.go b/staging/src/k8s.io/apimachinery/pkg/runtime/embedded.go index db11eb8bcf6..7251e65f6e0 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/embedded.go +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/embedded.go @@ -134,9 +134,16 @@ func Convert_runtime_RawExtension_To_runtime_Object(in *RawExtension, out *Objec return nil } -func DefaultEmbeddedConversions() []interface{} { - return []interface{}{ - Convert_runtime_Object_To_runtime_RawExtension, - Convert_runtime_RawExtension_To_runtime_Object, +func RegisterEmbeddedConversions(s *Scheme) error { + if err := s.AddConversionFunc((*Object)(nil), (*RawExtension)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_runtime_Object_To_runtime_RawExtension(a.(*Object), b.(*RawExtension), scope) + }); err != nil { + return err } + if err := s.AddConversionFunc((*RawExtension)(nil), (*Object)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_runtime_RawExtension_To_runtime_Object(a.(*RawExtension), b.(*Object), scope) + }); err != nil { + return err + } + return nil } diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/scheme.go b/staging/src/k8s.io/apimachinery/pkg/runtime/scheme.go index fd37e293ab1..c21ed0499ae 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/scheme.go +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/scheme.go @@ -102,10 +102,10 @@ func NewScheme() *Scheme { } s.converter = conversion.NewConverter(s.nameFunc) - utilruntime.Must(s.AddConversionFuncs(DefaultEmbeddedConversions()...)) + // Enable couple default conversions by default. + utilruntime.Must(RegisterEmbeddedConversions(s)) + utilruntime.Must(RegisterStringConversions(s)) - // Enable map[string][]string conversions by default - utilruntime.Must(s.AddConversionFuncs(DefaultStringConversions...)) utilruntime.Must(s.RegisterInputDefaults(&map[string][]string{}, JSONKeyMapper, conversion.AllowDifferentFieldTypeNames|conversion.IgnoreMissingFields)) utilruntime.Must(s.RegisterInputDefaults(&url.Values{}, JSONKeyMapper, conversion.AllowDifferentFieldTypeNames|conversion.IgnoreMissingFields)) return s