openstack
/
charm-keystone
Code Issues Proposed changes

Tell clients what roles were created 

If a client requests a role then inform them what role was
actually created or already existed.

If a client requests the creation of a role and that role already
exists with a different mix of upper and lower case then the new
role is not created. This is because keystone purports to be case
insensative. However the client may not be case insesative (horizon)
and may assume that the role was created. This change replies to
the client with a new key 'created_roles'. This tells the client
what the case sensative name actually is.

Change-Id: Idc0865a688886a2066dfcdbd15e30118ae5c5bb8
Closes-Bug: #1890437
This commit is contained in:
Liam Young 2020-08-20 15:24:37 +00:00
parent 9e2d5cf2b7
commit f72ae6160b
3 changed files with 74 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

32
hooks/keystone_utils.py
View File

@ -1795,9 +1795,32 @@ def ensure_all_service_accounts_protected_for_pci_dss_options():
protect_user_account_from_pci_dss_force_change_password(user['name'])
def get_real_role_names(roles, manager):
"""Return the name names of the roles.
Keystone attempts to be case insensative but not all client code is so
sometimes the case sensative role name as it is stored in the DB is
needed.
:param roles: List of role names
:type roles: List[str]
:param manager: Manager for this keystone api
:type manager: keystone_utils.KeystoneManagerProxy
:returns: List of role names
:rtype: List[str]
"""
resolved_roles = []
for role in roles:
resolved_role = manager.resolve_role_name(role)
if resolved_role:
resolved_roles.append(resolved_role)
return resolved_roles
def add_service_to_keystone(relation_id=None, remote_unit=None):
manager = get_manager()
settings = relation_get(rid=relation_id, unit=remote_unit)
requested_roles = get_requested_roles(settings)
# the minimum settings needed per endpoint
single = {'service', 'region', 'public_url', 'admin_url', 'internal_url'}
https_cns = []
@ -1822,12 +1845,14 @@ def add_service_to_keystone(relation_id=None, remote_unit=None):
relation_data["api_version"] = get_api_version()
relation_data["admin_domain_id"] = leader_get(
attribute='admin_domain_id')
# Allow the remote service to request creation of any additional
# roles. Currently used by Horizon
for role in get_requested_roles(settings):
for role in requested_roles:
log("Creating requested role: {}".format(role))
create_role(role)
relation_data["created_roles"] = ','.join(get_real_role_names(
requested_roles,
manager))
peer_store_and_set(relation_id=relation_id, **relation_data)
return
@ -1940,6 +1965,8 @@ def add_service_to_keystone(relation_id=None, remote_unit=None):
"admin_domain_id": leader_get(attribute='admin_domain_id'),
"admin_project_id": admin_project_id,
"admin_user_id": admin_user_id,
"created_roles": ','.join(
get_real_role_names(requested_roles, manager))
}
peer_store_and_set(relation_id=relation_id, **relation_data)
@ -2673,4 +2700,5 @@ def endpoints_dict(settings):
'admin': settings.get('admin_url', None),
'internal': settings.get('internal_url', None),
}
return endpoints

16
hooks/manager.py
View File

@ -190,6 +190,22 @@ class KeystoneManager(object):
def resolve_domain_id(self, name):
pass
def resolve_role_name(self, name):
"""Find the role_name of a given role
Find the case-sensative role name that matches the case-insensative
role name supplied.
:param name: Name of role to look up.
:type name: str
:returns: Role name
:rtype: Optional[str]
"""
roles = [r._info for r in self.api.roles.list()]
for r in roles:
if name.lower() == r['name'].lower():
return r['name']
def resolve_role_id(self, name):
"""Find the role_id of a given role"""
roles = [r._info for r in self.api.roles.list()]

33
unit_tests/test_keystone_utils.py
View File

@ -320,13 +320,16 @@ class TestKeystoneUtils(CharmTestCase):
_leader_set.assert_called_with({'db-initialised': True})
mock_stop_manager_instance.assert_called_once_with()
@patch.object(utils, 'get_real_role_names')
@patch.object(utils, 'leader_get')
@patch.object(utils, 'get_api_version')
@patch.object(utils, 'get_manager')
@patch.object(utils, 'resolve_address')
def test_add_service_to_keystone_clustered_https_none_values(
self, _resolve_address, _get_manager,
_get_api_version, _leader_get):
_get_api_version, _leader_get,
_get_real_role_names):
_get_real_role_names.return_value = ['Member', 'SpecialRole']
_get_api_version.return_value = 2
_leader_get.return_value = None
relation_id = 'identity-service:0'
@ -358,10 +361,12 @@ class TestKeystoneUtils(CharmTestCase):
'service_port': 81,
'region': 'RegionOne',
'api_version': 2,
'admin_domain_id': None}
'admin_domain_id': None,
'created_roles': 'Member,SpecialRole'}
self.peer_store_and_set.assert_called_with(relation_id=relation_id,
**relation_data)
@patch.object(utils, 'get_real_role_names')
@patch.object(utils, 'leader_set')
@patch.object(utils, 'leader_get')
@patch.object(utils, 'get_api_version')
@ -373,7 +378,8 @@ class TestKeystoneUtils(CharmTestCase):
def test_add_service_to_keystone_no_clustered_no_https_complete_values(
self, KeystoneManager, add_endpoint, ensure_valid_service,
_resolve_address, create_user, get_api_version, leader_get,
leader_set, test_api_version=2):
leader_set, _get_real_role_names, test_api_version=2):
_get_real_role_names.return_value = ['Member', 'SpecialRole']
get_api_version.return_value = test_api_version
leader_get.return_value = None
relation_id = 'identity-service:0'
@ -449,7 +455,8 @@ class TestKeystoneUtils(CharmTestCase):
'ca_cert': '__null__',
'auth_protocol': 'http', 'service_protocol': 'http',
'service_tenant_id': 'tenant_id',
'api_version': test_api_version}
'api_version': test_api_version,
'created_roles': 'Member,SpecialRole'}
filtered = collections.OrderedDict()
for k, v in relation_data.items():
@ -504,6 +511,7 @@ class TestKeystoneUtils(CharmTestCase):
adminurl='10.0.0.2',
internalurl='192.168.1.2')
@patch.object(utils, 'get_real_role_names')
@patch.object(utils, 'get_requested_roles')
@patch.object(utils, 'create_service_credentials')
@patch.object(utils, 'leader_get')
@ -514,7 +522,8 @@ class TestKeystoneUtils(CharmTestCase):
def test_add_service_to_keystone_multi_endpoints_bug_1739409(
self, KeystoneManager, add_endpoint, ensure_valid_service,
ip_config, leader_get, create_service_credentials,
get_requested_roles):
get_requested_roles, _get_real_role_names):
_get_real_role_names.return_value = ['Member', 'SpecialRole']
relation_id = 'identity-service:8'
remote_unit = 'nova-cloud-controller/0'
get_requested_roles.return_value = 'role1'
@ -1921,3 +1930,17 @@ class TestKeystoneUtils(CharmTestCase):
call({'transitional_charm_user_id': 'fakeid'}),
])
configs.write_all.assert_called_once_with()
def test_get_real_role_name(self):
def _resolve_role_name(role_name):
roles = {'member': 'Member'}
return roles.get(role_name)
manager = MagicMock()
manager.resolve_role_name.side_effect = _resolve_role_name
self.assertEqual(
utils.get_real_role_names(
['member', 'MissingRole'],
manager),
['Member'])