Skip to content

Conversation

@kobayashi
Copy link
Contributor

@kobayashi kobayashi commented Jan 5, 2020

Fixes: #3525

This makes it possible to filter by mpultiple address terms for ipaddress even w/ or w/o subnet.

$ curl -X GET -H "Accept: application/json; indent=4" -H "Authorization: Token 0123456789abcdef0123456789abcdef01234567" "http://127.0.0.1:8000/api/ipam/ip-addresses/?address=192.168.1.1/24&address=192.168.1.2/24&address=192.168.1.3/24" 
{
    "count": 3,
    "next": null,
    "previous": null, 
    "results": [...]
}

@kobayashi kobayashi changed the title Fixes: 3525 Fixes: #3525 Jan 5, 2020
@jeremystretch
Copy link
Member

It seems like we're mixing up two different function here:

  1. Enable querying for multiple address values; e.g. ?address=192.0.2.1&address=192.0.2.2
  2. Enable querying for all addresses within a given prefix, e.g. ?address=192.0.2.0/24

#3525 was scoped specifically to the first function; any other feature would need a new FR first. (However, I think it would make sense to extend this same behavior to the prefix field on PrefixFilter for consistency.)

Regarding the second function, this would need to be provided via a separate filter, as filtering by address implies that you are searching for a specific address. For example, a user might not expect a query for address=192.0.2.1/24 to return anything but that one specific IP.

PrefixFilter is a good reference for this: prefix performs an exact match, whereas within and within_include allowing searching by a given containing prefix. IMO it would make sense to introduce a within filter for the IPAddress model (but again, that would be a separate FR).

@kobayashi
Copy link
Contributor Author

The PR does not provide prefix filter function. This change extends the current ipaddress filtering method.
If we make a query like address=192.0.2.1/24&address=192.0.2.2, it returnes a specific 2 items.

Copy link
Member

@jeremystretch jeremystretch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just added a whole bunch of tests for filtersets. Can you please expand the test case IPAddressFilter for to test the new multi-value behavior? Per my comment inline, make sure that it checks three cases for mask length: matching, not matching, and not specified.

@kobayashi kobayashi changed the title Fixes: #3525 Fixes #3525: Filter muiltiple ipaddress terms Jan 10, 2020
@kobayashi
Copy link
Contributor Author

fixes issue of filtering ipaddress with subnet and adds ipaddress filter tests.

@jeremystretch jeremystretch merged commit d96f474 into netbox-community:develop Jan 14, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Apr 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow multiple search terms when searching for IP addresses

2 participants