community.rabbitmq icon indicating copy to clipboard operation
community.rabbitmq copied to clipboard

rabbitmq_user doesn't support Ansible check mode

Open aderixon opened this issue 3 years ago • 9 comments

SUMMARY

When applied with ansible --check, the rabbitmq_user module always makes any required changes to the users anyway. This is unexpected behaviour and potentially disruptive to the RabbitMQ service.

ISSUE TYPE
  • Bug Report
COMPONENT NAME

rabbitmq_user

ANSIBLE VERSION
ansible 2.9.23
  config file = /home/test/src/working/ansible/ansible.cfg
  configured module search path = [u'/usr/share/ansible']
  ansible python module location = /usr/lib/python2.7/site-packages/ansible
  executable location = /usr/bin/ansible
  python version = 2.7.5 (default, Nov 16 2020, 22:23:17) [GCC 4.8.5 20150623 (Red Hat 4.8.5-44)]
COLLECTION VERSION
Collection         Version
------------------ -------
community.rabbitmq 1.1.0  
CONFIGURATION
ANSIBLE_PIPELINING(/home/test/src/working/ansible/ansible.cfg) = True
DEFAULT_MODULE_PATH(/home/test/src/working/ansible/ansible.cfg) = [u'/usr/share/ansible']
HOST_KEY_CHECKING(/home/test/src/working/ansible/ansible.cfg) = False
TRANSFORM_INVALID_GROUP_CHARS(/home/test/src/working/ansible/ansible.cfg) = ignore
OS / ENVIRONMENT

CentOS 7.9

STEPS TO REPRODUCE

Create a new RabbitMQ user using the rabbitmq_user module, but apply the playbook using ansible-playbook --check, which should show what would happen.

- hosts: all
  become: true
  tasks:
  - name: configure rabbitmq users
    community.rabbitmq.rabbitmq_user:
      name: "test-user"
      password: "pebhebEk"
      configure_priv: "^$"
      read_priv: "^$"
      write_priv: "^$"
      state: "present"
$ ansible-playbook -i hosts rabbitmq-test.yml -l mqserver.test.com -K --check -v
$ ansible-playbook -i hosts rabbitmq-test.yml -l mqserver.test.com -K --check -v
EXPECTED RESULTS

Ansible output would consistently show 'changed: [mqserver.test.com] => {"changed": true, ...}' for task but test-user would not actually be created on RabbitMQ host.

ACTUAL RESULTS

test-user is created the first time playbook is applied with --check. A second run outputs 'ok: ... "changed": false', indicating that the user now exists.

ansible(master)] 1023$ ansible-playbook -i hosts rabbitmq-test.yml -l mqserver.test.com -K --check -v
Using /home/test/src/working/ansible/ansible.cfg as config file
BECOME password: 

PLAY [all] *********************************************************************

TASK [Gathering Facts] *********************************************************
ok: [mqserver.test.com]

TASK [configure rabbitmq users] ************************************************
changed: [mqserver.test.com] => {"changed": true, "state": "present", "user": "test-user"}

PLAY RECAP *********************************************************************
mqserver.test.com          : ok=2    changed=1    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0   

ansible(master)] 1024$ ansible-playbook -i hosts rabbitmq-test.yml -l mqserver.test.com -K --check -v
Using /home/test/src/working/ansible/ansible.cfg as config file
BECOME password: 

PLAY [all] *********************************************************************

TASK [Gathering Facts] *********************************************************
ok: [mqserver.test.com]

TASK [configure rabbitmq users] ************************************************
ok: [mqserver.test.com] => {"changed": false, "state": "present", "user": "test-user"}

PLAY RECAP *********************************************************************
mqserver.test.com          : ok=2    changed=0    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0   


[root@mqserver test]# rabbitmqctl list_users | grep test-user
test-user	[]

This is because the code for rabbitmq_user.py does not include support for running in check mode (e.g. compare implementation of _exec() with rabbitmq_vhost.py, which explicitly tests for check_mode being enabled and does not execute rabbitmqctl if so). I have a modified rabbitmq_user implementation that includes check_mode support (based on rabbitmq_vhost.py) if you want a PR.

aderixon avatar Nov 26 '21 11:11 aderixon

Any update on that? It's quite a critical bug, one testing user password change can bring production down without knowing it.

movergan avatar Apr 20 '22 09:04 movergan

Hi @movergan thanks for the report, I will try to have a look into it this week and provide an update.

csmart avatar Apr 20 '22 12:04 csmart

rabbitmq_user.py.txt

Attaching my patched version of this module here in case it's useful for anyone else who needs a workaround for now. (Caveat emptor, etc.)

aderixon avatar Apr 20 '22 13:04 aderixon

Any update on that? It's quite a critical bug, one testing user password change can bring production down without knowing it.

Agreed, this is what took me out of my sunday afternoon. But these things happen.

As it stands, the „smaller“ bug that we're having here is the fact that rabbitmq_user declares supports_check_mode=True, which at the moment is not true: https://github.com/ansible-collections/community.rabbitmq/blob/74f479f855e4dc66faecc4327d8cca01f853faf8/plugins/modules/rabbitmq_user.py#L466-L469

So setting this to False would be helpful already.

As a side note, to fix this issue „properly“, i.e. “supporting check mode“, can mean two different things:

  1. Checking for the presence of the users in the proper configuration
  2. On top of that, checking that authentication with the desired password is possible. This requires a decision beforehand.

lukasjuhrich avatar Jun 19 '22 20:06 lukasjuhrich

2. On top of that, checking that authentication with the desired password is possible.
   This requires a decision beforehand.

I just learned about the update_password option, which one would have to take into account.

lukasjuhrich avatar Jun 19 '22 20:06 lukasjuhrich

Hi @movergan thanks for the report, I will try to have a look into it this week and provide an update.

@csmart I hope this doesn't come across as impatient, but if time is an issue, would it be possible to set supports_check_mode=False in the meantime? It would be a trivial change but already very helpful.

lukasjuhrich avatar Jul 05 '22 12:07 lukasjuhrich

@lukasjuhrich not at all, you're right. This fell off my radar, so thanks for the reminder.

csmart avatar Jul 06 '22 11:07 csmart

This has been merged to main, I will arrange a new release soon.

csmart avatar Jul 07 '22 23:07 csmart

I've released 1.2.2 which should be available on galaxy soon.

csmart avatar Jul 13 '22 11:07 csmart