Skip to content

Commit

Permalink
Merge pull request #195 from patrikspiess/Make-fortinet.api-method-mo…
Browse files Browse the repository at this point in the history 
…re-generic-#194

Make fortinet.api method more generic #194
  • Loading branch information
@lucmurer
lucmurer committed Feb 26, 2024
2 parents 5c01700 + 08d2aa5 commit 8702b55
Show file tree
Hide file tree
Showing 6 changed files with 112 additions and 87 deletions.
2 changes: 2 additions & 0 deletions WHATSNEW.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
### Changed

- `fmg get devices` also shows ha nodes if device is a cluster
- Make `Fortinet.api()` more generic to support more methods
- Improve error handling and tests for `Fortinet.api()`
- Updated GitHub actions to latest major version due to Node.js 16 deprecation warning

### Fixed
Expand Down
77 changes: 39 additions & 38 deletions fotoobo/fortinet/fortinet.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
This is the Fortinet abstract base class (ABC) which is used to define some global and generic
variables and methods.
"""

import logging
from abc import ABC, abstractmethod
from time import time
Expand All @@ -22,6 +23,8 @@ class Fortinet(ABC):
defined here with the abstractmethod decorator.
"""

ALLOWED_HTTP_METHODS = ["GET", "POST"]

def __init__(self, hostname: str, **kwargs: Any) -> None:
"""
Set some initial parameters for the Fortinet super class.
Expand Down Expand Up @@ -77,43 +80,50 @@ def api( # pylint: disable=too-many-arguments
API request to a Fortinet device.
Args:
method: Request method from [get, post]
url: Rest API URL to request data from
headers: Dictionary with headers (if needed)
params: Dictionary with parameters (if needed)
payload: JSON body for post requests (if needed)
timeout: The requests read timeout
method: HTTP request method
url: Rest API URL to request data from
headers: Dictionary with headers (if needed)
params: Dictionary with parameters (if needed)
payload: JSON body for post requests (if needed)
timeout: The requests read timeout
Returns:
Response from the request
"""
full_url = f"{self.api_url}/{url.strip('/')}".strip("/")
params = params or {}
payload = payload or {}
timeout = timeout or self.timeout
start = time()

if method.upper() not in Fortinet.ALLOWED_HTTP_METHODS:
error = f"HTTP method '{method.upper()}' is not implemented"
log.error(error)
raise NotImplementedError(error)

try:
if method.lower() == "get":
response = self.session.get(
full_url,
params=params,
verify=self.ssl_verify,
timeout=timeout,
)

elif method.lower() == "post":
response = self.session.post(
full_url,
headers=headers,
json=payload,
verify=self.ssl_verify,
timeout=timeout,
)

else:
log.debug("Unknown API method")
raise GeneralError("Unknown API method")
response: requests.Response = getattr(self.session, method.lower())(
full_url,
headers=headers,
json=payload,
params=params,
timeout=timeout,
verify=self.ssl_verify,
)

except requests.exceptions.SSLError as err:
log.debug(err)
error = "Unknown SSL error"

try:
if (
err.args[0].reason.args[0].verify_message
== "unable to get local issuer certificate"
):
error = "Unable to get local issuer certificate"

except (AttributeError, IndexError):
pass

raise GeneralError(f"{error} ({self.hostname})") from err

except requests.exceptions.ConnectTimeout as err:
log.debug(err)
Expand All @@ -133,20 +143,11 @@ def api( # pylint: disable=too-many-arguments
except (IndexError, AttributeError, TypeError):
pass

try:
if (
err.args[0].reason.args[0].verify_message
== "unable to get local issuer certificate"
):
error = "Unable to get local issuer certificate"
except (IndexError, AttributeError, TypeError):
pass

raise GeneralError(f"{error} ({self.hostname})") from err

except requests.exceptions.ReadTimeout as err:
log.error(err)
raise GeneralError(f"read timeout ({self.hostname})") from err
raise GeneralError(f"Read timeout ({self.hostname})") from err

log.debug(
'Request time: [bold green]%2dms[/] "%s %s"',
Expand Down
3 changes: 2 additions & 1 deletion tests/fortinet/test_fortianalyzer.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ def test_get_version(response: MagicMock, expected: str, monkeypatch: MonkeyPatc
"https://host:443/jsonrpc",
headers=None,
json={"method": "get", "params": [{"url": "/sys/status"}], "session": ""},
verify=True,
params=None,
timeout=3,
verify=True,
)
12 changes: 8 additions & 4 deletions tests/fortinet/test_forticlientems.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,9 +143,11 @@ def test_get_version_ok(monkeypatch: MonkeyPatch) -> None:
response = ems.get_version()
requests.Session.get.assert_called_with(
"https://host:443/api/v1/system/consts/get?system_update_time=1",
params={},
verify=True,
headers=None,
json=None,
params=None,
timeout=3,
verify=True,
)
assert response == "1.2.3"

Expand All @@ -165,9 +167,11 @@ def test_get_version_invalid(monkeypatch: MonkeyPatch) -> None:
assert "Did not find any FortiClient EMS version number in response" in str(err.value)
requests.Session.get.assert_called_with(
"https://host:443/api/v1/system/consts/get?system_update_time=1",
params={},
verify=False,
headers=None,
json=None,
params=None,
timeout=3,
verify=False,
)

@staticmethod
Expand Down
49 changes: 33 additions & 16 deletions tests/fortinet/test_fortimanager.py
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"""
Test the FortiManager class
"""

# pylint: disable=no-member
from unittest.mock import MagicMock

Expand Down Expand Up @@ -54,8 +55,9 @@ def test_assign_all_objects(monkeypatch: MonkeyPatch) -> None:
],
"session": "",
},
verify=True,
params=None,
timeout=3,
verify=True,
)

@staticmethod
Expand Down Expand Up @@ -85,8 +87,9 @@ def test_assign_all_objects_http_404(monkeypatch: MonkeyPatch) -> None:
],
"session": "",
},
verify=True,
params=None,
timeout=3,
verify=True,
)

@staticmethod
Expand Down Expand Up @@ -130,8 +133,9 @@ def test_assign_all_objects_status_not_ok(monkeypatch: MonkeyPatch) -> None:
],
"session": "",
},
verify=True,
params=None,
timeout=3,
verify=True,
)

@staticmethod
Expand All @@ -150,8 +154,9 @@ def test_get_adoms(monkeypatch: MonkeyPatch) -> None:
"https://host:443/jsonrpc",
headers=None,
json={"method": "get", "params": [{"url": "/dvmdb/adom"}], "session": ""},
verify=True,
params=None,
timeout=3,
verify=True,
)

@staticmethod
Expand Down Expand Up @@ -188,8 +193,9 @@ def test_get_version(response: MagicMock, expected: str, monkeypatch: MonkeyPatc
"https://host:443/jsonrpc",
headers=None,
json={"method": "get", "params": [{"url": "/sys/status"}], "session": ""},
verify=True,
params=None,
timeout=3,
verify=True,
)

@staticmethod
Expand Down Expand Up @@ -219,8 +225,9 @@ def test_login(monkeypatch: MonkeyPatch) -> None:
"method": "exec",
"params": [{"data": {"passwd": "pass", "user": "user"}, "url": "/sys/login/user"}],
},
verify=True,
params=None,
timeout=3,
verify=True,
)

@staticmethod
Expand Down Expand Up @@ -256,8 +263,9 @@ def test_login_with_session_path(monkeypatch: MonkeyPatch) -> None:
"params": [{"url": "/sys/status"}],
"session": "dummy_session_key",
},
verify=True,
params=None,
timeout=3,
verify=True,
)

@staticmethod
Expand Down Expand Up @@ -294,8 +302,9 @@ def test_login_with_session_path_invalid_key(monkeypatch: MonkeyPatch) -> None:
"method": "exec",
"params": [{"data": {"passwd": "pass", "user": "user"}, "url": "/sys/login/user"}],
},
verify=True,
params=None,
timeout=3,
verify=True,
)

@staticmethod
Expand Down Expand Up @@ -332,8 +341,9 @@ def test_login_with_session_path_not_found(temp_dir: str, monkeypatch: MonkeyPat
"method": "exec",
"params": [{"data": {"passwd": "pass", "user": "user"}, "url": "/sys/login/user"}],
},
verify=True,
params=None,
timeout=3,
verify=True,
)

@staticmethod
Expand Down Expand Up @@ -362,8 +372,9 @@ def test_logout(monkeypatch: MonkeyPatch) -> None:
"params": [{"url": "/sys/logout"}],
"session": "dummy_session_key",
},
verify=True,
params=None,
timeout=3,
verify=True,
)

@staticmethod
Expand All @@ -380,8 +391,9 @@ def test_post_single(monkeypatch: MonkeyPatch) -> None:
"https://host:443/jsonrpc",
headers=None,
json={"params": [{"url": "adom/ADOM"}], "session": ""},
verify=True,
params=None,
timeout=10,
verify=True,
)

@staticmethod
Expand All @@ -398,8 +410,9 @@ def test_post_multiple(monkeypatch: MonkeyPatch) -> None:
"https://host:443/jsonrpc",
headers=None,
json={"params": [{"url": "adom/ADOM"}], "session": ""},
verify=True,
params=None,
timeout=10,
verify=True,
)

@staticmethod
Expand All @@ -416,8 +429,9 @@ def test_post_single_global(monkeypatch: MonkeyPatch) -> None:
"https://host:443/jsonrpc",
headers=None,
json={"params": [{"url": "global"}], "session": ""},
verify=True,
params=None,
timeout=10,
verify=True,
)

@staticmethod
Expand All @@ -441,8 +455,9 @@ def test_post_response_error(monkeypatch: MonkeyPatch) -> None:
"https://host:443/jsonrpc",
headers=None,
json={"params": [{"url": "adom/ADOM"}], "session": ""},
verify=True,
params=None,
timeout=10,
verify=True,
)

@staticmethod
Expand All @@ -459,8 +474,9 @@ def test_post_http_error(monkeypatch: MonkeyPatch) -> None:
"https://host:443/jsonrpc",
headers=None,
json={"params": [{"url": "adom/ADOM"}], "session": ""},
verify=True,
params=None,
timeout=10,
verify=True,
)

@staticmethod
Expand Down Expand Up @@ -501,6 +517,7 @@ def test_wait_for_task(monkeypatch: MonkeyPatch) -> None:
"https://host:443/jsonrpc",
headers=None,
json={"method": "get", "params": [{"url": "/task/task/222/line"}], "session": ""},
verify=True,
params=None,
timeout=3,
verify=True,
)
Loading

0 comments on commit 8702b55

Please sign in to comment.