From f7b20dbb48d73c134cede4a05eae6a02fd97b3b6 Mon Sep 17 00:00:00 2001 From: Rishabh Dave Date: Mon, 11 Mar 2019 16:19:32 +0530 Subject: [PATCH] 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 --- library/ceph_key.py | 39 ++++++++++++++++++++++++--------------- library/test_ceph_key.py | 30 ++++++++++++++++++++---------- 2 files changed, 44 insertions(+), 25 deletions(-) diff --git a/library/ceph_key.py b/library/ceph_key.py index d1150a727..03785e316 100644 --- 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 62856c510..3350e1a62 100644 --- 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', -- 2.39.5