openstack/ironic
Code Issues Proposed changes

Merge "Return 405 for old versions in allocation and deploy template APIs"

Browse Source
This commit is contained in:
Zuul 2019-03-13 20:17:38 +00:00 committed by Gerrit Code Review
commit c9556d8b51
4 changed files with 18 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
ironic/api/controllers/v1/allocation.py
View File

@ -204,8 +204,11 @@ class AllocationsController(pecan.rest.RestController):
@pecan.expose() @pecan.expose()
def _route(self, args, request=None): def _route(self, args, request=None):
if not api_utils.allow_allocations(): if not api_utils.allow_allocations():
raise webob_exc.HTTPNotFound(_( msg = _("The API version does not allow allocations")
"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) return super(AllocationsController, self)._route(args, request)
def _get_allocations_collection(self, node_ident=None, resource_class=None, def _get_allocations_collection(self, node_ident=None, resource_class=None,

7
ironic/api/controllers/v1/deploy_template.py
View File

@ -259,8 +259,11 @@ class DeployTemplatesController(rest.RestController):
@pecan.expose() @pecan.expose()
def _route(self, args, request=None): def _route(self, args, request=None):
if not api_utils.allow_deploy_templates(): if not api_utils.allow_deploy_templates():
raise webob_exc.HTTPNotFound(_( msg = _("The API version does not allow deploy templates")
"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) return super(DeployTemplatesController, self)._route(args, request)
def _update_changed_fields(self, template, rpc_template): def _update_changed_fields(self, template, rpc_template):

6
ironic/tests/unit/api/controllers/v1/test_allocation.py
View File

@ -450,7 +450,7 @@ class TestPost(test_api_base.BaseApiTest):
response = self.post_json( response = self.post_json(
'/allocations', adict, headers={api_base.Version.string: '1.50'}, '/allocations', adict, headers={api_base.Version.string: '1.50'},
expect_errors=True) 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): def test_create_allocation_doesnt_contain_id(self):
with mock.patch.object(self.dbapi, 'create_allocation', 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, response = self.delete('/allocations/%s' % self.allocation.uuid,
expect_errors=True, expect_errors=True,
headers={api_base.Version.string: '1.14'}) 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, def test_delete_allocation_invalid_api_version_without_check(self,
mock_destroy): mock_destroy):
@ -688,7 +688,7 @@ class TestDelete(test_api_base.BaseApiTest):
response = self.delete('/allocations/ba!na!na1', response = self.delete('/allocations/ba!na!na1',
expect_errors=True, expect_errors=True,
headers={api_base.Version.string: '1.14'}) 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): def test_delete_allocation_by_name(self, mock_destroy):
self.delete('/allocations/%s' % self.allocation.name, self.delete('/allocations/%s' % self.allocation.name,

10
ironic/tests/unit/api/controllers/v1/test_deploy_template.py
View File

@ -414,7 +414,7 @@ class TestPatch(BaseDeployTemplatesAPITest):
'op': 'add'}], 'op': 'add'}],
headers=headers, headers=headers,
expect_errors=True) 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) self.assertFalse(mock_save.called)
def test_update_by_name_old_api_version(self, mock_save): def test_update_by_name_old_api_version(self, mock_save):
@ -424,7 +424,7 @@ class TestPatch(BaseDeployTemplatesAPITest):
'value': name, 'value': name,
'op': 'add'}], 'op': 'add'}],
expect_errors=True) 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) self.assertFalse(mock_save.called)
def test_update_not_found(self, mock_save): def test_update_not_found(self, mock_save):
@ -715,7 +715,7 @@ class TestPost(BaseDeployTemplatesAPITest):
response = self.post_json( response = self.post_json(
'/deploy_templates', tdict, headers=self.invalid_version_headers, '/deploy_templates', tdict, headers=self.invalid_version_headers,
expect_errors=True) 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): def test_create_doesnt_contain_id(self):
with mock.patch.object( with mock.patch.object(
@ -944,14 +944,14 @@ class TestDelete(BaseDeployTemplatesAPITest):
response = self.delete('/deploy_templates/%s' % self.template.uuid, response = self.delete('/deploy_templates/%s' % self.template.uuid,
expect_errors=True, expect_errors=True,
headers=self.invalid_version_headers) 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): def test_delete_old_api_version(self, mock_dpt):
# Names like CUSTOM_1 were not valid in API 1.1, but the check should # Names like CUSTOM_1 were not valid in API 1.1, but the check should
# go after the microversion check. # go after the microversion check.
response = self.delete('/deploy_templates/%s' % self.template.name, response = self.delete('/deploy_templates/%s' % self.template.name,
expect_errors=True) 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): def test_delete_by_name_non_existent(self, mock_dpt):
res = self.delete('/deploy_templates/%s' % 'blah', expect_errors=True, res = self.delete('/deploy_templates/%s' % 'blah', expect_errors=True,