Cisco: handle vrf declared in address-family line (#5157)

And fix a re-entrant bug in vrf block, and add test/update existing ref test.
batfish · Nov 19, 2019 · fce15b6 · fce15b6
1 parent 17b1f98
commit fce15b6
Show file tree

Hide file tree

Showing 4 changed files with 95 additions and 13 deletions.
diff --git a/projects/batfish/src/main/java/org/batfish/grammar/cisco/CiscoControlPlaneExtractor.java b/projects/batfish/src/main/java/org/batfish/grammar/cisco/CiscoControlPlaneExtractor.java
@@ -1599,6 +1599,18 @@ private Vrf currentVrf() {
     return initVrf(_currentVrf);
   }
 
+  private void enterBgpVrfAndPushNewPeer(String vrfName) {
+    _currentVrf = vrfName;
+    Vrf v = currentVrf();
+    BgpProcess p = v.getBgpProcess();
+    if (p == null) {
+      long procNum = _configuration.getDefaultVrf().getBgpProcess().getProcnum();
+      p = new BgpProcess(_format, procNum);
+      v.setBgpProcess(p);
+    }
+    pushPeer(p.getMasterBgpPeerGroup());
+  }
+
   @Override
   public void enterAaa_accounting(Aaa_accountingContext ctx) {
     if (_configuration.getCf().getAaa().getAccounting() == null) {
@@ -1758,7 +1770,11 @@ public void enterAddress_family_header(Address_family_headerContext ctx) {
     if (af.VPNV4() != null || af.VPNV6() != null || af.MDT() != null || af.MULTICAST() != null) {
       pushPeer(_dummyPeerGroup);
     } else {
-      pushPeer(_currentPeerGroup);
+      if (ctx.af.vrf_name != null) {
+        enterBgpVrfAndPushNewPeer(ctx.af.vrf_name.getText());
+      } else {
+        pushPeer(_currentPeerGroup);
+      }
     }
     if (af.IPV6() != null || af.VPNV6() != null) {
       _inIpv6BgpPeer = true;
@@ -4603,12 +4619,7 @@ public void enterViaf_vrrp(Viaf_vrrpContext ctx) {
 
   @Override
   public void enterVrf_block_rb_stanza(Vrf_block_rb_stanzaContext ctx) {
-    _currentVrf = ctx.name.getText();
-    long procNum =
-        _configuration.getVrfs().get(Configuration.DEFAULT_VRF_NAME).getBgpProcess().getProcnum();
-    BgpProcess proc = new BgpProcess(_format, procNum);
-    currentVrf().setBgpProcess(proc);
-    pushPeer(proc.getMasterBgpPeerGroup());
+    enterBgpVrfAndPushNewPeer(ctx.name.getText());
   }
 
   @Override
@@ -4680,6 +4691,11 @@ public void exitAdditional_paths_rb_stanza(Additional_paths_rb_stanzaContext ctx
 
   @Override
   public void exitAddress_family_rb_stanza(Address_family_rb_stanzaContext ctx) {
+    if (ctx.address_family_header() != null
+        && ctx.address_family_header().af != null
+        && ctx.address_family_header().af.vrf_name != null) {
+      _currentVrf = Configuration.DEFAULT_VRF_NAME;
+    }
     popPeer();
   }
 

diff --git a/projects/batfish/src/test/java/org/batfish/grammar/cisco/CiscoGrammarTest.java b/projects/batfish/src/test/java/org/batfish/grammar/cisco/CiscoGrammarTest.java
@@ -52,6 +52,7 @@
 import static org.batfish.datamodel.matchers.ConfigurationMatchers.hasIpsecPhase2Proposal;
 import static org.batfish.datamodel.matchers.ConfigurationMatchers.hasMlagConfig;
 import static org.batfish.datamodel.matchers.ConfigurationMatchers.hasVendorFamily;
+import static org.batfish.datamodel.matchers.ConfigurationMatchers.hasVrf;
 import static org.batfish.datamodel.matchers.ConfigurationMatchers.hasVrfs;
 import static org.batfish.datamodel.matchers.DataModelMatchers.hasAclName;
 import static org.batfish.datamodel.matchers.DataModelMatchers.hasBandwidth;
@@ -1634,7 +1635,7 @@ public void testIosEigrpAddressFamily() throws IOException {
 
     /* Confirm both processes are present */
     assertThat(c, hasDefaultVrf(hasEigrpProcesses(hasKey(1L))));
-    assertThat(c, ConfigurationMatchers.hasVrf("vrf-name", hasEigrpProcesses(hasKey(2L))));
+    assertThat(c, hasVrf("vrf-name", hasEigrpProcesses(hasKey(2L))));
 
     /* Confirm interfaces were matched */
     assertThat(c, hasInterface("Ethernet0", hasEigrp(EigrpInterfaceSettingsMatchers.hasAsn(1L))));
@@ -1651,7 +1652,7 @@ public void testIosEigrpAsn() throws IOException {
 
     /* Confirm both processes are present */
     assertThat(c, hasDefaultVrf(hasEigrpProcesses(hasKey(1L))));
-    assertThat(c, ConfigurationMatchers.hasVrf("vrf-name", hasEigrpProcesses(hasKey(2L))));
+    assertThat(c, hasVrf("vrf-name", hasEigrpProcesses(hasKey(2L))));
 
     /* Confirm interfaces were matched */
     assertThat(c, hasInterface("Ethernet0", hasEigrp(EigrpInterfaceSettingsMatchers.hasAsn(1L))));
@@ -1690,7 +1691,7 @@ public void testIosEigrpMixed() throws IOException {
     assertThat(c, hasInterface("Ethernet0", hasEigrp(EigrpInterfaceSettingsMatchers.hasAsn(1L))));
 
     /* Confirm named mode networks are configured correctly */
-    assertThat(c, ConfigurationMatchers.hasVrf("vrf-name", hasEigrpProcesses(hasKey(2L))));
+    assertThat(c, hasVrf("vrf-name", hasEigrpProcesses(hasKey(2L))));
     assertThat(c, hasInterface("Ethernet1", hasEigrp(EigrpInterfaceSettingsMatchers.hasAsn(2L))));
   }
 
@@ -3622,6 +3623,21 @@ public void testBgpMultipleRouters() throws IOException {
     // Don't crash.
   }
 
+  /** Tests that we can append more BGP config at the bottom of a file. */
+  @Test
+  public void testBgpReentrantVrf() throws IOException {
+    CiscoConfiguration c = parseCiscoConfig("ios-bgp-reentrant-vrf", ConfigurationFormat.CISCO_IOS);
+    // Simple test that default VRF was parsed
+    org.batfish.representation.cisco.BgpProcess defBgp = c.getDefaultVrf().getBgpProcess();
+    assertThat(defBgp.getProcnum(), equalTo(1L));
+
+    // VRF keeps local-as from first declaration and overriden router-id from second.
+    assertThat(c.getVrfs(), hasKey("a"));
+    org.batfish.representation.cisco.BgpProcess vrfBgp = c.getVrfs().get("a").getBgpProcess();
+    assertThat(vrfBgp.getMasterBgpPeerGroup().getLocalAs(), equalTo(5L));
+    assertThat(vrfBgp.getRouterId(), equalTo(Ip.parse("1.2.3.5")));
+  }
+
   @Test
   public void testBgpUndeclaredPeer() throws IOException {
     parseConfig("ios-bgp-undeclared-peer");
@@ -5468,7 +5484,7 @@ public void testAsaStaticRoute() throws IOException {
     // Confirm static route is extracted properly
     assertThat(
         c,
-        ConfigurationMatchers.hasVrf(
+        hasVrf(
             "default",
             hasStaticRoutes(
                 equalTo(

diff --git a/...ts/batfish/src/test/resources/org/batfish/grammar/cisco/testconfigs/ios-bgp-reentrant-vrf b/...ts/batfish/src/test/resources/org/batfish/grammar/cisco/testconfigs/ios-bgp-reentrant-vrf
@@ -0,0 +1,13 @@
+!
+hostname ios-bgp-reentrant-vrf
+!
+router bgp 1
+!
+  vrf a
+    local-as 5
+    router-id 1.2.3.4
+!
+router bgp 1
+  vrf a
+    router-id 1.2.3.5
+!
diff --git a/tests/parsing-tests/unit-tests-vimodel.ref b/tests/parsing-tests/unit-tests-vimodel.ref
@@ -12342,8 +12342,8 @@
         "defaultInboundAction" : "PERMIT",
         "deviceType" : "ROUTER",
         "routingPolicies" : {
-          "~BGP_COMMON_EXPORT_POLICY:default~" : {
-            "name" : "~BGP_COMMON_EXPORT_POLICY:default~",
+          "~BGP_COMMON_EXPORT_POLICY:VRF:MMS:RMT:PBI:145~" : {
+            "name" : "~BGP_COMMON_EXPORT_POLICY:VRF:MMS:RMT:PBI:145~",
             "statements" : [
               {
                 "class" : "org.batfish.datamodel.routing_policy.statement.If",
@@ -12415,6 +12415,31 @@
                 "type" : "ReturnFalse"
               }
             ]
+          },
+          "~BGP_COMMON_EXPORT_POLICY:default~" : {
+            "name" : "~BGP_COMMON_EXPORT_POLICY:default~",
+            "statements" : [
+              {
+                "class" : "org.batfish.datamodel.routing_policy.statement.If",
+                "guard" : {
+                  "class" : "org.batfish.datamodel.routing_policy.expr.MatchProtocol",
+                  "protocols" : [
+                    "bgp",
+                    "ibgp"
+                  ]
+                },
+                "trueStatements" : [
+                  {
+                    "class" : "org.batfish.datamodel.routing_policy.statement.Statements$StaticStatement",
+                    "type" : "ReturnTrue"
+                  }
+                ]
+              },
+              {
+                "class" : "org.batfish.datamodel.routing_policy.statement.Statements$StaticStatement",
+                "type" : "ReturnFalse"
+              }
+            ]
           }
         },
         "vendorFamily" : {
@@ -12426,6 +12451,18 @@
           }
         },
         "vrfs" : {
+          "VRF:MMS:RMT:PBI:145" : {
+            "name" : "VRF:MMS:RMT:PBI:145",
+            "bgpProcess" : {
+              "ebgpAdminCost" : 20,
+              "ibgpAdminCost" : 200,
+              "routerId" : "0.0.0.0",
+              "multipathEbgp" : false,
+              "multipathEquivalentAsPathMatchMode" : "EXACT_PATH",
+              "multipathIbgp" : false,
+              "tieBreaker" : "ARRIVAL_ORDER"
+            }
+          },
           "default" : {
             "name" : "default",
             "bgpProcess" : {