use os.path.join() correctly

os.path.join adds the separator (i.e. '/') between the provided path
components only if needed. Providing a single path component doesn't
lead to any checks.

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

diff --git a/library/ceph_key.py b/library/ceph_key.py
index b55b0d59f149d425549ecfd65456143d4fd8d502..fa9439e5b423876ea2cd9773d2ecf7f30790bbd2 100644 (file)
--- a/library/ceph_key.py
+++ b/library/ceph_key.py
@@ -332,8 +332,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")
+        user_key = os.path.join("/etc/ceph/", cluster + "." + user + \
+                                ".keyring")
         cmd_list.append(generate_ceph_cmd(
             cluster, args, user, user_key, container_image))
 
@@ -354,8 +354,7 @@ 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")
+    user_key = os.path.join("/etc/ceph/", cluster + "." + user + ".keyring")
     cmd_list.append(generate_ceph_cmd(
         cluster, args, user, user_key, container_image))
 
@@ -375,8 +374,7 @@ def delete_key(cluster, name, container_image=None):
     ]
 
     user = "client.admin"
-    user_key = os.path.join(
-        "/etc/ceph/" + cluster + "." + user + ".keyring")
+    user_key = os.path.join("/etc/ceph/", cluster + "." + user + ".keyring")
     cmd_list.append(generate_ceph_cmd(
         cluster, args, user, user_key, container_image))
 
@@ -398,8 +396,7 @@ def get_key(cluster, name, dest, container_image=None):
     ]
 
     user = "client.admin"
-    user_key = os.path.join(
-        "/etc/ceph/" + cluster + "." + user + ".keyring")
+    user_key = os.path.join("/etc/ceph/", cluster + "." + user + ".keyring")
     cmd_list.append(generate_ceph_cmd(
         cluster, args, user, user_key, container_image))
 
@@ -497,16 +494,14 @@ def build_key_path(cluster, entity):
 
     if "admin" in entity:
         path = "/etc/ceph"
-        key_path = os.path.join(
-            path + "/" + cluster + "." + entity + ".keyring")
+        key_path = os.path.join(path, cluster + "." + entity + ".keyring")
     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")
+        key_path = os.path.join(path, entity_split, cluster + ".keyring")
     else:
         return None
 
@@ -562,8 +557,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")
+        user_key = os.path.join("/etc/ceph/", cluster + "." + user + \
+                                ".keyring")
         output_format = "json"
         rc, cmd, out, err = exec_commands(
             module, info_key(cluster, name, user, user_key, output_format, container_image))  # noqa E501
@@ -579,10 +574,9 @@ 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")
+            file_path = os.path.join(dest, cluster + ".keyring")
         else:
-            file_path = os.path.join(dest + "/" + cluster +
-                                     "." + name + ".keyring")
+            file_path = os.path.join(dest, cluster + "." + name + ".keyring")
 
         # We allow 'present' to override any existing key
         # ONLY if a secret is provided
@@ -628,24 +622,22 @@ def run_module():
             module.exit_json(**result)
 
         user = "client.admin"
-        user_key = os.path.join(
-            "/etc/ceph/" + cluster + "." + user + ".keyring")
+        user_key = os.path.join("/etc/ceph/", cluster + "." + user + ".keyring")
         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")
+        user_key = os.path.join("/etc/ceph/", cluster + "." + user + ".keyring")
         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")
+        user_key = os.path.join("/var/lib/ceph/mon/", cluster + "-" + \
+                                hostname + "/keyring")
         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 afa11c073e5f146c8f24c4a17998d3889e40dd03..62856c510207a7d89c520543dd620d78df0ef62a 100644 (file)
--- a/library/test_ceph_key.py
+++ b/library/test_ceph_key.py
@@ -104,8 +104,7 @@ class TestCephKeyModule(object):
             'osd': 'allow rwx',
         }
         fake_dest = "/fake/ceph"
-        fake_file_destination = os.path.join(
-            fake_dest + "/" + fake_cluster + "." + fake_name + ".keyring")
+        fake_file_destination = os.path.join(fake_dest, fake_cluster + "." + fake_name + ".keyring")
         expected_command_list = [
             'ceph-authtool',
             '--create-keyring',
@@ -134,8 +133,7 @@ class TestCephKeyModule(object):
             'osd': 'allow rwx',
         }
         fake_dest = "/fake/ceph"
-        fake_file_destination = os.path.join(
-            fake_dest + "/" + fake_cluster + "." + fake_name + ".keyring")
+        fake_file_destination = os.path.join(fake_dest, fake_cluster + "." + fake_name + ".keyring")
         fake_container_image = "docker.io/ceph/daemon:latest-luminous"
         expected_command_list = ['docker',
                                  'run',
@@ -174,8 +172,7 @@ 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_file_destination = os.path.join(fake_dest, fake_cluster + "." + fake_name + ".keyring")
         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
@@ -198,8 +195,7 @@ 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_file_destination = os.path.join(fake_dest, fake_cluster + "." + fake_name + ".keyring")
         fake_container_image = "docker.io/ceph/daemon:latest-luminous"
         expected_command_list = [
             ['docker',   # noqa E128
@@ -247,8 +243,7 @@ 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_file_destination = os.path.join(fake_dest, fake_cluster + "." + fake_name + ".keyring")
         # 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',
@@ -281,8 +276,7 @@ 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_file_destination = os.path.join(fake_dest, fake_cluster + "." + fake_name + ".keyring")
         # 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
@@ -440,8 +434,7 @@ 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_file_destination = os.path.join(fake_dest, fake_cluster + "." + fake_name + ".keyring")
         expected_command_list = [['docker',   # noqa E128
                                  'run',
                                  '--rm',
@@ -465,8 +458,7 @@ 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_file_destination = os.path.join(fake_dest, fake_cluster + "." + fake_name + ".keyring")
         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
@@ -479,7 +471,7 @@ 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_key = os.path.join("/var/lib/ceph/mon/", fake_cluster + "-" + fake_hostname, "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'],
@@ -491,7 +483,7 @@ 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_key = os.path.join("/var/lib/ceph/mon/", fake_cluster + "-" + fake_hostname, "keyring") # noqa E501
         fake_container_image = "docker.io/ceph/daemon:latest-luminous"
         expected_command_list = [['docker',   # noqa E128
                                  'run',

diff --git a/tests/functional/tests/osd/test_osds.py b/tests/functional/tests/osd/test_osds.py
index 2b8e2b2150878fc4dde10d5f269fe1c8d1a23547..d18167045a76ffedee2572aef7e90602b28b2fac 100644 (file)
--- a/tests/functional/tests/osd/test_osds.py
+++ b/tests/functional/tests/osd/test_osds.py
@@ -75,8 +75,8 @@ class TestOSDs(object):
     @pytest.mark.docker
     def test_all_docker_osds_are_up_and_in(self, node, host, setup):
         container_binary = setup["container_binary"]
-        osd_id = host.check_output(os.path.join(
-            container_binary + " ps -q --filter='name=ceph-osd' | head -1"))
+        osd_id = host.check_output(container_binary + " ps -q --filter='name="
+                                   "ceph-osd' | head -1")
         cmd = "sudo {container_binary} exec {osd_id} ceph --cluster={cluster} --connect-timeout 5 --keyring /var/lib/ceph/bootstrap-osd/{cluster}.keyring -n client.bootstrap-osd osd tree -f json".format(  # noqa E501
             osd_id=osd_id,
             cluster=setup["cluster_name"],