openstack
/
charm-keystone
Code Issues Proposed changes

Use leader storage for passwords 

There are no relation-level concerns for these values.

Any pre-existing deployments with recent charms (as in released
2015 or later) will have already migrated the peer storage to
leader storage, so this change can build on that work directly.

Ref: https://github.com/juju/charm-helpers/blame/master/charmhelpers/contrib/peerstorage/__init__.py

Change-Id: I85d1746bdf3e9d3e1ff514e6dd2b4c565dee4dfc
Closes-Bug: #1783943
This commit is contained in:
Frode Nordahl 2018-07-27 14:11:22 +02:00
parent a4c39edc24
commit 3f58b2861b
No known key found for this signature in database
GPG Key ID: 6A5D59A3BA48373F
3 changed files with 28 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

40
hooks/keystone_utils.py
View File

@ -72,6 +72,7 @@ from charmhelpers.core.decorators import (
from charmhelpers.core.hookenv import (
config,
is_leader,
leader_get,
leader_set,
log,
@ -105,8 +106,6 @@ from charmhelpers.core.host import (
from charmhelpers.contrib.peerstorage import (
peer_store_and_set,
peer_store,
peer_retrieve,
)
import keystone_context
@ -1035,18 +1034,12 @@ def store_data(backing_file, data):
def get_admin_passwd(user=None):
passwd = config("admin-password")
if passwd and passwd.lower() != "none":
# Previous charm versions did not always store on leader setting so do
# this now to avoid an initial update on install/upgrade
if (is_elected_leader(CLUSTER_RES) and
peer_retrieve('{}_passwd'.format(user)) is None):
set_admin_passwd(passwd, user=user)
return passwd
_migrate_admin_password()
passwd = peer_retrieve('{}_passwd'.format(user))
passwd = leader_get('{}_passwd'.format(user))
if not passwd and is_elected_leader(CLUSTER_RES):
if not passwd and is_leader():
log("Generating new passwd for user: %s" %
config("admin-user"))
cmd = ['pwgen', '-c', '16', '1']
@ -1059,7 +1052,7 @@ def set_admin_passwd(passwd, user=None):
if user is None:
user = 'admin'
peer_store('{}_passwd'.format(user), passwd)
leader_set({'{}_passwd'.format(user): passwd})
def get_api_version():
@ -1228,29 +1221,28 @@ def load_stored_passwords(path=SERVICE_PASSWD_PATH):
def _migrate_admin_password():
"""Migrate on-disk admin passwords to peer storage"""
if os.path.exists(STORED_PASSWD):
log('Migrating on-disk stored passwords to peer storage')
"""Migrate on-disk admin passwords to leader storage"""
if is_leader() and os.path.exists(STORED_PASSWD):
log('Migrating on-disk stored passwords to leader storage')
with open(STORED_PASSWD) as fd:
peer_store("admin_passwd", fd.readline().strip('\n'))
leader_set({"admin_passwd": fd.readline().strip('\n')})
os.unlink(STORED_PASSWD)
def _migrate_service_passwords():
"""Migrate on-disk service passwords to peer storage"""
if os.path.exists(SERVICE_PASSWD_PATH):
log('Migrating on-disk stored passwords to peer storage')
"""Migrate on-disk service passwords to leader storage"""
if is_leader() and os.path.exists(SERVICE_PASSWD_PATH):
log('Migrating on-disk stored passwords to leader storage')
creds = load_stored_passwords()
for k, v in creds.iteritems():
peer_store(key="{}_passwd".format(k), value=v)
leader_set({"{}_passwd".format(k): v})
os.unlink(SERVICE_PASSWD_PATH)
def get_service_password(service_username):
_migrate_service_passwords()
peer_key = "{}_passwd".format(service_username)
passwd = peer_retrieve(peer_key)
passwd = leader_get("{}_passwd".format(service_username))
if passwd is None:
passwd = pwgen(length=64)
@ -1258,13 +1250,11 @@ def get_service_password(service_username):
def set_service_password(passwd, user):
peer_key = "{}_passwd".format(user)
peer_store(key=peer_key, value=passwd)
leader_set({"{}_passwd".format(user): passwd})
def is_password_changed(username, passwd):
peer_key = "{}_passwd".format(username)
_passwd = peer_retrieve(peer_key)
_passwd = leader_get("{}_passwd".format(username))
return (_passwd is None or passwd != _passwd)

8
unit_tests/test_keystone_hooks.py
View File

@ -631,12 +631,10 @@ class KeystoneRelationTests(CharmTestCase):
self.assertTrue(self.update_dns_ha_resource_params.called)
self.relation_set.assert_called_with(**args)
@patch.object(utils, 'peer_retrieve')
@patch('keystone_utils.log')
@patch.object(hooks, 'CONFIGS')
def test_ha_relation_changed_not_clustered_not_leader(self, configs,
mock_log,
mock_peer_retrieve):
mock_log):
self.relation_get.return_value = False
hooks.ha_changed()
@ -842,7 +840,6 @@ class KeystoneRelationTests(CharmTestCase):
# Still updates relations
self.assertTrue(self.relation_ids.called)
@patch.object(utils, 'peer_retrieve')
@patch.object(hooks, 'update_all_identity_relation_units')
@patch.object(utils, 'os_release')
@patch('keystone_utils.log')
@ -850,11 +847,10 @@ class KeystoneRelationTests(CharmTestCase):
def test_upgrade_charm_not_leader(self,
mock_relation_ids,
mock_log,
os_release, update, mock_peer_retrieve):
os_release, update):
os_release.return_value = 'havana'
self.filter_installed_packages.return_value = []
mock_peer_retrieve.return_value = 'true'
self.is_elected_leader.return_value = False
hooks.upgrade_charm()
self.assertTrue(self.apt_install.called)

26
unit_tests/test_keystone_utils.py
View File

@ -60,7 +60,6 @@ TO_PATCH = [
'local_unit',
'related_units',
'https',
'peer_store',
'mkdir',
'write_file',
# generic
@ -291,6 +290,7 @@ class TestKeystoneUtils(CharmTestCase):
self.peer_store_and_set.assert_called_with(relation_id=relation_id,
**relation_data)
@patch.object(utils, 'leader_set')
@patch.object(utils, 'leader_get')
@patch.object(utils, 'get_api_version')
@patch.object(utils, 'create_user')
@ -301,7 +301,7 @@ 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,
test_api_version=2):
leader_set, test_api_version=2):
get_api_version.return_value = test_api_version
leader_get.return_value = None
relation_id = 'identity-service:0'
@ -395,6 +395,8 @@ class TestKeystoneUtils(CharmTestCase):
test_add_service_to_keystone_no_clustered_no_https_complete_values(
test_api_version=3)
@patch.object(utils, 'leader_set')
@patch.object(utils, 'is_leader')
@patch.object(utils, 'leader_get')
@patch('charmhelpers.contrib.openstack.ip.config')
@patch.object(utils, 'ensure_valid_service')
@ -402,7 +404,7 @@ class TestKeystoneUtils(CharmTestCase):
@patch.object(utils, 'get_manager')
def test_add_service_to_keystone_nosubset(
self, KeystoneManager, add_endpoint, ensure_valid_service,
ip_config, leader_get):
ip_config, leader_get, is_leader, leader_set):
relation_id = 'identity-service:0'
remote_unit = 'unit/0'
@ -666,22 +668,16 @@ class TestKeystoneUtils(CharmTestCase):
mock_relation_set.assert_called_once_with(relation_id=relation_id,
relation_settings=settings)
@patch.object(utils, 'is_elected_leader')
@patch.object(utils, 'peer_retrieve')
@patch.object(utils, 'peer_store')
def test_get_admin_passwd_pwd_set(self, mock_peer_store,
mock_peer_retrieve,
mock_is_elected_leader):
mock_peer_retrieve.return_value = None
def test_get_admin_passwd_pwd_set(self):
self.test_config.set('admin-password', 'supersecret')
mock_is_elected_leader.return_value = True
self.assertEqual(utils.get_admin_passwd(), 'supersecret')
mock_peer_store.assert_called_once_with('admin_passwd', 'supersecret')
@patch.object(utils, 'peer_retrieve')
@patch.object(utils, 'is_leader')
@patch.object(utils, 'leader_get')
@patch('os.path.isfile')
def test_get_admin_passwd_genpass(self, isfile, peer_retrieve):
peer_retrieve.return_value = 'supersecretgen'
def test_get_admin_passwd_genpass(self, isfile, leader_get, is_leader):
is_leader.return_value = True
leader_get.return_value = 'supersecretgen'
self.test_config.set('admin-password', '')
isfile.return_value = False
self.subprocess.check_output.return_value = 'supersecretgen'