forked from saltstack/salt
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Fix authentication error handling and incomplete error messages in pr…
…oxmox salt-cloud driver Fixes saltstack#62211
- Loading branch information
Showing
3 changed files
with
118 additions
and
4 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Improved error handling and diagnostics in the proxmox salt-cloud driver |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,13 +2,35 @@ | |
:codeauthor: Tyler Johnson <[email protected]> | ||
""" | ||
|
||
import io | ||
import textwrap | ||
|
||
import requests | ||
from salt import config | ||
from salt.cloud.clouds import proxmox | ||
from tests.support.helpers import TstSuiteLoggingHandler | ||
from tests.support.mixins import LoaderModuleMockMixin | ||
from tests.support.mock import ANY, MagicMock, patch | ||
from tests.support.unit import TestCase | ||
|
||
PROFILE = { | ||
"my_proxmox": { | ||
"provider": "my_proxmox", | ||
"image": "local:some_image.tgz", | ||
} | ||
} | ||
PROVIDER_CONFIG = { | ||
"my_proxmox": { | ||
"proxmox": { | ||
"driver": "proxmox", | ||
"url": "[email protected]", | ||
"user": "cloud@pve", | ||
"password": "verybadpass", | ||
"profiles": PROFILE, | ||
} | ||
} | ||
} | ||
|
||
|
||
class ProxmoxTest(TestCase, LoaderModuleMockMixin): | ||
def setup_loader_modules(self): | ||
|
@@ -22,8 +44,8 @@ def setup_loader_modules(self): | |
"__opts__": { | ||
"sock_dir": True, | ||
"transport": True, | ||
"providers": {"my_proxmox": {}}, | ||
"profiles": {"my_proxmox": {}}, | ||
"providers": PROVIDER_CONFIG, | ||
"profiles": PROFILE, | ||
}, | ||
"__active_provider_name__": "my_proxmox:proxmox", | ||
} | ||
|
@@ -262,3 +284,86 @@ def _test__import_api(self, response): | |
proxmox._import_api() | ||
self.assertEqual(proxmox.api, [{"info": {}}]) | ||
return | ||
|
||
def test__authenticate_success(self): | ||
response = requests.Response() | ||
response.status_code = 200 | ||
response.reason = "OK" | ||
response.raw = io.BytesIO( | ||
b"""{"data":{"CSRFPreventionToken":"01234567:dG9rZW4=","ticket":"PVE:cloud@pve:01234567::dGlja2V0"}}""" | ||
) | ||
with patch("requests.post", return_value=response): | ||
proxmox._authenticate() | ||
assert proxmox.csrf and proxmox.ticket | ||
return | ||
|
||
def test__authenticate_failure(self): | ||
""" | ||
Confirm that authentication failure raises an exception. | ||
""" | ||
response = requests.Response() | ||
response.status_code = 401 | ||
response.reason = "authentication failure" | ||
response.raw = io.BytesIO(b"""{"data":null}""") | ||
with patch("requests.post", return_value=response): | ||
self.assertRaises(requests.exceptions.HTTPError, proxmox._authenticate) | ||
return | ||
|
||
def test_creation_failure_logging(self): | ||
""" | ||
Test detailed logging on HTTP errors during VM creation. | ||
""" | ||
vm_ = { | ||
"profile": "my_proxmox", | ||
"name": "vm4", | ||
"technology": "lxc", | ||
"host": "127.0.0.1", | ||
"image": "local:some_image.tgz", | ||
"onboot": True, | ||
} | ||
self.assertEqual( | ||
config.is_profile_configured( | ||
proxmox.__opts__, "my_proxmox:proxmox", "my_proxmox", vm_=vm_ | ||
), | ||
True, | ||
) | ||
|
||
response = requests.Response() | ||
response.status_code = 400 | ||
response.reason = "Parameter verification failed." | ||
response.raw = io.BytesIO( | ||
b"""{"data":null,"errors":{"onboot":"type check ('boolean') failed - got 'True'"}}""" | ||
) | ||
|
||
def mock_query_response(conn_type, option, post_data=None): | ||
if conn_type == "get" and option == "cluster/nextid": | ||
return 104 | ||
if conn_type == "post" and option == "nodes/127.0.0.1/lxc": | ||
response.raise_for_status() | ||
return response | ||
return None | ||
|
||
with patch.object( | ||
proxmox, "query", side_effect=mock_query_response | ||
), patch.object( | ||
proxmox, "_get_properties", return_value=set() | ||
), TstSuiteLoggingHandler() as log_handler: | ||
self.assertEqual(proxmox.create(vm_), False) | ||
|
||
# Search for these messages in a multi-line log entry. | ||
missing = { | ||
"{} Client Error: {} for url:".format( | ||
response.status_code, response.reason | ||
), | ||
response.text, | ||
} | ||
for required in list(missing): | ||
for message in log_handler.messages: | ||
if required in message: | ||
missing.remove(required) | ||
break | ||
if missing: | ||
raise AssertionError( | ||
"Did not find error messages: {}".format(sorted(list(missing))) | ||
) | ||
return |