Correct api version check conditional for node.name

Currently the name acceptable check is under the condition of
"if name", so if you specify name to an empty value like "",
that will skip the API version check.

Change-Id: I85dc67ea7ac4f38801dc4675f85fac5e1fc1b364
Closes-Bug: #1563694

ironic/api/controllers/v1/node.py

@@ -1208,7 +1208,7 @@ class NodesController(rest.RestController):
             e.code = http_client.BAD_REQUEST
             raise e
 
-        if node.name:
+        if (node.name != wtypes.Unset and node.name is not None):
             error_msg = _("Cannot create node with invalid name "
                           "%(name)s") % {'name': node.name}
             self._check_name_acceptable(node.name, error_msg)
@@ -1259,7 +1259,7 @@ class NodesController(rest.RestController):
                 msg % node_ident, status_code=http_client.CONFLICT)
 
         name = api_utils.get_patch_value(patch, '/name')
-        if name:
+        if name is not None:
             error_msg = _("Node %(node)s: Cannot change name to invalid "
                           "name '%(name)s'") % {'node': node_ident,
                                                 'name': name}

ironic/tests/unit/api/v1/test_nodes.py

@@ -1269,6 +1269,29 @@ class TestPatch(test_api_base.BaseApiTest):
         self.assertEqual(http_client.BAD_REQUEST, response.status_code)
         self.assertTrue(response.json['error_message'])
 
+    def test_patch_add_name_empty_invalid(self):
+        test_name = ''
+        response = self.patch_json('/nodes/%s' % self.node_no_name.uuid,
+                                   [{'path': '/name',
+                                     'op': 'add',
+                                     'value': test_name}],
+                                   headers={api_base.Version.string: "1.5"},
+                                   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_empty_not_acceptable(self):
+        test_name = ''
+        response = self.patch_json('/nodes/%s' % self.node_no_name.uuid,
+                                   [{'path': '/name',
+                                     'op': 'add',
+                                     'value': test_name}],
+                                   expect_errors=True)
+        self.assertEqual('application/json', response.content_type)
+        self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_code)
+        self.assertTrue(response.json['error_message'])
+
     def test_patch_name_replace_ok(self):
         self.mock_update_node.return_value = self.node
         test_name = 'guido-van-rossum'
@@ -1380,6 +1403,22 @@ class TestPost(test_api_base.BaseApiTest):
         self.assertEqual(urlparse.urlparse(response.location).path,
                          expected_location)
 
+    def test_create_node_name_empty_invalid(self):
+        ndict = test_api_utils.post_get_test_node(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_name_empty_not_acceptable(self):
+        ndict = test_api_utils.post_get_test_node(name='')
+        response = self.post_json('/nodes', ndict, expect_errors=True)
+        self.assertEqual(http_client.NOT_ACCEPTABLE, 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,

releasenotes/notes/correct-api-version-check-conditional-for-nodename-439bebc02fb5493d.yaml

@@ -0,0 +1,5 @@
+---
+fixes:
+  - Correct api version check conditional for node.name to address
+    an issue that we could set node name to '' using API version lower than
+    1.5, where node names were introduced.