diff --git a/changelogs/fragments/85623-fix-pip-check-mode-vcs.yml b/changelogs/fragments/85623-fix-pip-check-mode-vcs.yml new file mode 100644 index 00000000000..669cc952c85 --- /dev/null +++ b/changelogs/fragments/85623-fix-pip-check-mode-vcs.yml @@ -0,0 +1,5 @@ +bugfixes: + - >- + ``ansible.builtin.pip`` - Running the built-in pip module with ``check_mode`` and packages coming from VCS URLs, archives, or local filepaths now correctly outputs the ``changed`` status of the task. + Previously, it was always reported as changed due to improper package name resolution. + (https://github.com/ansible/ansible/pull/85623) diff --git a/lib/ansible/modules/pip.py b/lib/ansible/modules/pip.py index 6af7e7afdeb..344936fb571 100644 --- a/lib/ansible/modules/pip.py +++ b/lib/ansible/modules/pip.py @@ -293,6 +293,7 @@ virtualenv: """ import argparse +import json import os import re import sys @@ -611,6 +612,57 @@ def setup_virtualenv(module, env, chdir, out, err): return out, err, cmd +def _resolve_package_names( + module: AnsibleModule, + package_list: list[Package], + pip: list[str], + python_bin: str, +) -> list[Package]: + """Resolve package references in the list. + + This helper function downloads metadata from PyPI + using ``pip install``'s ability to return JSON. + """ + pkgs_to_resolve = [pkg for pkg in package_list if not pkg.has_requirement] + + if not pkgs_to_resolve: + return package_list + + # pip install --dry-run is not available in pip versions older than 22.2 and it doesn't + # work correctly on all cases until 24.1, so check for this and use the non-resolved + # package names if pip is outdated. + pip_dep = _get_package_info(module, "pip", python_bin) + + installed_pip = LooseVersion(pip_dep.split('==')[1]) + minimum_pip = LooseVersion("24.1") + + if installed_pip < minimum_pip: + module.warn("Using check mode with packages from vcs urls, file paths, or archives will not behave as expected when using pip versions <24.1.") + return package_list # Just use the default behavior + + with tempfile.NamedTemporaryFile() as tmpfile: + # Uses a tmpfile instead of capturing and parsing stdout because it circumvents the need to fuss with ANSI color output + module.run_command( + [ + *pip, 'install', + '--dry-run', + '--ignore-installed', + f'--report={tmpfile.name}', + *map(str, pkgs_to_resolve), + ], + check_rc=True, + ) + report = json.load(tmpfile) + + package_objects = ( + Package(install_report['metadata']['name'], version_string=install_report['metadata']['version']) + for install_report in report['install'] + ) + + other_packages = (pkg for pkg in package_list if pkg.has_requirement) + return [*other_packages, *package_objects] + + class Package: """Python distribution package metadata wrapper. @@ -663,6 +715,11 @@ class Package: for op, ver in self._requirement.specs ) + @property + def has_requirement(self) -> bool: + """Compute whether the object represents complex requirement.""" + return self._requirement is not None + @staticmethod def canonicalize_name(name): # This is taken from PEP 503. @@ -845,7 +902,9 @@ def main(): pkg_list.append(formatted_dep) out += '%s\n' % formatted_dep - for package in packages: + normalized_package_list = _resolve_package_names(module, packages, pip, py_bin) + + for package in normalized_package_list: is_present = _is_present(module, package, pkg_list, pkg_cmd) if (state == 'present' and not is_present) or (state == 'absent' and is_present): changed = True diff --git a/test/integration/targets/pip/tasks/main.yml b/test/integration/targets/pip/tasks/main.yml index 268cc8c28ba..ee9158b504a 100644 --- a/test/integration/targets/pip/tasks/main.yml +++ b/test/integration/targets/pip/tasks/main.yml @@ -23,6 +23,8 @@ - include_tasks: pip.yml + - include_tasks: url_packages.yml + - include_tasks: no_setuptools.yml always: - name: platform specific cleanup diff --git a/test/integration/targets/pip/tasks/url_packages.yml b/test/integration/targets/pip/tasks/url_packages.yml new file mode 100644 index 00000000000..ff9b96a1b09 --- /dev/null +++ b/test/integration/targets/pip/tasks/url_packages.yml @@ -0,0 +1,174 @@ +- name: Create local package + vars: + repo_path: "{{ remote_tmp_dir }}/pip_vcs_pkgs" + block: + - name: Create a nested package directory layout + file: + path: "{{ repo_path }}/dummy-root/dummy-subdirectory" + recurse: yes + state: directory + mode: '0755' + + - name: Create a root-level pyproject.toml + copy: + dest: "{{ repo_path }}/pyproject.toml" + content: | + [build-system] + requires = ["setuptools == 80.0.0"] + build-backend = "setuptools.build_meta" + + [project] + name = "dummy-root" + version = "0.0.1" + dependencies = [] + + - name: Create nested package's pyproject.toml + copy: + dest: "{{ repo_path }}/dummy-root/dummy-subdirectory/pyproject.toml" + content: | + [build-system] + requires = ["setuptools == 80.0.0"] + build-backend = "setuptools.build_meta" + + [project] + name = "nested-package" + version = "0.0.1" + dependencies = [] + + - name: Create git repository + command: + cmd: "{{ item }}" + args: + chdir: "{{ repo_path }}" + environment: + GIT_CONFIG_NOSYSTEM: 1 + loop: + - git init . + - git add . + - git -c user.name="Ansible Test" -c user.email="ansible@ansible.ansible" commit -m "First commit" + +- name: Setup venvs + # We test for correct handling of vcs packages only in pip versions >= 24.1 + # Despite dry-run being added in 22.2, it fails in several cases until 24.1 + vars: + modern_pip_venv_version: "25.2" + outdated_pip_venv_version: "24.0" + block: + - name: Set venv paths + set_fact: + modern_pip_venv_path: "{{ remote_tmp_dir }}/modernvenv" + outdated_pip_venv_path: "{{ remote_tmp_dir }}/outdatedvenv" + + - name: Create modern venv + pip: + name: pip=={{modern_pip_venv_version}} + virtualenv: "{{ modern_pip_venv_path }}" + + - name: Create outdated venv + pip: + name: pip=={{outdated_pip_venv_version}} + virtualenv: "{{ outdated_pip_venv_path }}" + +- name: Check vcs package installation and checking functionality + vars: + repo_path: "{{ remote_tmp_dir }}/pip_vcs_pkgs" + venv_path: "{{ modern_pip_venv_path }}" + block: + - name: Check installation of packages via git + pip: + name: "git+file://{{ repo_path }}/.git" + virtualenv: "{{ venv_path }}" + check_mode: true + register: check_git_install_to_empty_venv + environment: + FORCE_COLOR: "1" # Here to encourage pip to include ansi color escape codes to validate pip output parsing + + - name: Install package via git + pip: + name: "git+file://{{ repo_path }}/.git" + virtualenv: "{{ venv_path }}" + + - name: Re-check to ensure not changed + pip: + name: "git+file://{{ repo_path }}/.git" + virtualenv: "{{ venv_path }}" + check_mode: true + register: check_reinstall_one_pkg + + - name: Re-check with multiple to ensure changed + pip: + name: + - "git+file://{{ repo_path }}/.git" + - "nested-package @ git+file://{{ repo_path }}/.git#subdirectory=dummy-root/dummy-subdirectory" + virtualenv: "{{ venv_path }}" + check_mode: true + register: check_reinstall_two + + - name: Re-install dummy-root to ensure not changed + pip: + name: "git+file://{{ repo_path }}/.git" + virtualenv: "{{ venv_path }}" + check_mode: true + register: reinstall_git_pkg + + - name: Assert that git repo wasn't installed + assert: + that: + - check_git_install_to_empty_venv is changed + - check_reinstall_one_pkg is not changed + - check_reinstall_two is changed + - reinstall_git_pkg is not changed + + - name: Install nonsense git repo + pip: + name: "git+https://notawebsite.tld/doesntexist/doesntexist" + virtualenv: "{{ venv_path }}" + check_mode: true + register: check_failure_on_bad_repo + ignore_errors: true + + - name: Assert failure on nonsense git + assert: + that: check_failure_on_bad_repo is failed + + - name: Check installation of file-path package + pip: + virtualenv: "{{ venv_path }}" + name: "{{ repo_path }}" + check_mode: true + register: check_file_install_root + + - name: Check installation of file-path package + pip: + virtualenv: "{{ venv_path }}" + name: "{{ repo_path }}/dummy-root/dummy-subdirectory" + check_mode: true + register: check_file_install_subdirectory + + - name: Assert changed for file-path packages + assert: + that: + - check_file_install_root is not changed + - check_file_install_subdirectory is changed + +# This tests for default behavior in the outdated versions of pip +- name: Check for failure in handling vcs packages with outdated pip + vars: + repo_path: "{{ remote_tmp_dir }}/pip_vcs_pkgs" + venv_path: "{{ outdated_pip_venv_path }}" + block: + - name: Install file-path package to fresh outdated venv + pip: + name: "{{ repo_path }}" + virtualenv: "{{ venv_path }}" + + - name: Check installation of file-path package to venv + pip: + name: "{{ repo_path }}" + virtualenv: "{{ venv_path }}" + check_mode: true + register: pip_result + + - name: Assert improper name resolution due to outdated pip + assert: + that: pip_result is changed