don't append path components while calling os.path.join()

This creates a confusion whether directory/file names are being
formed by appendng strings or path components are being appended.
Since latter should never be done manually, get rid of the statements
creating confusion.

Signed-off-by: Rishabh Dave <ridave@redhat.com>

diff --git a/library/ceph_key.py b/library/ceph_key.py
index d1150a7275e3f31e01159d46de1c176e1ae47347..03785e316af5a9b25370104b44e849cebea8e899 100644 (file)
--- a/library/ceph_key.py
+++ b/library/ceph_key.py
@@ -331,8 +331,8 @@ def create_key(module, result, cluster, name, secret, caps, import_key, dest, co
 
     if import_key:
         user = "client.admin"
-        user_key = os.path.join("/etc/ceph/", cluster + "." + user + \
-                                ".keyring")
+        keyring_filename = cluster + "." + user + ".keyring"
+        user_key = os.path.join("/etc/ceph/", keyring_filename)
         cmd_list.append(generate_ceph_cmd(
             cluster, args, user, user_key, container_image))
 
@@ -353,7 +353,8 @@ def update_key(cluster, name, caps, container_image=None):
 
     args = generate_caps(args, "ceph", caps)
     user = "client.admin"
-    user_key = os.path.join("/etc/ceph/", cluster + "." + user + ".keyring")
+    keyring_filename = cluster + "." + user + ".keyring"
+    user_key = os.path.join("/etc/ceph/", keyring_filename)
     cmd_list.append(generate_ceph_cmd(
         cluster, args, user, user_key, container_image))
 
@@ -373,7 +374,8 @@ def delete_key(cluster, name, container_image=None):
     ]
 
     user = "client.admin"
-    user_key = os.path.join("/etc/ceph/", cluster + "." + user + ".keyring")
+    keyring_filename = cluster + "." + user + ".keyring"
+    user_key = os.path.join("/etc/ceph/", keyring_filename)
     cmd_list.append(generate_ceph_cmd(
         cluster, args, user, user_key, container_image))
 
@@ -395,7 +397,8 @@ def get_key(cluster, name, dest, container_image=None):
     ]
 
     user = "client.admin"
-    user_key = os.path.join("/etc/ceph/", cluster + "." + user + ".keyring")
+    keyring_filename = cluster + "." + user + ".keyring"
+    user_key = os.path.join("/etc/ceph/", keyring_filename)
     cmd_list.append(generate_ceph_cmd(
         cluster, args, user, user_key, container_image))
 
@@ -493,14 +496,16 @@ def build_key_path(cluster, entity):
 
     if "admin" in entity:
         path = "/etc/ceph"
-        key_path = os.path.join(path, cluster + "." + entity + ".keyring")
+        keyring_filename = cluster + "." + entity + ".keyring"
+        key_path = os.path.join(path, keyring_filename)
     elif "bootstrap" in entity:
         path = "/var/lib/ceph"
         # bootstrap keys show up as 'client.boostrap-osd'
         # however the directory is called '/var/lib/ceph/bootstrap-osd'
         # so we need to substring 'client.'
         entity_split = entity.split('.')[1]
-        key_path = os.path.join(path, entity_split, cluster + ".keyring")
+        keyring_filename = cluster + ".keyring"
+        key_path = os.path.join(path, entity_split, keyring_filename)
     else:
         return None
 
@@ -556,8 +561,8 @@ def run_module():
     # There is no guarantee that any cluster is running and we don't need one
     if import_key:
         user = "client.admin"
-        user_key = os.path.join("/etc/ceph/", cluster + "." + user + \
-                                ".keyring")
+        keyring_filename = cluster + '.' + user + '.keyring'
+        user_key = os.path.join("/etc/ceph/", keyring_filename)
         output_format = "json"
         rc, cmd, out, err = exec_commands(
             module, info_key(cluster, name, user, user_key, output_format, container_image))  # noqa E501
@@ -573,9 +578,11 @@ def run_module():
         elif 'bootstrap' in dest:
             # Build a different path for bootstrap keys as there are stored as
             # /var/lib/ceph/bootstrap-rbd/ceph.keyring
-            file_path = os.path.join(dest, cluster + ".keyring")
+            keyring_filename = cluster + '.keyring'
+            file_path = os.path.join(dest, keyring_filename)
         else:
-            file_path = os.path.join(dest, cluster + "." + name + ".keyring")
+            keyring_filename = cluster + "." + name + ".keyring"
+            file_path = os.path.join(dest, keyring_filename)
 
         # We allow 'present' to override any existing key
         # ONLY if a secret is provided
@@ -621,22 +628,24 @@ def run_module():
             module.exit_json(**result)
 
         user = "client.admin"
-        user_key = os.path.join("/etc/ceph/", cluster + "." + user + ".keyring")
+        keyring_filename = cluster + '.' + user + '.keyring'
+        user_key = os.path.join("/etc/ceph/", keyring_filename)
         output_format = "json"
         rc, cmd, out, err = exec_commands(
             module, info_key(cluster, name, user, user_key, output_format, container_image))  # noqa E501
 
     elif state == "list":
         user = "client.admin"
-        user_key = os.path.join("/etc/ceph/", cluster + "." + user + ".keyring")
+        keyring_filename = cluster + '.' + user + '.keyring'
+        user_key = os.path.join("/etc/ceph/", keyring_filename)
         rc, cmd, out, err = exec_commands(
             module, list_keys(cluster, user, user_key, container_image))
 
     elif state == "fetch_initial_keys":
         hostname = socket.gethostname().split('.', 1)[0]
         user = "mon."
-        user_key = os.path.join("/var/lib/ceph/mon/", cluster + "-" + \
-                                hostname + "/keyring")
+        keyring_filename = cluster + "-" + hostname + "/keyring"
+        user_key = os.path.join("/var/lib/ceph/mon/", keyring_filename)
         rc, cmd, out, err = exec_commands(
             module, list_keys(cluster, user, user_key, container_image))
         if rc != 0:

diff --git a/library/test_ceph_key.py b/library/test_ceph_key.py
index 62856c510207a7d89c520543dd620d78df0ef62a..3350e1a6287302d7a16ccdd8151773d0bb47fc88 100644 (file)
--- a/library/test_ceph_key.py
+++ b/library/test_ceph_key.py
@@ -104,7 +104,8 @@ class TestCephKeyModule(object):
             'osd': 'allow rwx',
         }
         fake_dest = "/fake/ceph"
-        fake_file_destination = os.path.join(fake_dest, fake_cluster + "." + fake_name + ".keyring")
+        fake_keyring_filename = fake_cluster + "." + fake_name + ".keyring"
+        fake_file_destination = os.path.join(fake_dest, fake_keyring_filename)
         expected_command_list = [
             'ceph-authtool',
             '--create-keyring',
@@ -133,7 +134,8 @@ class TestCephKeyModule(object):
             'osd': 'allow rwx',
         }
         fake_dest = "/fake/ceph"
-        fake_file_destination = os.path.join(fake_dest, fake_cluster + "." + fake_name + ".keyring")
+        fake_keyring_filename = fake_cluster + "." + fake_name + ".keyring"
+        fake_file_destination = os.path.join(fake_dest, fake_keyring_filename)
         fake_container_image = "docker.io/ceph/daemon:latest-luminous"
         expected_command_list = ['docker',
                                  'run',
@@ -172,7 +174,8 @@ class TestCephKeyModule(object):
         }
         fake_import_key = True
         fake_dest = "/fake/ceph"
-        fake_file_destination = os.path.join(fake_dest, fake_cluster + "." + fake_name + ".keyring")
+        fake_keyring_filename = fake_cluster + "." + fake_name + ".keyring"
+        fake_file_destination = os.path.join(fake_dest, fake_keyring_filename)
         expected_command_list = [
             ['ceph-authtool', '--create-keyring', fake_file_destination, '--name', fake_name,  # noqa E501
                 '--add-key', fake_secret, '--cap', 'mon', 'allow *', '--cap', 'osd', 'allow rwx'],  # noqa E501
@@ -195,7 +198,8 @@ class TestCephKeyModule(object):
         }
         fake_dest = "/fake/ceph"
         fake_import_key = True
-        fake_file_destination = os.path.join(fake_dest, fake_cluster + "." + fake_name + ".keyring")
+        fake_keyring_filename = fake_cluster + "." + fake_name + ".keyring"
+        fake_file_destination = os.path.join(fake_dest, fake_keyring_filename)
         fake_container_image = "docker.io/ceph/daemon:latest-luminous"
         expected_command_list = [
             ['docker',   # noqa E128
@@ -243,7 +247,8 @@ class TestCephKeyModule(object):
         }
         fake_dest = "/fake/ceph"
         fake_import_key = False
-        fake_file_destination = os.path.join(fake_dest, fake_cluster + "." + fake_name + ".keyring")
+        fake_keyring_filename = fake_cluster + "." + fake_name + ".keyring"
+        fake_file_destination = os.path.join(fake_dest, fake_keyring_filename)
         # create_key passes (one for ceph-authtool and one for itself) itw own array so the expected result is an array within an array # noqa E501
         expected_command_list = [[
             'ceph-authtool',
@@ -276,7 +281,8 @@ class TestCephKeyModule(object):
         }
         fake_dest = "/fake/ceph"
         fake_import_key = False
-        fake_file_destination = os.path.join(fake_dest, fake_cluster + "." + fake_name + ".keyring")
+        fake_keyring_filename = fake_cluster + "." + fake_name + ".keyring"
+        fake_file_destination = os.path.join(fake_dest, fake_keyring_filename)
         # create_key passes (one for ceph-authtool and one for itself) itw own array so the expected result is an array within an array # noqa E501
         fake_container_image = "docker.io/ceph/daemon:latest-luminous"
         expected_command_list = [['docker',   # noqa E128
@@ -434,7 +440,8 @@ class TestCephKeyModule(object):
         fake_name = "client.fake"
         fake_container_image = "docker.io/ceph/daemon:latest-luminous"
         fake_dest = "/fake/ceph"
-        fake_file_destination = os.path.join(fake_dest, fake_cluster + "." + fake_name + ".keyring")
+        fake_keyring_filename = fake_cluster + "." + fake_name + ".keyring"
+        fake_file_destination = os.path.join(fake_dest, fake_keyring_filename)
         expected_command_list = [['docker',   # noqa E128
                                  'run',
                                  '--rm',
@@ -458,7 +465,8 @@ class TestCephKeyModule(object):
         fake_cluster = "fake"
         fake_dest = "/fake/ceph"
         fake_name = "client.fake"
-        fake_file_destination = os.path.join(fake_dest, fake_cluster + "." + fake_name + ".keyring")
+        fake_keyring_filename = fake_cluster + "." + fake_name + ".keyring"
+        fake_file_destination = os.path.join(fake_dest, fake_keyring_filename)
         expected_command_list = [
             ['ceph', '-n', "client.admin", '-k', "/etc/ceph/fake.client.admin.keyring",  # noqa E501
                 '--cluster', fake_cluster, 'auth', 'get', fake_name, '-o', fake_file_destination],  # noqa E501
@@ -471,7 +479,8 @@ class TestCephKeyModule(object):
         fake_hostname = "mon01"
         fake_cluster = "fake"
         fake_user = "mon."
-        fake_key = os.path.join("/var/lib/ceph/mon/", fake_cluster + "-" + fake_hostname, "keyring") # noqa E501
+        fake_keyring_dirname = fake_cluster + "-" + fake_hostname
+        fake_key = os.path.join("/var/lib/ceph/mon/", fake_keyring_dirname, 'keyring') # noqa E501
         expected_command_list = [
             ['ceph', '-n', "mon.", '-k', "/var/lib/ceph/mon/fake-mon01/keyring",  # noqa E501
                 '--cluster', fake_cluster, 'auth', 'ls', '-f', 'json'],
@@ -483,7 +492,8 @@ class TestCephKeyModule(object):
         fake_hostname = "mon01"
         fake_cluster = "fake"
         fake_user = "mon."
-        fake_key = os.path.join("/var/lib/ceph/mon/", fake_cluster + "-" + fake_hostname, "keyring") # noqa E501
+        fake_keyring_dirname = fake_cluster + "-" + fake_hostname
+        fake_key = os.path.join("/var/lib/ceph/mon/", fake_keyring_dirname, 'keyring') # noqa E501
         fake_container_image = "docker.io/ceph/daemon:latest-luminous"
         expected_command_list = [['docker',   # noqa E128
                                  'run',