From 284e22bc086ddaffbd4473b23b04a6165dca9c44 Mon Sep 17 00:00:00 2001 From: Steve Baker Date: Wed, 12 Aug 2020 16:01:25 +1200 Subject: [PATCH] Convert deploy_templates endpoint to plain JSON Change-Id: I1bb976ad4ad43c9801bc08ef7ed5835b5b7225e6 Story: 1651346 Task: 10551 --- ironic/api/controllers/v1/deploy_template.py | 392 +++++++----------- .../controllers/v1/test_deploy_template.py | 99 ++--- ironic/tests/unit/api/utils.py | 11 +- 3 files changed, 201 insertions(+), 301 deletions(-) diff --git a/ironic/api/controllers/v1/deploy_template.py b/ironic/api/controllers/v1/deploy_template.py index 90555bad29..6a56eec4a9 100644 --- a/ironic/api/controllers/v1/deploy_template.py +++ b/ironic/api/controllers/v1/deploy_template.py @@ -11,7 +11,6 @@ # under the License. import collections -import datetime from http import client as http_client from ironic_lib import metrics_utils @@ -23,14 +22,12 @@ from pecan import rest from webob import exc as webob_exc from ironic import api -from ironic.api.controllers import base from ironic.api.controllers import link from ironic.api.controllers.v1 import collection from ironic.api.controllers.v1 import notification_utils as notify -from ironic.api.controllers.v1 import types from ironic.api.controllers.v1 import utils as api_utils -from ironic.api import expose -from ironic.api import types as atypes +from ironic.api import method +from ironic.common import args from ironic.common import exception from ironic.common.i18n import _ from ironic.conductor import steps as conductor_steps @@ -41,215 +38,131 @@ CONF = ironic.conf.CONF LOG = log.getLogger(__name__) METRICS = metrics_utils.get_metrics_logger(__name__) -_DEFAULT_RETURN_FIELDS = ('uuid', 'name') +DEFAULT_RETURN_FIELDS = ['uuid', 'name'] -_DEPLOY_INTERFACE_TYPE = atypes.Enum( - str, *conductor_steps.DEPLOYING_INTERFACE_PRIORITY) +INTERFACE_NAMES = list( + conductor_steps.DEPLOYING_INTERFACE_PRIORITY.keys()) + +STEP_SCHEMA = { + 'type': 'object', + 'properties': { + 'args': {'type': 'object'}, + 'interface': {'type': 'string', 'enum': INTERFACE_NAMES}, + 'priority': {'anyOf': [ + {'type': 'integer', 'minimum': 0}, + {'type': 'string', 'minLength': 1, 'pattern': '^[0-9]+$'} + ]}, + 'step': {'type': 'string', 'minLength': 1}, + }, + 'required': ['interface', 'step', 'args', 'priority'], + 'additionalProperties': False, +} + +TEMPLATE_SCHEMA = { + 'type': 'object', + 'properties': { + 'description': {'type': ['string', 'null'], 'maxLength': 255}, + 'extra': {'type': ['object', 'null']}, + 'name': api_utils.TRAITS_SCHEMA, + 'steps': {'type': 'array', 'items': STEP_SCHEMA, 'minItems': 1}, + 'uuid': {'type': ['string', 'null']}, + }, + 'required': ['steps', 'name'], + 'additionalProperties': False, +} + +PATCH_ALLOWED_FIELDS = ['extra', 'name', 'steps', 'description'] +STEP_PATCH_ALLOWED_FIELDS = ['args', 'interface', 'priority', 'step'] -class DeployStepType(atypes.Base, base.AsDictMixin): - """A type describing a deployment step.""" +def duplicate_steps(name, value): + """Argument validator to check template for duplicate steps""" + # TODO(mgoddard): Determine the consequences of allowing duplicate + # steps. + # * What if one step has zero priority and another non-zero? + # * What if a step that is enabled by default is included in a + # template? Do we override the default or add a second invocation? - interface = atypes.wsattr(_DEPLOY_INTERFACE_TYPE, mandatory=True) - - step = atypes.wsattr(str, mandatory=True) - - args = atypes.wsattr({str: types.jsontype}, mandatory=True) - - priority = atypes.wsattr(atypes.IntegerType(0), mandatory=True) - - def __init__(self, **kwargs): - self.fields = ['interface', 'step', 'args', 'priority'] - for field in self.fields: - value = kwargs.get(field, atypes.Unset) - setattr(self, field, value) - - def sanitize(self): - """Removes sensitive data.""" - if self.args != atypes.Unset: - self.args = strutils.mask_dict_password(self.args, "******") + # Check for duplicate steps. Each interface/step combination can be + # specified at most once. + counter = collections.Counter((step['interface'], step['step']) + for step in value['steps']) + duplicates = {key for key, count in counter.items() if count > 1} + if duplicates: + duplicates = {"interface: %s, step: %s" % (interface, step) + for interface, step in duplicates} + err = _("Duplicate deploy steps. A deploy template cannot have " + "multiple deploy steps with the same interface and step. " + "Duplicates: %s") % "; ".join(duplicates) + raise exception.InvalidDeployTemplate(err=err) + return value -class DeployTemplate(base.APIBase): - """API representation of a deploy template.""" - - uuid = types.uuid - """Unique UUID for this deploy template.""" - - name = atypes.wsattr(str, mandatory=True) - """The logical name for this deploy template.""" - - steps = atypes.wsattr([DeployStepType], mandatory=True) - """The deploy steps of this deploy template.""" - - links = None - """A list containing a self link and associated deploy template links.""" - - extra = {str: types.jsontype} - """This deploy template's meta data""" - - def __init__(self, **kwargs): - self.fields = [] - fields = list(objects.DeployTemplate.fields) - - for field in fields: - # Skip fields we do not expose. - if not hasattr(self, field): - continue - - value = kwargs.get(field, atypes.Unset) - if field == 'steps' and value != atypes.Unset: - value = [DeployStepType(**step) for step in value] - self.fields.append(field) - setattr(self, field, value) - - @staticmethod - def validate(value): - if value is None: - return - - # The name is mandatory, but the 'mandatory' attribute support in - # wsattr allows None. - if value.name is None: - err = _("Deploy template name cannot be None") - raise exception.InvalidDeployTemplate(err=err) - - # The name must also be a valid trait. - api_utils.validate_trait( - value.name, - error_prefix=_("Deploy template name must be a valid trait")) - - # There must be at least one step. - if not value.steps: - err = _("No deploy steps specified. A deploy template must have " - "at least one deploy step.") - raise exception.InvalidDeployTemplate(err=err) - - # TODO(mgoddard): Determine the consequences of allowing duplicate - # steps. - # * What if one step has zero priority and another non-zero? - # * What if a step that is enabled by default is included in a - # template? Do we override the default or add a second invocation? - - # Check for duplicate steps. Each interface/step combination can be - # specified at most once. - counter = collections.Counter((step.interface, step.step) - for step in value.steps) - duplicates = {key for key, count in counter.items() if count > 1} - if duplicates: - duplicates = {"interface: %s, step: %s" % (interface, step) - for interface, step in duplicates} - err = _("Duplicate deploy steps. A deploy template cannot have " - "multiple deploy steps with the same interface and step. " - "Duplicates: %s") % "; ".join(duplicates) - raise exception.InvalidDeployTemplate(err=err) - return value - - @staticmethod - def _convert_with_links(template, url, fields=None): - template.links = [ - link.make_link('self', url, 'deploy_templates', - template.uuid), - link.make_link('bookmark', url, 'deploy_templates', - template.uuid, - bookmark=True) - ] - return template - - @classmethod - def convert_with_links(cls, rpc_template, fields=None, sanitize=True): - """Add links to the deploy template.""" - template = DeployTemplate(**rpc_template.as_dict()) - - if fields is not None: - api_utils.check_for_invalid_fields(fields, template.as_dict()) - - template = cls._convert_with_links(template, - api.request.public_url, - fields=fields) - if sanitize: - template.sanitize(fields) - - return template - - def sanitize(self, fields): - """Removes sensitive and unrequested data. - - Will only keep the fields specified in the ``fields`` parameter. - - :param fields: - list of fields to preserve, or ``None`` to preserve them all - :type fields: list of str - """ - if self.steps != atypes.Unset: - for step in self.steps: - step.sanitize() - - if fields is not None: - self.unset_fields_except(fields) - - @classmethod - def sample(cls, expand=True): - time = datetime.datetime(2000, 1, 1, 12, 0, 0) - template_uuid = '534e73fa-1014-4e58-969a-814cc0cb9d43' - template_name = 'CUSTOM_RAID1' - template_steps = [{ - "interface": "raid", - "step": "create_configuration", - "args": { - "logical_disks": [{ - "size_gb": "MAX", - "raid_level": "1", - "is_root_volume": True - }], - "delete_configuration": True - }, - "priority": 10 - }] - template_extra = {'foo': 'bar'} - sample = cls(uuid=template_uuid, - name=template_name, - steps=template_steps, - extra=template_extra, - created_at=time, - updated_at=time) - fields = None if expand else _DEFAULT_RETURN_FIELDS - return cls._convert_with_links(sample, 'http://localhost:6385', - fields=fields) +TEMPLATE_VALIDATOR = args.and_valid( + args.schema(TEMPLATE_SCHEMA), + duplicate_steps, + args.dict_valid(uuid=args.uuid) +) -class DeployTemplatePatchType(types.JsonPatchType): - - _api_base = DeployTemplate +def convert_steps(rpc_steps): + for step in rpc_steps: + yield { + 'interface': step['interface'], + 'step': step['step'], + 'args': step['args'], + 'priority': step['priority'], + } -class DeployTemplateCollection(collection.Collection): - """API representation of a collection of deploy templates.""" +def convert_with_links(rpc_template, fields=None, sanitize=True): + """Add links to the deploy template.""" + template = api_utils.object_to_dict( + rpc_template, + fields=('name', 'extra'), + link_resource='deploy_templates', + ) + template['steps'] = list(convert_steps(rpc_template.steps)) - _type = 'deploy_templates' + if fields is not None: + api_utils.check_for_invalid_fields(fields, template) - deploy_templates = [DeployTemplate] - """A list containing deploy template objects""" + if sanitize: + template_sanitize(template, fields) - @staticmethod - def convert_with_links(templates, limit, fields=None, **kwargs): - collection = DeployTemplateCollection() - collection.deploy_templates = [ - DeployTemplate.convert_with_links(t, fields=fields, sanitize=False) - for t in templates] - collection.next = collection.get_next(limit, fields=fields, **kwargs) + return template - for template in collection.deploy_templates: - template.sanitize(fields) - return collection +def template_sanitize(template, fields): + """Removes sensitive and unrequested data. - @classmethod - def sample(cls): - sample = cls() - template = DeployTemplate.sample(expand=False) - sample.deploy_templates = [template] - return sample + Will only keep the fields specified in the ``fields`` parameter. + + :param fields: + list of fields to preserve, or ``None`` to preserve them all + :type fields: list of str + """ + api_utils.sanitize_dict(template, fields) + if template.get('steps'): + for step in template['steps']: + step_sanitize(step) + + +def step_sanitize(step): + if step.get('args'): + step['args'] = strutils.mask_dict_password(step['args'], "******") + + +def list_convert_with_links(rpc_templates, limit, fields=None, **kwargs): + return collection.list_convert_with_links( + items=[convert_with_links(t, fields=fields, sanitize=False) + for t in rpc_templates], + item_name='deploy_templates', + limit=limit, + fields=fields, + sanitize_func=template_sanitize, + **kwargs + ) class DeployTemplatesController(rest.RestController): @@ -267,25 +180,11 @@ class DeployTemplatesController(rest.RestController): raise webob_exc.HTTPMethodNotAllowed(msg) return super(DeployTemplatesController, self)._route(args, request) - def _update_changed_fields(self, template, rpc_template): - """Update rpc_template based on changed fields in a template.""" - for field in objects.DeployTemplate.fields: - try: - patch_val = getattr(template, field) - except AttributeError: - # Ignore fields that aren't exposed in the API. - continue - if patch_val == atypes.Unset: - patch_val = None - if rpc_template[field] != patch_val: - if field == 'steps' and patch_val is not None: - # Convert from DeployStepType to dict. - patch_val = [s.as_dict() for s in patch_val] - rpc_template[field] = patch_val - @METRICS.timer('DeployTemplatesController.get_all') - @expose.expose(DeployTemplateCollection, types.name, int, str, - str, types.listtype, types.boolean) + @method.expose() + @args.validate(marker=args.name, limit=args.integer, sort_key=args.string, + sort_dir=args.string, fields=args.string_list, + detail=args.boolean) def get_all(self, marker=None, limit=None, sort_key='id', sort_dir='asc', fields=None, detail=None): """Retrieve a list of deploy templates. @@ -308,7 +207,7 @@ class DeployTemplatesController(rest.RestController): api_utils.check_allowed_fields([sort_key]) fields = api_utils.get_request_return_fields(fields, detail, - _DEFAULT_RETURN_FIELDS) + DEFAULT_RETURN_FIELDS) limit = api_utils.validate_limit(limit) sort_dir = api_utils.validate_sort_dir(sort_dir) @@ -332,11 +231,12 @@ class DeployTemplatesController(rest.RestController): if detail is not None: parameters['detail'] = detail - return DeployTemplateCollection.convert_with_links( + return list_convert_with_links( templates, limit, fields=fields, **parameters) @METRICS.timer('DeployTemplatesController.get_one') - @expose.expose(DeployTemplate, types.uuid_or_name, types.listtype) + @method.expose() + @args.validate(template_ident=args.uuid_or_name, fields=args.string_list) def get_one(self, template_ident, fields=None): """Retrieve information about the given deploy template. @@ -351,11 +251,12 @@ class DeployTemplatesController(rest.RestController): rpc_template = api_utils.get_rpc_deploy_template_with_suffix( template_ident) - return DeployTemplate.convert_with_links(rpc_template, fields=fields) + return convert_with_links(rpc_template, fields=fields) @METRICS.timer('DeployTemplatesController.post') - @expose.expose(DeployTemplate, body=DeployTemplate, - status_code=http_client.CREATED) + @method.expose(status_code=http_client.CREATED) + @method.body('template') + @args.validate(template=TEMPLATE_VALIDATOR) def post(self, template): """Create a new deploy template. @@ -364,12 +265,12 @@ class DeployTemplatesController(rest.RestController): api_utils.check_policy('baremetal:deploy_template:create') context = api.request.context - tdict = template.as_dict() - # NOTE(mgoddard): UUID is mandatory for notifications payload - if not tdict.get('uuid'): - tdict['uuid'] = uuidutils.generate_uuid() - new_template = objects.DeployTemplate(context, **tdict) + # NOTE(mgoddard): UUID is mandatory for notifications payload + if not template.get('uuid'): + template['uuid'] = uuidutils.generate_uuid() + + new_template = objects.DeployTemplate(context, **template) notify.emit_start_notification(context, new_template, 'create') with notify.handle_error_notification(context, new_template, 'create'): @@ -377,14 +278,14 @@ class DeployTemplatesController(rest.RestController): # Set the HTTP Location Header api.response.location = link.build_url('deploy_templates', new_template.uuid) - api_template = DeployTemplate.convert_with_links(new_template) + api_template = convert_with_links(new_template) notify.emit_end_notification(context, new_template, 'create') return api_template @METRICS.timer('DeployTemplatesController.patch') - @expose.validate(types.uuid, types.boolean, [DeployTemplatePatchType]) - @expose.expose(DeployTemplate, types.uuid_or_name, types.boolean, - body=[DeployTemplatePatchType]) + @method.expose() + @method.body('patch') + @args.validate(template_ident=args.uuid_or_name, patch=args.patch) def patch(self, template_ident, patch=None): """Update an existing deploy template. @@ -393,15 +294,28 @@ class DeployTemplatesController(rest.RestController): """ api_utils.check_policy('baremetal:deploy_template:update') + api_utils.patch_validate_allowed_fields(patch, PATCH_ALLOWED_FIELDS) + context = api.request.context rpc_template = api_utils.get_rpc_deploy_template_with_suffix( template_ident) - template_dict = rpc_template.as_dict() - template = DeployTemplate( - **api_utils.apply_jsonpatch(template_dict, patch)) - template.validate(template) - self._update_changed_fields(template, rpc_template) + template = rpc_template.as_dict() + + # apply the patch + template = api_utils.apply_jsonpatch(template, patch) + + # validate the result with the patch schema + for step in template.get('steps', []): + api_utils.patched_validate_with_schema( + step, STEP_SCHEMA) + api_utils.patched_validate_with_schema( + template, TEMPLATE_SCHEMA, TEMPLATE_VALIDATOR) + + api_utils.patch_update_changed_fields( + template, rpc_template, fields=objects.DeployTemplate.fields, + schema=TEMPLATE_SCHEMA + ) # NOTE(mgoddard): There could be issues with concurrent updates of a # template. This is particularly true for the complex 'steps' field, @@ -421,14 +335,14 @@ class DeployTemplatesController(rest.RestController): with notify.handle_error_notification(context, rpc_template, 'update'): rpc_template.save() - api_template = DeployTemplate.convert_with_links(rpc_template) + api_template = convert_with_links(rpc_template) notify.emit_end_notification(context, rpc_template, 'update') return api_template @METRICS.timer('DeployTemplatesController.delete') - @expose.expose(None, types.uuid_or_name, - status_code=http_client.NO_CONTENT) + @method.expose(status_code=http_client.NO_CONTENT) + @args.validate(template_ident=args.uuid_or_name) def delete(self, template_ident): """Delete a deploy template. diff --git a/ironic/tests/unit/api/controllers/v1/test_deploy_template.py b/ironic/tests/unit/api/controllers/v1/test_deploy_template.py index b194dafdd5..ed7239d5c5 100644 --- a/ironic/tests/unit/api/controllers/v1/test_deploy_template.py +++ b/ironic/tests/unit/api/controllers/v1/test_deploy_template.py @@ -24,12 +24,10 @@ from oslo_utils import uuidutils from ironic.api.controllers import base as api_base from ironic.api.controllers import v1 as api_v1 -from ironic.api.controllers.v1 import deploy_template as api_deploy_template from ironic.api.controllers.v1 import notification_utils from ironic.common import exception from ironic import objects from ironic.objects import fields as obj_fields -from ironic.tests import base from ironic.tests.unit.api import base as test_api_base from ironic.tests.unit.api import utils as test_api_utils from ironic.tests.unit.objects import utils as obj_utils @@ -45,27 +43,6 @@ def _obj_to_api_step(obj_step): } -class TestDeployTemplateObject(base.TestCase): - - def test_deploy_template_init(self): - template_dict = test_api_utils.deploy_template_post_data() - template = api_deploy_template.DeployTemplate(**template_dict) - self.assertEqual(template_dict['uuid'], template.uuid) - self.assertEqual(template_dict['name'], template.name) - self.assertEqual(template_dict['extra'], template.extra) - for t_dict_step, t_step in zip(template_dict['steps'], template.steps): - self.assertEqual(t_dict_step['interface'], t_step.interface) - self.assertEqual(t_dict_step['step'], t_step.step) - self.assertEqual(t_dict_step['args'], t_step.args) - self.assertEqual(t_dict_step['priority'], t_step.priority) - - def test_deploy_template_sample(self): - sample = api_deploy_template.DeployTemplate.sample(expand=False) - self.assertEqual('534e73fa-1014-4e58-969a-814cc0cb9d43', sample.uuid) - self.assertEqual('CUSTOM_RAID1', sample.name) - self.assertEqual({'foo': 'bar'}, sample.extra) - - class BaseDeployTemplatesAPITest(test_api_base.BaseApiTest): headers = {api_base.Version.string: str(api_v1.max_version())} invalid_version_headers = {api_base.Version.string: '1.54'} @@ -361,7 +338,7 @@ class TestPatch(BaseDeployTemplatesAPITest): self.assertEqual(http_client.BAD_REQUEST, response.status_code) self.assertTrue(response.json['error_message']) if error_msg: - self.assertRegex(response.json['error_message'], error_msg) + self.assertIn(error_msg, response.json['error_message']) self.assertFalse(mock_save.called) return response @@ -423,7 +400,7 @@ class TestPatch(BaseDeployTemplatesAPITest): self._test_update_bad_request( mock_save, [{'path': '/name', 'value': 'aa:bb_cc', 'op': 'replace'}], - 'Deploy template name must be a valid trait') + "'aa:bb_cc' does not match '^CUSTOM_[A-Z0-9_]+$'") def test_update_by_id_invalid_api_version(self, mock_save): name = 'CUSTOM_DT2' @@ -491,18 +468,19 @@ class TestPatch(BaseDeployTemplatesAPITest): name = 'CUSTOM_' + 'X' * 249 patch = [{'path': '/name', 'op': 'replace', 'value': name}] self._test_update_bad_request( - mock_save, patch, 'Deploy template name must be a valid trait') + mock_save, patch, "'%s' is too long" % name) def test_replace_invalid_name_not_a_trait(self, mock_save): name = 'not-a-trait' patch = [{'path': '/name', 'op': 'replace', 'value': name}] self._test_update_bad_request( - mock_save, patch, 'Deploy template name must be a valid trait') + mock_save, patch, + "'not-a-trait' does not match '^CUSTOM_[A-Z0-9_]+$'") def test_replace_invalid_name_none(self, mock_save): patch = [{'path': '/name', 'op': 'replace', 'value': None}] self._test_update_bad_request( - mock_save, patch, "Deploy template name cannot be None") + mock_save, patch, "None is not of type 'string'") def test_replace_duplicate_step(self, mock_save): # interface & step combination must be unique. @@ -528,7 +506,7 @@ class TestPatch(BaseDeployTemplatesAPITest): } patch = [{'path': '/steps/0', 'op': 'replace', 'value': step}] self._test_update_bad_request( - mock_save, patch, "Invalid input for field/attribute interface.") + mock_save, patch, "'foo' is not one of") def test_replace_non_existent_step_fail(self, mock_save): step = { @@ -543,7 +521,7 @@ class TestPatch(BaseDeployTemplatesAPITest): def test_replace_empty_step_list_fail(self, mock_save): patch = [{'path': '/steps', 'op': 'replace', 'value': []}] self._test_update_bad_request( - mock_save, patch, 'No deploy steps specified') + mock_save, patch, '[] is too short') def _test_remove_not_allowed(self, mock_save, field, error_msg=None): patch = [{'path': '/%s' % field, 'op': 'remove'}] @@ -552,17 +530,17 @@ class TestPatch(BaseDeployTemplatesAPITest): def test_remove_uuid(self, mock_save): self._test_remove_not_allowed( mock_save, 'uuid', - "'/uuid' is an internal attribute and can not be updated") + "Cannot patch /uuid") def test_remove_name(self, mock_save): self._test_remove_not_allowed( mock_save, 'name', - "'/name' is a mandatory attribute and can not be removed") + "'name' is a required property") def test_remove_steps(self, mock_save): self._test_remove_not_allowed( mock_save, 'steps', - "'/steps' is a mandatory attribute and can not be removed") + "'steps' is a required property") def test_remove_foo(self, mock_save): self._test_remove_not_allowed(mock_save, 'foo') @@ -571,7 +549,7 @@ class TestPatch(BaseDeployTemplatesAPITest): patch = [{'path': '/steps/0/interface', 'op': 'replace', 'value': 'foo'}] self._test_update_bad_request( - mock_save, patch, "Invalid input for field/attribute interface.") + mock_save, patch, "'foo' is not one of") def test_replace_multi(self, mock_save): steps = [ @@ -639,7 +617,7 @@ class TestPatch(BaseDeployTemplatesAPITest): def test_remove_only_step_fail(self, mock_save): patch = [{'path': '/steps/0', 'op': 'remove'}] self._test_update_bad_request( - mock_save, patch, "No deploy steps specified") + mock_save, patch, "[] is too short") def test_remove_non_existent_step_property_fail(self, mock_save): patch = [{'path': '/steps/0/non-existent', 'op': 'remove'}] @@ -648,7 +626,8 @@ class TestPatch(BaseDeployTemplatesAPITest): def test_add_root_non_existent(self, mock_save): patch = [{'path': '/foo', 'value': 'bar', 'op': 'add'}] self._test_update_bad_request( - mock_save, patch, "Adding a new attribute \\(/foo\\)") + mock_save, patch, + "Cannot patch /foo") def test_add_too_high_index_step_fail(self, mock_save): step = { @@ -790,13 +769,13 @@ class TestPost(BaseDeployTemplatesAPITest): name = 'CUSTOM_' + 'X' * 249 tdict = test_api_utils.post_get_test_deploy_template(name=name) self._test_create_bad_request( - tdict, 'Deploy template name must be a valid trait') + tdict, "'%s' is too long" % name) def test_create_name_invalid_not_a_trait(self): name = 'not-a-trait' tdict = test_api_utils.post_get_test_deploy_template(name=name) self._test_create_bad_request( - tdict, 'Deploy template name must be a valid trait') + tdict, "'not-a-trait' does not match '^CUSTOM_[A-Z0-9_]+$'") def test_create_steps_invalid_duplicate(self): steps = [ @@ -814,7 +793,7 @@ class TestPost(BaseDeployTemplatesAPITest): def _test_create_no_mandatory_field(self, field): tdict = test_api_utils.post_get_test_deploy_template() del tdict[field] - self._test_create_bad_request(tdict, "Mandatory field missing") + self._test_create_bad_request(tdict, "is a required property") def test_create_no_mandatory_field_name(self): self._test_create_no_mandatory_field('name') @@ -825,7 +804,7 @@ class TestPost(BaseDeployTemplatesAPITest): def _test_create_no_mandatory_step_field(self, field): tdict = test_api_utils.post_get_test_deploy_template() del tdict['steps'][0][field] - self._test_create_bad_request(tdict, "Mandatory field missing") + self._test_create_bad_request(tdict, "is a required property") def test_create_no_mandatory_step_field_interface(self): self._test_create_no_mandatory_step_field('interface') @@ -846,59 +825,69 @@ class TestPost(BaseDeployTemplatesAPITest): def test_create_invalid_field_name(self): self._test_create_invalid_field( - 'name', 42, 'Invalid input for field/attribute name') + 'name', 42, "42 is not of type 'string'") def test_create_invalid_field_name_none(self): self._test_create_invalid_field( - 'name', None, "Deploy template name cannot be None") + 'name', None, "None is not of type 'string'") def test_create_invalid_field_steps(self): self._test_create_invalid_field( - 'steps', {}, "Invalid input for field/attribute template") + 'steps', {}, "{} is not of type 'array'") def test_create_invalid_field_empty_steps(self): self._test_create_invalid_field( - 'steps', [], "No deploy steps specified") + 'steps', [], "[] is too short") def test_create_invalid_field_extra(self): self._test_create_invalid_field( - 'extra', 42, "Invalid input for field/attribute template") + 'extra', 42, "42 is not of type 'object'") def test_create_invalid_field_foo(self): self._test_create_invalid_field( - 'foo', 'bar', "Unknown attribute for argument template: foo") + 'foo', 'bar', + "Additional properties are not allowed ('foo' was unexpected)") def _test_create_invalid_step_field(self, field, value, error_msg=None): tdict = test_api_utils.post_get_test_deploy_template() tdict['steps'][0][field] = value if error_msg is None: - error_msg = "Invalid input for field/attribute" + error_msg = "Deploy template invalid: " self._test_create_bad_request(tdict, error_msg) def test_create_invalid_step_field_interface1(self): - self._test_create_invalid_step_field('interface', [3]) + self._test_create_invalid_step_field( + 'interface', [3], "[3] is not of type 'string'") def test_create_invalid_step_field_interface2(self): - self._test_create_invalid_step_field('interface', 'foo') + self._test_create_invalid_step_field( + 'interface', 'foo', "'foo' is not one of") def test_create_invalid_step_field_step(self): - self._test_create_invalid_step_field('step', 42) + self._test_create_invalid_step_field( + 'step', 42, "42 is not of type 'string'") def test_create_invalid_step_field_args1(self): - self._test_create_invalid_step_field('args', 'not a dict') + self._test_create_invalid_step_field( + 'args', 'not a dict', "'not a dict' is not of type 'object'") def test_create_invalid_step_field_args2(self): - self._test_create_invalid_step_field('args', []) + self._test_create_invalid_step_field( + 'args', [], "[] is not of type 'object'") def test_create_invalid_step_field_priority(self): - self._test_create_invalid_step_field('priority', 'not a number') + self._test_create_invalid_step_field( + 'priority', 'not a number', + "'not a number' is not of type 'integer'") def test_create_invalid_step_field_negative_priority(self): - self._test_create_invalid_step_field('priority', -1) + self._test_create_invalid_step_field( + 'priority', -1, "-1 is less than the minimum of 0") def test_create_invalid_step_field_foo(self): self._test_create_invalid_step_field( - 'foo', 'bar', "Unknown attribute for argument template.steps: foo") + 'foo', 'bar', + "Additional properties are not allowed ('foo' was unexpected)") def test_create_step_string_priority(self): tdict = test_api_utils.post_get_test_deploy_template() diff --git a/ironic/tests/unit/api/utils.py b/ironic/tests/unit/api/utils.py index 76a9419e63..fd005ed0ad 100644 --- a/ironic/tests/unit/api/utils.py +++ b/ironic/tests/unit/api/utils.py @@ -25,7 +25,6 @@ from ironic.api.controllers.v1 import deploy_template as dt_controller from ironic.api.controllers.v1 import node as node_controller from ironic.api.controllers.v1 import port as port_controller from ironic.api.controllers.v1 import portgroup as portgroup_controller -from ironic.api.controllers.v1 import types from ironic.api.controllers.v1 import utils as api_utils from ironic.api.controllers.v1 import volume_connector as vc_controller from ironic.api.controllers.v1 import volume_target as vt_controller @@ -212,14 +211,12 @@ def deploy_template_post_data(**kw): # These values are not part of the API object template.pop('version') # Remove internal attributes from each step. - step_internal = types.JsonPatchType.internal_attrs() - step_internal.append('deploy_template_id') - template['steps'] = [remove_internal(step, step_internal) + step_internal = dt_controller.STEP_SCHEMA['properties'] + template['steps'] = [remove_other_fields(step, step_internal) for step in template['steps']] # Remove internal attributes from the template. - dt_patch = dt_controller.DeployTemplatePatchType - internal = dt_patch.internal_attrs() - return remove_internal(template, internal) + return remove_other_fields( + template, dt_controller.TEMPLATE_SCHEMA['properties']) def post_get_test_deploy_template(**kw):