From 2a8c285a88ffee2a560719849769cbd544c44d94 Mon Sep 17 00:00:00 2001 From: Ubuntu Date: Mon, 2 May 2022 21:17:05 +0000 Subject: [PATCH 1/5] Add CLI to configure YANG config validatoin --- config/main.py | 15 +++++++++++++ tests/yang_config_validation_test.py | 33 ++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) create mode 100644 tests/yang_config_validation_test.py diff --git a/config/main.py b/config/main.py index ed38bcc841..e126ff8bc3 100644 --- a/config/main.py +++ b/config/main.py @@ -1714,6 +1714,21 @@ def synchronous_mode(sync_mode): else: raise click.BadParameter("Error: Invalid argument %s, expect either enable or disable" % sync_mode) +# +# 'yang_config_validation' command ('config yang_config_validation ...') +# +@config.command('yang_config_validation') +@click.argument('yang_config_validation', metavar='', required=True) +def yang_config_validation(yang_config_validation): + # Enable or disable YANG validation on updates to ConfigDB + if yang_config_validation == 'enable' or yang_config_validation == 'disable': + config_db = ConfigDBConnector() + config_db.connect() + config_db.mod_entry('DEVICE_METADATA', 'localhost', {"yang_config_validation": yang_config_validation}) + click.echo("""Wrote %s yang config validation into CONFIG_DB""" % yang_config_validation) + else: + raise click.BadParameter("Error: Invalid argument %s, expect either enable or disable" % yang_config_validation) + # # 'portchannel' group ('config portchannel ...') # diff --git a/tests/yang_config_validation_test.py b/tests/yang_config_validation_test.py new file mode 100644 index 0000000000..37b9e448cc --- /dev/null +++ b/tests/yang_config_validation_test.py @@ -0,0 +1,33 @@ +from click.testing import CliRunner +import config.main as config + +class TestYangConfigValidation(object): + @classmethod + def setup_class(cls): + print("SETUP") + + def __check_result(self, result_msg, mode): + if mode == "enable" or mode == "disable": + expected_msg = """Wrote %s yang config validation into CONFIG_DB""" % mode + else: + expected_msg = "Error: Invalid argument %s, expect either enable or disable" % mode + + return expected_msg in result_msg + + def test_yang_config_validation(self): + runner = CliRunner() + + result = runner.invoke(config.config.commands["yang_config_validation"], ["enable"]) + print(result.output) + assert result.exit_code == 0 + assert self.__check_result(result.output, "enable") + + result = runner.invoke(config.config.commands["yang_config_validation"], ["disable"]) + print(result.output) + assert result.exit_code == 0 + assert self.__check_result(result.output, "disable") + + result = runner.invoke(config.config.commands["yang_config_validation"], ["invalid-input"]) + print(result.output) + assert result.exit_code != 0 + assert self.__check_result(result.output, "invalid-input") From 11335b2d08b2d52fbec310d5f0f5d5d49ba9266f Mon Sep 17 00:00:00 2001 From: Ubuntu Date: Tue, 10 May 2022 10:45:37 +0000 Subject: [PATCH 2/5] add yang validation service --- config/main.py | 22 +++++++------ config/yang_validation_service.py | 51 +++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 10 deletions(-) create mode 100644 config/yang_validation_service.py diff --git a/config/main.py b/config/main.py index e126ff8bc3..56066439be 100644 --- a/config/main.py +++ b/config/main.py @@ -30,6 +30,7 @@ from utilities_common.general import load_db_config from .utils import log +from .yang_validation_service import YangValidationService from . import aaa from . import chassis_modules @@ -1753,10 +1754,6 @@ def portchannel(ctx, namespace): @click.pass_context def add_portchannel(ctx, portchannel_name, min_links, fallback): """Add port channel""" - if is_portchannel_name_valid(portchannel_name) != True: - ctx.fail("{} is invalid!, name should have prefix '{}' and suffix '{}'" - .format(portchannel_name, CFG_PORTCHANNEL_PREFIX, CFG_PORTCHANNEL_NO)) - db = ctx.obj['db'] if is_portchannel_present_in_db(db, portchannel_name): @@ -1769,17 +1766,18 @@ def add_portchannel(ctx, portchannel_name, min_links, fallback): fvs['min_links'] = str(min_links) if fallback != 'false': fvs['fallback'] = 'true' - db.set_entry('PORTCHANNEL', portchannel_name, fvs) + yvs = YangValidationService() + if not yvs.validate_set_entry('PORTCHANNEL', portchannel_name, fvs): + ctx.fail("Invalid configuration based on PortChannel YANG model") + else: + db.set_entry('PORTCHANNEL', portchannel_name, fvs) + @portchannel.command('del') @click.argument('portchannel_name', metavar='', required=True) @click.pass_context def remove_portchannel(ctx, portchannel_name): """Remove port channel""" - if is_portchannel_name_valid(portchannel_name) != True: - ctx.fail("{} is invalid!, name should have prefix '{}' and suffix '{}'" - .format(portchannel_name, CFG_PORTCHANNEL_PREFIX, CFG_PORTCHANNEL_NO)) - db = ctx.obj['db'] # Dont proceed if the port channel does not exist @@ -1789,7 +1787,11 @@ def remove_portchannel(ctx, portchannel_name): if len([(k, v) for k, v in db.get_table('PORTCHANNEL_MEMBER') if k == portchannel_name]) != 0: click.echo("Error: Portchannel {} contains members. Remove members before deleting Portchannel!".format(portchannel_name)) else: - db.set_entry('PORTCHANNEL', portchannel_name, None) + yvs = YangValidationService() + if not yvs.validate_set_entry('PORTCHANNEL', portchannel_name, None): + ctx.fail("Invalid configuration based on PortChannel YANG model") + else: + db.set_entry('PORTCHANNEL', portchannel_name, None) @portchannel.group(cls=clicommon.AbbreviationGroup, name='member') @click.pass_context diff --git a/config/yang_validation_service.py b/config/yang_validation_service.py new file mode 100644 index 0000000000..37d3bd2bc7 --- /dev/null +++ b/config/yang_validation_service.py @@ -0,0 +1,51 @@ +import json +import sonic_yang +import subprocess +import copy + +YANG_DIR = "/usr/local/yang-models" + +class YangValidationService: + def __init__(self, yang_dir = YANG_DIR): + self.yang_dir = YANG_DIR + self.sonic_yang_with_loaded_models = None + + def get_config_db_as_json(self): + cmd = "show runningconfiguration all" + result = subprocess.Popen(cmd, shell=True, text=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE) + text, err = result.communicate() + return_code = result.returncode + if return_code: + raise RuntimeError("Failed to get running config, Return Code: {}, Error: {}".format(return_code, err)) + return json.loads(text) + + def create_sonic_yang_with_loaded_models(self): + if self.sonic_yang_with_loaded_models is None: + loaded_models_sy = sonic_yang.SonicYang(self.yang_dir) + loaded_models_sy.loadYangModel() + self.sonic_yang_with_loaded_models = loaded_models_sy + + return copy.copy(self.sonic_yang_with_loaded_models) + + def validate_config_db_config(self, config_json): + sy = self.create_sonic_yang_with_loaded_models() + try: + tmp_config_json = copy.deepcopy(config_json) + sy.loadData(tmp_config_json) + sy.validate_data_tree() + return True + except sonic_yang.SonicYangException as ex: + return False + return False + + def validate_set_entry(self, table, key, data): + config_json = self.get_config_db_as_json() + if not self.validate_config_db_config(config_json): + return False + if data is not None: + config_json[table][key] = data + if not self.validate_config_db_config(config_json): + return False + return True + + From e73b4cb5dc45c5eb27f4fea04837eb14fb080c90 Mon Sep 17 00:00:00 2001 From: Ubuntu Date: Fri, 29 Jul 2022 22:05:07 +0000 Subject: [PATCH 3/5] Revert "Add CLI to configure YANG config validatoin" This reverts commit 2a8c285a88ffee2a560719849769cbd544c44d94. --- config/main.py | 15 ------------- tests/yang_config_validation_test.py | 33 ---------------------------- 2 files changed, 48 deletions(-) delete mode 100644 tests/yang_config_validation_test.py diff --git a/config/main.py b/config/main.py index 56066439be..32d697dad4 100644 --- a/config/main.py +++ b/config/main.py @@ -1715,21 +1715,6 @@ def synchronous_mode(sync_mode): else: raise click.BadParameter("Error: Invalid argument %s, expect either enable or disable" % sync_mode) -# -# 'yang_config_validation' command ('config yang_config_validation ...') -# -@config.command('yang_config_validation') -@click.argument('yang_config_validation', metavar='', required=True) -def yang_config_validation(yang_config_validation): - # Enable or disable YANG validation on updates to ConfigDB - if yang_config_validation == 'enable' or yang_config_validation == 'disable': - config_db = ConfigDBConnector() - config_db.connect() - config_db.mod_entry('DEVICE_METADATA', 'localhost', {"yang_config_validation": yang_config_validation}) - click.echo("""Wrote %s yang config validation into CONFIG_DB""" % yang_config_validation) - else: - raise click.BadParameter("Error: Invalid argument %s, expect either enable or disable" % yang_config_validation) - # # 'portchannel' group ('config portchannel ...') # diff --git a/tests/yang_config_validation_test.py b/tests/yang_config_validation_test.py deleted file mode 100644 index 37b9e448cc..0000000000 --- a/tests/yang_config_validation_test.py +++ /dev/null @@ -1,33 +0,0 @@ -from click.testing import CliRunner -import config.main as config - -class TestYangConfigValidation(object): - @classmethod - def setup_class(cls): - print("SETUP") - - def __check_result(self, result_msg, mode): - if mode == "enable" or mode == "disable": - expected_msg = """Wrote %s yang config validation into CONFIG_DB""" % mode - else: - expected_msg = "Error: Invalid argument %s, expect either enable or disable" % mode - - return expected_msg in result_msg - - def test_yang_config_validation(self): - runner = CliRunner() - - result = runner.invoke(config.config.commands["yang_config_validation"], ["enable"]) - print(result.output) - assert result.exit_code == 0 - assert self.__check_result(result.output, "enable") - - result = runner.invoke(config.config.commands["yang_config_validation"], ["disable"]) - print(result.output) - assert result.exit_code == 0 - assert self.__check_result(result.output, "disable") - - result = runner.invoke(config.config.commands["yang_config_validation"], ["invalid-input"]) - print(result.output) - assert result.exit_code != 0 - assert self.__check_result(result.output, "invalid-input") From 4b1f547a9438c215fe19ef4bdc60f9e4d9f8716f Mon Sep 17 00:00:00 2001 From: Ubuntu Date: Fri, 29 Jul 2022 22:06:56 +0000 Subject: [PATCH 4/5] Revert "Revert "Add CLI to configure YANG config validatoin"" This reverts commit e73b4cb5dc45c5eb27f4fea04837eb14fb080c90. --- config/main.py | 15 +++++++++++++ tests/yang_config_validation_test.py | 33 ++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) create mode 100644 tests/yang_config_validation_test.py diff --git a/config/main.py b/config/main.py index 32d697dad4..56066439be 100644 --- a/config/main.py +++ b/config/main.py @@ -1715,6 +1715,21 @@ def synchronous_mode(sync_mode): else: raise click.BadParameter("Error: Invalid argument %s, expect either enable or disable" % sync_mode) +# +# 'yang_config_validation' command ('config yang_config_validation ...') +# +@config.command('yang_config_validation') +@click.argument('yang_config_validation', metavar='', required=True) +def yang_config_validation(yang_config_validation): + # Enable or disable YANG validation on updates to ConfigDB + if yang_config_validation == 'enable' or yang_config_validation == 'disable': + config_db = ConfigDBConnector() + config_db.connect() + config_db.mod_entry('DEVICE_METADATA', 'localhost', {"yang_config_validation": yang_config_validation}) + click.echo("""Wrote %s yang config validation into CONFIG_DB""" % yang_config_validation) + else: + raise click.BadParameter("Error: Invalid argument %s, expect either enable or disable" % yang_config_validation) + # # 'portchannel' group ('config portchannel ...') # diff --git a/tests/yang_config_validation_test.py b/tests/yang_config_validation_test.py new file mode 100644 index 0000000000..37b9e448cc --- /dev/null +++ b/tests/yang_config_validation_test.py @@ -0,0 +1,33 @@ +from click.testing import CliRunner +import config.main as config + +class TestYangConfigValidation(object): + @classmethod + def setup_class(cls): + print("SETUP") + + def __check_result(self, result_msg, mode): + if mode == "enable" or mode == "disable": + expected_msg = """Wrote %s yang config validation into CONFIG_DB""" % mode + else: + expected_msg = "Error: Invalid argument %s, expect either enable or disable" % mode + + return expected_msg in result_msg + + def test_yang_config_validation(self): + runner = CliRunner() + + result = runner.invoke(config.config.commands["yang_config_validation"], ["enable"]) + print(result.output) + assert result.exit_code == 0 + assert self.__check_result(result.output, "enable") + + result = runner.invoke(config.config.commands["yang_config_validation"], ["disable"]) + print(result.output) + assert result.exit_code == 0 + assert self.__check_result(result.output, "disable") + + result = runner.invoke(config.config.commands["yang_config_validation"], ["invalid-input"]) + print(result.output) + assert result.exit_code != 0 + assert self.__check_result(result.output, "invalid-input") From e7418c77249760d266bc656af917135039d841d4 Mon Sep 17 00:00:00 2001 From: Ubuntu Date: Fri, 29 Jul 2022 22:07:13 +0000 Subject: [PATCH 5/5] Revert "add yang validation service" This reverts commit 11335b2d08b2d52fbec310d5f0f5d5d49ba9266f. --- config/main.py | 22 ++++++------- config/yang_validation_service.py | 51 ------------------------------- 2 files changed, 10 insertions(+), 63 deletions(-) delete mode 100644 config/yang_validation_service.py diff --git a/config/main.py b/config/main.py index 56066439be..e126ff8bc3 100644 --- a/config/main.py +++ b/config/main.py @@ -30,7 +30,6 @@ from utilities_common.general import load_db_config from .utils import log -from .yang_validation_service import YangValidationService from . import aaa from . import chassis_modules @@ -1754,6 +1753,10 @@ def portchannel(ctx, namespace): @click.pass_context def add_portchannel(ctx, portchannel_name, min_links, fallback): """Add port channel""" + if is_portchannel_name_valid(portchannel_name) != True: + ctx.fail("{} is invalid!, name should have prefix '{}' and suffix '{}'" + .format(portchannel_name, CFG_PORTCHANNEL_PREFIX, CFG_PORTCHANNEL_NO)) + db = ctx.obj['db'] if is_portchannel_present_in_db(db, portchannel_name): @@ -1766,18 +1769,17 @@ def add_portchannel(ctx, portchannel_name, min_links, fallback): fvs['min_links'] = str(min_links) if fallback != 'false': fvs['fallback'] = 'true' - yvs = YangValidationService() - if not yvs.validate_set_entry('PORTCHANNEL', portchannel_name, fvs): - ctx.fail("Invalid configuration based on PortChannel YANG model") - else: - db.set_entry('PORTCHANNEL', portchannel_name, fvs) - + db.set_entry('PORTCHANNEL', portchannel_name, fvs) @portchannel.command('del') @click.argument('portchannel_name', metavar='', required=True) @click.pass_context def remove_portchannel(ctx, portchannel_name): """Remove port channel""" + if is_portchannel_name_valid(portchannel_name) != True: + ctx.fail("{} is invalid!, name should have prefix '{}' and suffix '{}'" + .format(portchannel_name, CFG_PORTCHANNEL_PREFIX, CFG_PORTCHANNEL_NO)) + db = ctx.obj['db'] # Dont proceed if the port channel does not exist @@ -1787,11 +1789,7 @@ def remove_portchannel(ctx, portchannel_name): if len([(k, v) for k, v in db.get_table('PORTCHANNEL_MEMBER') if k == portchannel_name]) != 0: click.echo("Error: Portchannel {} contains members. Remove members before deleting Portchannel!".format(portchannel_name)) else: - yvs = YangValidationService() - if not yvs.validate_set_entry('PORTCHANNEL', portchannel_name, None): - ctx.fail("Invalid configuration based on PortChannel YANG model") - else: - db.set_entry('PORTCHANNEL', portchannel_name, None) + db.set_entry('PORTCHANNEL', portchannel_name, None) @portchannel.group(cls=clicommon.AbbreviationGroup, name='member') @click.pass_context diff --git a/config/yang_validation_service.py b/config/yang_validation_service.py deleted file mode 100644 index 37d3bd2bc7..0000000000 --- a/config/yang_validation_service.py +++ /dev/null @@ -1,51 +0,0 @@ -import json -import sonic_yang -import subprocess -import copy - -YANG_DIR = "/usr/local/yang-models" - -class YangValidationService: - def __init__(self, yang_dir = YANG_DIR): - self.yang_dir = YANG_DIR - self.sonic_yang_with_loaded_models = None - - def get_config_db_as_json(self): - cmd = "show runningconfiguration all" - result = subprocess.Popen(cmd, shell=True, text=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE) - text, err = result.communicate() - return_code = result.returncode - if return_code: - raise RuntimeError("Failed to get running config, Return Code: {}, Error: {}".format(return_code, err)) - return json.loads(text) - - def create_sonic_yang_with_loaded_models(self): - if self.sonic_yang_with_loaded_models is None: - loaded_models_sy = sonic_yang.SonicYang(self.yang_dir) - loaded_models_sy.loadYangModel() - self.sonic_yang_with_loaded_models = loaded_models_sy - - return copy.copy(self.sonic_yang_with_loaded_models) - - def validate_config_db_config(self, config_json): - sy = self.create_sonic_yang_with_loaded_models() - try: - tmp_config_json = copy.deepcopy(config_json) - sy.loadData(tmp_config_json) - sy.validate_data_tree() - return True - except sonic_yang.SonicYangException as ex: - return False - return False - - def validate_set_entry(self, table, key, data): - config_json = self.get_config_db_as_json() - if not self.validate_config_db_config(config_json): - return False - if data is not None: - config_json[table][key] = data - if not self.validate_config_db_config(config_json): - return False - return True - -