Skip to content

Commit

Permalink
Disallow non-admin users to force delete containers
Browse files Browse the repository at this point in the history
This patch introduces a separated policy rule for "--force",
to avoid non-admin users to force delete containers.

Change-Id: Ieaa419acb3e6888003ab2ab4b8c2695e34e3686c
Closes-Bug #1713522
  • Loading branch information
miaohb committed Aug 31, 2017
1 parent e2c5161 commit b6a8b36
Show file tree
Hide file tree
Showing 7 changed files with 53 additions and 5 deletions.
1 change: 1 addition & 0 deletions etc/zun/policy.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
"container:create": "rule:default",
"container:delete": "rule:default",
"container:delete_all_tenants": "rule:admin_api",
"container:delete_force": "rule:admin_api",
"container:get": "rule:default",
"container:get_one_all_tenants": "rule:admin_api",
"container:get_all": "rule:default",
Expand Down
32 changes: 32 additions & 0 deletions zun/api/controllers/v1/containers.py
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,7 @@ def rename(self, container_id, name):
container.save(context)
return view.format_container(pecan.request.host_url, container)

@base.Controller.api_version("1.1", "1.6")
@pecan.expose('json')
@exception.wrap_pecan_controller_exception
@validation.validate_query_param(pecan.request, schema.query_param_delete)
Expand Down Expand Up @@ -462,6 +463,37 @@ def delete(self, container_id, force=False, **kwargs):
compute_api.container_delete(context, container, force)
pecan.response.status = 204

@base.Controller.api_version("1.7") # noqa
@pecan.expose('json')
@exception.wrap_pecan_controller_exception
@validation.validate_query_param(pecan.request, schema.query_param_delete)
def delete(self, container_id, force=False, **kwargs):
"""Delete a container.
:param container_ident: UUID or Name of a container.
"""
context = pecan.request.context
if is_all_tenants(kwargs):
policy.enforce(context, "container:delete_all_tenants",
action="container:delete_all_tenants")
context.all_tenants = True
container = _get_container(container_id)
check_policy_on_container(container.as_dict(), "container:delete")
try:
force = strutils.bool_from_string(force, strict=True)
except ValueError:
msg = _('Valid force values are true, false, 0, 1, yes and no')
raise exception.InvalidValue(msg)
if not force:
utils.validate_container_state(container, 'delete')
else:
utils.validate_container_state(container, 'delete_force')
policy.enforce(context, "container:delete_force",
action="container:delete_force")
compute_api = pecan.request.compute_api
compute_api.container_delete(context, container, force)
pecan.response.status = 204

@pecan.expose('json')
@exception.wrap_pecan_controller_exception
def start(self, container_id, **kwargs):
Expand Down
3 changes: 2 additions & 1 deletion zun/api/controllers/versions.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,11 @@
* 1.4 - Support list all container host and show a container host
* 1.5 - Add runtime to container
* 1.6 - Support detach network from a container
* 1.7 - Disallow non-admin users to force delete containers
"""

BASE_VER = '1.1'
CURRENT_MAX_VER = '1.6'
CURRENT_MAX_VER = '1.7'


class Version(object):
Expand Down
6 changes: 6 additions & 0 deletions zun/api/rest_api_version_history.rst
Original file line number Diff line number Diff line change
Expand Up @@ -64,3 +64,9 @@ user documentation.

Add detach a network from a container api.
Users can use this api to detach a neutron network from a container.

1.7
---

Disallow non-admin users to force delete containers
Only Admin User can use "delete --force" to force delete a container.
1 change: 1 addition & 0 deletions zun/tests/fake_policy.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
"container:create": "",
"container:delete": "",
"container:delete_all_tenants": "",
"container:delete_force": "",
"container:get_one": "",
"container:get_one_all_tenants": "",
"container:get_all": "",
Expand Down
6 changes: 3 additions & 3 deletions zun/tests/unit/api/controllers/test_root.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
from zun.api import app
from zun.tests.unit.api import base as api_base

CURRENT_VERSION = "container 1.6"
CURRENT_VERSION = "container 1.7"


class TestRootController(api_base.FunctionalTest):
Expand All @@ -27,15 +27,15 @@ def setUp(self):
'default_version':
{'id': 'v1',
'links': [{'href': 'http://localhost/v1/', 'rel': 'self'}],
'max_version': '1.6',
'max_version': '1.7',
'min_version': '1.1',
'status': 'CURRENT'},
'description': 'Zun is an OpenStack project which '
'aims to provide container management.',
'versions': [{'id': 'v1',
'links': [{'href': 'http://localhost/v1/',
'rel': 'self'}],
'max_version': '1.6',
'max_version': '1.7',
'min_version': '1.1',
'status': 'CURRENT'}]}

Expand Down
9 changes: 8 additions & 1 deletion zun/tests/unit/api/controllers/v1/test_containers.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
from zun.tests.unit.db import utils
from zun.tests.unit.objects import utils as obj_utils

CURRENT_VERSION = "container 1.6"
CURRENT_VERSION = "container 1.7"


class TestContainerController(api_base.FunctionalTest):
Expand Down Expand Up @@ -1831,6 +1831,13 @@ def test_policy_disallow_delete_all_tenants(self):
'/v1/containers/%s/?all_tenants=1' % container.uuid,
expect_errors=True)

def test_policy_disallow_delete_force(self):
container = obj_utils.create_test_container(self.context)
self._common_policy_check(
'container:delete_force', self.app.delete,
'/v1/containers/%s/?force=True' % container.uuid,
expect_errors=True)

def _owner_check(self, rule, func, *args, **kwargs):
self.policy.set_rules({rule: "user_id:%(user_id)s"})
headers = {'OpenStack-API-Version': CURRENT_VERSION}
Expand Down

0 comments on commit b6a8b36

Please sign in to comment.