Skip to content

Conversation

@jadacyrus
Copy link

@jadacyrus jadacyrus commented May 7, 2024

According to ansible-lint the correct value for become_method su and sudo are the fully qualified names ansible.builtin.su and ansible.builtin.sudo (https://github.com/ansible/ansible-lint/blob/main/src/ansiblelint/schemas/ansible.json#L48)

Making the switch to these fully qualified names while using the Mitogen connection strategy produces the following error :

An exception occurred during task execution. To see the full traceback, use -vvv. The error was: KeyError: 'ansible.builtin.sudo'
fatal: [REDACTED]: FAILED! =>
  msg: 'Unexpected failure during module execution: ''ansible.builtin.sudo'''
  stdout: ''

Here is an example playbook that will reproduce the bug:

# test-playbook.yml
- hosts: localhost
  tasks:
    - name: Run a shell command
      ansible.builtin.command: /bin/echo $(id -un)
      become: true
      become_user: nobody
      become_method: ansible.builtin.sudo

@jadacyrus jadacyrus force-pushed the add-fqcn-for-become-method branch from c473514 to 22c3202 Compare May 7, 2024 18:51
@jadacyrus
Copy link
Author

@moreati Anything specifically you want for this?

@dramborleg
Copy link

I also ran into this issue, is it possible to merge this or is something else needed? It's pretty small so just curious if it's a problem with the MR or just lack of time. If there's anything that can be done to help move it forward I may be able to contribute depending on what is needed. Thanks!

@jadacyrus
Copy link
Author

@moreati Bump.

@jadacyrus
Copy link
Author

@moreati very simple change here to fix issue. do you need anything else?

@moreati
Copy link
Member

moreati commented Jan 28, 2025

Sorry for the wait.

You need

Optional

@jadacyrus jadacyrus force-pushed the add-fqcn-for-become-method branch from b316e74 to c481965 Compare January 28, 2025 17:46
@jadacyrus
Copy link
Author

I am not sure why some of these tests are failing with su ...

@moreati
Copy link
Member

moreati commented Jan 29, 2025 via email

@moreati
Copy link
Member

moreati commented Jan 31, 2025

After some investigation in #1233 I don't know why the su test is failing and I think I've found a second problem. So far

FQCN su is not supply the password in some Ansible versions, e.g. c481965 in https://github.com/mitogen-hq/mitogen/actions/runs/13016396756/job/36306555275?pr=1072

PLAY [regression/issue_1232__fully_qualified_names_become_method.yml (2/2)] ****

TASK [Run a shell command with su mitogen__user1 _raw_params=whoami] ***********
Tuesday 28 January 2025  18:12:10 +0000 (0:00:00.563)       0:01:45.081 ******* 
fatal: [target-centos6-1]: FAILED! => 
  msg: 'error occurred on host target-centos6-1: su password is required'

FQCN sudo appears to be reusing an old connection, even if a subsquent task specifies a different become_user, e.g. 09b44e4 in https://github.com/mitogen-hq/mitogen/actions/runs/13055004397/job/36423772033

TASK [assert that=["fqcn_sudo_password_whoami.stdout == 'mitogen__pw_required'"], fail_msg=fqcn_sudo_password_whoami={{ fqcn_sudo_password_whoami }}
] ***
task path: /home/runner/work/mitogen/mitogen/tests/ansible/regression/issue_1232__fully_qualified_names_become_method.yml:37
Thursday 30 January 2025  15:03:40 +0000 (0:00:00.074)       0:00:03.625 ****** 
...
fatal: [target-centos7-1]: FAILED! => changed=false 
  assertion: fqcn_sudo_password_whoami.stdout == 'mitogen__pw_required'
  evaluated_to: false
  msg: |-
    fqcn_sudo_password_whoami={'changed': False, 'end': '2025-01-30 15:03:40.281030', 'stdout': 'root', 'cmd': ['whoami'], 'start': '2025-01-30 15:03:40.277352', 'delta': '0:00:00.003678', 'stderr': '', 'rc': 0, 'msg': '', 'stdout_lines': ['root'], 'stderr_lines': [], 'failed': False}

@moreati
Copy link
Member

moreati commented Feb 4, 2025

For now my conclusion is that this will require more fundamental changes to ansible_mitogen. Possibly

  • moving mitogen_sudo et al from plugins/connections to plugins/become.
  • changing how Mitogen intercepts/integrates with Ansible collection loading.

@dramborleg
Copy link

Thanks for looking at it @moreati, I doubt I'd have time to help with a larger change but it's nice knowing the next steps in case I ever do get a chance, and I appreciate the time spent investigating.

'ansible.builtin.su': _connect_su,
'sudo': _connect_sudo,
'ansible.builtin.sudo': _connect_sudo,
'doas': _connect_doas,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi. Why is this not added for community.general.doas also? I myself ran into this bug with doas.

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

Successfully merging this pull request may close these issues.

4 participants