Open Stack

In the current AddRule form, validate_port_range is used as a validator,
so a value from 1 to 65535 is accepted.
I think this is the reason that _clean_rule_icmp check the exact value range.
You can improve it as you think.

bug 1511748 is a regression of validate_port_range change in
https://review.openstack.org/#/c/116508/.
Previously it accepts -1 and we can use it both for TCP/UDP and ICMP,
but after that change it no longer cannot be applied to ICMP.

IMO we need to use separate validators for TCP/UDP and ICMP.

Akihiro



2015-11-19 2:05 GMT+09:00 Suraj Deshmukh <surajdeshmukh9595 at gmail.com>:
> In file [1] in `class AddRule` in method `_clean_rule_icmp` why checkups are
> performed on if `icmp_type` or `icmp_code` is `None` or in range of `-1 to
> 255`.
>
> What I mean here is that in `class AddRule` while values are being accepted
> from form data and being stored, validators do their job of checking whether
> that field is valid, so why a redundant checkup in method
> `_clean_rule_icmp`?
>
> Please correct me if I am wrong in understanding anything. Currently I am
> working on the Bug #1511748 [2]. Previously while checking the validity of
> `icmp_type` and `icmp_code`, the functionality of tcp_ports was used. This
> is wrong because, TCP ports have a range of 0 to 65535 while `icmp_type` and
> `icmp_code` have a range of 0 to 255.
>
> So now oslo_utils.netutils has dedicated functionality to check if
> `icmp_type` and `icmp_code` are valid here is a recent code merger [3].
>
> So I was trying to add this newly added functionality into Horizon but the
> test cases run older code and hence needed help in getting my head around
> with the source.
>
>
> [1]
> openstack_dashboard/dashboards/project/access_and_security/security_groups/forms.py
> [2] https://bugs.launchpad.net/horizon/+bug/1511748
> [3] https://review.openstack.org/#/c/240661/
>
> Thanks and Regards
>
> __________________________________________________________________________
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>