Merge "API: Check for reserved words when naming a node"

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.

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

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,

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)))

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.