Skip to content

Commit

Permalink
Exit early if YANG validation fails in Golden Config (#3490)
Browse files Browse the repository at this point in the history
What I did
Exit early if golden config fails YANG validation

How I did it
Check before stop service in load_minigraph

How to verify it
Unit test

Previous command output (if the output of
  • Loading branch information
wen587 authored and mssonicbld committed Sep 6, 2024
1 parent b530116 commit 0efc6d9
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 107 deletions.
21 changes: 18 additions & 3 deletions config/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import itertools
import copy
import tempfile
import sonic_yang

from jsonpatch import JsonPatchConflict
from jsonpointer import JsonPointerException
Expand Down Expand Up @@ -55,7 +56,7 @@
from . import vlan
from . import vxlan
from . import plugins
from .config_mgmt import ConfigMgmtDPB, ConfigMgmt
from .config_mgmt import ConfigMgmtDPB, ConfigMgmt, YANG_DIR
from . import mclag
from . import syslog
from . import dns
Expand Down Expand Up @@ -1766,8 +1767,22 @@ def load_minigraph(db, no_service_restart, traffic_shift_away, override_config,
fg='magenta')
raise click.Abort()

# Dependency check golden config json
config_to_check = read_json_file(golden_config_path)
if multi_asic.is_multi_asic():
# Multiasic has not 100% fully validated. Thus pass here.
pass
else:
sy = sonic_yang.SonicYang(YANG_DIR)
sy.loadYangModel()
try:
sy.loadData(configdbJson=config_to_check)
sy.validate_data_tree()
except sonic_yang.SonicYangException as e:
click.secho("{} fails YANG validation! Error: {}".format(golden_config_path, str(e)),
fg='magenta')
raise click.Abort()

# Dependency check golden config json
if multi_asic.is_multi_asic():
host_config = config_to_check.get('localhost', {})
else:
Expand Down Expand Up @@ -2094,7 +2109,7 @@ def aaa_table_hard_dependency_check(config_json):
tacacs_enable = "tacacs+" in aaa_authentication_login.split(",")
tacplus_passkey = TACPLUS_TABLE.get("global", {}).get("passkey", "")
if tacacs_enable and len(tacplus_passkey) == 0:
click.secho("Authentication with 'tacacs+' is not allowed when passkey not exits.", fg="magenta")
click.secho("Authentication with 'tacacs+' is not allowed when passkey not exists.", fg="magenta")
sys.exit(1)


Expand Down
89 changes: 0 additions & 89 deletions tests/config_override_input/golden_input_yang_failure.json

This file was deleted.

24 changes: 24 additions & 0 deletions tests/config_override_input/partial_config_override.json
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,30 @@
"stage": "ingress",
"type": "CTRLPLANE"
}
},
"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"
}
}
},
"expected_config": {
Expand Down
14 changes: 1 addition & 13 deletions tests/config_override_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
EMPTY_TABLE_REMOVAL = os.path.join(DATA_DIR, "empty_table_removal.json")
AAA_YANG_HARD_CHECK = os.path.join(DATA_DIR, "aaa_yang_hard_check.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")
MULTI_ASIC_MACSEC_OV = os.path.join(DATA_DIR, "multi_asic_macsec_ov.json")
MULTI_ASIC_FEATURE_RM = os.path.join(DATA_DIR, "multi_asic_feature_rm.json")
Expand Down Expand Up @@ -179,7 +178,7 @@ def read_json_file_side_effect(filename):
['golden_config_db.json'], obj=db)

assert result.exit_code != 0
assert "Authentication with 'tacacs+' is not allowed when passkey not exits." in result.output
assert "Authentication with 'tacacs+' is not allowed when passkey not exists." in result.output

def check_override_config_table(self, db, config, running_config,
golden_config, expected_config):
Expand Down Expand Up @@ -233,17 +232,6 @@ def is_yang_config_validation_enabled_side_effect(filename):
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
Expand Down
3 changes: 1 addition & 2 deletions tests/config_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -660,7 +660,6 @@ def read_json_file_side_effect(filename):
},
"TACPLUS": {
"global": {
"passkey": ""
}
}
}
Expand All @@ -672,7 +671,7 @@ def read_json_file_side_effect(filename):
runner = CliRunner()
result = runner.invoke(config.config.commands["load_minigraph"], ["--override_config", "-y"])
assert result.exit_code != 0
assert "Authentication with 'tacacs+' is not allowed when passkey not exits." in result.output
assert "Authentication with 'tacacs+' is not allowed when passkey not exists." in result.output

@mock.patch('sonic_py_common.device_info.get_paths_to_platform_and_hwsku_dirs', mock.MagicMock(return_value=("dummy_path", None)))
def test_load_minigraph_with_traffic_shift_away(self, get_cmd_module):
Expand Down

0 comments on commit 0efc6d9

Please sign in to comment.