Fix pip package name resolution in check mode (#85623)

Co-authored-by: Abhijeet Kasurde <akasurde@redhat.com>
Co-authored-by: Sviatoslav Sydorenko (Святослав Сидоренко) <webknjaz@redhat.com>

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)

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

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

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