YANG validation for ConfigDB Updates: portchannel add/remove, loopback interface, VLAN

YANG validation for ConfigDB Updates: portchannel add/remove, loopback interface, VLAN

Author: isabelmsft

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')
@@ -2040,51 +2043,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='<portchannel_name>', 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
@@ -2113,8 +2129,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
@@ -6157,36 +6173,48 @@ def loopback(ctx, redis_unix_socket_path):
 @click.argument('loopback_name', metavar='<loopback_name>', 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='<loopback_name>', 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)

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)