From d6e94f9c6fc2583c710897afef03536d699cf07b Mon Sep 17 00:00:00 2001
From: AlanCoding <arominge@redhat.com>
Date: Thu, 19 Mar 2020 22:43:17 -0400
Subject: [PATCH] Fix regression in tower_settings module

---
 .../plugins/modules/tower_settings.py         | 40 ++++++++---
 awx_collection/test/awx/test_settings.py      | 67 +++++++++++++++++++
 2 files changed, 97 insertions(+), 10 deletions(-)
 create mode 100644 awx_collection/test/awx/test_settings.py

diff --git a/awx_collection/plugins/modules/tower_settings.py b/awx_collection/plugins/modules/tower_settings.py
index d242ea2bbb..caf3cb0e56 100644
--- a/awx_collection/plugins/modules/tower_settings.py
+++ b/awx_collection/plugins/modules/tower_settings.py
@@ -31,6 +31,8 @@ options:
     value:
       description:
         - Value to be modified for given setting.
+        - If given a non-string type, will make best effort to cast it to type API expects.
+        - For better control over types, use the C(settings) param instead.
       required: False
       type: str
     settings:
@@ -45,6 +47,8 @@ options:
       required: False
       type: str
       version_added: "3.7"
+requirements:
+  - pyyaml
 extends_documentation_fragment: awx.awx.auth
 '''
 
@@ -78,8 +82,31 @@ EXAMPLES = '''
 '''
 
 from ..module_utils.tower_api import TowerModule
-from json import loads
-import re
+
+try:
+    import yaml
+    HAS_YAML = True
+except ImportError:
+    HAS_YAML = False
+
+
+def coerce_type(module, value):
+    yaml_ish = bool((
+        value.startswith('{') and value.endswith('}')
+    ) or (
+        value.startswith('[') and value.endswith(']'))
+    )
+    if yaml_ish:
+        if not HAS_YAML:
+            module.fail_json(msg="yaml is not installed, try 'pip install pyyaml'")
+        return yaml.safe_load(value)
+    elif value.lower in ('true', 'false', 't', 'f'):
+        return {'t': True, 'f': False}[value[0].lower()]
+    try:
+        return int(value)
+    except ValueError:
+        pass
+    return value
 
 
 def main():
@@ -106,14 +133,7 @@ def main():
 
     # If we were given a name/value pair we will just make settings out of that and proceed normally
     if new_settings is None:
-        new_value = value
-        try:
-            new_value = loads(value)
-        except ValueError:
-            # JSONDecodeError only available on Python 3.5+
-            # Attempt to deal with old tower_cli array types
-            if ',' in value:
-                new_value = re.split(r",\s+", new_value)
+        new_value = coerce_type(module, value)
 
         new_settings = {name: new_value}
 
diff --git a/awx_collection/test/awx/test_settings.py b/awx_collection/test/awx/test_settings.py
new file mode 100644
index 0000000000..e39d7eaa0b
--- /dev/null
+++ b/awx_collection/test/awx/test_settings.py
@@ -0,0 +1,67 @@
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
+import pytest
+
+from awx.conf.models import Setting
+
+
+@pytest.mark.django_db
+def test_setting_flat_value(run_module, admin_user):
+    the_value = 'CN=service_account,OU=ServiceAccounts,DC=domain,DC=company,DC=org'
+    result = run_module('tower_settings', dict(
+        name='AUTH_LDAP_BIND_DN',
+        value=the_value
+    ), admin_user)
+    assert not result.get('failed', False), result.get('msg', result)
+    assert result.get('changed'), result
+
+    assert Setting.objects.get(key='AUTH_LDAP_BIND_DN').value == the_value
+
+
+@pytest.mark.django_db
+def test_setting_dict_value(run_module, admin_user):
+    the_value = {
+        'email': 'mail',
+        'first_name': 'givenName',
+        'last_name': 'surname'
+    }
+    result = run_module('tower_settings', dict(
+        name='AUTH_LDAP_USER_ATTR_MAP',
+        value=the_value
+    ), admin_user)
+    assert not result.get('failed', False), result.get('msg', result)
+    assert result.get('changed'), result
+
+    assert Setting.objects.get(key='AUTH_LDAP_USER_ATTR_MAP').value == the_value
+
+
+@pytest.mark.django_db
+def test_setting_nested_type(run_module, admin_user):
+    the_value = {
+        'email': 'mail',
+        'first_name': 'givenName',
+        'last_name': 'surname'
+    }
+    result = run_module('tower_settings', dict(
+        settings={
+            'AUTH_LDAP_USER_ATTR_MAP': the_value
+        }
+    ), admin_user)
+    assert not result.get('failed', False), result.get('msg', result)
+    assert result.get('changed'), result
+
+    assert Setting.objects.get(key='AUTH_LDAP_USER_ATTR_MAP').value == the_value
+
+
+@pytest.mark.django_db
+def test_setting_bool_value(run_module, admin_user):
+    for the_value in (True, False):
+        result = run_module('tower_settings', dict(
+            name='ACTIVITY_STREAM_ENABLED_FOR_INVENTORY_SYNC',
+            value=the_value
+        ), admin_user)
+        assert not result.get('failed', False), result.get('msg', result)
+        assert result.get('changed'), result
+
+        assert Setting.objects.get(key='ACTIVITY_STREAM_ENABLED_FOR_INVENTORY_SYNC').value is the_value