Error in zabbix_action

[rockaut]

SUMMARY

Error on zabbix_action if there's no change.

ISSUE TYPE

• Bug Report

COMPONENT NAME

zabbix_action

ANSIBLE VERSION

ansible 2.9.9
  config file = /d/Code/gitlab/zabbix-servers/ansible/ansible.cfg
  configured module search path = ['/home/fism/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /d/Code/gitlab/zabbix-servers/venv/lib/python3.8/site-packages/ansible
  executable location = /d/Code/gitlab/zabbix-servers/venv/bin/ansible
  python version = 3.8.2 (default, Apr 27 2020, 15:53:34) [GCC 9.3.0]

community.zabbix==0.2.0

ENVIRONMENT

Zabbix 5.0.1 (centos containers)

STEPS TO REPRODUCE

Running the play the first time creates the action. Subsequent runs are failing.

- name: "Defining default proxy actions"
      delegate_to: localhost
      zabbix_action:
        server_url: "{{ zabbix_apiserver_url }}"
        login_user: "{{ zabbix_apiuser_rw }}"
        login_password: "{{ zabbix_apipassword_rw }}"
        event_source: auto_registration
        name: "{{ zabbix_action_name }}"
        status: enabled
        esc_period: 1h
        state: present
        formula: A
        conditions:
          - type: 'proxy'
            operator: '='
            value: "proxya"
            formulaid: A
        operations:
          - type: 'add_to_host_group'
            host_groups: "INTRA"

EXPECTED RESULTS

Subsequent runs shouldn't fail.

ACTUAL RESULTS

The full traceback is:
  File "/tmp/ansible_zabbix_action_payload_mujf4st6/ansible_zabbix_action_payload.zip/ansible/modules/zabbix_action.py", line 882, in update_action
  File "/d/Code/gitlab/zabbix-servers/venv/lib/python3.8/site-packages/zabbix_api.py", line 341, in method
    return self.universal("%s.%s" % (self.data["prefix"], name), opts[0])
  File "/d/Code/gitlab/zabbix-servers/venv/lib/python3.8/site-packages/zabbix_api.py", line 79, in wrapper
    return self.do_request(self.json_obj(method, opts))['result']
  File "/d/Code/gitlab/zabbix-servers/venv/lib/python3.8/site-packages/zabbix_api.py", line 348, in do_request
    return self.parent.do_request(req)
  File "/d/Code/gitlab/zabbix-servers/venv/lib/python3.8/site-packages/zabbix_api.py", line 299, in do_request
    raise ZabbixAPIException(msg, jobj['error']['code'])
fatal: [zabbixintra01p -> localhost]: FAILED! => {
    "changed": false,
    "invocation": {
        "module_args": {
            "acknowledge_default_message": "",
            "acknowledge_default_subject": "",
            "acknowledge_operations": [],
            "conditions": [
                {
                    "formulaid": "A",
                    "operator": "=",
                    "type": "proxy",
                    "value": "xxx",
                    "value2": null
                }
            ],
            "default_message": "",
            "default_subject": "",
            "esc_period": "1h",
            "eval_type": null,
            "event_source": "auto_registration",
            "formula": "A",
            "http_login_password": null,
            "http_login_user": null,
            "login_password": "VALUE_SPECIFIED_IN_NO_LOG_PARAMETER",
            "login_user": "xxx",
            "name": "AUTO - add Proxy Group - intra",
            "operations": [
                {
                    "command": null,
                    "command_type": null,
                    "esc_period": null,
                    "esc_step_from": 1,
                    "esc_step_to": 1,
                    "execute_on": null,
                    "host_groups": [
                        "INTRA"
                    ],
                    "inventory": null,
                    "media_type": null,
                    "message": null,
                    "operation_condition": null,
                    "password": null,
                    "port": null,
                    "run_on_groups": null,
                    "run_on_hosts": null,
                    "script_name": null,
                    "send_to_groups": null,
                    "send_to_users": null,
                    "ssh_auth_type": "password",
                    "ssh_privatekey_file": null,
                    "ssh_publickey_file": null,
                    "subject": null,
                    "templates": null,
                    "type": "add_to_host_group",
                    "username": null
                }
            ],
            "pause_in_maintenance": true,
            "recovery_default_message": "",
            "recovery_default_subject": "",
            "recovery_operations": [],
            "server_url": "http://xxx/",
            "state": "present",
            "status": "enabled",
            "timeout": 10,
            "validate_certs": true
        }
    },
    "msg": "Failed to update action '28': ('Error -32500: Application error., Cannot perform update statement on table \"actions\" without values. while sending {\"jsonrpc\": \"2.0\", \"method\": \"action.update\", \"params\": {\"def_longdata\": \"\", \"def_shortdata\": \"\", \"r_longdata\": \"\", \"r_shortdata\": \"\", \"ack_longdata\": \"\", \"ack_shortdata\": \"\", \"actionid\": \"28\"}, \"auth\": \"xxx\", \"id\": 6}', -32500)"
}

[rockaut]

I think the problem is around the computation of the difference as it always gets this difference:

{'def_longdata': '', 'def_shortdata': '', 'r_longdata': '', 'r_shortdata': '', 'ack_longdata': '', 'ack_shortdata': ''

[rockaut]

Value of existing_action in check_difference:

{
  "actionid": "29",
  "name": "AUTO - add Proxy Group - intra",
  "eventsource": "2",
  "status": "0",
  "esc_period": "1h",
  "pause_suppressed": "1",
  "filter": {
    "evaltype": "0",
    "formula": "",
    "conditions": [
      {
        "conditiontype": "20",
        "operator": "0",
        "value": "10270",
        "value2": "",
        "formulaid": "A"
      }
    ],
    "eval_formula": "A"
  },
  "operations": [
    {
      "operationid": "93",
      "actionid": "29",
      "operationtype": "4",
      "esc_period": "0",
      "esc_step_from": "1",
      "esc_step_to": "1",
      "evaltype": "0",
      "opconditions": [],
      "opgroup": [
        {
          "groupid": "20"
        }
      ]
    }
  ],
  "recovery_operations": [],
  "acknowledge_operations": []
}

Value of kwargs in check_difference

{
  "action_id": "29",
  "name": "AUTO - add Proxy Group - intra",
  "event_source": "auto_registration",
  "esc_period": "1h",
  "status": "enabled",
  "pause_in_maintenance": True,
  "default_message": "",
  "default_subject": "",
  "recovery_default_message": "",
  "recovery_default_subject": "",
  "acknowledge_default_message": "",
  "acknowledge_default_subject": "",
  "operations": [
    {
      "operationtype": "4",
      "esc_step_from": 1,
      "esc_step_to": 1,
      "opgroup": [
        {
          "groupid": "20"
        }
      ]
    }
  ],
  "recovery_operations": [],
  "acknowledge_operations": [],
  "conditions": {
    "conditions": [
      {
        "conditiontype": "20",
        "value": "10270",
        "formulaid": "A",
        "operator": "0"
      }
    ],
    "evaltype": "0"
  }
}

So the difference fields are empty in kwargs because the come through the function call. The solution should be to drop them if they are empty && missing in the current action too?

[rockaut]

If I add a if v=="" to compare_dictionaries all works as expected for me.
I currently only do auto_register actions so I can't state on more.

def compare_dictionaries(d1, d2, diff_dict):
...
    for k, v in d1.items():
        if k not in d2:
            if v == "":
                continue
            diff_dict[k] = v
            continue
        if isinstance(v, dict):
            diff_dict[k] = {}
...

WDYT?

[D3DeFi]

You have got it right @rockaut . I've noticed before that Zabbix developers deprecated these six API parameters without stating it in the API changes (def_longdata, def_shortdata, r_longdata, r_shortdata, ack_longdata, ack_shortdata), but I have thought that API silently ignores them. So much for still not having integration tests for the module.

The fix itself should deprecate all of the mentioned parameters for Zbx 5.0+ and clearly state that in the module documentation. That means those 6 parameters should not be allowed to propagate so further in the code and should be terminated in the _construct_parameters() method.

I am worried that adding if v == "": to the code may break functionality when someone tries to null something via module (e.g. remove all recovery operations from the actions - need to confirm this).

Are you interested in contributing this by yourself @rockaut? If not, I can try to come up with a fix later this week.

Of course providing that @sky-joker haven't started working on this already :)

[rockaut]

Shure I would help out here - we transitioned to 5.0.1 and I'm having more in the hands coming in the next weeks.

Just to get that right:
A. The 6 fields are deprecated altogether so from >=5.0 not there anymore? If so I would say you're right that would be better to handle that more stricktly. That said how is the stance on dealing with such changes? Just dropping them in _construct_parameters() and document it or throwing an actual error?

B. I am also worried by the if v == "": it currently works for me but the bigger picture was the point I 've not already pushed a PR. I thought I come up over night with a clever way but nope.

That said, if A is true then B is obsolete anyways?

[rockaut]

I checked it in the docs:

https://www.zabbix.com/documentation/4.4/manual/api/reference/action/object
https://www.zabbix.com/documentation/5.0/manual/api/reference/action/object

Those 6 fields are removed.

Gnn... As much as I like Zabbix, their API documentation is Changed.md is bad.

[rockaut]

:D https://support.zabbix.com/browse/ZBX-17932

I'm already changing the if-thing to your suggested way in my workaround implementation. I will file the PR tomorrow.

[D3DeFi]

This one didn't help either :)
https://www.zabbix.com/documentation/current/manual/api/changes_5.0

Yes, A is right, those 6 parameters are not needed anymore. That means:

• State under each in DOCUMENTATION that they are no longer supported in 5.0+
• Create LooseVersion magic in _construct_parameters() method. e.g fixed zabbix_action to search for mediatype by its name in Zabbix 4.4+ #90 for reference how (or grep for LooseVersion in other modules)
• B is obsolete

[D3DeFi]

Maybe better example here:

community.zabbix/plugins/modules/zabbix_mediatype.py

Line 431 in 0ed2a70

parameters = dict(

[rockaut]

I haven't seen your example before I pushed the PR but I had a look into it. The mediatypes module has another layout/does things a little bit different than the action module. I more or less have done it just the action modules way as I otherwise would have to introduce some more changes and making the module structure inconsistent - refactoring should be a whole other process. Hope that's good anyways?

If you wish I will change it to LooseVersion tomorrow.

[D3DeFi]

I haven't realized that my patch haven't made it through yet. What you did is completely fine. Thanks for the patch!

It is true that all of the modules could use some unification and refactoring

[rockaut]

@D3DeFi Hm, how would you start that. I would like to help here out.
Would it be ok to open a new issue for smaller tasks, let's say, "Refactor and unify Zabbix API Version dependencies" and then provide an PR. Or would go go the big route and do a milestone/project thing?

[D3DeFi]

Good question. I am starting to feel bad for always replaying with I don't know yet 😆

These modules were mostly on life support with bugfixes when we were part of ansible/ansible. Now that roles are part of this repo and we are gaining some traction, lots of similar questions need to be answered. I tried to think a bit about where to direct the repository, but time is a concern so my work mostly consists of fixing bugs rn and porting to 5.0. And boy is it pain when each module is different.

If you want or have some ideas you can always hit me up on d3defi@gmail.com

Would it be ok to open a new issue for smaller tasks, let's say, "Refactor and unify Zabbix API Version dependencies" and then provide an PR.

It would. We don't have that much people and time to actually start something major so this ad-hoc approach is more preferred for now. But some implementations are better discussed in advance (issue, mail, or we can always try to organize IRC meeting).

I have in a personal TODO (more like, stuff I'd like to do, somewhere in my head) to check if we can benefit from common class (something like ZabbixBase) that would implement logins, logouts, basic modules and so on. That maybe could be our start for bigger refactoring.