Stop guessing mime types based on URLs

Currently we have a pecan feature enabled that strips extensions
from the end of the URL and treat it like requested content type.
E.g. /v1/nodes.json is treated as /v1/nodes with requested content
type Application/Json. However, this prevents certain node names:
e.g. /v1/nodes/small.1 is treated like /v1/nodes/small with content
type of a man page. It does not make any sense for ironic API,
as we only support Application/Json content type (and .json suffix).

This change disabled this pecan feature. To keep backward compability
a new middleware stips the .json prefix and saves a flag in the
environment about its presence. API accepting names try to find
their resource first without, then with .json suffix.

The following endpoints are special-cased to support names with .json:
* Node GET, PATCH and DELETE
* Ramdisk heartbeat
* Port group GET, PATCH and DELETE

VIF API is not updated, so VIF IDs still cannot have .json suffix.

Change-Id: I789ecfeac9b64a9c4105a20619f7bf5dfc133189
Closes-Bug: #1643995

3 files changed, 136 insertions, 2 deletions

diff --git a/ironic/tests/unit/api/controllers/v1/test_node.py b/ironic/tests/unit/api/controllers/v1/test_node.py
index 2b2102817..43de1d8a2 100644
--- a/ironic/tests/unit/api/controllers/v1/test_node.py
+++ b/ironic/tests/unit/api/controllers/v1/test_node.py
@@ -157,6 +157,45 @@ class TestListNodes(test_api_base.BaseApiTest):
         # never expose the chassis_id
         self.assertNotIn('chassis_id', data)
 
+    def test_get_one_with_json(self):
+        # Test backward compatibility with guess_content_type_from_ext
+        node = obj_utils.create_test_node(self.context,
+                                          chassis_id=self.chassis.id)
+        data = self.get_json(
+            '/nodes/%s.json' % node.uuid,
+            headers={api_base.Version.string: str(api_v1.max_version())})
+        self.assertEqual(node.uuid, data['uuid'])
+
+    def test_get_one_with_json_in_name(self):
+        # Test that it is possible to name a node ending with .json
+        node = obj_utils.create_test_node(self.context,
+                                          name='node.json',
+                                          chassis_id=self.chassis.id)
+        data = self.get_json(
+            '/nodes/%s' % node.name,
+            headers={api_base.Version.string: str(api_v1.max_version())})
+        self.assertEqual(node.uuid, data['uuid'])
+
+    def test_get_one_with_suffix(self):
+        # This tests that we don't mess with mime-like suffixes
+        node = obj_utils.create_test_node(self.context,
+                                          name='test.1',
+                                          chassis_id=self.chassis.id)
+        data = self.get_json(
+            '/nodes/%s' % node.name,
+            headers={api_base.Version.string: str(api_v1.max_version())})
+        self.assertEqual(node.uuid, data['uuid'])
+
+    def test_get_one_with_double_json(self):
+        # Check that .json is only stripped once
+        node = obj_utils.create_test_node(self.context,
+                                          name='node.json',
+                                          chassis_id=self.chassis.id)
+        data = self.get_json(
+            '/nodes/%s.json' % node.name,
+            headers={api_base.Version.string: str(api_v1.max_version())})
+        self.assertEqual(node.uuid, data['uuid'])
+
     def test_node_states_field_hidden_in_lower_version(self):
         node = obj_utils.create_test_node(self.context,
                                           chassis_id=self.chassis.id)
@@ -1381,7 +1420,7 @@ class TestPatch(test_api_base.BaseApiTest):
     def setUp(self):
         super(TestPatch, self).setUp()
         self.chassis = obj_utils.create_test_chassis(self.context)
-        self.node = obj_utils.create_test_node(self.context, name='node-57',
+        self.node = obj_utils.create_test_node(self.context, name='node-57.1',
                                                chassis_id=self.chassis.id)
         self.node_no_name = obj_utils.create_test_node(
             self.context, uuid='deadbeef-0000-1111-2222-333333333333',
@@ -1458,6 +1497,25 @@ class TestPatch(test_api_base.BaseApiTest):
         self.mock_update_node.assert_called_once_with(
             mock.ANY, mock.ANY, 'test-topic')
 
+    def test_update_ok_by_name_with_json(self):
+        self.mock_update_node.return_value = self.node
+        (self
+         .mock_update_node
+         .return_value
+         .updated_at) = "2013-12-03T06:20:41.184720+00:00"
+        response = self.patch_json(
+            '/nodes/%s.json' % self.node.name,
+            [{'path': '/instance_uuid',
+              'value': 'aaaaaaaa-1111-bbbb-2222-cccccccccccc',
+              'op': 'replace'}],
+            headers={api_base.Version.string: "1.5"})
+        self.assertEqual('application/json', response.content_type)
+        self.assertEqual(http_client.OK, response.status_code)
+        self.assertEqual(self.mock_update_node.return_value.updated_at,
+                         timeutils.parse_isotime(response.json['updated_at']))
+        self.mock_update_node.assert_called_once_with(
+            mock.ANY, mock.ANY, 'test-topic')
+
     def test_update_state(self):
         response = self.patch_json('/nodes/%s' % self.node.uuid,
                                    [{'power_state': 'new state'}],
@@ -2780,11 +2838,18 @@ class TestDelete(test_api_base.BaseApiTest):
 
     @mock.patch.object(rpcapi.ConductorAPI, 'destroy_node')
     def test_delete_node_by_name(self, mock_dn):
-        node = obj_utils.create_test_node(self.context, name='foo')
+        node = obj_utils.create_test_node(self.context, name='foo.1')
         self.delete('/nodes/%s' % node.name,
                     headers={api_base.Version.string: "1.5"})
         mock_dn.assert_called_once_with(mock.ANY, node.uuid, 'test-topic')
 
+    @mock.patch.object(rpcapi.ConductorAPI, 'destroy_node')
+    def test_delete_node_by_name_with_json(self, mock_dn):
+        node = obj_utils.create_test_node(self.context, name='foo')
+        self.delete('/nodes/%s.json' % node.name,
+                    headers={api_base.Version.string: "1.5"})
+        mock_dn.assert_called_once_with(mock.ANY, node.uuid, 'test-topic')
+
     @mock.patch.object(objects.Node, 'get_by_uuid')
     def test_delete_node_not_found(self, mock_gbu):
         node = obj_utils.get_test_node(self.context)
diff --git a/ironic/tests/unit/api/controllers/v1/test_portgroup.py b/ironic/tests/unit/api/controllers/v1/test_portgroup.py
index 48b301f55..fb047bfa0 100644
--- a/ironic/tests/unit/api/controllers/v1/test_portgroup.py
+++ b/ironic/tests/unit/api/controllers/v1/test_portgroup.py
@@ -86,6 +86,29 @@ class TestListPortgroups(test_api_base.BaseApiTest):
         # never expose the node_id
         self.assertNotIn('node_id', data)
 
+    def test_get_one_with_json(self):
+        portgroup = obj_utils.create_test_portgroup(self.context,
+                                                    node_id=self.node.id)
+        data = self.get_json('/portgroups/%s.json' % portgroup.uuid,
+                             headers=self.headers)
+        self.assertEqual(portgroup.uuid, data['uuid'])
+
+    def test_get_one_with_json_in_name(self):
+        portgroup = obj_utils.create_test_portgroup(self.context,
+                                                    name='pg.json',
+                                                    node_id=self.node.id)
+        data = self.get_json('/portgroups/%s' % portgroup.uuid,
+                             headers=self.headers)
+        self.assertEqual(portgroup.uuid, data['uuid'])
+
+    def test_get_one_with_suffix(self):
+        portgroup = obj_utils.create_test_portgroup(self.context,
+                                                    name='pg.1',
+                                                    node_id=self.node.id)
+        data = self.get_json('/portgroups/%s' % portgroup.uuid,
+                             headers=self.headers)
+        self.assertEqual(portgroup.uuid, data['uuid'])
+
     def test_get_one_custom_fields(self):
         portgroup = obj_utils.create_test_portgroup(self.context,
                                                     node_id=self.node.id)
@@ -477,6 +500,7 @@ class TestPatch(test_api_base.BaseApiTest):
         super(TestPatch, self).setUp()
         self.node = obj_utils.create_test_node(self.context)
         self.portgroup = obj_utils.create_test_portgroup(self.context,
+                                                         name='pg.1',
                                                          node_id=self.node.id)
 
         p = mock.patch.object(rpcapi.ConductorAPI, 'get_topic_for')
@@ -522,6 +546,19 @@ class TestPatch(test_api_base.BaseApiTest):
         self.assertEqual(http_client.OK, response.status_code)
         self.assertEqual(extra, response.json['extra'])
 
+    def test_update_byname_with_json(self, mock_upd):
+        extra = {'foo': 'bar'}
+        mock_upd.return_value = self.portgroup
+        mock_upd.return_value.extra = extra
+        response = self.patch_json('/portgroups/%s.json' % self.portgroup.name,
+                                   [{'path': '/extra/foo',
+                                     'value': 'bar',
+                                     'op': 'add'}],
+                                   headers=self.headers)
+        self.assertEqual('application/json', response.content_type)
+        self.assertEqual(http_client.OK, response.status_code)
+        self.assertEqual(extra, response.json['extra'])
+
     def test_update_invalid_name(self, mock_upd):
         mock_upd.return_value = self.portgroup
         response = self.patch_json('/portgroups/%s' % self.portgroup.name,
@@ -1211,6 +1248,7 @@ class TestDelete(test_api_base.BaseApiTest):
         super(TestDelete, self).setUp()
         self.node = obj_utils.create_test_node(self.context)
         self.portgroup = obj_utils.create_test_portgroup(self.context,
+                                                         name='pg.1',
                                                          node_id=self.node.id)
 
         gtf = mock.patch.object(rpcapi.ConductorAPI, 'get_topic_for')
@@ -1269,6 +1307,11 @@ class TestDelete(test_api_base.BaseApiTest):
                     headers=self.headers)
         self.assertTrue(mock_dpt.called)
 
+    def test_delete_portgroup_byname_with_json(self, mock_dpt):
+        self.delete('/portgroups/%s.json' % self.portgroup.name,
+                    headers=self.headers)
+        self.assertTrue(mock_dpt.called)
+
     def test_delete_portgroup_byname_not_existed(self, mock_dpt):
         res = self.delete('/portgroups/%s' % 'blah', expect_errors=True,
                           headers=self.headers)
diff --git a/ironic/tests/unit/api/controllers/v1/test_ramdisk.py b/ironic/tests/unit/api/controllers/v1/test_ramdisk.py
index 0c9e77419..ec667634f 100644
--- a/ironic/tests/unit/api/controllers/v1/test_ramdisk.py
+++ b/ironic/tests/unit/api/controllers/v1/test_ramdisk.py
@@ -189,6 +189,32 @@ class TestHeartbeat(test_api_base.BaseApiTest):
                                                topic='test-topic')
 
     @mock.patch.object(rpcapi.ConductorAPI, 'heartbeat', autospec=True)
+    def test_ok_with_json(self, mock_heartbeat):
+        node = obj_utils.create_test_node(self.context)
+        response = self.post_json(
+            '/heartbeat/%s.json' % node.uuid,
+            {'callback_url': 'url'},
+            headers={api_base.Version.string: str(api_v1.max_version())})
+        self.assertEqual(http_client.ACCEPTED, response.status_int)
+        self.assertEqual(b'', response.body)
+        mock_heartbeat.assert_called_once_with(mock.ANY, mock.ANY,
+                                               node.uuid, 'url', None,
+                                               topic='test-topic')
+
+    @mock.patch.object(rpcapi.ConductorAPI, 'heartbeat', autospec=True)
+    def test_ok_by_name(self, mock_heartbeat):
+        node = obj_utils.create_test_node(self.context, name='test.1')
+        response = self.post_json(
+            '/heartbeat/%s' % node.name,
+            {'callback_url': 'url'},
+            headers={api_base.Version.string: str(api_v1.max_version())})
+        self.assertEqual(http_client.ACCEPTED, response.status_int)
+        self.assertEqual(b'', response.body)
+        mock_heartbeat.assert_called_once_with(mock.ANY, mock.ANY,
+                                               node.uuid, 'url', None,
+                                               topic='test-topic')
+
+    @mock.patch.object(rpcapi.ConductorAPI, 'heartbeat', autospec=True)
     def test_ok_agent_version(self, mock_heartbeat):
         node = obj_utils.create_test_node(self.context)
         response = self.post_json(