Fix Github Issue google#187 with max backwards-compat (retaining supp…

…ort for 'fragments')
mpenning · May 30, 2020 · 66aac91 · 66aac91
1 parent 5e693b5
commit 66aac91
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 2 deletions.
diff --git a/capirca/lib/cisco.py b/capirca/lib/cisco.py
@@ -735,7 +735,8 @@ def __str__(self):
         and ('tcp-established' in opts or 'established' in opts)):
       if 'established' not in self.options:
         self.options.append('established')
-    if ('ip' in protocol) and ('fragments' in opts):
+    # Using both 'fragments' and 'is-fragment', ref Github Issue #187
+    if ('ip' in protocol) and (('fragments' in opts) or ('is-fragment' in opts)):
       if 'fragments' not in self.options:
         self.options.append('fragments')
 
@@ -1049,6 +1050,7 @@ def _BuildTokens(self):
 
     supported_sub_tokens.update({'option': {'established',
                                             'tcp-established',
+                                            'is-fragment',
                                             'fragments'},
                                  # Warning, some of these are mapped
                                  # differently. See _ACTION_TABLE

diff --git a/tests/lib/cisco_test.py b/tests/lib/cisco_test.py
@@ -324,6 +324,14 @@
   action:: accept
 }
 """
+GOOD_TERM_22 = """
+term good_term_22 {
+  source-address:: SOME_HOST
+  destination-address:: SOME_HOST
+  option:: is-fragment
+  action:: accept
+}
+"""
 LONG_COMMENT_TERM = """
 term long-comment-term {
   comment:: "%s "
@@ -406,6 +414,7 @@
     },
     'option': {'established',
                'tcp-established',
+               'is-fragment',
                'fragments'}
 }
 
@@ -819,7 +828,8 @@ def testProtoInts(self):
     self.failUnless('permit udp any any range 1024 65535' in str(acl),
                     str(acl))
 
-  def testFragments(self):
+  def testFragments_01(self):
+    """Test policy term using 'fragments' (ref Github issue #187)"""
     self.naming.GetNetAddr.return_value = [nacaddr.IP('10.0.0.0/24')]
     acl = cisco.Cisco(policy.ParsePolicy(GOOD_HEADER + GOOD_TERM_20,
                                          self.naming), EXP_INFO)
@@ -829,6 +839,17 @@ def testFragments(self):
     self.naming.GetNetAddr.assert_has_calls([mock.call('SOME_HOST'),
                                              mock.call('SOME_HOST')])
 
+  def testFragments_02(self):
+    """Test policy term using 'is-fragment' (ref Github issue #187)"""
+    self.naming.GetNetAddr.return_value = [nacaddr.IP('10.0.0.0/24')]
+    acl = cisco.Cisco(policy.ParsePolicy(GOOD_HEADER + GOOD_TERM_22,
+                                         self.naming), EXP_INFO)
+    expected = 'permit ip 10.0.0.0 0.0.0.255 10.0.0.0 0.0.0.255 fragments'
+    self.failUnless(expected in str(acl), str(acl))
+
+    self.naming.GetNetAddr.assert_has_calls([mock.call('SOME_HOST'),
+                                             mock.call('SOME_HOST')])
+
   def testTermDSCPMarker(self):
     self.naming.GetNetAddr.return_value = [nacaddr.IP('10.0.0.0/24')]
     acl = cisco.Cisco(policy.ParsePolicy(GOOD_HEADER + GOOD_TERM_21,