From cc0081fd10220a1e9ee9749d748dc712db33e2b0 Mon Sep 17 00:00:00 2001 From: Valeriy Ponomaryov Date: Tue, 3 Jun 2014 08:23:13 -0400 Subject: [PATCH] Added force-delete action to admin actions force-delete action will be available for shares and snapshots with admin creds. Partially implements: blueprint manila-client-enhancements Change-Id: I896e8a83109879b96f0d7ef2ca2e52fcb7528bb3 --- .../api/share/admin/test_admin_actions.py | 31 +++++ .../admin/test_admin_actions_negative.py | 30 +++- .../services/share/json/shares_client.py | 9 ++ etc/manila/policy.json | 2 + manila/api/contrib/admin_actions.py | 12 ++ manila/share/api.py | 8 +- .../tests/api/contrib/test_admin_actions.py | 129 ++++++++++++------ manila/tests/policy.json | 2 + 8 files changed, 175 insertions(+), 48 deletions(-) diff --git a/contrib/tempest/tempest/api/share/admin/test_admin_actions.py b/contrib/tempest/tempest/api/share/admin/test_admin_actions.py index 376f518663..acf1a20fd1 100644 --- a/contrib/tempest/tempest/api/share/admin/test_admin_actions.py +++ b/contrib/tempest/tempest/api/share/admin/test_admin_actions.py @@ -23,6 +23,7 @@ class AdminActionsTest(base.BaseSharesAdminTest): def setUpClass(cls): super(AdminActionsTest, cls).setUpClass() cls.states = ["error", "available"] + cls.bad_status = "error_deleting" __, cls.sh = cls.create_share() __, cls.sn = cls.create_snapshot_wait_for_active(cls.sh["id"]) @@ -42,3 +43,33 @@ class AdminActionsTest(base.BaseSharesAdminTest): status=status) self.assertIn(int(resp["status"]), test.HTTP_SUCCESS) self.shares_client.wait_for_snapshot_status(self.sn["id"], status) + + @test.attr(type=["gate", ]) + def test_force_delete_share(self): + __, share = self.create_share() + # Change status from 'available' to 'error_deleting' + __, __ = self.shares_client.reset_state(share["id"], + status=self.bad_status) + # Check that status was changed + __, check_status = self.shares_client.get_share(share["id"]) + self.assertEqual(check_status["status"], self.bad_status) + # Share with status 'error_deleting' should be deleted + resp, __ = self.shares_client.force_delete(share["id"]) + self.assertIn(int(resp["status"]), test.HTTP_SUCCESS) + self.shares_client.wait_for_resource_deletion(share_id=share["id"]) + + @test.attr(type=["gate", ]) + def test_force_delete_snapshot(self): + __, sn = self.create_snapshot_wait_for_active(self.sh["id"]) + # Change status from 'available' to 'error_deleting' + __, __ = self.shares_client.reset_state(sn["id"], + s_type="snapshots", + status=self.bad_status) + # Check that status was changed + __, check_status = self.shares_client.get_snapshot(sn["id"]) + self.assertEqual(check_status["status"], self.bad_status) + # Snapshot with status 'error_deleting' should be deleted + resp, __ = self.shares_client.force_delete(sn["id"], + s_type="snapshots") + self.assertIn(int(resp["status"]), test.HTTP_SUCCESS) + self.shares_client.wait_for_resource_deletion(snapshot_id=sn["id"]) diff --git a/contrib/tempest/tempest/api/share/admin/test_admin_actions_negative.py b/contrib/tempest/tempest/api/share/admin/test_admin_actions_negative.py index d7cee584df..a0374a8b7c 100644 --- a/contrib/tempest/tempest/api/share/admin/test_admin_actions_negative.py +++ b/contrib/tempest/tempest/api/share/admin/test_admin_actions_negative.py @@ -32,12 +32,12 @@ class AdminActionsNegativeTest(base.BaseSharesAdminTest): cls.member_shares_client = clients.Manager().shares_client @test.attr(type=["gate", "negative", ]) - def test_reset_unexistant_share_state(self): + def test_reset_nonexistent_share_state(self): self.assertRaises(exceptions.NotFound, self.shares_client.reset_state, "fake") @test.attr(type=["gate", "negative", ]) - def test_reset_unexistant_snapshot_state(self): + def test_reset_nonexistent_snapshot_state(self): self.assertRaises(exceptions.NotFound, self.shares_client.reset_state, "fake", s_type="snapshots") @@ -66,3 +66,29 @@ class AdminActionsNegativeTest(base.BaseSharesAdminTest): self.assertRaises(exceptions.Unauthorized, self.member_shares_client.reset_state, self.sn["id"], s_type="snapshots") + + @test.attr(type=["gate", "negative", ]) + def test_force_delete_nonexistent_share(self): + self.assertRaises(exceptions.NotFound, + self.shares_client.force_delete, "fake") + + @test.attr(type=["gate", "negative", ]) + def test_force_delete_nonexistent_snapshot(self): + self.assertRaises(exceptions.NotFound, + self.shares_client.force_delete, + "fake", + s_type="snapshots") + + @test.attr(type=["gate", "negative", ]) + def test_try_force_delete_share_with_member(self): + # If a non-admin tries to do force_delete, it should be unauthorized + self.assertRaises(exceptions.Unauthorized, + self.member_shares_client.force_delete, + self.sh["id"]) + + @test.attr(type=["gate", "negative", ]) + def test_try_force_delete_snapshot_with_member(self): + # If a non-admin tries to do force_delete, it should be unauthorized + self.assertRaises(exceptions.Unauthorized, + self.member_shares_client.force_delete, + self.sn["id"], s_type="snapshots") diff --git a/contrib/tempest/tempest/services/share/json/shares_client.py b/contrib/tempest/tempest/services/share/json/shares_client.py index 46babc1d11..24ffeb30f4 100644 --- a/contrib/tempest/tempest/services/share/json/shares_client.py +++ b/contrib/tempest/tempest/services/share/json/shares_client.py @@ -359,6 +359,15 @@ class SharesClient(rest_client.RestClient): body = json.dumps(body) return self.post("%s/%s/action" % (s_type, s_id), body) + def force_delete(self, s_id, s_type="shares"): + """ + Force delete share or snapshot + s_type: shares, snapshots + """ + body = {"os-force_delete": None} + body = json.dumps(body) + return self.post("%s/%s/action" % (s_type, s_id), body) + ############### def list_services(self, params=None): diff --git a/etc/manila/policy.json b/etc/manila/policy.json index 8c536d9077..522dbe0b1e 100644 --- a/etc/manila/policy.json +++ b/etc/manila/policy.json @@ -15,7 +15,9 @@ "share_extension:quotas:update_for_user": [["rule:admin_or_projectadmin"]], "share_extension:quota_classes": [], + "share_extension:share_admin_actions:force_delete": [["rule:admin_api"]], "share_extension:share_admin_actions:reset_status": [["rule:admin_api"]], + "share_extension:snapshot_admin_actions:force_delete": [["rule:admin_api"]], "share_extension:snapshot_admin_actions:reset_status": [["rule:admin_api"]], "share_extension:services": [["rule:admin_api"]], diff --git a/manila/api/contrib/admin_actions.py b/manila/api/contrib/admin_actions.py index 888948806d..852e736ff3 100644 --- a/manila/api/contrib/admin_actions.py +++ b/manila/api/contrib/admin_actions.py @@ -85,6 +85,18 @@ class AdminController(wsgi.Controller): raise exc.HTTPNotFound(e) return webob.Response(status_int=202) + @wsgi.action('os-force_delete') + def _force_delete(self, req, id, body): + """Delete a resource, bypassing the check that it must be available.""" + context = req.environ['manila.context'] + self.authorize(context, 'force_delete') + try: + resource = self._get(context, id) + except exception.NotFound as e: + raise exc.HTTPNotFound(e) + self._delete(context, resource, force=True) + return webob.Response(status_int=202) + class ShareAdminController(AdminController): """AdminController for Shares.""" diff --git a/manila/share/api.py b/manila/share/api.py index 66ab4d3a8b..288f92a972 100644 --- a/manila/share/api.py +++ b/manila/share/api.py @@ -174,7 +174,7 @@ class API(base.Base): return share @policy.wrap_check_policy('share') - def delete(self, context, share): + def delete(self, context, share, force=False): """Delete share.""" if context.is_admin and context.project_id != share['project_id']: project_id = share['project_id'] @@ -197,7 +197,7 @@ class API(base.Base): QUOTAS.commit(context, reservations, project_id=project_id) return - if share['status'] not in ["available", "error"]: + if not (force or share['status'] in ["available", "error"]): msg = _("Share status must be available or error") raise exception.InvalidShare(reason=msg) @@ -276,8 +276,8 @@ class API(base.Base): @policy.wrap_check_policy('share') def delete_snapshot(self, context, snapshot, force=False): - if not force and snapshot['status'] not in ["available", "error"]: - msg = _("Share Snapshot status must be available or ") + if not (force or snapshot['status'] in ["available", "error"]): + msg = _("Share Snapshot status must be 'available' or 'error'.") raise exception.InvalidShareSnapshot(reason=msg) self.db.share_snapshot_update(context, snapshot['id'], diff --git a/manila/tests/api/contrib/test_admin_actions.py b/manila/tests/api/contrib/test_admin_actions.py index 98db83c0d7..8de2690e7c 100644 --- a/manila/tests/api/contrib/test_admin_actions.py +++ b/manila/tests/api/contrib/test_admin_actions.py @@ -25,6 +25,7 @@ from manila import exception from manila.openstack.common import jsonutils from manila.share import api as share_api from manila import test +from manila.tests.api.contrib import stubs from manila.tests.api import fakes CONF = cfg.CONF @@ -34,7 +35,7 @@ def app(): # no auth, just let environ['manila.context'] pass through api = fakes.router.APIRouter() mapper = fakes.urlmap.URLMap() - mapper['/v2'] = api + mapper['/v1'] = api return mapper @@ -46,27 +47,27 @@ class AdminActionsTest(test.TestCase): self.flags(rpc_backend='manila.openstack.common.rpc.impl_fake') self.flags(lock_path=self.tempdir) self.share_api = share_api.API() + self.admin_context = context.RequestContext('admin', 'fake', True) + self.member_context = context.RequestContext('fake', 'fake') def tearDown(self): shutil.rmtree(self.tempdir) super(AdminActionsTest, self).tearDown() def test_reset_status_as_admin(self): - # admin context - ctx = context.RequestContext('admin', 'fake', True) # current status is available - share = db.share_create(ctx, {'status': 'available'}) - req = webob.Request.blank('/v2/fake/shares/%s/action' % share['id']) + share = db.share_create(self.admin_context, {'status': 'available'}) + req = webob.Request.blank('/v1/fake/shares/%s/action' % share['id']) req.method = 'POST' req.headers['content-type'] = 'application/json' # request status of 'error' req.body = jsonutils.dumps({'os-reset_status': {'status': 'error'}}) # attach admin context to request - req.environ['manila.context'] = ctx + req.environ['manila.context'] = self.admin_context resp = req.get_response(app()) # request is accepted self.assertEqual(resp.status_int, 202) - share = db.share_get(ctx, share['id']) + share = db.share_get(self.admin_context, share['id']) # status changed to 'error' self.assertEqual(share['status'], 'error') @@ -74,14 +75,14 @@ class AdminActionsTest(test.TestCase): # current status is 'error' share = db.share_create(context.get_admin_context(), {'status': 'error'}) - req = webob.Request.blank('/v2/fake/shares/%s/action' % share['id']) + req = webob.Request.blank('/v1/fake/shares/%s/action' % share['id']) req.method = 'POST' req.headers['content-type'] = 'application/json' # request changing status to available req.body = jsonutils.dumps({'os-reset_status': {'status': 'available'}}) # non-admin context - req.environ['manila.context'] = context.RequestContext('fake', 'fake') + req.environ['manila.context'] = self.member_context resp = req.get_response(app()) # request is not authorized self.assertEqual(resp.status_int, 403) @@ -90,48 +91,42 @@ class AdminActionsTest(test.TestCase): self.assertEqual(share['status'], 'error') def test_malformed_reset_status_body(self): - # admin context - ctx = context.RequestContext('admin', 'fake', True) # current status is available - share = db.share_create(ctx, {'status': 'available'}) - req = webob.Request.blank('/v2/fake/shares/%s/action' % share['id']) + share = db.share_create(self.admin_context, {'status': 'available'}) + req = webob.Request.blank('/v1/fake/shares/%s/action' % share['id']) req.method = 'POST' req.headers['content-type'] = 'application/json' # malformed request body req.body = jsonutils.dumps({'os-reset_status': {'x-status': 'bad'}}) # attach admin context to request - req.environ['manila.context'] = ctx + req.environ['manila.context'] = self.admin_context resp = req.get_response(app()) # bad request self.assertEqual(resp.status_int, 400) - share = db.share_get(ctx, share['id']) + share = db.share_get(self.admin_context, share['id']) # status is still 'available' self.assertEqual(share['status'], 'available') def test_invalid_status_for_share(self): - # admin context - ctx = context.RequestContext('admin', 'fake', True) # current status is available - share = db.share_create(ctx, {'status': 'available'}) - req = webob.Request.blank('/v2/fake/shares/%s/action' % share['id']) + share = db.share_create(self.admin_context, {'status': 'available'}) + req = webob.Request.blank('/v1/fake/shares/%s/action' % share['id']) req.method = 'POST' req.headers['content-type'] = 'application/json' # 'invalid' is not a valid status req.body = jsonutils.dumps({'os-reset_status': {'status': 'invalid'}}) # attach admin context to request - req.environ['manila.context'] = ctx + req.environ['manila.context'] = self.admin_context resp = req.get_response(app()) # bad request self.assertEqual(resp.status_int, 400) - share = db.share_get(ctx, share['id']) + share = db.share_get(self.admin_context, share['id']) # status is still 'available' self.assertEqual(share['status'], 'available') def test_reset_status_for_missing_share(self): - # admin context - ctx = context.RequestContext('admin', 'fake', True) # missing-share-id - req = webob.Request.blank('/v2/fake/shares/%s/action' % + req = webob.Request.blank('/v1/fake/shares/%s/action' % 'missing-share-id') req.method = 'POST' req.headers['content-type'] = 'application/json' @@ -139,43 +134,41 @@ class AdminActionsTest(test.TestCase): req.body = jsonutils.dumps({'os-reset_status': {'status': 'available'}}) # attach admin context to request - req.environ['manila.context'] = ctx + req.environ['manila.context'] = self.admin_context resp = req.get_response(app()) # not found self.assertEqual(resp.status_int, 404) - self.assertRaises(exception.NotFound, db.share_get, ctx, + self.assertRaises(exception.NotFound, + db.share_get, + self.admin_context, 'missing-share-id') def test_snapshot_reset_status(self): - # admin context - ctx = context.RequestContext('admin', 'fake', True) # snapshot in 'error_deleting' - share = db.share_create(ctx, {}) - snapshot = db.share_snapshot_create(ctx, {'status': 'error_deleting', - 'share_id': share['id']}) - req = webob.Request.blank('/v2/fake/snapshots/%s/action' % + share = db.share_create(self.admin_context, {}) + snapshot = db.share_snapshot_create(self.admin_context, + {'status': 'error_deleting', 'share_id': share['id']}) + req = webob.Request.blank('/v1/fake/snapshots/%s/action' % snapshot['id']) req.method = 'POST' req.headers['content-type'] = 'application/json' # request status of 'error' req.body = jsonutils.dumps({'os-reset_status': {'status': 'error'}}) # attach admin context to request - req.environ['manila.context'] = ctx + req.environ['manila.context'] = self.admin_context resp = req.get_response(app()) # request is accepted self.assertEqual(resp.status_int, 202) - snapshot = db.share_snapshot_get(ctx, snapshot['id']) + snapshot = db.share_snapshot_get(self.admin_context, snapshot['id']) # status changed to 'error' self.assertEqual(snapshot['status'], 'error') def test_invalid_status_for_snapshot(self): - # admin context - ctx = context.RequestContext('admin', 'fake', True) # snapshot in 'available' - share = db.share_create(ctx, {}) - snapshot = db.share_snapshot_create(ctx, {'status': 'available', - 'share_id': share['id']}) - req = webob.Request.blank('/v2/fake/snapshots/%s/action' % + share = db.share_create(self.admin_context, {}) + snapshot = db.share_snapshot_create(self.admin_context, + {'status': 'available', 'share_id': share['id']}) + req = webob.Request.blank('/v1/fake/snapshots/%s/action' % snapshot['id']) req.method = 'POST' req.headers['content-type'] = 'application/json' @@ -183,10 +176,62 @@ class AdminActionsTest(test.TestCase): req.body = jsonutils.dumps({'os-reset_status': {'status': 'attaching'}}) # attach admin context to request - req.environ['manila.context'] = ctx + req.environ['manila.context'] = self.admin_context resp = req.get_response(app()) # request is accepted self.assertEqual(resp.status_int, 400) - snapshot = db.share_snapshot_get(ctx, snapshot['id']) + snapshot = db.share_snapshot_get(self.admin_context, snapshot['id']) # status is still 'available' self.assertEqual(snapshot['status'], 'available') + + def test_admin_force_delete_share(self): + share = db.share_create(self.admin_context, {'size': 1}) + req = webob.Request.blank('/v1/fake/shares/%s/action' % share['id']) + req.method = 'POST' + req.headers['content-type'] = 'application/json' + req.body = jsonutils.dumps({'os-force_delete': {}}) + req.environ['manila.context'] = self.admin_context + resp = req.get_response(app()) + self.assertEqual(resp.status_int, 202) + self.assertRaises(exception.NotFound, + db.share_get, + self.admin_context, + share['id']) + + def test_member_force_delete_share(self): + share = db.share_create(self.admin_context, {'size': 1}) + req = webob.Request.blank('/v1/fake/shares/%s/action' % share['id']) + req.method = 'POST' + req.headers['content-type'] = 'application/json' + req.body = jsonutils.dumps({'os-force_delete': {}}) + req.environ['manila.context'] = self.member_context + resp = req.get_response(app()) + self.assertEqual(resp.status_int, 403) + + def test_admin_force_delete_snapshot(self): + snapshot = stubs.stub_snapshot(1, host='foo') + self.stubs.Set(db, 'share_get', lambda x, y: snapshot) + self.stubs.Set(db, 'share_snapshot_get', lambda x, y: snapshot) + self.stubs.Set(share_api.API, 'delete_snapshot', lambda *x, **y: True) + path = '/v1/fake/snapshots/%s/action' % snapshot['id'] + req = webob.Request.blank(path) + req.method = 'POST' + req.headers['content-type'] = 'application/json' + req.body = jsonutils.dumps({'os-force_delete': {}}) + req.environ['manila.context'] = self.admin_context + resp = req.get_response(app()) + self.assertEqual(resp.status_int, 202) + + def test_member_force_delete_snapshot(self): + snapshot = stubs.stub_snapshot(1, host='foo') + self.stubs.Set(db, 'share_get', lambda x, y: snapshot) + self.stubs.Set(db, 'share_snapshot_get', lambda x, y: snapshot) + self.stubs.Set(share_api.API, 'delete_snapshot', lambda *x, **y: True) + path = '/v1/fake/snapshots/%s/action' % snapshot['id'] + req = webob.Request.blank(path) + req.method = 'POST' + req.headers['content-type'] = 'application/json' + req.body = jsonutils.dumps({'os-force_delete': {}}) + req.environ['manila.context'] = self.member_context + resp = req.get_response(app()) + self.assertEqual(resp.status_int, 403) diff --git a/manila/tests/policy.json b/manila/tests/policy.json index ab526b9550..774f0962f3 100644 --- a/manila/tests/policy.json +++ b/manila/tests/policy.json @@ -20,7 +20,9 @@ "share:get_share_metadata": [], "share:delete_share_metadata": [], "share:update_share_metadata": [], + "share_extension:share_admin_actions:force_delete": [["rule:admin_api"]], "share_extension:share_admin_actions:reset_status": [["rule:admin_api"]], + "share_extension:snapshot_admin_actions:force_delete": [["rule:admin_api"]], "share_extension:snapshot_admin_actions:reset_status": [["rule:admin_api"]], "share_extension:types_manage": [], "share_extension:types_extra_specs": [],