Skip to content

Commit

Permalink
Consider all configured collection paths when installing collections (a…
Browse files Browse the repository at this point in the history
…nsible#81243)

* Only install collections which can't be satisfied by a collection in any of the configured paths.

* Improve warning for unexpected collection install path

Fix warning when path is configured, but is a pip-managed path

Normalize the path before validating to fix warning consistency
  • Loading branch information
s-hertel authored Jul 20, 2023
1 parent c5d18c3 commit efbc00b
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 8 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
bugfixes:
- >-
``ansible-galaxy`` now considers all collection paths when identifying which collection requirements are already installed.
Use the ``COLLECTIONS_PATHS`` and ``COLLECTIONS_SCAN_SYS_PATHS`` config options to modify these.
Previously only the install path was considered when resolving the candidates.
The install path will remain the only one potentially modified.
(https://github.com/ansible/ansible/issues/79767, https://github.com/ansible/ansible/issues/81163)
18 changes: 14 additions & 4 deletions lib/ansible/cli/galaxy.py
Original file line number Diff line number Diff line change
Expand Up @@ -1414,10 +1414,19 @@ def _execute_install_collection(
upgrade = context.CLIARGS.get('upgrade', False)

collections_path = C.COLLECTIONS_PATHS
if (
C.GALAXY_COLLECTIONS_PATH_WARNING
and len([p for p in collections_path if p.startswith(path)]) == 0
):

managed_paths = set(validate_collection_path(p) for p in C.COLLECTIONS_PATHS)
read_req_paths = set(validate_collection_path(p) for p in AnsibleCollectionConfig.collection_paths)

unexpected_path = C.GALAXY_COLLECTIONS_PATH_WARNING and not any(p.startswith(path) for p in managed_paths)
if unexpected_path and any(p.startswith(path) for p in read_req_paths):
display.warning(
f"The specified collections path '{path}' appears to be part of the pip Ansible package. "
"Managing these directly with ansible-galaxy could break the Ansible package. "
"Install collections to a configured collections path, which will take precedence over "
"collections found in the PYTHONPATH."
)
elif unexpected_path:
display.warning("The specified collections path '%s' is not part of the configured Ansible "
"collections paths '%s'. The installed collection will not be picked up in an Ansible "
"run, unless within a playbook-adjacent collections directory." % (to_text(path), to_text(":".join(collections_path))))
Expand All @@ -1434,6 +1443,7 @@ def _execute_install_collection(
artifacts_manager=artifacts_manager,
disable_gpg_verify=disable_gpg_verify,
offline=context.CLIARGS.get('offline', False),
read_requirement_paths=read_req_paths,
)

return 0
Expand Down
4 changes: 3 additions & 1 deletion lib/ansible/galaxy/collection/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -654,6 +654,7 @@ def install_collections(
artifacts_manager, # type: ConcreteArtifactsManager
disable_gpg_verify, # type: bool
offline, # type: bool
read_requirement_paths, # type: set[str]
): # type: (...) -> None
"""Install Ansible collections to the path specified.
Expand All @@ -668,7 +669,8 @@ def install_collections(
"""
existing_collections = {
Requirement(coll.fqcn, coll.ver, coll.src, coll.type, None)
for coll in find_existing_collections(output_path, artifacts_manager)
for path in {output_path} | read_requirement_paths
for coll in find_existing_collections(path, artifacts_manager)
}

unsatisfied_requirements = set(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
command: 'ansible-galaxy collection install {{ artifact_path }} -p {{ alt_install_path }} --no-deps'
vars:
artifact_path: "{{ galaxy_dir }}/ansible_test-collection_1-1.0.0.tar.gz"
environment:
ANSIBLE_COLLECTIONS_PATH: ""

- name: check if the files and folders in build_ignore were respected
stat:
Expand Down
9 changes: 6 additions & 3 deletions test/units/galaxy/test_collection_install.py
Original file line number Diff line number Diff line change
Expand Up @@ -822,7 +822,8 @@ def test_install_collections_from_tar(collection_artifact, monkeypatch):
concrete_artifact_cm = collection.concrete_artifact_manager.ConcreteArtifactsManager(temp_path, validate_certs=False)

requirements = [Requirement('ansible_namespace.collection', '0.1.0', to_text(collection_tar), 'file', None)]
collection.install_collections(requirements, to_text(temp_path), [], False, False, False, False, False, False, concrete_artifact_cm, True, False)
collection.install_collections(
requirements, to_text(temp_path), [], False, False, False, False, False, False, concrete_artifact_cm, True, False, set())

assert os.path.isdir(collection_path)

Expand Down Expand Up @@ -860,7 +861,8 @@ def test_install_collection_with_circular_dependency(collection_artifact, monkey

concrete_artifact_cm = collection.concrete_artifact_manager.ConcreteArtifactsManager(temp_path, validate_certs=False)
requirements = [Requirement('ansible_namespace.collection', '0.1.0', to_text(collection_tar), 'file', None)]
collection.install_collections(requirements, to_text(temp_path), [], False, False, False, False, False, False, concrete_artifact_cm, True, False)
collection.install_collections(
requirements, to_text(temp_path), [], False, False, False, False, False, False, concrete_artifact_cm, True, False, set())

assert os.path.isdir(collection_path)

Expand Down Expand Up @@ -897,7 +899,8 @@ def test_install_collection_with_no_dependency(collection_artifact, monkeypatch)

concrete_artifact_cm = collection.concrete_artifact_manager.ConcreteArtifactsManager(temp_path, validate_certs=False)
requirements = [Requirement('ansible_namespace.collection', '0.1.0', to_text(collection_tar), 'file', None)]
collection.install_collections(requirements, to_text(temp_path), [], False, False, False, False, False, False, concrete_artifact_cm, True, False)
collection.install_collections(
requirements, to_text(temp_path), [], False, False, False, False, False, False, concrete_artifact_cm, True, False, set())

assert os.path.isdir(collection_path)

Expand Down

0 comments on commit efbc00b

Please sign in to comment.