diff --git a/aerleon/lib/openconfig.py b/aerleon/lib/openconfig.py index 10232e4e..de9888c8 100644 --- 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 index af2823d1..347b0161 100644 --- 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 index 514ab20d..e9c0fc12 100644 --- 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 new file mode 100644 index 00000000..1fb28bc0 --- /dev/null +++ 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 index ee93235c..7c639a0f 100644 --- 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 index 0cdb507d..d746b5eb 100644 --- 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']]