From 52556bb1015feb000b3e5dcf3c109e92071b48a2 Mon Sep 17 00:00:00 2001 From: Alexei Znamensky Date: Fri, 16 Jun 2023 16:25:59 +1200 Subject: [PATCH 01/11] pacman: support yay as root --- plugins/modules/pacman.py | 35 +++++--- .../integration/targets/pacman/tasks/main.yml | 13 +-- .../targets/pacman/tasks/yay-become.yml | 88 +++++++++++++++++++ 3 files changed, 116 insertions(+), 20 deletions(-) create mode 100644 tests/integration/targets/pacman/tasks/yay-become.yml diff --git a/plugins/modules/pacman.py b/plugins/modules/pacman.py index 83b24923176..ebf6961a5f0 100644 --- a/plugins/modules/pacman.py +++ b/plugins/modules/pacman.py @@ -263,7 +263,7 @@ reason_for: all """ -import shlex +import re, shlex from ansible.module_utils.basic import AnsibleModule from collections import defaultdict, namedtuple @@ -418,7 +418,7 @@ def _build_install_diff(pacman_verb, pkglist): for p in name_ver: # With Pacman v6.0.1 - libalpm v13.0.1, --upgrade outputs "loading packages..." on stdout. strip that. # When installing from URLs, pacman can also output a 'nothing to do' message. strip that too. - if "loading packages" in p or "there is nothing to do" in p: + if "loading packages" in p or "there is nothing to do" in p or 'Avoid running' in p: continue name, version = p.split() if name in self.inventory["installed_pkgs"]: @@ -706,11 +706,12 @@ def _build_inventory(self): installed_pkgs = {} dummy, stdout, dummy = self.m.run_command([self.pacman_path, "--query"], check_rc=True) # Format of a line: "pacman 6.0.1-2" + query_re = re.compile(r'^(?P\S+)\s+(?P\S+)\s*$') for l in stdout.splitlines(): - l = l.strip() - if not l: + query_match = query_re.match(l) + if not query_match: continue - pkg, ver = l.split() + pkg, ver = query_match.groups() installed_pkgs[pkg] = ver installed_groups = defaultdict(set) @@ -721,11 +722,12 @@ def _build_inventory(self): # base-devel file # base-devel findutils # ... + query_groups_re = re.compile(r'^(?P\S+)\s+(?P\S+)\s*$') for l in stdout.splitlines(): - l = l.strip() - if not l: + query_groups_match = query_groups_re.match(l) + if not query_groups_match: continue - group, pkgname = l.split() + group, pkgname = query_groups_match.groups() installed_groups[group].add(pkgname) available_pkgs = {} @@ -747,11 +749,12 @@ def _build_inventory(self): # vim-plugins vim-airline-themes # vim-plugins vim-ale # ... + sync_groups_re = re.compile(r'^(?P\S+)\s+(?P\S+)\s*$') for l in stdout.splitlines(): - l = l.strip() - if not l: + sync_groups_match = sync_groups_re.match(l) + if not sync_groups_match: continue - group, pkg = l.split() + group, pkg = sync_groups_match.groups() available_groups[group].add(pkg) upgradable_pkgs = {} @@ -759,19 +762,23 @@ def _build_inventory(self): [self.pacman_path, "--query", "--upgrades"], check_rc=False ) + stdout = stdout.splitlines() + if stdout and "Avoid running" in stdout[0]: + stdout = stdout[1:] + # non-zero exit with nothing in stdout -> nothing to upgrade, all good # stderr can have warnings, so not checked here - if rc == 1 and stdout == "": + if rc == 1 and not stdout: pass # nothing to upgrade elif rc == 0: # Format of lines: # strace 5.14-1 -> 5.15-1 # systemd 249.7-1 -> 249.7-2 [ignored] - for l in stdout.splitlines(): + for l in stdout: l = l.strip() if not l: continue - if "[ignored]" in l: + if "[ignored]" in l or "Avoid running" in l: continue s = l.split() if len(s) != 4: diff --git a/tests/integration/targets/pacman/tasks/main.yml b/tests/integration/targets/pacman/tasks/main.yml index 12d28a2d3eb..73e0e01e015 100644 --- a/tests/integration/targets/pacman/tasks/main.yml +++ b/tests/integration/targets/pacman/tasks/main.yml @@ -11,9 +11,10 @@ - when: ansible_os_family == 'Archlinux' block: # Add more tests here by including more task files: - - include_tasks: 'basic.yml' - - include_tasks: 'package_urls.yml' - - include_tasks: 'remove_nosave.yml' - - include_tasks: 'update_cache.yml' - - include_tasks: 'locally_installed_package.yml' - - include_tasks: 'reason.yml' + # - include_tasks: 'basic.yml' + # - include_tasks: 'package_urls.yml' + # - include_tasks: 'remove_nosave.yml' + # - include_tasks: 'update_cache.yml' + # - include_tasks: 'locally_installed_package.yml' + # - include_tasks: 'reason.yml' + - include_tasks: 'yay-become.yml' diff --git a/tests/integration/targets/pacman/tasks/yay-become.yml b/tests/integration/targets/pacman/tasks/yay-become.yml new file mode 100644 index 00000000000..48fbef38293 --- /dev/null +++ b/tests/integration/targets/pacman/tasks/yay-become.yml @@ -0,0 +1,88 @@ +--- +# Copyright (c) Ansible Project +# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) +# SPDX-License-Identifier: GPL-3.0-or-later + +# This is more convoluted that one might expect, because: +# - yay is not available or installation in ArchLinux (as it is in Manjaro - issue 6184 reports using it) +# - to install yay in ArchLinux requires building the package +# - makepkg cannot be run as root, but the user running it must have sudo to install the resulting package + +- name: create user + ansible.builtin.user: + name: builder + state: present + +- name: grant sudo powers to builder + community.general.sudoers: + name: builder + user: builder + commands: ALL + nopassword: true + +- name: Install base packages + ansible.builtin.package: + name: "{{ item }}" + state: present + with_items: + - base-devel + - git + - go + +- name: Hack permssions for the remote_tmp_dir + ansible.builtin.file: + path: "{{ remote_tmp_dir }}" + mode: 777 + +- name: Create temp directory for builder + ansible.builtin.file: + path: "{{ remote_tmp_dir }}/builder" + owner: builder + state: directory + mode: '755' + +- name: clone yay git repo + become: true + become_user: builder + ansible.builtin.git: + repo: https://aur.archlinux.org/yay.git + dest: "{{ remote_tmp_dir }}/builder/yay" + depth: 1 + +- name: make package + become: true + become_user: builder + ansible.builtin.command: + chdir: "{{ remote_tmp_dir }}/builder/yay" + cmd: makepkg -si --noconfirm + +- name: Install nmap + community.general.pacman: + name: nmap + state: present + executable: yay + extra_args: --builddir /var/cache/yay + +- name: Install XRDP + become: true + become_user: builder + community.general.pacman: + name: xrdp + state: present + executable: yay + extra_args: --builddir /var/cache/yay + +- name: Remove packages + ansible.builtin.package: + name: "{{ item }}" + state: absent + loop: + - base-devel + - yay + - git + - nmap + +- name: Remove user + ansible.builtin.user: + name: builder + state: absent From 414ab187a48a6df8dbd6d9afc990f7393de8016a Mon Sep 17 00:00:00 2001 From: Alexei Znamensky Date: Fri, 16 Jun 2023 18:17:39 +1200 Subject: [PATCH 02/11] make pylint happy --- plugins/modules/pacman.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/modules/pacman.py b/plugins/modules/pacman.py index ebf6961a5f0..34af969f5c7 100644 --- a/plugins/modules/pacman.py +++ b/plugins/modules/pacman.py @@ -263,7 +263,8 @@ reason_for: all """ -import re, shlex +import re +import shlex from ansible.module_utils.basic import AnsibleModule from collections import defaultdict, namedtuple From dea59866d68d0e65ecd619aeb4205eb656e78852 Mon Sep 17 00:00:00 2001 From: Alexei Znamensky Date: Sat, 17 Jun 2023 20:03:24 +1200 Subject: [PATCH 03/11] minor adjustments --- plugins/modules/pacman.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/plugins/modules/pacman.py b/plugins/modules/pacman.py index 34af969f5c7..5393899b0cc 100644 --- a/plugins/modules/pacman.py +++ b/plugins/modules/pacman.py @@ -707,7 +707,7 @@ def _build_inventory(self): installed_pkgs = {} dummy, stdout, dummy = self.m.run_command([self.pacman_path, "--query"], check_rc=True) # Format of a line: "pacman 6.0.1-2" - query_re = re.compile(r'^(?P\S+)\s+(?P\S+)\s*$') + query_re = re.compile(r'^\s*(?P\S+)\s+(?P\S+)\s*$') for l in stdout.splitlines(): query_match = query_re.match(l) if not query_match: @@ -723,7 +723,7 @@ def _build_inventory(self): # base-devel file # base-devel findutils # ... - query_groups_re = re.compile(r'^(?P\S+)\s+(?P\S+)\s*$') + query_groups_re = re.compile(r'^\s*(?P\S+)\s+(?P\S+)\s*$') for l in stdout.splitlines(): query_groups_match = query_groups_re.match(l) if not query_groups_match: @@ -750,7 +750,7 @@ def _build_inventory(self): # vim-plugins vim-airline-themes # vim-plugins vim-ale # ... - sync_groups_re = re.compile(r'^(?P\S+)\s+(?P\S+)\s*$') + sync_groups_re = re.compile(r'^\s*(?P\S+)\s+(?P\S+)\s*$') for l in stdout.splitlines(): sync_groups_match = sync_groups_re.match(l) if not sync_groups_match: @@ -766,6 +766,7 @@ def _build_inventory(self): stdout = stdout.splitlines() if stdout and "Avoid running" in stdout[0]: stdout = stdout[1:] + stdout = "\n".join(stdout) # non-zero exit with nothing in stdout -> nothing to upgrade, all good # stderr can have warnings, so not checked here @@ -775,7 +776,7 @@ def _build_inventory(self): # Format of lines: # strace 5.14-1 -> 5.15-1 # systemd 249.7-1 -> 249.7-2 [ignored] - for l in stdout: + for l in stdout.splitlines(): l = l.strip() if not l: continue From fcbd0b873fc452b96c931e360955bd2cb4783c62 Mon Sep 17 00:00:00 2001 From: Alexei Znamensky Date: Sat, 17 Jun 2023 20:10:38 +1200 Subject: [PATCH 04/11] rollback some test actions --- tests/integration/targets/pacman/tasks/main.yml | 12 ++++++------ .../integration/targets/pacman/tasks/yay-become.yml | 9 --------- 2 files changed, 6 insertions(+), 15 deletions(-) diff --git a/tests/integration/targets/pacman/tasks/main.yml b/tests/integration/targets/pacman/tasks/main.yml index 73e0e01e015..1f6001a669a 100644 --- a/tests/integration/targets/pacman/tasks/main.yml +++ b/tests/integration/targets/pacman/tasks/main.yml @@ -11,10 +11,10 @@ - when: ansible_os_family == 'Archlinux' block: # Add more tests here by including more task files: - # - include_tasks: 'basic.yml' - # - include_tasks: 'package_urls.yml' - # - include_tasks: 'remove_nosave.yml' - # - include_tasks: 'update_cache.yml' - # - include_tasks: 'locally_installed_package.yml' - # - include_tasks: 'reason.yml' + - include_tasks: 'basic.yml' + - include_tasks: 'package_urls.yml' + - include_tasks: 'remove_nosave.yml' + - include_tasks: 'update_cache.yml' + - include_tasks: 'locally_installed_package.yml' + - include_tasks: 'reason.yml' - include_tasks: 'yay-become.yml' diff --git a/tests/integration/targets/pacman/tasks/yay-become.yml b/tests/integration/targets/pacman/tasks/yay-become.yml index 48fbef38293..4b04ebd3c12 100644 --- a/tests/integration/targets/pacman/tasks/yay-become.yml +++ b/tests/integration/targets/pacman/tasks/yay-become.yml @@ -63,15 +63,6 @@ executable: yay extra_args: --builddir /var/cache/yay -- name: Install XRDP - become: true - become_user: builder - community.general.pacman: - name: xrdp - state: present - executable: yay - extra_args: --builddir /var/cache/yay - - name: Remove packages ansible.builtin.package: name: "{{ item }}" From b0364199883b9339cebf884731ece0cad6509ada Mon Sep 17 00:00:00 2001 From: Alexei Znamensky Date: Sat, 17 Jun 2023 20:43:54 +1200 Subject: [PATCH 05/11] removal of user and pkgs in handlers --- .../targets/pacman/handlers/main.yml | 24 +++++++++++++++ .../targets/pacman/tasks/yay-become.yml | 30 ++++++------------- 2 files changed, 33 insertions(+), 21 deletions(-) create mode 100644 tests/integration/targets/pacman/handlers/main.yml diff --git a/tests/integration/targets/pacman/handlers/main.yml b/tests/integration/targets/pacman/handlers/main.yml new file mode 100644 index 00000000000..1a4b6d52095 --- /dev/null +++ b/tests/integration/targets/pacman/handlers/main.yml @@ -0,0 +1,24 @@ +--- +# Copyright (c) Ansible Project +# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) +# SPDX-License-Identifier: GPL-3.0-or-later + +- name: Remove user yaybuilder + ansible.builtin.user: + name: yaybuilder + state: absent + +- name: Remove yay + ansible.builtin.package: + name: yay + state: absent + +- name: Remove packages for yay-become + ansible.builtin.package: + name: "{{ item }}" + state: absent + loop: + - base-devel + - yay + - git + - nmap diff --git a/tests/integration/targets/pacman/tasks/yay-become.yml b/tests/integration/targets/pacman/tasks/yay-become.yml index 4b04ebd3c12..0b5a3db94e7 100644 --- a/tests/integration/targets/pacman/tasks/yay-become.yml +++ b/tests/integration/targets/pacman/tasks/yay-become.yml @@ -10,13 +10,14 @@ - name: create user ansible.builtin.user: - name: builder + name: yaybuilder state: present + notify: Remove user yaybuilder - name: grant sudo powers to builder community.general.sudoers: - name: builder - user: builder + name: yaybuilder + user: yaybuilder commands: ALL nopassword: true @@ -28,6 +29,7 @@ - base-devel - git - go + notify: Remove packages for yay-become - name: Hack permssions for the remote_tmp_dir ansible.builtin.file: @@ -37,13 +39,13 @@ - name: Create temp directory for builder ansible.builtin.file: path: "{{ remote_tmp_dir }}/builder" - owner: builder + owner: yaybuilder state: directory mode: '755' - name: clone yay git repo become: true - become_user: builder + become_user: yaybuilder ansible.builtin.git: repo: https://aur.archlinux.org/yay.git dest: "{{ remote_tmp_dir }}/builder/yay" @@ -51,10 +53,11 @@ - name: make package become: true - become_user: builder + become_user: yaybuilder ansible.builtin.command: chdir: "{{ remote_tmp_dir }}/builder/yay" cmd: makepkg -si --noconfirm + notify: Remove yay - name: Install nmap community.general.pacman: @@ -62,18 +65,3 @@ state: present executable: yay extra_args: --builddir /var/cache/yay - -- name: Remove packages - ansible.builtin.package: - name: "{{ item }}" - state: absent - loop: - - base-devel - - yay - - git - - nmap - -- name: Remove user - ansible.builtin.user: - name: builder - state: absent From 9d02f8f5885c7cad532ab5f3576a2f3687f7eb07 Mon Sep 17 00:00:00 2001 From: Alexei Znamensky Date: Sat, 17 Jun 2023 22:30:50 +1200 Subject: [PATCH 06/11] add comment to note --- plugins/modules/pacman.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/plugins/modules/pacman.py b/plugins/modules/pacman.py index 5393899b0cc..c2ed18d0e79 100644 --- a/plugins/modules/pacman.py +++ b/plugins/modules/pacman.py @@ -133,6 +133,8 @@ it is much more efficient to pass the list directly to the O(name) option. - To use an AUR helper (O(executable) option), a few extra setup steps might be required beforehand. For example, a dedicated build user with permissions to install packages could be necessary. + - In the tests, while using C(yay) as the O(executable) option, the module failed to install AUR packages + with the error: C(error: target not found: ). """ RETURN = """ From 68d0c7b5637293e0c30261a9006ed4129df1d5be Mon Sep 17 00:00:00 2001 From: Alexei Znamensky Date: Sat, 17 Jun 2023 22:34:38 +1200 Subject: [PATCH 07/11] add changelog frag --- changelogs/fragments/6713-yay-become.yml | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 changelogs/fragments/6713-yay-become.yml diff --git a/changelogs/fragments/6713-yay-become.yml b/changelogs/fragments/6713-yay-become.yml new file mode 100644 index 00000000000..389b4c23e47 --- /dev/null +++ b/changelogs/fragments/6713-yay-become.yml @@ -0,0 +1,2 @@ +bugfixes: + - pacman - module recognizes the output of ``yay`` running as ``root`` (https://github.com/ansible-collections/community.general/pull/6713). From dbb8b602c07e339d4f49b8ac807ebe5701c611d7 Mon Sep 17 00:00:00 2001 From: Alexei Znamensky Date: Sun, 18 Jun 2023 00:16:37 +1200 Subject: [PATCH 08/11] fix doc --- plugins/modules/pacman.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/modules/pacman.py b/plugins/modules/pacman.py index c2ed18d0e79..7f67b910399 100644 --- a/plugins/modules/pacman.py +++ b/plugins/modules/pacman.py @@ -133,7 +133,8 @@ it is much more efficient to pass the list directly to the O(name) option. - To use an AUR helper (O(executable) option), a few extra setup steps might be required beforehand. For example, a dedicated build user with permissions to install packages could be necessary. - - In the tests, while using C(yay) as the O(executable) option, the module failed to install AUR packages + - > + In the tests, while using C(yay) as the O(executable) option, the module failed to install AUR packages with the error: C(error: target not found: ). """ From 8f4db1e270dfaa293a1e0004ed9544b4bb614238 Mon Sep 17 00:00:00 2001 From: Alexei Znamensky <103110+russoz@users.noreply.github.com> Date: Thu, 22 Jun 2023 21:31:34 +1200 Subject: [PATCH 09/11] Update tests/integration/targets/pacman/tasks/yay-become.yml Co-authored-by: Felix Fontein --- tests/integration/targets/pacman/tasks/yay-become.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/targets/pacman/tasks/yay-become.yml b/tests/integration/targets/pacman/tasks/yay-become.yml index 0b5a3db94e7..9a1500afcf7 100644 --- a/tests/integration/targets/pacman/tasks/yay-become.yml +++ b/tests/integration/targets/pacman/tasks/yay-become.yml @@ -41,7 +41,7 @@ path: "{{ remote_tmp_dir }}/builder" owner: yaybuilder state: directory - mode: '755' + mode: '0755' - name: clone yay git repo become: true From db61fd72b8c3ba60d09dbd14f0081b8f6fb9c09a Mon Sep 17 00:00:00 2001 From: Alexei Znamensky <103110+russoz@users.noreply.github.com> Date: Thu, 22 Jun 2023 21:31:46 +1200 Subject: [PATCH 10/11] Update tests/integration/targets/pacman/tasks/yay-become.yml Co-authored-by: Felix Fontein --- tests/integration/targets/pacman/tasks/yay-become.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/targets/pacman/tasks/yay-become.yml b/tests/integration/targets/pacman/tasks/yay-become.yml index 9a1500afcf7..434bfa3b0d6 100644 --- a/tests/integration/targets/pacman/tasks/yay-become.yml +++ b/tests/integration/targets/pacman/tasks/yay-become.yml @@ -34,7 +34,7 @@ - name: Hack permssions for the remote_tmp_dir ansible.builtin.file: path: "{{ remote_tmp_dir }}" - mode: 777 + mode: '0777' - name: Create temp directory for builder ansible.builtin.file: From 58075fd70e58ba34256390e61a52c665cdd07080 Mon Sep 17 00:00:00 2001 From: Alexei Znamensky Date: Sat, 24 Jun 2023 18:29:03 +1200 Subject: [PATCH 11/11] simplify pkg install in int. tests --- tests/integration/targets/pacman/handlers/main.yml | 11 +++++------ tests/integration/targets/pacman/tasks/yay-become.yml | 9 ++++----- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/tests/integration/targets/pacman/handlers/main.yml b/tests/integration/targets/pacman/handlers/main.yml index 1a4b6d52095..484482bba36 100644 --- a/tests/integration/targets/pacman/handlers/main.yml +++ b/tests/integration/targets/pacman/handlers/main.yml @@ -15,10 +15,9 @@ - name: Remove packages for yay-become ansible.builtin.package: - name: "{{ item }}" + name: + - base-devel + - yay + - git + - nmap state: absent - loop: - - base-devel - - yay - - git - - nmap diff --git a/tests/integration/targets/pacman/tasks/yay-become.yml b/tests/integration/targets/pacman/tasks/yay-become.yml index 434bfa3b0d6..30f322effe0 100644 --- a/tests/integration/targets/pacman/tasks/yay-become.yml +++ b/tests/integration/targets/pacman/tasks/yay-become.yml @@ -23,12 +23,11 @@ - name: Install base packages ansible.builtin.package: - name: "{{ item }}" + name: + - base-devel + - git + - go state: present - with_items: - - base-devel - - git - - go notify: Remove packages for yay-become - name: Hack permssions for the remote_tmp_dir