Repository: ansible/ansible
Pull request: Fix uptimerobot stacktrace on decode
`req.read()` always returns bytes whereas the json module expects
strings.

I don't know how this worked before.

##### SUMMARY

Fixes #66242 

##### ISSUE TYPE

- Bugfix Pull Request

##### COMPONENT NAME
uptimerobot module.

##### ADDITIONAL INFORMATION

Currently the uptimerobot module fails with a stacktrace because it feeds bytes to the json module (that expects a string): `TypeError: the JSON object must be str, not 'bytes'`. This PR makes sure we (utf-8) decode the bytes to a string before (JSON) decoding the string to a dict.


Problem statement:
uptimerobot: fails with stacktrace
##### SUMMARY
The uptimerobot module fails with stacktrace on decoding the upstream response.

##### ISSUE TYPE
- Bug Report

##### COMPONENT NAME
The uptimerobot module.

##### ANSIBLE VERSION
```
ansible 2.8.1
  config file = /var/lib/jenkins/workspace/Subtask_Openstack_Playbook/ansible.cfg
  configured module search path = ['/var/lib/jenkins/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/local/lib/python3.5/dist-packages/ansible
  executable location = /usr/local/bin/ansible
  python version = 3.5.2 (default, Oct  8 2019, 13:06:37) [GCC 5.4.0 20160609]
```

##### CONFIGURATION
```
ANSIBLE_NOCOWS(/var/lib/jenkins/workspace/Subtask_Openstack_Playbook/ansible.cfg) = True
ANSIBLE_PIPELINING(/var/lib/jenkins/workspace/Subtask_Openstack_Playbook/ansible.cfg) = True
CONDITIONAL_BARE_VARS(/var/lib/jenkins/workspace/Subtask_Openstack_Playbook/ansible.cfg) = False
DEFAULT_FORKS(/var/lib/jenkins/workspace/Subtask_Openstack_Playbook/ansible.cfg) = 10
DEFAULT_HOST_LIST(/var/lib/jenkins/workspace/Subtask_Openstack_Playbook/ansible.cfg) = ['/var/lib/jenkins/workspace/Subtask_Openstack_Playbook/<redacted>/inventory.ini']
DEFAULT_ROLES_PATH(/var/lib/jenkins/workspace/Subtask_Openstack_Playbook/ansible.cfg) = ['/var/lib/jenkins/workspace/Subtask_Openstack_Playbook/roles']
DEFAULT_VAULT_PASSWORD_FILE(/var/lib/jenkins/workspace/Subtask_Openstack_Playbook/ansible.cfg) = /var/lib/jenkins/.vault
MAX_FILE_SIZE_FOR_DIFF(/var/lib/jenkins/workspace/Subtask_Openstack_Playbook/ansible.cfg) = 10485760
```

##### OS / ENVIRONMENT
Not relevant.

##### STEPS TO REPRODUCE
Run something like the following task.

```yaml
 - name: Monitor - {{ desired_state }}
    uptimerobot:
      monitorid: "{{ monitor_id | string }}"
      apikey: "{{ uptimerobot_api_key }}"
      state: "{{ desired_state }}"
```

<!--- HINT: You can paste gist.github.com links for larger files -->

##### EXPECTED RESULTS
The playbook runs without stacktraces and changes the uptimerobot state to `{{ desired_state }}`.


##### ACTUAL RESULTS

```
FAILED - RETRYING: Monitor - paused (291 retries left).Result was: {
    "attempts": 10,                                                   
    "changed": true,                                                  
    "module_stderr": "Traceback (most recent call last):\n  File \"<stdin>\", line 114, in <module>\n  File \"<stdin>\", line 106, in _ansiballz_main\n  File \"<stdin>\", line 49, in invoke_module\n  File \"/usr/lib/python3.5/imp.py\", line 234, in load_module\n    return load_sourc
e(name, filename, file)\n  File \"/usr/lib/python3.5/imp.py\", line 170, in load_source\n    module = _exec(spec, sys.modules[name])\n  File \"<frozen importlib._bootstrap>\", line 626, in _exec\n  File \"<frozen importlib._bootstrap_external>\", line 665, in exec_module\n  File \"<
frozen importlib._bootstrap>\", line 222, in _call_with_frames_removed\n  File \"/tmp/ansible_uptimerobot_payload_gp2xmpdx/__main__.py\", line 151, in <module>\n  File \"/tmp/ansible_uptimerobot_payload_gp2xmpdx/__main__.py\", line 131, in main\n  File \"/tmp/ansible_uptimerobot_pay
load_gp2xmpdx/__main__.py\", line 83, in checkID\n  File \"/usr/lib/python3.5/json/__init__.py\", line 312, in loads\n    s.__class__.__name__))\nTypeError: the JSON object must be str, not 'bytes'\n",
    "module_stdout": "",                                              
    "msg": "MODULE FAILURE\nSee stdout/stderr for the exact error",
    "rc": 1,                                                          
    "retries": 301                                                    
}
```

Hints from issue discussion:

Commit message: Fix uptimerobot stacktrace on decode

`req.read()` always returns bytes whereas the json module expects
strings.

I don't know how this worked before.

Patch to review:
diff --git a/lib/ansible/modules/monitoring/uptimerobot.py b/lib/ansible/modules/monitoring/uptimerobot.py
index 8a16170cd96b20..fb5aa7672d9c40 100644
--- a/lib/ansible/modules/monitoring/uptimerobot.py
+++ b/lib/ansible/modules/monitoring/uptimerobot.py
@@ -79,7 +79,7 @@ def checkID(module, params):
     data = urlencode(params)
     full_uri = API_BASE + API_ACTIONS['status'] + data
     req, info = fetch_url(module, full_uri)
-    result = req.read()
+    result = req.read().decode("utf-8")
     jsonresult = json.loads(result)
     req.close()
     return jsonresult
@@ -91,7 +91,7 @@ def startMonitor(module, params):
     data = urlencode(params)
     full_uri = API_BASE + API_ACTIONS['editMonitor'] + data
     req, info = fetch_url(module, full_uri)
-    result = req.read()
+    result = req.read().decode("utf-8")
     jsonresult = json.loads(result)
     req.close()
     return jsonresult['stat']
@@ -103,7 +103,7 @@ def pauseMonitor(module, params):
     data = urlencode(params)
     full_uri = API_BASE + API_ACTIONS['editMonitor'] + data
     req, info = fetch_url(module, full_uri)
-    result = req.read()
+    result = req.read().decode("utf-8")
     jsonresult = json.loads(result)
     req.close()
     return jsonresult['stat']


Code review comments:
diff_hunk: @@ -79,7 +79,7 @@ def checkID(module, params):
     data = urlencode(params)
     full_uri = API_BASE + API_ACTIONS['status'] + data
     req, info = fetch_url(module, full_uri)
-    result = req.read()
+    result = req.read().decode("utf-8")
path: lib/ansible/modules/monitoring/uptimerobot.py
text: @user1:
This should utilize `ansible.module_utils._text.to_native` as opposed to using `decode` here.

@author:
done!

@user2:
@user1 Hi, could you please look at it again? It would be nice to get this merged :)

@user2:
@author Based on https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md#for-pull-request-submitters I see that in order to proceed, you as a pull request author should use `ready_for_review` command. Could you do that, please? (I can't as I'm not neither an author nor maintainer).