From 4c151f11194c3d012717ce492850f0b38dc91bf3 Mon Sep 17 00:00:00 2001 From: Sonic Build Admin Date: Fri, 17 Oct 2025 18:30:03 +0000 Subject: [PATCH] sonic-yang-mgmt: uses clause with leaf-list, choice not honored #### Why I did it When a uses clause imports a grouping, it was only processing leaf entries and ignoring leaf-list and choice clauses. That means that for instance in bgp route maps, route_map_in and route_map_out validations would fail. Honoring the uses `refine` clause is now also honored which is depended upon in sonic-utilities. This now precompiles the uses clause and integrates it as if the uses clause was not part of the schema as multiple end users were having to do this additional processing. This fixes that behavior and adds test cases to ensure it doesn't regress in the future. This is the proper fix, replacing #21078 that just worked around it. Removal of `uses` logic in sonic-utilities here: https://github.com/sonic-net/sonic-utilities/pull/3814 Fixes https://github.com/sonic-net/sonic-buildimage/issues/22382 ##### Work item tracking - Microsoft ADO **(number only)**: #### How I did it Added leaf-list lookup. #### How to verify it See test cases pass #### Which release branch to backport (provide reason below if selected) - [X] 202405 - [X] 202411 #### Tested branch (Please provide the tested image version) master as of 20250304 #### Description for the changelog sonic-yang-mgmt: uses clause with leaf-list, choice not honored #### Link to config_db schema for YANG module changes N/A #### A picture of a cute animal (not mandatory but encouraged) Signed-off-by: Brad House (@bradh352) --- src/sonic-yang-mgmt/sonic_yang_ext.py | 163 ++++++++++++------ .../tests/files/sample_config_db.json | 4 + .../tests/yang_model_tests/tests/bgp.json | 6 + .../yang_model_tests/tests_config/bgp.json | 112 ++++++++++++ 4 files changed, 234 insertions(+), 51 deletions(-) diff --git a/src/sonic-yang-mgmt/sonic_yang_ext.py b/src/sonic-yang-mgmt/sonic_yang_ext.py index bb81632659..83bce89ded 100644 --- a/src/sonic-yang-mgmt/sonic_yang_ext.py +++ b/src/sonic-yang-mgmt/sonic_yang_ext.py @@ -70,6 +70,8 @@ def loadYangModel(self): self._loadJsonYangModel() # create a map from config DB table to yang container self._createDBTableToModuleMap() + # compile uses clause (embed into schema) + self._compileUsesClause() except Exception as e: self.sysLog(msg="Yang Models Load failed:{}".format(str(e)), \ debug=syslog.LOG_ERR, doPrint=True) @@ -128,8 +130,10 @@ def _preProcessYangGrouping(self, moduleName, module): for grouping in groupings: gName = grouping["@name"] - gLeaf = grouping["leaf"] - self.preProcessedYang['grouping'][moduleName][gName] = gLeaf + self.preProcessedYang['grouping'][moduleName][gName] = dict() + self.preProcessedYang['grouping'][moduleName][gName]["leaf"] = grouping.get('leaf') + self.preProcessedYang['grouping'][moduleName][gName]["leaf-list"] = grouping.get('leaf-list') + self.preProcessedYang['grouping'][moduleName][gName]["choice"] = grouping.get('choice') except Exception as e: self.sysLog(msg="_preProcessYangGrouping failed:{}".format(str(e)), \ @@ -160,14 +164,113 @@ def _preProcessYang(self, moduleName, module): raise e return - """ - Create a map from config DB tables to container in yang model - This module name and topLevelContainer are fetched considering YANG models are - written using below Guidelines: - https://github.com/Azure/SONiC/blob/master/doc/mgmt/SONiC_YANG_Model_Guidelines.md. - """ - def _createDBTableToModuleMap(self): + def _compileUsesClauseRefineApply(self, refine, model): + for item in model: + if item["@name"] == refine["@target-node"]: + # NOTE: We only handle 'description' and 'mandatory' + if refine.get('description') is not None: + item["description"] = refine["description"] + if refine.get('mandatory') is not None: + item["mandatory"] = refine["mandatory"] + return + + def _compileUsesClauseRefine(self, refine, model): + # Nothing to refine + if refine is None: + return + + # Convert to list if not already + if not isinstance(refine, list): + refine = [ refine ] + + # Iterate across each refine + for r in refine: + self._compileUsesClauseRefineApply(r, model) + + def _compileUsesClauseList(self, model, group, name, refine): + # If group doesn't have this entry, nothing to do. + groupobj = group.get(name) + if groupobj is None: + return + + # model doesn't have this entry type, create it as a list + if model.get(name) is None: + model[name] = [] + + # model has this entry type, but its not a list, convert + if not isinstance(model[name], list): + model[name] = [ model[name] ] + + if isinstance(groupobj, list): + model[name].extend(groupobj) + else: + model[name].append(groupobj) + + self._compileUsesClauseRefine(refine, model[name]) + + def _compileUsesClauseModel(self, module, model): + """ + Recursively process the yang schema looking for the "uses" clause under + "container", "list", and "choice"/"case" nodes. Merge in the "uses" + dictionaries for leaf and leaf-list so callers don't need to try to do + their own "uses" processing. Remove the "uses" member when processed so + anyone expecting it won't try to re-process. It will just look like a + yang model that doesn't use "uses" so shouldn't cause compatibility issues. + """ + if isinstance(model, list): + for item in model: + self._compileUsesClauseModel(module, item) + return + + for model_name in [ "container", "list", "choice", "case" ]: + node = model.get(model_name) + if node: + self._compileUsesClauseModel(module, node) + + uses_s = model.get("uses") + if not uses_s: + return + + # Always make as a list + if isinstance(uses_s, dict): + uses_s = [uses_s] + + # uses Example: "@name": "bgpcmn:sonic-bgp-cmn" + for uses in uses_s: + # Assume ':' means reference to another module + if ':' in uses['@name']: + prefix = uses['@name'].split(':')[0].strip() + uses_module_name = self._findYangModuleFromPrefix(prefix, module) + else: + uses_module_name = module['@name'] + grouping = uses['@name'].split(':')[-1].strip() + groupdata = self.preProcessedYang['grouping'][uses_module_name][grouping] + + # Merge leaf from uses + refine = uses.get("refine") + self._compileUsesClauseList(model, groupdata, 'leaf', refine) + self._compileUsesClauseList(model, groupdata, 'leaf-list', refine) + self._compileUsesClauseList(model, groupdata, 'choice', refine) + + # Delete the uses node so callers don't use it. + del model["uses"] + + def _compileUsesClause(self): + try: + for module in self.yJson: + self._compileUsesClauseModel(module["module"], module["module"]) + except Exception as e: + traceback.print_exc() + raise e + + def _createDBTableToModuleMap(self): + """ + Create a map from config DB tables to container in yang model + This module name and topLevelContainer are fetched considering YANG models are + written using below Guidelines: + https://github.com/Azure/SONiC/blob/master/doc/mgmt/SONiC_YANG_Model_Guidelines.md. + """ for j in self.yJson: # get module name moduleName = j['module']['@name'] @@ -320,44 +423,6 @@ def _findYangModuleFromPrefix(self, prefix, module): raise e return None - def _fillLeafDictUses(self, uses_s, table, leafDict): - ''' - Find the leaf(s) in a grouping which maps to given uses statement, - then fill leafDict with leaf(s) information. - - Parameters: - uses_s (str): uses statement in yang module. - table (str): config DB table, this table is being translated. - leafDict (dict): dict with leaf(s) information for List\Container - corresponding to config DB table. - - Returns: - (void) - ''' - try: - # make a list - if isinstance(uses_s, dict): - uses_s = [uses_s] - # find yang module for current table - table_module = self.confDbYangMap[table]['yangModule'] - # uses Example: "@name": "bgpcmn:sonic-bgp-cmn" - for uses in uses_s: - # Assume ':' means reference to another module - if ':' in uses['@name']: - prefix = uses['@name'].split(':')[0].strip() - uses_module_name = self._findYangModuleFromPrefix(prefix, table_module) - else: - uses_module_name = table_module['@name'] - grouping = uses['@name'].split(':')[-1].strip() - leafs = self.preProcessedYang['grouping'][uses_module_name][grouping] - self._fillLeafDict(leafs, leafDict) - except Exception as e: - self.sysLog(msg="_fillLeafDictUses failed:{}".format(str(e)), \ - debug=syslog.LOG_ERR, doPrint=True) - raise e - - return - def _createLeafDict(self, model, table): ''' create a dict to map each key under primary key with a leaf in yang model. @@ -394,10 +459,6 @@ def _createLeafDict(self, model, table): # leaf-lists self._fillLeafDict(model.get('leaf-list'), leafDict, True) - # uses should map to grouping, - if model.get('uses') is not None: - self._fillLeafDictUses(model.get('uses'), table, leafDict) - return leafDict """ diff --git a/src/sonic-yang-models/tests/files/sample_config_db.json b/src/sonic-yang-models/tests/files/sample_config_db.json index fb974b23c5..50524fa974 100644 --- a/src/sonic-yang-models/tests/files/sample_config_db.json +++ b/src/sonic-yang-models/tests/files/sample_config_db.json @@ -1882,6 +1882,8 @@ }, "BGP_NEIGHBOR_AF": { "default|192.168.1.1|ipv4_unicast": { + "route_map_in": [ "map1" ], + "route_map_out": [ "map1" ] } }, "BGP_PEER_GROUP": { @@ -1890,6 +1892,8 @@ }, "BGP_PEER_GROUP_AF": { "default|PG1|ipv4_unicast": { + "route_map_in": [ "map1" ], + "route_map_out": [ "map1" ] } }, "BGP_GLOBALS_LISTEN_PREFIX": { diff --git a/src/sonic-yang-models/tests/yang_model_tests/tests/bgp.json b/src/sonic-yang-models/tests/yang_model_tests/tests/bgp.json index d8ab57f610..9e5241330c 100644 --- a/src/sonic-yang-models/tests/yang_model_tests/tests/bgp.json +++ b/src/sonic-yang-models/tests/yang_model_tests/tests/bgp.json @@ -70,6 +70,9 @@ "eStrKey" : "InvalidValue", "eStr": ["send_community"] }, + "BGP_NEIGHBOR_AF_ROUTE_MAP_VALID": { + "desc": "Reference valid route map" + }, "BGP_NEIGHBOR_AF_NEG_ROUTE_MAP_NOT_EXIST": { "desc": "Reference to non-existent route-map.", "eStrKey" : "LeafRef" @@ -114,6 +117,9 @@ "eStrKey" : "InvalidValue", "eStr": ["send_community"] }, + "BGP_PEERGROUP_AF_ROUTE_MAP_VALID": { + "desc": "Validate route-map reference in peergroup AF." + }, "BGP_PEERGROUP_AF_ROUTE_MAP_NOT_EXIST": { "desc": "Missing or non-existent route-map reference in peergroup AF.", "eStrKey" : "LeafRef" diff --git a/src/sonic-yang-models/tests/yang_model_tests/tests_config/bgp.json b/src/sonic-yang-models/tests/yang_model_tests/tests_config/bgp.json index 4c06316dee..39d9b5b8cc 100644 --- a/src/sonic-yang-models/tests/yang_model_tests/tests_config/bgp.json +++ b/src/sonic-yang-models/tests/yang_model_tests/tests_config/bgp.json @@ -823,6 +823,62 @@ } }, + "BGP_NEIGHBOR_AF_ROUTE_MAP_VALID": { + "sonic-route-map:sonic-route-map": { + "sonic-route-map:ROUTE_MAP_SET": { + "ROUTE_MAP_SET_LIST": [ + { + "name": "ALLOW" + } + ] + }, + "sonic-route-map:ROUTE_MAP": { + "ROUTE_MAP_LIST": [ + { + "name": "ALLOW", + "stmt_name": 1, + "route_operation": "permit" + } + ] + } + }, + "sonic-bgp-global:sonic-bgp-global": { + "sonic-bgp-global:BGP_GLOBALS": { + "BGP_GLOBALS_LIST": [ + { + "vrf_name": "default", + "local_asn": 65001 + } + ] + } + }, + "sonic-bgp-neighbor:sonic-bgp-neighbor": { + "sonic-bgp-neighbor:BGP_NEIGHBOR": { + "BGP_NEIGHBOR_LIST": [ + { + "vrf_name": "default", + "neighbor": "20.0.0.1", + "local_asn": 1, + "name": "BGP nbr 1", + "asn": 65003 + } + ] + }, + "sonic-bgp-neighbor:BGP_NEIGHBOR_AF": { + "BGP_NEIGHBOR_AF_LIST": [ + { + "vrf_name": "default", + "neighbor": "20.0.0.1", + "afi_safi": "ipv4_unicast", + "admin_status": "up", + "route_map_in": [ "ALLOW" ], + "route_map_out": [ "ALLOW" ] + } + ] + } + } + }, + "BGP_NEIGHBOR_AF_NEG_INVALID_MAP_IN": { "sonic-route-map:sonic-route-map": { "sonic-route-map:ROUTE_MAP_SET": { @@ -1181,6 +1237,62 @@ } }, + "BGP_PEERGROUP_AF_ROUTE_MAP_VALID": { + "sonic-route-map:sonic-route-map": { + "sonic-route-map:ROUTE_MAP_SET": { + "ROUTE_MAP_SET_LIST": [ + { + "name": "ALLOW" + } + ] + }, + "sonic-route-map:ROUTE_MAP": { + "ROUTE_MAP_LIST": [ + { + "name": "ALLOW", + "stmt_name": 1, + "route_operation": "permit" + } + ] + } + }, + "sonic-bgp-global:sonic-bgp-global": { + "sonic-bgp-global:BGP_GLOBALS": { + "BGP_GLOBALS_LIST": [ + { + "vrf_name": "default", + "local_asn": 65001 + } + ] + } + }, + "sonic-bgp-peergroup:sonic-bgp-peergroup": { + "sonic-bgp-peergroup:BGP_PEER_GROUP": { + "BGP_PEER_GROUP_LIST": [ + { + "vrf_name": "default", + "peer_group_name": "PG1", + "local_asn": 1, + "name": "BGP PeerGroup 01", + "asn": 65003 + } + ] + }, + "sonic-bgp-peergroup:BGP_PEER_GROUP_AF": { + "BGP_PEER_GROUP_AF_LIST": [ + { + "vrf_name": "default", + "peer_group_name": "PG1", + "afi_safi": "ipv4_unicast", + "admin_status": "up", + "route_map_in": [ "ALLOW" ], + "route_map_out": [ "ALLOW" ] + } + ] + } + } + }, + "BGP_PEERGROUP_AF_ROUTE_MAP_NOT_EXIST": { "sonic-bgp-global:sonic-bgp-global": { "sonic-bgp-global:BGP_GLOBALS": {