Skip to content

Commit 97f1e8b

Browse files
authored
Fixes #695: Filter validation (#697)
* Validating filters on filter Endpoint * Refactor validation. Tests and docs are incomplete * Tests * Tests do pass * Linting with black * Fix error message * Corrections after review, add infos in README.md * Typo * Remove unused boilerplate * Remove unused import --------- Co-authored-by: m0nkey c0de <m0nkey@overfl0w.net>
1 parent c159a76 commit 97f1e8b

File tree

8 files changed

+211
-12
lines changed

8 files changed

+211
-12
lines changed

README.md

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,30 @@ nb = pynetbox.api(
7373
threading=True,
7474
)
7575
```
76+
### Filters validation
77+
78+
NetBox doesn't validate filters passed to the GET API endpoints, which are accessed with `.get()` and `.filter()`. If a filter is incorrect, NetBox silently returns the entire database table content. Pynetbox allows to check provided parameters against NetBox OpenAPI specification before doing the call, and raise an exception if a parameter is incorrect.
79+
80+
This can be enabled globally by setting `strict_filters=True` in the API object initialization:
81+
82+
```python
83+
nb = pynetbox.api(
84+
'http://localhost:8000',
85+
strict_filters=True,
86+
)
87+
```
88+
89+
This can also be enabled and disabled on a per-request basis:
90+
91+
```python
92+
# Disable for one request when enabled globally.
93+
# Will not raise an exception and return the entire Device table.
94+
nb.dcim.devices.filter(non_existing_filter="aaaa", strict_filters=False)
95+
96+
# Enable for one request when not enabled globally.
97+
# Will raise an exception.
98+
nb.dcim.devices.filter(non_existing_filter="aaaa", strict_filters=True)
99+
```
76100

77101
## Running Tests
78102

pynetbox/__init__.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,9 @@
11
from pynetbox.core.api import Api as api
2-
from pynetbox.core.query import AllocationError, ContentError, RequestError
2+
from pynetbox.core.query import (
3+
AllocationError,
4+
ContentError,
5+
RequestError,
6+
ParameterValidationError,
7+
)
38

49
__version__ = "7.5.0"

pynetbox/core/api.py

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -79,19 +79,22 @@ def __init__(
7979
url,
8080
token=None,
8181
threading=False,
82+
strict_filters=False,
8283
):
8384
"""Initialize the API client.
8485
8586
Args:
8687
url (str): The base URL to the instance of NetBox you wish to connect to.
8788
token (str, optional): Your NetBox API token. If not provided, authentication will be required for each request.
8889
threading (bool, optional): Set to True to use threading in `.all()` and `.filter()` requests, defaults to False.
90+
strict_filters (bool, optional): Set to True to check GET call filters against OpenAPI specifications (intentionally not done in NetBox API), defaults to False.
8991
"""
9092
base_url = "{}/api".format(url if url[-1] != "/" else url[:-1])
9193
self.token = token
9294
self.base_url = base_url
9395
self.http_session = requests.Session()
9496
self.threading = threading
97+
self.strict_filters = strict_filters
9598

9699
# Initialize NetBox apps
97100
self.circuits = App(self, "circuits")
@@ -139,6 +142,7 @@ def openapi(self):
139142
"""Returns the OpenAPI spec.
140143
141144
Quick helper function to pull down the entire OpenAPI spec.
145+
It is stored in memory to avoid repeated calls on NetBox API.
142146
143147
## Returns
144148
dict: The OpenAPI specification as a dictionary.
@@ -155,10 +159,13 @@ def openapi(self):
155159
# {...}
156160
```
157161
"""
158-
return Request(
159-
base=self.base_url,
160-
http_session=self.http_session,
161-
).get_openapi()
162+
if not (openapi := getattr(self, "_openapi", None)):
163+
openapi = self._openapi = Request(
164+
base=self.base_url,
165+
http_session=self.http_session,
166+
).get_openapi()
167+
168+
return openapi
162169

163170
def status(self):
164171
"""Gets the status information from NetBox.

pynetbox/core/endpoint.py

Lines changed: 69 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
limitations under the License.
1515
"""
1616

17-
from pynetbox.core.query import Request, RequestError
17+
from pynetbox.core.query import Request, RequestError, ParameterValidationError
1818
from pynetbox.core.response import Record, RecordSet
1919

2020
RESERVED_KWARGS = ()
@@ -46,6 +46,7 @@ def __init__(self, api, app, name, model=None):
4646
self.return_obj = self._lookup_ret_obj(name, model)
4747
self.name = name.replace("_", "-")
4848
self.api = api
49+
self.app = app
4950
self.base_url = api.base_url
5051
self.token = api.token
5152
self.url = "{base_url}/{app}/{endpoint}".format(
@@ -76,6 +77,58 @@ def _lookup_ret_obj(self, name, model):
7677
ret = Record
7778
return ret
7879

80+
def _validate_openapi_parameters(self, method: str, parameters: dict) -> None:
81+
"""Validate GET request parameters against OpenAPI specification
82+
83+
This method raises a **ParameterValidationError** if parameters passed to NetBox API
84+
do not match the OpenAPI specification or validation fails.
85+
86+
## Parameters
87+
88+
* **method** : Only "get" is supported as for other methods NetBox already does proper validation
89+
* **parameters** : kwargs passed to filter() method
90+
91+
## Returns
92+
None
93+
"""
94+
if method.lower() != "get":
95+
raise RuntimeError(f"Unsupported method '{method}'.")
96+
97+
openapi_definition_path = "/api/{app}/{endpoint}/".format(
98+
app=self.app.name,
99+
endpoint=self.name,
100+
)
101+
102+
# Parse NetBox OpenAPI definition
103+
try:
104+
openapi_definition = self.api.openapi()["paths"].get(
105+
openapi_definition_path
106+
)
107+
108+
if not openapi_definition:
109+
raise ParameterValidationError(
110+
f"Path '{openapi_definition_path}' does not exist in NetBox OpenAPI specification."
111+
)
112+
113+
openapi_parameters = openapi_definition[method]["parameters"]
114+
allowed_parameters = [p["name"] for p in openapi_parameters]
115+
116+
except KeyError as exc:
117+
raise ParameterValidationError(
118+
f"Error while parsing Netbox OpenAPI specification: {exc}"
119+
)
120+
121+
# Validate all parameters
122+
validation_errors = []
123+
for p in parameters:
124+
if p not in allowed_parameters:
125+
validation_errors.append(
126+
f"'{p}' is not allowed as parameter on path '{openapi_definition_path}'."
127+
)
128+
129+
if len(validation_errors) > 0:
130+
raise ParameterValidationError(validation_errors)
131+
79132
def all(self, limit=0, offset=None):
80133
"""Queries the 'ListView' of a given endpoint.
81134
@@ -134,6 +187,8 @@ def get(self, *args, **kwargs):
134187
* **key** (int, optional): id for the item to be retrieved.
135188
* **kwargs**: Accepts the same keyword args as filter(). Any search argument the endpoint accepts can
136189
be added as a keyword arg.
190+
* **strict_filters** (bool, optional): Overrides the global filter
191+
validation per-request basis. Handled by the filter() method.
137192
138193
## Returns
139194
A single Record object or None
@@ -164,7 +219,6 @@ def get(self, *args, **kwargs):
164219
# Row 1
165220
```
166221
"""
167-
168222
try:
169223
key = args[0]
170224
except IndexError:
@@ -217,6 +271,8 @@ def filter(self, *args, **kwargs):
217271
be returned with each query to the Netbox server. The queries
218272
will be made as you iterate through the result set.
219273
* **offset** (int, optional): Overrides the offset on paginated returns.
274+
* **strict_filters** (bool, optional): Overrides the global filter
275+
validation per-request basis.
220276
221277
## Returns
222278
A RecordSet object.
@@ -272,9 +328,20 @@ def filter(self, *args, **kwargs):
272328
)
273329
limit = kwargs.pop("limit") if "limit" in kwargs else 0
274330
offset = kwargs.pop("offset") if "offset" in kwargs else None
331+
strict_filters = (
332+
# kwargs value takes precedence on globally set value
333+
kwargs.pop("strict_filters")
334+
if "strict_filters" in kwargs
335+
else self.api.strict_filters
336+
)
337+
275338
if limit == 0 and offset is not None:
276339
raise ValueError("offset requires a positive limit value")
277340
filters = {x: y if y is not None else "null" for x, y in kwargs.items()}
341+
342+
if strict_filters:
343+
self._validate_openapi_parameters("get", filters)
344+
278345
req = Request(
279346
filters=filters,
280347
base=self.url,

pynetbox/core/query.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,29 @@ def __str__(self):
109109
return self.error
110110

111111

112+
class ParameterValidationError(Exception):
113+
"""API parameter validation Exception.
114+
115+
Raised when filter parameters do not match Netbox OpenAPI specification.
116+
117+
## Examples
118+
119+
```python
120+
try:
121+
nb.dcim.devices.filter(field_which_does_not_exist="destined-for-failure")
122+
except pynetbox.ParameterValidationError as e:
123+
print(e.error)
124+
```
125+
"""
126+
127+
def __init__(self, errors):
128+
super().__init__(errors)
129+
self.error = f"The request parameter validation returned an error: {errors}"
130+
131+
def __str__(self):
132+
return self.error
133+
134+
112135
class Request:
113136
"""Creates requests to the Netbox API.
114137

tests/unit/test_endpoint.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ def test_filter(self):
99
with patch(
1010
"pynetbox.core.query.Request._make_call", return_value=Mock()
1111
) as mock:
12-
api = Mock(base_url="http://localhost:8000/api")
12+
api = Mock(base_url="http://localhost:8000/api", strict_filters=False)
1313
app = Mock(name="test")
1414
mock.return_value = [{"id": 123}, {"id": 321}]
1515
test_obj = Endpoint(api, app, "test")
@@ -24,7 +24,7 @@ def test_filter_invalid_pagination_args(self):
2424
test_obj.filter(offset=1)
2525

2626
def test_filter_replace_none_with_null(self):
27-
api = Mock(base_url="http://localhost:8000/api")
27+
api = Mock(base_url="http://localhost:8000/api", strict_filters=False)
2828
app = Mock(name="test")
2929
test_obj = Endpoint(api, app, "test")
3030
test = test_obj.filter(name=None, id=0)
@@ -118,7 +118,7 @@ def test_get_with_filter(self):
118118
"pynetbox.core.query.Request._make_call", return_value=Mock()
119119
) as mock:
120120
mock.return_value = [{"id": 123}]
121-
api = Mock(base_url="http://localhost:8000/api")
121+
api = Mock(base_url="http://localhost:8000/api", strict_filters=False)
122122
app = Mock(name="test")
123123
test_obj = Endpoint(api, app, "test")
124124
test = test_obj.get(name="test")
@@ -181,7 +181,7 @@ def test_get_greater_than_one(self):
181181
"pynetbox.core.query.Request._make_call", return_value=Mock()
182182
) as mock:
183183
mock.return_value = [{"id": 123}, {"id": 321}]
184-
api = Mock(base_url="http://localhost:8000/api")
184+
api = Mock(base_url="http://localhost:8000/api", strict_filters=False)
185185
app = Mock(name="test")
186186
test_obj = Endpoint(api, app, "test")
187187
with self.assertRaises(ValueError) as _:
@@ -192,7 +192,7 @@ def test_get_no_results(self):
192192
"pynetbox.core.query.Request._make_call", return_value=Mock()
193193
) as mock:
194194
mock.return_value = []
195-
api = Mock(base_url="http://localhost:8000/api")
195+
api = Mock(base_url="http://localhost:8000/api", strict_filters=False)
196196
app = Mock(name="test")
197197
test_obj = Endpoint(api, app, "test")
198198
test = test_obj.get(name="test")
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
import unittest
2+
from unittest.mock import Mock
3+
from tests.util import openapi_mock
4+
5+
from pynetbox.core.endpoint import Endpoint
6+
from pynetbox import ParameterValidationError
7+
8+
9+
class StrictFilterTestCase(unittest.TestCase):
10+
11+
def test_filter_strict_valid_kwargs(self):
12+
api = Mock(
13+
base_url="http://localhost:8000/api",
14+
strict_filters=True,
15+
openapi=openapi_mock,
16+
)
17+
app = Mock(name="test")
18+
app.name = "test"
19+
test_obj = Endpoint(api, app, "test")
20+
test_obj.filter(test="test")
21+
22+
def test_filter_strict_invalid_kwarg(self):
23+
api = Mock(
24+
base_url="http://localhost:8000/api",
25+
strict_filters=True,
26+
openapi=openapi_mock,
27+
)
28+
app = Mock(name="test")
29+
app.name = "test"
30+
test_obj = Endpoint(api, app, "test")
31+
with self.assertRaises(ParameterValidationError):
32+
test_obj.filter(very_invalid_kwarg="test")
33+
34+
def test_filter_strict_per_request_disable_invalid_kwarg(self):
35+
api = Mock(
36+
base_url="http://localhost:8000/api",
37+
strict_filters=True, # Enable globally
38+
openapi=openapi_mock,
39+
)
40+
app = Mock(name="test")
41+
app.name = "test"
42+
test_obj = Endpoint(api, app, "test")
43+
# Disable strict_filters only for this specific request
44+
test_obj.filter(very_invalid_kwarg="test", strict_filters=False)
45+
46+
def test_filter_strict_per_request_enable_invalid_kwarg(self):
47+
api = Mock(
48+
base_url="http://localhost:8000/api",
49+
strict_filters=False, # Disable globally
50+
openapi=openapi_mock,
51+
)
52+
app = Mock(name="test")
53+
app.name = "test"
54+
test_obj = Endpoint(api, app, "test")
55+
with self.assertRaises(ParameterValidationError):
56+
# Enable strict_filters only for this specific request
57+
test_obj.filter(very_invalid_kwarg="test", strict_filters=True)

tests/util.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,3 +15,19 @@ def load_fixture(self, path):
1515

1616
def json(self):
1717
return json.loads(self.content)
18+
19+
20+
def openapi_mock():
21+
"""Mock function to simulate Api.openapi()"""
22+
return {
23+
"paths": {
24+
"/api/test/test/": {
25+
"get": {
26+
"parameters": [
27+
{"name": "test"},
28+
{"name": "name"},
29+
]
30+
},
31+
},
32+
},
33+
}

0 commit comments

Comments
 (0)