Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove old-style conversions registration #85891

Merged
merged 4 commits into from Dec 16, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -47,19 +47,6 @@ func addToGroupVersion(scheme *runtime.Scheme) error {
if err := scheme.AddIgnoredConversionType(&metav1.TypeMeta{}, &metav1.TypeMeta{}); err != nil {
return err
}
err := scheme.AddConversionFuncs(
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't needed because below we're calling metav1.AddToGroupVersion, which is registering all the necessary conversions (including these ones).

metav1.Convert_string_To_labels_Selector,
metav1.Convert_labels_Selector_To_string,

metav1.Convert_string_To_fields_Selector,
metav1.Convert_fields_Selector_To_string,

metav1.Convert_Map_string_To_string_To_v1_LabelSelector,
metav1.Convert_v1_LabelSelector_To_Map_string_To_string,
)
if err != nil {
return err
}
// ListOptions is the only options struct which needs conversion (it exposes labels and fields
// as selectors for convenience). The other types have only a single representation today.
scheme.AddKnownTypes(SchemeGroupVersion,
Expand Down
4 changes: 1 addition & 3 deletions staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/register.go
Expand Up @@ -95,9 +95,7 @@ func AddMetaToScheme(scheme *runtime.Scheme) error {
&PartialObjectMetadataList{},
)

return scheme.AddConversionFuncs(
Convert_Slice_string_To_v1_IncludeObjectPolicy,
)
return nil
}

func init() {
Expand Down
Expand Up @@ -21,7 +21,6 @@ go_library(
"//staging/src/k8s.io/apimachinery/pkg/conversion: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/util/runtime:go_default_library",
"//vendor/github.com/gogo/protobuf/proto:go_default_library",
],
)
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Expand Up @@ -19,7 +19,6 @@ package v1beta1
import (
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
)

// GroupName is the group name for this API.
Expand All @@ -33,12 +32,6 @@ func Kind(kind string) schema.GroupKind {
return SchemeGroupVersion.WithKind(kind).GroupKind()
}

// scheme is the registry for the common types that adhere to the meta v1beta1 API spec.
var scheme = runtime.NewScheme()

// ParameterCodec knows about query parameters used with the meta v1beta1 API spec.
var ParameterCodec = runtime.NewParameterCodec(scheme)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No one was using the ParameterCodec here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From v1beta1 - noone in k/k repo and anywhere else that I managed to check.


// AddMetaToScheme registers base meta types into schemas.
func AddMetaToScheme(scheme *runtime.Scheme) error {
scheme.AddKnownTypes(SchemeGroupVersion,
Expand All @@ -48,14 +41,5 @@ func AddMetaToScheme(scheme *runtime.Scheme) error {
&PartialObjectMetadataList{},
)

return scheme.AddConversionFuncs(
Convert_Slice_string_To_v1beta1_IncludeObjectPolicy,
)
}

func init() {
utilruntime.Must(AddMetaToScheme(scheme))

// register manually. This usually goes through the SchemeBuilder, which we cannot use here.
utilruntime.Must(RegisterDefaults(scheme))
return nil
}
117 changes: 21 additions & 96 deletions staging/src/k8s.io/apimachinery/pkg/conversion/converter.go
Expand Up @@ -54,7 +54,8 @@ type Converter struct {
generatedConversionFuncs ConversionFuncs

// Set of conversions that should be treated as a no-op
ignoredConversions map[typePair]struct{}
ignoredConversions map[typePair]struct{}
ignoredUntypedConversions map[typePair]struct{}

// This is a map from a source field type and name, to a list of destination
// field type and name.
Expand Down Expand Up @@ -83,17 +84,23 @@ type Converter struct {
// NewConverter creates a new Converter object.
func NewConverter(nameFn NameFunc) *Converter {
c := &Converter{
conversionFuncs: NewConversionFuncs(),
generatedConversionFuncs: NewConversionFuncs(),
ignoredConversions: make(map[typePair]struct{}),
nameFunc: nameFn,
structFieldDests: make(map[typeNamePair][]typeNamePair),
structFieldSources: make(map[typeNamePair][]typeNamePair),
conversionFuncs: NewConversionFuncs(),
generatedConversionFuncs: NewConversionFuncs(),
ignoredConversions: make(map[typePair]struct{}),
ignoredUntypedConversions: make(map[typePair]struct{}),
nameFunc: nameFn,
structFieldDests: make(map[typeNamePair][]typeNamePair),
structFieldSources: make(map[typeNamePair][]typeNamePair),

inputFieldMappingFuncs: make(map[reflect.Type]FieldMappingFunc),
inputDefaultFlags: make(map[reflect.Type]FieldMatchingFlags),
}
c.RegisterConversionFunc(Convert_Slice_byte_To_Slice_byte)
c.RegisterUntypedConversionFunc(
(*[]byte)(nil), (*[]byte)(nil),
func(a, b interface{}, s Scope) error {
return Convert_Slice_byte_To_Slice_byte(a.(*[]byte), b.(*[]byte), s)
},
)
return c
}

Expand Down Expand Up @@ -153,31 +160,14 @@ type FieldMappingFunc func(key string, sourceTag, destTag reflect.StructTag) (so

func NewConversionFuncs() ConversionFuncs {
return ConversionFuncs{
fns: make(map[typePair]reflect.Value),
untyped: make(map[typePair]ConversionFunc),
}
}

type ConversionFuncs struct {
fns map[typePair]reflect.Value
untyped map[typePair]ConversionFunc
}

// Add adds the provided conversion functions to the lookup table - they must have the signature
// `func(type1, type2, Scope) error`. Functions are added in the order passed and will override
// previously registered pairs.
func (c ConversionFuncs) Add(fns ...interface{}) error {
for _, fn := range fns {
fv := reflect.ValueOf(fn)
ft := fv.Type()
if err := verifyConversionFunctionSignature(ft); err != nil {
return err
}
c.fns[typePair{ft.In(0).Elem(), ft.In(1).Elem()}] = fv
}
return nil
}

// AddUntyped adds the provided conversion function to the lookup table for the types that are
// supplied as a and b. a and b must be pointers or an error is returned. This method overwrites
// previously defined functions.
Expand All @@ -197,12 +187,6 @@ func (c ConversionFuncs) AddUntyped(a, b interface{}, fn ConversionFunc) error {
// both other and c, with other conversions taking precedence.
func (c ConversionFuncs) Merge(other ConversionFuncs) ConversionFuncs {
merged := NewConversionFuncs()
for k, v := range c.fns {
merged.fns[k] = v
}
for k, v := range other.fns {
merged.fns[k] = v
}
for k, v := range c.untyped {
merged.untyped[k] = v
}
Expand Down Expand Up @@ -360,29 +344,6 @@ func verifyConversionFunctionSignature(ft reflect.Type) error {
return nil
}

// RegisterConversionFunc registers a conversion func with the
// Converter. conversionFunc must take three parameters: a pointer to the input
// type, a pointer to the output type, and a conversion.Scope (which should be
// used if recursive conversion calls are desired). It must return an error.
//
// Example:
// c.RegisterConversionFunc(
// func(in *Pod, out *v1.Pod, s Scope) error {
// // conversion logic...
// return nil
// })
// DEPRECATED: Will be removed in favor of RegisterUntypedConversionFunc
func (c *Converter) RegisterConversionFunc(conversionFunc interface{}) error {
return c.conversionFuncs.Add(conversionFunc)
}

// Similar to RegisterConversionFunc, but registers conversion function that were
// automatically generated.
// DEPRECATED: Will be removed in favor of RegisterGeneratedUntypedConversionFunc
func (c *Converter) RegisterGeneratedConversionFunc(conversionFunc interface{}) error {
return c.generatedConversionFuncs.Add(conversionFunc)
}

// RegisterUntypedConversionFunc registers a function that converts between a and b by passing objects of those
// types to the provided function. The function *must* accept objects of a and b - this machinery will not enforce
// any other guarantee.
Expand All @@ -409,6 +370,7 @@ func (c *Converter) RegisterIgnoredConversion(from, to interface{}) error {
return fmt.Errorf("expected pointer arg for 'to' param 1, got: %v", typeTo)
}
c.ignoredConversions[typePair{typeFrom.Elem(), typeTo.Elem()}] = struct{}{}
c.ignoredUntypedConversions[typePair{typeFrom, typeTo}] = struct{}{}
return nil
}

Expand Down Expand Up @@ -491,6 +453,11 @@ func (c *Converter) doConversion(src, dest interface{}, flags FieldMatchingFlags
flags: flags,
meta: meta,
}

// ignore conversions of this type
if _, ok := c.ignoredUntypedConversions[pair]; ok {
return nil
}
if fn, ok := c.conversionFuncs.untyped[pair]; ok {
return fn(src, dest, scope)
}
Expand All @@ -517,35 +484,6 @@ func (c *Converter) doConversion(src, dest interface{}, flags FieldMatchingFlags
return f(sv, dv, scope)
}

// callCustom calls 'custom' with sv & dv. custom must be a conversion function.
func (c *Converter) callCustom(sv, dv, custom reflect.Value, scope *scope) error {
if !sv.CanAddr() {
sv2 := reflect.New(sv.Type())
sv2.Elem().Set(sv)
sv = sv2
} else {
sv = sv.Addr()
}
if !dv.CanAddr() {
if !dv.CanSet() {
return scope.errorf("can't addr or set dest.")
}
dvOrig := dv
dv := reflect.New(dvOrig.Type())
defer func() { dvOrig.Set(dv) }()
} else {
dv = dv.Addr()
}
args := []reflect.Value{sv, dv, reflect.ValueOf(scope)}
ret := custom.Call(args)[0].Interface()
// This convolution is necessary because nil interfaces won't convert
// to errors.
if ret == nil {
return nil
}
return ret.(error)
}

// callUntyped calls predefined conversion func.
func (c *Converter) callUntyped(sv, dv reflect.Value, f ConversionFunc, scope *scope) error {
if !dv.CanAddr() {
Expand Down Expand Up @@ -577,19 +515,6 @@ func (c *Converter) convert(sv, dv reflect.Value, scope *scope) error {
}

// Convert sv to dv.
if fv, ok := c.conversionFuncs.fns[pair]; ok {
if c.Debug != nil {
c.Debug.Logf("Calling custom conversion of '%v' to '%v'", st, dt)
}
return c.callCustom(sv, dv, fv, scope)
}
if fv, ok := c.generatedConversionFuncs.fns[pair]; ok {
if c.Debug != nil {
c.Debug.Logf("Calling generated conversion of '%v' to '%v'", st, dt)
}
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)
Expand Down