diff --git a/manila/policies/share_snapshot.py b/manila/policies/share_snapshot.py index f60dfbb90e..d074e489ab 100644 --- a/manila/policies/share_snapshot.py +++ b/manila/policies/share_snapshot.py @@ -278,11 +278,15 @@ share_snapshot_policies = [ name=BASE_POLICY_NAME % 'list_snapshots_in_deferred_deletion_states', check_str=base.ADMIN, scope_types=['project'], - description="List share snapshots whose deletion has been deferred", + description="List (or get) snapshots whose deletion has been deferred", operations=[ { 'method': 'GET', 'path': '/v2/snapshots', + }, + { + 'method': 'GET', + 'path': '/snapshots/{snapshot_id}' } ], deprecated_rule=deprecated_list_snapshots_in_deferred_deletion_states diff --git a/manila/policies/shares.py b/manila/policies/shares.py index c0895988c3..90bff62399 100644 --- a/manila/policies/shares.py +++ b/manila/policies/shares.py @@ -669,12 +669,16 @@ shares_policies = [ name=BASE_POLICY_NAME % 'list_shares_in_deferred_deletion_states', check_str=base.ADMIN, scope_types=['project'], - description="List shares whose deletion has been deferred", + description="List (or get) shares whose deletion has been deferred", operations=[ { 'method': 'GET', 'path': '/v2/shares', }, + { + 'method': 'GET', + 'path': '/shares/{share_id}', + } ], deprecated_rule=deprecated_list_shares_in_deferred_deletion_states ), diff --git a/manila/share/api.py b/manila/share/api.py index e51f67e28d..3a89a2f771 100644 --- a/manila/share/api.py +++ b/manila/share/api.py @@ -2213,13 +2213,21 @@ class API(base.Base): return self.db.share_snapshot_update(context, snapshot['id'], fields) def get(self, context, share_id): - rv = self.db.share_get(context, share_id) - if not rv['is_public']: + share = self.db.share_get(context, share_id) + if not share['is_public']: authorized = policy.check_policy( - context, 'share', 'get', rv, do_raise=False) + context, 'share', 'get', share, do_raise=False) if not authorized: 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', sort_dir='desc'): @@ -2329,7 +2337,17 @@ class API(base.Base): def get_snapshot(self, context, snapshot_id): 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, offset=None, sort_key='share_id', sort_dir='desc'): diff --git a/manila/tests/share/test_api.py b/manila/tests/share/test_api.py index 3707d3ef33..1df56ec1e8 100644 --- a/manila/tests/share/test_api.py +++ b/manila/tests/share/test_api.py @@ -2675,6 +2675,17 @@ class ShareAPITestCase(test.TestCase): db_api.share_snapshot_get.assert_called_once_with( 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): snapshot = db_utils.create_snapshot( with_share=True, status=constants.STATUS_ERROR) @@ -2899,6 +2910,44 @@ class ShareAPITestCase(test.TestCase): db_api.share_get.assert_called_once_with( 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): share = db_utils.create_share( is_public=False, diff --git a/releasenotes/notes/bug-2067456-handle-share-and-snapshot-show-for-deferred-deletion-37654e034eabccc6.yaml b/releasenotes/notes/bug-2067456-handle-share-and-snapshot-show-for-deferred-deletion-37654e034eabccc6.yaml new file mode 100644 index 0000000000..f880917ba1 --- /dev/null +++ b/releasenotes/notes/bug-2067456-handle-share-and-snapshot-show-for-deferred-deletion-37654e034eabccc6.yaml @@ -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 `_