From c83669a83e55ee500d9defe2f29affd64993f106 Mon Sep 17 00:00:00 2001 From: Sloane Hertel <19572925+s-hertel@users.noreply.github.com> Date: Wed, 18 Jan 2023 17:22:49 -0500 Subject: [PATCH] ansible-galaxy - fix turning off the ConcreteArtifactManager's validate certs at the global level (#79561) (#79696) Fix ignoring certs when downloading tarballs Fix ignoring certs when downloading a collection from a specific source that isn't in the configured servers list (cherry picked from commit acbf4cc60e9338dc08421c8355d69bfcdfde0280) --- .../79561-fix-a-g-global-ignore-certs-cfg.yml | 3 + lib/ansible/cli/galaxy.py | 19 ++-- test/units/galaxy/test_collection.py | 98 ++++++++++++------- 3 files changed, 72 insertions(+), 48 deletions(-) create mode 100644 changelogs/fragments/79561-fix-a-g-global-ignore-certs-cfg.yml diff --git a/changelogs/fragments/79561-fix-a-g-global-ignore-certs-cfg.yml b/changelogs/fragments/79561-fix-a-g-global-ignore-certs-cfg.yml new file mode 100644 index 00000000000..6abb50ad85c --- /dev/null +++ b/changelogs/fragments/79561-fix-a-g-global-ignore-certs-cfg.yml @@ -0,0 +1,3 @@ +bugfixes: + - Fix using ``GALAXY_IGNORE_CERTS`` when downloading tarballs from Galaxy servers (https://github.com/ansible/ansible/issues/79557). + - Fix using ``GALAXY_IGNORE_CERTS`` in conjunction with collections in requirements files which specify a specific ``source`` that isn't in the configured servers. diff --git a/lib/ansible/cli/galaxy.py b/lib/ansible/cli/galaxy.py index bc691b2859f..3cb7fe2c0b5 100755 --- a/lib/ansible/cli/galaxy.py +++ b/lib/ansible/cli/galaxy.py @@ -79,15 +79,11 @@ SERVER_DEF = [ # config definition fields SERVER_ADDITIONAL = { 'v3': {'default': 'False'}, - 'validate_certs': {'default': True, 'cli': [{'name': 'validate_certs'}]}, + 'validate_certs': {'cli': [{'name': 'validate_certs'}]}, 'timeout': {'default': '60', 'cli': [{'name': 'timeout'}]}, 'token': {'default': None}, } -# override default if the generic is set -if C.GALAXY_IGNORE_CERTS is not None: - SERVER_ADDITIONAL['validate_certs'].update({'default': not C.GALAXY_IGNORE_CERTS}) - def with_collection_artifacts_manager(wrapped_method): """Inject an artifacts manager if not passed explicitly. @@ -100,7 +96,8 @@ def with_collection_artifacts_manager(wrapped_method): if 'artifacts_manager' in kwargs: return wrapped_method(*args, **kwargs) - artifacts_manager_kwargs = {'validate_certs': context.CLIARGS['validate_certs']} + # FIXME: use validate_certs context from Galaxy servers when downloading collections + artifacts_manager_kwargs = {'validate_certs': context.CLIARGS['resolved_validate_certs']} keyring = context.CLIARGS.get('keyring', None) if keyring is not None: @@ -588,6 +585,8 @@ class GalaxyCLI(CLI): # ensure we have 'usable' cli option setattr(options, 'validate_certs', (None if options.ignore_certs is None else not options.ignore_certs)) + # the default if validate_certs is None + setattr(options, 'resolved_validate_certs', (options.validate_certs if options.validate_certs is not None else not C.GALAXY_IGNORE_CERTS)) display.verbosity = options.verbosity return options @@ -645,6 +644,8 @@ class GalaxyCLI(CLI): token_val = server_options['token'] or NoTokenSentinel username = server_options['username'] v3 = server_options.pop('v3') + if server_options['validate_certs'] is None: + server_options['validate_certs'] = context.CLIARGS['resolved_validate_certs'] validate_certs = server_options['validate_certs'] if v3: @@ -680,9 +681,7 @@ class GalaxyCLI(CLI): cmd_server = context.CLIARGS['api_server'] cmd_token = GalaxyToken(token=context.CLIARGS['api_key']) - # resolve validate_certs - v_config_default = True if C.GALAXY_IGNORE_CERTS is None else not C.GALAXY_IGNORE_CERTS - validate_certs = v_config_default if context.CLIARGS['validate_certs'] is None else context.CLIARGS['validate_certs'] + validate_certs = context.CLIARGS['resolved_validate_certs'] if cmd_server: # Cmd args take precedence over the config entry but fist check if the arg was a name and use that config # entry, otherwise create a new API entry for the server specified. @@ -848,7 +847,7 @@ class GalaxyCLI(CLI): name=coll_req['name'], ), coll_req['source'], - validate_certs=not context.CLIARGS['ignore_certs'], + validate_certs=context.CLIARGS['resolved_validate_certs'], ), ) diff --git a/test/units/galaxy/test_collection.py b/test/units/galaxy/test_collection.py index 28a69b2814a..106251c5eab 100644 --- a/test/units/galaxy/test_collection.py +++ b/test/units/galaxy/test_collection.py @@ -201,24 +201,6 @@ def manifest(manifest_info): yield fake_file, sha256(b_data).hexdigest() -@pytest.fixture() -def server_config(monkeypatch): - monkeypatch.setattr(C, 'GALAXY_SERVER_LIST', ['server1', 'server2', 'server3']) - - default_options = dict((k, None) for k, v, t in SERVER_DEF) - - server1 = dict(default_options) - server1.update({'url': 'https://galaxy.ansible.com/api/', 'validate_certs': False}) - - server2 = dict(default_options) - server2.update({'url': 'https://galaxy.ansible.com/api/', 'validate_certs': True}) - - server3 = dict(default_options) - server3.update({'url': 'https://galaxy.ansible.com/api/'}) - - return server1, server2, server3 - - @pytest.mark.parametrize( 'required_signature_count,valid', [ @@ -340,8 +322,18 @@ def test_validate_certs(global_ignore_certs, monkeypatch): assert galaxy_cli.api_servers[0].validate_certs is not global_ignore_certs -@pytest.mark.parametrize('global_ignore_certs', [True, False]) -def test_validate_certs_with_server_url(global_ignore_certs, monkeypatch): +@pytest.mark.parametrize( + ["ignore_certs_cli", "ignore_certs_cfg", "expected_validate_certs"], + [ + (None, None, True), + (None, True, False), + (None, False, True), + (True, None, False), + (True, True, False), + (True, False, False), + ] +) +def test_validate_certs_with_server_url(ignore_certs_cli, ignore_certs_cfg, expected_validate_certs, monkeypatch): cli_args = [ 'ansible-galaxy', 'collection', @@ -350,8 +342,10 @@ def test_validate_certs_with_server_url(global_ignore_certs, monkeypatch): '-s', 'https://galaxy.ansible.com' ] - if global_ignore_certs: + if ignore_certs_cli: cli_args.append('--ignore-certs') + if ignore_certs_cfg is not None: + monkeypatch.setattr(C, 'GALAXY_IGNORE_CERTS', ignore_certs_cfg) galaxy_cli = GalaxyCLI(args=cli_args) mock_execute_install = MagicMock() @@ -359,34 +353,62 @@ def test_validate_certs_with_server_url(global_ignore_certs, monkeypatch): galaxy_cli.run() assert len(galaxy_cli.api_servers) == 1 - assert galaxy_cli.api_servers[0].validate_certs is not global_ignore_certs - - -@pytest.mark.parametrize('global_ignore_certs', [True, False]) -def test_validate_certs_with_server_config(global_ignore_certs, server_config, monkeypatch): + assert galaxy_cli.api_servers[0].validate_certs == expected_validate_certs - # test sidesteps real resolution and forces the server config to override the cli option - get_plugin_options = MagicMock(side_effect=server_config) - monkeypatch.setattr(C.config, 'get_plugin_options', get_plugin_options) +@pytest.mark.parametrize( + ["ignore_certs_cli", "ignore_certs_cfg", "expected_server2_validate_certs", "expected_server3_validate_certs"], + [ + (None, None, True, True), + (None, True, True, False), + (None, False, True, True), + (True, None, False, False), + (True, True, False, False), + (True, False, False, False), + ] +) +def test_validate_certs_server_config(ignore_certs_cfg, ignore_certs_cli, expected_server2_validate_certs, expected_server3_validate_certs, monkeypatch): + server_names = ['server1', 'server2', 'server3'] + cfg_lines = [ + "[galaxy]", + "server_list=server1,server2,server3", + "[galaxy_server.server1]", + "url=https://galaxy.ansible.com/api/", + "validate_certs=False", + "[galaxy_server.server2]", + "url=https://galaxy.ansible.com/api/", + "validate_certs=True", + "[galaxy_server.server3]", + "url=https://galaxy.ansible.com/api/", + ] cli_args = [ 'ansible-galaxy', 'collection', 'install', 'namespace.collection:1.0.0', ] - if global_ignore_certs: + if ignore_certs_cli: cli_args.append('--ignore-certs') + if ignore_certs_cfg is not None: + monkeypatch.setattr(C, 'GALAXY_IGNORE_CERTS', ignore_certs_cfg) - galaxy_cli = GalaxyCLI(args=cli_args) - mock_execute_install = MagicMock() - monkeypatch.setattr(galaxy_cli, '_execute_install_collection', mock_execute_install) - galaxy_cli.run() + monkeypatch.setattr(C, 'GALAXY_SERVER_LIST', server_names) + + with tempfile.NamedTemporaryFile(suffix='.cfg') as tmp_file: + tmp_file.write(to_bytes('\n'.join(cfg_lines), errors='surrogate_or_strict')) + tmp_file.flush() + + monkeypatch.setattr(C.config, '_config_file', tmp_file.name) + C.config._parse_config_file() + galaxy_cli = GalaxyCLI(args=cli_args) + mock_execute_install = MagicMock() + monkeypatch.setattr(galaxy_cli, '_execute_install_collection', mock_execute_install) + galaxy_cli.run() - # server cfg, so should match def above, if not specified so it should use default (true) - assert galaxy_cli.api_servers[0].validate_certs is server_config[0].get('validate_certs', True) - assert galaxy_cli.api_servers[1].validate_certs is server_config[1].get('validate_certs', True) - assert galaxy_cli.api_servers[2].validate_certs is server_config[2].get('validate_certs', True) + # (not) --ignore-certs > server's validate_certs > (not) GALAXY_IGNORE_CERTS > True + assert galaxy_cli.api_servers[0].validate_certs is False + assert galaxy_cli.api_servers[1].validate_certs is expected_server2_validate_certs + assert galaxy_cli.api_servers[2].validate_certs is expected_server3_validate_certs def test_build_collection_no_galaxy_yaml():