Add diff mode support to hashivault_pki_role

ansible/module_utils/hashivault.py

@@ -399,3 +399,22 @@ def is_state_changed(desired_state, current_state, ignore=None):
     :rtype: bool
     """
     return(len(get_keys_updated(desired_state, current_state)) > 0)
+
+def parse_duration(duration, fallback=None):
+    if isinstance(duration, int):
+        return duration
+    elif not isinstance(duration, str):
+        return fallback
+
+    if duration.endswith('d'):
+        return int(duration[:-1]) * 60 * 60 * 24
+    if duration.endswith('h'):
+        return int(duration[:-1]) * 60 * 60
+    if duration.endswith('m'):
+        return int(duration[:-1]) * 60
+    if duration.endswith('s'):
+        return int(duration[:-1])
+    if duration != "":
+        return int(duration)
+
+    return fallback

ansible/modules/hashivault/hashivault_pki_role.py

@@ -1,11 +1,13 @@
 #!/usr/bin/python
 # -*- coding: utf-8 -*-
+import copy
+
 from ansible.module_utils.hashivault import hashivault_argspec
 from ansible.module_utils.hashivault import hashivault_auth_client
 from ansible.module_utils.hashivault import hashivault_init
 from ansible.module_utils.hashivault import hashivault_normalize_from_doc
 from ansible.module_utils.hashivault import hashiwrapper
-from ansible.module_utils.hashivault import is_state_changed
+from ansible.module_utils.hashivault import parse_duration
 
 ANSIBLE_METADATA = {'status': ['preview'], 'supported_by': 'community', 'version': '1.1'}
 DOCUMENTATION = r'''
@@ -25,7 +27,7 @@
         description:
             - location where secrets engine is mounted. also known as path
     name:
-        recuired: true
+        required: true
         description:
             - Specifies the name of the role to create.
     role_file:
@@ -358,6 +360,24 @@ def hashivault_pki_role(module):
         except Exception as e:
             return e.args[0]
 
+    # For EC and ED25519 this field is ignored and leads to misleading diff.
+    if desired_state.get("key_type") in ("ed25519", "ec"):
+        desired_state.pop("signature_bits")
+
+    # Normalize some keys. This is a quirk of the vault api that it
+    # expects a different data format in the PUT/POST endpoint than
+    # it returns in the GET endpoint.
+    # Thus we'll keep desired_state_comp for the diff purposes and use
+    # desired_state as the actual params to be POSTed
+    desired_state_comp = copy.deepcopy(desired_state)
+
+    if desired_state_comp.get('ttl'):
+        desired_state_comp['ttl'] = parse_duration(desired_state_comp['ttl'])
+    if desired_state_comp.get('max_ttl'):
+        desired_state_comp['max_ttl'] = parse_duration(desired_state_comp['max_ttl'])
+    if desired_state_comp.get('not_before_duration'):
+        desired_state_comp['not_before_duration'] = parse_duration(desired_state_comp['not_before_duration'])
+
     changed = False
     try:
         current_state = client.secrets.pki.read_role(name=name, mount_point=mount_point).get('data')
@@ -369,18 +389,33 @@ def hashivault_pki_role(module):
     if (exists and state == 'absent') or (not exists and state == 'present'):
         changed = True
 
-    # compare current_state to desired_state
-    if exists and state == 'present' and not changed:
-        changed = is_state_changed(desired_state, current_state)
+    # compare current_state to desired_state_comp
+    if exists and state == "present" and not changed:
+        # Update all keys not present in the desired_state_comp with data from the
+        # current_state, to ensure a proper diff output.
+        for key in current_state:
+            if desired_state_comp.get(key) is None:
+                desired_state_comp[key] = current_state[key]
+
+        changed = desired_state_comp != current_state
 
     # make the changes!
     if changed and state == 'present' and not module.check_mode:
         client.secrets.pki.create_or_update_role(name=name, mount_point=mount_point, extra_params=desired_state)
 
-    elif changed and state == 'absent' and not module.check_mode:
-        client.secrets.pki.delete_role(name=name, mount_point=mount_point)
+    elif changed and state == 'absent':
+        if not module.check_mode:
+            client.secrets.pki.delete_role(name=name, mount_point=mount_point)
+        # after deleting it the item is no more
+        desired_state_comp = {}
 
-    return {'changed': changed}
+    return {
+        "changed": changed,
+        "diff": {
+            "before": current_state,
+            "after": desired_state_comp,
+        },
+    }
 
 
 if __name__ == '__main__':

ansible/modules/hashivault/hashivault_secret_engine.py

@@ -5,6 +5,7 @@
 from ansible.module_utils.hashivault import hashivault_auth_client
 from ansible.module_utils.hashivault import hashivault_init
 from ansible.module_utils.hashivault import hashiwrapper
+from ansible.module_utils.hashivault import parse_duration
 
 DEFAULT_TTL = 2764800
 ANSIBLE_METADATA = {'status': ['stableinterface'], 'supported_by': 'community', 'version': '1.1'}
@@ -125,26 +126,6 @@ def main():
         module.exit_json(**result)
 
 
-def parse_duration(duration):
-    if isinstance(duration, int):
-        return duration
-    elif not isinstance(duration, str):
-        return DEFAULT_TTL
-
-    if duration.endswith('d'):
-        return int(duration[:-1]) * 60 * 60 * 24
-    if duration.endswith('h'):
-        return int(duration[:-1]) * 60 * 60
-    if duration.endswith('m'):
-        return int(duration[:-1]) * 60
-    if duration.endswith('s'):
-        return int(duration[:-1])
-    if duration != "":
-        return int(duration)
-
-    return DEFAULT_TTL
-
-
 @hashiwrapper
 def hashivault_secret_engine(module):
     params = module.params
@@ -154,9 +135,10 @@ def hashivault_secret_engine(module):
     description = params.get('description')
     config = params.get('config')
     if 'default_lease_ttl' in config:
-        config['default_lease_ttl'] = parse_duration(config['default_lease_ttl'])
+        config['default_lease_ttl'] = parse_duration(config['default_lease_ttl'], DEFAULT_TTL)
     if 'max_lease_ttl' in config:
-        config['max_lease_ttl'] = parse_duration(config['max_lease_ttl'])
+        config['max_lease_ttl'] = parse_duration(config['max_lease_ttl'],
+                                                 DEFAULT_TTL)
     if params.get('state') in ['present', 'enabled']:
         state = 'enabled'
     else:

functional/test_pki.yml

@@ -28,7 +28,7 @@
 
     - debug:
         msg: "mount_root:\t{{ mount_root }}\nmount_inter:\t{{ mount_inter }}\nrole_name:\t{{ role }}"
-    - name: Enabele PKI secrets engine
+    - name: Enable PKI secrets engine
       hashivault_secret_engine:
         name: "{{mount_root}}"
         backend: "pki"
@@ -56,7 +56,7 @@
           - response.rc == 0
           - response.changed == False
 
-    - name: Enabele PKI secrets engine
+    - name: Enable PKI secrets engine
       hashivault_secret_engine:
         name: "{{mount_inter}}"
         backend: "pki"
@@ -253,15 +253,80 @@
         that:
           - response.rc == 0
           - response.changed == True
+    - name: Create/Update Role check_mode
+      hashivault_pki_role:
+        mount_point: "{{mount_inter}}"
+        name: "{{role}}"
+      check_mode: true
+      register: response
+    - assert:
+        that:
+          - response.rc == 0
+          - response.changed == False
+          - response.diff.before == response.diff.after
+    - name: Create/Update Role
+      hashivault_pki_role:
+        mount_point: "{{mount_inter}}"
+        name: "{{role}}"
+      register: response
+    - assert:
+        that:
+          - response.rc == 0
+          - response.changed == False
+          - response.diff.before == response.diff.after
+    - name: Create/Update Role
+      hashivault_pki_role:
+        mount_point: "{{mount_inter}}"
+        name: "{{role}}"
+        config:
+          max_ttl: "153"
+          ttl: "150"
+          not_before_duration: "45s"
+      register: response
+    - assert:
+        that:
+          - response.rc == 0
+          - response.changed == True
+          - response.diff.before.max_ttl != response.diff.after.max_ttl
+          - response.diff.before.ttl != response.diff.after.ttl
+          - response.diff.before.not_before_duration != response.diff.after.not_before_duration
     - name: Create/Update Role
       hashivault_pki_role:
         mount_point: "{{mount_inter}}"
         name: "{{role}}"
+        config:
+          max_ttl: "153"
+          ttl: "150"
+          not_before_duration: "45s"
       register: response
     - assert:
         that:
           - response.rc == 0
           - response.changed == False
+          - response.diff.before == response.diff.after
+    - name: Create/Update Role check_mode
+      hashivault_pki_role:
+        mount_point: "{{mount_inter}}"
+        name: "{{role}}"
+        config:
+          allow_bare_domains: True
+          allow_subdomains: True
+          allow_any_name: True
+          not_before_duration: "15s"
+      check_mode: true
+      register: response
+    - assert:
+        that:
+          - response.rc == 0
+          - response.changed == True
+          - response.diff.before != response.diff.after
+          - response.diff.before.allow_bare_domains == False
+          - response.diff.after.allow_bare_domains == True
+          - response.diff.before.allow_subdomains == False
+          - response.diff.after.allow_subdomains == True
+          - response.diff.before.allow_any_name == False
+          - response.diff.after.allow_any_name == True
+          - response.diff.before.not_before_duration != response.diff.after.not_before_duration
     - name: Create/Update Role
       hashivault_pki_role:
         mount_point: "{{mount_inter}}"
@@ -276,6 +341,29 @@
         that:
           - response.rc == 0
           - response.changed == True
+          - response.diff.before != response.diff.after
+          - response.diff.before.allow_bare_domains == False
+          - response.diff.after.allow_bare_domains == True
+          - response.diff.before.allow_subdomains == False
+          - response.diff.after.allow_subdomains == True
+          - response.diff.before.allow_any_name == False
+          - response.diff.after.allow_any_name == True
+          - response.diff.before.not_before_duration != response.diff.after.not_before_duration
+    - name: Create/Update Role, no diff
+      hashivault_pki_role:
+        mount_point: "{{mount_inter}}"
+        name: "{{role}}"
+        config:
+          allow_bare_domains: True
+          allow_subdomains: True
+          allow_any_name: True
+          not_before_duration: "15s"
+      register: response
+    - assert:
+        that:
+          - response.rc == 0
+          - response.changed == False
+          - response.diff.before == response.diff.after
 
     - name: List Roles
       hashivault_pki_role_list: