Skip to content
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.

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

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

Merged
merged 17 commits into from
Nov 4, 2022

Conversation

isabelmsft
Copy link
Contributor

@isabelmsft isabelmsft commented Oct 25, 2022

What I did

-Add YANG validation using GCU for writes to TACPLUS, TACPLUS_SERVER, AAA, VLAN_SUB_INTERFACE in ConfigDB
-decorate mod_entry with validated_mod_entry to include YANG validation
-process all values in GCU JsonPatch, convert to string

How I did it

Using same method as https://github.com/sonic-net/sonic-utilities/pull/2190/files, extend to TACPLUS, TACPLUS_SERVER, AAA, VLAN_SUB_INTERFACE use cases

How to verify it

Verified testing CLI on virtual switch

@lgtm-com
Copy link

lgtm-com bot commented Oct 25, 2022

This pull request introduces 1 alert when merging b4f466d into c2841b8 - view on LGTM.com

new alerts:

  • 1 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Oct 25, 2022

This pull request introduces 1 alert when merging c3544a2 into 7e7d05c - view on LGTM.com

new alerts:

  • 1 for Unused import

value = cleaned_value
add_patch_entry()
else:
add_patch_entry()
Copy link
Contributor

@qiluo-msft qiluo-msft Oct 25, 2022

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

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

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'

config/aaa.py Outdated Show resolved Hide resolved
@qiluo-msft
Copy link
Contributor

@preetham-singh Could you help review?

@sonic-net sonic-net deleted a comment from lgtm-com bot Oct 26, 2022
@lgtm-com
Copy link

lgtm-com bot commented Oct 26, 2022

This pull request introduces 1 alert when merging b20dfb9 into 7e7d05c - view on LGTM.com

new alerts:

  • 1 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Oct 28, 2022

This pull request introduces 2 alerts when merging 650e723 into 4a3d49d - view on LGTM.com

new alerts:

  • 1 for Unused import
  • 1 for Wrong number of arguments in a call

@lgtm-com
Copy link

lgtm-com bot commented Oct 28, 2022

This pull request introduces 1 alert when merging 18a5e1a into 4a3d49d - view on LGTM.com

new alerts:

  • 1 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Oct 28, 2022

This pull request introduces 1 alert when merging 0bc9fa7 into 4a3d49d - view on LGTM.com

new alerts:

  • 1 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Nov 1, 2022

This pull request introduces 1 alert when merging 3ce3dbc into e8b1dcd - view on LGTM.com

new alerts:

  • 1 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Nov 1, 2022

This pull request introduces 1 alert when merging 5899db9 into e8b1dcd - view on LGTM.com

new alerts:

  • 1 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Nov 1, 2022

This pull request introduces 1 alert when merging 08ee911 into e8b1dcd - view on LGTM.com

new alerts:

  • 1 for Unused import

@isabelmsft isabelmsft changed the title YANG Validation for ConfigDB Updates: TACPLUS, TACPLUS_SERVER, AAA, VLAN_SUB_INTERFACE tables + decorated validated_mod_entry YANG Validation for ConfigDB Updates: TACPLUS, TACPLUS_SERVER, AAA, VLAN_SUB_INTERFACE, NETWORKING_METADATA tables + decorated validated_mod_entry Nov 1, 2022
@isabelmsft isabelmsft changed the title YANG Validation for ConfigDB Updates: TACPLUS, TACPLUS_SERVER, AAA, VLAN_SUB_INTERFACE, NETWORKING_METADATA tables + decorated validated_mod_entry YANG Validation for ConfigDB Updates: TACPLUS, TACPLUS_SERVER, AAA, VLAN_SUB_INTERFACE, DEVICE_METADATA tables + decorated validated_mod_entry Nov 1, 2022
@isabelmsft isabelmsft changed the title YANG Validation for ConfigDB Updates: TACPLUS, TACPLUS_SERVER, AAA, VLAN_SUB_INTERFACE, DEVICE_METADATA tables + decorated validated_mod_entry YANG Validation for ConfigDB Updates: TACPLUS, TACPLUS_SERVER, AAA, VLAN_SUB_INTERFACE tables + decorated validated_mod_entry Nov 1, 2022
@lgtm-com
Copy link

lgtm-com bot commented Nov 1, 2022

This pull request introduces 1 alert when merging d945fb2 into e8b1dcd - view on LGTM.com

new alerts:

  • 1 for Unused import

@isabelmsft
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@qiluo-msft
Copy link
Contributor

I see a new lgtm alert.

qiluo-msft
qiluo-msft previously approved these changes Nov 2, 2022
@isabelmsft isabelmsft merged commit 58dbb3e into sonic-net:master Nov 4, 2022
preetham-singh pushed a commit to preetham-singh/sonic-utilities that referenced this pull request Nov 21, 2022
…LAN_SUB_INTERFACE tables + decorated validated_mod_entry (sonic-net#2452)
mdanish-kh pushed a commit to mdanish-kh/sonic-utilities that referenced this pull request Nov 23, 2022
…LAN_SUB_INTERFACE tables + decorated validated_mod_entry (sonic-net#2452)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants