From 4f79838dca3c66ed8c000684485b0ebfd90a02c8 Mon Sep 17 00:00:00 2001 From: Jingwen Xie Date: Wed, 10 Aug 2022 04:02:09 +0000 Subject: [PATCH 1/7] Add verification for override --- config/main.py | 81 +++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 63 insertions(+), 18 deletions(-) diff --git a/config/main.py b/config/main.py index 0f1fcf28fd..f88a2b38e8 100644 --- a/config/main.py +++ b/config/main.py @@ -12,6 +12,7 @@ import sys import time import itertools +import sonic_yang from collections import OrderedDict from generic_config_updater.generic_updater import GenericUpdater, ConfigFormat @@ -77,6 +78,7 @@ DEFAULT_GOLDEN_CONFIG_DB_FILE = '/etc/sonic/golden_config_db.json' INIT_CFG_FILE = '/etc/sonic/init_cfg.json' +YANG_DIR = '/usr/local/yang-models' DEFAULT_NAMESPACE = '' CFG_LOOPBACK_PREFIX = "Loopback" @@ -1852,27 +1854,70 @@ def override_config_table(db, input_config_db, dry_run): config_db = db.cfgdb + # Read config from configDB + current_config = config_db.get_config() + # Serialize to the same format as json input + sonic_cfggen.FormatConverter.to_serialized(current_config) + + updated_config = update_config(current_config, config_input) + + yang_enabled = is_yang_config_verification_enabled(config_db) + if yang_enabled: + sy = sonic_yang_with_loaded_models(YANG_DIR) + # Validate running config + validate_config_by_yang(sy, current_config) + # Validate input config + validate_config_by_yang(sy, config_input) + # Validate updated whole config + validate_config_by_yang(sy, updated_config) + if dry_run: - # Read config from configDB - current_config = config_db.get_config() - # Serialize to the same format as json input - sonic_cfggen.FormatConverter.to_serialized(current_config) - # Override current config with golden config - for table in config_input: - current_config[table] = config_input[table] - print(json.dumps(current_config, sort_keys=True, + print(json.dumps(updated_config, sort_keys=True, indent=4, cls=minigraph_encoder)) else: - # Deserialized golden config to DB recognized format - sonic_cfggen.FormatConverter.to_deserialized(config_input) - # Delete table from DB then mod_config to apply golden config - click.echo("Removing configDB overriden table first ...") - for table in config_input: - config_db.delete_table(table) - click.echo("Overriding input config to configDB ...") - data = sonic_cfggen.FormatConverter.output_to_db(config_input) - config_db.mod_config(data) - click.echo("Overriding completed. No service is restarted.") + override_config_db(config_db, config_input) + + +def is_yang_config_verification_enabled(config_db): + device_metadata = config_db.get_entry('DEVICE_METADATA', 'localhost') + return 'enable' == device_metadata.get('yang_config_validation', None) + + +def sonic_yang_with_loaded_models(yang_dir): + sy = sonic_yang.SonicYang(yang_dir) + sy.loadYangModel() + return sy + + +def validate_config_by_yang(sy, config_json): + try: + tmp_config_json = copy.deepcopy(config_json) + sy.loadData(tmp_config_json) + sy.validate_data_tree() + except sonic_yang.SonicYangException as ex: + click.secho("Failed to validate config. Error: {}".format(ex), fg="red") + sys.exit(1) + + +def update_config(current_config, config_input): + # Override current config with golden config + for table in config_input: + current_config[table] = config_input[table] + return current_config + + +def override_config_db(config_db, config_input): + # Deserialized golden config to DB recognized format + sonic_cfggen.FormatConverter.to_deserialized(config_input) + # Delete table from DB then mod_config to apply golden config + click.echo("Removing configDB overriden table first ...") + for table in config_input: + config_db.delete_table(table) + click.echo("Overriding input config to configDB ...") + data = sonic_cfggen.FormatConverter.output_to_db(config_input) + config_db.mod_config(data) + click.echo("Overriding completed. No service is restarted.") + # # 'hostname' command From 245d114d489162cc3bd621f573ba272a132ba7ec Mon Sep 17 00:00:00 2001 From: Jingwen Xie Date: Wed, 10 Aug 2022 06:52:54 +0000 Subject: [PATCH 2/7] fix bug and add unit test --- config/main.py | 6 +- .../final_config_yang_failure.json | 71 +++++++++++++++ .../golden_input_yang_failure.json | 89 +++++++++++++++++++ .../running_config_yang_failure.json | 89 +++++++++++++++++++ tests/config_override_test.py | 63 +++++++++++++ 5 files changed, 316 insertions(+), 2 deletions(-) create mode 100644 tests/config_override_input/final_config_yang_failure.json create mode 100644 tests/config_override_input/golden_input_yang_failure.json create mode 100644 tests/config_override_input/running_config_yang_failure.json diff --git a/config/main.py b/config/main.py index f88a2b38e8..540d40ef39 100644 --- a/config/main.py +++ b/config/main.py @@ -13,6 +13,7 @@ import time import itertools import sonic_yang +import copy from collections import OrderedDict from generic_config_updater.generic_updater import GenericUpdater, ConfigFormat @@ -1900,10 +1901,11 @@ def validate_config_by_yang(sy, config_json): def update_config(current_config, config_input): + updated_config = copy.deepcopy(current_config) # Override current config with golden config for table in config_input: - current_config[table] = config_input[table] - return current_config + updated_config[table] = config_input[table] + return updated_config def override_config_db(config_db, config_input): diff --git a/tests/config_override_input/final_config_yang_failure.json b/tests/config_override_input/final_config_yang_failure.json new file mode 100644 index 0000000000..51e5e40098 --- /dev/null +++ b/tests/config_override_input/final_config_yang_failure.json @@ -0,0 +1,71 @@ +{ + "running_config": { + "ACL_TABLE": { + "DATAACL": { + "policy_desc": "DATAACL", + "ports": [ + "Ethernet4" + ], + "stage": "ingress", + "type": "L3" + }, + "NTP_ACL": { + "policy_desc": "NTP_ACL", + "services": [ + "NTP" + ], + "stage": "ingress", + "type": "CTRLPLANE" + } + }, + "AUTO_TECHSUPPORT_FEATURE": { + "bgp": { + "rate_limit_interval": "600", + "state": "enabled" + }, + "database": { + "rate_limit_interval": "600", + "state": "enabled" + } + }, + "PORT": { + "Ethernet4": { + "admin_status": "up", + "alias": "fortyGigE0/4", + "description": "Servers0:eth0", + "index": "1", + "lanes": "29,30,31,32", + "mtu": "9100", + "pfc_asym": "off", + "speed": "40000", + "tpid": "0x8100" + }, + "Ethernet8": { + "admin_status": "up", + "alias": "fortyGigE0/8", + "description": "Servers1:eth0", + "index": "2", + "lanes": "33,34,35,36", + "mtu": "9100", + "pfc_asym": "off", + "speed": "40000", + "tpid": "0x8100" + } + } + }, + "golden_config": { + "PORT": { + "Ethernet12": { + "admin_status": "up", + "alias": "fortyGigE0/12", + "description": "Servers2:eth0", + "index": "3", + "lanes": "37,38,39,40", + "mtu": "9100", + "pfc_asym": "off", + "speed": "40000", + "tpid": "0x8100" + } + } + } +} diff --git a/tests/config_override_input/golden_input_yang_failure.json b/tests/config_override_input/golden_input_yang_failure.json new file mode 100644 index 0000000000..4b533e1598 --- /dev/null +++ b/tests/config_override_input/golden_input_yang_failure.json @@ -0,0 +1,89 @@ +{ + "running_config": { + "ACL_TABLE": { + "DATAACL": { + "policy_desc": "DATAACL", + "ports": [ + "Ethernet4" + ], + "stage": "ingress", + "type": "L3" + }, + "NTP_ACL": { + "policy_desc": "NTP_ACL", + "services": [ + "NTP" + ], + "stage": "ingress", + "type": "CTRLPLANE" + } + }, + "AUTO_TECHSUPPORT_FEATURE": { + "bgp": { + "rate_limit_interval": "600", + "state": "enabled" + }, + "database": { + "rate_limit_interval": "600", + "state": "enabled" + } + }, + "PORT": { + "Ethernet4": { + "admin_status": "up", + "alias": "fortyGigE0/4", + "description": "Servers0:eth0", + "index": "1", + "lanes": "29,30,31,32", + "mtu": "9100", + "pfc_asym": "off", + "speed": "40000", + "tpid": "0x8100" + }, + "Ethernet8": { + "admin_status": "up", + "alias": "fortyGigE0/8", + "description": "Servers1:eth0", + "index": "2", + "lanes": "33,34,35,36", + "mtu": "9100", + "pfc_asym": "off", + "speed": "40000", + "tpid": "0x8100" + } + } + }, + "golden_config": { + "ACL_TABLE": { + "EVERFLOWV6": { + "policy_desc": "EVERFLOWV6", + "ports": [ + "Ethernet0" + ], + "stage": "ingress", + "type": "MIRRORV6" + } + }, + "AUTO_TECHSUPPORT_FEATURE": { + "bgp": { + "state": "disabled" + }, + "database": { + "state": "disabled" + } + }, + "PORT": { + "Ethernet12": { + "admin_status": "up", + "alias": "fortyGigE0/12", + "description": "Servers2:eth0", + "index": "3", + "lanes": "37,38,39,40", + "mtu": "9100", + "pfc_asym": "off", + "speed": "40000", + "tpid": "0x8100" + } + } + } +} diff --git a/tests/config_override_input/running_config_yang_failure.json b/tests/config_override_input/running_config_yang_failure.json new file mode 100644 index 0000000000..7060dd4d22 --- /dev/null +++ b/tests/config_override_input/running_config_yang_failure.json @@ -0,0 +1,89 @@ +{ + "running_config": { + "ACL_TABLE": { + "DATAACL": { + "policy_desc": "DATAACL", + "ports": [ + "Ethernet0" + ], + "stage": "ingress", + "type": "L3" + }, + "NTP_ACL": { + "policy_desc": "NTP_ACL", + "services": [ + "NTP" + ], + "stage": "ingress", + "type": "CTRLPLANE" + } + }, + "AUTO_TECHSUPPORT_FEATURE": { + "bgp": { + "rate_limit_interval": "600", + "state": "enabled" + }, + "database": { + "rate_limit_interval": "600", + "state": "enabled" + } + }, + "PORT": { + "Ethernet4": { + "admin_status": "up", + "alias": "fortyGigE0/4", + "description": "Servers0:eth0", + "index": "1", + "lanes": "29,30,31,32", + "mtu": "9100", + "pfc_asym": "off", + "speed": "40000", + "tpid": "0x8100" + }, + "Ethernet8": { + "admin_status": "up", + "alias": "fortyGigE0/8", + "description": "Servers1:eth0", + "index": "2", + "lanes": "33,34,35,36", + "mtu": "9100", + "pfc_asym": "off", + "speed": "40000", + "tpid": "0x8100" + } + } + }, + "golden_config": { + "ACL_TABLE": { + "EVERFLOWV6": { + "policy_desc": "EVERFLOWV6", + "ports": [ + "Ethernet12" + ], + "stage": "ingress", + "type": "MIRRORV6" + } + }, + "AUTO_TECHSUPPORT_FEATURE": { + "bgp": { + "state": "disabled" + }, + "database": { + "state": "disabled" + } + }, + "PORT": { + "Ethernet12": { + "admin_status": "up", + "alias": "fortyGigE0/12", + "description": "Servers2:eth0", + "index": "3", + "lanes": "37,38,39,40", + "mtu": "9100", + "pfc_asym": "off", + "speed": "40000", + "tpid": "0x8100" + } + } + } +} diff --git a/tests/config_override_test.py b/tests/config_override_test.py index 255e63989d..be28afb3e4 100644 --- a/tests/config_override_test.py +++ b/tests/config_override_test.py @@ -17,6 +17,9 @@ FULL_CONFIG_OVERRIDE = os.path.join(DATA_DIR, "full_config_override.json") PORT_CONFIG_OVERRIDE = os.path.join(DATA_DIR, "port_config_override.json") EMPTY_TABLE_REMOVAL = os.path.join(DATA_DIR, "empty_table_removal.json") +RUNNING_CONFIG_YANG_FAILURE = os.path.join(DATA_DIR, "running_config_yang_failure.json") +GOLDEN_INPUT_YANG_FAILURE = os.path.join(DATA_DIR, "golden_input_yang_failure.json") +FINAL_CONFIG_YANG_FAILURE = os.path.join(DATA_DIR, "final_config_yang_failure.json") # Load sonic-cfggen from source since /usr/local/bin/sonic-cfggen does not have .py extension. sonic_cfggen = load_module_from_source('sonic_cfggen', '/usr/local/bin/sonic-cfggen') @@ -163,6 +166,66 @@ def read_json_file_side_effect(filename): assert result.exit_code == 0 assert current_config == expected_config + def test_yang_verification_enabled(self): + def is_yang_config_verification_enabled_side_effect(filename): + return True + db = Db() + with open(FULL_CONFIG_OVERRIDE, "r") as f: + read_data = json.load(f) + with mock.patch('config.main.is_yang_config_verification_enabled', + mock.MagicMock(side_effect=is_yang_config_verification_enabled_side_effect)): + self.check_override_config_table( + db, config, read_data['running_config'], read_data['golden_config'], + read_data['expected_config']) + + def test_running_config_yang_failure(self): + def is_yang_config_verification_enabled_side_effect(filename): + return True + db = Db() + with open(RUNNING_CONFIG_YANG_FAILURE, "r") as f: + read_data = json.load(f) + with mock.patch('config.main.is_yang_config_verification_enabled', + mock.MagicMock(side_effect=is_yang_config_verification_enabled_side_effect)): + self.check_yang_verification_failure( + db, config, read_data['running_config'], read_data['golden_config']) + + def test_golden_input_yang_failure(self): + def is_yang_config_verification_enabled_side_effect(filename): + return True + db = Db() + with open(GOLDEN_INPUT_YANG_FAILURE, "r") as f: + read_data = json.load(f) + with mock.patch('config.main.is_yang_config_verification_enabled', + mock.MagicMock(side_effect=is_yang_config_verification_enabled_side_effect)): + self.check_yang_verification_failure( + db, config, read_data['running_config'], read_data['golden_config']) + + def test_final_config_yang_failure(self): + def is_yang_config_verification_enabled_side_effect(filename): + return True + db = Db() + with open(FINAL_CONFIG_YANG_FAILURE, "r") as f: + read_data = json.load(f) + with mock.patch('config.main.is_yang_config_verification_enabled', + mock.MagicMock(side_effect=is_yang_config_verification_enabled_side_effect)): + self.check_yang_verification_failure( + db, config, read_data['running_config'], read_data['golden_config']) + + def check_yang_verification_failure(self, db, config, running_config, + golden_config): + def read_json_file_side_effect(filename): + return golden_config + with mock.patch('config.main.read_json_file', + mock.MagicMock(side_effect=read_json_file_side_effect)): + write_init_config_db(db.cfgdb, running_config) + + runner = CliRunner() + result = runner.invoke(config.config.commands["override-config-table"], + ['golden_config_db.json'], obj=db) + assert result.exit_code == 1 + assert "Failed to validate config. Error:" in result.output + + @classmethod def teardown_class(cls): print("TEARDOWN") From 9f01b4860799ed48d41d9a1714b8df733adc3df1 Mon Sep 17 00:00:00 2001 From: Jingwen Xie Date: Mon, 15 Aug 2022 06:14:14 +0000 Subject: [PATCH 3/7] resolve comment --- config/main.py | 10 +++++----- tests/config_override_test.py | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/config/main.py b/config/main.py index 540d40ef39..da760b4a6c 100644 --- a/config/main.py +++ b/config/main.py @@ -1866,11 +1866,11 @@ def override_config_table(db, input_config_db, dry_run): if yang_enabled: sy = sonic_yang_with_loaded_models(YANG_DIR) # Validate running config - validate_config_by_yang(sy, current_config) + validate_config_by_yang(sy, current_config, "current_config") # Validate input config - validate_config_by_yang(sy, config_input) + validate_config_by_yang(sy, config_input, "config_input") # Validate updated whole config - validate_config_by_yang(sy, updated_config) + validate_config_by_yang(sy, updated_config, "updated_config") if dry_run: print(json.dumps(updated_config, sort_keys=True, @@ -1890,13 +1890,13 @@ def sonic_yang_with_loaded_models(yang_dir): return sy -def validate_config_by_yang(sy, config_json): +def validate_config_by_yang(sy, config_json, jname): try: tmp_config_json = copy.deepcopy(config_json) sy.loadData(tmp_config_json) sy.validate_data_tree() except sonic_yang.SonicYangException as ex: - click.secho("Failed to validate config. Error: {}".format(ex), fg="red") + click.secho("Failed to validate {}. Error: {}".format(jname, ex), fg="magenta") sys.exit(1) diff --git a/tests/config_override_test.py b/tests/config_override_test.py index be28afb3e4..ff901308a0 100644 --- a/tests/config_override_test.py +++ b/tests/config_override_test.py @@ -187,7 +187,7 @@ def is_yang_config_verification_enabled_side_effect(filename): with mock.patch('config.main.is_yang_config_verification_enabled', mock.MagicMock(side_effect=is_yang_config_verification_enabled_side_effect)): self.check_yang_verification_failure( - db, config, read_data['running_config'], read_data['golden_config']) + db, config, read_data['running_config'], read_data['golden_config'], "current_config") def test_golden_input_yang_failure(self): def is_yang_config_verification_enabled_side_effect(filename): @@ -198,7 +198,7 @@ def is_yang_config_verification_enabled_side_effect(filename): with mock.patch('config.main.is_yang_config_verification_enabled', mock.MagicMock(side_effect=is_yang_config_verification_enabled_side_effect)): self.check_yang_verification_failure( - db, config, read_data['running_config'], read_data['golden_config']) + db, config, read_data['running_config'], read_data['golden_config'], "config_input") def test_final_config_yang_failure(self): def is_yang_config_verification_enabled_side_effect(filename): @@ -209,10 +209,10 @@ def is_yang_config_verification_enabled_side_effect(filename): with mock.patch('config.main.is_yang_config_verification_enabled', mock.MagicMock(side_effect=is_yang_config_verification_enabled_side_effect)): self.check_yang_verification_failure( - db, config, read_data['running_config'], read_data['golden_config']) + db, config, read_data['running_config'], read_data['golden_config'], "updated_config") def check_yang_verification_failure(self, db, config, running_config, - golden_config): + golden_config, jname): def read_json_file_side_effect(filename): return golden_config with mock.patch('config.main.read_json_file', @@ -223,7 +223,7 @@ def read_json_file_side_effect(filename): result = runner.invoke(config.config.commands["override-config-table"], ['golden_config_db.json'], obj=db) assert result.exit_code == 1 - assert "Failed to validate config. Error:" in result.output + assert "Failed to validate {}. Error:".format(jname) in result.output @classmethod From 73f8b252173944d577aca1652eb0fd51e783a484 Mon Sep 17 00:00:00 2001 From: Jingwen Xie Date: Wed, 17 Aug 2022 06:16:21 +0000 Subject: [PATCH 4/7] Change to use ConfigMgmt for validation --- config/main.py | 35 +++++++++++++---------------- tests/config_override_test.py | 42 ++++++++++++++++++++++++++--------- 2 files changed, 48 insertions(+), 29 deletions(-) diff --git a/config/main.py b/config/main.py index da760b4a6c..40b20d6dde 100644 --- a/config/main.py +++ b/config/main.py @@ -12,7 +12,6 @@ import sys import time import itertools -import sonic_yang import copy from collections import OrderedDict @@ -47,7 +46,7 @@ from . import vlan from . import vxlan from . import plugins -from .config_mgmt import ConfigMgmtDPB +from .config_mgmt import ConfigMgmtDPB, ConfigMgmt from . import mclag from . import syslog @@ -79,7 +78,6 @@ DEFAULT_GOLDEN_CONFIG_DB_FILE = '/etc/sonic/golden_config_db.json' INIT_CFG_FILE = '/etc/sonic/init_cfg.json' -YANG_DIR = '/usr/local/yang-models' DEFAULT_NAMESPACE = '' CFG_LOOPBACK_PREFIX = "Loopback" @@ -1864,13 +1862,18 @@ def override_config_table(db, input_config_db, dry_run): yang_enabled = is_yang_config_verification_enabled(config_db) if yang_enabled: - sy = sonic_yang_with_loaded_models(YANG_DIR) - # Validate running config - validate_config_by_yang(sy, current_config, "current_config") + # The ConfigMgmt will load YANG and validate + # runnning config during initialization. + try: + cm = ConfigMgmt() + except Exception as ex: + click.secho("Failed to validate running config. Error: {}".format(ex), fg="magenta") + sys.exit(1) + # Validate input config - validate_config_by_yang(sy, config_input, "config_input") + validate_config_by_cm(cm, config_input, "config_input") # Validate updated whole config - validate_config_by_yang(sy, updated_config, "updated_config") + validate_config_by_cm(cm, updated_config, "updated_config") if dry_run: print(json.dumps(updated_config, sort_keys=True, @@ -1884,18 +1887,12 @@ def is_yang_config_verification_enabled(config_db): return 'enable' == device_metadata.get('yang_config_validation', None) -def sonic_yang_with_loaded_models(yang_dir): - sy = sonic_yang.SonicYang(yang_dir) - sy.loadYangModel() - return sy - - -def validate_config_by_yang(sy, config_json, jname): +def validate_config_by_cm(cm, config_json, jname): + tmp_config_json = copy.deepcopy(config_json) try: - tmp_config_json = copy.deepcopy(config_json) - sy.loadData(tmp_config_json) - sy.validate_data_tree() - except sonic_yang.SonicYangException as ex: + cm.loadData(tmp_config_json) + cm.validateConfigData() + except Exception as ex: click.secho("Failed to validate {}. Error: {}".format(jname, ex), fg="magenta") sys.exit(1) diff --git a/tests/config_override_test.py b/tests/config_override_test.py index ff901308a0..253ddc05b8 100644 --- a/tests/config_override_test.py +++ b/tests/config_override_test.py @@ -24,6 +24,9 @@ # Load sonic-cfggen from source since /usr/local/bin/sonic-cfggen does not have .py extension. sonic_cfggen = load_module_from_source('sonic_cfggen', '/usr/local/bin/sonic-cfggen') +config_mgmt_py_path = os.path.join(os.path.dirname(__file__), '..', 'config', 'config_mgmt.py') +config_mgmt = load_module_from_source('config_mgmt', config_mgmt_py_path) + def write_init_config_db(cfgdb, config): tables = cfgdb.get_config() @@ -169,15 +172,27 @@ def read_json_file_side_effect(filename): def test_yang_verification_enabled(self): def is_yang_config_verification_enabled_side_effect(filename): return True + + def config_mgmt_side_effect(): + return config_mgmt.ConfigMgmt(source=CONFIG_DB_JSON_FILE) + db = Db() with open(FULL_CONFIG_OVERRIDE, "r") as f: read_data = json.load(f) + # ConfigMgmt will call ConfigDBConnector to load default config_db.json. + # Here I modify the ConfigMgmt initialization and make it initiated with + # a source file which share the same as what we write to cfgdb. + CONFIG_DB_JSON_FILE = "startConfigDb.json" + write_config_to_file(read_data['running_config'], CONFIG_DB_JSON_FILE) with mock.patch('config.main.is_yang_config_verification_enabled', - mock.MagicMock(side_effect=is_yang_config_verification_enabled_side_effect)): + mock.MagicMock(side_effect=is_yang_config_verification_enabled_side_effect)), \ + mock.patch('config.main.ConfigMgmt', + mock.MagicMock(side_effect=config_mgmt_side_effect)): self.check_override_config_table( db, config, read_data['running_config'], read_data['golden_config'], read_data['expected_config']) + def test_running_config_yang_failure(self): def is_yang_config_verification_enabled_side_effect(filename): return True @@ -187,7 +202,7 @@ def is_yang_config_verification_enabled_side_effect(filename): with mock.patch('config.main.is_yang_config_verification_enabled', mock.MagicMock(side_effect=is_yang_config_verification_enabled_side_effect)): self.check_yang_verification_failure( - db, config, read_data['running_config'], read_data['golden_config'], "current_config") + db, config, read_data['running_config'], read_data['golden_config'], "running config") def test_golden_input_yang_failure(self): def is_yang_config_verification_enabled_side_effect(filename): @@ -215,16 +230,23 @@ def check_yang_verification_failure(self, db, config, running_config, golden_config, jname): def read_json_file_side_effect(filename): return golden_config - with mock.patch('config.main.read_json_file', - mock.MagicMock(side_effect=read_json_file_side_effect)): - write_init_config_db(db.cfgdb, running_config) - runner = CliRunner() - result = runner.invoke(config.config.commands["override-config-table"], - ['golden_config_db.json'], obj=db) - assert result.exit_code == 1 - assert "Failed to validate {}. Error:".format(jname) in result.output + def config_mgmt_side_effect(): + return config_mgmt.ConfigMgmt(source=CONFIG_DB_JSON_FILE) + CONFIG_DB_JSON_FILE = "startConfigDb.json" + write_config_to_file(running_config, CONFIG_DB_JSON_FILE) + with mock.patch('config.main.read_json_file', + mock.MagicMock(side_effect=read_json_file_side_effect)), \ + mock.patch('config.main.ConfigMgmt', + mock.MagicMock(side_effect=config_mgmt_side_effect)): + write_init_config_db(db.cfgdb, running_config) + + runner = CliRunner() + result = runner.invoke(config.config.commands["override-config-table"], + ['golden_config_db.json'], obj=db) + assert result.exit_code == 1 + assert "Failed to validate {}. Error:".format(jname) in result.output @classmethod def teardown_class(cls): From 76c7efa5a18e569c052e960ecdf43c7d44a0fd9e Mon Sep 17 00:00:00 2001 From: Jingwen Xie Date: Fri, 19 Aug 2022 06:55:19 +0000 Subject: [PATCH 5/7] missing validation for running config --- config/main.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/config/main.py b/config/main.py index 40b20d6dde..b1bf74d258 100644 --- a/config/main.py +++ b/config/main.py @@ -1862,10 +1862,11 @@ def override_config_table(db, input_config_db, dry_run): yang_enabled = is_yang_config_verification_enabled(config_db) if yang_enabled: - # The ConfigMgmt will load YANG and validate - # runnning config during initialization. + # The ConfigMgmt will load YANG and running + # config during initialization. try: cm = ConfigMgmt() + cm.validateConfigData() except Exception as ex: click.secho("Failed to validate running config. Error: {}".format(ex), fg="magenta") sys.exit(1) From d37d35f63b06f75b38486a579c0d1b7c87b31356 Mon Sep 17 00:00:00 2001 From: Jingwen Xie Date: Wed, 17 Aug 2022 07:42:58 +0000 Subject: [PATCH 6/7] test --- config/main.py | 2 +- tests/config_override_test.py | 28 ++++++++++++++++------------ 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/config/main.py b/config/main.py index b1bf74d258..36fd626369 100644 --- a/config/main.py +++ b/config/main.py @@ -1860,7 +1860,7 @@ def override_config_table(db, input_config_db, dry_run): updated_config = update_config(current_config, config_input) - yang_enabled = is_yang_config_verification_enabled(config_db) + yang_enabled = device_info.is_yang_config_validation_enabled(config_db) if yang_enabled: # The ConfigMgmt will load YANG and running # config during initialization. diff --git a/tests/config_override_test.py b/tests/config_override_test.py index 253ddc05b8..1b058ace13 100644 --- a/tests/config_override_test.py +++ b/tests/config_override_test.py @@ -170,7 +170,7 @@ def read_json_file_side_effect(filename): assert current_config == expected_config def test_yang_verification_enabled(self): - def is_yang_config_verification_enabled_side_effect(filename): + def is_yang_config_validation_enabled_side_effect(filename): return True def config_mgmt_side_effect(): @@ -179,13 +179,14 @@ def config_mgmt_side_effect(): db = Db() with open(FULL_CONFIG_OVERRIDE, "r") as f: read_data = json.load(f) + # ConfigMgmt will call ConfigDBConnector to load default config_db.json. # Here I modify the ConfigMgmt initialization and make it initiated with # a source file which share the same as what we write to cfgdb. CONFIG_DB_JSON_FILE = "startConfigDb.json" write_config_to_file(read_data['running_config'], CONFIG_DB_JSON_FILE) - with mock.patch('config.main.is_yang_config_verification_enabled', - mock.MagicMock(side_effect=is_yang_config_verification_enabled_side_effect)), \ + with mock.patch('config.main.device_info.is_yang_config_validation_enabled', + mock.MagicMock(side_effect=is_yang_config_validation_enabled_side_effect)), \ mock.patch('config.main.ConfigMgmt', mock.MagicMock(side_effect=config_mgmt_side_effect)): self.check_override_config_table( @@ -194,35 +195,35 @@ def config_mgmt_side_effect(): def test_running_config_yang_failure(self): - def is_yang_config_verification_enabled_side_effect(filename): + def is_yang_config_validation_enabled_side_effect(filename): return True db = Db() with open(RUNNING_CONFIG_YANG_FAILURE, "r") as f: read_data = json.load(f) - with mock.patch('config.main.is_yang_config_verification_enabled', - mock.MagicMock(side_effect=is_yang_config_verification_enabled_side_effect)): + with mock.patch('config.main.device_info.is_yang_config_validation_enabled', + mock.MagicMock(side_effect=is_yang_config_validation_enabled_side_effect)): self.check_yang_verification_failure( db, config, read_data['running_config'], read_data['golden_config'], "running config") def test_golden_input_yang_failure(self): - def is_yang_config_verification_enabled_side_effect(filename): + def is_yang_config_validation_enabled_side_effect(filename): return True db = Db() with open(GOLDEN_INPUT_YANG_FAILURE, "r") as f: read_data = json.load(f) - with mock.patch('config.main.is_yang_config_verification_enabled', - mock.MagicMock(side_effect=is_yang_config_verification_enabled_side_effect)): + with mock.patch('config.main.device_info.is_yang_config_validation_enabled', + mock.MagicMock(side_effect=is_yang_config_validation_enabled_side_effect)): self.check_yang_verification_failure( db, config, read_data['running_config'], read_data['golden_config'], "config_input") def test_final_config_yang_failure(self): - def is_yang_config_verification_enabled_side_effect(filename): + def is_yang_config_validation_enabled_side_effect(filename): return True db = Db() with open(FINAL_CONFIG_YANG_FAILURE, "r") as f: read_data = json.load(f) - with mock.patch('config.main.is_yang_config_verification_enabled', - mock.MagicMock(side_effect=is_yang_config_verification_enabled_side_effect)): + with mock.patch('config.main.device_info.is_yang_config_validation_enabled', + mock.MagicMock(side_effect=is_yang_config_validation_enabled_side_effect)): self.check_yang_verification_failure( db, config, read_data['running_config'], read_data['golden_config'], "updated_config") @@ -234,6 +235,9 @@ def read_json_file_side_effect(filename): def config_mgmt_side_effect(): return config_mgmt.ConfigMgmt(source=CONFIG_DB_JSON_FILE) + # ConfigMgmt will call ConfigDBConnector to load default config_db.json. + # Here I modify the ConfigMgmt initialization and make it initiated with + # a source file which share the same as what we write to cfgdb. CONFIG_DB_JSON_FILE = "startConfigDb.json" write_config_to_file(running_config, CONFIG_DB_JSON_FILE) with mock.patch('config.main.read_json_file', From f7966fccdeb4178b4ee78bdffba0e6e4af263ac5 Mon Sep 17 00:00:00 2001 From: Jingwen Xie Date: Thu, 1 Sep 2022 07:48:33 +0000 Subject: [PATCH 7/7] remove unused --- config/main.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/config/main.py b/config/main.py index 36fd626369..508ac9a511 100644 --- a/config/main.py +++ b/config/main.py @@ -1883,11 +1883,6 @@ def override_config_table(db, input_config_db, dry_run): override_config_db(config_db, config_input) -def is_yang_config_verification_enabled(config_db): - device_metadata = config_db.get_entry('DEVICE_METADATA', 'localhost') - return 'enable' == device_metadata.get('yang_config_validation', None) - - def validate_config_by_cm(cm, config_json, jname): tmp_config_json = copy.deepcopy(config_json) try: