Skip to content

Commit a1f8864

Browse files
bradh352bhouse-nexthop
authored andcommitted
sonic-yang-mgmt: uses clause with leaf-list not honored (PR sonic-net#21907)
When a uses clause imports a grouping, it was only processing leaf entries and ignoring leaf-list. That means that for instance in bgp route maps, route_map_in and route_map_out validations would fail. This fixes that behavior and adds test cases to ensure it doesn't regress in the future. Signed-off-by: Brad House (@bradh352)
1 parent db8120f commit a1f8864

File tree

4 files changed

+197
-44
lines changed

4 files changed

+197
-44
lines changed

src/sonic-yang-mgmt/sonic_yang_ext.py

+77-44
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,8 @@ def loadYangModel(self):
7070
self._loadJsonYangModel()
7171
# create a map from config DB table to yang container
7272
self._createDBTableToModuleMap()
73+
# compile uses clause (embed into schema)
74+
self._compileUsesClause()
7375
except Exception as e:
7476
self.sysLog(msg="Yang Models Load failed:{}".format(str(e)), \
7577
debug=syslog.LOG_ERR, doPrint=True)
@@ -128,8 +130,9 @@ def _preProcessYangGrouping(self, moduleName, module):
128130

129131
for grouping in groupings:
130132
gName = grouping["@name"]
131-
gLeaf = grouping["leaf"]
132-
self.preProcessedYang['grouping'][moduleName][gName] = gLeaf
133+
self.preProcessedYang['grouping'][moduleName][gName] = dict()
134+
self.preProcessedYang['grouping'][moduleName][gName]["leaf"] = grouping.get('leaf')
135+
self.preProcessedYang['grouping'][moduleName][gName]["leaf-list"] = grouping.get('leaf-list')
133136

134137
except Exception as e:
135138
self.sysLog(msg="_preProcessYangGrouping failed:{}".format(str(e)), \
@@ -160,6 +163,78 @@ def _preProcessYang(self, moduleName, module):
160163
raise e
161164
return
162165

166+
def _compileUsesClauseList(self, model, group, name):
167+
# If group doesn't have this entry, nothing to do.
168+
if not group.get(name):
169+
return
170+
171+
# model doesn't have this entry type, create it as a list
172+
if not model.get(name):
173+
model[name] = []
174+
175+
# model has this entry type, but its not a list, convert
176+
if not isinstance(model.get(name), list):
177+
model[name] = [ model[name] ]
178+
179+
if isinstance(group.get(name), list):
180+
model[name].extend(group.get(name))
181+
else:
182+
model[name].append(group.get(name))
183+
184+
# Recursively process the yang schema looking for the "uses" clause under
185+
# "container" and "list" nodes. Merge in the "uses" dictionaries for leaf
186+
# and leaf-list so callers don't need to try to do their own "uses"
187+
# processing. Remove the "uses" member when processed so anyone expecting
188+
# it won't try to re-process. It will just look like a yang model that
189+
# doesn't use "uses" so shouldn't cause compatibility issues.
190+
def _compileUsesClauseModel(self, module, model):
191+
if isinstance(model, list):
192+
for item in model:
193+
self._compileUsesClauseModel(module, item)
194+
return
195+
196+
node = model.get("container")
197+
if node:
198+
self._compileUsesClauseModel(module, node)
199+
200+
node = model.get("list")
201+
if node:
202+
self._compileUsesClauseModel(module, node)
203+
204+
uses_s = model.get("uses")
205+
if not uses_s:
206+
return
207+
208+
# Always make as a list
209+
if isinstance(uses_s, dict):
210+
uses_s = [uses_s]
211+
212+
# uses Example: "@name": "bgpcmn:sonic-bgp-cmn"
213+
for uses in uses_s:
214+
# Assume ':' means reference to another module
215+
if ':' in uses['@name']:
216+
prefix = uses['@name'].split(':')[0].strip()
217+
uses_module_name = self._findYangModuleFromPrefix(prefix, module)
218+
else:
219+
uses_module_name = module['@name']
220+
grouping = uses['@name'].split(':')[-1].strip()
221+
groupdata = self.preProcessedYang['grouping'][uses_module_name][grouping]
222+
223+
# Merge leaf from uses
224+
self._compileUsesClauseList(model, groupdata, 'leaf')
225+
self._compileUsesClauseList(model, groupdata, 'leaf-list')
226+
227+
# Delete the uses node so callers don't use it.
228+
del model["uses"]
229+
230+
def _compileUsesClause(self):
231+
try:
232+
for module in self.yJson:
233+
self._compileUsesClauseModel(module["module"], module["module"])
234+
except Exception as e:
235+
traceback.print_exc()
236+
raise e
237+
163238
"""
164239
Create a map from config DB tables to container in yang model
165240
This module name and topLevelContainer are fetched considering YANG models are
@@ -320,44 +395,6 @@ def _findYangModuleFromPrefix(self, prefix, module):
320395
raise e
321396
return None
322397

323-
def _fillLeafDictUses(self, uses_s, table, leafDict):
324-
'''
325-
Find the leaf(s) in a grouping which maps to given uses statement,
326-
then fill leafDict with leaf(s) information.
327-
328-
Parameters:
329-
uses_s (str): uses statement in yang module.
330-
table (str): config DB table, this table is being translated.
331-
leafDict (dict): dict with leaf(s) information for List\Container
332-
corresponding to config DB table.
333-
334-
Returns:
335-
(void)
336-
'''
337-
try:
338-
# make a list
339-
if isinstance(uses_s, dict):
340-
uses_s = [uses_s]
341-
# find yang module for current table
342-
table_module = self.confDbYangMap[table]['yangModule']
343-
# uses Example: "@name": "bgpcmn:sonic-bgp-cmn"
344-
for uses in uses_s:
345-
# Assume ':' means reference to another module
346-
if ':' in uses['@name']:
347-
prefix = uses['@name'].split(':')[0].strip()
348-
uses_module_name = self._findYangModuleFromPrefix(prefix, table_module)
349-
else:
350-
uses_module_name = table_module['@name']
351-
grouping = uses['@name'].split(':')[-1].strip()
352-
leafs = self.preProcessedYang['grouping'][uses_module_name][grouping]
353-
self._fillLeafDict(leafs, leafDict)
354-
except Exception as e:
355-
self.sysLog(msg="_fillLeafDictUses failed:{}".format(str(e)), \
356-
debug=syslog.LOG_ERR, doPrint=True)
357-
raise e
358-
359-
return
360-
361398
def _createLeafDict(self, model, table):
362399
'''
363400
create a dict to map each key under primary key with a leaf in yang model.
@@ -394,10 +431,6 @@ def _createLeafDict(self, model, table):
394431
# leaf-lists
395432
self._fillLeafDict(model.get('leaf-list'), leafDict, True)
396433

397-
# uses should map to grouping,
398-
if model.get('uses') is not None:
399-
self._fillLeafDictUses(model.get('uses'), table, leafDict)
400-
401434
return leafDict
402435

403436
"""

src/sonic-yang-models/tests/files/sample_config_db.json

+2
Original file line numberDiff line numberDiff line change
@@ -1874,6 +1874,8 @@
18741874
},
18751875
"BGP_PEER_GROUP_AF": {
18761876
"default|PG1|ipv4_unicast": {
1877+
"route_map_in": [ "map1" ],
1878+
"route_map_out": [ "map1" ]
18771879
}
18781880
},
18791881
"BGP_GLOBALS_LISTEN_PREFIX": {

src/sonic-yang-models/tests/yang_model_tests/tests/bgp.json

+6
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,9 @@
9090
"eStrKey" : "InvalidValue",
9191
"eStr": ["send_community"]
9292
},
93+
"BGP_NEIGHBOR_AF_ROUTE_MAP_VALID": {
94+
"desc": "Reference valid route map"
95+
},
9396
"BGP_NEIGHBOR_AF_NEG_ROUTE_MAP_NOT_EXIST": {
9497
"desc": "Reference to non-existent route-map.",
9598
"eStrKey" : "LeafRef"
@@ -134,6 +137,9 @@
134137
"eStrKey" : "InvalidValue",
135138
"eStr": ["send_community"]
136139
},
140+
"BGP_PEERGROUP_AF_ROUTE_MAP_VALID": {
141+
"desc": "Validate route-map reference in peergroup AF."
142+
},
137143
"BGP_PEERGROUP_AF_ROUTE_MAP_NOT_EXIST": {
138144
"desc": "Missing or non-existent route-map reference in peergroup AF.",
139145
"eStrKey" : "LeafRef"

src/sonic-yang-models/tests/yang_model_tests/tests_config/bgp.json

+112
Original file line numberDiff line numberDiff line change
@@ -948,6 +948,62 @@
948948
}
949949
},
950950

951+
"BGP_NEIGHBOR_AF_ROUTE_MAP_VALID": {
952+
"sonic-route-map:sonic-route-map": {
953+
"sonic-route-map:ROUTE_MAP_SET": {
954+
"ROUTE_MAP_SET_LIST": [
955+
{
956+
"name": "ALLOW"
957+
}
958+
]
959+
},
960+
"sonic-route-map:ROUTE_MAP": {
961+
"ROUTE_MAP_LIST": [
962+
{
963+
"name": "ALLOW",
964+
"stmt_name": 1,
965+
"route_operation": "permit"
966+
}
967+
]
968+
}
969+
},
970+
"sonic-bgp-global:sonic-bgp-global": {
971+
"sonic-bgp-global:BGP_GLOBALS": {
972+
"BGP_GLOBALS_LIST": [
973+
{
974+
"vrf_name": "default",
975+
"local_asn": 65001
976+
}
977+
]
978+
}
979+
},
980+
"sonic-bgp-neighbor:sonic-bgp-neighbor": {
981+
"sonic-bgp-neighbor:BGP_NEIGHBOR": {
982+
"BGP_NEIGHBOR_LIST": [
983+
{
984+
"vrf_name": "default",
985+
"neighbor": "20.0.0.1",
986+
"local_asn": 1,
987+
"name": "BGP nbr 1",
988+
"asn": 65003
989+
}
990+
]
991+
},
992+
"sonic-bgp-neighbor:BGP_NEIGHBOR_AF": {
993+
"BGP_NEIGHBOR_AF_LIST": [
994+
{
995+
"vrf_name": "default",
996+
"neighbor": "20.0.0.1",
997+
"afi_safi": "ipv4_unicast",
998+
"admin_status": "up",
999+
"route_map_in": [ "ALLOW" ],
1000+
"route_map_out": [ "ALLOW" ]
1001+
}
1002+
]
1003+
}
1004+
}
1005+
},
1006+
9511007
"BGP_NEIGHBOR_AF_NEG_INVALID_MAP_IN": {
9521008
"sonic-route-map:sonic-route-map": {
9531009
"sonic-route-map:ROUTE_MAP_SET": {
@@ -1306,6 +1362,62 @@
13061362
}
13071363
},
13081364

1365+
"BGP_PEERGROUP_AF_ROUTE_MAP_VALID": {
1366+
"sonic-route-map:sonic-route-map": {
1367+
"sonic-route-map:ROUTE_MAP_SET": {
1368+
"ROUTE_MAP_SET_LIST": [
1369+
{
1370+
"name": "ALLOW"
1371+
}
1372+
]
1373+
},
1374+
"sonic-route-map:ROUTE_MAP": {
1375+
"ROUTE_MAP_LIST": [
1376+
{
1377+
"name": "ALLOW",
1378+
"stmt_name": 1,
1379+
"route_operation": "permit"
1380+
}
1381+
]
1382+
}
1383+
},
1384+
"sonic-bgp-global:sonic-bgp-global": {
1385+
"sonic-bgp-global:BGP_GLOBALS": {
1386+
"BGP_GLOBALS_LIST": [
1387+
{
1388+
"vrf_name": "default",
1389+
"local_asn": 65001
1390+
}
1391+
]
1392+
}
1393+
},
1394+
"sonic-bgp-peergroup:sonic-bgp-peergroup": {
1395+
"sonic-bgp-peergroup:BGP_PEER_GROUP": {
1396+
"BGP_PEER_GROUP_LIST": [
1397+
{
1398+
"vrf_name": "default",
1399+
"peer_group_name": "PG1",
1400+
"local_asn": 1,
1401+
"name": "BGP PeerGroup 01",
1402+
"asn": 65003
1403+
}
1404+
]
1405+
},
1406+
"sonic-bgp-peergroup:BGP_PEER_GROUP_AF": {
1407+
"BGP_PEER_GROUP_AF_LIST": [
1408+
{
1409+
"vrf_name": "default",
1410+
"peer_group_name": "PG1",
1411+
"afi_safi": "ipv4_unicast",
1412+
"admin_status": "up",
1413+
"route_map_in": [ "ALLOW" ],
1414+
"route_map_out": [ "ALLOW" ]
1415+
}
1416+
]
1417+
}
1418+
}
1419+
},
1420+
13091421
"BGP_PEERGROUP_AF_ROUTE_MAP_NOT_EXIST": {
13101422
"sonic-bgp-global:sonic-bgp-global": {
13111423
"sonic-bgp-global:BGP_GLOBALS": {

0 commit comments

Comments
 (0)