CiscoDevNet/ansible-meraki

meraki_webhook query raises exception if no webhooks are defined

mystery-rabbit opened this issue · 13 comments

A query of a Meraki Network with no defined webhooks raises an exception;
Using release 2.14.0 of the cisco.meraki collection.

with the following in a playbook

      - name: "Query all webhooks"
          cisco.meraki.meraki_webhook:
            org_name: "{{ meraki_organisation.name }}"
            net_name: "{{ inventory_hostname }}"
            state: "query"
          register: all_webhooks

the following is seen:

An exception occurred during task execution. To see the full traceback, use -vvv. The error was: IndexError: list index out of range
fatal: [Mock FCN]: FAILED! => {"ansible_facts": {"discovered_interpreter_python": "/usr/bin/python3"}, "changed": false, "module_stderr": "Traceback (most recent call last):\n  File \"/home/vagrant/.ansible/tmp/ansible-tmp-1674493336.4396753-132736-128698658989421/AnsiballZ_meraki_webhook.py\", line 107, in <module>\n    _ansiballz_main()\n  File \"/home/vagrant/.ansible/tmp/ansible-tmp-1674493336.4396753-132736-128698658989421/AnsiballZ_meraki_webhook.py\", line 99, in _ansiballz_main\n    invoke_module(zipped_mod, temp_path, ANSIBALLZ_PARAMS)\n  File \"/home/vagrant/.ansible/tmp/ansible-tmp-1674493336.4396753-132736-128698658989421/AnsiballZ_meraki_webhook.py\", line 47, in invoke_module\n    runpy.run_module(mod_name='ansible_collections.cisco.meraki.plugins.modules.meraki_webhook', init_globals=dict(_module_fqn='ansible_collections.cisco.meraki.plugins.modules.meraki_webhook', _modlib_path=modlib_path),\n  File \"/usr/lib/python3.8/runpy.py\", line 207, in run_module\n    return _run_module_code(code, init_globals, run_name, mod_spec)\n  File \"/usr/lib/python3.8/runpy.py\", line 97, in _run_module_code\n    _run_code(code, mod_globals, init_globals,\n  File \"/usr/lib/python3.8/runpy.py\", line 87, in _run_code\n    exec(code, run_globals)\n  File \"/tmp/ansible_cisco.meraki.meraki_webhook_payload_j7yuon28/ansible_cisco.meraki.meraki_webhook_payload.zip/ansible_collections/cisco/meraki/plugins/modules/meraki_webhook.py\", line 429, in <module>\n  File \"/tmp/ansible_cisco.meraki.meraki_webhook_payload_j7yuon28/ansible_cisco.meraki.meraki_webhook_payload.zip/ansible_collections/cisco/meraki/plugins/modules/meraki_webhook.py\", line 350, in main\n  File \"/tmp/ansible_cisco.meraki.meraki_webhook_payload_j7yuon28/ansible_cisco.meraki.meraki_webhook_payload.zip/ansible_collections/cisco/meraki/plugins/modules/meraki_webhook.py\", line 213, in sanitize_no_log_values\nIndexError: list index out of range\n", "module_stdout": "", "msg": "MODULE FAILURE\nSee stdout/stderr for the exact error", "rc": 1}

Note that using Postman to drive the Meraki API endpoint for the same scenario results in a 200 OK response with an empty [] list returned.

With a webhook defined, task works as expected.

Thank you for submitting this issue. This part of the code on this module was actually refactored a week or two ago and I just need to get a new release out to fix it. I'll try for that tonight but lets keep this open until we verify this fixes your bug.

Hi - sorry to say that the issue persists with 2.15.0

(.venv) vagrant@terra01:~/meraki-as-code/ansible$ ansible-playbook configure_network.yml -i inv_xxx.yml -l "Mock*Branch*2" -t meraki_webhook

PLAY [Configure a network in a Meraki Estate] **************************************************************************************************************************************************************************************************************************

TASK [Query all webhooks] **********************************************************************************************************************************************************************************************************************************************
An exception occurred during task execution. To see the full traceback, use -vvv. The error was: IndexError: list index out of range
fatal: [Mock Branch 2]: FAILED! => {"ansible_facts": {"discovered_interpreter_python": "/usr/bin/python3"}, "changed": false, "module_stderr": "Traceback (most recent call last):\n  File \"/home/vagrant/.ansible/tmp/ansible-tmp-1674661813.7333255-234388-76998232246689/AnsiballZ_meraki_webhook.py\", line 107, in <module>\n    _ansiballz_main()\n  File \"/home/vagrant/.ansible/tmp/ansible-tmp-1674661813.7333255-234388-76998232246689/AnsiballZ_meraki_webhook.py\", line 99, in _ansiballz_main\n    invoke_module(zipped_mod, temp_path, ANSIBALLZ_PARAMS)\n  File \"/home/vagrant/.ansible/tmp/ansible-tmp-1674661813.7333255-234388-76998232246689/AnsiballZ_meraki_webhook.py\", line 47, in invoke_module\n    runpy.run_module(mod_name='ansible_collections.cisco.meraki.plugins.modules.meraki_webhook', init_globals=dict(_module_fqn='ansible_collections.cisco.meraki.plugins.modules.meraki_webhook', _modlib_path=modlib_path),\n  File \"/usr/lib/python3.8/runpy.py\", line 207, in run_module\n    return _run_module_code(code, init_globals, run_name, mod_spec)\n  File \"/usr/lib/python3.8/runpy.py\", line 97, in _run_module_code\n    _run_code(code, mod_globals, init_globals,\n  File \"/usr/lib/python3.8/runpy.py\", line 87, in _run_code\n    exec(code, run_globals)\n  File \"/tmp/ansible_cisco.meraki.meraki_webhook_payload_eytwvxsf/ansible_cisco.meraki.meraki_webhook_payload.zip/ansible_collections/cisco/meraki/plugins/modules/meraki_webhook.py\", line 429, in <module>\n  File \"/tmp/ansible_cisco.meraki.meraki_webhook_payload_eytwvxsf/ansible_cisco.meraki.meraki_webhook_payload.zip/ansible_collections/cisco/meraki/plugins/modules/meraki_webhook.py\", line 350, in main\n  File \"/tmp/ansible_cisco.meraki.meraki_webhook_payload_eytwvxsf/ansible_cisco.meraki.meraki_webhook_payload.zip/ansible_collections/cisco/meraki/plugins/modules/meraki_webhook.py\", line 213, in sanitize_no_log_values\nIndexError: list index out of range\n", "module_stdout": "", "msg": "MODULE FAILURE\nSee stdout/stderr for the exact error", "rc": 1}

PLAY RECAP *************************************************************************************************************************************************************************************************************************************************************
Mock Branch 2              : ok=0    changed=0    unreachable=0    failed=1    skipped=0    rescued=0    ignored=0   

(.venv) vagrant@terra01:~/meraki-as-code/ansible$ ansible-galaxy collection list
...
# /home/vagrant/meraki-as-code/ansible/roles/community/ansible_collections
Collection   Version
------------ -------
cisco.meraki 2.15.0

HI; Not sure if the following should be opened up as separate issues or not so for the moment, lets call these observations...
1> Setting the same webhook twice always returns "changed".

2> the sanitize_no_log_values() method assumes a single entry in data so will not deal correctly with anything other than one webhook - Line 213: meraki.result["data"][0]["shared_secret"] = ...

this is interesting since given the example below, I'm not sure the shared_secret is even in the response - so the code is adding it needlessly?

if you see the output below - one has a shared_secret key, sanitized, one doesn't;

ok: [Mock Branch 1] => {
    "changed": false,
    "data": [
        {
            "id": "aHR0cHM6Ly93d3cuZHVtbXkub3Jn",
            "name": "dummy B",
            "network_id": "L_690176642894535187",
            "payload_template": {
                "name": "Slack (included)",
                "payload_template_id": "wpt_00003"
            },
            "shared_secret": "VALUE_SPECIFIED_IN_NO_LOG_PARAMETER",
            "url": "https://www.dummy.org"
        },
        {
            "id": "aHR0cHM6Ly9kdW1teS5vcmc=",
            "name": "dummy",
            "network_id": "L_690176642894535187",
            "payload_template": {
                "name": "Slack (included)",
                "payload_template_id": "wpt_00003"
            },
            "url": "https://dummy.org"
        }
    ],
    "invocation": {
        "module_args": {
            "auth_key": "VALUE_SPECIFIED_IN_NO_LOG_PARAMETER",
            "follow_redirects": "all",
            "host": "api.meraki.com",
            "internal_error_retry_time": 60,
            "name": null,
            "net_id": null,
            "net_name": "Mock Branch 1",
            "org_id": null,
            "org_name": "xxx",
            "output_format": "snakecase",
            "output_level": "normal",
            "payload_template_id": null,
            "payload_template_name": null,
            "protocol": "https",
            "rate_limit_retry_time": 165,
            "shared_secret": null,
            "state": "query",
            "test": null,
            "test_id": null,
            "timeout": 30,
            "url": null,
            "use_https": true,
            "use_proxy": false,
            "validate_certs": true,
            "webhook_id": null
        }
    },
    "response": "OK (unknown bytes)",
    "status": 200
}

Hello!
I got the same error when I tested the same code. I tried to debug it and I found the issue in it. The above code for query is not working due to function sanitize_no_log_values. After added the exception in that function the tests for that module worked, and your @mystery-rabbit worked fine too.
The PR (#435) for resolving the issue.

EDIT: I think I only resolve the issue with that empty data (array)

@y0rune - I've just looked at the patch, and as i think you might have already noticed, my observation about the hardcoded array index is still valid.. and the fact it's setting a key that doesn't exist, but that might be a conscious decision.

Out of curiosity I looked at why the an "update" of a webhook shows "changed" when it shouldn't necessarily. As far as i can see, you're in a rock/hard place here because of the API; shared-secret is never returned in the API get or update responses - and so you can't know that it does or doesn't need changing, so an API call will always be made to update it and thus "changed" .. just in case .. which is why i assume the tests for this are commented out in the test pack. so.. case closed on that point. Is it possible to update the documentation to call this out?

@y0rune -
Noting that I've only briefly looked at the code, rather than adding a second Exception match, I've applied this locally with success - since this deals with the list indexing issue, this might be a better fit?

    try:
        i=0
        while i < len(meraki.result['data']):
            meraki.result['data'][i][
                "shared_secret"
            ] = "VALUE_SPECIFIED_IN_NO_LOG_PARAMETER"
            i = i + 1
    except KeyError:
        pass

I also added this to the test file:


  - name: Query for any webhooks
    meraki_webhook:
      auth_key: '{{auth_key}}'
      state: query
      org_name: '{{test_org_name}}'
      net_name: '{{test_net_name}}'
    register: query_none

  - debug:
      var: query_none
  
  - assert:
      that:
        - query_none is not changed
        - query_none.data[0] is not defined


So I will close mine PR. Please create a PR with you code. ;)

I would say each of those are different issues. "Query all" not working should be a new one outside of the no_log one.

@y0rune challenge accepted...

I can certainly update the documentation to mention how secrets are ignored by idempotency changes. And yes, since they're not returned by the API (nor should they be most likely) it's impossible to state whether it was changed. Do you have any open PR's for this one right now?

I can certainly update the documentation to mention how secrets are ignored by idempotency changes. And yes, since they're not returned by the API (nor should they be most likely) it's impossible to state whether it was changed. Do you have any open PR's for this one right now?

#436 is open. I have a response to follow up.

#436 is now merged - propose this issue can be closed.