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-models] Update sonic-vlan.yang to better constrain Vlan name #18207

Merged

Conversation

matiAlfaro
Copy link
Contributor

@matiAlfaro matiAlfaro commented Feb 28, 2024

~
Related to #3155
Update sonic-vlan.yang model to match requirement as described in #16975

Why I did it

Work item tracking
  • Microsoft ADO (number only):

How I did it

Update Vlan name regex
Update runVlanSpecialTest to skip Vlan0 (as it is illegal)

How to verify it

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • 202205
  • 202211
  • 202305

Tested branch (Please provide the tested image version)

master

Description for the changelog

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

~

Signed-off-by: matiAlfaro <mati@marvell.com>
@matiAlfaro matiAlfaro changed the title Update sonic-vlan.yang to better constrain Vlan name [Yang-models] Update sonic-vlan.yang to better constrain Vlan name Feb 28, 2024
@isabelmsft
Copy link
Contributor

isabelmsft commented Feb 29, 2024

Thanks for the effort to improve sonic-vlan.yang!

@@ -178,7 +182,7 @@ module sonic-vlan {

leaf name {
type string {
pattern 'Vlan([0-9]{1,3}|[1-3][0-9]{3}|[4][0][0-8][0-9]|[4][0][9][0-4])';

This comment was marked as resolved.

Copy link
Contributor Author

@matiAlfaro matiAlfaro Feb 29, 2024

Choose a reason for hiding this comment

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

The original regex allows for Vlan0, Vlan00, Vlan000, Vlan01 ext...
I'll have a go at #12256

Copy link
Contributor

Choose a reason for hiding this comment

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

you're right, the first group in original regex was too broad. And thanks!

@isabelmsft isabelmsft self-requested a review February 29, 2024 06:49
Copy link
Contributor

@wen587 wen587 left a comment

Choose a reason for hiding this comment

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

lgtm

@lguohan lguohan added the YANG YANG model related changes label Mar 30, 2024
@lguohan lguohan merged commit 814badb into sonic-net:master Mar 30, 2024
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
YANG YANG model related changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants