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 <gabrioux@redhat.com>
(cherry picked from commit 553584cbd0d014429e665f998776e8d198f72d2b)

diff --git a/library/ceph_key.py b/library/ceph_key.py
index 3c83a78bca99fb1800b623c524703fa3a836c77c..baffeb316f7a3af4df5570f9d43c3e5047886600 100644 (file)
--- 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 b06178e3a0bf8c669c0a423742b44d8f058704bb..18bc486c25b791c8d1eb328136b083a79274ad45 100644 (file)
--- 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"