Junos: fix name-server

[zergling-aws]

Parsing Junos config set system name-server <ip> fails. This allows it.

[batfish-bot]

This change is 

[codecov]

Codecov Report

Merging #8839 (6e198cf) into master (bce462e) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 6e198cf differs from pull request most recent head 8ca7acf. Consider uploading reports for the commit 8ca7acf to get more accurate results

@@           Coverage Diff           @@
##           master    #8839   +/-   ##
=======================================
  Coverage   72.39%   72.39%           
=======================================
  Files        3295     3295           
  Lines      168869   168869           
  Branches    19804    19804           
=======================================
+ Hits       122259   122261    +2     
- Misses      37499    37502    +3     
+ Partials     9111     9106    -5     

Files	Coverage Δ
...fish/grammar/flatjuniper/ConfigurationBuilder.java	67.53% <100.00%> (+0.10%)	⬆️

... and 5 files with indirect coverage changes

[dhalperi]

Reviewed 5 of 5 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @zergling-aws)

---

projects/batfish/src/main/antlr4/org/batfish/grammar/flatjuniper/FlatJuniper_system.g4 line 64 at r1 (raw file):

sy_name_server
:
  NAME_SERVER server = ip_address

We learned via investigation that source-address ip_address is actually still valid, just optional.

To support that in ANTLR4, you would do (SOURCE_ADDRESS src = ip_address)?

Please update the test file with a new line that includes a new name-server and source-address, then check that both different name-servers are extracted.

[zergling-aws]

projects/batfish/src/main/antlr4/org/batfish/grammar/flatjuniper/FlatJuniper_system.g4 line 64 at r1 (raw file):

Previously, dhalperi (Dan Halperin) wrote…

We learned via investigation that source-address ip_address is actually still valid, just optional.

To support that in ANTLR4, you would do (SOURCE_ADDRESS src = ip_address)?

Please update the test file with a new line that includes a new name-server and source-address, then check that both different name-servers are extracted.

IIUC, the changed line would be:
NAME\_SERVER server \= junos\_name (SOURCE\_ADDRESS src \= ip\_address)?

[dhalperi]

projects/batfish/src/main/antlr4/org/batfish/grammar/flatjuniper/FlatJuniper_system.g4 line 64 at r1 (raw file):

Previously, zergling-aws wrote…

IIUC, the changed line would be:
NAME\_SERVER server \= junos\_name (SOURCE\_ADDRESS src \= ip\_address)?

weird escaping, but otherwise yes

[zergling-aws]

projects/batfish/src/main/antlr4/org/batfish/grammar/flatjuniper/FlatJuniper_system.g4 line 64 at r1 (raw file):

Previously, dhalperi (Dan Halperin) wrote…

weird escaping, but otherwise yes

Build fails with an error

Building projects/batfish/src/main/java/org/batfish/grammar/flatjuniper/libflatjuniper.jar (18 source files) failed: (Exit 1): java failed: error executing command (from target //projects/batfish/src/main/java/org/batfish/grammar/flatjuniper:flatjuniper) external/remotejdk11_macos/bin/java -XX:-CompactStrings '--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED' '--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED' ... (remaining 18 arguments skipped)
Note: Some messages have been simplified; recompile with -Xdiags:verbose to get full output
projects/batfish/src/main/java/org/batfish/grammar/flatjuniper/ConfigurationBuilder.java:7028: error: incompatible types: Junos_nameContext cannot be converted to Ip_addressContext
    String hostname = toIp(ctx.server).toString();

That's due to the change I earlier made here. It looks like when the optional source IP address is present, I need to create the hostname like in the original code. How do I detect this condition to handle it differently?

[dhalperi]

projects/batfish/src/main/antlr4/org/batfish/grammar/flatjuniper/FlatJuniper_system.g4 line 64 at r1 (raw file):

Previously, zergling-aws wrote…

Build fails with an error

Building projects/batfish/src/main/java/org/batfish/grammar/flatjuniper/libflatjuniper.jar (18 source files) failed: (Exit 1): java failed: error executing command (from target //projects/batfish/src/main/java/org/batfish/grammar/flatjuniper:flatjuniper) external/remotejdk11_macos/bin/java -XX:-CompactStrings '--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED' '--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED' ... (remaining 18 arguments skipped)
Note: Some messages have been simplified; recompile with -Xdiags:verbose to get full output
projects/batfish/src/main/java/org/batfish/grammar/flatjuniper/ConfigurationBuilder.java:7028: error: incompatible types: Junos_nameContext cannot be converted to Ip_addressContext
    String hostname = toIp(ctx.server).toString();

That's due to the change I earlier made here. It looks like when the optional source IP address is present, I need to create the hostname like in the original code. How do I detect this condition to handle it differently?

I checked out your branch and made the described change and it compiles fine:

diff --git a/projects/batfish/src/main/antlr4/org/batfish/grammar/flatjuniper/FlatJuniper_system.g4 b/projects/batfish/src/main/antlr4/org/batfish/grammar/flatjuniper/FlatJuniper_system.g4
index 83a81a7e46..9ea13395ef 100644
--- a/projects/batfish/src/main/antlr4/org/batfish/grammar/flatjuniper/FlatJuniper_system.g4
+++ b/projects/batfish/src/main/antlr4/org/batfish/grammar/flatjuniper/FlatJuniper_system.g4
@@ -61,7 +61,7 @@ sy_host_name
 
 sy_name_server
 :
-  NAME_SERVER server = ip_address
+  NAME_SERVER server = ip_address (SOURCE_ADDRESS src_ip = ip_address)?
 ;
 
 sy_ntp

[dhalperi]

Reviewed 3 of 3 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @zergling-aws)

---

projects/batfish/src/test/java/org/batfish/grammar/flatjuniper/FlatJuniperGrammarTest.java line 2728 at r2 (raw file):

    Configuration config = parseConfig("name-server");
    assertEquals(config.getDnsServers().toArray()[0], "1.2.3.4");
    assertEquals(config.getDnsServers().toArray()[1], "2.0.0.0");

Use assertThat still. You can either use contains(1.2.3.4, 2.0.0.0) or containsInAnyOrder(1.2.3.4, 2.0.0.0) -- the latter if the order is not guaranteed.

Code quote:

    assertEquals(config.getDnsServers().toArray()[0], "1.2.3.4");
    assertEquals(config.getDnsServers().toArray()[1], "2.0.0.0");

[dhalperi]

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @zergling-aws)

[dhalperi]

Reviewed 3 of 3 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @zergling-aws)

[dhalperi]

Reviewed 1 of 1 files at r5, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @zergling-aws)