Fix share/snapshot show for deferred deletion
Deferred deletion states deferred_deleting, error_deferred_deleting are
visible for non-admin user in 'share/snapshot show' command. Fixed this.
partially-implements: bp/deferred-deletion
Closes-bug: #2067456
Change-Id: I42ddda3144a3f52b4bc0420d5acde1e5e7560264
(cherry picked from commit be0c8561c9
)
This commit is contained in:
parent
6095b13f21
commit
983189916a
@ -278,11 +278,15 @@ share_snapshot_policies = [
|
|||||||
name=BASE_POLICY_NAME % 'list_snapshots_in_deferred_deletion_states',
|
name=BASE_POLICY_NAME % 'list_snapshots_in_deferred_deletion_states',
|
||||||
check_str=base.ADMIN,
|
check_str=base.ADMIN,
|
||||||
scope_types=['project'],
|
scope_types=['project'],
|
||||||
description="List share snapshots whose deletion has been deferred",
|
description="List (or get) snapshots whose deletion has been deferred",
|
||||||
operations=[
|
operations=[
|
||||||
{
|
{
|
||||||
'method': 'GET',
|
'method': 'GET',
|
||||||
'path': '/v2/snapshots',
|
'path': '/v2/snapshots',
|
||||||
|
},
|
||||||
|
{
|
||||||
|
'method': 'GET',
|
||||||
|
'path': '/snapshots/{snapshot_id}'
|
||||||
}
|
}
|
||||||
],
|
],
|
||||||
deprecated_rule=deprecated_list_snapshots_in_deferred_deletion_states
|
deprecated_rule=deprecated_list_snapshots_in_deferred_deletion_states
|
||||||
|
@ -669,12 +669,16 @@ shares_policies = [
|
|||||||
name=BASE_POLICY_NAME % 'list_shares_in_deferred_deletion_states',
|
name=BASE_POLICY_NAME % 'list_shares_in_deferred_deletion_states',
|
||||||
check_str=base.ADMIN,
|
check_str=base.ADMIN,
|
||||||
scope_types=['project'],
|
scope_types=['project'],
|
||||||
description="List shares whose deletion has been deferred",
|
description="List (or get) shares whose deletion has been deferred",
|
||||||
operations=[
|
operations=[
|
||||||
{
|
{
|
||||||
'method': 'GET',
|
'method': 'GET',
|
||||||
'path': '/v2/shares',
|
'path': '/v2/shares',
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
'method': 'GET',
|
||||||
|
'path': '/shares/{share_id}',
|
||||||
|
}
|
||||||
],
|
],
|
||||||
deprecated_rule=deprecated_list_shares_in_deferred_deletion_states
|
deprecated_rule=deprecated_list_shares_in_deferred_deletion_states
|
||||||
),
|
),
|
||||||
|
@ -2213,13 +2213,21 @@ class API(base.Base):
|
|||||||
return self.db.share_snapshot_update(context, snapshot['id'], fields)
|
return self.db.share_snapshot_update(context, snapshot['id'], fields)
|
||||||
|
|
||||||
def get(self, context, share_id):
|
def get(self, context, share_id):
|
||||||
rv = self.db.share_get(context, share_id)
|
share = self.db.share_get(context, share_id)
|
||||||
if not rv['is_public']:
|
if not share['is_public']:
|
||||||
authorized = policy.check_policy(
|
authorized = policy.check_policy(
|
||||||
context, 'share', 'get', rv, do_raise=False)
|
context, 'share', 'get', share, do_raise=False)
|
||||||
if not authorized:
|
if not authorized:
|
||||||
raise exception.NotFound()
|
raise exception.NotFound()
|
||||||
return rv
|
if share['status'] in (
|
||||||
|
constants.STATUS_DEFERRED_DELETING,
|
||||||
|
constants.STATUS_ERROR_DEFERRED_DELETING):
|
||||||
|
policy_str = "list_shares_in_deferred_deletion_states"
|
||||||
|
authorized = policy.check_policy(
|
||||||
|
context, 'share', policy_str, share, do_raise=False)
|
||||||
|
if not authorized:
|
||||||
|
raise exception.NotFound()
|
||||||
|
return share
|
||||||
|
|
||||||
def get_all(self, context, search_opts=None, sort_key='created_at',
|
def get_all(self, context, search_opts=None, sort_key='created_at',
|
||||||
sort_dir='desc'):
|
sort_dir='desc'):
|
||||||
@ -2329,7 +2337,17 @@ class API(base.Base):
|
|||||||
|
|
||||||
def get_snapshot(self, context, snapshot_id):
|
def get_snapshot(self, context, snapshot_id):
|
||||||
policy.check_policy(context, 'share_snapshot', 'get_snapshot')
|
policy.check_policy(context, 'share_snapshot', 'get_snapshot')
|
||||||
return self.db.share_snapshot_get(context, snapshot_id)
|
snapshot = self.db.share_snapshot_get(context, snapshot_id)
|
||||||
|
if snapshot.get('status') in (
|
||||||
|
constants.STATUS_DEFERRED_DELETING,
|
||||||
|
constants.STATUS_ERROR_DEFERRED_DELETING):
|
||||||
|
policy_str = "list_snapshots_in_deferred_deletion_states"
|
||||||
|
authorized = policy.check_policy(
|
||||||
|
context, 'share_snapshot', policy_str,
|
||||||
|
snapshot, do_raise=False)
|
||||||
|
if not authorized:
|
||||||
|
raise exception.NotFound()
|
||||||
|
return snapshot
|
||||||
|
|
||||||
def get_all_snapshots(self, context, search_opts=None, limit=None,
|
def get_all_snapshots(self, context, search_opts=None, limit=None,
|
||||||
offset=None, sort_key='share_id', sort_dir='desc'):
|
offset=None, sort_key='share_id', sort_dir='desc'):
|
||||||
|
@ -2675,6 +2675,17 @@ class ShareAPITestCase(test.TestCase):
|
|||||||
db_api.share_snapshot_get.assert_called_once_with(
|
db_api.share_snapshot_get.assert_called_once_with(
|
||||||
self.context, 'fakeid')
|
self.context, 'fakeid')
|
||||||
|
|
||||||
|
def test_get_snapshot_non_admin_deferred_state(self):
|
||||||
|
fake_get_snap = {
|
||||||
|
'fake_key': 'fake_val', 'status': 'deferred_deleting'
|
||||||
|
}
|
||||||
|
with mock.patch.object(db_api, 'share_snapshot_get',
|
||||||
|
mock.Mock(return_value=fake_get_snap)):
|
||||||
|
self.mock_object(
|
||||||
|
policy, 'check_policy', mock.Mock(side_effect=[True, False]))
|
||||||
|
self.assertRaises(exception.NotFound, self.api.get_snapshot,
|
||||||
|
self.context, 'fakeid')
|
||||||
|
|
||||||
def test_create_from_snapshot_not_available(self):
|
def test_create_from_snapshot_not_available(self):
|
||||||
snapshot = db_utils.create_snapshot(
|
snapshot = db_utils.create_snapshot(
|
||||||
with_share=True, status=constants.STATUS_ERROR)
|
with_share=True, status=constants.STATUS_ERROR)
|
||||||
@ -2899,6 +2910,44 @@ class ShareAPITestCase(test.TestCase):
|
|||||||
db_api.share_get.assert_called_once_with(
|
db_api.share_get.assert_called_once_with(
|
||||||
self.context, 'fakeid')
|
self.context, 'fakeid')
|
||||||
|
|
||||||
|
def test_get_admin_deferred_state(self):
|
||||||
|
rv = {
|
||||||
|
'id': 'fake_id',
|
||||||
|
'is_public': False,
|
||||||
|
'name': 'bar',
|
||||||
|
'status': constants.STATUS_ERROR_DEFERRED_DELETING,
|
||||||
|
'project_id': 'fake_pid_2',
|
||||||
|
'share_server_id': 'fake_server_3',
|
||||||
|
}
|
||||||
|
|
||||||
|
self.mock_object(db_api, 'share_get',
|
||||||
|
mock.Mock(return_value=rv))
|
||||||
|
ctx = context.RequestContext('fake_uid', 'fake_pid_1', is_admin=True)
|
||||||
|
self.mock_object(
|
||||||
|
policy, 'check_policy', mock.Mock(side_effect=[True, True]))
|
||||||
|
share = self.api.get(ctx, 'fake_id')
|
||||||
|
self.assertEqual(rv, share)
|
||||||
|
|
||||||
|
def test_get_non_admin_deferred_state(self):
|
||||||
|
rv = {
|
||||||
|
'id': 'fake_id',
|
||||||
|
'is_public': False,
|
||||||
|
'name': 'bar',
|
||||||
|
'status': constants.STATUS_ERROR_DEFERRED_DELETING,
|
||||||
|
'project_id': 'fake_pid_2',
|
||||||
|
'share_server_id': 'fake_server_3',
|
||||||
|
}
|
||||||
|
|
||||||
|
self.mock_object(db_api, 'share_get',
|
||||||
|
mock.Mock(return_value=rv))
|
||||||
|
ctx = context.RequestContext('fake_uid', 'fake_pid_1', is_admin=False)
|
||||||
|
self.mock_object(
|
||||||
|
policy, 'check_policy', mock.Mock(side_effect=[True, False]))
|
||||||
|
|
||||||
|
self.assertRaises(
|
||||||
|
exception.NotFound,
|
||||||
|
self.api.get, ctx, 'fake_id')
|
||||||
|
|
||||||
def test_get_not_authorized(self):
|
def test_get_not_authorized(self):
|
||||||
share = db_utils.create_share(
|
share = db_utils.create_share(
|
||||||
is_public=False,
|
is_public=False,
|
||||||
|
@ -0,0 +1,6 @@
|
|||||||
|
---
|
||||||
|
fixes:
|
||||||
|
- |
|
||||||
|
Share and snapshot in deferred deletion states will be hidden from non-admin
|
||||||
|
user in show API along-with existing list API. For more details, please check
|
||||||
|
Launchpad `bug 2067456 <https://bugs.launchpad.net/manila/+bug/2067456>`_
|
Loading…
Reference in New Issue
Block a user