Repository: ansible/awx
Pull request: Prevent exception for Non value
##### SUMMARY
<!--- Describe the change, including rationale and design decisions -->
Fixes #7267
<!---
If you are fixing an existing issue, please include "related #nnn" in your
commit message and your description; but you should still explain what
the change does.
-->

##### ISSUE TYPE
<!--- Pick one below and delete the rest: -->
 - Bugfix Pull Request

##### COMPONENT NAME
<!--- Name of the module/plugin/module/task -->
 - Collection

##### AWX VERSION
<!--- Paste verbatim output from `make VERSION` between quotes below -->
```
awx: 11.2.0
```


##### ADDITIONAL INFORMATION
<!---
Include additional information to help people understand the change here.
For bugs that don't have a linked bug report, a step-by-step reproduction
of the problem is helpful.
  -->

<!--- Paste verbatim command output below, e.g. before and after your change -->
```
This can be tested with the following task:
- name: Handle an omit value
  tower_settings:
    name: AWX_PROOT_BASE_PATH
    value: '{{ junk_var | default(omit) }}'

Without the change you get the stack.
With the new change it now long stacks but will return something like:

fatal: [localhost]: FAILED! => {"changed": false, "msg": "Unable to update settings, see response", "response": {"json": {"AWX_PROOT_BASE_PATH": ["This field may not be null."]}, "status_code": 400}}
```



Problem statement:
Tower settings fails with stacktrace - expected to exit gracefully
##### ISSUE TYPE
 - Bug Report

##### SUMMARY
Running awx.awx.settings module throws `AttributeError: 'NoneType' object has no attribute 'startswith'`

##### ENVIRONMENT
* AWX version: 11.2.0
* AWX install method: setup.sh
* Ansible version:  2.9.7
* Operating System: Tower on RHEL, Ansible on Fedora

##### STEPS TO REPRODUCE

```
---
# tasks file for ansible_tower_genie_settings- name: Update Ansible Tower Settings
  awx.awx.tower_settings:
    name:                "{{ tower_setting_item.name | default(omit) }}"
    value:               "{{ tower_setting_item.value | default(omit) }}"
    tower_config_file:   "{{ tower_config_file | default(omit) }}"
    tower_host:          "{{ tower_hostname | default(omit) }}"
    tower_password:      "{{ tower_password | default(omit) }}"
    tower_username:      "{{ tower_username | default(omit) }}"
    validate_certs:      "{{ validate_certs | default('false') }}"
  loop: "{{ tower_settings }}"
  loop_control:
    loop_var: tower_setting_item
...

```
Data:
```
---
tower_settings:
  - name: AWX_TASK_ENV
    setting: {'GIT_SSL_NO_VERIFY': 'True'}
```

##### EXPECTED RESULTS

Error should be more helpful, gracefully handled vs flat out traceback if possible.

##### ACTUAL RESULTS

```
The full traceback is:
Traceback (most recent call last):
  File "/home/kkulkarni/.ansible/tmp/ansible-tmp-1591366383.968238-878504-224766098821440/AnsiballZ_tower_settings.py", line 102, in <module>
    _ansiballz_main()
  File "/home/kkulkarni/.ansible/tmp/ansible-tmp-1591366383.968238-878504-224766098821440/AnsiballZ_tower_settings.py", line 94, in _ansiballz_main
    invoke_module(zipped_mod, temp_path, ANSIBALLZ_PARAMS)
  File "/home/kkulkarni/.ansible/tmp/ansible-tmp-1591366383.968238-878504-224766098821440/AnsiballZ_tower_settings.py", line 40, in invoke_module
    runpy.run_module(mod_name='ansible_collections.awx.awx.plugins.modules.tower_settings', init_globals=None, run_name='__main__', alter_sys=True)
  File "/usr/lib64/python3.8/runpy.py", line 206, in run_module
    return _run_module_code(code, init_globals, run_name, mod_spec)
  File "/usr/lib64/python3.8/runpy.py", line 96, in _run_module_code
    _run_code(code, mod_globals, init_globals,
  File "/usr/lib64/python3.8/runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "/tmp/ansible_awx.awx.tower_settings_payload_x13vlw6v/ansible_awx.awx.tower_settings_payload.zip/ansible_collections/awx/awx/plugins/modules/tower_settings.py", line 184, in <module>
  File "/tmp/ansible_awx.awx.tower_settings_payload_x13vlw6v/ansible_awx.awx.tower_settings_payload.zip/ansible_collections/awx/awx/plugins/modules/tower_settings.py", line 136, in main
  File "/tmp/ansible_awx.awx.tower_settings_payload_x13vlw6v/ansible_awx.awx.tower_settings_payload.zip/ansible_collections/awx/awx/plugins/modules/tower_settings.py", line 95, in coerce_type
AttributeError: 'NoneType' object has no attribute 'startswith'
failed: [localhost] (item={'name': 'AWX_TASK_ENV', 'setting': {'GIT_SSL_NO_VERIFY': 'True'}}) => {
    "ansible_loop_var": "tower_setting_item",
    "changed": false,
    "module_stderr": "Traceback (most recent call last):\n  File \"/home/kkulkarni/.ansible/tmp/ansible-tmp-1591366383.968238-878504-224766098821440/AnsiballZ_tower_settings.py\", line 102, in <module>\n    _ansiballz_main()\n  File \"/home/kkulkarni/.ansible/tmp/ansible-tmp-1591366383.968238-878504-224766098821440/AnsiballZ_tower_settings.py\", line 94, in _ansiballz_main\n    invoke_module(zipped_mod, temp_path, ANSIBALLZ_PARAMS)\n  File \"/home/kkulkarni/.ansible/tmp/ansible-tmp-1591366383.968238-878504-224766098821440/AnsiballZ_tower_settings.py\", line 40, in invoke_module\n    runpy.run_module(mod_name='ansible_collections.awx.awx.plugins.modules.tower_settings', init_globals=None, run_name='__main__', alter_sys=True)\n  File \"/usr/lib64/python3.8/runpy.py\", line 206, in run_module\n    return _run_module_code(code, init_globals, run_name, mod_spec)\n  File \"/usr/lib64/python3.8/runpy.py\", line 96, in _run_module_code\n    _run_code(code, mod_globals, init_globals,\n  File \"/usr/lib64/python3.8/runpy.py\", line 86, in _run_code\n    exec(code, run_globals)\n  File \"/tmp/ansible_awx.awx.tower_settings_payload_x13vlw6v/ansible_awx.awx.tower_settings_payload.zip/ansible_collections/awx/awx/plugins/modules/tower_settings.py\", line 184, in <module>\n  File \"/tmp/ansible_awx.awx.tower_settings_payload_x13vlw6v/ansible_awx.awx.tower_settings_payload.zip/ansible_collections/awx/awx/plugins/modules/tower_settings.py\", line 136, in main\n  File \"/tmp/ansible_awx.awx.tower_settings_payload_x13vlw6v/ansible_awx.awx.tower_settings_payload.zip/ansible_collections/awx/awx/plugins/modules/tower_settings.py\", line 95, in coerce_type\nAttributeError: 'NoneType' object has no attribute 'startswith'\n",
    "module_stdout": "",
    "msg": "MODULE FAILURE\nSee stdout/stderr for the exact error",
    "rc": 1,
    "tower_setting_item": {
        "name": "AWX_TASK_ENV",
        "setting": {
            "GIT_SSL_NO_VERIFY": "True"
        }
    }
}
```

##### ADDITIONAL INFORMATION

<!-- Include any links to sosreport, database dumps, screenshots or other
information. -->

Hints from issue discussion:
You are using module incorrecly. See documentation at the top of module source.
Either `name + value` or `settings` properties should be used.

Perhaps, you mean:
```
tower_settings:
  - name: AWX_TASK_ENV
    value: "{'GIT_SSL_NO_VERIFY': 'True'}"
```
or

```
tower_settings:
  - settings:
      AWX_TASK_ENV: "{'GIT_SSL_NO_VERIFY': 'True'}"
```

Latest devel already have a requirement for one use case, though type for input value is not explicitly checked to be string.

https://github.com/ansible/awx/blob/devel/awx_collection/plugins/modules/tower_settings.py
Marking as low, it's an incorrect invocation, but we may consider a nicer error.

Commit message: Prevent exception for Non value

Patch to review:
diff --git a/awx_collection/plugins/modules/tower_settings.py b/awx_collection/plugins/modules/tower_settings.py
index b7ecde45ef87..a66acadf44c7 100644
--- a/awx_collection/plugins/modules/tower_settings.py
+++ b/awx_collection/plugins/modules/tower_settings.py
@@ -82,6 +82,10 @@
 
 
 def coerce_type(module, value):
+    # If our value is already None we can just return directly
+    if value == None:
+        return value
+
     yaml_ish = bool((
         value.startswith('{') and value.endswith('}')
     ) or (
diff --git a/awx_collection/tests/integration/targets/tower_settings/tasks/main.yml b/awx_collection/tests/integration/targets/tower_settings/tasks/main.yml
index a02ca673de00..6cebe979e29b 100644
--- a/awx_collection/tests/integration/targets/tower_settings/tasks/main.yml
+++ b/awx_collection/tests/integration/targets/tower_settings/tasks/main.yml
@@ -74,3 +74,9 @@
 - assert:
     that:
       - "result is changed"
+
+- name: Handle an omit value
+  tower_settings:
+    name: AWX_PROOT_BASE_PATH
+    value: '{{ junk_var | default(omit) }}'
+


Code review comments:
diff_hunk: @@ -82,6 +82,10 @@
 
 
 def coerce_type(module, value):
+    # If our value is already None we can just return directly
+    if value == None:
path: awx_collection/plugins/modules/tower_settings.py
text: @user1:
if `value is None`.