Panic at `gosnappi.(*deviceBgpRouter).RouterId` if BGP is not present in the configuration

[bortok]

After upgrading to gosnappi:0.13.0 the following code of otgen (run.go) causes panic:

        for _, d := range config.Devices().Items() {
            if d.Bgp().RouterId() != "" {}

encountered with the following OTG configuration otg.panic.yml.txt – it doesn't have BGP configuration, and the code above is trying to detect if BGP is configured

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0xa8e868]

goroutine 1 [running]:
github.com/open-traffic-generator/snappi/gosnappi.(*deviceBgpRouter).RouterId(0xc0006a2bb0?)
    /home/parallels/go/pkg/mod/github.com/open-traffic-generator/snappi/gosnappi@v0.13.0/gosnappi.go:26718 +0x8
github.com/open-traffic-generator/otgen/cmd.startProtocols({0x15ecc00, 0xc00009c900}, {0x15df7b0, 0xc000259d40})
    /home/parallels/ix/otg/code/otgen/cmd/run.go:228 +0x342
github.com/open-traffic-generator/otgen/cmd.glob..func13(0xc0001cec00?, {0x1435733?, 0x4?, 0x1435737?})
    /home/parallels/ix/otg/code/otgen/cmd/run.go:70 +0x53
github.com/spf13/cobra.(*Command).execute(0x21e6180, {0xc0000a6780, 0x5, 0x5})
    /home/parallels/go/pkg/mod/github.com/spf13/cobra@v1.7.0/command.go:944 +0x863
github.com/spf13/cobra.(*Command).ExecuteC(0x21e5ea0)
    /home/parallels/go/pkg/mod/github.com/spf13/cobra@v1.7.0/command.go:1068 +0x3a5
github.com/spf13/cobra.(*Command).Execute(...)
    /home/parallels/go/pkg/mod/github.com/spf13/cobra@v1.7.0/command.go:992
github.com/open-traffic-generator/otgen/cmd.Execute()
    /home/parallels/ix/otg/code/otgen/cmd/root.go:57 +0x1a
main.main()
    /home/parallels/ix/otg/code/otgen/main.go:27 +0xf

See the CI test run for details https://github.com/open-traffic-generator/otgen/actions/runs/6601539315/job/17933084738

[Vibaswan]

Hi @bortok basically we made a change in openapiart that all the fields in proto will be made optional, https://github.com/open-traffic-generator/snappi/pull/212 This helps in validating required fields more efficiently as earlier required fields for primitive types were difficult to check for example lets a and required field of integer as we know by default the integer in gosnappi is assigned a value of zero now in order for us to check if user has set a value zero to this field it becomes difficult for us to know whether the required integer field was set by user or not

That's why accessing router id without setting it results in an error as accessing nil pointer, User get to know this cause SDK throws an error if it is not set.

I guess a better way of checking would be this

          for _, d := range config.Devices().Items() {
            if d.HasBgp() {}
          }

we were also thinking to introduce has method for every field i.e to have HasRouterId() for which we have raised a PR (currently in review with @ashutshkumr) but we have not decided on it yet https://github.com/open-traffic-generator/openapiart/pull/454

          for _, d := range config.Devices().Items() {
            if d.Bgp().HasRouterId() != nil {} // as router id is now a string pointer and not a string
           }

[bortok]

Thanks, confirming the fix works