openconfig / ygot

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

SetNode API behavior change when schema path is sent with value as nil #636

Open dlakshma opened 2 years ago

dlakshma commented 2 years ago

Hi,

With ygot version 0.12.3, in SetNode() API if schema path is used till leaf level and value is sent as nil, then valid pointer was returned till the leaf level for the ygot structure and pointer was initialized with the nil value.

Recently upgraded to 0.16.x, and noticed behavior change where pointer itself is nil.

Is there any other means to have 0.12.x behavior.

Thanks, Deepak

wenovus commented 2 years ago

I'm not quite understanding the issue, you mean the intervening structs are not being initialized? Would supplying &ytypes.InitMissingElements{} as a parameter to SetNode as a SetNodeOpt achieve what you're trying to do?

gsindigi commented 2 years ago

@wenovus , here is a test with two different versions and corresponding pointers for two Config leaves i.e., config/mtu & config/description. With 0.12.x, nil values for the leaves are resulting in initializing the corresponding leaf field (pointer) and set the value accordingly. With 0.16, the pointer field itself is un-initialized.

func TestSetNode(t *testing.T) {
    intf := &yoc.OpenconfigInterfaces_Interfaces_Interface{Name: ygot.String("eth 0/1")}
    opts := []ytypes.SetNodeOpt{&ytypes.InitMissingElements{}}
    mtuPath, err := ygot.StringToStructuredPath("config/mtu")
    assert.Nil(t, err)
    intfNode, _ := yoc.SchemaTree["Device"].Dir["interfaces"].Dir["interface"]
    assert.NotNil(t, intfNode)
    mtuValues := []interface{}{nil, &gnmi.TypedValue{Value: &gnmi.TypedValue_UintVal{UintVal: 3480}}}
    for _, v := range mtuValues {
        err = ytypes.SetNode(intfNode, intf, mtuPath, v, opts...)
        assert.Nilf(t, err, `Error: %v`, err)
        if intf.Config.Mtu == nil {
            t.Logf(`intfConfig: %v, mtu is nil`, intf.Config)
        } else {
            t.Logf(`intfConfig: %v, mtu-notnil, mtu: %v`, intf.Config, *intf.Config.Mtu)
        }
    }
    descPath, err := ygot.StringToStructuredPath("config/description")
    assert.Nil(t, err)
    descValues := []interface{}{nil, &gnmi.TypedValue{Value: &gnmi.TypedValue_StringVal{StringVal: `intf description`}}}
    for _, v := range descValues {
        err = ytypes.SetNode(intfNode, intf, descPath, v, opts...)
        assert.Nilf(t, err, `Error: %v`, err)
        if intf.Config.Description == nil {
            t.Logf(`intfConfig: %v, Description is nil`, intf.Config)
        } else {
            t.Logf(`intfConfig: %v, Description-notnil, Description: %v`, intf.Config, *intf.Config.Description)
        }
    }
}
 
With ygot@v0.12.3
=== RUN   TestSetNode
intfConfig: &{  map[]  0xc000a2d8c8  out-of-range E_OpenconfigVlanTypes_TPID_TYPES enum value: 0 out-of-range E_IETFInterfaces_InterfaceType enum value: 0}, mtu-notnil, mtu: 0
intfConfig: &{  map[]  0xc000a2d9a8  out-of-range E_OpenconfigVlanTypes_TPID_TYPES enum value: 0 out-of-range E_IETFInterfaces_InterfaceType enum value: 0}, mtu-notnil, mtu: 3480
intfConfig: &{0xc000b04eb0  map[]  0xc000a2d9a8  out-of-range E_OpenconfigVlanTypes_TPID_TYPES enum value: 0 out-of-range E_IETFInterfaces_InterfaceType enum value: 0}, Description-notnil, Description:
intfConfig: &{0xc000b05250  map[]  0xc000a2d9a8  out-of-range E_OpenconfigVlanTypes_TPID_TYPES enum value: 0 out-of-range E_IETFInterfaces_InterfaceType enum value: 0}, Description-notnil, Description: intf description
--- PASS: TestSetNode (0.00s)
PASS

With ygot@v0.16.1
=== RUN   TestSetNode
intfConfig: &{  map[]    out-of-range E_OpenconfigVlanTypes_TPID_TYPES enum value: 0 out-of-range E_IETFInterfaces_InterfaceType enum value: 0}, mtu is nil
intfConfig: &{  map[]  0xc0011fab10  out-of-range E_OpenconfigVlanTypes_TPID_TYPES enum value: 0 out-of-range E_IETFInterfaces_InterfaceType enum value: 0}, mtu-notnil, mtu: 3480
intfConfig: &{  map[]  0xc0011fab10  out-of-range E_OpenconfigVlanTypes_TPID_TYPES enum value: 0 out-of-range E_IETFInterfaces_InterfaceType enum value: 0}, Description is nil
intfConfig: &{0xc00122e0b0  map[]  0xc0011fab10  out-of-range E_OpenconfigVlanTypes_TPID_TYPES enum value: 0 out-of-range E_IETFInterfaces_InterfaceType enum value: 0}, Description-notnil, Description: intf description
--- PASS: TestSetNode (0.00s)
PASS

wenovus commented 2 years ago

This was a behaviour that was explicitly fixed by https://github.com/openconfig/ygot/pull/595 wherein we don't want nil to actually mean the zero value of the leaf.

If you want the previous behaviour you can use ytypes.GetOrCreateNode(intfNode, intf, mtuPath) when v is nil.