From 7480e581d988de2bdcdb6dec483026e307b4bff2 Mon Sep 17 00:00:00 2001 From: Mark Goddard Date: Tue, 12 Mar 2019 14:23:03 +0000 Subject: [PATCH] Return 405 for old versions in allocation and deploy template APIs This is the response given by ironic's API for POST, PATCH or DELETE against unsupported endpoints. New endpoints with old microversions should do the same. Change-Id: Ife4077fd52ff14597c4bee588b914685bdd3d95f Depends-On: https://review.openstack.org/642759 --- ironic/api/controllers/v1/allocation.py | 7 +++++-- ironic/api/controllers/v1/deploy_template.py | 7 +++++-- .../tests/unit/api/controllers/v1/test_allocation.py | 6 +++--- .../unit/api/controllers/v1/test_deploy_template.py | 10 +++++----- 4 files changed, 18 insertions(+), 12 deletions(-) diff --git a/ironic/api/controllers/v1/allocation.py b/ironic/api/controllers/v1/allocation.py index d370575fb8..d461f6d4f2 100644 --- a/ironic/api/controllers/v1/allocation.py +++ b/ironic/api/controllers/v1/allocation.py @@ -204,8 +204,11 @@ class AllocationsController(pecan.rest.RestController): @pecan.expose() def _route(self, args, request=None): if not api_utils.allow_allocations(): - raise webob_exc.HTTPNotFound(_( - "The API version does not allow allocations")) + msg = _("The API version does not allow allocations") + if pecan.request.method == "GET": + raise webob_exc.HTTPNotFound(msg) + else: + raise webob_exc.HTTPMethodNotAllowed(msg) return super(AllocationsController, self)._route(args, request) def _get_allocations_collection(self, node_ident=None, resource_class=None, diff --git a/ironic/api/controllers/v1/deploy_template.py b/ironic/api/controllers/v1/deploy_template.py index 5b5c8a046e..b084847a07 100644 --- a/ironic/api/controllers/v1/deploy_template.py +++ b/ironic/api/controllers/v1/deploy_template.py @@ -259,8 +259,11 @@ class DeployTemplatesController(rest.RestController): @pecan.expose() def _route(self, args, request=None): if not api_utils.allow_deploy_templates(): - raise webob_exc.HTTPNotFound(_( - "The API version does not allow deploy templates")) + msg = _("The API version does not allow deploy templates") + if pecan.request.method == "GET": + raise webob_exc.HTTPNotFound(msg) + else: + raise webob_exc.HTTPMethodNotAllowed(msg) return super(DeployTemplatesController, self)._route(args, request) def _update_changed_fields(self, template, rpc_template): diff --git a/ironic/tests/unit/api/controllers/v1/test_allocation.py b/ironic/tests/unit/api/controllers/v1/test_allocation.py index 875a531db4..05d78faa2f 100644 --- a/ironic/tests/unit/api/controllers/v1/test_allocation.py +++ b/ironic/tests/unit/api/controllers/v1/test_allocation.py @@ -450,7 +450,7 @@ class TestPost(test_api_base.BaseApiTest): response = self.post_json( '/allocations', adict, headers={api_base.Version.string: '1.50'}, expect_errors=True) - self.assertEqual(http_client.NOT_FOUND, response.status_int) + self.assertEqual(http_client.METHOD_NOT_ALLOWED, response.status_int) def test_create_allocation_doesnt_contain_id(self): with mock.patch.object(self.dbapi, 'create_allocation', @@ -680,7 +680,7 @@ class TestDelete(test_api_base.BaseApiTest): response = self.delete('/allocations/%s' % self.allocation.uuid, expect_errors=True, headers={api_base.Version.string: '1.14'}) - self.assertEqual(http_client.NOT_FOUND, response.status_int) + self.assertEqual(http_client.METHOD_NOT_ALLOWED, response.status_int) def test_delete_allocation_invalid_api_version_without_check(self, mock_destroy): @@ -688,7 +688,7 @@ class TestDelete(test_api_base.BaseApiTest): response = self.delete('/allocations/ba!na!na1', expect_errors=True, headers={api_base.Version.string: '1.14'}) - self.assertEqual(http_client.NOT_FOUND, response.status_int) + self.assertEqual(http_client.METHOD_NOT_ALLOWED, response.status_int) def test_delete_allocation_by_name(self, mock_destroy): self.delete('/allocations/%s' % self.allocation.name, diff --git a/ironic/tests/unit/api/controllers/v1/test_deploy_template.py b/ironic/tests/unit/api/controllers/v1/test_deploy_template.py index 6bc77f1916..25499589b9 100644 --- a/ironic/tests/unit/api/controllers/v1/test_deploy_template.py +++ b/ironic/tests/unit/api/controllers/v1/test_deploy_template.py @@ -414,7 +414,7 @@ class TestPatch(BaseDeployTemplatesAPITest): 'op': 'add'}], headers=headers, expect_errors=True) - self.assertEqual(http_client.NOT_FOUND, response.status_int) + self.assertEqual(http_client.METHOD_NOT_ALLOWED, response.status_int) self.assertFalse(mock_save.called) def test_update_by_name_old_api_version(self, mock_save): @@ -424,7 +424,7 @@ class TestPatch(BaseDeployTemplatesAPITest): 'value': name, 'op': 'add'}], expect_errors=True) - self.assertEqual(http_client.NOT_FOUND, response.status_int) + self.assertEqual(http_client.METHOD_NOT_ALLOWED, response.status_int) self.assertFalse(mock_save.called) def test_update_not_found(self, mock_save): @@ -715,7 +715,7 @@ class TestPost(BaseDeployTemplatesAPITest): response = self.post_json( '/deploy_templates', tdict, headers=self.invalid_version_headers, expect_errors=True) - self.assertEqual(http_client.NOT_FOUND, response.status_int) + self.assertEqual(http_client.METHOD_NOT_ALLOWED, response.status_int) def test_create_doesnt_contain_id(self): with mock.patch.object( @@ -944,14 +944,14 @@ class TestDelete(BaseDeployTemplatesAPITest): response = self.delete('/deploy_templates/%s' % self.template.uuid, expect_errors=True, headers=self.invalid_version_headers) - self.assertEqual(http_client.NOT_FOUND, response.status_int) + self.assertEqual(http_client.METHOD_NOT_ALLOWED, response.status_int) def test_delete_old_api_version(self, mock_dpt): # Names like CUSTOM_1 were not valid in API 1.1, but the check should # go after the microversion check. response = self.delete('/deploy_templates/%s' % self.template.name, expect_errors=True) - self.assertEqual(http_client.NOT_FOUND, response.status_int) + self.assertEqual(http_client.METHOD_NOT_ALLOWED, response.status_int) def test_delete_by_name_non_existent(self, mock_dpt): res = self.delete('/deploy_templates/%s' % 'blah', expect_errors=True,