Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid crash in check mode #243

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions changelogs/fragments/243-permission-check-crash.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
bugfixes:
- "various modules - prevent crashes when modules try to set attributes on not yet existing files in check mode.
This will be fixed in ansible-core 2.12, but it is not backported to every Ansible version we support
(https://github.com/ansible-collections/community.crypto/issue/242, https://github.com/ansible-collections/community.crypto/pull/243)."
2 changes: 2 additions & 0 deletions plugins/module_utils/crypto/support.py
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,8 @@ def _check_state():

def _check_perms(module):
file_args = module.load_file_common_arguments(module.params)
if module.check_file_absent_if_check_mode(file_args['path']):
return False
return not module.set_fs_attributes_if_different(file_args, False)

if not perms_required:
Expand Down
3 changes: 2 additions & 1 deletion plugins/module_utils/io.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,8 @@ def write_file(module, content, default_mode=None, path=None):
# Move tempfile to final destination
module.atomic_move(tmp_name, file_args['path'])
# Try to update permissions again
module.set_fs_attributes_if_different(file_args, False)
if not module.check_file_absent_if_check_mode(file_args['path']):
module.set_fs_attributes_if_different(file_args, False)
except Exception as e:
try:
os.remove(tmp_name)
Expand Down
2 changes: 2 additions & 0 deletions plugins/module_utils/openssh/backends/keypair_backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,8 @@ def _permissions_changed(self, public_key=False):
file_args = self.module.load_file_common_arguments(self.module.params)
if public_key:
file_args['path'] = file_args['path'] + '.pub'
if self.module.check_file_absent_if_check_mode(file_args['path']):
return True
return self.module.set_fs_attributes_if_different(file_args, False)

@property
Expand Down
4 changes: 3 additions & 1 deletion plugins/modules/openssh_cert.py
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,9 @@ def generate(self, module):
module.fail_json(msg="%s" % to_native(e))

file_args = module.load_file_common_arguments(module.params)
if module.set_fs_attributes_if_different(file_args, False):
if module.check_file_absent_if_check_mode(file_args['path']):
self.changed = True
elif module.set_fs_attributes_if_different(file_args, False):
self.changed = True

def convert_to_datetime(self, module, timestring):
Expand Down
5 changes: 4 additions & 1 deletion plugins/modules/openssl_csr.py
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,10 @@ def generate(self, module):
self.changed = True

file_args = module.load_file_common_arguments(module.params)
self.changed = module.set_fs_attributes_if_different(file_args, self.changed)
if module.check_file_absent_if_check_mode(file_args['path']):
self.changed = True
else:
self.changed = module.set_fs_attributes_if_different(file_args, self.changed)

def remove(self, module):
self.module_backend.set_existing(None)
Expand Down
6 changes: 3 additions & 3 deletions plugins/modules/openssl_dhparam.py
Original file line number Diff line number Diff line change
Expand Up @@ -221,9 +221,9 @@ def _check_params_valid(self, module):
def _check_fs_attributes(self, module):
"""Checks (and changes if not in check mode!) fs attributes"""
file_args = module.load_file_common_arguments(module.params)
attrs_changed = module.set_fs_attributes_if_different(file_args, False)

return not attrs_changed
if module.check_file_absent_if_check_mode(file_args['path']):
return False
return not module.set_fs_attributes_if_different(file_args, False)

def dump(self):
"""Serialize the object into a dictionary."""
Expand Down
4 changes: 3 additions & 1 deletion plugins/modules/openssl_pkcs12.py
Original file line number Diff line number Diff line change
Expand Up @@ -733,7 +733,9 @@ def main():
changed = True

file_args = module.load_file_common_arguments(module.params)
if module.set_fs_attributes_if_different(file_args, changed):
if module.check_file_absent_if_check_mode(file_args['path']):
changed = True
elif module.set_fs_attributes_if_different(file_args, changed):
changed = True
else:
if module.check_mode:
Expand Down
5 changes: 4 additions & 1 deletion plugins/modules/openssl_privatekey.py
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,10 @@ def generate(self, module):
self.changed = True

file_args = module.load_file_common_arguments(module.params)
self.changed = module.set_fs_attributes_if_different(file_args, self.changed)
if module.check_file_absent_if_check_mode(file_args['path']):
self.changed = True
else:
self.changed = module.set_fs_attributes_if_different(file_args, self.changed)

def remove(self, module):
self.module_backend.set_existing(None)
Expand Down
4 changes: 3 additions & 1 deletion plugins/modules/openssl_publickey.py
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,9 @@ def generate(self, module):
backend=self.backend,
)
file_args = module.load_file_common_arguments(module.params)
if module.set_fs_attributes_if_different(file_args, False):
if module.check_file_absent_if_check_mode(file_args['path']):
self.changed = True
elif module.set_fs_attributes_if_different(file_args, False):
self.changed = True

def check(self, module, perms_required=True):
Expand Down
5 changes: 4 additions & 1 deletion plugins/modules/x509_certificate.py
Original file line number Diff line number Diff line change
Expand Up @@ -472,7 +472,10 @@ def generate(self, module):
self.changed = True

file_args = module.load_file_common_arguments(module.params)
self.changed = module.set_fs_attributes_if_different(file_args, self.changed)
if module.check_file_absent_if_check_mode(file_args['path']):
self.changed = True
else:
self.changed = module.set_fs_attributes_if_different(file_args, self.changed)

def check(self, module, perms_required=True):
"""Ensure the resource is in its desired state."""
Expand Down
4 changes: 3 additions & 1 deletion plugins/modules/x509_crl.py
Original file line number Diff line number Diff line change
Expand Up @@ -710,7 +710,9 @@ def generate(self):
self.changed = True

file_args = self.module.load_file_common_arguments(self.module.params)
if self.module.set_fs_attributes_if_different(file_args, False):
if self.module.check_file_absent_if_check_mode(file_args['path']):
self.changed = True
elif self.module.set_fs_attributes_if_different(file_args, False):
self.changed = True

def dump(self, check_mode=False):
Expand Down
18 changes: 18 additions & 0 deletions tests/integration/targets/openssl_dhparam/tasks/impl.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,15 @@
---
# The tests for this module generate unsafe parameters for testing purposes;
# otherwise tests would be too slow. Use sizes of at least 2048 in production!
- name: "[{{ select_crypto_backend }}] Generate parameter (check mode)"
openssl_dhparam:
size: 768
path: '{{ output_dir }}/dh768.pem'
select_crypto_backend: "{{ select_crypto_backend }}"
return_content: yes
check_mode: true
register: dhparam_check

- name: "[{{ select_crypto_backend }}] Generate parameter"
openssl_dhparam:
size: 768
Expand All @@ -9,6 +18,15 @@
return_content: yes
register: dhparam

- name: "[{{ select_crypto_backend }}] Don't regenerate parameters with no change (check mode)"
openssl_dhparam:
size: 768
path: '{{ output_dir }}/dh768.pem'
select_crypto_backend: "{{ select_crypto_backend }}"
return_content: yes
check_mode: true
register: dhparam_changed_check

- name: "[{{ select_crypto_backend }}] Don't regenerate parameters with no change"
openssl_dhparam:
size: 768
Expand Down
3 changes: 3 additions & 0 deletions tests/integration/targets/openssl_dhparam/tests/validate.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@
- name: "[{{ select_crypto_backend }}] Check if changed works correctly"
assert:
that:
- dhparam_check is changed
- dhparam is changed
- dhparam_changed_check is not changed
- dhparam_changed is not changed
- dhparam_changed_512 is not changed
- dhparam_changed_to_512 is changed
Expand Down
24 changes: 24 additions & 0 deletions tests/integration/targets/openssl_pkcs12/tasks/impl.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,16 @@
- block:
- name: "({{ select_crypto_backend }}) Generate PKCS#12 file (check mode)"
openssl_pkcs12:
select_crypto_backend: '{{ select_crypto_backend }}'
path: '{{ output_dir }}/ansible.p12'
friendly_name: abracadabra
privatekey_path: '{{ output_dir }}/ansible_pkey1.pem'
certificate_path: '{{ output_dir }}/ansible1.crt'
state: present
return_content: true
check_mode: true
register: p12_standard_check

- name: "({{ select_crypto_backend }}) Generate PKCS#12 file"
openssl_pkcs12:
select_crypto_backend: '{{ select_crypto_backend }}'
Expand All @@ -10,6 +22,18 @@
return_content: true
register: p12_standard

- name: "({{ select_crypto_backend }}) Generate PKCS#12 file again, idempotency (check mode)"
openssl_pkcs12:
select_crypto_backend: '{{ select_crypto_backend }}'
path: '{{ output_dir }}/ansible.p12'
friendly_name: abracadabra
privatekey_path: '{{ output_dir }}/ansible_pkey1.pem'
certificate_path: '{{ output_dir }}/ansible1.crt'
state: present
return_content: true
check_mode: true
register: p12_standard_idempotency_check

- name: "({{ select_crypto_backend }}) Generate PKCS#12 file again, idempotency"
openssl_pkcs12:
select_crypto_backend: '{{ select_crypto_backend }}'
Expand Down
17 changes: 10 additions & 7 deletions tests/integration/targets/openssl_pkcs12/tests/validate.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,20 @@
- name: '({{ select_crypto_backend }}) Validate PKCS#12 (assert)'
assert:
that:
- p12_standard_check is changed
- p12_standard is changed
- p12.stdout_lines[2].split(':')[-1].strip() == 'abracadabra'
- p12_standard.mode == '0400'
- p12_no_pkey.changed
- p12_no_pkey is changed
- p12_validate_no_pkey.stdout_lines[-1] == '-----END CERTIFICATE-----'
- p12_force.changed
- p12_force is changed
- p12_force_and_mode.mode == '0644' and p12_force_and_mode.changed
- p12_dumped.changed
- not p12_standard_idempotency.changed
- not p12_multiple_certs_idempotency.changed
- not p12_dumped_idempotency.changed
- not p12_dumped_check_mode.changed
- p12_dumped is changed
- p12_standard_idempotency is not changed
- p12_standard_idempotency_check is not changed
- p12_multiple_certs_idempotency is not changed
- p12_dumped_idempotency is not changed
- p12_dumped_check_mode is not changed
- "'www1.' in p12_validate_multi_certs.stdout"
- "'www2.' in p12_validate_multi_certs.stdout"
- "'www3.' in p12_validate_multi_certs.stdout"
Expand Down
16 changes: 16 additions & 0 deletions tests/integration/targets/openssl_privatekey/tasks/impl.yml
Original file line number Diff line number Diff line change
@@ -1,11 +1,27 @@
---
- name: "({{ select_crypto_backend }}) Generate privatekey1 - standard (check mode)"
openssl_privatekey:
path: '{{ output_dir }}/privatekey1.pem'
select_crypto_backend: '{{ select_crypto_backend }}'
return_content: yes
check_mode: true
register: privatekey1_check

- name: "({{ select_crypto_backend }}) Generate privatekey1 - standard"
openssl_privatekey:
path: '{{ output_dir }}/privatekey1.pem'
select_crypto_backend: '{{ select_crypto_backend }}'
return_content: yes
register: privatekey1

- name: "({{ select_crypto_backend }}) Generate privatekey1 - standard (idempotence, check mode)"
openssl_privatekey:
path: '{{ output_dir }}/privatekey1.pem'
select_crypto_backend: '{{ select_crypto_backend }}'
return_content: yes
check_mode: true
register: privatekey1_idempotence_check

- name: "({{ select_crypto_backend }}) Generate privatekey1 - standard (idempotence)"
openssl_privatekey:
path: '{{ output_dir }}/privatekey1.pem'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
- name: "({{ select_crypto_backend }}) Validate privatekey1 idempotency and content returned"
assert:
that:
- privatekey1_check is changed
- privatekey1 is changed
- privatekey1_idempotence_check is not changed
- privatekey1_idempotence is not changed
- privatekey1.privatekey == lookup('file', output_dir ~ '/privatekey1.pem', rstrip=False)
- privatekey1.privatekey == privatekey1_idempotence.privatekey
Expand Down
26 changes: 26 additions & 0 deletions tests/integration/targets/openssl_publickey/tasks/impl.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,15 @@
path: '{{ output_dir }}/privatekey.pem'
size: '{{ default_rsa_key_size }}'

- name: "({{ select_crypto_backend }}) Generate publickey - PEM format (check mode)"
openssl_publickey:
path: '{{ output_dir }}/publickey.pub'
privatekey_path: '{{ output_dir }}/privatekey.pem'
select_crypto_backend: '{{ select_crypto_backend }}'
return_content: yes
check_mode: true
register: publickey_check

- name: "({{ select_crypto_backend }}) Generate publickey - PEM format"
openssl_publickey:
path: '{{ output_dir }}/publickey.pub'
Expand All @@ -12,6 +21,15 @@
return_content: yes
register: publickey

- name: "({{ select_crypto_backend }}) Generate publickey - PEM format (check mode, idempotence)"
openssl_publickey:
path: '{{ output_dir }}/publickey.pub'
privatekey_path: '{{ output_dir }}/privatekey.pem'
select_crypto_backend: '{{ select_crypto_backend }}'
return_content: yes
check_mode: true
register: publickey_check2

- name: "({{ select_crypto_backend }}) Generate publickey - PEM format (idempotence)"
openssl_publickey:
path: '{{ output_dir }}/publickey.pub'
Expand All @@ -20,6 +38,14 @@
return_content: yes
register: publickey_idempotence

- name: "({{ select_crypto_backend }}) Verify check mode"
assert:
that:
- publickey_check is changed
- publickey is changed
- publickey_check2 is not changed
- publickey_idempotence is not changed

- name: "({{ select_crypto_backend }}) Generate publickey - OpenSSH format"
openssl_publickey:
path: '{{ output_dir }}/publickey-ssh.pub'
Expand Down
18 changes: 18 additions & 0 deletions tests/integration/targets/x509_certificate/tasks/ownca.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,17 @@
- 'CA:TRUE'
basic_constraints_critical: yes

- name: (OwnCA, {{select_crypto_backend}}) Generate selfsigned CA certificate (check mode)
x509_certificate:
path: '{{ output_dir }}/ca_cert.pem'
csr_path: '{{ output_dir }}/ca_csr.csr'
privatekey_path: '{{ output_dir }}/ca_privatekey.pem'
provider: selfsigned
selfsigned_digest: sha256
select_crypto_backend: '{{ select_crypto_backend }}'
check_mode: true
register: result_check_mode

- name: (OwnCA, {{select_crypto_backend}}) Generate selfsigned CA certificate
x509_certificate:
path: '{{ output_dir }}/ca_cert.pem'
Expand All @@ -43,6 +54,13 @@
provider: selfsigned
selfsigned_digest: sha256
select_crypto_backend: '{{ select_crypto_backend }}'
register: result

- name: (OwnCA, {{select_crypto_backend}}) Verify changed
assert:
that:
- result_check_mode is changed
- result is changed

- name: (OwnCA, {{select_crypto_backend}}) Generate selfsigned CA certificate (privatekey passphrase)
x509_certificate:
Expand Down
26 changes: 26 additions & 0 deletions tests/integration/targets/x509_crl/tasks/impl.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,27 @@
check_mode: yes
register: crl_1_check

- name: Create CRL 1 (check mode)
x509_crl:
path: '{{ output_dir }}/ca-crl1.crl'
privatekey_path: '{{ output_dir }}/ca.key'
issuer:
CN: Ansible
last_update: 20191013000000Z
next_update: 20191113000000Z
revoked_certificates:
- path: '{{ output_dir }}/cert-1.pem'
revocation_date: 20191013000000Z
- path: '{{ output_dir }}/cert-2.pem'
revocation_date: 20191013000000Z
reason: key_compromise
reason_critical: yes
invalidity_date: 20191012000000Z
- serial_number: 1234
revocation_date: 20191001000000Z
check_mode: true
register: crl_1_check

- name: Create CRL 1
x509_crl:
path: '{{ output_dir }}/ca-crl1.crl'
Expand All @@ -40,6 +61,11 @@
revocation_date: 20191001000000Z
register: crl_1

- assert:
that:
- crl_1_check is changed
- crl_1 is changed

- name: Retrieve CRL 1 infos
x509_crl_info:
path: '{{ output_dir }}/ca-crl1.crl'
Expand Down