From 24be3c22866db8acbd03ea9419866dddcdb74404 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Fri, 26 Mar 2021 15:22:31 +0100 Subject: [PATCH] Allow using per-site network_data schema I have been against it since the beginning of this work, hoping that we can settle down on one network data format, one is more native for Ironic because of our relation to OpenStack. This has not happened, with e.g. CoreOS only using its own formats. So, let it be. Use with caution. Change-Id: I872d010517cd343fcbcafadb4535f07ca15c2c95 --- ironic/api/controllers/v1/node.py | 161 ++++++++++-------- ironic/conf/api.py | 4 + ironic/tests/unit/api/base.py | 5 + .../unit/api/controllers/v1/test_node.py | 42 +++++ ironic/tests/unit/api/utils.py | 2 +- .../network_data_schema-9342edf3c47b2a66.yaml | 5 + 6 files changed, 151 insertions(+), 68 deletions(-) create mode 100644 releasenotes/notes/network_data_schema-9342edf3c47b2a66.yaml diff --git a/ironic/api/controllers/v1/node.py b/ironic/api/controllers/v1/node.py index 328d020a56..ba36d261c6 100644 --- a/ironic/api/controllers/v1/node.py +++ b/ironic/api/controllers/v1/node.py @@ -17,7 +17,6 @@ import copy import datetime from http import client as http_client import json -import os from ironic_lib import metrics_utils import jsonschema @@ -123,62 +122,75 @@ ALLOWED_TARGET_POWER_STATES = (ir_states.POWER_ON, _NODE_DESCRIPTION_MAX_LENGTH = 4096 -with open(os.path.join(os.path.dirname(__file__), - 'network-data-schema.json'), 'rb') as fl: - NETWORK_DATA_SCHEMA = json.load(fl) +_NETWORK_DATA_SCHEMA = None -NODE_SCHEMA = { - 'type': 'object', - 'properties': { - 'automated_clean': {'type': ['string', 'boolean', 'null']}, - 'bios_interface': {'type': ['string', 'null']}, - 'boot_interface': {'type': ['string', 'null']}, - 'chassis_uuid': {'type': ['string', 'null']}, - 'conductor_group': {'type': ['string', 'null']}, - 'console_enabled': {'type': ['string', 'boolean', 'null']}, - 'console_interface': {'type': ['string', 'null']}, - 'deploy_interface': {'type': ['string', 'null']}, - 'description': {'type': ['string', 'null'], - 'maxLength': _NODE_DESCRIPTION_MAX_LENGTH}, - 'driver': {'type': 'string'}, - 'driver_info': {'type': ['object', 'null']}, - 'extra': {'type': ['object', 'null']}, - 'inspect_interface': {'type': ['string', 'null']}, - 'instance_info': {'type': ['object', 'null']}, - 'instance_uuid': {'type': ['string', 'null']}, - 'lessee': {'type': ['string', 'null']}, - 'management_interface': {'type': ['string', 'null']}, - 'maintenance': {'type': ['string', 'boolean', 'null']}, - 'name': {'type': ['string', 'null']}, - 'network_data': {'anyOf': [ - {'type': 'null'}, - {'type': 'object', 'additionalProperties': False}, - NETWORK_DATA_SCHEMA - ]}, - 'network_interface': {'type': ['string', 'null']}, - 'owner': {'type': ['string', 'null']}, - 'power_interface': {'type': ['string', 'null']}, - 'properties': {'type': ['object', 'null']}, - 'raid_interface': {'type': ['string', 'null']}, - 'rescue_interface': {'type': ['string', 'null']}, - 'resource_class': {'type': ['string', 'null'], 'maxLength': 80}, - 'retired': {'type': ['string', 'boolean', 'null']}, - 'retired_reason': {'type': ['string', 'null']}, - 'storage_interface': {'type': ['string', 'null']}, - 'uuid': {'type': ['string', 'null']}, - 'vendor_interface': {'type': ['string', 'null']}, - }, - 'required': ['driver'], - 'additionalProperties': False, - 'definitions': NETWORK_DATA_SCHEMA.get('definitions') -} -NODE_PATCH_SCHEMA = copy.deepcopy(NODE_SCHEMA) -# add schema for patchable fields -NODE_PATCH_SCHEMA['properties']['protected'] = { - 'type': ['string', 'boolean', 'null']} -NODE_PATCH_SCHEMA['properties']['protected_reason'] = { - 'type': ['string', 'null']} +def network_data_schema(): + global _NETWORK_DATA_SCHEMA + if _NETWORK_DATA_SCHEMA is None: + with open(CONF.api.network_data_schema) as fl: + _NETWORK_DATA_SCHEMA = json.load(fl) + return _NETWORK_DATA_SCHEMA + + +def node_schema(): + network_data = network_data_schema() + return { + 'type': 'object', + 'properties': { + 'automated_clean': {'type': ['string', 'boolean', 'null']}, + 'bios_interface': {'type': ['string', 'null']}, + 'boot_interface': {'type': ['string', 'null']}, + 'chassis_uuid': {'type': ['string', 'null']}, + 'conductor_group': {'type': ['string', 'null']}, + 'console_enabled': {'type': ['string', 'boolean', 'null']}, + 'console_interface': {'type': ['string', 'null']}, + 'deploy_interface': {'type': ['string', 'null']}, + 'description': {'type': ['string', 'null'], + 'maxLength': _NODE_DESCRIPTION_MAX_LENGTH}, + 'driver': {'type': 'string'}, + 'driver_info': {'type': ['object', 'null']}, + 'extra': {'type': ['object', 'null']}, + 'inspect_interface': {'type': ['string', 'null']}, + 'instance_info': {'type': ['object', 'null']}, + 'instance_uuid': {'type': ['string', 'null']}, + 'lessee': {'type': ['string', 'null']}, + 'management_interface': {'type': ['string', 'null']}, + 'maintenance': {'type': ['string', 'boolean', 'null']}, + 'name': {'type': ['string', 'null']}, + 'network_data': {'anyOf': [ + {'type': 'null'}, + {'type': 'object', 'additionalProperties': False}, + network_data + ]}, + 'network_interface': {'type': ['string', 'null']}, + 'owner': {'type': ['string', 'null']}, + 'power_interface': {'type': ['string', 'null']}, + 'properties': {'type': ['object', 'null']}, + 'raid_interface': {'type': ['string', 'null']}, + 'rescue_interface': {'type': ['string', 'null']}, + 'resource_class': {'type': ['string', 'null'], 'maxLength': 80}, + 'retired': {'type': ['string', 'boolean', 'null']}, + 'retired_reason': {'type': ['string', 'null']}, + 'storage_interface': {'type': ['string', 'null']}, + 'uuid': {'type': ['string', 'null']}, + 'vendor_interface': {'type': ['string', 'null']}, + }, + 'required': ['driver'], + 'additionalProperties': False, + 'definitions': network_data.get('definitions', {}) + } + + +def node_patch_schema(): + node_patch = copy.deepcopy(node_schema()) + # add schema for patchable fields + node_patch['properties']['protected'] = { + 'type': ['string', 'boolean', 'null']} + node_patch['properties']['protected_reason'] = { + 'type': ['string', 'null']} + return node_patch + NODE_VALIDATE_EXTRA = args.dict_valid( automated_clean=args.boolean, @@ -191,15 +203,30 @@ NODE_VALIDATE_EXTRA = args.dict_valid( uuid=args.uuid, ) -NODE_VALIDATOR = args.and_valid( - args.schema(NODE_SCHEMA), - NODE_VALIDATE_EXTRA -) -NODE_PATCH_VALIDATOR = args.and_valid( - args.schema(NODE_PATCH_SCHEMA), - NODE_VALIDATE_EXTRA -) +_NODE_VALIDATOR = None +_NODE_PATCH_VALIDATOR = None + + +def node_validator(name, value): + global _NODE_VALIDATOR + if _NODE_VALIDATOR is None: + _NODE_VALIDATOR = args.and_valid( + args.schema(node_schema()), + NODE_VALIDATE_EXTRA + ) + return _NODE_VALIDATOR(name, value) + + +def node_patch_validator(name, value): + global _NODE_PATCH_VALIDATOR + if _NODE_PATCH_VALIDATOR is None: + _NODE_PATCH_VALIDATOR = args.and_valid( + args.schema(node_patch_schema()), + NODE_VALIDATE_EXTRA + ) + return _NODE_PATCH_VALIDATOR(name, value) + PATCH_ALLOWED_FIELDS = [ 'automated_clean', @@ -334,7 +361,7 @@ def validate_network_data(network_data): :raises: Invalid if network data is not schema-compliant """ try: - jsonschema.validate(network_data, NETWORK_DATA_SCHEMA) + jsonschema.validate(network_data, network_data_schema()) except json_schema_exc.ValidationError as e: # NOTE: Even though e.message is deprecated in general, it is @@ -1838,7 +1865,7 @@ class NodesController(rest.RestController): api_utils.patch_update_changed_fields( node, rpc_node, fields=set(objects.Node.fields) - {'traits'}, - schema=NODE_PATCH_SCHEMA, + schema=node_patch_schema(), id_map={'chassis_id': chassis and chassis.id or None} ) @@ -2098,7 +2125,7 @@ class NodesController(rest.RestController): @METRICS.timer('NodesController.post') @method.expose(status_code=http_client.CREATED) @method.body('node') - @args.validate(node=NODE_VALIDATOR) + @args.validate(node=node_validator) def post(self, node): """Create a new node. @@ -2335,7 +2362,7 @@ class NodesController(rest.RestController): node_dict = api_utils.apply_jsonpatch(node_dict, patch) api_utils.patched_validate_with_schema( - node_dict, NODE_PATCH_SCHEMA, NODE_PATCH_VALIDATOR) + node_dict, node_patch_schema(), node_patch_validator) self._update_changed_fields(node_dict, rpc_node) # NOTE(tenbrae): we calculate the rpc topic here in case node.driver diff --git a/ironic/conf/api.py b/ironic/conf/api.py index ba23627b08..dcf235eddc 100644 --- a/ironic/conf/api.py +++ b/ironic/conf/api.py @@ -66,6 +66,10 @@ opts = [ default=300, mutable=True, help=_('Maximum interval (in seconds) for agent heartbeats.')), + cfg.StrOpt( + 'network_data_schema', + default='$pybasedir/api/controllers/v1/network-data-schema.json', + help=_("Schema for network data used by this deployment.")), ] opt_group = cfg.OptGroup(name='api', diff --git a/ironic/tests/unit/api/base.py b/ironic/tests/unit/api/base.py index ebf89df32a..6670be14e3 100644 --- a/ironic/tests/unit/api/base.py +++ b/ironic/tests/unit/api/base.py @@ -27,6 +27,7 @@ from oslo_config import cfg import pecan import pecan.testing +from ironic.api.controllers.v1 import node as api_node from ironic.tests.unit.db import base as db_base PATH_PREFIX = '/v1' @@ -65,6 +66,10 @@ class BaseApiTest(db_base.DbTestCase): self._check_version = p.start() self.addCleanup(p.stop) + api_node._NETWORK_DATA_SCHEMA = None + api_node._NODE_VALIDATOR = None + api_node._NODE_PATCH_VALIDATOR = None + def _make_app(self): # Determine where we are so we can set up paths in the config root_dir = self.path_get() diff --git a/ironic/tests/unit/api/controllers/v1/test_node.py b/ironic/tests/unit/api/controllers/v1/test_node.py index 185e8f27e1..302da10551 100644 --- a/ironic/tests/unit/api/controllers/v1/test_node.py +++ b/ironic/tests/unit/api/controllers/v1/test_node.py @@ -17,6 +17,7 @@ import datetime from http import client as http_client import json import os +import tempfile from unittest import mock from urllib import parse as urlparse @@ -3807,6 +3808,47 @@ class TestPatch(test_api_base.BaseApiTest): self.assertEqual('application/json', response.content_type) self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_code) + def test_update_network_data_wrong_format(self): + node = obj_utils.create_test_node(self.context, + uuid=uuidutils.generate_uuid()) + self.mock_update_node.return_value = node + headers = {api_base.Version.string: '1.66'} + + response = self.patch_json('/nodes/%s' % node.uuid, + [{'path': '/network_data', + 'value': {'cat': 'meow'}, + 'op': 'replace'}], + headers=headers, + expect_errors=True) + self.assertEqual('application/json', response.content_type) + self.assertEqual(http_client.BAD_REQUEST, response.status_code) + + def test_update_network_data_custom(self): + custom_schema = { + 'type': 'object', + 'properties': { + 'cat': {'type': 'string'}, + }, + } + with tempfile.NamedTemporaryFile('wt') as fp: + json.dump(custom_schema, fp) + fp.flush() + self.config(network_data_schema=fp.name, group='api') + + node = obj_utils.create_test_node(self.context, + uuid=uuidutils.generate_uuid(), + provision_state='active') + self.mock_update_node.return_value = node + headers = {api_base.Version.string: '1.66'} + + response = self.patch_json('/nodes/%s' % node.uuid, + [{'path': '/network_data', + 'value': {'cat': 'meow'}, + 'op': 'replace'}], + headers=headers) + self.assertEqual('application/json', response.content_type) + self.assertEqual(http_client.OK, response.status_code) + @mock.patch.object(api_utils, 'check_multiple_node_policies_and_retrieve', autospec=True) def test_patch_policy_update(self, mock_cmnpar): diff --git a/ironic/tests/unit/api/utils.py b/ironic/tests/unit/api/utils.py index d3963fc15a..9be350e8b5 100644 --- a/ironic/tests/unit/api/utils.py +++ b/ironic/tests/unit/api/utils.py @@ -111,7 +111,7 @@ def node_post_data(**kw): node.pop(field, None) return remove_other_fields( - node, node_controller.NODE_SCHEMA['properties']) + node, node_controller.node_schema()['properties']) def port_post_data(**kw): diff --git a/releasenotes/notes/network_data_schema-9342edf3c47b2a66.yaml b/releasenotes/notes/network_data_schema-9342edf3c47b2a66.yaml new file mode 100644 index 0000000000..803551bb41 --- /dev/null +++ b/releasenotes/notes/network_data_schema-9342edf3c47b2a66.yaml @@ -0,0 +1,5 @@ +--- +features: + - | + The network data schema is now configurable via the new configuration + options ``[api]network_data_schema``.