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

Fix handling of optional fields in get_bgp_peer function #3565

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

matofeder
Copy link

@matofeder matofeder commented Oct 3, 2024

What I did

This commit fixes an issue in get_bgp_peer function where local_addr and name were treated as mandatory. However, local_addr and name fields are not required in BGP neighbor configuration.

How I did it

The local_addr and name fields are no longer mandatory in the get_bgp_peer function.

How to verify it

Apply BGP neighbor configuration like below (utilizing BGP_PEER_GROUP), where fields such as local_addr or name are omitted:

{ 
    "BGP_GLOBALS": {
        "default": {
            "local_asn": "65001",
            "log_nbr_state_changes": "true",
            "router_id": "10.0.0.1"
        }
    },
   "BGP_PEER_GROUP": {
        "default|core": {
          "peer_group_name": "core",
          "admin_status": "true",
          "asn": "65002",
          "peer_type": "external"
        }
     },
    "BGP_NEIGHBOR": {
        "default|Ethernet4": {
            "peer_group_name": "core"
        }
    },
    "BGP_NEIGHBOR_AF": {
        "default|core|ipv4_unicast": {
            "admin_status": "true",
            "route_map_in": [
                "ALLOW"
            ],
            "route_map_out": [
                "ALLOW"
            ]
        }
    },
    "ROUTE_MAP": {
        "ALLOW|1": {
            "route_operation": "permit"
        }
    }
}

Previous command output (if the output of a command-line utility has changed)

root@sw1:~# show ip interfaces 
Traceback (most recent call last):
  File "/usr/local/bin/ipintutil", line 280, in <module>
    main()
  File "/usr/local/bin/ipintutil", line 273, in main
    ip_intfs = get_ip_intfs(af, namespace, display)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/bin/ipintutil", line 236, in get_ip_intfs
    ip_intfs_in_ns = get_ip_intfs_in_namespace(af, namespace, display)
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/bin/ipintutil", line 149, in get_ip_intfs_in_namespace
    bgp_peer = get_bgp_peer()
               ^^^^^^^^^^^^^^
  File "/usr/local/bin/ipintutil", line 51, in get_bgp_peer
    local_addr = data[neighbor_ip]['local_addr']
                 ~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^
KeyError: 'local_addr'

New command output (if the output of a command-line utility has changed)

root@sw1:~# show ip interfaces 
Interface    Master    IPv4 address/mask    Admin/Oper    BGP Neighbor    Neighbor IP
-----------  --------  -------------------  ------------  --------------  -------------
Ethernet4              1.1.1.0/31           up/up         N/A             N/A

Fixes #19930

This commit fixes an issue in get_bgp_peer function where
`local_addr` and `name` were treated as mandatory.
However, `local_addr` and `name` fields are not required in
BGP neighbor configuration.

Fixes #19930

Signed-off-by: Matej Feder <matej.feder@dnation.cloud>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Need to add check for local_addr in get_bgp_peer
1 participant