From f02d9e1a9faef5527e279c298b9a3829f71e7d13 Mon Sep 17 00:00:00 2001 From: Guillaume Abrioux Date: Tue, 17 Mar 2020 15:34:11 +0100 Subject: [PATCH] ceph_key: remove 'update' state With this change, the state `present` is enough to update a keyring. If the keyring already exist, it will be updated if caps or secret passed to the module are different. If the keyring doen't exist, it will be created. Closes: https://bugzilla.redhat.com/show_bug.cgi?id=1808367 Signed-off-by: Guillaume Abrioux (cherry picked from commit 553584cbd0d014429e665f998776e8d198f72d2b) --- library/ceph_key.py | 108 ++++++++++++++------------------- tests/library/test_ceph_key.py | 42 ------------- 2 files changed, 44 insertions(+), 106 deletions(-) diff --git a/library/ceph_key.py b/library/ceph_key.py index 3c83a78bc..baffeb316 100644 --- a/library/ceph_key.py +++ b/library/ceph_key.py @@ -1,4 +1,4 @@ -#!/usr/bin/python +#!/usr/bin/python3 # Copyright 2018, Red Hat, Inc. # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -57,12 +57,10 @@ options: If 'absent' is used, the module will simply delete the keyring. If 'list' is used, the module will list all the keys and will return a json output. - If 'update' is used, the module will **only** update - the capabilities of a given keyring. If 'info' is used, the module will return in a json format the description of a given keyring. required: true - choices: ['present', 'absent', 'list', 'update', 'info'] + choices: ['present', 'absent', 'list', 'info'] default: list caps: description: @@ -142,12 +140,6 @@ caps: caps: "{{ caps }}" import_key: False -- name: update cephx key - ceph_key: - name: "my_key" - state: update - caps: "{{ caps }}" - - name: delete cephx key ceph_key: name: "my_key" @@ -351,28 +343,6 @@ def create_key(module, result, cluster, name, secret, caps, import_key, dest, co return cmd_list -def update_key(cluster, name, caps, container_image=None): - ''' - Update a CephX key's capabilities - ''' - - cmd_list = [] - - args = [ - 'caps', - name, - ] - - args = generate_caps(args, "ceph", caps) - user = "client.admin" - 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)) - - return cmd_list - - def delete_key(cluster, name, container_image=None): ''' Delete a CephX key @@ -552,8 +522,10 @@ def run_module(): import_key = module.params.get('import_key') dest = module.params.get('dest') + changed = False + result = dict( - changed=False, + changed=changed, stdout='', stderr='', rc='', @@ -573,17 +545,33 @@ 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 - if import_key: + key_exist = 1 + _secret = secret + _caps = caps + if (state in ["present", "update"] and import_key) or state == "info": user = "client.admin" keyring_filename = cluster + '.' + user + '.keyring' user_key = os.path.join("/etc/ceph/", keyring_filename) 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) + _secret = _info_key[0]['key'] + _caps = _info_key[0]['caps'] + if import_key and secret == _secret and caps == _caps: + result["stdout"] = "{0} already exists and doesn't need to be updated.".format(name) # noqa E501 + result["rc"] = 0 + module.exit_json(**result) - if state == "present": - if not caps: + # "update" is here only for backward compatibility + if state in ["present", "update"]: + if not caps and import_key and rc != 0: fatal("Capabilities must be provided when state is 'present'", module) # noqa E501 + if import_key and key_exist != 0 and secret is None and caps is None: + fatal("Keyring doesn't exist, you must provide 'secret' and 'caps'", module) # noqa E501 # if dest is not a directory, the user wants to change the file's name # (e,g: /etc/ceph/ceph.mgr.ceph-mon2.keyring) @@ -600,36 +588,28 @@ def run_module(): file_args['path'] = file_path - # We allow 'present' to override any existing key - # ONLY if a secret is provided - # if not we skip the creation - if import_key: - if rc == 0 and not secret: - # If the key exists in Ceph we must fetch it on the system - # because nothing tells us it exists on the fs or not - rc, cmd, out, err = exec_commands(module, get_key(cluster, name, file_path, container_image)) # noqa E501 - result["stdout"] = "skipped, since {0} already exists, we only fetched the key at {1}. If you want to update a key use 'state: update'".format( # noqa E501 + # There's no need to run create_key() if neither secret nor caps have changed + if (key_exist == 0 and (secret != _secret or caps != _caps)) or key_exist != 0: + rc, cmd, out, err = exec_commands(module, create_key( + module, result, cluster, name, secret, caps, import_key, file_path, container_image)) # noqa E501 + if rc != 0: + result["stdout"] = "Couldn't create or update {0}".format(name) + result["stderr"] = err + module.exit_json(**result) + changed = True + # fetch the key on the system + if key_exist != 1 and not import_key: + 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( # noqa E501 name, file_path) - result['rc'] = rc - module.set_fs_attributes_if_different(file_args, False) module.exit_json(**result) - rc, cmd, out, err = exec_commands(module, create_key( - module, result, cluster, name, secret, caps, import_key, file_path, container_image)) # noqa E501 - module.set_fs_attributes_if_different(file_args, False) - elif state == "update": - if not caps: - fatal("Capabilities must be provided when state is 'update'", module) # noqa E501 - - if rc != 0: - result["stdout"] = "skipped, since {0} does not exist".format(name) - result['rc'] = 0 - module.exit_json(**result) + result["stdout"] = "fetched the key {0} at {1}.".format( # noqa E501 + name, file_path) - rc, cmd, out, err = exec_commands( - module, update_key(cluster, name, caps, container_image)) - # After the update we don't need to overwrite the key on the filesystem - # since the secret has not changed + module.set_fs_attributes_if_different(file_args, False) elif state == "absent": rc, cmd, out, err = exec_commands( @@ -697,7 +677,7 @@ def run_module(): module.set_fs_attributes_if_different(file_args, False) else: module.fail_json( - msg='State must either be "present" or "absent" or "update" or "list" or "info" or "fetch_initial_keys".', changed=False, rc=1) # noqa E501 + msg='State must either be "present" or "absent" or "list" or "info" or "fetch_initial_keys".', changed=False, rc=1) # noqa E501 endd = datetime.datetime.now() delta = endd - startd @@ -710,7 +690,7 @@ def run_module(): rc=rc, stdout=out.rstrip("\r\n"), stderr=err.rstrip("\r\n"), - changed=True, + changed=changed, ) if rc != 0: diff --git a/tests/library/test_ceph_key.py b/tests/library/test_ceph_key.py index b06178e3a..18bc486c2 100644 --- a/tests/library/test_ceph_key.py +++ b/tests/library/test_ceph_key.py @@ -312,48 +312,6 @@ class TestCephKeyModule(object): fake_secret, fake_caps, fake_import_key, fake_file_destination, fake_container_image) # noqa E501 assert result == expected_command_list - def test_update_key_non_container(self): - fake_cluster = "fake" - fake_name = "client.fake" - fake_caps = { - 'mon': 'allow *', - 'osd': 'allow rwx', - } - expected_command_list = [ - ['ceph', '-n', 'client.admin', '-k', '/etc/ceph/fake.client.admin.keyring', '--cluster', fake_cluster, 'auth', 'caps', # noqa E501 - fake_name, 'mon', 'allow *', 'osd', 'allow rwx'], - ] - result = ceph_key.update_key(fake_cluster, fake_name, fake_caps) - assert result == expected_command_list - - def test_update_key_container(self): - fake_cluster = "fake" - fake_name = "client.fake" - fake_caps = { - 'mon': 'allow *', - 'osd': 'allow rwx', - } - fake_container_image = "docker.io/ceph/daemon:latest-luminous" - expected_command_list = [['docker', # noqa E128 - 'run', - '--rm', - '--net=host', - '-v', '/etc/ceph:/etc/ceph:z', - '-v', '/var/lib/ceph/:/var/lib/ceph/:z', - '-v', '/var/log/ceph/:/var/log/ceph/:z', - '--entrypoint=ceph', - 'docker.io/ceph/daemon:latest-luminous', - '-n', 'client.admin', - '-k', '/etc/ceph/fake.client.admin.keyring', - '--cluster', fake_cluster, - 'auth', - 'caps', fake_name, - 'mon', 'allow *', 'osd', 'allow rwx'] - ] - result = ceph_key.update_key( - fake_cluster, fake_name, fake_caps, fake_container_image) - assert result == expected_command_list - def test_delete_key_non_container(self): fake_cluster = "fake" fake_name = "client.fake" -- 2.39.5