osx_defaults: refactor (#52452)

* Code refactor
* Documentation update
* Add 'list' parameter
* Example update
* Testcase for osx_defaults

Fixes: #29329

Signed-off-by: Abhijeet Kasurde <akasurde@redhat.com>

lib/ansible/modules/system/osx_defaults.py

@@ -2,14 +2,18 @@
 # -*- coding: utf-8 -*-
 
 # Copyright: (c) 2014, GeekChimp - Franck Nijhof <franck@geekchimp.com>
+# Copyright: (c) 2019, Ansible project
+# Copyright: (c) 2019, Abhijeet Kasurde <akasurde@redhat.com>
 # 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
 __metaclass__ = type
 
-ANSIBLE_METADATA = {'metadata_version': '1.1',
+ANSIBLE_METADATA = {
+    'metadata_version': '1.1',
     'status': ['stableinterface'],
-                    'supported_by': 'community'}
+    'supported_by': 'community'
+}
 
 DOCUMENTATION = r'''
 ---
@@ -59,8 +63,10 @@ options:
   state:
     description:
       - The state of the user defaults.
+      - If set to C(list) will query the given parameter specified by C(key). Returns 'null' is nothing found or mis-spelled.
+      - C(list) added in version 2.8.
     type: str
-    choices: [ absent, present ]
+    choices: [ absent, list, present ]
     default: present
   path:
     description:
@@ -82,7 +88,7 @@ EXAMPLES = r'''
 - osx_defaults:
     domain: NSGlobalDomain
     key: AppleMeasurementUnits
-    type: str
+    type: string
     value: Centimeters
     state: present
 
@@ -95,7 +101,7 @@ EXAMPLES = r'''
 
 - osx_defaults:
     key: AppleMeasurementUnits
-    type: str
+    type: string
     value: Centimeters
 
 - osx_defaults:
@@ -111,7 +117,7 @@ EXAMPLES = r'''
     state: absent
 '''
 
-import datetime
+from datetime import datetime
 import re
 
 from ansible.module_utils.basic import AnsibleModule
@@ -120,7 +126,8 @@ from ansible.module_utils.six import binary_type, text_type
 
 # exceptions --------------------------------------------------------------- {{{
 class OSXDefaultsException(Exception):
-    pass
+    def __init__(self, msg):
+        self.message = msg
 
 
 # /exceptions -------------------------------------------------------------- }}}
@@ -130,16 +137,19 @@ class OSXDefaults(object):
     """ Class to manage Mac OS user defaults """
 
     # init ---------------------------------------------------------------- {{{
+    def __init__(self, module):
         """ Initialize this module. Finds 'defaults' executable and preps the parameters """
-
-    def __init__(self, **kwargs):
-
         # Initial var for storing current defaults value
         self.current_value = None
-
+        self.module = module
-        # Just set all given parameters
+        self.domain = module.params['domain']
-        for key, val in kwargs.items():
+        self.host = module.params['host']
-            setattr(self, key, val)
+        self.key = module.params['key']
+        self.type = module.params['type']
+        self.array_add = module.params['array_add']
+        self.value = module.params['value']
+        self.state = module.params['state']
+        self.path = module.params['path']
 
         # Try to find the defaults executable
         self.executable = self.module.get_bin_path(
@@ -151,23 +161,19 @@ class OSXDefaults(object):
         if not self.executable:
             raise OSXDefaultsException("Unable to locate defaults executable.")
 
-        # When state is present, we require a parameter
-        if self.state == "present" and self.value is None:
-            raise OSXDefaultsException("Missing value parameter")
-
         # Ensure the value is the correct type
+        if self.state != 'absent':
             self.value = self._convert_type(self.type, self.value)
 
     # /init --------------------------------------------------------------- }}}
 
     # tools --------------------------------------------------------------- {{{
+    @staticmethod
+    def _convert_type(data_type, value):
         """ Converts value to given type """
-
+        if data_type == "string":
-    def _convert_type(self, type, value):
-
-        if type == "string":
             return str(value)
-        elif type in ["bool", "boolean"]:
+        elif data_type in ["bool", "boolean"]:
             if isinstance(value, (binary_type, text_type)):
                 value = value.lower()
             if value in [True, 1, "true", "1", "yes"]:
@@ -175,33 +181,32 @@ class OSXDefaults(object):
             elif value in [False, 0, "false", "0", "no"]:
                 return False
             raise OSXDefaultsException("Invalid boolean value: {0}".format(repr(value)))
-        elif type == "date":
+        elif data_type == "date":
             try:
-                return datetime.datetime.strptime(value.split("+")[0].strip(), "%Y-%m-%d %H:%M:%S")
+                return datetime.strptime(value.split("+")[0].strip(), "%Y-%m-%d %H:%M:%S")
             except ValueError:
                 raise OSXDefaultsException(
                     "Invalid date value: {0}. Required format yyy-mm-dd hh:mm:ss.".format(repr(value))
                 )
-        elif type in ["int", "integer"]:
+        elif data_type in ["int", "integer"]:
             if not str(value).isdigit():
                 raise OSXDefaultsException("Invalid integer value: {0}".format(repr(value)))
             return int(value)
-        elif type == "float":
+        elif data_type == "float":
             try:
                 value = float(value)
             except ValueError:
                 raise OSXDefaultsException("Invalid float value: {0}".format(repr(value)))
             return value
-        elif type == "array":
+        elif data_type == "array":
             if not isinstance(value, list):
                 raise OSXDefaultsException("Invalid value. Expected value to be an array")
             return value
 
-        raise OSXDefaultsException('Type is not supported: {0}'.format(type))
+        raise OSXDefaultsException('Type is not supported: {0}'.format(data_type))
-
-    """ Returns a normalized list of commandline arguments based on the "host" attribute """
 
     def _host_args(self):
+        """ Returns a normalized list of commandline arguments based on the "host" attribute """
         if self.host is None:
             return []
         elif self.host == 'currentHost':
@@ -209,16 +214,13 @@ class OSXDefaults(object):
         else:
             return ['-host', self.host]
 
-    """ Returns a list containing the "defaults" executable and any common base arguments """
-
     def _base_command(self):
+        """ Returns a list containing the "defaults" executable and any common base arguments """
         return [self.executable] + self._host_args()
 
-    """ Converts array output from defaults to an list """
-
     @staticmethod
     def _convert_defaults_str_to_list(value):
-
+        """ Converts array output from defaults to an list """
         # Split output of defaults. Every line contains a value
         value = value.splitlines()
 
@@ -234,9 +236,8 @@ class OSXDefaults(object):
     # /tools -------------------------------------------------------------- }}}
 
     # commands ------------------------------------------------------------ {{{
-    """ Reads value of this domain & key from defaults """
-
     def read(self):
+        """ Reads value of this domain & key from defaults """
         # First try to find out the type
         rc, out, err = self.module.run_command(self._base_command() + ["read-type", self.domain, self.key])
 
@@ -246,10 +247,10 @@ class OSXDefaults(object):
 
         # If the RC is not 0, then terrible happened! Ooooh nooo!
         if rc != 0:
-            raise OSXDefaultsException("An error occurred while reading key type from defaults: " + out)
+            raise OSXDefaultsException("An error occurred while reading key type from defaults: %s" % out)
 
         # Ok, lets parse the type from output
-        type = out.strip().replace('Type is ', '')
+        data_type = out.strip().replace('Type is ', '')
 
         # Now get the current value
         rc, out, err = self.module.run_command(self._base_command() + ["read", self.domain, self.key])
@@ -259,19 +260,17 @@ class OSXDefaults(object):
 
         # An non zero RC at this point is kinda strange...
         if rc != 0:
-            raise OSXDefaultsException("An error occurred while reading key value from defaults: " + out)
+            raise OSXDefaultsException("An error occurred while reading key value from defaults: %s" % out)
 
         # Convert string to list when type is array
-        if type == "array":
+        if data_type == "array":
             out = self._convert_defaults_str_to_list(out)
 
         # Store the current_value
-        self.current_value = self._convert_type(type, out)
+        self.current_value = self._convert_type(data_type, out)
-
-    """ Writes value to this domain & key to defaults """
 
     def write(self):
-
+        """ Writes value to this domain & key to defaults """
         # We need to convert some values so the defaults commandline understands it
         if isinstance(self.value, bool):
             if self.value:
@@ -282,7 +281,7 @@ class OSXDefaults(object):
             value = str(self.value)
         elif self.array_add and self.current_value is not None:
             value = list(set(self.value) - set(self.current_value))
-        elif isinstance(self.value, datetime.datetime):
+        elif isinstance(self.value, datetime):
             value = self.value.strftime('%Y-%m-%d %H:%M:%S')
         else:
             value = self.value
@@ -298,14 +297,13 @@ class OSXDefaults(object):
         rc, out, err = self.module.run_command(self._base_command() + ['write', self.domain, self.key, '-' + self.type] + value)
 
         if rc != 0:
-            raise OSXDefaultsException('An error occurred while writing value to defaults: ' + out)
+            raise OSXDefaultsException('An error occurred while writing value to defaults: %s' % out)
-
-    """ Deletes defaults key from domain """
 
     def delete(self):
+        """ Deletes defaults key from domain """
         rc, out, err = self.module.run_command(self._base_command() + ['delete', self.domain, self.key])
         if rc != 0:
-            raise OSXDefaultsException("An error occurred while deleting key from defaults: " + out)
+            raise OSXDefaultsException("An error occurred while deleting key from defaults: %s" % out)
 
     # /commands ----------------------------------------------------------- }}}
 
@@ -317,6 +315,9 @@ class OSXDefaults(object):
         # Get the current value from defaults
         self.read()
 
+        if self.state == 'list':
+            self.module.exit_json(key=self.key, value=self.current_value)
+
         # Handle absent state
         if self.state == "absent":
             if self.current_value is None:
@@ -329,7 +330,7 @@ class OSXDefaults(object):
         # There is a type mismatch! Given type does not match the type in defaults
         value_type = type(self.value)
         if self.current_value is not None and not isinstance(self.current_value, value_type):
-            raise OSXDefaultsException("Type mismatch. Type in defaults: " + type(self.current_value).__name__)
+            raise OSXDefaultsException("Type mismatch. Type in defaults: %s" % type(self.current_value).__name__)
 
         # Current value matches the given value. Nothing need to be done. Arrays need extra care
         if self.type == "array" and self.current_value is not None and not self.array_add and \
@@ -363,26 +364,18 @@ def main():
             type=dict(type='str', default='string', choices=['array', 'bool', 'boolean', 'date', 'float', 'int', 'integer', 'string']),
             array_add=dict(type='bool', default=False),
             value=dict(type='raw'),
-            state=dict(type='str', default='present', choices=['absent', 'present']),
+            state=dict(type='str', default='present', choices=['absent', 'list', 'present']),
             path=dict(type='str', default='/usr/bin:/usr/local/bin'),
         ),
         supports_check_mode=True,
+        required_if=(
+            ('state', 'present', ['value']),
+        ),
     )
 
-    domain = module.params['domain']
-    host = module.params['host']
-    key = module.params['key']
-    type = module.params['type']
-    array_add = module.params['array_add']
-    value = module.params['value']
-    state = module.params['state']
-    path = module.params['path']
-
     try:
-        defaults = OSXDefaults(module=module, domain=domain, host=host, key=key, type=type,
+        defaults = OSXDefaults(module=module)
-                               array_add=array_add, value=value, state=state, path=path)
+        module.exit_json(changed=defaults.run())
-        changed = defaults.run()
-        module.exit_json(changed=changed)
     except OSXDefaultsException as e:
         module.fail_json(msg=e.message)
 

test/integration/targets/osx_defaults/aliases

@@ -0,0 +1,5 @@
+shippable/posix/group1
+skip/freebsd
+skip/rhel
+skip/docker
+skip/rhel8.0

test/integration/targets/osx_defaults/tasks/main.yml

@@ -0,0 +1,204 @@
+# Test code for the osx_defaults module.
+# Copyright: (c) 2019, Abhijeet Kasurde <akasurde@redhat.com>
+# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
+---
+- name: Check if name is required for present
+  osx_defaults:
+    domain: NSGlobalDomain
+    key: AppleMeasurementUnits
+    type: string
+    state: present
+  register: missing_value
+  ignore_errors: yes
+
+- name: Test if state and value are required together
+  assert:
+    that:
+    - "'following are missing: value' in '{{ missing_value['msg'] }}'"
+
+- name: Change value of AppleMeasurementUnits to centimeter in check_mode
+  osx_defaults:
+    domain: NSGlobalDomain
+    key: AppleMeasurementUnits
+    type: string
+    value: Centimeter
+    state: present
+  register: measure_task_check_mode
+  check_mode: yes
+
+- name: Test if AppleMeasurementUnits value is changed to Centimeters in check_mode
+  assert:
+    that:
+    - measure_task_check_mode.changed
+
+- name: Find the current value of AppleMeasurementUnits
+  osx_defaults:
+    domain: NSGlobalDomain
+    key: AppleMeasurementUnits
+    state: list
+  register: apple_measure_value
+
+- debug:
+    msg: "{{ apple_measure_value['value'] }}"
+
+- set_fact:
+    new_value: "Centimeters"
+  when: apple_measure_value['value'] == 'Inches' or apple_measure_value['value'] == None
+
+- set_fact:
+    new_value: "Inches"
+  when: apple_measure_value['value'] == 'Centimeters'
+
+- name: Change value of AppleMeasurementUnits to {{ new_value }}
+  osx_defaults:
+    domain: NSGlobalDomain
+    key: AppleMeasurementUnits
+    type: string
+    value: "{{ new_value }}"
+    state: present
+  register: change_value
+
+- name: Test if AppleMeasurementUnits value is changed to {{ new_value }}
+  assert:
+    that:
+    - change_value.changed
+
+- name: Again change value of AppleMeasurementUnits to {{ new_value }}
+  osx_defaults:
+    domain: NSGlobalDomain
+    key: AppleMeasurementUnits
+    type: string
+    value: "{{ new_value }}"
+    state: present
+  register: change_value
+
+- name: Again test if AppleMeasurementUnits value is not changed to {{ new_value }}
+  assert:
+    that:
+    - not change_value.changed
+
+- name: Check a fake setting for delete operation
+  osx_defaults:
+    domain: com.ansible.fake_value
+    key: ExampleKeyToRemove
+    state: list
+  register: list_fake_value
+
+- debug:
+    msg: "{{ list_fake_value }}"
+
+- name: Check if fake value is listed
+  assert:
+    that:
+      - not list_fake_value.changed
+
+- name: Create a fake setting for delete operation
+  osx_defaults:
+    domain: com.ansible.fake_value
+    key: ExampleKeyToRemove
+    state: present
+    value: sample
+  register: present_fake_value
+
+- debug:
+    msg: "{{ present_fake_value }}"
+
+- name: Check if fake is created
+  assert:
+    that:
+      - present_fake_value.changed
+  when: present_fake_value.changed
+
+- name: List a fake setting
+  osx_defaults:
+    domain: com.ansible.fake_value
+    key: ExampleKeyToRemove
+    state: list
+  register: list_fake
+
+- debug:
+    msg: "{{ list_fake }}"
+
+- name: Delete a fake setting
+  osx_defaults:
+    domain: com.ansible.fake_value
+    key: ExampleKeyToRemove
+    state: absent
+  register: absent_task
+
+- debug:
+    msg: "{{ absent_task }}"
+
+- name: Check if fake setting is deleted
+  assert:
+    that:
+      - absent_task.changed
+  when: present_fake_value.changed
+
+- name: Try deleting a fake setting again
+  osx_defaults:
+    domain: com.ansible.fake_value
+    key: ExampleKeyToRemove
+    state: absent
+  register: absent_task
+
+- debug:
+    msg: "{{ absent_task }}"
+
+- name: Check if fake setting is not deleted
+  assert:
+    that:
+      - not absent_task.changed
+
+- name: Delete operation in check_mode
+  osx_defaults:
+    domain: com.ansible.fake_value
+    key: ExampleKeyToRemove
+    state: absent
+  register: absent_check_mode_task
+  check_mode: yes
+
+- debug:
+    msg: "{{ absent_check_mode_task }}"
+
+- name: Check delete operation with check mode
+  assert:
+    that:
+      - not absent_check_mode_task.changed
+
+
+- name: Use different data types and check if it works with them
+  osx_defaults:
+    domain: com.ansible.fake_values
+    key: "{{ item.key }}"
+    type: "{{ item.type }}"
+    value: "{{ item.value }}"
+    state: present
+  with_items: &data_type
+    - { type: 'int', value: 1, key: 'sample_int'}
+    - { type: 'integer', value: 1, key: 'sample_int_2'}
+    - { type: 'bool', value: True, key: 'sample_bool'}
+    - { type: 'boolean', value: True, key: 'sample_bool_2'}
+    - { type: 'date', value: "2019-02-19 10:10:10", key: 'sample_date'}
+    - { type: 'float', value: 1.2, key: 'sample_float'}
+    - { type: 'string', value: 'sample', key: 'sample_string'}
+    - { type: 'array', value: ['1', '2'], key: 'sample_array'}
+  register: test_data_types
+
+- assert:
+    that: "{{ item.changed }}"
+  with_items: "{{ test_data_types.results }}"
+
+- name: Use different data types and delete them
+  osx_defaults:
+    domain: com.ansible.fake_values
+    key: "{{ item.key }}"
+    value: "{{ item.value }}"
+    type: "{{ item.type }}"
+    state: absent
+  with_items: *data_type
+  register: test_data_types
+
+- assert:
+    that: "{{ item.changed }}"
+  with_items: "{{ test_data_types.results }}"