openstack
/
ironic
Code Issues Proposed changes

Merge "Add better error messages for invalid conf molds" into stable/wallaby

This commit is contained in:
Zuul 2021-10-06 12:22:38 +00:00 committed by Gerrit Code Review
parent 15b1cdb766 dd8e4fdbe9
commit d0c70aaf28
4 changed files with 119 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
ironic/common/molds.py
View File

@ -81,7 +81,17 @@ def get_configuration(task, url):
auth_header = _get_auth_header(task)
response = _request(url, auth_header)
if response.status_code == requests.codes.ok:
return response.json()
if not response.content:
raise exception.IronicException(_(
"Configuration mold for node %(node_uuid)s at %(url)s is "
"empty") % {'node_uuid': task.node.uuid, 'url': url})
try:
return response.json()
except json.decoder.JSONDecodeError as jde:
raise exception.IronicException(_(
"Configuration mold for node %(node_uuid)s at %(url)s has "
"invalid JSON: %(error)s)")
% {'node_uuid': task.node.uuid, 'url': url, 'error': jde})
response.raise_for_status()

42
ironic/drivers/modules/drac/management.py
View File

@ -25,6 +25,8 @@ import time
from futurist import periodics
from ironic_lib import metrics_utils
import jsonschema
from jsonschema import exceptions as json_schema_exc
from oslo_log import log as logging
from oslo_utils import importutils
@ -93,6 +95,23 @@ _CLEAR_JOB_IDS = 'JID_CLEARALL'
# Clean steps constant
_CLEAR_JOBS_CLEAN_STEPS = ['clear_job_queue', 'known_good_state']
_CONF_MOLD_SCHEMA = {
'type': 'object',
'properties': {
'oem': {
'type': 'object',
'properties': {
'interface': {'const': 'idrac-redfish'},
'data': {'type': 'object', 'minProperties': 1}
},
'required': ['interface', 'data']
}
},
'required': ['oem'],
'additionalProperties': False
}
def _get_boot_device(node, drac_boot_devices=None):
client = drac_common.get_drac_client(node)
@ -186,6 +205,19 @@ def _flexibly_program_boot_order(device, drac_boot_mode):
return bios_settings
def _validate_conf_mold(data):
"""Validates iDRAC configuration mold JSON schema
:param data: dictionary of configuration mold data
:raises InvalidParameterValue: If configuration mold validation fails
"""
try:
jsonschema.validate(data, _CONF_MOLD_SCHEMA)
except json_schema_exc.ValidationError as e:
raise exception.InvalidParameterValue(
_("Invalid configuration mold: %(error)s") % {'error': e})
def set_boot_device(node, device, persistent=False):
"""Set the boot device for a node.
@ -407,15 +439,7 @@ class DracRedfishManagement(redfish_management.RedfishManagement):
{'node': task.node.uuid,
'configuration_name': import_configuration_location}))
interface = configuration["oem"]["interface"]
if interface != "idrac-redfish":
raise exception.DracOperationError(
error=(_("Invalid configuration for node %(node)s "
"in %(configuration_name)s. Supports only "
"idrac-redfish, but found %(interface)s") %
{'node': task.node.uuid,
'configuration_name': import_configuration_location,
'interface': interface}))
_validate_conf_mold(configuration)
task_monitor = drac_utils.execute_oem_manager_method(
task, 'import system configuration',

32
ironic/tests/unit/common/test_molds.py
View File

@ -12,6 +12,7 @@
# License for the specific language governing permissions and limitations
# under the License.
import json
from unittest import mock
from oslo_config import cfg
@ -166,6 +167,7 @@ class ConfigurationMoldTestCase(db_base.DbTestCase):
cfg.CONF.molds.storage = 'swift'
response = mock.MagicMock()
response.status_code = 200
response.content = "{'key': 'value'}"
response.json.return_value = {'key': 'value'}
mock_get.return_value = response
url = 'https://example.com/file1'
@ -199,6 +201,7 @@ class ConfigurationMoldTestCase(db_base.DbTestCase):
cfg.CONF.molds.password = 'password'
response = mock.MagicMock()
response.status_code = 200
response.content = "{'key': 'value'}"
response.json.return_value = {'key': 'value'}
mock_get.return_value = response
url = 'https://example.com/file2'
@ -217,6 +220,7 @@ class ConfigurationMoldTestCase(db_base.DbTestCase):
cfg.CONF.molds.password = None
response = mock.MagicMock()
response.status_code = 200
response.content = "{'key': 'value'}"
response.json.return_value = {'key': 'value'}
mock_get.return_value = response
url = 'https://example.com/file2'
@ -289,3 +293,31 @@ class ConfigurationMoldTestCase(db_base.DbTestCase):
'https://example.com/file2',
headers={'Authorization': 'Basic dXNlcjpwYXNzd29yZA=='})
self.assertEqual(mock_get.call_count, 2)
@mock.patch.object(requests, 'get', autospec=True)
def test_get_configuration_empty(self, mock_get):
cfg.CONF.molds.storage = 'http'
response = mock.MagicMock()
response.status_code = 200
response.content = ''
mock_get.return_value = response
url = 'https://example.com/file2'
with task_manager.acquire(self.context, self.node.uuid) as task:
self.assertRaises(exception.IronicException,
molds.get_configuration, task, url)
@mock.patch.object(requests, 'get', autospec=True)
def test_get_configuration_invalid_json(self, mock_get):
cfg.CONF.molds.storage = 'http'
response = mock.MagicMock()
response.status_code = 200
response.content = 'not json'
response.json.side_effect = json.decoder.JSONDecodeError(
'Expecting value', 'not json', 0)
mock_get.return_value = response
url = 'https://example.com/file2'
with task_manager.acquire(self.context, self.node.uuid) as task:
self.assertRaises(exception.IronicException,
molds.get_configuration, task, url)

46
ironic/tests/unit/drivers/modules/drac/test_management.py
View File

@ -909,8 +909,8 @@ class DracRedfishManagementTestCase(test_utils.BaseDracTest):
@mock.patch.object(molds, 'get_configuration', autospec=True)
@mock.patch.object(redfish_utils, 'get_system', autospec=True)
def test_import_configuration_incorrect_interface(self, mock_get_system,
mock_get_configuration):
def test_import_configuration_incorrect_schema(self, mock_get_system,
mock_get_configuration):
task = mock.Mock(node=self.node, context=self.context)
fake_manager_oem1 = mock.Mock()
fake_manager1 = mock.Mock()
@ -920,7 +920,7 @@ class DracRedfishManagementTestCase(test_utils.BaseDracTest):
'{"oem": {"interface": "idrac-wsman", '
'"data": {"prop1": "value1", "prop2": 2}}}')
self.assertRaises(exception.DracOperationError,
self.assertRaises(exception.InvalidParameterValue,
self.management.import_configuration, task, 'edge')
@mock.patch.object(deploy_utils, 'get_async_step_return_state',
@ -1462,3 +1462,43 @@ class DracRedfishManagementTestCase(test_utils.BaseDracTest):
mock_manager_oem.job_service.delete_jobs.assert_called_once_with(
job_ids=['JID_CLEARALL'])
mock_manager_oem.reset_idrac.assert_called_once_with()
def test__validate_conf_mold(self):
drac_mgmt._validate_conf_mold({'oem': {'interface': 'idrac-redfish',
'data': {'SystemConfiguration': {}}}})
def test__validate_conf_mold_oem_missing(self):
self.assertRaisesRegex(
exception.InvalidParameterValue,
"'oem' is a required property",
drac_mgmt._validate_conf_mold,
{'bios': {'reset': False}})
def test__validate_conf_mold_interface_missing(self):
self.assertRaisesRegex(
exception.InvalidParameterValue,
"'interface' is a required property",
drac_mgmt._validate_conf_mold,
{'oem': {'data': {'SystemConfiguration': {}}}})
def test__validate_conf_mold_interface_not_supported(self):
self.assertRaisesRegex(
exception.InvalidParameterValue,
"'idrac-redfish' was expected",
drac_mgmt._validate_conf_mold,
{'oem': {'interface': 'idrac-wsman',
'data': {'SystemConfiguration': {}}}})
def test__validate_conf_mold_data_missing(self):
self.assertRaisesRegex(
exception.InvalidParameterValue,
"'data' is a required property",
drac_mgmt._validate_conf_mold,
{'oem': {'interface': 'idrac-redfish'}})
def test__validate_conf_mold_data_empty(self):
self.assertRaisesRegex(
exception.InvalidParameterValue,
"does not have enough properties",
drac_mgmt._validate_conf_mold,
{'oem': {'interface': 'idrac-redfish', 'data': {}}})