From bb52390b0416876257d41fd16d7cc11e62f61288 Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Tue, 9 Apr 2019 11:32:22 +0200 Subject: [PATCH] luks_device: add basic check mode (#54477) * Add basic check mode. * One more early exit. * Fix naming. * Check that device is actually an existing device. --- lib/ansible/modules/crypto/luks_device.py | 102 ++++++----- .../tasks/tests/create-destroy.yml | 170 +++++++++--------- .../luks_device/tasks/tests/device-check.yml | 48 +++++ 3 files changed, 195 insertions(+), 125 deletions(-) create mode 100644 test/integration/targets/luks_device/tasks/tests/device-check.yml diff --git a/lib/ansible/modules/crypto/luks_device.py b/lib/ansible/modules/crypto/luks_device.py index 4efb3eeded..a123d62025 100644 --- a/lib/ansible/modules/crypto/luks_device.py +++ b/lib/ansible/modules/crypto/luks_device.py @@ -101,13 +101,6 @@ requirements: - "wipefs" - "lsblk" -notes: - - "This module does not support check mode. The reason being that - while it is possible to chain several operations together - (e.g. 'create' and 'open'), the latter usually depends on changes - to the system done by the previous one. (LUKS cannot be opened, - when it does not exist.)" - author: "Jan Pokorny (@japokorn)" ''' @@ -172,7 +165,9 @@ name: sample: "luks-c1da9a58-2fde-4256-9d9f-6ab008b4dd1b" ''' +import os import re +import stat from ansible.module_utils.basic import AnsibleModule @@ -249,7 +244,7 @@ class CryptHandler(Handler): return device def is_luks(self, device): - ''' check if the LUKS device does exist + ''' check if the LUKS container does exist ''' result = self._run_command([self._cryptsetup_bin, 'isLuks', device]) return result[RETURN_CODE] == 0 @@ -464,7 +459,16 @@ def run_module(): ) module = AnsibleModule(argument_spec=module_args, - supports_check_mode=False) + supports_check_mode=True) + + if module.params['device'] is not None: + try: + statinfo = os.stat(module.params['device']) + mode = statinfo.st_mode + if not stat.S_ISBLK(mode) and not stat.S_ISCHR(mode): + raise Exception('{0} is not a device'.format(module.params['device'])) + except Exception as e: + module.fail_json(msg=str(e)) crypt = CryptHandler(module) conditions = ConditionsHandler(module, crypt) @@ -474,12 +478,15 @@ def run_module(): # luks create if conditions.luks_create(): - try: - crypt.run_luks_create(module.params['device'], - module.params['keyfile']) - except ValueError as e: - module.fail_json(msg="luks_device error: %s" % e) + if not module.check_mode: + try: + crypt.run_luks_create(module.params['device'], + module.params['keyfile']) + except ValueError as e: + module.fail_json(msg="luks_device error: %s" % e) result['changed'] = True + if module.check_mode: + module.exit_json(**result) # luks open @@ -494,14 +501,17 @@ def run_module(): name = crypt.generate_luks_name(module.params['device']) except ValueError as e: module.fail_json(msg="luks_device error: %s" % e) - try: - crypt.run_luks_open(module.params['device'], - module.params['keyfile'], - name) - except ValueError as e: - module.fail_json(msg="luks_device error: %s" % e) + if not module.check_mode: + try: + crypt.run_luks_open(module.params['device'], + module.params['keyfile'], + name) + except ValueError as e: + module.fail_json(msg="luks_device error: %s" % e) result['name'] = name result['changed'] = True + if module.check_mode: + module.exit_json(**result) # luks close if conditions.luks_close(): @@ -513,39 +523,51 @@ def run_module(): module.fail_json(msg="luks_device error: %s" % e) else: name = module.params['name'] - try: - crypt.run_luks_close(name) - except ValueError as e: - module.fail_json(msg="luks_device error: %s" % e) + if not module.check_mode: + try: + crypt.run_luks_close(name) + except ValueError as e: + module.fail_json(msg="luks_device error: %s" % e) result['changed'] = True + if module.check_mode: + module.exit_json(**result) # luks add key if conditions.luks_add_key(): - try: - crypt.run_luks_add_key(module.params['device'], - module.params['keyfile'], - module.params['new_keyfile']) - except ValueError as e: - module.fail_json(msg="luks_device error: %s" % e) + if not module.check_mode: + try: + crypt.run_luks_add_key(module.params['device'], + module.params['keyfile'], + module.params['new_keyfile']) + except ValueError as e: + module.fail_json(msg="luks_device error: %s" % e) result['changed'] = True + if module.check_mode: + module.exit_json(**result) # luks remove key if conditions.luks_remove_key(): - try: - crypt.run_luks_remove_key(module.params['device'], - module.params['remove_keyfile'], - force_remove_last_key=module.params['force_remove_last_key']) - except ValueError as e: - module.fail_json(msg="luks_device error: %s" % e) + if not module.check_mode: + try: + crypt.run_luks_remove_key(module.params['device'], + module.params['remove_keyfile'], + force_remove_last_key=module.params['force_remove_last_key']) + except ValueError as e: + module.fail_json(msg="luks_device error: %s" % e) result['changed'] = True + if module.check_mode: + module.exit_json(**result) # luks remove if conditions.luks_remove(): - try: - crypt.run_luks_remove(module.params['device']) - except ValueError as e: - module.fail_json(msg="luks_device error: %s" % e) + if not module.check_mode: + try: + crypt.run_luks_remove(module.params['device']) + except ValueError as e: + module.fail_json(msg="luks_device error: %s" % e) result['changed'] = True + if module.check_mode: + module.exit_json(**result) # Success - return result module.exit_json(**result) diff --git a/test/integration/targets/luks_device/tasks/tests/create-destroy.yml b/test/integration/targets/luks_device/tasks/tests/create-destroy.yml index b5bdd73ea1..6ac4d1c388 100644 --- a/test/integration/targets/luks_device/tasks/tests/create-destroy.yml +++ b/test/integration/targets/luks_device/tasks/tests/create-destroy.yml @@ -1,12 +1,12 @@ --- -#- name: Create (check) -# luks_device: -# device: "{{ cryptfile_device }}" -# state: present -# keyfile: "{{ role_path }}/files/keyfile1" -# check_mode: yes -# become: yes -# register: create_check +- name: Create (check) + luks_device: + device: "{{ cryptfile_device }}" + state: present + keyfile: "{{ role_path }}/files/keyfile1" + check_mode: yes + become: yes + register: create_check - name: Create luks_device: device: "{{ cryptfile_device }}" @@ -21,29 +21,29 @@ keyfile: "{{ role_path }}/files/keyfile1" become: yes register: create_idem -#- name: Create (idempotent, check) -# luks_device: -# device: "{{ cryptfile_device }}" -# state: present -# keyfile: "{{ role_path }}/files/keyfile1" -# check_mode: yes -# become: yes -# register: create_idem_check +- name: Create (idempotent, check) + luks_device: + device: "{{ cryptfile_device }}" + state: present + keyfile: "{{ role_path }}/files/keyfile1" + check_mode: yes + become: yes + register: create_idem_check - assert: that: - #- create_check is changed + - create_check is changed - create is changed - create_idem is not changed - #- create_idem_check is not changed + - create_idem_check is not changed -#- name: Open (check) -# luks_device: -# device: "{{ cryptfile_device }}" -# state: opened -# keyfile: "{{ role_path }}/files/keyfile1" -# check_mode: yes -# become: yes -# register: open_check +- name: Open (check) + luks_device: + device: "{{ cryptfile_device }}" + state: opened + keyfile: "{{ role_path }}/files/keyfile1" + check_mode: yes + become: yes + register: open_check - name: Open luks_device: device: "{{ cryptfile_device }}" @@ -58,28 +58,28 @@ keyfile: "{{ role_path }}/files/keyfile1" become: yes register: open_idem -#- name: Open (idempotent, check) -# luks_device: -# device: "{{ cryptfile_device }}" -# state: opened -# keyfile: "{{ role_path }}/files/keyfile1" -# check_mode: yes -# become: yes -# register: open_idem_check +- name: Open (idempotent, check) + luks_device: + device: "{{ cryptfile_device }}" + state: opened + keyfile: "{{ role_path }}/files/keyfile1" + check_mode: yes + become: yes + register: open_idem_check - assert: that: - #- open_check is changed + - open_check is changed - open is changed - open_idem is not changed - #- open_idem_check is not changed + - open_idem_check is not changed -#- name: Closed (via name, check) -# luks_device: -# name: "{{ open.name }}" -# state: closed -# check_mode: yes -# become: yes -# register: close_check +- name: Closed (via name, check) + luks_device: + name: "{{ open.name }}" + state: closed + check_mode: yes + become: yes + register: close_check - name: Closed (via name) luks_device: name: "{{ open.name }}" @@ -92,19 +92,19 @@ state: closed become: yes register: close_idem -#- name: Closed (via name, idempotent, check) -# luks_device: -# name: "{{ open.name }}" -# state: closed -# check_mode: yes -# become: yes -# register: close_idem_check +- name: Closed (via name, idempotent, check) + luks_device: + name: "{{ open.name }}" + state: closed + check_mode: yes + become: yes + register: close_idem_check - assert: that: - #- close_check is changed + - close_check is changed - close is changed - close_idem is not changed - #- close_idem_check is not changed + - close_idem_check is not changed - name: Re-open luks_device: @@ -113,13 +113,13 @@ keyfile: "{{ role_path }}/files/keyfile1" become: yes -#- name: Closed (via device, check) -# luks_device: -# device: "{{ cryptfile_device }}" -# state: closed -# check_mode: yes -# become: yes -# register: close_check +- name: Closed (via device, check) + luks_device: + device: "{{ cryptfile_device }}" + state: closed + check_mode: yes + become: yes + register: close_check - name: Closed (via device) luks_device: device: "{{ cryptfile_device }}" @@ -132,20 +132,20 @@ state: closed become: yes register: close_idem -#- name: Closed (via device, idempotent, check) -# luks_device: -# device: "{{ cryptfile_device }}" -# state: closed -# check_mode: yes -# become: yes -# register: close_idem_check +- name: Closed (via device, idempotent, check) + luks_device: + device: "{{ cryptfile_device }}" + state: closed + check_mode: yes + become: yes + register: close_idem_check - assert: that: - #- close_check is changed + - close_check is changed - close is changed - close_idem is not changed - #- close_idem_check is not changed - + - close_idem_check is not changed + - name: Re-opened luks_device: device: "{{ cryptfile_device }}" @@ -153,13 +153,13 @@ keyfile: "{{ role_path }}/files/keyfile1" become: yes -#- name: Absent (check) -# luks_device: -# device: "{{ cryptfile_device }}" -# state: absent -# check_mode: yes -# become: yes -# register: absent_check +- name: Absent (check) + luks_device: + device: "{{ cryptfile_device }}" + state: absent + check_mode: yes + become: yes + register: absent_check - name: Absent luks_device: device: "{{ cryptfile_device }}" @@ -172,16 +172,16 @@ state: absent become: yes register: absent_idem -#- name: Absent (idempotence, check) -# luks_device: -# device: "{{ cryptfile_device }}" -# state: absent -# check_mode: yes -# become: yes -# register: absent_idem_check +- name: Absent (idempotence, check) + luks_device: + device: "{{ cryptfile_device }}" + state: absent + check_mode: yes + become: yes + register: absent_idem_check - assert: that: - #- absent_check is changed + - absent_check is changed - absent is changed - absent_idem is not changed - #- absent_idem_check is not changed + - absent_idem_check is not changed diff --git a/test/integration/targets/luks_device/tasks/tests/device-check.yml b/test/integration/targets/luks_device/tasks/tests/device-check.yml new file mode 100644 index 0000000000..3682a33e6a --- /dev/null +++ b/test/integration/targets/luks_device/tasks/tests/device-check.yml @@ -0,0 +1,48 @@ +--- +- name: Create with invalid device name (check) + luks_device: + device: /dev/asdfasdfasdf + state: present + keyfile: "{{ role_path }}/files/keyfile1" + check_mode: yes + ignore_errors: yes + become: yes + register: create_check +- name: Create with invalid device name + luks_device: + device: /dev/asdfasdfasdf + state: present + keyfile: "{{ role_path }}/files/keyfile1" + ignore_errors: yes + become: yes + register: create +- assert: + that: + - create_check is failed + - create is failed + - "'o such file or directory' in create_check.msg" + - "'o such file or directory' in create.msg" + +- name: Create with something which is not a device (check) + luks_device: + device: /tmp/ + state: present + keyfile: "{{ role_path }}/files/keyfile1" + check_mode: yes + ignore_errors: yes + become: yes + register: create_check +- name: Create with something which is not a device + luks_device: + device: /tmp/ + state: present + keyfile: "{{ role_path }}/files/keyfile1" + ignore_errors: yes + become: yes + register: create +- assert: + that: + - create_check is failed + - create is failed + - "'is not a device' in create_check.msg" + - "'is not a device' in create.msg"