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

Add support for fully qualified collection name in become_method #1072

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

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.

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.

3 participants