diff --git a/netjsonconfig/backends/openwrt/converters/firewall.py b/netjsonconfig/backends/openwrt/converters/firewall.py index c4567784d..a45cbd440 100644 --- a/netjsonconfig/backends/openwrt/converters/firewall.py +++ b/netjsonconfig/backends/openwrt/converters/firewall.py @@ -131,8 +131,10 @@ def __intermediate_redirects(self, redirects): redirect["proto"] = proto[0] elif set(proto) == {"tcp", "udp"}: redirect["proto"] = "tcpudp" + resultdict.update(redirect) result.append(resultdict) + return result def __get_auto_name_redirect(self, redirect): @@ -203,5 +205,7 @@ def __netjson_redirect(self, redirect): redirect["proto"] = ["tcp", "udp"] else: redirect["proto"] = [proto] + if "monthdays" in redirect: + redirect["monthdays"] = [int(x) for x in redirect["monthdays"]] return self.type_cast(redirect) diff --git a/netjsonconfig/backends/openwrt/schema.py b/netjsonconfig/backends/openwrt/schema.py index ec427ea2c..a20ff7f46 100644 --- a/netjsonconfig/backends/openwrt/schema.py +++ b/netjsonconfig/backends/openwrt/schema.py @@ -873,27 +873,44 @@ "pattern": time_regex, "propertyOrder": 17, }, - # FIXME: regex needed. Also, should this be an array? + # Note: here we don't support negation of values like + # the UCI syntax does, as it's not necessary. "weekdays": { - "type": "string", + "type": "array", "title": "weekdays", "description": "Only match traffic during the given week days, " - 'e.g. "sun mon thu fri" to only match on Sundays, Mondays, Thursdays and ' - "Fridays. The list can be inverted by prefixing it with an exclamation " - 'mark, e.g. "! sat sun" to always match but not on Saturdays and ' - "Sundays.", + 'e.g. ["sun", "mon", "thu", "fri"] to only match on Sundays, ' + "Mondays, Thursdays and Fridays.", "propertyOrder": 18, + "items": { + "type": "string", + "title": "weekday", + "enum": [ + "mon", + "tue", + "wed", + "thu", + "fri", + "sat", + "sun", + ], + }, }, - # FIXME: regex needed. Also, should this be an array? + # Note: here we don't support negation of values like + # the UCI syntax does, as it's not necessary. "monthdays": { - "type": "string", + "type": "array", "title": "monthdays", "description": "Only match traffic during the given days of the " - 'month, e.g. "2 5 30" to only match on every 2nd, 5th and 30th ' - "day of the month. The list can be inverted by prefixing it with " - 'an exclamation mark, e.g. "! 31" to always match but on the ' - "31st of the month.", + "month, e.g. [2, 5, 30] to only match on every 2nd, 5th and 30th " + "day of the month.", "propertyOrder": 19, + "items": { + "type": "integer", + "title": "day of month", + "minimum": 1, + "maximum": 31, + }, }, "utc_time": { "type": "boolean", diff --git a/tests/openwrt/test_firewall.py b/tests/openwrt/test_firewall.py index 478336d0b..0fe7ca1eb 100644 --- a/tests/openwrt/test_firewall.py +++ b/tests/openwrt/test_firewall.py @@ -413,3 +413,74 @@ def test_render_redirect_1(self): def test_parse_redirect_1(self): o = OpenWrt(native=self._redirect_1_uci) self.assertEqual(o.config, self._redirect_1_netjson) + + _redirect_2_netjson = { + "firewall": { + "redirects": [ + { + "name": "Adblock DNS, port 53", + "src": "lan", + "proto": ["tcp", "udp"], + "src_dport": "53", + "dest_port": "53", + "target": "DNAT", + # Contrived, unrealistic example for testing + "weekdays": ["mon", "tue", "wed"], + "monthdays": [1, 2, 3, 29, 30], + } + ] + } + } + + _redirect_2_uci = textwrap.dedent( + """\ + package firewall + + config defaults 'defaults' + + config redirect 'redirect_Adblock DNS, port 53' + option name 'Adblock DNS, port 53' + option src 'lan' + option proto 'tcpudp' + option src_dport '53' + option dest_port '53' + option target 'DNAT' + list weekdays 'mon' + list weekdays 'tue' + list weekdays 'wed' + list monthdays '1' + list monthdays '2' + list monthdays '3' + list monthdays '29' + list monthdays '30' + """ + ) + + def test_render_redirect_2(self): + o = OpenWrt(self._redirect_2_netjson) + expected = self._tabs(self._redirect_2_uci) + self.assertEqual(o.render(), expected) + + def test_parse_redirect_2(self): + o = OpenWrt(native=self._redirect_2_uci) + self.assertEqual(o.config, self._redirect_2_netjson) + + def test_redirect_weekdays_validation_error_1(self): + o = OpenWrt({"firewall": {"redirects": [{"weekdays": ["mon", "xxx"]}]}}) + with self.assertRaises(ValidationError): + o.validate() + + def test_redirect_weekdays_validation_error_2(self): + o = OpenWrt({"firewall": {"redirects": [{"weekdays": ["mon", 1]}]}}) + with self.assertRaises(ValidationError): + o.validate() + + def test_redirect_monthdays_validation_error_1(self): + o = OpenWrt({"firewall": {"redirects": [{"monthdays": [2, 8, 32]}]}}) + with self.assertRaises(ValidationError): + o.validate() + + def test_redirect_monthdays_validation_error_2(self): + o = OpenWrt({"firewall": {"redirects": [{"monthdays": [0, 2, 8]}]}}) + with self.assertRaises(ValidationError): + o.validate()