From 676c31bd0e230e6aa613ca3a76008b8ffade9123 Mon Sep 17 00:00:00 2001 From: jingwenxie Date: Mon, 5 Sep 2022 11:02:24 +0800 Subject: [PATCH] Add verification for override (#2305) What I did Add Yang verification for config override-config-table How I did it Make 3 step verification: running config, golden input, final config How to verify it Run unit test. --- config/main.py | 78 ++++++++++++---- .../final_config_yang_failure.json | 71 +++++++++++++++ .../golden_input_yang_failure.json | 89 +++++++++++++++++++ .../running_config_yang_failure.json | 89 +++++++++++++++++++ tests/config_override_test.py | 89 +++++++++++++++++++ 5 files changed, 397 insertions(+), 19 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 53180cf519..bbcfc5d84f 100644 --- a/config/main.py +++ b/config/main.py @@ -12,6 +12,7 @@ import sys import time import itertools +import copy from collections import OrderedDict from generic_config_updater.generic_updater import GenericUpdater, ConfigFormat @@ -46,7 +47,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 @@ -1885,27 +1886,66 @@ 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 = device_info.is_yang_config_validation_enabled(config_db) + if yang_enabled: + # 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) + + # Validate input config + validate_config_by_cm(cm, config_input, "config_input") + # Validate updated whole config + validate_config_by_cm(cm, updated_config, "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 validate_config_by_cm(cm, config_json, jname): + tmp_config_json = copy.deepcopy(config_json) + try: + 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) + + +def update_config(current_config, config_input): + updated_config = copy.deepcopy(current_config) + # Override current config with golden config + for table in config_input: + updated_config[table] = config_input[table] + return updated_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 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..1b058ace13 100644 --- a/tests/config_override_test.py +++ b/tests/config_override_test.py @@ -17,10 +17,16 @@ 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') +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() @@ -163,6 +169,89 @@ 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_validation_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.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( + 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_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.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_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.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_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.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") + + def check_yang_verification_failure(self, db, config, running_config, + golden_config, jname): + def read_json_file_side_effect(filename): + return golden_config + + 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', + 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): print("TEARDOWN")