diff --git a/ironic/api/controllers/v1/node.py b/ironic/api/controllers/v1/node.py index 6de1c83771..c28110898a 100644 --- a/ironic/api/controllers/v1/node.py +++ b/ironic/api/controllers/v1/node.py @@ -1032,11 +1032,21 @@ class NodesController(rest.RestController): """ if not api_utils.allow_node_logical_names(): raise exception.NotAcceptable() + + reserved_names = api_utils.get_controller_reserved_names( + NodesController) for name in names: if not api_utils.is_valid_node_name(name): raise wsme.exc.ClientSideError( error_msg % {'name': name}, status_code=http_client.BAD_REQUEST) + if name in reserved_names: + raise wsme.exc.ClientSideError( + 'The word "%(name)s" is reserved and can not be used as a ' + 'node name. Other reserved words are: %(reserved)s' % + {'name': name, + 'reserved': ', '.join(reserved_names)}, + status_code=http_client.BAD_REQUEST) def _update_changed_fields(self, node, rpc_node): """Update rpc_node based on changed fields in a node. diff --git a/ironic/api/controllers/v1/utils.py b/ironic/api/controllers/v1/utils.py index fedf0da14d..85ffd3486d 100644 --- a/ironic/api/controllers/v1/utils.py +++ b/ironic/api/controllers/v1/utils.py @@ -13,10 +13,13 @@ # License for the specific language governing permissions and limitations # under the License. +import inspect + import jsonpatch from oslo_config import cfg from oslo_utils import uuidutils import pecan +from pecan import rest import six from six.moves import http_client from webob.static import FileIter @@ -284,3 +287,23 @@ def allow_links_node_states_and_driver_properties(): """ return (pecan.request.version.minor >= versions.MINOR_14_LINKS_NODESTATES_DRIVERPROPERTIES) + + +def get_controller_reserved_names(cls): + """Get reserved names for a given controller. + + Inspect the controller class and return the reserved names within + it. Reserved names are names that can not be used as an identifier + for a resource because the names are either being used as a custom + action or is the name of a nested controller inside the given class. + + :param cls: The controller class to be inspected. + """ + reserved_names = [ + name for name, member in inspect.getmembers(cls) + if isinstance(member, rest.RestController)] + + if hasattr(cls, '_custom_actions'): + reserved_names += cls._custom_actions.keys() + + return reserved_names diff --git a/ironic/tests/unit/api/v1/test_nodes.py b/ironic/tests/unit/api/v1/test_nodes.py index 0097d37bfb..5e26b5d6ea 100644 --- a/ironic/tests/unit/api/v1/test_nodes.py +++ b/ironic/tests/unit/api/v1/test_nodes.py @@ -1256,19 +1256,27 @@ class TestPatch(test_api_base.BaseApiTest): self.assertEqual('application/json', response.content_type) self.assertEqual(http_client.OK, response.status_code) - def test_patch_add_name_invalid(self): + def _patch_add_name_invalid_or_reserved(self, name): self.mock_update_node.return_value = self.node_no_name - test_name = 'i am invalid' response = self.patch_json('/nodes/%s' % self.node_no_name.uuid, [{'path': '/name', 'op': 'add', - 'value': test_name}], + 'value': name}], headers={api_base.Version.string: "1.10"}, expect_errors=True) self.assertEqual('application/json', response.content_type) self.assertEqual(http_client.BAD_REQUEST, response.status_code) self.assertTrue(response.json['error_message']) + def test_patch_add_name_invalid(self): + self._patch_add_name_invalid_or_reserved('i am invalid') + + def test_patch_add_name_reserved(self): + reserved_names = api_utils.get_controller_reserved_names( + api_node.NodesController) + for name in reserved_names: + self._patch_add_name_invalid_or_reserved(name) + def test_patch_add_name_empty_invalid(self): test_name = '' response = self.patch_json('/nodes/%s' % self.node_no_name.uuid, @@ -1450,6 +1458,18 @@ class TestPost(test_api_base.BaseApiTest): self.assertEqual('application/json', response.content_type) self.assertTrue(response.json['error_message']) + def test_create_node_reserved_name(self): + reserved_names = api_utils.get_controller_reserved_names( + api_node.NodesController) + for name in reserved_names: + ndict = test_api_utils.post_get_test_node(name=name) + response = self.post_json( + '/nodes', ndict, headers={api_base.Version.string: "1.10"}, + expect_errors=True) + self.assertEqual(http_client.BAD_REQUEST, response.status_int) + self.assertEqual('application/json', response.content_type) + self.assertTrue(response.json['error_message']) + def test_create_node_default_state_none(self): ndict = test_api_utils.post_get_test_node() response = self.post_json('/nodes', ndict, diff --git a/ironic/tests/unit/api/v1/test_utils.py b/ironic/tests/unit/api/v1/test_utils.py index 1b77c5b29c..3f9da1e424 100644 --- a/ironic/tests/unit/api/v1/test_utils.py +++ b/ironic/tests/unit/api/v1/test_utils.py @@ -22,6 +22,7 @@ from six.moves import http_client from webob.static import FileIter import wsme +from ironic.api.controllers.v1 import node as api_node from ironic.api.controllers.v1 import utils from ironic.common import exception from ironic import objects @@ -356,3 +357,10 @@ class TestVendorPassthru(base.TestCase): def test_vendor_passthru_attach_byte_to_byte(self): self._test_vendor_passthru_attach(b'\x00\x01', b'\x00\x01') + + def test_get_controller_reserved_names(self): + expected = ['maintenance', 'management', 'ports', 'states', + 'vendor_passthru', 'validate', 'detail'] + self.assertEqual(sorted(expected), + sorted(utils.get_controller_reserved_names( + api_node.NodesController))) diff --git a/releasenotes/notes/reserved-node-names-67a08012ed1131ae.yaml b/releasenotes/notes/reserved-node-names-67a08012ed1131ae.yaml new file mode 100644 index 0000000000..c490b74f88 --- /dev/null +++ b/releasenotes/notes/reserved-node-names-67a08012ed1131ae.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - Fixes a problem which allowed nodes to be named with some reserved + words that are implicitly not allowed due the way the Ironic API + works.