diff --git a/config/main.py b/config/main.py index 9bb403284d..c880e2f2e8 100644 --- a/config/main.py +++ b/config/main.py @@ -14,6 +14,7 @@ import itertools import copy +from jsonpatch import JsonPatchConflict from collections import OrderedDict from generic_config_updater.generic_updater import GenericUpdater, ConfigFormat from minigraph import parse_device_desc_xml, minigraph_encoder @@ -31,6 +32,7 @@ import utilities_common.cli as clicommon from utilities_common.helper import get_port_pbh_binding, get_port_acl_binding from utilities_common.general import load_db_config, load_module_from_source +from .validated_config_db_connector import ValidatedConfigDBConnector import utilities_common.multi_asic as multi_asic_util from .utils import log @@ -104,6 +106,7 @@ TTL_RANGE = click.IntRange(min=0, max=255) QUEUE_RANGE = click.IntRange(min=0, max=255) GRE_TYPE_RANGE = click.IntRange(min=0, max=65535) +ADHOC_VALIDATION = True # 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') @@ -2031,51 +2034,64 @@ def portchannel(db, ctx, namespace): @click.pass_context def add_portchannel(ctx, portchannel_name, min_links, fallback, fast_rate): """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): - ctx.fail("{} already exists!".format(portchannel_name)) - + fvs = { 'admin_status': 'up', 'mtu': '9100', 'lacp_key': 'auto', 'fast_rate': fast_rate.lower(), } + if min_links != 0: fvs['min_links'] = str(min_links) if fallback != 'false': fvs['fallback'] = 'true' - db.set_entry('PORTCHANNEL', portchannel_name, fvs) - + + if ADHOC_VALIDATION: + db = ctx.obj['db'] + 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)) + if is_portchannel_present_in_db(db, portchannel_name): + ctx.fail("{} already exists!".format(portchannel_name)) # TODO: MISSING CONSTRAINT IN YANG MODEL + else: + db = ValidatedConfigDBConnector(ctx.obj['db']) + + try: + db.set_entry('PORTCHANNEL', portchannel_name, fvs) + except ValueError: + ctx.fail("{} is invalid!, name should have prefix '{}' and suffix '{}'".format(portchannel_name, CFG_PORTCHANNEL_PREFIX, CFG_PORTCHANNEL_NO)) + @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 - if is_portchannel_present_in_db(db, portchannel_name) is False: - ctx.fail("{} is not present.".format(portchannel_name)) - - # Dont let to remove port channel if vlan membership exists - for k,v in db.get_table('VLAN_MEMBER'): - if v == portchannel_name: - ctx.fail("{} has vlan {} configured, remove vlan membership to proceed".format(portchannel_name, str(k))) - - 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)) + + if ADHOC_VALIDATION: + db = ctx.obj['db'] + 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)) + + # Don't proceed if the port channel does not exist + if is_portchannel_present_in_db(db, portchannel_name) is False: + ctx.fail("{} is not present.".format(portchannel_name)) + + # Dont let to remove port channel if vlan membership exists + for k,v in db.get_table('VLAN_MEMBER'): # TODO: MISSING CONSTRAINT IN YANG MODEL + if v == portchannel_name: + ctx.fail("{} has vlan {} configured, remove vlan membership to proceed".format(portchannel_name, str(k))) + + if len([(k, v) for k, v in db.get_table('PORTCHANNEL_MEMBER') if k == portchannel_name]) != 0: # TODO: MISSING CONSTRAINT IN YANG MODEL + ctx.fail("Error: Portchannel {} contains members. Remove members before deleting Portchannel!".format(portchannel_name)) else: + db = ValidatedConfigDBConnector(ctx.obj['db']) + + try: db.set_entry('PORTCHANNEL', portchannel_name, None) + except JsonPatchConflict: + ctx.fail("{} is not present.".format(portchannel_name)) @portchannel.group(cls=clicommon.AbbreviationGroup, name='member') @click.pass_context @@ -2104,8 +2120,8 @@ def add_portchannel_member(ctx, portchannel_name, port_name): # Dont proceed if the port channel does not exist if is_portchannel_present_in_db(db, portchannel_name) is False: ctx.fail("{} is not present.".format(portchannel_name)) - - # Dont allow a port to be member of port channel if it is configured with an IP address + + # Don't allow a port to be member of port channel if it is configured with an IP address for key,value in db.get_table('INTERFACE').items(): if type(key) == tuple: continue @@ -6148,36 +6164,48 @@ def loopback(ctx, redis_unix_socket_path): @click.argument('loopback_name', metavar='', required=True) @click.pass_context def add_loopback(ctx, loopback_name): - config_db = ctx.obj['db'] - if is_loopback_name_valid(loopback_name) is False: - ctx.fail("{} is invalid, name should have prefix '{}' and suffix '{}' " - .format(loopback_name, CFG_LOOPBACK_PREFIX, CFG_LOOPBACK_NO)) - - lo_intfs = [k for k, v in config_db.get_table('LOOPBACK_INTERFACE').items() if type(k) != tuple] - if loopback_name in lo_intfs: - ctx.fail("{} already exists".format(loopback_name)) - - config_db.set_entry('LOOPBACK_INTERFACE', loopback_name, {"NULL" : "NULL"}) + if ADHOC_VALIDATION: + config_db = ctx.obj['db'] + if is_loopback_name_valid(loopback_name) is False: + ctx.fail("{} is invalid, name should have prefix '{}' and suffix '{}' " + .format(loopback_name, CFG_LOOPBACK_PREFIX, CFG_LOOPBACK_NO)) + + lo_intfs = [k for k, v in config_db.get_table('LOOPBACK_INTERFACE').items() if type(k) != tuple] + if loopback_name in lo_intfs: + ctx.fail("{} already exists".format(loopback_name)) # TODO: MISSING CONSTRAINT IN YANG VALIDATION + else: + config_db = ValidatedConfigDBConnector(ctx.obj['db']) + + try: + config_db.set_entry('LOOPBACK_INTERFACE', loopback_name, {"NULL" : "NULL"}) + except ValueError: + ctx.fail("{} is invalid, name should have prefix '{}' and suffix '{}' ".format(loopback_name, CFG_LOOPBACK_PREFIX, CFG_LOOPBACK_NO)) @loopback.command('del') @click.argument('loopback_name', metavar='', required=True) @click.pass_context def del_loopback(ctx, loopback_name): config_db = ctx.obj['db'] - if is_loopback_name_valid(loopback_name) is False: - ctx.fail("{} is invalid, name should have prefix '{}' and suffix '{}' " - .format(loopback_name, CFG_LOOPBACK_PREFIX, CFG_LOOPBACK_NO)) - lo_config_db = config_db.get_table('LOOPBACK_INTERFACE') - lo_intfs = [k for k, v in lo_config_db.items() if type(k) != tuple] - if loopback_name not in lo_intfs: - ctx.fail("{} does not exists".format(loopback_name)) + + if ADHOC_VALIDATION: + if is_loopback_name_valid(loopback_name) is False: + ctx.fail("{} is invalid, name should have prefix '{}' and suffix '{}' " + .format(loopback_name, CFG_LOOPBACK_PREFIX, CFG_LOOPBACK_NO)) + lo_intfs = [k for k, v in lo_config_db.items() if type(k) != tuple] + if loopback_name not in lo_intfs: + ctx.fail("{} does not exist".format(loopback_name)) + else: + config_db = ValidatedConfigDBConnector(ctx.obj['db']) ips = [ k[1] for k in lo_config_db if type(k) == tuple and k[0] == loopback_name ] for ip in ips: config_db.set_entry('LOOPBACK_INTERFACE', (loopback_name, ip), None) - - config_db.set_entry('LOOPBACK_INTERFACE', loopback_name, None) + + try: + config_db.set_entry('LOOPBACK_INTERFACE', loopback_name, None) + except JsonPatchConflict: + ctx.fail("{} does not exist".format(loopback_name)) @config.group(cls=clicommon.AbbreviationGroup) diff --git a/config/validated_config_db_connector.py b/config/validated_config_db_connector.py new file mode 100644 index 0000000000..b94a2df4a5 --- /dev/null +++ b/config/validated_config_db_connector.py @@ -0,0 +1,63 @@ +import jsonpatch +from jsonpointer import JsonPointer + +from sonic_py_common import device_info +from generic_config_updater.generic_updater import GenericUpdater, ConfigFormat +from generic_config_updater.gu_common import EmptyTableError, genericUpdaterLogging + +def ValidatedConfigDBConnector(config_db_connector): + yang_enabled = device_info.is_yang_config_validation_enabled(config_db_connector) + if yang_enabled: + config_db_connector.set_entry = validated_set_entry + config_db_connector.delete_table = validated_delete_table + return config_db_connector + +def make_path_value_jsonpatch_compatible(table, key, value): + if type(key) == tuple: + path = JsonPointer.from_parts([table, '|'.join(key)]).path + else: + path = JsonPointer.from_parts([table, key]).path + if value == {"NULL" : "NULL"}: + value = {} + return path, value + +def create_gcu_patch(op, table, key=None, value=None): + if key: + path, value = make_path_value_jsonpatch_compatible(table, key, value) + else: + path = "/{}".format(table) + + gcu_json_input = [] + gcu_json = {"op": "{}".format(op), + "path": "{}".format(path)} + if op == "add": + gcu_json["value"] = value + + gcu_json_input.append(gcu_json) + gcu_patch = jsonpatch.JsonPatch(gcu_json_input) + return gcu_patch + +def validated_delete_table(table): + gcu_patch = create_gcu_patch("remove", table) + format = ConfigFormat.CONFIGDB.name + config_format = ConfigFormat[format.upper()] + try: + GenericUpdater().apply_patch(patch=gcu_patch, config_format=config_format, verbose=False, dry_run=False, ignore_non_yang_tables=False, ignore_paths=None) + except ValueError as e: + logger = genericUpdaterLogging.get_logger(title="Patch Applier", print_all_to_console=True) + logger.log_notice("Unable to remove entry, as doing so will result in invalid config. Error: {}".format(e)) + +def validated_set_entry(table, key, value): + if value is not None: + op = "add" + else: + op = "remove" + + gcu_patch = create_gcu_patch(op, table, key, value) + format = ConfigFormat.CONFIGDB.name + config_format = ConfigFormat[format.upper()] + + try: + GenericUpdater().apply_patch(patch=gcu_patch, config_format=config_format, verbose=False, dry_run=False, ignore_non_yang_tables=False, ignore_paths=None) + except EmptyTableError: + validated_delete_table(table) diff --git a/config/vlan.py b/config/vlan.py index 80b4ff4984..39aeefae7c 100644 --- a/config/vlan.py +++ b/config/vlan.py @@ -1,8 +1,12 @@ import click import utilities_common.cli as clicommon +from jsonpatch import JsonPatchConflict from time import sleep from .utils import log +from .validated_config_db_connector import ValidatedConfigDBConnector + +ADHOC_VALIDATION = True # # 'vlan' group ('config vlan ...') @@ -19,19 +23,25 @@ def add_vlan(db, vid): """Add VLAN""" ctx = click.get_current_context() - - if not clicommon.is_vlanid_in_range(vid): - ctx.fail("Invalid VLAN ID {} (1-4094)".format(vid)) - vlan = 'Vlan{}'.format(vid) - if vid == 1: - ctx.fail("{} is default VLAN".format(vlan)) - - if clicommon.check_if_vlanid_exist(db.cfgdb, vlan): - ctx.fail("{} already exists".format(vlan)) + if ADHOC_VALIDATION: + config_db = db.cfgdb + if not clicommon.is_vlanid_in_range(vid): + ctx.fail("Invalid VLAN ID {} (1-4094)".format(vid)) - db.cfgdb.set_entry('VLAN', vlan, {'vlanid': vid}) + if vid == 1: + ctx.fail("{} is default VLAN".format(vlan)) # TODO: MISSING CONSTRAINT IN YANG MODEL + + if clicommon.check_if_vlanid_exist(db.cfgdb, vlan): # TODO: MISSING CONSTRAINT IN YANG MODEL + ctx.fail("{} already exists".format(vlan)) + else: + config_db = ValidatedConfigDBConnector(db.cfgdb) + + try: + config_db.set_entry('VLAN', vlan, {'vlanid': str(vid)}) + except ValueError: + ctx.fail("Invalid VLAN ID {} (1-4094)".format(vid)) @vlan.command('del') @click.argument('vid', metavar='', required=True, type=int) @@ -42,26 +52,33 @@ def del_vlan(db, vid): log.log_info("'vlan del {}' executing...".format(vid)) ctx = click.get_current_context() + vlan = 'Vlan{}'.format(vid) - if not clicommon.is_vlanid_in_range(vid): - ctx.fail("Invalid VLAN ID {} (1-4094)".format(vid)) + if ADHOC_VALIDATION: + config_db = db.cfgdb + if not clicommon.is_vlanid_in_range(vid): + ctx.fail("Invalid VLAN ID {} (1-4094)".format(vid)) - vlan = 'Vlan{}'.format(vid) - if clicommon.check_if_vlanid_exist(db.cfgdb, vlan) == False: - ctx.fail("{} does not exist".format(vlan)) + if clicommon.check_if_vlanid_exist(db.cfgdb, vlan) == False: + ctx.fail("{} does not exist".format(vlan)) - intf_table = db.cfgdb.get_table('VLAN_INTERFACE') - for intf_key in intf_table: - if ((type(intf_key) is str and intf_key == 'Vlan{}'.format(vid)) or - (type(intf_key) is tuple and intf_key[0] == 'Vlan{}'.format(vid))): - ctx.fail("{} can not be removed. First remove IP addresses assigned to this VLAN".format(vlan)) + intf_table = db.cfgdb.get_table('VLAN_INTERFACE') + for intf_key in intf_table: + if ((type(intf_key) is str and intf_key == 'Vlan{}'.format(vid)) or # TODO: MISSING CONSTRAINT IN YANG MODEL + (type(intf_key) is tuple and intf_key[0] == 'Vlan{}'.format(vid))): + ctx.fail("{} can not be removed. First remove IP addresses assigned to this VLAN".format(vlan)) - keys = [ (k, v) for k, v in db.cfgdb.get_table('VLAN_MEMBER') if k == 'Vlan{}'.format(vid) ] + keys = [ (k, v) for k, v in db.cfgdb.get_table('VLAN_MEMBER') if k == 'Vlan{}'.format(vid) ] - if keys: - ctx.fail("VLAN ID {} can not be removed. First remove all members assigned to this VLAN.".format(vid)) - - db.cfgdb.set_entry('VLAN', 'Vlan{}'.format(vid), None) + if keys: # TODO: MISSING CONSTRAINT IN YANG MODEL + ctx.fail("VLAN ID {} can not be removed. First remove all members assigned to this VLAN.".format(vid)) + else: + config_db = ValidatedConfigDBConnector(db.cfgdb) + + try: + config_db.set_entry('VLAN', 'Vlan{}'.format(vid), None) + except JsonPatchConflict: + ctx.fail("{} does not exist".format(vlan)) def restart_ndppd(): verify_swss_running_cmd = "docker container inspect -f '{{.State.Status}}' swss" @@ -118,46 +135,54 @@ def add_vlan_member(db, vid, port, untagged): log.log_info("'vlan member add {} {}' executing...".format(vid, port)) - if not clicommon.is_vlanid_in_range(vid): - ctx.fail("Invalid VLAN ID {} (1-4094)".format(vid)) - vlan = 'Vlan{}'.format(vid) - if clicommon.check_if_vlanid_exist(db.cfgdb, vlan) == False: - ctx.fail("{} does not exist".format(vlan)) - - if clicommon.get_interface_naming_mode() == "alias": - alias = port - iface_alias_converter = clicommon.InterfaceAliasConverter(db) - port = iface_alias_converter.alias_to_name(alias) - if port is None: - ctx.fail("cannot find port name for alias {}".format(alias)) - - if clicommon.is_port_mirror_dst_port(db.cfgdb, port): - ctx.fail("{} is configured as mirror destination port".format(port)) - - if clicommon.is_port_vlan_member(db.cfgdb, port, vlan): - ctx.fail("{} is already a member of {}".format(port, vlan)) - - if clicommon.is_valid_port(db.cfgdb, port): - is_port = True - elif clicommon.is_valid_portchannel(db.cfgdb, port): - is_port = False - else: - ctx.fail("{} does not exist".format(port)) - - if (is_port and clicommon.is_port_router_interface(db.cfgdb, port)) or \ - (not is_port and clicommon.is_pc_router_interface(db.cfgdb, port)): - ctx.fail("{} is a router interface!".format(port)) + + if ADHOC_VALIDATION: + config_db = db.cfgdb + if not clicommon.is_vlanid_in_range(vid): + ctx.fail("Invalid VLAN ID {} (1-4094)".format(vid)) + + if clicommon.check_if_vlanid_exist(db.cfgdb, vlan) == False: + ctx.fail("{} does not exist".format(vlan)) + + if clicommon.get_interface_naming_mode() == "alias": # TODO: MISSING CONSTRAINT IN YANG MODEL + alias = port + iface_alias_converter = clicommon.InterfaceAliasConverter(db) + port = iface_alias_converter.alias_to_name(alias) + if port is None: + ctx.fail("cannot find port name for alias {}".format(alias)) + + if clicommon.is_port_mirror_dst_port(db.cfgdb, port): # TODO: MISSING CONSTRAINT IN YANG MODEL + ctx.fail("{} is configured as mirror destination port".format(port)) + + if clicommon.is_port_vlan_member(db.cfgdb, port, vlan): # TODO: MISSING CONSTRAINT IN YANG MODEL + ctx.fail("{} is already a member of {}".format(port, vlan)) + + if clicommon.is_valid_port(db.cfgdb, port): + is_port = True + elif clicommon.is_valid_portchannel(db.cfgdb, port): + is_port = False + else: + ctx.fail("{} does not exist".format(port)) + + if (is_port and clicommon.is_port_router_interface(db.cfgdb, port)) or \ + (not is_port and clicommon.is_pc_router_interface(db.cfgdb, port)): # TODO: MISSING CONSTRAINT IN YANG MODEL + ctx.fail("{} is a router interface!".format(port)) - portchannel_member_table = db.cfgdb.get_table('PORTCHANNEL_MEMBER') + portchannel_member_table = db.cfgdb.get_table('PORTCHANNEL_MEMBER') - if (is_port and clicommon.interface_is_in_portchannel(portchannel_member_table, port)): - ctx.fail("{} is part of portchannel!".format(port)) + if (is_port and clicommon.interface_is_in_portchannel(portchannel_member_table, port)): # TODO: MISSING CONSTRAINT IN YANG MODEL + ctx.fail("{} is part of portchannel!".format(port)) - if (clicommon.interface_is_untagged_member(db.cfgdb, port) and untagged): - ctx.fail("{} is already untagged member!".format(port)) + if (clicommon.interface_is_untagged_member(db.cfgdb, port) and untagged): # TODO: MISSING CONSTRAINT IN YANG MODEL + ctx.fail("{} is already untagged member!".format(port)) + else: + config_db = ValidatedConfigDBConnector(db.cfgdb) - db.cfgdb.set_entry('VLAN_MEMBER', (vlan, port), {'tagging_mode': "untagged" if untagged else "tagged" }) + try: + config_db.set_entry('VLAN_MEMBER', (vlan, port), {'tagging_mode': "untagged" if untagged else "tagged" }) + except ValueError: + ctx.fail("{} invalid or does not exist, or {} invalid or does not exist".format(vlan, port)) @vlan_member.command('del') @click.argument('vid', metavar='', required=True, type=int) @@ -167,25 +192,31 @@ def del_vlan_member(db, vid, port): """Delete VLAN member""" ctx = click.get_current_context() - log.log_info("'vlan member del {} {}' executing...".format(vid, port)) - - if not clicommon.is_vlanid_in_range(vid): - ctx.fail("Invalid VLAN ID {} (1-4094)".format(vid)) - vlan = 'Vlan{}'.format(vid) - if clicommon.check_if_vlanid_exist(db.cfgdb, vlan) == False: - ctx.fail("{} does not exist".format(vlan)) - - if clicommon.get_interface_naming_mode() == "alias": - alias = port - iface_alias_converter = clicommon.InterfaceAliasConverter(db) - port = iface_alias_converter.alias_to_name(alias) - if port is None: - ctx.fail("cannot find port name for alias {}".format(alias)) - - if not clicommon.is_port_vlan_member(db.cfgdb, port, vlan): - ctx.fail("{} is not a member of {}".format(port, vlan)) + + if ADHOC_VALIDATION: + config_db = db.cfgdb + if not clicommon.is_vlanid_in_range(vid): + ctx.fail("Invalid VLAN ID {} (1-4094)".format(vid)) + + if clicommon.check_if_vlanid_exist(db.cfgdb, vlan) == False: + ctx.fail("{} does not exist".format(vlan)) + + if clicommon.get_interface_naming_mode() == "alias": # TODO: MISSING CONSTRAINT IN YANG MODEL + alias = port + iface_alias_converter = clicommon.InterfaceAliasConverter(db) + port = iface_alias_converter.alias_to_name(alias) + if port is None: + ctx.fail("cannot find port name for alias {}".format(alias)) + + if not clicommon.is_port_vlan_member(db.cfgdb, port, vlan): # TODO: MISSING CONSTRAINT IN YANG MODEL + ctx.fail("{} is not a member of {}".format(port, vlan)) + else: + config_db = ValidatedConfigDBConnector(db.cfgdb) - db.cfgdb.set_entry('VLAN_MEMBER', (vlan, port), None) + try: + config_db.set_entry('VLAN_MEMBER', (vlan, port), None) + except JsonPatchConflict: + ctx.fail("{} invalid or does not exist, or {} is not a member of {}".format(vlan, port, vlan)) diff --git a/generic_config_updater/generic_updater.py b/generic_config_updater/generic_updater.py index 56297039aa..be2ddb0091 100644 --- a/generic_config_updater/generic_updater.py +++ b/generic_config_updater/generic_updater.py @@ -1,7 +1,7 @@ import json import os from enum import Enum -from .gu_common import GenericConfigUpdaterError, ConfigWrapper, \ +from .gu_common import GenericConfigUpdaterError, EmptyTableError, ConfigWrapper, \ DryRunConfigWrapper, PatchWrapper, genericUpdaterLogging from .patch_sorter import StrictPatchSorter, NonStrictPatchSorter, ConfigSplitter, \ TablesWithoutYangConfigSplitter, IgnorePathsFromYangConfigSplitter @@ -54,7 +54,7 @@ def apply(self, patch): empty_tables = self.config_wrapper.get_empty_tables(target_config) if empty_tables: # if there are empty tables empty_tables_txt = ", ".join(empty_tables) - raise ValueError("Given patch is not valid because it will result in empty tables " \ + raise EmptyTableError("Given patch is not valid because it will result in empty tables " \ "which is not allowed in ConfigDb. " \ f"Table{'s' if len(empty_tables) != 1 else ''}: {empty_tables_txt}") diff --git a/generic_config_updater/gu_common.py b/generic_config_updater/gu_common.py index 1397396b75..6667f3360f 100644 --- a/generic_config_updater/gu_common.py +++ b/generic_config_updater/gu_common.py @@ -16,6 +16,9 @@ class GenericConfigUpdaterError(Exception): pass +class EmptyTableError(ValueError): + pass + class JsonChange: """ A class that describes a partial change to a JSON object. diff --git a/tests/config_test.py b/tests/config_test.py index a9f4982548..f0246a3522 100644 --- a/tests/config_test.py +++ b/tests/config_test.py @@ -15,10 +15,12 @@ from sonic_py_common import device_info from utilities_common.db import Db from utilities_common.general import load_module_from_source +from mock import patch from generic_config_updater.generic_updater import ConfigFormat import config.main as config +import config.validated_config_db_connector as validated_config_db_connector # Add Test, module and script path. test_path = os.path.dirname(os.path.abspath(__file__)) @@ -1614,3 +1616,89 @@ def test_hostname_add(self, db_conn_patch, get_cmd_module): @classmethod def teardown_class(cls): print("TEARDOWN") + + +class TestConfigLoopback(object): + @classmethod + def setup_class(cls): + print("SETUP") + import config.main + importlib.reload(config.main) + + @patch("validated_config_db_connector.device_info.is_yang_config_validation_enabled", mock.Mock(return_value=True)) + @patch("config.validated_config_db_connector.validated_set_entry", mock.Mock(side_effect=ValueError)) + def test_add_loopback_with_invalid_name_yang_validation(self): + config.ADHOC_VALIDATION = False + runner = CliRunner() + db = Db() + obj = {'db':db.cfgdb} + + result = runner.invoke(config.config.commands["loopback"].commands["add"], ["Loopbax1"], obj=obj) + print(result.exit_code) + print(result.output) + assert result.exit_code != 0 + assert "Error: Loopbax1 is invalid, name should have prefix 'Loopback' and suffix '<0-999>'" in result.output + + def test_add_loopback_with_invalid_name_adhoc_validation(self): + config.ADHOC_VALIDATION = True + runner = CliRunner() + db = Db() + obj = {'db':db.cfgdb} + + result = runner.invoke(config.config.commands["loopback"].commands["add"], ["Loopbax1"], obj=obj) + print(result.exit_code) + print(result.output) + assert result.exit_code != 0 + assert "Error: Loopbax1 is invalid, name should have prefix 'Loopback' and suffix '<0-999>'" in result.output + + def test_del_nonexistent_loopback_adhoc_validation(self): + config.ADHOC_VALIDATION = True + runner = CliRunner() + db = Db() + obj = {'db':db.cfgdb} + + result = runner.invoke(config.config.commands["loopback"].commands["del"], ["Loopback12"], obj=obj) + print(result.exit_code) + print(result.output) + assert result.exit_code != 0 + assert "Loopback12 does not exist" in result.output + + def test_del_nonexistent_loopback_adhoc_validation(self): + config.ADHOC_VALIDATION = True + runner = CliRunner() + db = Db() + obj = {'db':db.cfgdb} + + result = runner.invoke(config.config.commands["loopback"].commands["del"], ["Loopbax1"], obj=obj) + print(result.exit_code) + print(result.output) + assert result.exit_code != 0 + assert "Loopbax1 is invalid, name should have prefix 'Loopback' and suffix '<0-999>'" in result.output + + @patch("config.validated_config_db_connector.validated_set_entry", mock.Mock(return_value=True)) + @patch("validated_config_db_connector.device_info.is_yang_config_validation_enabled", mock.Mock(return_value=True)) + def test_add_loopback_yang_validation(self): + config.ADHOC_VALIDATION = False + runner = CliRunner() + db = Db() + obj = {'db':db.cfgdb} + + result = runner.invoke(config.config.commands["loopback"].commands["add"], ["Loopback12"], obj=obj) + print(result.exit_code) + print(result.output) + assert result.exit_code == 0 + + def test_add_loopback_adhoc_validation(self): + config.ADHOC_VALIDATION = True + runner = CliRunner() + db = Db() + obj = {'db':db.cfgdb} + + result = runner.invoke(config.config.commands["loopback"].commands["add"], ["Loopback12"], obj=obj) + print(result.exit_code) + print(result.output) + assert result.exit_code == 0 + + @classmethod + def teardown_class(cls): + print("TEARDOWN") diff --git a/tests/portchannel_test.py b/tests/portchannel_test.py index bd30c73649..b35b93d552 100644 --- a/tests/portchannel_test.py +++ b/tests/portchannel_test.py @@ -1,12 +1,16 @@ import os import pytest import traceback +import mock from click.testing import CliRunner +from jsonpatch import JsonPatchConflict import config.main as config +import config.validated_config_db_connector as validated_config_db_connector import show.main as show from utilities_common.db import Db +from mock import patch class TestPortChannel(object): @classmethod @@ -14,7 +18,23 @@ def setup_class(cls): os.environ['UTILITIES_UNIT_TESTING'] = "1" print("SETUP") - def test_add_portchannel_with_invalid_name(self): + @patch("config.main.is_portchannel_present_in_db", mock.Mock(return_value=False)) + @patch("config.validated_config_db_connector.validated_set_entry", mock.Mock(side_effect=ValueError)) + @patch("validated_config_db_connector.device_info.is_yang_config_validation_enabled", mock.Mock(return_value=True)) + def test_add_portchannel_with_invalid_name_yang_validation(self): + config.ADHOC_VALIDATION = False + runner = CliRunner() + db = Db() + obj = {'db':db.cfgdb} + + result = runner.invoke(config.config.commands["portchannel"].commands["add"], ["PortChan005"], obj=obj) + print(result.exit_code) + print(result.output) + assert result.exit_code != 0 + assert "Error: PortChan005 is invalid!, name should have prefix 'PortChannel' and suffix '<0-9999>'" in result.output + + def test_add_portchannel_with_invalid_name_adhoc_validation(self): + config.ADHOC_VALIDATION = True runner = CliRunner() db = Db() obj = {'db':db.cfgdb} @@ -26,7 +46,23 @@ def test_add_portchannel_with_invalid_name(self): assert result.exit_code != 0 assert "Error: PortChan005 is invalid!, name should have prefix 'PortChannel' and suffix '<0-9999>'" in result.output - def test_delete_portchannel_with_invalid_name(self): + @patch("config.validated_config_db_connector.validated_set_entry", mock.Mock(side_effect=JsonPatchConflict)) + @patch("validated_config_db_connector.device_info.is_yang_config_validation_enabled", mock.Mock(return_value=True)) + def test_delete_nonexistent_portchannel_yang_validation(self): + config.ADHOC_VALIDATION = False + runner = CliRunner() + db = Db() + obj = {'db':db.cfgdb} + + # delete a portchannel with invalid name + result = runner.invoke(config.config.commands["portchannel"].commands["del"], ["PortChan005"], obj=obj) + print(result.exit_code) + print(result.output) + assert result.exit_code != 0 + assert "PortChan005 is not present" in result.output + + def test_delete_portchannel_with_invalid_name_adhoc_validation(self): + config.ADHOC_VALIDATION = True runner = CliRunner() db = Db() obj = {'db':db.cfgdb} @@ -50,7 +86,8 @@ def test_add_existing_portchannel_again(self): assert result.exit_code != 0 assert "Error: PortChannel0001 already exists!" in result.output - def test_delete_non_existing_portchannel(self): + def test_delete_non_existing_portchannel_adhoc_validation(self): + config.ADHOC_VALIDATION = True runner = CliRunner() db = Db() obj = {'db':db.cfgdb} @@ -63,7 +100,8 @@ def test_delete_non_existing_portchannel(self): assert "Error: PortChannel0005 is not present." in result.output @pytest.mark.parametrize("fast_rate", ["False", "True", "false", "true"]) - def test_add_portchannel_with_fast_rate(self, fast_rate): + def test_add_portchannel_with_fast_rate_adhoc_validation(self, fast_rate): + config.ADHOC_VALIDATION = True runner = CliRunner() db = Db() obj = {'db':db.cfgdb} @@ -79,7 +117,7 @@ def test_add_portchannel_with_invalid_fast_rate(self, fast_rate): runner = CliRunner() db = Db() obj = {'db':db.cfgdb} - + # add a portchannel with invalid fats rate result = runner.invoke(config.config.commands["portchannel"].commands["add"], ["PortChannel0005", "--fast-rate", fast_rate], obj=obj) print(result.exit_code) diff --git a/tests/validated_config_db_connector_test.py b/tests/validated_config_db_connector_test.py new file mode 100644 index 0000000000..48d559cd9a --- /dev/null +++ b/tests/validated_config_db_connector_test.py @@ -0,0 +1,30 @@ +import imp +import os +import mock + +imp.load_source('validated_config_db_connector', \ + os.path.join(os.path.dirname(__file__), '..', 'config', 'validated_config_db_connector.py')) +import validated_config_db_connector + +from unittest import TestCase +from mock import patch +from generic_config_updater.gu_common import EmptyTableError +from utilities_common.db import Db + +SAMPLE_TABLE = 'VLAN' +SAMPLE_KEY = 'Vlan1000' +SAMPLE_VALUE_EMPTY = None + + +class TestValidatedConfigDBConnector(TestCase): + ''' + + Test Class for validated_config_db_connector.py + + ''' + def test_validated_config_db_connector_empty_table(self): + mock_generic_updater = mock.Mock() + mock_generic_updater.apply_patch = mock.Mock(side_effect=EmptyTableError) + with mock.patch('validated_config_db_connector.GenericUpdater', return_value=mock_generic_updater): + remove_entry_success = validated_config_db_connector.validated_set_entry(SAMPLE_TABLE, SAMPLE_KEY, SAMPLE_VALUE_EMPTY) + assert not remove_entry_success