[fix] Openwrt backend validation problem for property ip_rules/src #96 #160
Conversation
| "(:(((:[0-9A-Fa-f]{1,4}){1,7})|((:[0-9A-Fa-f]{1,4}){0,5}:((25[0-5]|2[0-4]d|1dd|[1-9]?d)" | ||
| "(.(25[0-5]|2[0-4]d|1dd|[1-9]?d)){3}))|:)))(%.+)?s*(\\/(12[0-8]|1[0-1][0-9]|[1-9][0-9]|[0-9])))|" | ||
| "(((^|\\.)((25[0-5])|(2[0-4]\\d)|(1\\d\\d)|([1-9]?\\d))){4}\\/(?:\\d|[12]\\d|3[01]))$" | ||
| ) |
There was a problem hiding this comment.
This is huge!
Let's simplify, we don't need to explicitly support ipv4 or ipv6, because it is possible to express a single IP using the CIDR notation, eg:
192.168.1.2/32 is equivalent to 192.168.1.2
Please shorten this as much as you can, because this is shown to the user in openwisp-controller.
There was a problem hiding this comment.
Instead of these self defined pattern, I could just use the jsonschema ipv4 and ipv6 format validation. But the problem is that these formats doesn't support cidr notation. That's 192.168.1.2/32 will be invalid while 192.168.1.2 will be valid. And this will go against the description which states that src should be in cidr notation.
Should I use these formats or try to shorten the one above which ensures the src and dest are in valid cidr notations
| @@ -8,6 +8,22 @@ | |||
|
|
|||
| default_radio_driver = "mac80211" | |||
|
|
|||
| # pattern to match ipv4 and ipv6 in CIDR notation | |||
| src_pattern = ( | |||
There was a problem hiding this comment.
this should be called cidr_pattern
| # fix 'dest' and expect no ValidationError raised | ||
| rule['ip_rules'][0]['dest'] = '192.168.1.1/24' | ||
| o = OpenWrt(rule) | ||
| o.validate() |
There was a problem hiding this comment.
please use subtests (or write another test) to ensure the pattern works with several use cases (try at least a few ipv4 and ipv6 addresses)
Fixed issue by defining a new format checker for CIDR notation. Fixes #96
NoumbissiValere
force-pushed
the
issues/96
branch
from
7d6aa0a
to
cadb36b
Compare
| @@ -122,6 +123,14 @@ def _deduplicate_files(self): | |||
| files_dict[file['path']] = file | |||
| self.config['files'] = list(files_dict.values()) | |||
|
|
|||
| @draft4_format_checker.checks('cidr', AssertionError) | |||
Fixed the issue by adding a pattern to match the valid src and dest notation.
Fixes #96