OpenConfig: move the transport key one level up (aerleon#311)

* OpenConfig: move the transport key one level up Fixes aerleon#308 * OpenConfig: update tests for transport change
jronnblom · Jun 23, 2023 · 2e85262 · 2e85262
1 parent c7a36a8
commit 2e85262
Show file tree

Hide file tree

Showing 6 changed files with 109 additions and 49 deletions.
diff --git a/aerleon/lib/openconfig.py b/aerleon/lib/openconfig.py
@@ -162,9 +162,9 @@ def ConvertToDict(
  # 'any' starts and ends with zero.
  if not start == end == 0:
  if start == end:
- ace_dict[family]['transport']['config']['source-port'] = int(start)
+ ace_dict['transport']['config']['source-port'] = int(start)
  else:
- ace_dict[family]['transport']['config']['source-port'] = '%d..%d' % (
+ ace_dict['transport']['config']['source-port'] = '%d..%d' % (
  start,
  end,
  )
@@ -173,13 +173,12 @@ def ConvertToDict(
  for start, end in dports:
  if not start == end == 0:
  if start == end:
- ace_dict[family]['transport']['config']['destination-port'] = int(
- start
- )
+ ace_dict['transport']['config']['destination-port'] = int(start)
  else:
- ace_dict[family]['transport']['config'][
- 'destination-port'
- ] = '%d..%d' % (start, end)
+ ace_dict['transport']['config']['destination-port'] = '%d..%d' % (
+ start,
+ end,
+ )
 
  # Protocol
  for proto in protos:
@@ -245,7 +244,6 @@ def _TranslatePolicy(self, pol: Policy, exp_info: int) -> None:
  filter_options.remove(i)
 
  for term in terms:
-
  # TODO(b/196430344): Add support for options such as
  # established/rst/first-fragment
  if term.option:

diff --git a/tests/regression/openconfig/OpenConfigTest.testDport.stdout.ref b/tests/regression/openconfig/OpenConfigTest.testDport.stdout.ref
@@ -8,11 +8,11 @@
  "ipv4": {
  "config": {
  "protocol": 6
- },
-  "transport": {
-  "config": {
-  "destination-port": 53
- }
+ }
+ },
+ "transport": {
+ "config": {
+ "destination-port": 53
  }
  }
  }

diff --git a/tests/regression/openconfig/OpenConfigTest.testEverything.stdout.ref b/tests/regression/openconfig/OpenConfigTest.testEverything.stdout.ref
@@ -10,11 +10,11 @@
  "destination-address": "10.2.3.4/32",
  "protocol": 17,
  "source-address": "10.2.3.4/32"
- },
-  "transport": {
-  "config": {
-  "destination-port": 53
- }
+ }
+ },
+ "transport": {
+ "config": {
+ "destination-port": 53
  }
  }
  },
@@ -29,11 +29,11 @@
  "destination-address": "10.2.3.4/32",
  "protocol": 6,
  "source-address": "10.2.3.4/32"
- },
-  "transport": {
-  "config": {
-  "destination-port": 53
- }
+ }
+ },
+ "transport": {
+ "config": {
+ "destination-port": 53
  }
  }
  }

diff --git a/tests/regression/openconfig/OpenConfigTest.testMultiDport.stdout.ref b/tests/regression/openconfig/OpenConfigTest.testMultiDport.stdout.ref
@@ -0,0 +1,40 @@
+[
+ {
+ "actions": {
+ "config": {
+ "forwarding-action": "ACCEPT"
+ }
+ },
+ "ipv4": {
+ "config": {
+ "protocol": 17
+ }
+ },
+ "transport": {
+ "config": {
+ "destination-port": "1024..65535",
+ "source-port": "1024..65535"
+ }
+ }
+ },
+ {
+ "actions": {
+ "config": {
+ "forwarding-action": "ACCEPT"
+ }
+ },
+ "ipv4": {
+ "config": {
+ "protocol": 6
+ }
+ },
+ "transport": {
+ "config": {
+ "destination-port": "1024..65535",
+ "source-port": "1024..65535"
+ }
+ }
+ }
+]
+
+
diff --git a/tests/regression/openconfig/OpenConfigTest.testSport.stdout.ref b/tests/regression/openconfig/OpenConfigTest.testSport.stdout.ref
@@ -8,11 +8,11 @@
  "ipv4": {
  "config": {
  "protocol": 6
- },
-  "transport": {
-  "config": {
-  "source-port": 53
- }
+ }
+ },
+ "transport": {
+ "config": {
+ "source-port": 53
  }
  }
  }

diff --git a/tests/regression/openconfig/openconfig_test.py b/tests/regression/openconfig/openconfig_test.py
@@ -66,8 +66,9 @@
 
 GOOD_MULTI_PROTO_DPORT = """
 term good-term-1 {
- comment:: "Allow TCP & UDP 53."
- destination-port:: DNS
+ comment:: "Allow TCP & UDP high."
+ source-port:: HIGH_PORTS
+ destination-port:: HIGH_PORTS
  protocol:: udp tcp
  action:: accept
 }
@@ -192,10 +193,11 @@
  "ipv4": {
  "config": {
  "protocol": 6
+ }
  },
  "transport": {
  "config": {
- "source-port": 53}
+ "source-port": 53
  }
  }
  }
@@ -213,10 +215,11 @@
  "ipv4": {
  "config": {
  "protocol": 6
+ }
  },
  "transport": {
  "config": {
- "destination-port": 53}
+ "destination-port": 53
  }
  }
  }
@@ -234,10 +237,12 @@
  "ipv4": {
  "config": {
  "protocol": 17
+ }
  },
  "transport": {
  "config": {
- "destination-port": 53}
+ "destination-port": "1024..65535",
+ "source-port": "1024..65535"
  }
  }
  },
@@ -250,12 +255,14 @@
  "ipv4": {
  "config": {
  "protocol": 6
- },
- "transport": {
- "config": {
- "destination-port": 53}
- }
  }
+ },
+ "transport": {
+ "config": {
+ "destination-port": "1024..65535",
+ "source-port": "1024..65535"
+ }
+ }
  }
 ]
 """
@@ -273,11 +280,11 @@
  "destination-address": "10.2.3.4/32",
  "protocol": 17,
  "source-address": "10.2.3.4/32"
- },
-  "transport": {
-  "config": {
-  "destination-port": 53
- }
+ }
+ },
+ "transport": {
+ "config": {
+ "destination-port": 53
  }
  }
  },
@@ -292,11 +299,11 @@
  "destination-address": "10.2.3.4/32",
  "protocol": 6,
  "source-address": "10.2.3.4/32"
- },
-  "transport": {
-  "config": {
-  "destination-port": 53
- }
+ }
+ },
+ "transport": {
+ "config": {
+ "destination-port": 53
  }
  }
  }
@@ -391,6 +398,21 @@ def testDport(self):
  self.naming.GetServiceByProto.assert_has_calls([mock.call('DNS', 'tcp')])
  print(acl)
 
+ @capture.stdout
+ def testMultiDport(self):
+ self.naming.GetServiceByProto.return_value = ['1024-65535']
+
+ acl = openconfig.OpenConfig(
+ policy.ParsePolicy(GOOD_HEADER + GOOD_MULTI_PROTO_DPORT, self.naming), EXP_INFO
+ )
+ expected = json.loads(GOOD_JSON_MULTI_PROTO_DPORT)
+ self.assertEqual(expected, json.loads(str(acl)))
+
+ self.naming.GetServiceByProto.assert_has_calls(
+ [mock.call('HIGH_PORTS', 'tcp'), mock.call('HIGH_PORTS', 'udp')], any_order=True
+ )
+ print(acl)
+
  @capture.stdout
  def testEverything(self):
  self.naming.GetServiceByProto.side_effect = [['53'], ['53']]