From 9a950b8f0fe0e60fe658a518f8f4cf066edddf73 Mon Sep 17 00:00:00 2001 From: Guillaume Abrioux Date: Thu, 16 Jul 2020 15:57:14 +0200 Subject: [PATCH] ceph_key: refact the code and minor fixes wip Signed-off-by: Guillaume Abrioux --- library/ceph_key.py | 81 ++++++++++++++++++---------------- tests/library/test_ceph_key.py | 70 ++++++++++++++++++++++++++++- 2 files changed, 112 insertions(+), 39 deletions(-) diff --git a/library/ceph_key.py b/library/ceph_key.py index bdd74902a..0a4df4c20 100644 --- a/library/ceph_key.py +++ b/library/ceph_key.py @@ -545,10 +545,19 @@ def run_module(): # Test if the key exists, if it does we skip its creation # We only want to run this check when a key needs to be added # There is no guarantee that any cluster is running and we don't need one - key_exist = 1 _secret = secret _caps = caps - if (state in ["present", "update", "info"]): + + user = "client.admin" + user_key = os.path.join( + "/etc/ceph/" + cluster + ".client.admin.keyring") + output_format = "json" + _info_key = [] + rc, cmd, out, err = exec_commands( + module, info_key(cluster, name, user, user_key, output_format, container_image)) # noqa E501 + key_exist = rc + + if (state in ["present", "update"]): # if dest is not a directory, the user wants to change the file's name # (e,g: /etc/ceph/ceph.mgr.ceph-mon2.keyring) if not os.path.isdir(dest): @@ -564,42 +573,33 @@ def run_module(): file_args['path'] = file_path - if state != 'info': - if import_key: - user = "client.admin" - user_key = os.path.join( - "/etc/ceph/" + cluster + ".client.admin.keyring") - output_format = "json" - _info_key = [] - rc, cmd, out, err = exec_commands( - module, info_key(cluster, name, user, user_key, output_format, container_image)) # noqa E501 - key_exist = rc - if key_exist == 0: - _info_key = json.loads(out) - if not secret: - secret = _info_key[0]['key'] - _secret = _info_key[0]['key'] - if not caps: - caps = _info_key[0]['caps'] - _caps = _info_key[0]['caps'] - if secret == _secret and caps == _caps: - if not os.path.isfile(file_path): - rc, cmd, out, err = exec_commands(module, get_key(cluster, name, file_path, container_image)) # noqa E501 - result["rc"] = rc - if rc != 0: - result["stdout"] = "Couldn't fetch the key {0} at {1}.".format(name, file_path) # noqa E501 - module.exit_json(**result) - result["stdout"] = "fetched the key {0} at {1}.".format(name, file_path) # noqa E501 - - result["stdout"] = "{0} already exists and doesn't need to be updated.".format(name) # noqa E501 - result["rc"] = 0 - module.set_fs_attributes_if_different(file_args, False) - module.exit_json(**result) - else: - if os.path.isfile(file_path) and not secret or not caps: - result["stdout"] = "{0} already exists in {1} you must provide secret *and* caps when import_key is {2}".format(name, dest, import_key) # noqa E501 + if import_key: + if key_exist == 0: + _info_key = json.loads(out) + if not secret: + secret = _info_key[0]['key'] + _secret = _info_key[0]['key'] + if not caps: + caps = _info_key[0]['caps'] + _caps = _info_key[0]['caps'] + if secret == _secret and caps == _caps: + if not os.path.isfile(file_path): + rc, cmd, out, err = exec_commands(module, get_key(cluster, name, file_path, container_image)) # noqa E501 + result["rc"] = rc + if rc != 0: + result["stdout"] = "Couldn't fetch the key {0} at {1}.".format(name, file_path) # noqa E501 + module.exit_json(**result) + result["stdout"] = "fetched the key {0} at {1}.".format(name, file_path) # noqa E501 + + result["stdout"] = "{0} already exists and doesn't need to be updated.".format(name) # noqa E501 result["rc"] = 0 + module.set_fs_attributes_if_different(file_args, False) module.exit_json(**result) + else: + if os.path.isfile(file_path) and not secret or not caps: + result["stdout"] = "{0} already exists in {1} you must provide secret *and* caps when import_key is {2}".format(name, dest, import_key) # noqa E501 + result["rc"] = 0 + module.exit_json(**result) # "update" is here only for backward compatibility if state in ["present", "update"]: @@ -621,8 +621,13 @@ def run_module(): module.set_fs_attributes_if_different(file_args, False) elif state == "absent": - rc, cmd, out, err = exec_commands( - module, delete_key(cluster, name, container_image)) + if key_exist == 0: + rc, cmd, out, err = exec_commands( + module, delete_key(cluster, name, container_image)) + if rc == 0: + changed = True + else: + rc = 0 elif state == "info": user = "client.admin" diff --git a/tests/library/test_ceph_key.py b/tests/library/test_ceph_key.py index 2604cc02c..496108080 100644 --- a/tests/library/test_ceph_key.py +++ b/tests/library/test_ceph_key.py @@ -555,4 +555,72 @@ class TestCephKeyModule(object): assert result['changed'] == False assert result['stdout'] == '[{"entity":"client.admin","key":"AQC1tw5fF156GhAAoJCvHGX/jl/k7/N4VZm8iQ==","caps":{"mds":"allow *","mgr":"allow *","mon":"allow *","osd":"allow *"}}]' assert result['stderr'] == 'exported keyring for client.admin' - assert result['rc'] == 0 \ No newline at end of file + assert result['rc'] == 0 + + @mock.patch('ansible.module_utils.basic.AnsibleModule.exit_json') + @mock.patch('ceph_key.exec_commands') + def test_state_absent_existing_keyring(self, m_exec_commands, m_exit_json): + set_module_args({"state": "absent", + "cluster": "ceph", + "name": "client.foo" + }) + m_exit_json.side_effect = exit_json + m_exec_commands.return_value = (0, + ['ceph', 'auth', 'rm', 'client.foo'] , + '', 'updated' + ) + + with pytest.raises(AnsibleExitJson) as result: + ceph_key.run_module() + + result = result.value.args[0] + assert result['changed'] == True + assert result['stdout'] == '' + assert result['stderr'] == 'updated' + assert result['rc'] == 0 + + @mock.patch('ansible.module_utils.basic.AnsibleModule.exit_json') + @mock.patch('ceph_key.exec_commands') + def test_state_absent_non_existing_keyring(self, m_exec_commands, m_exit_json): + set_module_args({"state": "absent", + "cluster": "ceph", + "name": "client.foo" + }) + m_exit_json.side_effect = exit_json + m_exec_commands.return_value = (1, + ['ceph', 'auth', 'get', 'client.foo'] , + '', 'Error ENOENT: failed to find client.foo in keyring' + ) + + with pytest.raises(AnsibleExitJson) as result: + ceph_key.run_module() + + result = result.value.args[0] + assert result['changed'] == False + assert result['stdout'] == '' + assert result['stderr'] == 'Error ENOENT: failed to find client.foo in keyring' + assert result['rc'] == 0 + + @mock.patch('ansible.module_utils.basic.AnsibleModule.exit_json') + @mock.patch('ceph_key.exec_commands') + def test_state_absent_non_existing_keyring(self, m_exec_commands, m_exit_json): + set_module_args({"state": "absent", + "cluster": "ceph", + "name": "client.foo" + }) + m_exit_json.side_effect = exit_json + m_exec_commands.return_value = (1, + ['ceph', 'auth', 'get', 'client.foo'] , + '', 'Error ENOENT: failed to find client.foo in keyring' + ) + + with pytest.raises(AnsibleExitJson) as result: + ceph_key.run_module() + + result = result.value.args[0] + assert result['changed'] == False + assert result['stdout'] == '' + assert result['stderr'] == 'Error ENOENT: failed to find client.foo in keyring' + assert result['rc'] == 0 + +# entity client.test2 does not exist \ No newline at end of file -- 2.39.5