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>
pull/3716/head
Rishabh Dave 2019-03-11 15:50:08 +05:30 committed by mergify[bot]
parent 31617afca9
commit 62abe7068a
3 changed files with 27 additions and 43 deletions

View File

@ -332,8 +332,8 @@ def create_key(module, result, cluster, name, secret, caps, import_key, dest, co
if import_key: if import_key:
user = "client.admin" user = "client.admin"
user_key = os.path.join( user_key = os.path.join("/etc/ceph/", cluster + "." + user + \
"/etc/ceph/" + cluster + "." + user + ".keyring") ".keyring")
cmd_list.append(generate_ceph_cmd( cmd_list.append(generate_ceph_cmd(
cluster, args, user, user_key, container_image)) 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) args = generate_caps(args, "ceph", caps)
user = "client.admin" user = "client.admin"
user_key = os.path.join( user_key = os.path.join("/etc/ceph/", cluster + "." + user + ".keyring")
"/etc/ceph/" + cluster + "." + user + ".keyring")
cmd_list.append(generate_ceph_cmd( cmd_list.append(generate_ceph_cmd(
cluster, args, user, user_key, container_image)) cluster, args, user, user_key, container_image))
@ -375,8 +374,7 @@ def delete_key(cluster, name, container_image=None):
] ]
user = "client.admin" user = "client.admin"
user_key = os.path.join( user_key = os.path.join("/etc/ceph/", cluster + "." + user + ".keyring")
"/etc/ceph/" + cluster + "." + user + ".keyring")
cmd_list.append(generate_ceph_cmd( cmd_list.append(generate_ceph_cmd(
cluster, args, user, user_key, container_image)) cluster, args, user, user_key, container_image))
@ -398,8 +396,7 @@ def get_key(cluster, name, dest, container_image=None):
] ]
user = "client.admin" user = "client.admin"
user_key = os.path.join( user_key = os.path.join("/etc/ceph/", cluster + "." + user + ".keyring")
"/etc/ceph/" + cluster + "." + user + ".keyring")
cmd_list.append(generate_ceph_cmd( cmd_list.append(generate_ceph_cmd(
cluster, args, user, user_key, container_image)) cluster, args, user, user_key, container_image))
@ -497,16 +494,14 @@ def build_key_path(cluster, entity):
if "admin" in entity: if "admin" in entity:
path = "/etc/ceph" path = "/etc/ceph"
key_path = os.path.join( key_path = os.path.join(path, cluster + "." + entity + ".keyring")
path + "/" + cluster + "." + entity + ".keyring")
elif "bootstrap" in entity: elif "bootstrap" in entity:
path = "/var/lib/ceph" path = "/var/lib/ceph"
# bootstrap keys show up as 'client.boostrap-osd' # bootstrap keys show up as 'client.boostrap-osd'
# however the directory is called '/var/lib/ceph/bootstrap-osd' # however the directory is called '/var/lib/ceph/bootstrap-osd'
# so we need to substring 'client.' # so we need to substring 'client.'
entity_split = entity.split('.')[1] entity_split = entity.split('.')[1]
key_path = os.path.join( key_path = os.path.join(path, entity_split, cluster + ".keyring")
path + "/" + entity_split + "/" + cluster + ".keyring")
else: else:
return None return None
@ -562,8 +557,8 @@ def run_module():
# There is no guarantee that any cluster is running and we don't need one # There is no guarantee that any cluster is running and we don't need one
if import_key: if import_key:
user = "client.admin" user = "client.admin"
user_key = os.path.join( user_key = os.path.join("/etc/ceph/", cluster + "." + user + \
"/etc/ceph/" + cluster + "." + user + ".keyring") ".keyring")
output_format = "json" output_format = "json"
rc, cmd, out, err = exec_commands( rc, cmd, out, err = exec_commands(
module, info_key(cluster, name, user, user_key, output_format, container_image)) # noqa E501 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: elif 'bootstrap' in dest:
# Build a different path for bootstrap keys as there are stored as # Build a different path for bootstrap keys as there are stored as
# /var/lib/ceph/bootstrap-rbd/ceph.keyring # /var/lib/ceph/bootstrap-rbd/ceph.keyring
file_path = os.path.join(dest + "/" + cluster + ".keyring") file_path = os.path.join(dest, cluster + ".keyring")
else: else:
file_path = os.path.join(dest + "/" + cluster + file_path = os.path.join(dest, cluster + "." + name + ".keyring")
"." + name + ".keyring")
# We allow 'present' to override any existing key # We allow 'present' to override any existing key
# ONLY if a secret is provided # ONLY if a secret is provided
@ -628,24 +622,22 @@ def run_module():
module.exit_json(**result) module.exit_json(**result)
user = "client.admin" user = "client.admin"
user_key = os.path.join( user_key = os.path.join("/etc/ceph/", cluster + "." + user + ".keyring")
"/etc/ceph/" + cluster + "." + user + ".keyring")
output_format = "json" output_format = "json"
rc, cmd, out, err = exec_commands( rc, cmd, out, err = exec_commands(
module, info_key(cluster, name, user, user_key, output_format, container_image)) # noqa E501 module, info_key(cluster, name, user, user_key, output_format, container_image)) # noqa E501
elif state == "list": elif state == "list":
user = "client.admin" user = "client.admin"
user_key = os.path.join( user_key = os.path.join("/etc/ceph/", cluster + "." + user + ".keyring")
"/etc/ceph/" + cluster + "." + user + ".keyring")
rc, cmd, out, err = exec_commands( rc, cmd, out, err = exec_commands(
module, list_keys(cluster, user, user_key, container_image)) module, list_keys(cluster, user, user_key, container_image))
elif state == "fetch_initial_keys": elif state == "fetch_initial_keys":
hostname = socket.gethostname().split('.', 1)[0] hostname = socket.gethostname().split('.', 1)[0]
user = "mon." user = "mon."
user_key = os.path.join( user_key = os.path.join("/var/lib/ceph/mon/", cluster + "-" + \
"/var/lib/ceph/mon/" + cluster + "-" + hostname + "/keyring") hostname + "/keyring")
rc, cmd, out, err = exec_commands( rc, cmd, out, err = exec_commands(
module, list_keys(cluster, user, user_key, container_image)) module, list_keys(cluster, user, user_key, container_image))
if rc != 0: if rc != 0:

View File

@ -104,8 +104,7 @@ class TestCephKeyModule(object):
'osd': 'allow rwx', 'osd': 'allow rwx',
} }
fake_dest = "/fake/ceph" fake_dest = "/fake/ceph"
fake_file_destination = os.path.join( fake_file_destination = os.path.join(fake_dest, fake_cluster + "." + fake_name + ".keyring")
fake_dest + "/" + fake_cluster + "." + fake_name + ".keyring")
expected_command_list = [ expected_command_list = [
'ceph-authtool', 'ceph-authtool',
'--create-keyring', '--create-keyring',
@ -134,8 +133,7 @@ class TestCephKeyModule(object):
'osd': 'allow rwx', 'osd': 'allow rwx',
} }
fake_dest = "/fake/ceph" fake_dest = "/fake/ceph"
fake_file_destination = os.path.join( fake_file_destination = os.path.join(fake_dest, fake_cluster + "." + fake_name + ".keyring")
fake_dest + "/" + fake_cluster + "." + fake_name + ".keyring")
fake_container_image = "docker.io/ceph/daemon:latest-luminous" fake_container_image = "docker.io/ceph/daemon:latest-luminous"
expected_command_list = ['docker', expected_command_list = ['docker',
'run', 'run',
@ -174,8 +172,7 @@ class TestCephKeyModule(object):
} }
fake_import_key = True fake_import_key = True
fake_dest = "/fake/ceph" fake_dest = "/fake/ceph"
fake_file_destination = os.path.join( fake_file_destination = os.path.join(fake_dest, fake_cluster + "." + fake_name + ".keyring")
fake_dest + "/" + fake_cluster + "." + fake_name + ".keyring")
expected_command_list = [ expected_command_list = [
['ceph-authtool', '--create-keyring', fake_file_destination, '--name', fake_name, # noqa E501 ['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 '--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_dest = "/fake/ceph"
fake_import_key = True fake_import_key = True
fake_file_destination = os.path.join( fake_file_destination = os.path.join(fake_dest, fake_cluster + "." + fake_name + ".keyring")
fake_dest + "/" + fake_cluster + "." + fake_name + ".keyring")
fake_container_image = "docker.io/ceph/daemon:latest-luminous" fake_container_image = "docker.io/ceph/daemon:latest-luminous"
expected_command_list = [ expected_command_list = [
['docker', # noqa E128 ['docker', # noqa E128
@ -247,8 +243,7 @@ class TestCephKeyModule(object):
} }
fake_dest = "/fake/ceph" fake_dest = "/fake/ceph"
fake_import_key = False fake_import_key = False
fake_file_destination = os.path.join( fake_file_destination = os.path.join(fake_dest, fake_cluster + "." + fake_name + ".keyring")
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 # 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 = [[ expected_command_list = [[
'ceph-authtool', 'ceph-authtool',
@ -281,8 +276,7 @@ class TestCephKeyModule(object):
} }
fake_dest = "/fake/ceph" fake_dest = "/fake/ceph"
fake_import_key = False fake_import_key = False
fake_file_destination = os.path.join( fake_file_destination = os.path.join(fake_dest, fake_cluster + "." + fake_name + ".keyring")
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 # 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" fake_container_image = "docker.io/ceph/daemon:latest-luminous"
expected_command_list = [['docker', # noqa E128 expected_command_list = [['docker', # noqa E128
@ -440,8 +434,7 @@ class TestCephKeyModule(object):
fake_name = "client.fake" fake_name = "client.fake"
fake_container_image = "docker.io/ceph/daemon:latest-luminous" fake_container_image = "docker.io/ceph/daemon:latest-luminous"
fake_dest = "/fake/ceph" fake_dest = "/fake/ceph"
fake_file_destination = os.path.join( fake_file_destination = os.path.join(fake_dest, fake_cluster + "." + fake_name + ".keyring")
fake_dest + "/" + fake_cluster + "." + fake_name + ".keyring")
expected_command_list = [['docker', # noqa E128 expected_command_list = [['docker', # noqa E128
'run', 'run',
'--rm', '--rm',
@ -465,8 +458,7 @@ class TestCephKeyModule(object):
fake_cluster = "fake" fake_cluster = "fake"
fake_dest = "/fake/ceph" fake_dest = "/fake/ceph"
fake_name = "client.fake" fake_name = "client.fake"
fake_file_destination = os.path.join( fake_file_destination = os.path.join(fake_dest, fake_cluster + "." + fake_name + ".keyring")
fake_dest + "/" + fake_cluster + "." + fake_name + ".keyring")
expected_command_list = [ expected_command_list = [
['ceph', '-n', "client.admin", '-k', "/etc/ceph/fake.client.admin.keyring", # noqa E501 ['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 '--cluster', fake_cluster, 'auth', 'get', fake_name, '-o', fake_file_destination], # noqa E501
@ -479,7 +471,7 @@ class TestCephKeyModule(object):
fake_hostname = "mon01" fake_hostname = "mon01"
fake_cluster = "fake" fake_cluster = "fake"
fake_user = "mon." 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 = [ expected_command_list = [
['ceph', '-n', "mon.", '-k', "/var/lib/ceph/mon/fake-mon01/keyring", # noqa E501 ['ceph', '-n', "mon.", '-k', "/var/lib/ceph/mon/fake-mon01/keyring", # noqa E501
'--cluster', fake_cluster, 'auth', 'ls', '-f', 'json'], '--cluster', fake_cluster, 'auth', 'ls', '-f', 'json'],
@ -491,7 +483,7 @@ class TestCephKeyModule(object):
fake_hostname = "mon01" fake_hostname = "mon01"
fake_cluster = "fake" fake_cluster = "fake"
fake_user = "mon." 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" fake_container_image = "docker.io/ceph/daemon:latest-luminous"
expected_command_list = [['docker', # noqa E128 expected_command_list = [['docker', # noqa E128
'run', 'run',

View File

@ -75,8 +75,8 @@ class TestOSDs(object):
@pytest.mark.docker @pytest.mark.docker
def test_all_docker_osds_are_up_and_in(self, node, host, setup): def test_all_docker_osds_are_up_and_in(self, node, host, setup):
container_binary = setup["container_binary"] container_binary = setup["container_binary"]
osd_id = host.check_output(os.path.join( osd_id = host.check_output(container_binary + " ps -q --filter='name="
container_binary + " ps -q --filter='name=ceph-osd' | head -1")) "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 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, osd_id=osd_id,
cluster=setup["cluster_name"], cluster=setup["cluster_name"],