more

hooks/keystone_hooks.py

@@ -73,7 +73,8 @@ from keystone_utils import (
     clear_ssl_synced_units,
     is_db_initialised,
     is_pki_enabled,
-    ensure_pki_cert_permissions,
+    ensure_ssl_dir,
+    ensure_pki_dir_permissions,
 )
 
 from charmhelpers.contrib.hahelpers.cluster import (
@@ -133,6 +134,9 @@ def config_changed():
     if openstack_upgrade_available('keystone'):
         do_openstack_upgrade(configs=CONFIGS)
 
+    # Ensure ssl dir exists and is unison-accessible
+    ensure_ssl_dir()
+
     check_call(['chmod', '-R', 'g+wrx', '/var/lib/keystone/'])
 
     # Ensure unison can write to certs dir.
@@ -180,7 +184,7 @@ def initialise_pki():
                '--keystone-group', 'keystone']
         check_call(cmd)
 
-    ensure_pki_cert_permissions()
+    ensure_pki_dir_permissions()
 
 
 @hooks.hook('shared-db-relation-joined')

hooks/keystone_utils.py

@@ -231,7 +231,7 @@ valid_services = {
 }
 
 
-def ensure_pki_cert_permissions():
+def ensure_pki_dir_permissions():
     perms = 0o755
     # Ensure accessible by unison user and group (for sync).
     for path in glob.glob("%s/*" % PKI_CERTS_DIR):
@@ -772,7 +772,7 @@ def check_peer_actions():
                     subprocess.check_call(['update-ca-certificates'])
                 elif action == 'ensure-pki-permissions':
                     log("Running %s" % (action), level=DEBUG)
-                    ensure_pki_cert_permissions()
+                    ensure_pki_dir_permissions()
                 else:
                     log("Unknown action flag=%s" % (flag), level=WARNING)
 
@@ -1153,20 +1153,23 @@ def synchronize_ca_if_changed(force=False, fatal=False):
     return inner_synchronize_ca_if_changed1
 
 
+def ensure_ssl_dir():
+    """Ensure juju ssl dir exists and is unsion read/writable."""
+    perms = 0o755
+    if not os.path.isdir(SSL_DIR):
+        mkdir(SSL_DIR, SSH_USER, 'keystone', perms)
+    else:
+        ensure_permissions(SSL_DIR, user=SSH_USER, group='keystone',
+                           perms=perms)
+
+
 def get_ca(user='keystone', group='keystone'):
     """Initialize a new CA object if one hasn't already been loaded.
 
     This will create a new CA or load an existing one.
     """
     if not ssl.CA_SINGLETON:
-        # Ensure unsion read/writable
-        perms = 0o755
-        if not os.path.isdir(SSL_DIR):
-            mkdir(SSL_DIR, SSH_USER, 'keystone', perms)
-        else:
-            ensure_permissions(SSL_DIR, user=SSH_USER, group='keystone',
-                               perms=perms)
-
+        ensure_ssl_dir()
         d_name = '_'.join(SSL_CA_NAME.lower().split(' '))
         ca = ssl.JujuCA(name=SSL_CA_NAME, user=user, group=group,
                         ca_dir=os.path.join(SSL_DIR,
@@ -1174,12 +1177,6 @@ def get_ca(user='keystone', group='keystone'):
                         root_ca_dir=os.path.join(SSL_DIR,
                                                  '%s_root_ca' % d_name))
 
-        # SSL_DIR is synchronized via all peers over unison+ssh, need
-        # to ensure permissions.
-        subprocess.check_output(['chown', '-R', '%s.%s' % (user, group),
-                                 '%s' % SSL_DIR])
-        subprocess.check_output(['chmod', '-R', 'g+rwx', '%s' % SSL_DIR])
-
         # Ensure a master is elected. This should cover the following cases:
         # * single unit == 'oldest' unit is elected as master
         # * multi unit + not clustered == 'oldest' unit is elcted as master

unit_tests/test_keystone_hooks.py

@@ -273,6 +273,7 @@ class KeystoneRelationTests(CharmTestCase):
 
     @patch('keystone_utils.log')
     @patch('keystone_utils.ensure_ssl_cert_master')
+    @patch.object(hooks, 'ensure_ssl_dir')
     @patch.object(hooks, 'is_pki_enabled')
     @patch.object(hooks, 'is_ssl_cert_master')
     @patch.object(hooks, 'is_db_initialised')
@@ -298,6 +299,7 @@ class KeystoneRelationTests(CharmTestCase):
                                               mock_is_db_initialised,
                                               mock_is_ssl_cert_master,
                                               mock_is_pki_enabled,
+                                              mock_ensure_ssl_dir,
                                               mock_ensure_ssl_cert_master,
                                               mock_log):
         mock_is_pki_enabled.return_value = True
@@ -330,6 +332,7 @@ class KeystoneRelationTests(CharmTestCase):
 
     @patch('keystone_utils.log')
     @patch('keystone_utils.ensure_ssl_cert_master')
+    @patch.object(hooks, 'ensure_ssl_dir')
     @patch.object(hooks, 'is_pki_enabled')
     @patch.object(hooks, 'is_ssl_cert_master')
     @patch.object(hooks, 'ensure_permissions')
@@ -343,6 +346,7 @@ class KeystoneRelationTests(CharmTestCase):
             self, configure_https, identity_changed,
             configs, get_homedir, ensure_user, cluster_joined,
             ensure_permissions, mock_is_ssl_cert_master, mock_is_pki_enabled,
+            mock_ensure_ssl_dir,
             mock_ensure_ssl_cert_master, mock_log):
         mock_is_pki_enabled.return_value = True
         mock_is_ssl_cert_master.return_value = True
@@ -364,6 +368,7 @@ class KeystoneRelationTests(CharmTestCase):
 
     @patch('keystone_utils.log')
     @patch('keystone_utils.ensure_ssl_cert_master')
+    @patch.object(hooks, 'ensure_ssl_dir')
     @patch.object(hooks, 'is_pki_enabled')
     @patch.object(hooks, 'is_ssl_cert_master')
     @patch.object(hooks, 'is_db_initialised')
@@ -388,6 +393,7 @@ class KeystoneRelationTests(CharmTestCase):
                                                    mock_is_db_initialised,
                                                    mock_is_ssl_cert_master,
                                                    mock_is_pki_enabled,
+                                                   mock_ensure_ssl_dir,
                                                    mock_ensure_ssl_cert_master,
                                                    mock_log):
         mock_is_pki_enabled.return_value = True