[gnuoy, r=hopem]

Set admin password in identity-admin relation regardless of how it was generated

hooks/keystone_hooks.py

@@ -45,6 +45,7 @@ from keystone_utils import (
     determine_packages,
     do_openstack_upgrade,
     ensure_initial_admin,
+    get_admin_passwd,
     migrate_database,
     save_script_rc,
     synchronize_ca,
@@ -55,7 +56,6 @@ from keystone_utils import (
     CLUSTER_RES,
     KEYSTONE_CONF,
     SSH_USER,
-    STORED_PASSWD,
     setup_ipv6,
     send_notifications,
 )
@@ -71,12 +71,16 @@ from charmhelpers.contrib.peerstorage import (
     peer_retrieve_by_prefix,
     peer_echo,
 )
+from charmhelpers.contrib.openstack.ip import (
+    ADMIN,
+    resolve_address,
+)
 from charmhelpers.contrib.network.ip import (
     get_iface_for_address,
     get_netmask_for_address,
     get_address_in_network,
     get_ipv6_addr,
-    is_ipv6
+    is_ipv6,
 )
 from charmhelpers.contrib.openstack.context import ADDRESS_TYPES
 
@@ -127,7 +131,10 @@ def config_changed():
                 identity_changed(relation_id=r_id,
                                  remote_unit=unit)
 
-    [cluster_joined(rid) for rid in relation_ids('cluster')]
+    for rid in relation_ids('identity-admin'):
+        admin_relation_changed(rid)
+    for rid in relation_ids('cluster'):
+        cluster_joined(rid)
 
 
 @hooks.hook('shared-db-relation-joined')
@@ -172,7 +179,6 @@ def db_changed():
             # units acl entry has been added. So, if the db supports passing
             # a list of permitted units then check if we're in the list.
             allowed_units = relation_get('allowed_units')
-            print "allowed_units:" + str(allowed_units)
             if allowed_units and local_unit() not in allowed_units.split():
                 log('Allowed_units list provided and this unit not present')
                 return
@@ -273,6 +279,8 @@ def cluster_changed():
         for unit in relation_list(r_id):
             identity_changed(relation_id=r_id,
                              remote_unit=unit)
+    for rid in relation_ids('identity-admin'):
+        admin_relation_changed(rid)
 
 
 @hooks.hook('ha-relation-joined')
@@ -344,19 +352,17 @@ def ha_changed():
 
 
 @hooks.hook('identity-admin-relation-changed')
-def admin_relation_changed():
+def admin_relation_changed(relation_id=None):
     # TODO: fixup
     relation_data = {
-        'service_hostname': unit_get('private-address'),
+        'service_hostname': resolve_address(ADMIN),
         'service_port': config('service-port'),
         'service_username': config('admin-user'),
         'service_tenant_name': config('admin-role'),
         'service_region': config('region'),
     }
-    if os.path.isfile(STORED_PASSWD):
+    relation_data['service_password'] = get_admin_passwd()
-        with open(STORED_PASSWD) as f:
+    relation_set(relation_id=relation_id, **relation_data)
-            relation_data['service_password'] = f.readline().strip('\n')
-    relation_set(**relation_data)
 
 
 def configure_https():

hooks/keystone_utils.py

@@ -41,6 +41,7 @@ import charmhelpers.contrib.unison as unison
 
 from charmhelpers.core.hookenv import (
     config,
+    is_relation_made,
     log,
     local_unit,
     relation_get,
@@ -484,6 +485,42 @@ def grant_role(user, role, tenant):
             (user, role, tenant))
 
 
+def store_admin_passwd(passwd):
+    with open(STORED_PASSWD, 'w+') as fd:
+        fd.writelines("%s\n" % passwd)
+
+
+def get_admin_passwd():
+    passwd = config("admin-password")
+    if passwd and passwd.lower() != "none":
+        return passwd
+
+    if eligible_leader(CLUSTER_RES):
+        if os.path.isfile(STORED_PASSWD):
+            log("Loading stored passwd from %s" % STORED_PASSWD, level=INFO)
+            with open(STORED_PASSWD, 'r') as fd:
+                passwd = fd.readline().strip('\n')
+
+        if not passwd:
+            log("Generating new passwd for user: %s" %
+                config("admin-user"))
+            cmd = ['pwgen', '-c', '16', '1']
+            passwd = str(subprocess.check_output(cmd)).strip()
+            store_admin_passwd(passwd)
+
+        if is_relation_made("cluster"):
+            peer_store("admin_passwd", passwd)
+
+        return passwd
+
+    if is_relation_made("cluster"):
+        passwd = peer_retrieve('admin_passwd')
+        if passwd:
+            store_admin_passwd(passwd)
+
+    return passwd
+
+
 def ensure_initial_admin(config):
     """ Ensures the minimum admin stuff exists in whatever database we're
         using.
@@ -496,21 +533,10 @@ def ensure_initial_admin(config):
     """
     create_tenant("admin")
     create_tenant(config("service-tenant"))
-
-    passwd = ""
-    if config("admin-password") != "None":
-        passwd = config("admin-password")
-    elif os.path.isfile(STORED_PASSWD):
-        log("Loading stored passwd from %s" % STORED_PASSWD)
-        passwd = open(STORED_PASSWD, 'r').readline().strip('\n')
-    if passwd == "":
-        log("Generating new passwd for user: %s" %
-            config("admin-user"))
-        cmd = ['pwgen', '-c', '16', '1']
-        passwd = str(subprocess.check_output(cmd)).strip()
-        open(STORED_PASSWD, 'w+').writelines("%s\n" % passwd)
     # User is managed by ldap backend when using ldap identity
     if not (config('identity-backend') == 'ldap' and config('ldap-readonly')):
+        passwd = get_admin_passwd()
+        if passwd:
             create_user(config('admin-user'), passwd, tenant='admin')
             update_user_password(config('admin-user'), passwd)
             create_role(config('admin-role'), config('admin-user'), 'admin')

unit_tests/test_keystone_hooks.py

@@ -235,6 +235,7 @@ class KeystoneRelationTests(CharmTestCase):
             relation_id='identity-service:0',
             remote_unit='unit/0')
 
+    @patch.object(hooks, 'admin_relation_changed')
     @patch.object(hooks, 'cluster_joined')
     @patch.object(unison, 'ensure_user')
     @patch.object(unison, 'get_homedir')
@@ -243,10 +244,11 @@ class KeystoneRelationTests(CharmTestCase):
     @patch.object(hooks, 'configure_https')
     def test_config_changed_no_openstack_upgrade_leader(
             self, configure_https, identity_changed,
-            configs, get_homedir, ensure_user, cluster_joined):
+            configs, get_homedir, ensure_user, cluster_joined,
+            admin_relation_changed):
         self.openstack_upgrade_available.return_value = False
         self.eligible_leader.return_value = True
-        self.relation_ids.return_value = ['identity-service:0']
+        self.relation_ids.return_value = ['dummyid:0']
         self.relation_list.return_value = ['unit/0']
 
         hooks.config_changed()
@@ -262,8 +264,9 @@ class KeystoneRelationTests(CharmTestCase):
         self.log.assert_called_with(
             'Firing identity_changed hook for all related services.')
         identity_changed.assert_called_with(
-            relation_id='identity-service:0',
+            relation_id='dummyid:0',
             remote_unit='unit/0')
+        admin_relation_changed.assert_called_with('dummyid:0')
 
     @patch.object(hooks, 'cluster_joined')
     @patch.object(unison, 'ensure_user')
@@ -289,6 +292,7 @@ class KeystoneRelationTests(CharmTestCase):
         self.assertFalse(self.ensure_initial_admin.called)
         self.assertFalse(identity_changed.called)
 
+    @patch.object(hooks, 'admin_relation_changed')
     @patch.object(hooks, 'cluster_joined')
     @patch.object(unison, 'ensure_user')
     @patch.object(unison, 'get_homedir')
@@ -297,10 +301,11 @@ class KeystoneRelationTests(CharmTestCase):
     @patch.object(hooks, 'configure_https')
     def test_config_changed_with_openstack_upgrade(
             self, configure_https, identity_changed,
-            configs, get_homedir, ensure_user, cluster_joined):
+            configs, get_homedir, ensure_user, cluster_joined,
+            admin_relation_changed):
         self.openstack_upgrade_available.return_value = True
         self.eligible_leader.return_value = True
-        self.relation_ids.return_value = ['identity-service:0']
+        self.relation_ids.return_value = ['dummyid:0']
         self.relation_list.return_value = ['unit/0']
 
         hooks.config_changed()
@@ -318,8 +323,9 @@ class KeystoneRelationTests(CharmTestCase):
         self.log.assert_called_with(
             'Firing identity_changed hook for all related services.')
         identity_changed.assert_called_with(
-            relation_id='identity-service:0',
+            relation_id='dummyid:0',
             remote_unit='unit/0')
+        admin_relation_changed.assert_called_with('dummyid:0')
 
     @patch.object(hooks, 'hashlib')
     @patch.object(hooks, 'send_notifications')

unit_tests/test_keystone_utils.py

@@ -1,4 +1,4 @@
-from mock import patch, call, MagicMock
+from mock import patch, call, MagicMock, Mock
 from test_utils import CharmTestCase
 import os
 import manager
@@ -35,6 +35,8 @@ TO_PATCH = [
     'relation_get',
     'relation_set',
     'https',
+    'is_relation_made',
+    'peer_store',
     # generic
     'apt_update',
     'apt_upgrade',
@@ -323,3 +325,25 @@ class TestKeystoneUtils(CharmTestCase):
         settings['trigger'] = '1234'
         mock_relation_set.assert_called_once_with(relation_id=relation_id,
                                                   relation_settings=settings)
+
+    def test_get_admin_passwd_pwd_set(self):
+        self.test_config.set('admin-password', 'supersecret')
+        self.assertEqual(utils.get_admin_passwd(), 'supersecret')
+
+    @patch('os.path.isfile')
+    def test_get_admin_passwd_pwd_file_load(self, isfile):
+        self.test_config.set('admin-password', '')
+        isfile.return_value = True
+        with patch('__builtin__.open') as mock_open:
+            mock_open.return_value.__enter__ = lambda s: s
+            mock_open.return_value.__exit__ = Mock()
+            mock_open.return_value.readline.return_value = 'supersecretfilepwd'
+            self.assertEqual(utils.get_admin_passwd(), 'supersecretfilepwd')
+
+    @patch.object(utils, 'store_admin_passwd')
+    @patch('os.path.isfile')
+    def test_get_admin_passwd_genpass(self, isfile, store_admin_passwd):
+        self.test_config.set('admin-password', '')
+        isfile.return_value = False
+        self.subprocess.check_output.return_value = 'supersecretgen'
+        self.assertEqual(utils.get_admin_passwd(), 'supersecretgen')