openstack/nova
Code Issues Proposed changes

Add regression test for Cinder 403 forwarding

Browse Source 
The Nova API should be returning a 403 whenever it gets a Forbidden from
Cinder. Nova currently only returns a 403 when it runs into a policy
exception from its own policy (which is a subclass of Forbidden). This
changes the handler over to return a 403 when any Forbidden comes in.

Also, the volume helper methods in the function API client had incorrect
URLs, so those were also fixed (/volumes --> /os-volumes)

Change-Id: If9dd002454338d07fcea0c2092c12f3961c3dcdf
Partial-Bug: #1554631
Closes-Bug: #1555826
This commit is contained in:
Ryan Rossiter
2016-03-10 22:07:09 +00:00
parent 859ff4893f
commit 50f4f95ae4
3 changed files with 53 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
nova/api/openstack/extensions.py
View File

@@ -484,9 +484,9 @@ def expected_errors(errors):
t_errors = errors
if exc.code in t_errors:
raise
elif isinstance(exc, exception.PolicyNotAuthorized):
elif isinstance(exc, exception.Forbidden):
# Note(cyeoh): Special case to handle
# PolicyNotAuthorized exceptions so every
# Forbidden exceptions so every
# extension method does not need to wrap authorize
# calls. ResourceExceptionHandler silently
# converts NotAuthorized to HTTPForbidden

8
nova/tests/functional/api/client.py
View File

@@ -305,17 +305,17 @@ class TestOpenStackClient(object):
flavor_id, spec)
def get_volume(self, volume_id):
return self.api_get('/volumes/%s' % volume_id).body['volume']
return self.api_get('/os-volumes/%s' % volume_id).body['volume']
def get_volumes(self, detail=True):
rel_url = '/volumes/detail' if detail else '/volumes'
rel_url = '/os-volumes/detail' if detail else '/os-volumes'
return self.api_get(rel_url).body['volumes']
def post_volume(self, volume):
return self.api_post('/volumes', volume).body['volume']
return self.api_post('/os-volumes', volume).body['volume']
def delete_volume(self, volume_id):
return self.api_delete('/volumes/%s' % volume_id)
return self.api_delete('/os-volumes/%s' % volume_id)
def get_server_volume(self, server_id, attachment_id):
return self.api_get('/servers/%s/os-volume_attachments/%s' %

47
nova/tests/functional/regressions/test_bug_1554631.py Normal file
View File

@@ -0,0 +1,47 @@
# Copyright 2016 IBM Corp.
#
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
# a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
from cinderclient import exceptions as cinder_exceptions
import mock
from nova import test
from nova.tests import fixtures as nova_fixtures
from nova.tests.functional.api import client
from nova.tests.unit import policy_fixture
class TestCinderForbidden(test.TestCase):
def setUp(self):
super(TestCinderForbidden, self).setUp()
self.useFixture(policy_fixture.RealPolicyFixture())
api_fixture = self.useFixture(nova_fixtures.OSAPIFixture(
api_version='v2.1'))
self.api = api_fixture.api
@mock.patch('nova.volume.cinder.cinderclient')
def test_forbidden_cinder_operation_returns_403(self, mock_cinder):
"""Regression test for bug #1554631.
When the Cinder client returns a 403 Forbidden on any operation,
the Nova API should forward on the 403 instead of returning 500.
"""
cinder_client = mock.Mock()
mock_cinder.return_value = cinder_client
exc = cinder_exceptions.Forbidden('')
cinder_client.volumes.create.side_effect = exc
volume = {'display_name': 'vol1', 'size': 3}
e = self.assertRaises(client.OpenStackApiException,
self.api.post_volume, {'volume': volume})
self.assertEqual(403, e.response.status_code)