From d0f794d1f66ef7264dae7312e230558641b7489a Mon Sep 17 00:00:00 2001 From: Sloane Hertel <19572925+s-hertel@users.noreply.github.com> Date: Mon, 14 Oct 2024 11:59:22 -0400 Subject: [PATCH] ansible-galaxy - fix ignoring certs when installing from git repos (#83332) (#84070) * Fix installing collections|roles from git repos with GALAXY_IGNORE_CERTS * Fix installing collections from git repos with --ignore-certs * Update unit test * Add test case (cherry picked from commit d0df3a174a7fd79f91ed88dbb15e9999fa7d927b) --- .../fix-ansible-galaxy-ignore-certs.yml | 2 + .../collection/concrete_artifact_manager.py | 11 +++-- lib/ansible/utils/galaxy.py | 2 +- test/units/galaxy/test_collection_install.py | 40 ++++++++++++++++++- 4 files changed, 49 insertions(+), 6 deletions(-) create mode 100644 changelogs/fragments/fix-ansible-galaxy-ignore-certs.yml diff --git a/changelogs/fragments/fix-ansible-galaxy-ignore-certs.yml b/changelogs/fragments/fix-ansible-galaxy-ignore-certs.yml new file mode 100644 index 00000000000..aba789bdadd --- /dev/null +++ b/changelogs/fragments/fix-ansible-galaxy-ignore-certs.yml @@ -0,0 +1,2 @@ +bugfixes: + - Fix disabling SSL verification when installing collections and roles from git repositories. If ``--ignore-certs`` isn't provided, the value for the ``GALAXY_IGNORE_CERTS`` configuration option will be used (https://github.com/ansible/ansible/issues/83326). diff --git a/lib/ansible/galaxy/collection/concrete_artifact_manager.py b/lib/ansible/galaxy/collection/concrete_artifact_manager.py index 06c1cf6f93b..f54e4d686e5 100644 --- a/lib/ansible/galaxy/collection/concrete_artifact_manager.py +++ b/lib/ansible/galaxy/collection/concrete_artifact_manager.py @@ -10,6 +10,7 @@ import os import tarfile import subprocess import typing as t +import yaml from contextlib import contextmanager from hashlib import sha256 @@ -24,6 +25,7 @@ if t.TYPE_CHECKING: ) from ansible.galaxy.token import GalaxyToken +from ansible import context from ansible.errors import AnsibleError from ansible.galaxy import get_collections_galaxy_meta_info from ansible.galaxy.api import should_retry_error @@ -38,7 +40,7 @@ from ansible.module_utils.urls import open_url from ansible.utils.display import Display from ansible.utils.sentinel import Sentinel -import yaml +import ansible.constants as C display = Display() @@ -425,11 +427,14 @@ def _extract_collection_from_git(repo_url, coll_ver, b_path): # Perform a shallow clone if simply cloning HEAD if version == 'HEAD': - git_clone_cmd = git_executable, 'clone', '--depth=1', git_url, to_text(b_checkout_path) + git_clone_cmd = [git_executable, 'clone', '--depth=1', git_url, to_text(b_checkout_path)] else: - git_clone_cmd = git_executable, 'clone', git_url, to_text(b_checkout_path) + git_clone_cmd = [git_executable, 'clone', git_url, to_text(b_checkout_path)] # FIXME: '--branch', version + if context.CLIARGS['ignore_certs'] or C.GALAXY_IGNORE_CERTS: + git_clone_cmd.extend(['-c', 'http.sslVerify=false']) + try: subprocess.check_call(git_clone_cmd) except subprocess.CalledProcessError as proc_err: diff --git a/lib/ansible/utils/galaxy.py b/lib/ansible/utils/galaxy.py index 977ae2cbd0a..4c2f81cd0bc 100644 --- a/lib/ansible/utils/galaxy.py +++ b/lib/ansible/utils/galaxy.py @@ -64,7 +64,7 @@ def scm_archive_resource(src, scm='git', name=None, version='HEAD', keep_scm_met clone_cmd = [scm_path, 'clone'] # Add specific options for ignoring certificates if requested - ignore_certs = context.CLIARGS['ignore_certs'] + ignore_certs = context.CLIARGS['ignore_certs'] or C.GALAXY_IGNORE_CERTS if ignore_certs: if scm == 'git': diff --git a/test/units/galaxy/test_collection_install.py b/test/units/galaxy/test_collection_install.py index 9988dafdc1a..dc6dbe5b6f3 100644 --- a/test/units/galaxy/test_collection_install.py +++ b/test/units/galaxy/test_collection_install.py @@ -30,6 +30,8 @@ from ansible.module_utils.common.process import get_bin_path from ansible.utils import context_objects as co from ansible.utils.display import Display +import ansible.constants as C + class RequirementCandidates(): def __init__(self): @@ -127,6 +129,7 @@ def test_concrete_artifact_manager_scm_no_executable(monkeypatch): ] ) def test_concrete_artifact_manager_scm_cmd(url, version, trailing_slash, monkeypatch): + context.CLIARGS._store = {'ignore_certs': False} mock_subprocess_check_call = MagicMock() monkeypatch.setattr(collection.concrete_artifact_manager.subprocess, 'check_call', mock_subprocess_check_call) mock_mkdtemp = MagicMock(return_value='') @@ -141,7 +144,7 @@ def test_concrete_artifact_manager_scm_cmd(url, version, trailing_slash, monkeyp repo += '/' git_executable = get_bin_path('git') - clone_cmd = (git_executable, 'clone', repo, '') + clone_cmd = [git_executable, 'clone', repo, ''] assert mock_subprocess_check_call.call_args_list[0].args[0] == clone_cmd assert mock_subprocess_check_call.call_args_list[1].args[0] == (git_executable, 'checkout', 'commitish') @@ -158,6 +161,7 @@ def test_concrete_artifact_manager_scm_cmd(url, version, trailing_slash, monkeyp ] ) def test_concrete_artifact_manager_scm_cmd_shallow(url, version, trailing_slash, monkeypatch): + context.CLIARGS._store = {'ignore_certs': False} mock_subprocess_check_call = MagicMock() monkeypatch.setattr(collection.concrete_artifact_manager.subprocess, 'check_call', mock_subprocess_check_call) mock_mkdtemp = MagicMock(return_value='') @@ -171,12 +175,44 @@ def test_concrete_artifact_manager_scm_cmd_shallow(url, version, trailing_slash, if trailing_slash: repo += '/' git_executable = get_bin_path('git') - shallow_clone_cmd = (git_executable, 'clone', '--depth=1', repo, '') + shallow_clone_cmd = [git_executable, 'clone', '--depth=1', repo, ''] assert mock_subprocess_check_call.call_args_list[0].args[0] == shallow_clone_cmd assert mock_subprocess_check_call.call_args_list[1].args[0] == (git_executable, 'checkout', 'HEAD') +@pytest.mark.parametrize( + 'ignore_certs_cli,ignore_certs_config,expected_ignore_certs', + [ + (False, False, False), + (False, True, True), + (True, False, True), + ] +) +def test_concrete_artifact_manager_scm_cmd_validate_certs(ignore_certs_cli, ignore_certs_config, expected_ignore_certs, monkeypatch): + context.CLIARGS._store = {'ignore_certs': ignore_certs_cli} + monkeypatch.setattr(C, 'GALAXY_IGNORE_CERTS', ignore_certs_config) + + mock_subprocess_check_call = MagicMock() + monkeypatch.setattr(collection.concrete_artifact_manager.subprocess, 'check_call', mock_subprocess_check_call) + mock_mkdtemp = MagicMock(return_value='') + monkeypatch.setattr(collection.concrete_artifact_manager, 'mkdtemp', mock_mkdtemp) + + url = 'https://github.com/org/repo' + version = 'HEAD' + collection.concrete_artifact_manager._extract_collection_from_git(url, version, b'path') + + assert mock_subprocess_check_call.call_count == 2 + + git_executable = get_bin_path('git') + clone_cmd = [git_executable, 'clone', '--depth=1', url, ''] + if expected_ignore_certs: + clone_cmd.extend(['-c', 'http.sslVerify=false']) + + assert mock_subprocess_check_call.call_args_list[0].args[0] == clone_cmd + assert mock_subprocess_check_call.call_args_list[1].args[0] == (git_executable, 'checkout', 'HEAD') + + def test_build_requirement_from_path(collection_artifact): tmp_path = os.path.join(os.path.split(collection_artifact[1])[0], b'temp') concrete_artifact_cm = collection.concrete_artifact_manager.ConcreteArtifactsManager(tmp_path, validate_certs=False)