Accepting port ranges when creating firewall rules

[andreisusanu]

API definition: port string (optional) TCP/UDP only. This field can be an integer value specifying a port or a colon separated port range.

@see https://www.vultr.com/api/#firewall

[malc0mn]

Thanks for bringing this up, but I would prefer to separate the ports in that case, something along the lines of:

<?php
public function createRule(
        $groupId,
        $ipType,
        $protocol,
        $subnet,
        $subnetSize,
        $port = null,
        $portEnd = null,
        $direction = 'in'
    ) {	    ) {

        // ...

        if ($port !== null) {
            $args['port'] = (int) $port;
            if ($portEnd !== null) {
                $args['port'] .= ':' . $portEnd;
            }
        }

        // ...

Ideally, this will also throw an exception when using ports with protocol icmp or gre. If you could adjust the PR, then we have a nice fix ;-)

[andreisusanu]

Thanks for checking this out.

I see two BIG issues:

1. There is a limit of maximum 50 rules for each Vultr Firewall Group. So using your solution is not OK.
   As example, if I will need to whitelist 50 diffrente IPs for ports 5901, 5902, 5903, I will hit the limit.
2. If the API allows us to use port ranges, as string, it's just wrong that the library removes this option.

I think the case is clear, and the library should allow port ranges when creating firewall rules.

Thanks again!

[malc0mn]

Just want to understand your comment properly as I'm not familiar with the Vultr firewall rules feature...

1. How will the solution I propose hit the limit of max 50 rules per group? For a port range You would just be calling:

<?php

createRule(5, 'v4', 'tcp', '10.234.22.0', 24, 5090, 5100);

Which will cover 11 ports in this case ranging from 5090 up to and including 5100.

2. The library is a PHP implementation (abstraction if you will) of the Vultr API which should be implemented in such a way that the user of this library should not have to care about how the Vultr API works and receives its data.
   If the Vultr API wants 5090:5100 then the library will make sure of that. If Vultr would ever decide that it would be best to drop the colon separated port ranges and split them up in two arguments, then we adjust the library to match and all implementations of this library will work as expected immediately when upgrading to this new version without the need for the users implementing this library to modify their own code.