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

Adding support for Cisco ASA banner when nested underneath a parent config #590

Closed
wants to merge 2 commits into from

Conversation

huacchob
Copy link

Banner for Cisco ASA could be a child config, and would start with a white space. I added that support to both the BaseSpaceConfigParser.banner_start list and to CiscoConfigParser.regex_banner attribute

…white space. I added that support to both the BaseSpaceConfigParser.banner_start list and to CiscoConfigParser.regex_banner attribute
@itdependsnetworks
Copy link
Contributor

hey, can you provide test cases for this?

@huacchob
Copy link
Author

Yes, I can add them

@huacchob
Copy link
Author

Would this test be enough?

@itdependsnetworks
Copy link
Contributor

Would you mind just using the existing folder structure? Should just be here: https://github.com/networktocode/netutils/tree/3c1f8de1a8da7d0cc86b6e5cdc7e79dd49d55369/tests/unit/mock/config/parser/base/cisco_asa

@@ -53,7 +53,7 @@ class BaseSpaceConfigParser(BaseConfigParser):
# pylint: disable=abstract-method

comment_chars = ["!"]
banner_start = ["banner", "vacant-message"]
banner_start = ["banner", "vacant-message", " banner", " banner"]
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand this, why are we adding banner_start multiple times?

Copy link
Author

Choose a reason for hiding this comment

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

The first one has has 1 white space and the second one has two.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would not think this is how we want to do this then. I would think we would understand the parent / child relationship would be how we determine it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Check in with @jmcgill298

Copy link
Author

Choose a reason for hiding this comment

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

Understood, thank you

Copy link
Contributor

Choose a reason for hiding this comment

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

ya, that banner is a child of the group-policy, so it should be captured there. I guess for this case the banner is not multiline?

Copy link
Author

Choose a reason for hiding this comment

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

The banner is a bit odd. It seems that each line of the banner begins with "banner value". Yes, it is a child config of group-policy

@huacchob huacchob closed this Oct 30, 2024
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.

4 participants