From 62abe7068a8176b77b7eb94a43a5f8c5a9b4f2b4 Mon Sep 17 00:00:00 2001 From: Rishabh Dave Date: Mon, 11 Mar 2019 15:50:08 +0530 Subject: [PATCH] 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 --- library/ceph_key.py | 38 ++++++++++--------------- library/test_ceph_key.py | 28 +++++++----------- tests/functional/tests/osd/test_osds.py | 4 +-- 3 files changed, 27 insertions(+), 43 deletions(-) diff --git a/library/ceph_key.py b/library/ceph_key.py index b55b0d59f..fa9439e5b 100644 --- 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 afa11c073..62856c510 100644 --- 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 2b8e2b215..d18167045 100644 --- 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"],