Explicit become=false to avoid issues with HTTAPI in certain scenarios

[santiagomr]

Hello, I had been on version 1.* of the collection for some time and today I decided to upgrade to version 2.*

Everything has gone pretty well in the transition, but due to the change in the way the Zabbix API is interacted with with this version change, I have lost some time to understand and diagnose a bug that I assume may be affecting many people.

The problem is finally so simple that in addition to explaining it, I send it as PR.

SUMMARY

When using httpapi to interact with the Zabbix Server, already indicating a user-password, no type of become is necessary for things to work.

However, since in the tasks corresponding to the interaction with the API the become: false has not been explicitly indicated, as the role is today, it fails in every project where we do not connect to our Zabbix Server directly as the root user (this is because whatever become_method is defined in the project/inventory will interfere with calls originating via HTTPAPI)

ISSUE TYPE

• Bugfix Pull Request

COMPONENT NAME

zabbix_agent role

ADDITIONAL INFORMATION

For example, if for my Zabbix Server host I have these variables defined:

ansible_ssh_user: an_unprivileged_user
ansible_become_method: su
ansible_become_user: root

API tasks fail as follows:

TASK [community.zabbix.zabbix_agent : API | Create host groups] ***************************************************************************************************************
FAILED - RETRYING: [example.zabbix.com]: API | Create host groups (3 retries left).
FAILED - RETRYING: [example.zabbix.com]: API | Create host groups (2 retries left).
FAILED - RETRYING: [example.zabbix.com]: API | Create host groups (1 retries left).
fatal: [example.zabbix.com]: FAILED! => {"attempts": 3, "changed": false, "module_stderr": "Password: su: Authentication failure\n", "module_stdout": "", "msg": "MODULE FAILURE\nSee stdout/stderr for the exact error", "rc": 1}

[flowerysong]

-1. These tasks are not explicitly setting become: true, so the only reason you are seeing this error is that you are setting that somewhere else. The role should not attempt to second-guess the user's privilege escalation choices.

If you've enabled escalation by default or at the playbook level, every method of calling a role allows you to override it at that point:

  roles:
    - role: community.zabbix.zabbix_agent
      become: false

  tasks:
    - include_role:
        name: community.zabbix.zabbix_agent
        apply:
          become: false

    - import_role:
        name: community.zabbix.zabbix_agent
      become: false

[santiagomr]

Hi @flowerysong, as you assumed, in my use case the escalation is setted at the playbook level when invoking a series of roles that require it. Among that series of roles is zabbix_agent.

As a result of your comment, I have gone to review the tasks of this role that require elevated privileges in the system and I have found that each of them explicitly establishes the become: true when needed. This makes escalation at the playbook level not strictly necessary and therefore the solutions you propose work properly.

Even so, I consider that the proposed changes are still an improvement to the role. Being explicit with the become: false for those tasks that use HTTPAPI in particular ensures correct and expected operation in 100% of cases, whether become: true has been set anywhere else or not.

In this way I understand that a smoother operation is achieved out of the box. It could have actually saved me the only error that occurred during the process of updating to the new major version of this collection. Everything else works perfectly whether become: true is inherited or not.

[pyrodie18]

I think I'm probably with @flowerysong on this one. The roles are all currently written to elevate when needed, and leave it up to the user outside of that. I don't see see an advantage to changing that, and forcing the user to figure out why privlidges change in unexpected places. @BGmot thoughts?

[santiagomr]

Update: For anyone who is passing by after this PR was closed, please note that the changes proposed here were incorporated into the main branch of the collection recently in this other PR: #1393