search

openconfig / ygot

A YANG-centric Go toolkit - Go/Protobuf Code Generation; Validation; Marshaling/Unmarshaling
Apache License 2.0
285 stars 107 forks source link

Fix `Marshal7951` for direct union leaf calls. #947

Closed wenovus closed 9 months ago

wenovus commented 9 months ago

When jsonValue is called using the output of reflect.ValueOf() instead of reflect.StructField, any interface type is lost through re-packing to the empty interface (any). This means the reflect.Kind of a union field is no longer the union type, but instead its underlying concrete type. The current code doesn't handle this case, leading to runtime errors.

This handling code is now added, with a couple more fixes related to empty types.

Tested manually that the following ygnmi call is no longer affected by the original error:

sp := gnmi.OC().NetworkInstance("DEFAULT").Protocol(oc.PolicyTypes_INSTALL_PROTOCOL_TYPE_STATIC, "STATIC")
ygnmi.Replace(context.Background(), c, sp.Static("1.1.1.1/32").SetTag().Config(), oc.NetworkInstance_Protocol_Static_SetTag_Union(oc.UnionUint32(42)))
coveralls commented 9 months ago

Coverage Status

coverage: 89.606% (-0.006%) from 89.612% when pulling da103aae6762e45794aded15b7b391304c1c203e on marshal7951-union-bug into dca67e299ee14893d2f9a55eaf61e6048a3c44cf on master.

wenovus commented 9 months ago

Thanks for fixing this Wen!

Thanks for the review! and finding the bug!