openstack
/
keystone
Code Issues Proposed changes

Merge "add circular check when updating region"

This commit is contained in:
Jenkins 2015-02-03 07:28:11 +00:00 committed by Gerrit Code Review
parent 6c0b08a09e 35a2834aec
commit 332a9bf3a8
5 changed files with 122 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
keystone/catalog/backends/kvs.py
View File

@ -25,7 +25,7 @@ class Catalog(kvs.Base, catalog.Driver):
# region crud
def _delete_child_regions(self, region_id):
def _delete_child_regions(self, region_id, root_region_id):
"""Delete all child regions.
Recursively delete any region that has the supplied region
@ -34,8 +34,11 @@ class Catalog(kvs.Base, catalog.Driver):
children = [r for r in self.list_regions(driver_hints.Hints())
if r['parent_region_id'] == region_id]
for child in children:
self._delete_child_regions(child['id'])
self.delete_region(child['id'])
if child['id'] == root_region_id:
# Hit a circular region hierarchy
return
self._delete_child_regions(child['id'], root_region_id)
self._delete_region(child['id'])
def _check_parent_region(self, region_ref):
"""Raise a NotFound if the parent region does not exist.
@ -69,16 +72,20 @@ class Catalog(kvs.Base, catalog.Driver):
self._check_parent_region(region)
old_region = self.get_region(region_id)
old_region.update(region)
self._ensure_no_circle_in_hierarchical_regions(old_region)
self.db.set('region-%s' % region_id, old_region)
return old_region
def delete_region(self, region_id):
self._delete_child_regions(region_id)
def _delete_region(self, region_id):
self.db.delete('region-%s' % region_id)
region_list = set(self.db.get('region_list', []))
region_list.remove(region_id)
self.db.set('region_list', list(region_list))
def delete_region(self, region_id):
self._delete_child_regions(region_id, region_id)
self._delete_region(region_id)
# service crud
def create_service(self, service_id, service):

19
keystone/catalog/backends/sql.py
View File

@ -99,7 +99,7 @@ class Catalog(catalog.Driver):
raise exception.RegionNotFound(region_id=region_id)
return ref
def _delete_child_regions(self, session, region_id):
def _delete_child_regions(self, session, region_id, root_region_id):
"""Delete all child regions.
Recursively delete any region that has the supplied region
@ -107,7 +107,10 @@ class Catalog(catalog.Driver):
"""
children = session.query(Region).filter_by(parent_region_id=region_id)
for child in children:
self._delete_child_regions(session, child.id)
if child.id == root_region_id:
# Hit a circular region hierarchy
return
self._delete_child_regions(session, child.id, root_region_id)
session.delete(child)
def _check_parent_region(self, session, region_ref):
@ -122,14 +125,17 @@ class Catalog(catalog.Driver):
# which is the behavior we want.
self._get_region(session, parent_region_id)
def _has_endpoints(self, session, region):
def _has_endpoints(self, session, region, root_region):
if region.endpoints is not None and len(region.endpoints) > 0:
return True
q = session.query(Region)
q = q.filter_by(parent_region_id=region.id)
for child in q.all():
if self._has_endpoints(session, child):
if child.id == root_region.id:
# Hit a circular region hierarchy
return False
if self._has_endpoints(session, child, root_region):
return True
return False
@ -141,9 +147,9 @@ class Catalog(catalog.Driver):
session = sql.get_session()
with session.begin():
ref = self._get_region(session, region_id)
if self._has_endpoints(session, ref):
if self._has_endpoints(session, ref, ref):
raise exception.RegionDeletionError(region_id=region_id)
self._delete_child_regions(session, region_id)
self._delete_child_regions(session, region_id, region_id)
session.delete(ref)
@sql.handle_conflicts(conflict_type='region')
@ -162,6 +168,7 @@ class Catalog(catalog.Driver):
ref = self._get_region(session, region_id)
old_dict = ref.to_dict()
old_dict.update(region_ref)
self._ensure_no_circle_in_hierarchical_regions(old_dict)
new_region = Region.from_dict(old_dict)
for attr in Region.attributes:
if attr != 'id':

16
keystone/catalog/core.py
View File

@ -231,6 +231,22 @@ class Driver(object):
def _get_list_limit(self):
return CONF.catalog.list_limit or CONF.list_limit
def _ensure_no_circle_in_hierarchical_regions(self, region_ref):
if region_ref.get('parent_region_id') is None:
return
root_region_id = region_ref['id']
parent_region_id = region_ref['parent_region_id']
while parent_region_id:
# NOTE(wanghong): check before getting parent region can ensure no
# self circle
if parent_region_id == root_region_id:
raise exception.CircularRegionHierarchyError(
parent_region_id=parent_region_id)
parent_region = self.get_region(parent_region_id)
parent_region_id = parent_region.get('parent_region_id')
@abc.abstractmethod
def create_region(self, region_ref):
"""Creates a new region.

7
keystone/exception.py
View File

@ -115,6 +115,13 @@ class ValidationSizeError(Error):
title = 'Bad Request'
class CircularRegionHierarchyError(Error):
message_format = _("The specified parent region %(parent_region_id)s "
"would create a circular region hierarchy.")
code = 400
title = 'Bad Request'
class PasswordVerificationError(Error):
message_format = _("The password length must be less than or equal "
"to %(size)i. The server could not comply with the "

74
keystone/tests/test_backend.py
View File

@ -23,6 +23,7 @@ from oslo_utils import timeutils
import six
from testtools import matchers
from keystone.catalog import core
from keystone.common import driver_hints
from keystone import config
from keystone import exception
@ -4515,6 +4516,79 @@ class CatalogTests(object):
self.catalog_api.create_region,
new_region)
def test_avoid_creating_circular_references_in_regions_update(self):
region_one = self._create_region_with_parent_id()
# self circle: region_one->region_one
self.assertRaises(exception.CircularRegionHierarchyError,
self.catalog_api.update_region,
region_one['id'],
{'parent_region_id': region_one['id']})
# region_one->region_two->region_one
region_two = self._create_region_with_parent_id(region_one['id'])
self.assertRaises(exception.CircularRegionHierarchyError,
self.catalog_api.update_region,
region_one['id'],
{'parent_region_id': region_two['id']})
# region_one region_two->region_three->region_four->region_two
region_three = self._create_region_with_parent_id(region_two['id'])
region_four = self._create_region_with_parent_id(region_three['id'])
self.assertRaises(exception.CircularRegionHierarchyError,
self.catalog_api.update_region,
region_two['id'],
{'parent_region_id': region_four['id']})
@mock.patch.object(core.Driver,
"_ensure_no_circle_in_hierarchical_regions")
def test_circular_regions_can_be_deleted(self, mock_ensure_on_circle):
# turn off the enforcement so that cycles can be created for the test
mock_ensure_on_circle.return_value = None
region_one = self._create_region_with_parent_id()
# self circle: region_one->region_one
self.catalog_api.update_region(
region_one['id'],
{'parent_region_id': region_one['id']})
self.catalog_api.delete_region(region_one['id'])
self.assertRaises(exception.RegionNotFound,
self.catalog_api.get_region,
region_one['id'])
# region_one->region_two->region_one
region_one = self._create_region_with_parent_id()
region_two = self._create_region_with_parent_id(region_one['id'])
self.catalog_api.update_region(
region_one['id'],
{'parent_region_id': region_two['id']})
self.catalog_api.delete_region(region_one['id'])
self.assertRaises(exception.RegionNotFound,
self.catalog_api.get_region,
region_one['id'])
self.assertRaises(exception.RegionNotFound,
self.catalog_api.get_region,
region_two['id'])
# region_one->region_two->region_three->region_one
region_one = self._create_region_with_parent_id()
region_two = self._create_region_with_parent_id(region_one['id'])
region_three = self._create_region_with_parent_id(region_two['id'])
self.catalog_api.update_region(
region_one['id'],
{'parent_region_id': region_three['id']})
self.catalog_api.delete_region(region_two['id'])
self.assertRaises(exception.RegionNotFound,
self.catalog_api.get_region,
region_two['id'])
self.assertRaises(exception.RegionNotFound,
self.catalog_api.get_region,
region_one['id'])
self.assertRaises(exception.RegionNotFound,
self.catalog_api.get_region,
region_three['id'])
def test_service_crud(self):
# create
service_id = uuid.uuid4().hex