java_keystore: add certificate_path and private_key_path options (#2230)

* java_keystore: add `certificate_path` and `private_key_path` options

* Update DOCUMENTATION and EXAMPLES accordingly.
* Refactor integration tests to play the same tasks twice.
* Add a changelog fragment (minor_changes).

refactor DOCUMENTATION

* Add useful info for better understanding of what options allow
  keystore regeneration on the fly, and what other options lead the
  module to fail, if their values change.
* Fix indentation and tenses.
* Add myself as author.

* readability-related stuff + changelog fragment

changelogs/fragments/2230-java_keystore-1669-ssl-input-files-by-path.yml

@@ -0,0 +1,6 @@
+---
+minor_changes:
+  - "java_keystore - add options ``certificate_path`` and ``private_key_path``,
+    mutually exclusive with ``certificate`` and ``private_key`` respectively, and
+    targetting files on remote hosts rather than their contents on the controller.
+    (https://github.com/ansible-collections/community.general/issues/1669)."

plugins/modules/system/java_keystore.py

@@ -1,7 +1,8 @@
 #!/usr/bin/python
 # -*- coding: utf-8 -*-
 
-# (c) 2016, Guillaume Grossetie <ggrossetie@yuzutech.fr>
+# Copyright: (c) 2016, Guillaume Grossetie <ggrossetie@yuzutech.fr>
+# Copyright: (c) 2021, quidame <quidame@poivron.org>
 # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
 
 from __future__ import (absolute_import, division, print_function)
@@ -11,41 +12,67 @@ __metaclass__ = type
 DOCUMENTATION = '''
 ---
 module: java_keystore
-short_description: Create or delete a Java keystore in JKS format.
+short_description: Create a Java keystore in JKS format
 description:
-     - Create or delete a Java keystore in JKS format for a given certificate.
+  - Bundle a x509 certificate and its private key into a Java Keystore in JKS format.
 options:
   name:
-        type: str
     description:
-          - Name of the certificate.
+      - 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.
+    type: str
     required: true
   certificate:
-        type: str
     description:
-          - Certificate that should be used to create the key store.
-        required: true
+      - Content of the certificate used to create the keystore.
+      - If the fingerprint of the provided certificate does not match the
+        fingerprint of the certificate bundled in the keystore, the keystore
+        is regenerated with the provided certificate.
+      - Exactly one of I(certificate) or I(certificate_path) is required.
+    type: str
+  certificate_path:
+    description:
+      - Location of the certificate used to create the keystore.
+      - If the fingerprint of the provided certificate does not match the
+        fingerprint of the certificate bundled in the keystore, the keystore
+        is regenerated with the provided certificate.
+      - Exactly one of I(certificate) or I(certificate_path) is required.
+    type: path
+    version_added: '3.0.0'
   private_key:
-        type: str
     description:
-          - Private key that should be used to create the key store.
-        required: true
+      - Content of the private key used to create the keystore.
+      - Exactly one of I(private_key) or I(private_key_path) is required.
+    type: str
+  private_key_path:
+    description:
+      - Location of the private key used to create the keystore.
+      - Exactly one of I(private_key) or I(private_key_path) is required.
+    type: path
+    version_added: '3.0.0'
   private_key_passphrase:
     description:
-          - Pass phrase for reading the private key, if required.
+      - Passphrase used to read the private key, if required.
     type: str
-        required: false
     version_added: '0.2.0'
   password:
-        type: str
     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.
+    type: str
     required: true
   dest:
-        type: path
     description:
-          - Absolute path where the jks should be generated.
+      - Absolute path of the generated keystore.
+    type: path
     required: true
+  force:
+    description:
+      - Keystore is created even if it already exists.
+    type: bool
+    default: 'no'
   owner:
     description:
       - Name of the user that should own jks file.
@@ -58,21 +85,25 @@ options:
     description:
       - Mode the file should be.
     required: false
-    force:
-        description:
-          - Key store will be created even if it already exists.
-        required: false
-        type: bool
-        default: 'no'
-requirements: [openssl, keytool]
-author: Guillaume Grossetie (@Mogztter)
+requirements:
+  - openssl in PATH
+  - keytool in PATH
+author:
+  - Guillaume Grossetie (@Mogztter)
+  - quidame (@quidame)
 extends_documentation_fragment:
   - files
-
+seealso:
+  - module: community.general.java_cert
+notes:
+  - I(certificate) and I(private_key) require that their contents are available
+    on the controller (either inline in a playbook, or with the C(file) lookup),
+    while I(certificate_path) and I(private_key_path) require that the files are
+    available on the target host.
 '''
 
 EXAMPLES = '''
-- name: Create a key store for the given certificate (inline)
+- name: Create a keystore for the given certificate/private key pair (inline)
   community.general.java_keystore:
     name: example
     certificate: |
@@ -88,13 +119,21 @@ EXAMPLES = '''
     password: changeit
     dest: /etc/security/keystore.jks
 
-- name: Create a key store for the given certificate (lookup)
+- name: Create a keystore for the given certificate/private key pair (with files on controller)
   community.general.java_keystore:
     name: example
     certificate: "{{ lookup('file', '/path/to/certificate.crt') }}"
     private_key: "{{ lookup('file', '/path/to/private.key') }}"
     password: changeit
     dest: /etc/security/keystore.jks
+
+- name: Create a keystore for the given certificate/private key pair (with files on target host)
+  community.general.java_keystore:
+    name: snakeoil
+    certificate_path: /etc/ssl/certs/ssl-cert-snakeoil.pem
+    private_key_path: /etc/ssl/private/ssl-cert-snakeoil.key
+    password: changeit
+    dest: /etc/security/keystore.jks
 '''
 
 RETURN = '''
@@ -198,12 +237,15 @@ def create_tmp_private_key(module):
 
 
 def cert_changed(module, openssl_bin, keytool_bin, keystore_path, keystore_pass, alias):
+    certificate_path = module.params['certificate_path']
+    if certificate_path is None:
         certificate_path = create_tmp_certificate(module)
     try:
         current_certificate_fingerprint = read_certificate_fingerprint(module, openssl_bin, certificate_path)
         stored_certificate_fingerprint = read_stored_certificate_fingerprint(module, keytool_bin, alias, keystore_path, keystore_pass)
         return current_certificate_fingerprint != stored_certificate_fingerprint
     finally:
+        if module.params['certificate_path'] is None:
             os.remove(certificate_path)
 
 
@@ -211,9 +253,16 @@ def create_jks(module, name, openssl_bin, keytool_bin, keystore_path, password,
     if module.check_mode:
         return module.exit_json(changed=True)
 
+    certificate_path = module.params['certificate_path']
+    if certificate_path is None:
         certificate_path = create_tmp_certificate(module)
+
+    private_key_path = module.params['private_key_path']
+    if private_key_path is None:
         private_key_path = create_tmp_private_key(module)
+
     keystore_p12_path = create_path()
+
     try:
         if os.path.exists(keystore_path):
             os.remove(keystore_path)
@@ -257,7 +306,9 @@ def create_jks(module, name, openssl_bin, keytool_bin, keystore_path, password,
                                 cmd=import_keystore_cmd,
                                 rc=rc)
     finally:
+        if module.params['certificate_path'] is None:
             os.remove(certificate_path)
+        if module.params['private_key_path'] is None:
             os.remove(private_key_path)
         os.remove(keystore_p12_path)
 
@@ -301,23 +352,33 @@ class ArgumentSpec(object):
         self.supports_check_mode = True
         self.add_file_common_args = True
         argument_spec = dict(
-            name=dict(required=True),
-            certificate=dict(required=True, no_log=True),
-            private_key=dict(required=True, no_log=True),
-            password=dict(required=True, no_log=True),
-            dest=dict(required=True, type='path'),
-            force=dict(required=False, default=False, type='bool'),
-            private_key_passphrase=dict(required=False, no_log=True, type='str')
+            name=dict(type='str', required=True),
+            dest=dict(type='path', required=True),
+            certificate=dict(type='str', no_log=True),
+            certificate_path=dict(type='path'),
+            private_key=dict(type='str', no_log=True),
+            private_key_path=dict(type='path', no_log=False),
+            private_key_passphrase=dict(type='str', no_log=True),
+            password=dict(type='str', required=True, no_log=True),
+            force=dict(type='bool', default=False),
+        )
+        choose_between = (
+            ['certificate', 'certificate_path'],
+            ['private_key', 'private_key_path'],
         )
         self.argument_spec = argument_spec
+        self.required_one_of = choose_between
+        self.mutually_exclusive = choose_between
 
 
 def main():
     spec = ArgumentSpec()
     module = AnsibleModule(
         argument_spec=spec.argument_spec,
+        required_one_of=spec.required_one_of,
+        mutually_exclusive=spec.mutually_exclusive,
+        supports_check_mode=spec.supports_check_mode,
         add_file_common_args=spec.add_file_common_args,
-        supports_check_mode=spec.supports_check_mode
     )
     module.run_command_environ_update = dict(LANG='C', LC_ALL='C', LC_MESSAGES='C')
     process_jks(module)

tests/integration/targets/java_keystore/defaults/main.yml

@@ -0,0 +1,16 @@
+---
+java_keystore_certs:
+  - name: cert
+    commonName: example.com
+  - name: cert-pw
+    passphrase: hunter2
+    commonName: example.com
+
+java_keystore_new_certs:
+  - name: cert2
+    keyname: cert
+    commonName: example.org
+  - name: cert2-pw
+    keyname: cert-pw
+    passphrase: hunter2
+    commonName: example.org

tests/integration/targets/java_keystore/tasks/main.yml

@@ -4,134 +4,22 @@
 # and should not be used as examples of how to write Ansible roles #
 ####################################################################
 - when: has_java_keytool
+  connection: local
   block:
-    - name: Create private keys
-      community.crypto.openssl_privatekey:
-        path: "{{ output_dir ~ '/' ~ (item.keyname | default(item.name)) ~ '.key' }}"
-        size: 2048  # this should work everywhere
-        # The following is more efficient, but might not work everywhere:
-        # type: ECC
-        # curve: secp384r1
-        cipher: "{{ 'auto' if item.passphrase is defined else omit }}"
-        passphrase: "{{ item.passphrase | default(omit) }}"
-      loop:
-        - name: cert
-        - name: cert-pw
-          passphrase: hunter2
+    - name: Include tasks to create ssl materials on the controller
+      include_tasks: prepare.yml
 
-    - name: Create CSRs
-      community.crypto.openssl_csr:
-        path: "{{ output_dir ~ '/' ~ item.name ~ '.csr' }}"
-        privatekey_path: "{{ output_dir ~ '/' ~ (item.keyname | default(item.name)) ~ '.key' }}"
-        privatekey_passphrase: "{{ item.passphrase | default(omit) }}"
-        commonName: "{{ item.commonName }}"
-      loop:
-        - name: cert
-          commonName: example.com
-        - name: cert-pw
-          passphrase: hunter2
-          commonName: example.com
-        - name: cert2
-          keyname: cert
-          commonName: example.org
-        - name: cert2-pw
-          keyname: cert-pw
-          passphrase: hunter2
-          commonName: example.org
+- when: has_java_keytool
+  block:
+    - name: Include tasks to play with 'certificate' and 'private_key' contents
+      include_tasks: tests.yml
+      vars:
+        remote_cert: false
 
-    - name: Create certificates
-      community.crypto.x509_certificate:
-        path: "{{ output_dir ~ '/' ~ item.name ~ '.pem' }}"
-        csr_path: "{{ output_dir ~ '/' ~ item.name ~ '.csr' }}"
-        privatekey_path: "{{ output_dir ~ '/' ~ (item.keyname | default(item.name)) ~ '.key' }}"
-        privatekey_passphrase: "{{ item.passphrase | default(omit) }}"
-        provider: selfsigned
-      loop:
-        - name: cert
-          commonName: example.com
-        - name: cert-pw
-          passphrase: hunter2
-          commonName: example.com
-        - name: cert2
-          keyname: cert
-          commonName: example.org
-        - name: cert2-pw
-          keyname: cert-pw
-          passphrase: hunter2
-          commonName: example.org
+    - name: Include tasks to create ssl materials on the remote host
+      include_tasks: prepare.yml
 
-    - name: Create a Java key store for the given certificates (check mode)
-      community.general.java_keystore: &create_key_store_data
-        name: example
-        certificate: "{{ lookup('file', output_dir ~ '/' ~ item.name ~ '.pem') }}"
-        private_key: "{{ lookup('file', output_dir ~ '/' ~ (item.keyname | default(item.name)) ~ '.key') }}"
-        private_key_passphrase: "{{ item.passphrase | default(omit) }}"
-        password: changeit
-        dest: "{{ output_dir ~ '/' ~ (item.keyname | default(item.name)) ~ '.jks' }}"
-      loop: &create_key_store_loop
-        - name: cert
-        - name: cert-pw
-          passphrase: hunter2
-      check_mode: yes
-      register: result_check
-
-    - name: Create a Java key store for the given certificates
-      community.general.java_keystore: *create_key_store_data
-      loop: *create_key_store_loop
-      register: result
-
-    - name: Create a Java key store for the given certificates (idempotency, check mode)
-      community.general.java_keystore: *create_key_store_data
-      loop: *create_key_store_loop
-      check_mode: yes
-      register: result_idem_check
-
-    - name: Create a Java key store for the given certificates (idempotency)
-      community.general.java_keystore: *create_key_store_data
-      loop: *create_key_store_loop
-      register: result_idem
-
-    - name: Create a Java key store for the given certificates (certificate changed, check mode)
-      community.general.java_keystore: *create_key_store_data
-      loop: &create_key_store_loop_new_certs
-        - name: cert2
-          keyname: cert
-        - name: cert2-pw
-          keyname: cert-pw
-          passphrase: hunter2
-      check_mode: yes
-      register: result_change_check
-
-    - name: Create a Java key store for the given certificates (certificate changed)
-      community.general.java_keystore: *create_key_store_data
-      loop: *create_key_store_loop_new_certs
-      register: result_change
-
-    - name: Create a Java key store for the given certificates (password changed, check mode)
-      community.general.java_keystore:
-        <<: *create_key_store_data
-        password: hunter2
-      loop: *create_key_store_loop_new_certs
-      check_mode: yes
-      register: result_pw_change_check
-      when: false  # FIXME: module currently crashes
-
-    - name: Create a Java key store for the given certificates (password changed)
-      community.general.java_keystore:
-        <<: *create_key_store_data
-        password: hunter2
-      loop: *create_key_store_loop_new_certs
-      register: result_pw_change
-      when: false  # FIXME: module currently crashes
-
-    - name: Validate results
-      assert:
-        that:
-          - result is changed
-          - result_check is changed
-          - result_idem is not changed
-          - result_idem_check is not changed
-          - result_change is changed
-          - result_change_check is changed
-          # - result_pw_change is changed        # FIXME: module currently crashes
-          # - result_pw_change_check is changed  # FIXME: module currently crashes
+    - name: Include tasks to play with 'certificate_path' and 'private_key_path' locations
+      include_tasks: tests.yml
+      vars:
+        remote_cert: true

tests/integration/targets/java_keystore/tasks/prepare.yml

@@ -0,0 +1,33 @@
+---
+- name: Create test directory
+  ansible.builtin.file:
+    path: "{{ output_dir }}"
+    state: directory
+
+- name: Create private keys
+  community.crypto.openssl_privatekey:
+    path: "{{ output_dir ~ '/' ~ (item.keyname | default(item.name)) ~ '.key' }}"
+    size: 2048  # this should work everywhere
+    # The following is more efficient, but might not work everywhere:
+    # type: ECC
+    # curve: secp384r1
+    cipher: "{{ 'auto' if item.passphrase is defined else omit }}"
+    passphrase: "{{ item.passphrase | default(omit) }}"
+  loop: "{{ java_keystore_certs }}"
+
+- name: Create CSRs
+  community.crypto.openssl_csr:
+    path: "{{ output_dir ~ '/' ~ item.name ~ '.csr' }}"
+    privatekey_path: "{{ output_dir ~ '/' ~ (item.keyname | default(item.name)) ~ '.key' }}"
+    privatekey_passphrase: "{{ item.passphrase | default(omit) }}"
+    commonName: "{{ item.commonName }}"
+  loop: "{{ java_keystore_certs + java_keystore_new_certs }}"
+
+- name: Create certificates
+  community.crypto.x509_certificate:
+    path: "{{ output_dir ~ '/' ~ item.name ~ '.pem' }}"
+    csr_path: "{{ output_dir ~ '/' ~ item.name ~ '.csr' }}"
+    privatekey_path: "{{ output_dir ~ '/' ~ (item.keyname | default(item.name)) ~ '.key' }}"
+    privatekey_passphrase: "{{ item.passphrase | default(omit) }}"
+    provider: selfsigned
+  loop: "{{ java_keystore_certs + java_keystore_new_certs }}"

tests/integration/targets/java_keystore/tasks/tests.yml

@@ -0,0 +1,123 @@
+---
+- name: Create test directory
+  ansible.builtin.file:
+    path: "{{ output_dir }}"
+    state: directory
+
+- name: Ensure the Java keystore does not exist (cleanup between tests)
+  ansible.builtin.file:
+    path: "{{ output_dir ~ '/' ~ item.name ~ '.jks' }}"
+    state: absent
+  loop: "{{ java_keystore_certs }}"
+  loop_control:
+    label: "{{ output_dir ~ '/' ~ item.name ~ '.jks' }}"
+
+
+- name: Create a Java keystore for the given ({{ 'remote' if remote_cert else 'local' }}) certificates (check mode)
+  community.general.java_keystore: &java_keystore_params
+    name: example
+    dest: "{{ output_dir ~ '/' ~ (item.keyname | d(item.name)) ~ '.jks' }}"
+    certificate: "{{ omit if remote_cert else lookup('file', output_dir ~ '/' ~ item.name ~ '.pem') }}"
+    private_key: "{{ omit if remote_cert else lookup('file', output_dir ~ '/' ~ (item.keyname | d(item.name)) ~ '.key') }}"
+    certificate_path: "{{ omit if not remote_cert else output_dir ~ '/' ~ item.name ~ '.pem' }}"
+    private_key_path: "{{ omit if not remote_cert else output_dir ~ '/' ~ (item.keyname | d(item.name)) ~ '.key' }}"
+    private_key_passphrase: "{{ item.passphrase | d(omit) }}"
+    password: changeit
+  loop: "{{ java_keystore_certs }}"
+  check_mode: yes
+  register: result_check
+
+- name: Create a Java keystore for the given certificates
+  community.general.java_keystore: *java_keystore_params
+  loop: "{{ java_keystore_certs }}"
+  register: result
+
+
+- name: Create a Java keystore for the given certificates (idempotency, check mode)
+  community.general.java_keystore: *java_keystore_params
+  loop: "{{ java_keystore_certs }}"
+  check_mode: yes
+  register: result_idem_check
+
+- name: Create a Java keystore for the given certificates (idempotency)
+  community.general.java_keystore: *java_keystore_params
+  loop: "{{ java_keystore_certs }}"
+  register: result_idem
+
+
+- name: Create a Java keystore for the given certificates (certificate changed, check mode)
+  community.general.java_keystore: *java_keystore_params
+  loop: "{{ java_keystore_new_certs }}"
+  check_mode: yes
+  register: result_change_check
+
+- name: Create a Java keystore for the given certificates (certificate changed)
+  community.general.java_keystore: *java_keystore_params
+  loop: "{{ java_keystore_new_certs }}"
+  register: result_change
+
+
+- name: Create a Java keystore for the given certificates (alias changed, check mode)
+  community.general.java_keystore:
+    <<: *java_keystore_params
+    name: foobar
+  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:
+    <<: *java_keystore_params
+    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)
+  community.general.java_keystore:
+    <<: *java_keystore_params
+    name: foobar
+    password: hunter2
+  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:
+    <<: *java_keystore_params
+    name: foobar
+    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:
+    path: "{{ output_dir ~ '/' ~ item.name ~ '.pem' }}"
+    state: file
+  loop: "{{ java_keystore_certs + java_keystore_new_certs }}"
+  when: remote_cert
+
+- name: Check that the remote private keys have not been removed
+  ansible.builtin.file:
+    path: "{{ output_dir ~ '/' ~ item.name ~ '.key' }}"
+    state: file
+  loop: "{{ java_keystore_certs }}"
+  when: remote_cert
+
+- name: Validate results
+  assert:
+    that:
+      - result is changed
+      - result_check is changed
+      - result_idem is not changed
+      - 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