mirror of
https://github.com/ansible-collections/community.general.git
synced 2024-09-14 20:13:21 +02:00
* improve temporary files storage (naming/removal)
* update unit tests
* Update changelogs/fragments/2163-java_keystore_1667_improve_temp_files_storage.yml
Co-authored-by: Felix Fontein <felix@fontein.de>
* add dedicated function to randomize PKCS#12 filename
fix unit tests (also mock the new function)
Co-authored-by: Felix Fontein <felix@fontein.de>
(cherry picked from commit 533e01a3f9)
Co-authored-by: quidame <quidame@poivron.org>
This commit is contained in:
patchback[bot]
GitHub
parent
a14b525bdc
commit
ab391c2cfa
3 changed files with 51 additions and 24 deletions
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
bugfixes:
|
||||
- "java_keystore - use tempfile lib to create temporary files with randomized
|
||||
names, and remove the temporary PKCS#12 keystore as well as other materials
|
||||
(https://github.com/ansible-collections/community.general/issues/1667)."
|
||||
|
|
@ -114,13 +114,15 @@ cmd:
|
|||
description: Executed command to get action done
|
||||
returned: changed and failure
|
||||
type: str
|
||||
sample: "openssl x509 -noout -in /tmp/cert.crt -fingerprint -sha256"
|
||||
sample: "/usr/bin/openssl x509 -noout -in /tmp/user/1000/tmp8jd_lh23 -fingerprint -sha256"
|
||||
'''
|
||||
|
||||
|
||||
from ansible.module_utils.basic import AnsibleModule
|
||||
import os
|
||||
import re
|
||||
import tempfile
|
||||
|
||||
from ansible.module_utils.basic import AnsibleModule
|
||||
|
||||
|
||||
def read_certificate_fingerprint(module, openssl_bin, certificate_path):
|
||||
|
|
@ -170,18 +172,25 @@ def run_commands(module, cmd, data=None, check_rc=True):
|
|||
return module.run_command(cmd, check_rc=check_rc, data=data)
|
||||
|
||||
|
||||
def create_file(path, content):
|
||||
with open(path, 'w') as f:
|
||||
def create_path():
|
||||
tmpfd, tmpfile = tempfile.mkstemp()
|
||||
os.remove(tmpfile)
|
||||
return tmpfile
|
||||
|
||||
|
||||
def create_file(content):
|
||||
tmpfd, tmpfile = tempfile.mkstemp()
|
||||
with os.fdopen(tmpfd, 'w') as f:
|
||||
f.write(content)
|
||||
return path
|
||||
return tmpfile
|
||||
|
||||
|
||||
def create_tmp_certificate(module):
|
||||
return create_file("/tmp/%s.crt" % module.params['name'], module.params['certificate'])
|
||||
return create_file(module.params['certificate'])
|
||||
|
||||
|
||||
def create_tmp_private_key(module):
|
||||
return create_file("/tmp/%s.key" % module.params['name'], module.params['private_key'])
|
||||
return create_file(module.params['private_key'])
|
||||
|
||||
|
||||
def cert_changed(module, openssl_bin, keytool_bin, keystore_path, keystore_pass, alias):
|
||||
|
|
@ -200,17 +209,13 @@ def create_jks(module, name, openssl_bin, keytool_bin, keystore_path, password,
|
|||
else:
|
||||
certificate_path = create_tmp_certificate(module)
|
||||
private_key_path = create_tmp_private_key(module)
|
||||
keystore_p12_path = create_path()
|
||||
try:
|
||||
if os.path.exists(keystore_path):
|
||||
os.remove(keystore_path)
|
||||
|
||||
keystore_p12_path = "/tmp/keystore.p12"
|
||||
if os.path.exists(keystore_p12_path):
|
||||
os.remove(keystore_p12_path)
|
||||
|
||||
export_p12_cmd = [openssl_bin, "pkcs12", "-export", "-name", name, "-in", certificate_path,
|
||||
"-inkey", private_key_path, "-out",
|
||||
keystore_p12_path, "-passout", "stdin"]
|
||||
"-inkey", private_key_path, "-out", keystore_p12_path, "-passout", "stdin"]
|
||||
|
||||
# when keypass is provided, add -passin
|
||||
cmd_stdin = ""
|
||||
|
|
@ -249,6 +254,7 @@ def create_jks(module, name, openssl_bin, keytool_bin, keystore_path, password,
|
|||
finally:
|
||||
os.remove(certificate_path)
|
||||
os.remove(private_key_path)
|
||||
os.remove(keystore_p12_path)
|
||||
|
||||
|
||||
def update_jks_perm(module, keystore_path):
|
||||
|
|
|
|||
|
|
@ -26,8 +26,8 @@ class TestCreateJavaKeystore(ModuleTestCase):
|
|||
|
||||
orig_exists = os.path.exists
|
||||
self.spec = ArgumentSpec()
|
||||
self.mock_create_file = patch('ansible_collections.community.general.plugins.modules.system.java_keystore.create_file',
|
||||
side_effect=lambda path, content: path)
|
||||
self.mock_create_file = patch('ansible_collections.community.general.plugins.modules.system.java_keystore.create_file')
|
||||
self.mock_create_path = patch('ansible_collections.community.general.plugins.modules.system.java_keystore.create_path')
|
||||
self.mock_run_commands = patch('ansible_collections.community.general.plugins.modules.system.java_keystore.run_commands')
|
||||
self.mock_os_path_exists = patch('os.path.exists',
|
||||
side_effect=lambda path: True if path == '/path/to/keystore.jks' else orig_exists(path))
|
||||
|
|
@ -37,6 +37,7 @@ class TestCreateJavaKeystore(ModuleTestCase):
|
|||
side_effect=lambda path: (False, None))
|
||||
self.run_commands = self.mock_run_commands.start()
|
||||
self.create_file = self.mock_create_file.start()
|
||||
self.create_path = self.mock_create_path.start()
|
||||
self.selinux_context = self.mock_selinux_context.start()
|
||||
self.is_special_selinux_path = self.mock_is_special_selinux_path.start()
|
||||
self.os_path_exists = self.mock_os_path_exists.start()
|
||||
|
|
@ -45,6 +46,7 @@ class TestCreateJavaKeystore(ModuleTestCase):
|
|||
"""Teardown."""
|
||||
super(TestCreateJavaKeystore, self).tearDown()
|
||||
self.mock_create_file.stop()
|
||||
self.mock_create_path.stop()
|
||||
self.mock_run_commands.stop()
|
||||
self.mock_selinux_context.stop()
|
||||
self.mock_is_special_selinux_path.stop()
|
||||
|
|
@ -67,13 +69,15 @@ class TestCreateJavaKeystore(ModuleTestCase):
|
|||
module.exit_json = Mock()
|
||||
|
||||
with patch('os.remove', return_value=True):
|
||||
self.create_path.side_effect = ['/tmp/tmpgrzm2ah7']
|
||||
self.create_file.side_effect = ['/tmp/etacifitrec', '/tmp/yek_etavirp']
|
||||
self.run_commands.side_effect = lambda module, cmd, data: (0, '', '')
|
||||
create_jks(module, "test", "openssl", "keytool", "/path/to/keystore.jks", "changeit", "")
|
||||
module.exit_json.assert_called_once_with(
|
||||
changed=True,
|
||||
cmd=["keytool", "-importkeystore",
|
||||
"-destkeystore", "/path/to/keystore.jks",
|
||||
"-srckeystore", "/tmp/keystore.p12", "-srcstoretype", "pkcs12", "-alias", "test",
|
||||
"-srckeystore", "/tmp/tmpgrzm2ah7", "-srcstoretype", "pkcs12", "-alias", "test",
|
||||
"-deststorepass", "changeit", "-srcstorepass", "changeit", "-noprompt"],
|
||||
msg='',
|
||||
rc=0,
|
||||
|
|
@ -98,12 +102,15 @@ class TestCreateJavaKeystore(ModuleTestCase):
|
|||
module.fail_json = Mock()
|
||||
|
||||
with patch('os.remove', return_value=True):
|
||||
self.create_path.side_effect = ['/tmp/tmp1cyp12xa']
|
||||
self.create_file.side_effect = ['/tmp/tmpvalcrt32', '/tmp/tmpwh4key0c']
|
||||
self.run_commands.side_effect = [(1, '', ''), (0, '', '')]
|
||||
create_jks(module, "test", "openssl", "keytool", "/path/to/keystore.jks", "changeit", "passphrase-foo")
|
||||
module.fail_json.assert_called_once_with(
|
||||
cmd=["openssl", "pkcs12", "-export", "-name", "test",
|
||||
"-in", "/tmp/foo.crt", "-inkey", "/tmp/foo.key",
|
||||
"-out", "/tmp/keystore.p12",
|
||||
"-in", "/tmp/tmpvalcrt32",
|
||||
"-inkey", "/tmp/tmpwh4key0c",
|
||||
"-out", "/tmp/tmp1cyp12xa",
|
||||
"-passout", "stdin",
|
||||
"-passin", "stdin"],
|
||||
msg='',
|
||||
|
|
@ -127,12 +134,15 @@ class TestCreateJavaKeystore(ModuleTestCase):
|
|||
module.fail_json = Mock()
|
||||
|
||||
with patch('os.remove', return_value=True):
|
||||
self.create_path.side_effect = ['/tmp/tmp1cyp12xa']
|
||||
self.create_file.side_effect = ['/tmp/tmpvalcrt32', '/tmp/tmpwh4key0c']
|
||||
self.run_commands.side_effect = [(1, '', ''), (0, '', '')]
|
||||
create_jks(module, "test", "openssl", "keytool", "/path/to/keystore.jks", "changeit", "")
|
||||
module.fail_json.assert_called_once_with(
|
||||
cmd=["openssl", "pkcs12", "-export", "-name", "test",
|
||||
"-in", "/tmp/foo.crt", "-inkey", "/tmp/foo.key",
|
||||
"-out", "/tmp/keystore.p12",
|
||||
"-in", "/tmp/tmpvalcrt32",
|
||||
"-inkey", "/tmp/tmpwh4key0c",
|
||||
"-out", "/tmp/tmp1cyp12xa",
|
||||
"-passout", "stdin"],
|
||||
msg='',
|
||||
rc=1
|
||||
|
|
@ -155,12 +165,14 @@ class TestCreateJavaKeystore(ModuleTestCase):
|
|||
module.fail_json = Mock()
|
||||
|
||||
with patch('os.remove', return_value=True):
|
||||
self.create_path.side_effect = ['/tmp/tmpgrzm2ah7']
|
||||
self.create_file.side_effect = ['/tmp/etacifitrec', '/tmp/yek_etavirp']
|
||||
self.run_commands.side_effect = [(0, '', ''), (1, '', '')]
|
||||
create_jks(module, "test", "openssl", "keytool", "/path/to/keystore.jks", "changeit", "")
|
||||
module.fail_json.assert_called_once_with(
|
||||
cmd=["keytool", "-importkeystore",
|
||||
"-destkeystore", "/path/to/keystore.jks",
|
||||
"-srckeystore", "/tmp/keystore.p12", "-srcstoretype", "pkcs12", "-alias", "test",
|
||||
"-srckeystore", "/tmp/tmpgrzm2ah7", "-srcstoretype", "pkcs12", "-alias", "test",
|
||||
"-deststorepass", "changeit", "-srcstorepass", "changeit", "-noprompt"],
|
||||
msg='',
|
||||
rc=1
|
||||
|
|
@ -174,8 +186,7 @@ class TestCertChanged(ModuleTestCase):
|
|||
"""Setup."""
|
||||
super(TestCertChanged, self).setUp()
|
||||
self.spec = ArgumentSpec()
|
||||
self.mock_create_file = patch('ansible_collections.community.general.plugins.modules.system.java_keystore.create_file',
|
||||
side_effect=lambda path, content: path)
|
||||
self.mock_create_file = patch('ansible_collections.community.general.plugins.modules.system.java_keystore.create_file')
|
||||
self.mock_run_commands = patch('ansible_collections.community.general.plugins.modules.system.java_keystore.run_commands')
|
||||
self.run_commands = self.mock_run_commands.start()
|
||||
self.create_file = self.mock_create_file.start()
|
||||
|
|
@ -201,6 +212,7 @@ class TestCertChanged(ModuleTestCase):
|
|||
)
|
||||
|
||||
with patch('os.remove', return_value=True):
|
||||
self.create_file.side_effect = ['/tmp/placeholder']
|
||||
self.run_commands.side_effect = [(0, 'foo=abcd:1234:efgh', ''), (0, 'SHA256: abcd:1234:efgh', '')]
|
||||
result = cert_changed(module, "openssl", "keytool", "/path/to/keystore.jks", "changeit", 'foo')
|
||||
self.assertFalse(result, 'Fingerprint is identical')
|
||||
|
|
@ -220,6 +232,7 @@ class TestCertChanged(ModuleTestCase):
|
|||
)
|
||||
|
||||
with patch('os.remove', return_value=True):
|
||||
self.create_file.side_effect = ['/tmp/placeholder']
|
||||
self.run_commands.side_effect = [(0, 'foo=abcd:1234:efgh', ''), (0, 'SHA256: wxyz:9876:stuv', '')]
|
||||
result = cert_changed(module, "openssl", "keytool", "/path/to/keystore.jks", "changeit", 'foo')
|
||||
self.assertTrue(result, 'Fingerprint mismatch')
|
||||
|
|
@ -239,6 +252,7 @@ class TestCertChanged(ModuleTestCase):
|
|||
)
|
||||
|
||||
with patch('os.remove', return_value=True):
|
||||
self.create_file.side_effect = ['/tmp/placeholder']
|
||||
self.run_commands.side_effect = [(0, 'foo=abcd:1234:efgh', ''),
|
||||
(1, 'keytool error: java.lang.Exception: Alias <foo> does not exist', '')]
|
||||
result = cert_changed(module, "openssl", "keytool", "/path/to/keystore.jks", "changeit", 'foo')
|
||||
|
|
@ -261,10 +275,11 @@ class TestCertChanged(ModuleTestCase):
|
|||
module.fail_json = Mock()
|
||||
|
||||
with patch('os.remove', return_value=True):
|
||||
self.create_file.side_effect = ['/tmp/tmpdj6bvvme']
|
||||
self.run_commands.side_effect = [(1, '', 'Oops'), (0, 'SHA256: wxyz:9876:stuv', '')]
|
||||
cert_changed(module, "openssl", "keytool", "/path/to/keystore.jks", "changeit", 'foo')
|
||||
module.fail_json.assert_called_once_with(
|
||||
cmd=["openssl", "x509", "-noout", "-in", "/tmp/foo.crt", "-fingerprint", "-sha256"],
|
||||
cmd=["openssl", "x509", "-noout", "-in", "/tmp/tmpdj6bvvme", "-fingerprint", "-sha256"],
|
||||
msg='',
|
||||
err='Oops',
|
||||
rc=1
|
||||
|
|
@ -287,6 +302,7 @@ class TestCertChanged(ModuleTestCase):
|
|||
module.fail_json = Mock(return_value=True)
|
||||
|
||||
with patch('os.remove', return_value=True):
|
||||
self.create_file.side_effect = ['/tmp/placeholder']
|
||||
self.run_commands.side_effect = [(0, 'foo: wxyz:9876:stuv', ''), (1, '', 'Oops')]
|
||||
cert_changed(module, "openssl", "keytool", "/path/to/keystore.jks", "changeit", 'foo')
|
||||
module.fail_json.assert_called_with(
|
||||
|
|
|
|||
Loading…
Reference in a new issue