Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rabbitmq_user doesn't support Ansible check mode #107

Open
aderixon opened this issue Nov 26, 2021 · 11 comments
Open

rabbitmq_user doesn't support Ansible check mode #107

aderixon opened this issue Nov 26, 2021 · 11 comments

Comments

@aderixon
Copy link

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.

@movergan
Copy link

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

@csmart
Copy link
Collaborator

csmart commented Apr 20, 2022

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

@aderixon
Copy link
Author

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.)

@lukasjuhrich
Copy link

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:

module = AnsibleModule(
argument_spec=arg_spec,
supports_check_mode=True
)

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
Copy link

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
Copy link

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.

@csmart
Copy link
Collaborator

csmart commented Jul 6, 2022

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

csmart added a commit to csmart/community.rabbitmq that referenced this issue Jul 7, 2022
Currently, check_mode is not supported on the user module and as such, a
user is actually added when run with `--check` option.

This an initial temporary fix in order to avoid issues on existing systems.

See ansible-collections#107
@csmart csmart mentioned this issue Jul 7, 2022
csmart added a commit to csmart/community.rabbitmq that referenced this issue Jul 7, 2022
Currently, check_mode is not supported on the user module and as such, a
user is actually added when run with `--check` option.

This an initial temporary fix in order to avoid issues on existing systems.

See ansible-collections#107
csmart added a commit to csmart/community.rabbitmq that referenced this issue Jul 7, 2022
Currently, check_mode is not supported on the user module and as such, a
user is actually modified when run with the `--check` option.

This an initial temporary fix in order to avoid issues on existing
systems.

See ansible-collections#107
csmart added a commit to csmart/community.rabbitmq that referenced this issue Jul 7, 2022
Currently, check_mode is not supported on the user module and as such, a
user is actually modified when run with the `--check` option.

This an initial temporary fix in order to avoid issues on existing
systems.

See ansible-collections#107
csmart added a commit to csmart/community.rabbitmq that referenced this issue Jul 7, 2022
Currently, check_mode is not supported on the user module and as such, a
user is actually modified when run with the `--check` option.

This an initial temporary fix in order to avoid issues on existing
systems.

See ansible-collections#107
csmart added a commit that referenced this issue Jul 7, 2022
Currently, check_mode is not supported on the user module and as such, a
user is actually modified when run with the `--check` option.

This an initial temporary fix in order to avoid issues on existing
systems.

See #107
@csmart
Copy link
Collaborator

csmart commented Jul 7, 2022

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

@csmart
Copy link
Collaborator

csmart commented Jul 13, 2022

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

@tubemeister
Copy link

Any chance of actually supporting check mode? It's a useful feature, as crossing your fingers and blindly running a playbook over an environment that may have local changes is still not great.

@tubemeister
Copy link

For whoever needs it, I hacked up a quick ugly workaround role:

---
- name: Check user credentials
  ansible.builtin.include_tasks: user.yml
  loop: "{{ rabbitmq_user_credentials }}"
  loop_control:
    loop_var: user
  when: ansible_check_mode
  run_once: true

user.yml:

---
- name: Password for {{ user.username }}
  ansible.builtin.command:
    cmd: 'rabbitmqctl authenticate_user "{{ user.username }}" "{{ user.password }}"'
  register: userauth
  changed_when: userauth.rc != 0
  failed_when: false
  check_mode: false

- name: Fetch permissions for {{ user.username }}
  ansible.builtin.command:
    cmd: 'rabbitmqctl -s list_user_permissions {{ user.username }}'
  register: perms_on_server
  changed_when: false
  check_mode: false
  
- name: Permissions for {{ user.username }}
  ansible.builtin.debug:
    msg: "Permission for user {{ user.username }} on vhost {{ vhost }}\n          Vhost\tConfigure\tWrite\tRead\nPlaybook: {{ line }}\nServer:   {{ server }}\n\n"
  loop: "{{ user.permissions }}"
  loop_control:
    loop_var: perms
  vars:
    vhost: "{{ perms.vhost }}"
    line: "{{ vhost }}\t{{perms.configure_priv}}\t{{perms.write_priv}}\t{{perms.read_priv}}"
    server: '{{ perms_on_server.stdout_lines | select("match", vhost) | first | default("undef")}}'
  changed_when: line != server

assumes the following data structure in var rabbitmq_user_credentials:

rabbitmq_user_credentials:
  - username: 'monitoring'
    password: "{{ vault_rabbitmq_monitoring_password }}"
    tags: 'monitoring'
    permissions:
      - vhost: 'test123'
        configure_priv: ''
        write_priv: ''
        read_priv: '.*'
      - vhost: 'timeseries'
        configure_priv: ''
        write_priv: ''
        read_priv: '.*'

It'll certainly have its faults but for now it's allowed me to compare state between the ansible playbook and reality on the cluster without wrecking production...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants