flowspy
flowspy copied to clipboard
Support port ranges
The user should be able to specify a port range instead of single ports when adding rules. Any discussion / implementation details will be mentioned here.
Current implementation in https://github.com/cejkato2/flowspy/tree/removeaddport uses models.CharField()
for sourceport
, destinationport
, port
.
Users are allowed to enter list of ports or port ranges.
Example of a valid input is: 80,100-120,443
From my opinion, there is no need to store ports separatly in DB because it makes no sense to select previously used values by user - it is much faster to write down the numbers than to look it up from some list.
Additionally, it was very user unfriendly to use a special form to add a port before it could be used e.g. in sourceport
.
I understand your issues. However, there are some concerns about the whole CharField
approach:
-
Validation is a problem. You need to validate a specific format, which includes dashes and commas and so on. This can break quite easily, and you have to validate this on multiple levels (django admin, django shell, API serializers & so on). And then there's the database level where you cannot actually do any validation of such sort (that would be ensure that the string entered contains the required dashes and commas and makes sence).
-
It is not really that "clean" as an approach, since you can no longer do SQL queries to find routes affecting ports. E.g. if a
Route
has aport
field that contains the string80, 100-120, 443
and you want to query for all theRoute
s affecting port114
, you can't do that. You will have to iterate allRoute
objects and find the ones that satisfy your criterion with python. So we are kinda throwing all the SQL performance out the window (in such queries at least).
These are the major issues I see with this approach. Instead, I would propose that we:
- Keep the
MatchPort
model, but upgrade it to support port ranges. RenameMatchPort.port
toMatchPort.port_start
and addMatchPort.port_end
. These should be bothIntegerField
s to allow SQL queries. - Create a data migration to reform all existing
MatchPort
objects - Introduce the required serializers & validators.
As far as the user experience issues you mentioned, this is a different aspect I believe. Even with this
approach, we could add a custom form where the user adds ports in a string format (just as you specified) and the back end parses that and creates the required MatchPort
objects.
As far as the API is concerned, we could do the same thing. That would be:
- Create a custom field for each of the
port
,sourceport
,destinationport
- Use
to_internal_value
to parse the string & create the requiredMatchPort
objects - Use
to_representation
to read the associated objects from the database & create that string
This will allow the user to do POST
requests using a string and when they GET
from the API the same string will be sent for those fields.
I am not that sure about the API though, I think maybe we could expose the models as they are there.
Let me know what you think.