Skip to content

Commit d7953d2

Browse files
authored
[GCU] Validate peer_group_range ip_range are correct (sonic-net#2145)
#### What I did Fixes sonic-net#2119 #### Previous command output (if the output of a command-line utility has changed) Sorting output: ``` Patch Applier: Applying 4 changes in order: Patch Applier: * [{"op": "remove", "path": "/BGP_PEER_RANGE/BGPSLBPassiveV6"}] Patch Applier: * [{"op": "add", "path": "/BGP_PEER_RANGE/BGPVac", "value": {"ip_range": ["192.168.0.0/21"], "name": "BGPVac", "src_address": "10.1.0.32"}}] Patch Applier: * [{"op": "remove", "path": "/BGP_PEER_RANGE/BGPSLBPassive"}] Patch Applier: * [{"op": "add", "path": "/BGP_PEER_RANGE/BGPSLBPassive", "value": {"ip_range": ["10.255.0.0/25"], "name": "BGPSLBPassive", "src_address": "10.1.0.32"}}] ``` #### New command output (if the output of a command-line utility has changed) Sorting output: ``` Patch Applier: Applying 4 changes in order: Patch Applier: * [{"op": "remove", "path": "/BGP_PEER_RANGE/BGPSLBPassiveV6"}] Patch Applier: * [{"op": "remove", "path": "/BGP_PEER_RANGE"}] Patch Applier: * [{"op": "add", "path": "/BGP_PEER_RANGE", "value": {"BGPSLBPassive": {"ip_range": ["10.255.0.0/25"], "name": "BGPSLBPassive", "src_address": "10.1.0.32"}}}] Patch Applier: * [{"op": "add", "path": "/BGP_PEER_RANGE/BGPVac", "value": {"ip_range": ["192.168.0.0/21"], "name": "BGPVac", "src_address": "10.1.0.32"}}] ```
1 parent aa81b97 commit d7953d2

File tree

2 files changed

+98
-1
lines changed

2 files changed

+98
-1
lines changed

generic_config_updater/gu_common.py

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,10 +118,33 @@ def validate_config_db_config(self, config_db_as_json):
118118
sy.loadData(tmp_config_db_as_json)
119119

120120
sy.validate_data_tree()
121-
return True, None
121+
122+
# TODO: modularize custom validations better or move directly to sonic-yang module
123+
return self.validate_bgp_peer_group(config_db_as_json)
122124
except sonic_yang.SonicYangException as ex:
123125
return False, ex
124126

127+
def validate_bgp_peer_group(self, config_db):
128+
if "BGP_PEER_RANGE" not in config_db:
129+
return True, None
130+
131+
visited = {}
132+
table = config_db["BGP_PEER_RANGE"]
133+
for peer_group_name in table:
134+
peer_group = table[peer_group_name]
135+
if "ip_range" not in peer_group:
136+
continue
137+
138+
# TODO: convert string to IpAddress object for better handling of IPs
139+
# TODO: validate range intersection
140+
ip_range = peer_group["ip_range"]
141+
for ip in ip_range:
142+
if ip in visited:
143+
return False, f"{ip} is duplicated in BGP_PEER_RANGE: {set([peer_group_name, visited[ip]])}"
144+
visited[ip] = peer_group_name
145+
146+
return True, None
147+
125148
def crop_tables_without_yang(self, config_db_as_json):
126149
sy = self.create_sonic_yang_with_loaded_models()
127150

tests/generic_config_updater/gu_common_test.py

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,80 @@ def test_validate_config_db_config__invalid_config__returns_false(self):
186186
self.assertEqual(expected, actual)
187187
self.assertIsNotNone(error)
188188

189+
def test_validate_bgp_peer_group__valid_non_intersecting_ip_ranges__returns_true(self):
190+
# Arrange
191+
config_wrapper = gu_common.ConfigWrapper()
192+
config = {
193+
"BGP_PEER_RANGE":
194+
{
195+
"BGPSLBPassive": {
196+
"ip_range": ["1.1.1.1/31", "10.10.10.10/16", "100.100.100.100/24"]
197+
},
198+
"BgpVac": {
199+
"ip_range": ["2.2.2.2/31", "20.20.20.20/16", "200.200.200.200/24"]
200+
}
201+
}
202+
}
203+
204+
# Act
205+
actual, error = config_wrapper.validate_bgp_peer_group(config)
206+
207+
# Assert
208+
self.assertTrue(actual)
209+
self.assertIsNone(error)
210+
211+
def test_validate_bgp_peer_group__same_ip_prefix__return_false(self):
212+
# duplicate v4 within same ip_range
213+
self.check_validate_bgp_peer_group(
214+
["1.1.1.1/16", "1.1.1.1/16"],
215+
duplicated_ip="1.1.1.1/16")
216+
# duplicate v4 within different ip_ranges
217+
self.check_validate_bgp_peer_group(
218+
["1.1.1.1/16"],
219+
["1.1.1.1/16"],
220+
duplicated_ip="1.1.1.1/16")
221+
# duplicate v4 within different ip_ranges, but many ips
222+
self.check_validate_bgp_peer_group(
223+
["1.1.1.1/16", "1.1.1.1/31", "10.10.10.10/16", "100.100.100.100/24"],
224+
["2.2.2.2/31", "20.20.20.20/16", "200.200.200.200/24", "1.1.1.1/16"],
225+
duplicated_ip="1.1.1.1/16")
226+
# duplicate v6 within same ip_range
227+
self.check_validate_bgp_peer_group(
228+
["fc00:1::32/16", "fc00:1::32/16"],
229+
duplicated_ip="fc00:1::32/16")
230+
# duplicate v6 within different ip_ranges
231+
self.check_validate_bgp_peer_group(
232+
["fc00:1::32/16"],
233+
["fc00:1::32/16"],
234+
duplicated_ip="fc00:1::32/16")
235+
# duplicate v6 within different ip_ranges, but many ips
236+
self.check_validate_bgp_peer_group(
237+
["fc00:1::32/16", "fc00:1::32/31", "10:1::1/16", "100:1::1/24"],
238+
["2:1::1/31", "20:1::1/16", "200:1::1/24", "fc00:1::32/16"],
239+
duplicated_ip="fc00:1::32/16")
240+
241+
def check_validate_bgp_peer_group(self, ip_range, other_ip_range=[], duplicated_ip=None):
242+
# Arrange
243+
config_wrapper = gu_common.ConfigWrapper()
244+
config = {
245+
"BGP_PEER_RANGE":
246+
{
247+
"BGPSLBPassive": {
248+
"ip_range": ip_range
249+
},
250+
"BgpVac": {
251+
"ip_range": other_ip_range
252+
},
253+
}
254+
}
255+
256+
# Act
257+
actual, error = config_wrapper.validate_bgp_peer_group(config)
258+
259+
# Assert
260+
self.assertFalse(actual)
261+
self.assertTrue(duplicated_ip in error)
262+
189263
def test_crop_tables_without_yang__returns_cropped_config_db_as_json(self):
190264
# Arrange
191265
config_wrapper = gu_common.ConfigWrapper()

0 commit comments

Comments
 (0)