diff --git a/changelogs/fragments/2262-java_keystore-passphrase.yml b/changelogs/fragments/2262-java_keystore-passphrase.yml new file mode 100644 index 0000000000..882ada97c3 --- /dev/null +++ b/changelogs/fragments/2262-java_keystore-passphrase.yml @@ -0,0 +1,8 @@ +breaking_changes: +- "java_keystore - instead of failing, now overwrites keystore if the alias (name) is changed. + This was originally the intended behavior, but did not work due to a logic error. Make sure + that your playbooks and roles do not depend on the old behavior of failing instead of + overwriting (https://github.com/ansible-collections/community.general/issues/1671)." +- "java_keystore - instead of failing, now overwrites keystore if the passphrase is changed. + Make sure that your playbooks and roles do not depend on the old behavior of failing instead + of overwriting (https://github.com/ansible-collections/community.general/issues/1671)." diff --git a/plugins/modules/system/java_keystore.py b/plugins/modules/system/java_keystore.py index 2a34175552..ebfe6abdd7 100644 --- a/plugins/modules/system/java_keystore.py +++ b/plugins/modules/system/java_keystore.py @@ -19,8 +19,9 @@ options: name: description: - Name of the certificate in the keystore. - - If the provided name does not exist in the keystore, the module fails. - This behavior will change in a next release. + - If the provided name does not exist in the keystore, the module + will re-create the keystore. This behavior changed in community.general 3.0.0, + before that the module would fail when the name did not match. type: str required: true certificate: @@ -60,7 +61,9 @@ options: description: - Password that should be used to secure the keystore. - If the provided password fails to unlock the keystore, the module - fails. This behavior will change in a next release. + will re-create the keystore with the new passphrase. This behavior + changed in community.general 3.0.0, before that the module would fail + when the password did not match. type: str required: true dest: @@ -187,16 +190,11 @@ def read_stored_certificate_fingerprint(module, keytool_bin, alias, keystore_pat (rc, stored_certificate_fingerprint_out, stored_certificate_fingerprint_err) = run_commands( module, stored_certificate_fingerprint_cmd, environ_update=dict(STOREPASS=keystore_password)) if rc != 0: - # First intention was to not fail, and overwrite the keystore instead, - # in case of alias mismatch; but an issue in error handling caused the - # module to fail anyway. - # See: https://github.com/ansible-collections/community.general/issues/1671 - # And: https://github.com/ansible-collections/community.general/pull/2183 - # if "keytool error: java.lang.Exception: Alias <%s> does not exist" % alias in stored_certificate_fingerprint_out: - # return "alias mismatch" - # if re.match(r'keytool error: java\.io\.IOException: [Kk]eystore( was tampered with, or)? password was incorrect', - # stored_certificate_fingerprint_out): - # return "password mismatch" + if "keytool error: java.lang.Exception: Alias <%s> does not exist" % alias in stored_certificate_fingerprint_out: + return "alias mismatch" + if re.match(r'keytool error: java\.io\.IOException: [Kk]eystore( was tampered with, or)? password was incorrect', + stored_certificate_fingerprint_out): + return "password mismatch" return module.fail_json(msg=stored_certificate_fingerprint_out, err=stored_certificate_fingerprint_err, cmd=stored_certificate_fingerprint_cmd, diff --git a/tests/integration/targets/java_keystore/tasks/tests.yml b/tests/integration/targets/java_keystore/tasks/tests.yml index 4511af033d..e0de1c6836 100644 --- a/tests/integration/targets/java_keystore/tasks/tests.yml +++ b/tests/integration/targets/java_keystore/tasks/tests.yml @@ -64,7 +64,6 @@ loop: "{{ java_keystore_new_certs }}" check_mode: yes register: result_alias_change_check - when: false # FIXME: module currently crashes - name: Create a Java keystore for the given certificates (alias changed) community.general.java_keystore: @@ -72,7 +71,6 @@ name: foobar loop: "{{ java_keystore_new_certs }}" register: result_alias_change - when: false # FIXME: module currently crashes - name: Create a Java keystore for the given certificates (password changed, check mode) @@ -83,7 +81,6 @@ loop: "{{ java_keystore_new_certs }}" check_mode: yes register: result_pw_change_check - when: false # FIXME: module currently crashes - name: Create a Java keystore for the given certificates (password changed) community.general.java_keystore: @@ -92,7 +89,6 @@ password: hunter2 loop: "{{ java_keystore_new_certs }}" register: result_pw_change - when: false # FIXME: module currently crashes - name: Check that the remote certificates have not been removed ansible.builtin.file: @@ -117,7 +113,7 @@ - result_idem_check is not changed - result_change is changed - result_change_check is changed - # - result_alias_change is changed # FIXME: module currently crashes - # - result_alias_change_check is changed # FIXME: module currently crashes - # - result_pw_change is changed # FIXME: module currently crashes - # - result_pw_change_check is changed # FIXME: module currently crashes + - result_alias_change is changed + - result_alias_change_check is changed + - result_pw_change is changed + - result_pw_change_check is changed diff --git a/tests/unit/plugins/modules/system/test_java_keystore.py b/tests/unit/plugins/modules/system/test_java_keystore.py index 94332d6192..ec14b3734d 100644 --- a/tests/unit/plugins/modules/system/test_java_keystore.py +++ b/tests/unit/plugins/modules/system/test_java_keystore.py @@ -250,19 +250,33 @@ class TestCertChanged(ModuleTestCase): supports_check_mode=self.spec.supports_check_mode ) - module.fail_json = Mock() - 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 does not exist', '')] - cert_changed(module, "openssl", "keytool", "/path/to/keystore.jks", "changeit", 'foo') - module.fail_json.assert_called_once_with( - cmd=["keytool", "-list", "-alias", "foo", "-keystore", "/path/to/keystore.jks", "-storepass:env", "STOREPASS", "-v"], - msg='keytool error: java.lang.Exception: Alias does not exist', - err='', - rc=1 - ) + result = cert_changed(module, "openssl", "keytool", "/path/to/keystore.jks", "changeit", 'foo') + self.assertTrue(result, 'Alias mismatch detected') + + def test_cert_changed_password_mismatch(self): + set_module_args(dict( + certificate='cert-foo', + private_key='private-foo', + dest='/path/to/keystore.jks', + name='foo', + password='changeit' + )) + + module = AnsibleModule( + argument_spec=self.spec.argument_spec, + supports_check_mode=self.spec.supports_check_mode + ) + + 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.io.IOException: Keystore password was incorrect', '')] + result = cert_changed(module, "openssl", "keytool", "/path/to/keystore.jks", "changeit", 'foo') + self.assertTrue(result, 'Password mismatch detected') def test_cert_changed_fail_read_cert(self): set_module_args(dict(