From 7e006ca39fc57e35244f471c0f0bb6773fdbbc28 Mon Sep 17 00:00:00 2001 From: Andrew Laski Date: Thu, 18 Apr 2013 14:36:26 -0400 Subject: Don't swallow PolicyNotAuthorized for resize/reboot actions Removes 'except Exception' lines in the api which were preventing PolicyNotAuthorized exceptions from bubbling up and returning 403 to the caller. This addresses policy exceptions in confirmResize, revertResize, and reboot. The try block calls into the compute api which sends a cast to perform to bulk of the work, so there is little chance of an exception bubbling up which is not already being caught. Removes a unit test for a condition that should not exist. Bug 1170453 Change-Id: I516a19894ab3d183057c774e84c4faa7053a6463 --- nova/api/openstack/compute/servers.py | 9 ----- .../api/openstack/compute/test_server_actions.py | 12 ------- nova/tests/api/openstack/compute/test_servers.py | 40 ++++++++++++++++++++++ 3 files changed, 40 insertions(+), 21 deletions(-) diff --git a/nova/api/openstack/compute/servers.py b/nova/api/openstack/compute/servers.py index 12efa5eb4..cf200e50a 100644 --- a/nova/api/openstack/compute/servers.py +++ b/nova/api/openstack/compute/servers.py @@ -1031,9 +1031,6 @@ class Controller(wsgi.Controller): except exception.InstanceInvalidState as state_error: common.raise_http_conflict_for_instance_invalid_state(state_error, 'confirmResize') - except Exception, e: - LOG.exception(_("Error in confirm-resize %s"), e) - raise exc.HTTPBadRequest() return exc.HTTPNoContent() @wsgi.response(202) @@ -1054,9 +1051,6 @@ class Controller(wsgi.Controller): except exception.InstanceInvalidState as state_error: common.raise_http_conflict_for_instance_invalid_state(state_error, 'revertResize') - except Exception, e: - LOG.exception(_("Error in revert-resize %s"), e) - raise exc.HTTPBadRequest() return webob.Response(status_int=202) @wsgi.response(202) @@ -1084,9 +1078,6 @@ class Controller(wsgi.Controller): except exception.InstanceInvalidState as state_error: common.raise_http_conflict_for_instance_invalid_state(state_error, 'reboot') - except Exception, e: - LOG.exception(_("Error in reboot %s"), e, instance=instance) - raise exc.HTTPUnprocessableEntity() return webob.Response(status_int=202) def _resize(self, req, instance_id, flavor_id, **kwargs): diff --git a/nova/tests/api/openstack/compute/test_server_actions.py b/nova/tests/api/openstack/compute/test_server_actions.py index 754e103d4..c5d57ecbb 100644 --- a/nova/tests/api/openstack/compute/test_server_actions.py +++ b/nova/tests/api/openstack/compute/test_server_actions.py @@ -195,18 +195,6 @@ class ServerActionsControllerTest(test.TestCase): self.controller._action_reboot, req, FAKE_UUID, body) - def test_reboot_raises_unprocessable_entity(self): - body = dict(reboot=dict(type="HARD")) - - def fake_reboot(*args, **kwargs): - raise NotImplementedError() - - self.stubs.Set(compute_api.API, 'reboot', fake_reboot) - req = fakes.HTTPRequest.blank(self.url) - self.assertRaises(webob.exc.HTTPUnprocessableEntity, - self.controller._action_reboot, - req, FAKE_UUID, body) - def test_rebuild_accepted_minimum(self): return_server = fakes.fake_instance_get(image_ref='2', vm_state=vm_states.ACTIVE, host='fake_host') diff --git a/nova/tests/api/openstack/compute/test_servers.py b/nova/tests/api/openstack/compute/test_servers.py index 5a0c8a901..7748c2e33 100644 --- a/nova/tests/api/openstack/compute/test_servers.py +++ b/nova/tests/api/openstack/compute/test_servers.py @@ -1636,6 +1636,20 @@ class ServerStatusTest(test.TestCase): task_states.REBOOTING_HARD) self.assertEqual(response['server']['status'], 'HARD_REBOOT') + def test_reboot_resize_policy_fail(self): + def fake_get_server(context, req, id): + return fakes.stub_instance(id) + + self.stubs.Set(self.controller, '_get_server', fake_get_server) + + rule = {'compute:reboot': + common_policy.parse_rule('role:admin')} + common_policy.set_rules(common_policy.Rules(rule)) + req = fakes.HTTPRequest.blank('/v2/fake/servers/1234/action') + self.assertRaises(exception.PolicyNotAuthorized, + self.controller._action_reboot, req, '1234', + {'reboot': {'type': 'HARD'}}) + def test_rebuild(self): response = self._get_with_state(vm_states.ACTIVE, task_states.REBUILDING) @@ -1650,6 +1664,19 @@ class ServerStatusTest(test.TestCase): task_states.RESIZE_PREP) self.assertEqual(response['server']['status'], 'RESIZE') + def test_confirm_resize_policy_fail(self): + def fake_get_server(context, req, id): + return fakes.stub_instance(id) + + self.stubs.Set(self.controller, '_get_server', fake_get_server) + + rule = {'compute:confirm_resize': + common_policy.parse_rule('role:admin')} + common_policy.set_rules(common_policy.Rules(rule)) + req = fakes.HTTPRequest.blank('/v2/fake/servers/1234/action') + self.assertRaises(exception.PolicyNotAuthorized, + self.controller._action_confirm_resize, req, '1234', {}) + def test_verify_resize(self): response = self._get_with_state(vm_states.RESIZED, None) self.assertEqual(response['server']['status'], 'VERIFY_RESIZE') @@ -1659,6 +1686,19 @@ class ServerStatusTest(test.TestCase): task_states.RESIZE_REVERTING) self.assertEqual(response['server']['status'], 'REVERT_RESIZE') + def test_revert_resize_policy_fail(self): + def fake_get_server(context, req, id): + return fakes.stub_instance(id) + + self.stubs.Set(self.controller, '_get_server', fake_get_server) + + rule = {'compute:revert_resize': + common_policy.parse_rule('role:admin')} + common_policy.set_rules(common_policy.Rules(rule)) + req = fakes.HTTPRequest.blank('/v2/fake/servers/1234/action') + self.assertRaises(exception.PolicyNotAuthorized, + self.controller._action_revert_resize, req, '1234', {}) + def test_password_update(self): response = self._get_with_state(vm_states.ACTIVE, task_states.UPDATING_PASSWORD) -- cgit