Merge "Use leader storage for passwords"

This commit is contained in:
Zuul 2018-07-30 08:33:17 +00:00 committed by Gerrit Code Review
commit 68d173ff82
3 changed files with 28 additions and 46 deletions

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)

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)

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'