Merge "Use leader storage for passwords"
This commit is contained in:
commit
68d173ff82
|
@ -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)
|
||||
|
||||
|
||||
|
|
|
@ -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)
|
||||
|
|
|
@ -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'
|
||||
|
|
Loading…
Reference in New Issue