- Notifications
You must be signed in to change notification settings - Fork41k
Description
What happened?
As the title says, conversion-gen has a bug that generates incorrect conversion code.
struct in the old apiVersion:
typeJSONSchemaPropsstruct {Items*JSONSchemaProps`json:"items,omitempty"`Propertiesmap[string]JSONSchemaProps`json:"properties,omitempty"`AllOf []JSONSchemaProps`json:"allOf,omitempty"`ExclusiveMaximumbool`json:"exclusiveMaximum,omitempty"`}
struct in the new apiVersion: (ExclusiveMaximum was changed frombool to*bool)
typeJSONSchemaPropsstruct {Items*JSONSchemaProps`json:"items,omitempty"`Propertiesmap[string]JSONSchemaProps`json:"properties,omitempty"`AllOf []JSONSchemaProps`json:"allOf,omitempty"`ExclusiveMaximum*bool`json:"exclusiveMaximum,omitempty"`}
Generated conversion code:
funcautoConvert_v1beta2_JSONSchemaProps_To_v1beta1_JSONSchemaProps(in*v1beta2.JSONSchemaProps,out*JSONSchemaProps,s conversion.Scope)error {out.Items= (*JSONSchemaProps)(unsafe.Pointer(in.Items))out.Properties=*(*map[string]JSONSchemaProps)(unsafe.Pointer(&in.Properties))out.AllOf=*(*[]JSONSchemaProps)(unsafe.Pointer(&in.AllOf))iferr:=v1.Convert_Pointer_bool_To_bool(&in.ExclusiveMaximum,&out.ExclusiveMaximum,s);err!=nil {returnerr}returnnil}
Issue:out.Items = (*JSONSchemaProps)(unsafe.Pointer(in.Items)) is wrong (similar forProperties andAllOf).autoConvert_v1beta2_JSONSchemaProps_To_v1beta1_JSONSchemaProps should be called instead.
Accessing theExlusiveMaximum field after conversion leads to a panic.
Full example can be seen here:https://github.com/sbueringer/conversion-gen-panic-repro (just checkout and runmake test).
What did you expect to happen?
conversion-gen should generate correct code
How can we reproduce it (as minimally and precisely as possible)?
Full example can be seen here:https://github.com/sbueringer/conversion-gen-panic-repro (just checkout and runmake test).
Kubernetes version
conversion-gen v0.33.0
Related Slack thread:https://kubernetes.slack.com/archives/C0EG7JC6T/p1752170662367799 (but all relevant information is contained in this issue)