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

Fix 'forwarding-options dhcp-security' parse warning for Juniper #8828

Merged
merged 6 commits into from
Oct 4, 2023
Merged

Fix 'forwarding-options dhcp-security' parse warning for Juniper #8828

merged 6 commits into from
Oct 4, 2023

Conversation

saidvandeklundert
Copy link
Contributor

@saidvandeklundert saidvandeklundert commented Oct 3, 2023

A 'Parse warning' is being triggered by the following configuration:

set vlans <name> forwarding-options dhcp-security

The dhcp-security configuration is valid only under the ‘forwarding-options’ inside the vlans stanza.

This PR addresses the parse warning by extending the lexer/parser.

The configuration statement is documented here

@batfish-bot
Copy link

This change is Reviewable

@dhalperi
Copy link
Member

dhalperi commented Oct 3, 2023

Because I'm lazy, I'll just point you at the review I left you on the prior PR https://reviewable.io/reviews/batfish/batfish/8827 :)

@saidvandeklundert
Copy link
Contributor Author

Updated the description of the PR.

Removed lexer/parser rules for the grammar outside the vlans hierarchy since the Juniper syntax is not valid anywhere else.

Also worked on the other comments by reworking the parser rules and the naming of the rules.

Copy link
Member

@dhalperi dhalperi left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 4 files at r1, 1 of 2 files at r2, 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @saidvandeklundert)


projects/batfish/src/main/antlr4/org/batfish/grammar/flatjuniper/FlatJuniperParser.g4 line 183 at r3 (raw file):

;

vlt_forwarding_options

can you move this code block to be located after vlt_filter in the file?


projects/batfish/src/main/antlr4/org/batfish/grammar/flatjuniper/FlatJuniperParser.g4 line 192 at r3 (raw file):

;

vlt_fo_null

vltfo -- the prefix is "underscore-less" so that it's clear where the token being recognized begins.


projects/batfish/src/main/antlr4/org/batfish/grammar/flatjuniper/FlatJuniperParser.g4 line 206 at r3 (raw file):

    | vlt_description
    | vlt_filter
    | vlt_forwarding_options    

nit: extra whitespace here.

@dhalperi
Copy link
Member

dhalperi commented Oct 4, 2023

projects/batfish/src/main/antlr4/org/batfish/grammar/flatjuniper/FlatJuniperParser.g4 line 206 at r3 (raw file):

Previously, dhalperi (Dan Halperin) wrote…

nit: extra whitespace here.

(four trailing spaces)

@saidvandeklundert
Copy link
Contributor Author

Code block moved, vltfo_ renamed according to the standard, trailing spaces removed and ran code formatter.

Copy link
Member

@dhalperi dhalperi left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @saidvandeklundert)

@dhalperi dhalperi enabled auto-merge (squash) October 4, 2023 13:00
@dhalperi dhalperi merged commit 6f8bde5 into batfish:master Oct 4, 2023
6 of 7 checks passed
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.

None yet

3 participants