Skip to content

sonic-cfggen should return boolean values as lowercase true and false #22526

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.

Sign up for GitHub

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

Open
bhouse-nexthop wants to merge 1 commit into
base: master
Choose a base branch
from
Open

sonic-cfggen should return boolean values as lowercase true and false #22526

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 30 additions & 3 deletions src/sonic-config-engine/sonic-cfggen
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import sys
import yaml
import ipaddress
import base64
import sonic_yang
Copy link
Copy Markdown
Collaborator

@qiluo-msft qiluo-msft Jun 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sonic_yang

config-cfggen currently does not depend on sonic_yang. It is used in some critical time window and need high performance, for example during warm-boot.

if introducing sonic_yang, the runtime speed will hurt a lot.

Copy link
Copy Markdown
Collaborator Author

@bhouse-nexthop bhouse-nexthop Jun 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have measurable numbers to show this to be the case?

The prior solution I presented wasn't sufficient for you because it would convert potentially string values into boolean values.

You suggested I only handle values that were stored as booleans in CONFIG_DB to output 'true' / 'false', but there were 2 issues with that:

  1. Sometimes in "FEATURES" the config_db value is stored as a boolean, but it really wants True or False (you can see that from a test case I added here). This is fixed in config_db should not mandate True and False for featured sonic-host-services#252 that you just merged, thank you.
  2. Sometimes in config_db the value is stored as a string of "True" or "False" but needs to be output as 'true' or 'false', as is the case with Management VRF enablement that this ticket was created for.

Shall I go back to my original PR where I just properly case-convert all string and boolean values that case-insensitively match "true" and "false" to their lowercase variants regardless of if they are boolean or string?

Without using the YANG models to know for sure the proper datatype, there is really no other option. I tried to limit the exposure on this by just loading the yang models once and only validating when necessary, the overhead in theory should be very low with this methodology in the PR as it currently sits.

Copy link
Copy Markdown
Collaborator Author

@bhouse-nexthop bhouse-nexthop Jun 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@qiluo-msft need your guidance to move forward here since no solution I've presented has met your requirements.


from collections import OrderedDict
from config_samples import generate_sample_config, get_available_config
Expand All @@ -39,6 +40,7 @@ from swsscommon.swsscommon import ConfigDBConnector, SonicDBConfig, ConfigDBPipe
from asic_sensors_config import get_asic_sensors_config

PY3x = sys.version_info >= (3, 0)
YANG_MODELS = "/usr/local/yang-models"

# TODO: Remove STR_TYPE, FILE_TYPE once SONiC moves to Python 3.x
# TODO: Remove the import SonicYangCfgDbGenerator once SONiC moves to python3.x
Expand Down Expand Up @@ -212,7 +214,13 @@ TODO(taoyl): Current version of config db only supports BGP admin states.
return db_data

@staticmethod
def to_serialized(data, lookup_key = None):
def to_serialized(data, lookup_key = None, sy = None, path_tokens = None):
if path_tokens is None:
path_tokens = []
if sy is None:
sy = sonic_yang.SonicYang(YANG_MODELS, print_log_enabled = False)
sy.loadYangModel()

if type(data) is dict:
if lookup_key is not None:
newData = {}
Expand All @@ -227,7 +235,25 @@ TODO(taoyl): Current version of config db only supports BGP admin states.
new_key = ConfigDBConnector.serialize_key(key)
if new_key != key:
data[new_key] = data.pop(key)
data[new_key] = FormatConverter.to_serialized(data[new_key])
path_tokens.append(new_key)
data[new_key] = FormatConverter.to_serialized(data[new_key], sy = sy, path_tokens = path_tokens)
path_tokens.pop()

if isinstance(data, bool):
data = 'true' if data else 'false'

# If we have a string that we know looks like true/false, but differs by
# Case, look up from the yang schema to see if we should convert into
# a properly formatted boolean for yang.
if isinstance(data, str) and data.lower() in [ "true", "false" ] and data not in [ "true", "false" ]:
try:
schema_xpath = sy.configdb_path_to_xpath(sy.configdb_path_join(path_tokens), schema_xpath = True)
datatype = sy.get_data_type(schema_xpath)
if datatype == "BOOL":
data = 'true' if data.lower() == "true" else 'false'
except:
pass

return data

@staticmethod
Expand Down Expand Up @@ -504,7 +530,8 @@ def main():

if args.var_json is not None and args.var_json in data:
if args.key is not None:
print(json.dumps(FormatConverter.to_serialized(data[args.var_json], args.key), indent=4, cls=minigraph_encoder))
print(json.dumps(FormatConverter.to_serialized(data[args.var_json], args.key), indent=4,
cls=minigraph_encoder))
else:
print(json.dumps(FormatConverter.to_serialized(data[args.var_json]), indent=4, cls=minigraph_encoder))

Expand Down
20 changes: 20 additions & 0 deletions src/sonic-config-engine/tests/test_cfggen.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,26 @@ def test_additional_json_data_level2_key(self):
output = self.run_script(argument)
self.assertEqual(utils.to_dict(output.strip()), utils.to_dict('{\n "k11": "v11"\n}'))

def test_boolean_case_1(self):
# Detect boolean field and value isn't of proper YANG case
argument = ['-a', '{"MGMT_VRF_CONFIG":{"vrf_global":{"mgmtVrfEnabled":"TRUe"}}}', '--print-data']
output = self.run_script(argument)
self.assertEqual(utils.to_dict(output.strip()), utils.to_dict('{"MGMT_VRF_CONFIG":{"vrf_global":{"mgmtVrfEnabled":"true"}}}'))

def test_boolean_case_2(self):
# Detect field is passed as boolean, make sure it doesn't translate into Python "True"
argument = ['-a', '{"MGMT_VRF_CONFIG":{"vrf_global":{"mgmtVrfEnabled":true}}}', '--print-data']
output = self.run_script(argument)
self.assertEqual(utils.to_dict(output.strip()), utils.to_dict('{"MGMT_VRF_CONFIG":{"vrf_global":{"mgmtVrfEnabled":"true"}}}'))

def test_boolean_case_3(self):
# Field looks like a boolean, but its really a string field in YANG, nothing should actually be converted
# in this string, note the pythonic True and False with capital first letters.
data = '{"FEATURE":{"bgp":{"auto_restart":"enabled","check_up_status":"false","has_global_scope":"false","has_per_asic_scope":"true","delayed":"false","has_global_scope":"False","has_per_asic_scope":"True","delayed":"False","high_mem_alert":"disabled","set_owner":"local","state":"enabled"}}}'
argument = ['-a', data, '--print-data']
output = self.run_script(argument)
self.assertEqual(utils.to_dict(output.strip()), utils.to_dict(data))

def test_var_json_data(self, **kwargs):
graph_file = kwargs.get('graph_file', self.sample_graph_simple)
tag_mode = kwargs.get('tag_mode', 'untagged')
Expand Down
4 changes: 2 additions & 2 deletions src/sonic-config-engine/tests/test_j2files.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ def test_t1_smartswitch_template(self):
with open(sample_output_file) as sample_output_fd:
sample_output_json = json.load(sample_output_fd)

self.assertTrue(json.dumps(sample_output_json, sort_keys=True) == json.dumps(output_json, sort_keys=True))
self.assertEqual(json.dumps(sample_output_json, sort_keys=True), json.dumps(output_json, sort_keys=True))

def test_t1_smartswitch_dpu_template(self):
argument = ['-k', 'SS-DPU-1x400Gb', '--preset', 't1-smartswitch', '-p', self.t1_ss_dpu_port_config]
Expand All @@ -347,7 +347,7 @@ def test_t1_smartswitch_dpu_template(self):
with open(sample_output_file) as sample_output_fd:
sample_output_json = json.load(sample_output_fd)

self.assertTrue(json.dumps(sample_output_json, sort_keys=True) == json.dumps(output_json, sort_keys=True))
self.assertEqual(json.dumps(sample_output_json, sort_keys=True), json.dumps(output_json, sort_keys=True))

def test_qos_arista7050_render_template(self):
self._test_qos_render_template('arista', 'x86_64-arista_7050_qx32s', 'Arista-7050-QX-32S', 'sample-arista-7050-t0-minigraph.xml', 'qos-arista7050.json')
Expand Down
91 changes: 62 additions & 29 deletions src/sonic-yang-mgmt/sonic_yang.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,30 @@ def __init__(self, yang_dir, debug=False, print_log_enabled=True, sonic_yang_opt
# element path for CONFIG DB. An example for this list could be:
# ['PORT', 'Ethernet0', 'speed']
self.elementPath = []
self.mapped_types = {
"DER":ly.LY_TYPE_DER,
"BINARY":ly.LY_TYPE_BINARY,
"BITS":ly.LY_TYPE_BITS,
"BOOL":ly.LY_TYPE_BOOL,
"DEC64":ly.LY_TYPE_DEC64,
"EMPTY":ly.LY_TYPE_EMPTY,
"ENUM":ly.LY_TYPE_ENUM,
"IDENT":ly.LY_TYPE_IDENT,
"INST":ly.LY_TYPE_INST,
"LEAFREF":ly.LY_TYPE_LEAFREF,
"STRING":ly.LY_TYPE_STRING,
"UNION":ly.LY_TYPE_UNION,
"INT8":ly.LY_TYPE_INT8,
"UINT8":ly.LY_TYPE_UINT8,
"INT16":ly.LY_TYPE_INT16,
"UINT16":ly.LY_TYPE_UINT16,
"INT32":ly.LY_TYPE_INT32,
"UINT32":ly.LY_TYPE_UINT32,
"INT64":ly.LY_TYPE_INT64,
"UINT64":ly.LY_TYPE_UINT64,
"UNKNOWN":ly.LY_TYPE_UNKNOWN
}

try:
self.ctx = ly.Context(yang_dir, sonic_yang_options)
except Exception as e:
Expand Down Expand Up @@ -686,48 +710,40 @@ def _get_module_prefix(self, module_name):
output: type
"""
def _str_to_type(self, type_str):
mapped_type = {
"LY_TYPE_DER":ly.LY_TYPE_DER,
"LY_TYPE_BINARY":ly.LY_TYPE_BINARY,
"LY_TYPE_BITS":ly.LY_TYPE_BITS,
"LY_TYPE_BOOL":ly.LY_TYPE_BOOL,
"LY_TYPE_DEC64":ly.LY_TYPE_DEC64,
"LY_TYPE_EMPTY":ly.LY_TYPE_EMPTY,
"LY_TYPE_ENUM":ly.LY_TYPE_ENUM,
"LY_TYPE_IDENT":ly.LY_TYPE_IDENT,
"LY_TYPE_INST":ly.LY_TYPE_INST,
"LY_TYPE_LEAFREF":ly.LY_TYPE_LEAFREF,
"LY_TYPE_STRING":ly.LY_TYPE_STRING,
"LY_TYPE_UNION":ly.LY_TYPE_UNION,
"LY_TYPE_INT8":ly.LY_TYPE_INT8,
"LY_TYPE_UINT8":ly.LY_TYPE_UINT8,
"LY_TYPE_INT16":ly.LY_TYPE_INT16,
"LY_TYPE_UINT16":ly.LY_TYPE_UINT16,
"LY_TYPE_INT32":ly.LY_TYPE_INT32,
"LY_TYPE_UINT32":ly.LY_TYPE_UINT32,
"LY_TYPE_INT64":ly.LY_TYPE_INT64,
"LY_TYPE_UINT64":ly.LY_TYPE_UINT64,
"LY_TYPE_UNKNOWN":ly.LY_TYPE_UNKNOWN
}

if type_str not in mapped_type:
return ly.LY_TYPE_UNKNOWN

return mapped_type[type_str]
if type_str not in self.mapped_types:
return ly.LY_TYPE_UNKNOWN

return self.mapped_types[type_str]

def _type_to_str(self, type_int):
name=""
try:
name = list(self.mapped_types.keys())[list(self.mapped_types.values()).index(type_int)]
except ValueError:
name = "UNKNOWN"

return name

def _get_data_type(self, schema_xpath):
try:
schema_node = self._find_schema_node(schema_xpath)
except Exception as e:
self.sysLog(msg="get_data_type(): Failed to find schema node from xpath: {}".format(schema_xpath), debug=syslog.LOG_ERR, doPrint=True)
self.fail(e)
return None

if (schema_node is not None):
return schema_node.subtype().type().base()

return ly.LY_TYPE_UNKNOWN

"""
get_data_type: Retrieve the data type of the node
input: schema_xpath
output: string representing type of node this points to
"""
def get_data_type(self, schema_xpath):
return self._type_to_str(self._get_data_type(schema_xpath))

"""
get_leafref_type: find the type of node that leafref references to
input: data_xpath - xpath of a data node
Expand All @@ -746,6 +762,14 @@ def _get_leafref_type(self, data_xpath):

return ly.LY_TYPE_UNKNOWN

"""
get_leafref_type: find the type of node that leafref references to
input: data_xpath - xpath of a data node
output: string representing type of node this points to
"""
def get_leafref_type(self, data_xpath):
return self._type_to_str(self._get_leafref_type(data_xpath))

"""
get_leafref_path(): find the leafref path
input: schema_xpath - xpath of a schema node
Expand Down Expand Up @@ -783,3 +807,12 @@ def _get_leafref_type_schema(self, schema_xpath):
return target_type

return None

"""
get_leafref_type_schema: find the type of node that leafref references to
input: schema_xpath - xpath of a data node
output: string representing type of node this points to
"""
def get_leafref_type_schema(self, schema_xpath):
return self._type_to_str(self._get_leafref_type_schema(schema_xpath))

20 changes: 10 additions & 10 deletions src/sonic-yang-mgmt/tests/libyang-python-tests/test_SonicYang.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@
],
"data_type" : [
{
"data_type" : "LY_TYPE_STRING",
"data_type" : "STRING",
"xpath" : "/test-port:test-port/test-port:PORT/test-port:PORT_LIST/test-port:port_name"
},
{
"data_type" : "LY_TYPE_LEAFREF",
"data_type" : "LEAFREF",
"xpath" : "/test-vlan:test-vlan/test-vlan:VLAN_INTERFACE/test-vlan:VLAN_INTERFACE_LIST/test-vlan:vlanid"
}
],
Expand Down Expand Up @@ -81,37 +81,37 @@
],
"leafref_type" : [
{
"data_type" : "LY_TYPE_UINT16",
"data_type" : "UINT16",
"xpath" : "/test-vlan:test-vlan/VLAN_INTERFACE/VLAN_INTERFACE_LIST[vlanid='111'][ip-prefix='2000:f500:45:6709::/64']/vlanid"
},
{
"data_type" : "LY_TYPE_STRING",
"data_type" : "STRING",
"xpath" : "/test-interface:test-interface/INTERFACE/INTERFACE_LIST[interface='Ethernet8'][ip-prefix='2000:f500:40:a749::/126']/interface"
},
{
"data_type" : "LY_TYPE_STRING",
"data_type" : "STRING",
"xpath" : "/test-vlan:test-vlan/VLAN_MEMBER/VLAN_MEMBER_LIST[vlanid='111'][port='Ethernet0']/port"
},
{
"data_type" : "LY_TYPE_UINT16",
"data_type" : "UINT16",
"xpath" : "/test-vlan:test-vlan/VLAN_MEMBER/VLAN_MEMBER_LIST[vlanid='111'][port='Ethernet0']/vlanid"
}
],
"leafref_type_schema" : [
{
"data_type" : "LY_TYPE_UINT16",
"data_type" : "UINT16",
"xpath" : "/test-vlan:test-vlan/test-vlan:VLAN_INTERFACE/test-vlan:VLAN_INTERFACE_LIST/test-vlan:vlanid"
},
{
"data_type" : "LY_TYPE_STRING",
"data_type" : "STRING",
"xpath" : "/test-interface:test-interface/test-interface:INTERFACE/test-interface:INTERFACE_LIST/test-interface:interface"
},
{
"data_type" : "LY_TYPE_STRING",
"data_type" : "STRING",
"xpath" : "/test-vlan:test-vlan/test-vlan:VLAN_MEMBER/test-vlan:VLAN_MEMBER_LIST/test-vlan:port"
},
{
"data_type" : "LY_TYPE_UINT16",
"data_type" : "UINT16",
"xpath" : "/test-vlan:test-vlan/test-vlan:VLAN_MEMBER/test-vlan:VLAN_MEMBER_LIST/test-vlan:vlanid"
}
],
Expand Down
15 changes: 6 additions & 9 deletions src/sonic-yang-mgmt/tests/libyang-python-tests/test_sonic_yang.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,17 +233,15 @@ def test_get_module_prefix(self, yang_s, data):
def test_get_data_type(self, yang_s, data):
for node in data['data_type']:
xpath = str(node['xpath'])
expected = node['data_type']
expected_type = yang_s._str_to_type(expected)
data_type = yang_s._get_data_type(xpath)
expected_type = node['data_type']
data_type = yang_s.get_data_type(xpath)
assert expected_type == data_type

def test_get_leafref_type(self, yang_s, data):
for node in data['leafref_type']:
xpath = str(node['xpath'])
expected = node['data_type']
expected_type = yang_s._str_to_type(expected)
data_type = yang_s._get_leafref_type(xpath)
expected_type = node['data_type']
data_type = yang_s.get_leafref_type(xpath)
assert expected_type == data_type

def test_get_leafref_path(self, yang_s, data):
Expand All @@ -256,9 +254,8 @@ def test_get_leafref_path(self, yang_s, data):
def test_get_leafref_type_schema(self, yang_s, data):
for node in data['leafref_type_schema']:
xpath = str(node['xpath'])
expected = node['data_type']
expected_type = yang_s._str_to_type(expected)
data_type = yang_s._get_leafref_type_schema(xpath)
expected_type = node['data_type']
data_type = yang_s.get_leafref_type_schema(xpath)
assert expected_type == data_type

def test_configdb_path_to_xpath(self, yang_s, data):
Expand Down
Loading