Skip to content

YANG Validation for ConfigDB Updates: TACPLUS, TACPLUS_SERVER, AAA, VLAN_SUB_INTERFACE tables + decorated validated_mod_entry #2452

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
isabelmsft merged 17 commits into from
Nov 4, 2022
Merged

YANG Validation for ConfigDB Updates: TACPLUS, TACPLUS_SERVER, AAA, VLAN_SUB_INTERFACE tables + decorated validated_mod_entry #2452

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
b4f466d
TACPLUS, TACPLUS_SERVER, AAA, VLAN_SUB_INTERFACE tables + mod_entry +…
isabelmsft Oct 25, 2022
c3544a2
fix UT
isabelmsft Oct 25, 2022
6313ce7
Address PR comments
isabelmsft Oct 26, 2022
3e25495
Fix syntax
isabelmsft Oct 26, 2022
b20dfb9
readd import statement bc it is used in UT
isabelmsft Oct 26, 2022
650e723
fix UT
isabelmsft Oct 28, 2022
18a5e1a
fix syntax
isabelmsft Oct 28, 2022
c257eb9
Fix UT
isabelmsft Oct 28, 2022
0bc9fa7
Fix UT
isabelmsft Oct 28, 2022
3ce3dbc
fix UT
isabelmsft Nov 1, 2022
5899db9
fix UT
isabelmsft Nov 1, 2022
08ee911
Add DEVICE_METADATA table
isabelmsft Nov 1, 2022
f29e499
Revert "fix UT"
isabelmsft Nov 1, 2022
07fd1ce
Revert "Revert "fix UT""
isabelmsft Nov 1, 2022
b7902d8
Revert "Add DEVICE_METADATA table"
isabelmsft Nov 1, 2022
d945fb2
Make exception handling more specific
isabelmsft Nov 1, 2022
3c57a56
fix LGTM
isabelmsft Nov 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 35 additions & 14 deletions config/aaa.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@
import ipaddress
import re
from swsscommon.swsscommon import ConfigDBConnector
from .validated_config_db_connector import ValidatedConfigDBConnector
from jsonpatch import JsonPatchConflict
import utilities_common.cli as clicommon

ADHOC_VALIDATION = True
RADIUS_MAXSERVERS = 8
RADIUS_PASSKEY_MAX_LEN = 65
VALID_CHARS_MSG = "Valid chars are ASCII printable except SPACE, '#', and ','"
Expand All @@ -13,19 +16,27 @@ def is_secret(secret):


def add_table_kv(table, entry, key, val):
config_db = ConfigDBConnector()
config_db = ValidatedConfigDBConnector(ConfigDBConnector())
config_db.connect()
config_db.mod_entry(table, entry, {key:val})
try:
config_db.mod_entry(table, entry, {key:val})
except ValueError as e:
ctx = click.get_current_context()
ctx.fail("Invalid ConfigDB. Error: {}".format(e))


def del_table_key(table, entry, key):
config_db = ConfigDBConnector()
config_db = ValidatedConfigDBConnector(ConfigDBConnector())
config_db.connect()
data = config_db.get_entry(table, entry)
if data:
if key in data:
del data[key]
config_db.set_entry(table, entry, data)
try:
config_db.set_entry(table, entry, data)
except (ValueError, JsonPatchConflict) as e:
ctx = click.get_current_context()
ctx.fail("Invalid ConfigDB. Error: {}".format(e))

@click.group()
def aaa():
Expand Down Expand Up @@ -246,11 +257,12 @@ def passkey(ctx, secret):
@click.option('-m', '--use-mgmt-vrf', help="Management vrf, default is no vrf", is_flag=True)
def add(address, timeout, key, auth_type, port, pri, use_mgmt_vrf):
"""Specify a TACACS+ server"""
if not clicommon.is_ipaddress(address):
click.echo('Invalid ip address')
return
if ADHOC_VALIDATION:
if not clicommon.is_ipaddress(address):
click.echo('Invalid ip address') # TODO: MISSING CONSTRAINT IN YANG MODEL
return

config_db = ConfigDBConnector()
config_db = ValidatedConfigDBConnector(ConfigDBConnector())
config_db.connect()
old_data = config_db.get_entry('TACPLUS_SERVER', address)
if old_data != {}:
Expand All @@ -268,7 +280,11 @@ def add(address, timeout, key, auth_type, port, pri, use_mgmt_vrf):
data['passkey'] = key
if use_mgmt_vrf :
data['vrf'] = "mgmt"
config_db.set_entry('TACPLUS_SERVER', address, data)
try:
config_db.set_entry('TACPLUS_SERVER', address, data)
except ValueError as e:
ctx = click.get_current_context()
ctx.fail("Invalid ip address. Error: {}".format(e))
tacacs.add_command(add)


Expand All @@ -278,13 +294,18 @@ def add(address, timeout, key, auth_type, port, pri, use_mgmt_vrf):
@click.argument('address', metavar='<ip_address>')
def delete(address):
"""Delete a TACACS+ server"""
if not clicommon.is_ipaddress(address):
click.echo('Invalid ip address')
return
if ADHOC_VALIDATION:
if not clicommon.is_ipaddress(address):
click.echo('Invalid ip address')
return

config_db = ConfigDBConnector()
config_db = ValidatedConfigDBConnector(ConfigDBConnector())
config_db.connect()
config_db.set_entry('TACPLUS_SERVER', address, None)
try:
config_db.set_entry('TACPLUS_SERVER', address, None)
except JsonPatchConflict as e:
ctx = click.get_current_context()
ctx.fail("Invalid ip address. Error: {}".format(e))
tacacs.add_command(delete)


Expand Down
116 changes: 64 additions & 52 deletions config/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6796,73 +6796,82 @@ def is_subintf_shortname(intf):
@click.argument('vid', metavar='<vid>', required=False, type=click.IntRange(1,4094))
@click.pass_context
def add_subinterface(ctx, subinterface_name, vid):
config_db = ValidatedConfigDBConnector(ctx.obj['db'])
sub_intf_sep_idx = subinterface_name.find(VLAN_SUB_INTERFACE_SEPARATOR)
if sub_intf_sep_idx == -1:
ctx.fail("{} is invalid vlan subinterface".format(subinterface_name))

interface_alias = subinterface_name[:sub_intf_sep_idx]
if interface_alias is None:
ctx.fail("{} invalid subinterface".format(interface_alias))

if interface_alias.startswith("Po") is True:
intf_table_name = CFG_PORTCHANNEL_PREFIX
elif interface_alias.startswith("Eth") is True:
intf_table_name = 'PORT'

config_db = ctx.obj['db']
port_dict = config_db.get_table(intf_table_name)
parent_intf = get_intf_longname(interface_alias)
if interface_alias is not None:
if not port_dict:
ctx.fail("{} parent interface not found. {} table none".format(interface_alias, intf_table_name))
if parent_intf not in port_dict.keys():
ctx.fail("{} parent interface not found".format(subinterface_name))

# Validate if parent is portchannel member
portchannel_member_table = config_db.get_table('PORTCHANNEL_MEMBER')
if interface_is_in_portchannel(portchannel_member_table, parent_intf):
ctx.fail("{} is configured as a member of portchannel. Cannot configure subinterface"
.format(parent_intf))
if ADHOC_VALIDATION:
if sub_intf_sep_idx == -1:
ctx.fail("{} is invalid vlan subinterface".format(subinterface_name))

# Validate if parent is vlan member
vlan_member_table = config_db.get_table('VLAN_MEMBER')
if interface_is_in_vlan(vlan_member_table, parent_intf):
ctx.fail("{} is configured as a member of vlan. Cannot configure subinterface"
.format(parent_intf))
if interface_alias is None:
ctx.fail("{} invalid subinterface".format(interface_alias))

sub_intfs = [k for k,v in config_db.get_table('VLAN_SUB_INTERFACE').items() if type(k) != tuple]
if subinterface_name in sub_intfs:
ctx.fail("{} already exists".format(subinterface_name))
if interface_alias.startswith("Po") is True:
intf_table_name = CFG_PORTCHANNEL_PREFIX
elif interface_alias.startswith("Eth") is True:
intf_table_name = 'PORT'
else:
ctx.fail("{} is invalid vlan subinterface".format(subinterface_name))

port_dict = config_db.get_table(intf_table_name)
parent_intf = get_intf_longname(interface_alias)
if interface_alias is not None:
if not port_dict:
ctx.fail("{} parent interface not found. {} table none".format(interface_alias, intf_table_name))
if parent_intf not in port_dict.keys():
ctx.fail("{} parent interface not found".format(subinterface_name))

# Validate if parent is portchannel member
portchannel_member_table = config_db.get_table('PORTCHANNEL_MEMBER')
if interface_is_in_portchannel(portchannel_member_table, parent_intf): # TODO: MISSING CONSTRAINT IN YANG MODEL
ctx.fail("{} is configured as a member of portchannel. Cannot configure subinterface"
.format(parent_intf))

# Validate if parent is vlan member
vlan_member_table = config_db.get_table('VLAN_MEMBER')
if interface_is_in_vlan(vlan_member_table, parent_intf): # TODO: MISSING CONSTRAINT IN YANG MODEL
ctx.fail("{} is configured as a member of vlan. Cannot configure subinterface"
.format(parent_intf))

sub_intfs = [k for k,v in config_db.get_table('VLAN_SUB_INTERFACE').items() if type(k) != tuple]
if subinterface_name in sub_intfs:
ctx.fail("{} already exists".format(subinterface_name)) # TODO: MISSING CONSTRAINT IN YANG MODEL

if subintf_vlan_check(config_db, get_intf_longname(interface_alias), vid) is True:
ctx.fail("Vlan {} encap already configured on other subinterface on {}".format(vid, interface_alias)) # TODO: MISSING CONSTRAINT IN YANG MODEL

if vid is None and is_subintf_shortname(subinterface_name):
ctx.fail("{} Encap vlan is mandatory or short name subinterfaces".format(subinterface_name)) # TODO: MISSING CONSTRAINT IN YANG MODEL

subintf_dict = {}
if vid is not None:
subintf_dict.update({"vlan" : vid})
elif is_subintf_shortname(subinterface_name):
ctx.fail("{} Encap vlan is mandatory for short name subinterfaces".format(subinterface_name))

if subintf_vlan_check(config_db, get_intf_longname(interface_alias), vid) is True:
ctx.fail("Vlan {} encap already configured on other subinterface on {}".format(vid, interface_alias))

subintf_dict.update({"admin_status" : "up"})
config_db.set_entry('VLAN_SUB_INTERFACE', subinterface_name, subintf_dict)

try:
config_db.set_entry('VLAN_SUB_INTERFACE', subinterface_name, subintf_dict)
except ValueError as e:
ctx.fail("Invalid vlan subinterface. Error: {}".format(e))

@subinterface.command('del')
@click.argument('subinterface_name', metavar='<subinterface_name>', required=True)
@click.pass_context
def del_subinterface(ctx, subinterface_name):
sub_intf_sep_idx = subinterface_name.find(VLAN_SUB_INTERFACE_SEPARATOR)
if sub_intf_sep_idx == -1:
ctx.fail("{} is invalid vlan subinterface".format(subinterface_name))
config_db = ValidatedConfigDBConnector(ctx.obj['db'])

config_db = ctx.obj['db']
#subinterface_name = subintf_get_shortname(subinterface_name)
if interface_name_is_valid(config_db, subinterface_name) is False:
ctx.fail("{} is invalid ".format(subinterface_name))
if ADHOC_VALIDATION:
sub_intf_sep_idx = subinterface_name.find(VLAN_SUB_INTERFACE_SEPARATOR)
if sub_intf_sep_idx == -1:
ctx.fail("{} is invalid vlan subinterface".format(subinterface_name))

#subinterface_name = subintf_get_shortname(subinterface_name)
if interface_name_is_valid(config_db, subinterface_name) is False:
ctx.fail("{} is invalid ".format(subinterface_name))

subintf_config_db = config_db.get_table('VLAN_SUB_INTERFACE')
sub_intfs = [k for k,v in subintf_config_db.items() if type(k) != tuple]
if subinterface_name not in sub_intfs:
ctx.fail("{} does not exists".format(subinterface_name))
subintf_config_db = config_db.get_table('VLAN_SUB_INTERFACE')
sub_intfs = [k for k,v in subintf_config_db.items() if type(k) != tuple]
if subinterface_name not in sub_intfs:
ctx.fail("{} does not exists".format(subinterface_name))

ips = {}
ips = [ k[1] for k in config_db.get_table('VLAN_SUB_INTERFACE') if type(k) == tuple and k[0] == subinterface_name ]
Expand All @@ -6878,7 +6887,10 @@ def del_subinterface(ctx, subinterface_name):
for ip in ips:
config_db.set_entry('INTERFACE', (subinterface_name, ip), None)

config_db.set_entry('VLAN_SUB_INTERFACE', subinterface_name, None)
try:
config_db.set_entry('VLAN_SUB_INTERFACE', subinterface_name, None)
except JsonPatchConflict as e:
ctx.fail("{} is invalid vlan subinterface. Error: {}".format(subinterface_name, e))

if __name__ == '__main__':
config()
92 changes: 73 additions & 19 deletions config/validated_config_db_connector.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import jsonpatch
import copy
from jsonpointer import JsonPointer

from sonic_py_common import device_info
Expand All @@ -17,33 +18,83 @@ def __getattr__(self, name):
return self.validated_set_entry
if name == "delete_table":
return self.validated_delete_table
if name == "mod_entry":
return self.validated_mod_entry
return self.connector.__getattribute__(name)

def stringify_value(self, value):
if isinstance(value, dict):
value = {str(k):str(v) for k, v in value.items()}
else:
value = str(value)
return value

def make_path_value_jsonpatch_compatible(self, table, key, value):
if type(key) == tuple:
path = JsonPointer.from_parts([table, '|'.join(key)]).path
elif type(key) == list:
path = JsonPointer.from_parts([table, *key]).path
else:
path = JsonPointer.from_parts([table, key]).path
if value == {"NULL" : "NULL"}:
value = {}
else:
value = self.stringify_value(value)
return path, value

def create_gcu_patch(self, op, table, key=None, value=None):
if key:
path, value = self.make_path_value_jsonpatch_compatible(table, key, value)
else:
path = "/{}".format(table)

def create_gcu_patch(self, op, table, key=None, value=None, mod_entry=False):
gcu_json_input = []
gcu_json = {"op": "{}".format(op),
"path": "{}".format(path)}
if op == "add":
gcu_json["value"] = value
"""Add patch element to create new table if necessary, as GCU is unable to add to nonexistent table"""
if op == "add" and not self.get_table(table):
gcu_json = {"op": "{}".format(op),
"path": "/{}".format(table),
"value": {}}
gcu_json_input.append(gcu_json)

"""Add patch element to create ConfigDB path if necessary, as GCU is unable to add to a nonexistent path"""
if op == "add" and not self.get_entry(table, key):
path = JsonPointer.from_parts([table, key]).path
gcu_json = {"op": "{}".format(op),
"path": "{}".format(path),
"value": {}}
gcu_json_input.append(gcu_json)

def add_patch_entry():
if key:
patch_path, patch_value = self.make_path_value_jsonpatch_compatible(table, key, value)
else:
patch_path = "/{}".format(table)

gcu_json = {"op": "{}".format(op),
"path": "{}".format(patch_path)}
if op == "add":
gcu_json["value"] = patch_value

gcu_json_input.append(gcu_json)

"""mod_entry makes path more granular so that preexisting fields in db are not removed"""
if mod_entry:
key_start = key
value_copy = copy.deepcopy(value)
for key_end, cleaned_value in value_copy.items():
key = [key_start, key_end]
value = cleaned_value
add_patch_entry()
else:
add_patch_entry()
Copy link
Contributor

@qiluo-msft qiluo-msft Oct 25, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add_patch_entry

For set_entry, is it directly translated to jsonpatch replace op? #Closed

Copy link
Contributor Author

@isabelmsft isabelmsft Oct 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

set_entry and mod_entry both use jsonpatch add op.

The difference between set_entry and mod_entry is that set_entry will remove extra fields in the db which are not in the data. So this difference is captured by differing paths in the jsonpatch. The path for mod_entry is more granular.

Copy link
Contributor Author

@isabelmsft isabelmsft Nov 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

set_entry is not able to be directly translated to jsonpatch replace op because replace does not allow for addition of a field that does not already exist.

For example, if we try to add a new portchannel, we will get the following error:

admin@vlab-01:~$ sudo config apply-patch patch.json
Patch Applier: Patch application starting.
Patch Applier: Patch: [{"op": "replace", "path": "/PORTCHANNEL/PortChannel04", "value": {"admin_status": "up", "mtu": "9100", "lacp_key": "auto", "min_links": "1"}}]
Patch Applier: Getting current config db.
Patch Applier: Simulating the target full config after applying the patch.
Failed to apply patch
Usage: config apply-patch [OPTIONS] PATCH_FILE_PATH
Try "config apply-patch -h" for help.



Error: can't replace a non-existent object 'PortChannel04'


gcu_json_input.append(gcu_json)
gcu_patch = jsonpatch.JsonPatch(gcu_json_input)
return gcu_patch

def apply_patch(self, gcu_patch, 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 EmptyTableError:
self.validated_delete_table(table)

def validated_delete_table(self, table):
gcu_patch = self.create_gcu_patch("remove", table)
format = ConfigFormat.CONFIGDB.name
Expand All @@ -54,17 +105,20 @@ def validated_delete_table(self, table):
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_mod_entry(self, table, key, value):
if value is not None:
op = "add"
else:
op = "remove"

gcu_patch = self.create_gcu_patch(op, table, key, value, mod_entry=True)
self.apply_patch(gcu_patch, table)

def validated_set_entry(self, table, key, value):
if value is not None:
op = "add"
else:
op = "remove"

gcu_patch = self.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:
self.validated_delete_table(table)
gcu_patch = self.create_gcu_patch(op, table, key, value)
self.apply_patch(gcu_patch, table)
Loading