From ccb00b74fe429e202c12c77ce8104c2cb348670b Mon Sep 17 00:00:00 2001 From: Sviatoslav Sydorenko Date: Tue, 3 Oct 2023 11:29:59 -0400 Subject: [PATCH] [backport][stable-2.15] Always allow "no-other-choice" pre-release dependencies when resolving collection dependency tree (#81747) * Always allow "no-other-choice" pre-release dependencies when resolving collection dependency tree PR #81606. Prior to this patch, when `--pre` CLI flag was not passed, the dependency resolver would treat concrete collection dependency candidates (Git repositories, subdirs, tarball URLs, or local dirs or files etc) as not meeting the requirements. This patch makes it so pre-releases in any concrete artifact references, and the ones being specifically pinned dependencies or user requests, met anywhere in the dependency tree, are allowed unconditionally. This is achieved by moving the pre-release check from `is_satisfied_by()` to the `find_matches()` hook, following the Pip's example. As a bonus, this change also fixes the situation when a collection pre-releases weren't considered if it didn't have any stable releases. This now works even if `--pre` wasn't requested explicitly. Finally, this patch partially reverts commit 6f4b4c345b44d84cce55ba6502a72b420a52c7b3, except for the tests. And it also improves the `--pre` hint warning to explain that it mostly affects Galaxy/Automation Hub-hosted collection releases. Ref #73416 Ref #79112 Fixes #79168 Fixes #80048 Resolves #81605 Co-authored-by: Sloane Hertel <19572925+s-hertel@users.noreply.github.com> (cherry picked from commit 7662a050853b41189a66c9a840091e9b05bcba49) * Shorten the collection namespace and name @ tests This is needed on the 2.15 branch which uses older galaxy containers that have restricted FQCN size. --- ...ansible-galaxy-collection-pre-releases.yml | 16 +++ lib/ansible/galaxy/collection/__init__.py | 1 - .../galaxy/dependency_resolution/__init__.py | 7 +- .../dependency_resolution/dataclasses.py | 21 +++ .../galaxy/dependency_resolution/providers.py | 130 ++++++++---------- .../ansible-galaxy-collection/tasks/main.yml | 11 ++ .../tasks/pinned_pre_releases_in_deptree.yml | 79 +++++++++++ .../tasks/virtual_direct_requests.yml | 77 +++++++++++ .../ansible-galaxy-collection/vars/main.yml | 38 +++++ 9 files changed, 304 insertions(+), 76 deletions(-) create mode 100644 changelogs/fragments/81606-ansible-galaxy-collection-pre-releases.yml create mode 100644 test/integration/targets/ansible-galaxy-collection/tasks/pinned_pre_releases_in_deptree.yml create mode 100644 test/integration/targets/ansible-galaxy-collection/tasks/virtual_direct_requests.yml diff --git a/changelogs/fragments/81606-ansible-galaxy-collection-pre-releases.yml b/changelogs/fragments/81606-ansible-galaxy-collection-pre-releases.yml new file mode 100644 index 00000000000..129ccfd2507 --- /dev/null +++ b/changelogs/fragments/81606-ansible-galaxy-collection-pre-releases.yml @@ -0,0 +1,16 @@ +--- + +bugfixes: +- >- + ansible-galaxy - started allowing the use of pre-releases + for dependencies on any level of the dependency tree that + specifically demand exact pre-release versions of + collections and not version ranges. + (https://github.com/ansible/ansible/pull/81606) +- >- + ansible-galaxy - started allowing the use of pre-releases + for collections that do not have any stable versions + published. + (https://github.com/ansible/ansible/pull/81606) + +... diff --git a/lib/ansible/galaxy/collection/__init__.py b/lib/ansible/galaxy/collection/__init__.py index 5257c705984..36e4c487bcf 100644 --- a/lib/ansible/galaxy/collection/__init__.py +++ b/lib/ansible/galaxy/collection/__init__.py @@ -1814,7 +1814,6 @@ def _resolve_depenency_map( collection_dep_resolver = build_collection_dependency_resolver( galaxy_apis=galaxy_apis, concrete_artifacts_manager=concrete_artifacts_manager, - user_requirements=requested_requirements, preferred_candidates=preferred_candidates, with_deps=not no_deps, with_pre_releases=allow_pre_release, diff --git a/lib/ansible/galaxy/dependency_resolution/__init__.py b/lib/ansible/galaxy/dependency_resolution/__init__.py index cfde7df0f98..eeffd299648 100644 --- a/lib/ansible/galaxy/dependency_resolution/__init__.py +++ b/lib/ansible/galaxy/dependency_resolution/__init__.py @@ -13,10 +13,7 @@ if t.TYPE_CHECKING: from ansible.galaxy.collection.concrete_artifact_manager import ( ConcreteArtifactsManager, ) - from ansible.galaxy.dependency_resolution.dataclasses import ( - Candidate, - Requirement, - ) + from ansible.galaxy.dependency_resolution.dataclasses import Candidate from ansible.galaxy.collection.galaxy_api_proxy import MultiGalaxyAPIProxy from ansible.galaxy.dependency_resolution.providers import CollectionDependencyProvider @@ -27,7 +24,6 @@ from ansible.galaxy.dependency_resolution.resolvers import CollectionDependencyR def build_collection_dependency_resolver( galaxy_apis, # type: t.Iterable[GalaxyAPI] concrete_artifacts_manager, # type: ConcreteArtifactsManager - user_requirements, # type: t.Iterable[Requirement] preferred_candidates=None, # type: t.Iterable[Candidate] with_deps=True, # type: bool with_pre_releases=False, # type: bool @@ -44,7 +40,6 @@ def build_collection_dependency_resolver( CollectionDependencyProvider( apis=MultiGalaxyAPIProxy(galaxy_apis, concrete_artifacts_manager, offline=offline), concrete_artifacts_manager=concrete_artifacts_manager, - user_requirements=user_requirements, preferred_candidates=preferred_candidates, with_deps=with_deps, with_pre_releases=with_pre_releases, diff --git a/lib/ansible/galaxy/dependency_resolution/dataclasses.py b/lib/ansible/galaxy/dependency_resolution/dataclasses.py index e1940d771a6..393d729af7e 100644 --- a/lib/ansible/galaxy/dependency_resolution/dataclasses.py +++ b/lib/ansible/galaxy/dependency_resolution/dataclasses.py @@ -564,6 +564,27 @@ class _ComputedReqKindsMixin: def is_online_index_pointer(self): return not self.is_concrete_artifact + @property + def is_pinned(self): + """Indicate if the version set is considered pinned. + + This essentially computes whether the version field of the current + requirement explicitly requests a specific version and not an allowed + version range. + + It is then used to help the resolvelib-based dependency resolver judge + whether it's acceptable to consider a pre-release candidate version + despite pre-release installs not being requested by the end-user + explicitly. + + See https://github.com/ansible/ansible/pull/81606 for extra context. + """ + version_string = self.ver[0] + return version_string.isdigit() or not ( + version_string == '*' or + version_string.startswith(('<', '>', '!=')) + ) + @property def source_info(self): return self._source_info diff --git a/lib/ansible/galaxy/dependency_resolution/providers.py b/lib/ansible/galaxy/dependency_resolution/providers.py index 19fb2acae54..f13d3ecf2df 100644 --- a/lib/ansible/galaxy/dependency_resolution/providers.py +++ b/lib/ansible/galaxy/dependency_resolution/providers.py @@ -51,7 +51,6 @@ class CollectionDependencyProviderBase(AbstractProvider): self, # type: CollectionDependencyProviderBase apis, # type: MultiGalaxyAPIProxy concrete_artifacts_manager=None, # type: ConcreteArtifactsManager - user_requirements=None, # type: t.Iterable[Requirement] preferred_candidates=None, # type: t.Iterable[Candidate] with_deps=True, # type: bool with_pre_releases=False, # type: bool @@ -87,58 +86,12 @@ class CollectionDependencyProviderBase(AbstractProvider): Requirement.from_requirement_dict, art_mgr=concrete_artifacts_manager, ) - self._pinned_candidate_requests = set( - # NOTE: User-provided signatures are supplemental, so signatures - # NOTE: are not used to determine if a candidate is user-requested - Candidate(req.fqcn, req.ver, req.src, req.type, None) - for req in (user_requirements or ()) - if req.is_concrete_artifact or ( - req.ver != '*' and - not req.ver.startswith(('<', '>', '!=')) - ) - ) self._preferred_candidates = set(preferred_candidates or ()) self._with_deps = with_deps self._with_pre_releases = with_pre_releases self._upgrade = upgrade self._include_signatures = include_signatures - def _is_user_requested(self, candidate): # type: (Candidate) -> bool - """Check if the candidate is requested by the user.""" - if candidate in self._pinned_candidate_requests: - return True - - if candidate.is_online_index_pointer and candidate.src is not None: - # NOTE: Candidate is a namedtuple, it has a source server set - # NOTE: to a specific GalaxyAPI instance or `None`. When the - # NOTE: user runs - # NOTE: - # NOTE: $ ansible-galaxy collection install ns.coll - # NOTE: - # NOTE: then it's saved in `self._pinned_candidate_requests` - # NOTE: as `('ns.coll', '*', None, 'galaxy')` but then - # NOTE: `self.find_matches()` calls `self.is_satisfied_by()` - # NOTE: with Candidate instances bound to each specific - # NOTE: server available, those look like - # NOTE: `('ns.coll', '*', GalaxyAPI(...), 'galaxy')` and - # NOTE: wouldn't match the user requests saved in - # NOTE: `self._pinned_candidate_requests`. This is why we - # NOTE: normalize the collection to have `src=None` and try - # NOTE: again. - # NOTE: - # NOTE: When the user request comes from `requirements.yml` - # NOTE: with the `source:` set, it'll match the first check - # NOTE: but it still can have entries with `src=None` so this - # NOTE: normalized check is still necessary. - # NOTE: - # NOTE: User-provided signatures are supplemental, so signatures - # NOTE: are not used to determine if a candidate is user-requested - return Candidate( - candidate.fqcn, candidate.ver, None, candidate.type, None - ) in self._pinned_candidate_requests - - return False - def identify(self, requirement_or_candidate): # type: (t.Union[Candidate, Requirement]) -> str """Given requirement or candidate, return an identifier for it. @@ -342,25 +295,79 @@ class CollectionDependencyProviderBase(AbstractProvider): latest_matches = [] signatures = [] extra_signature_sources = [] # type: list[str] + + discarding_pre_releases_acceptable = any( + not is_pre_release(candidate_version) + for candidate_version, _src_server in coll_versions + ) + + # NOTE: The optimization of conditionally looping over the requirements + # NOTE: is used to skip having to compute the pinned status of all + # NOTE: requirements and apply version normalization to the found ones. + all_pinned_requirement_version_numbers = { + # NOTE: Pinned versions can start with a number, but also with an + # NOTE: equals sign. Stripping it at the beginning should be + # NOTE: enough. If there's a space after equals, the second strip + # NOTE: will take care of it. + # NOTE: Without this conversion, requirements versions like + # NOTE: '1.2.3-alpha.4' work, but '=1.2.3-alpha.4' don't. + requirement.ver.lstrip('=').strip() + for requirement in requirements + if requirement.is_pinned + } if discarding_pre_releases_acceptable else set() + for version, src_server in coll_versions: tmp_candidate = Candidate(fqcn, version, src_server, 'galaxy', None) - unsatisfied = False for requirement in requirements: - unsatisfied |= not self.is_satisfied_by(requirement, tmp_candidate) + candidate_satisfies_requirement = self.is_satisfied_by( + requirement, tmp_candidate, + ) + if not candidate_satisfies_requirement: + break + + should_disregard_pre_release_candidate = ( + # NOTE: Do not discard pre-release candidates in the + # NOTE: following cases: + # NOTE: * the end-user requested pre-releases explicitly; + # NOTE: * the candidate is a concrete artifact (e.g. a + # NOTE: Git repository, subdirs, a tarball URL, or a + # NOTE: local dir or file etc.); + # NOTE: * the candidate's pre-release version exactly + # NOTE: matches a version specifically requested by one + # NOTE: of the requirements in the current match + # NOTE: discovery round (i.e. matching a requirement + # NOTE: that is not a range but an explicit specific + # NOTE: version pin). This works when some requirements + # NOTE: request version ranges but others (possibly on + # NOTE: different dependency tree level depths) demand + # NOTE: pre-release dependency versions, even if those + # NOTE: dependencies are transitive. + is_pre_release(tmp_candidate.ver) + and discarding_pre_releases_acceptable + and not ( + self._with_pre_releases + or tmp_candidate.is_concrete_artifact + or version in all_pinned_requirement_version_numbers + ) + ) + if should_disregard_pre_release_candidate: + break + # FIXME - # unsatisfied |= not self.is_satisfied_by(requirement, tmp_candidate) or not ( - # requirement.src is None or # if this is true for some candidates but not all it will break key param - Nonetype can't be compared to str + # candidate_is_from_requested_source = ( + # requirement.src is None # if this is true for some candidates but not all it will break key param - Nonetype can't be compared to str # or requirement.src == candidate.src # ) - if unsatisfied: - break + # if not candidate_is_from_requested_source: + # break + if not self._include_signatures: continue extra_signature_sources.extend(requirement.signature_sources or []) - if not unsatisfied: + else: # candidate satisfies requirements, `break` never happened if self._include_signatures: for extra_source in extra_signature_sources: signatures.append(get_signature_from_source(extra_source)) @@ -405,21 +412,6 @@ class CollectionDependencyProviderBase(AbstractProvider): :returns: Indication whether the `candidate` is a viable \ solution to the `requirement`. """ - # NOTE: Only allow pre-release candidates if we want pre-releases - # NOTE: or the req ver was an exact match with the pre-release - # NOTE: version. Another case where we'd want to allow - # NOTE: pre-releases is when there are several user requirements - # NOTE: and one of them is a pre-release that also matches a - # NOTE: transitive dependency of another requirement. - allow_pre_release = self._with_pre_releases or not ( - requirement.ver == '*' or - requirement.ver.startswith('<') or - requirement.ver.startswith('>') or - requirement.ver.startswith('!=') - ) or self._is_user_requested(candidate) - if is_pre_release(candidate.ver) and not allow_pre_release: - return False - # NOTE: This is a set of Pipenv-inspired optimizations. Ref: # https://github.com/sarugaku/passa/blob/2ac00f1/src/passa/models/providers.py#L58-L74 if ( diff --git a/test/integration/targets/ansible-galaxy-collection/tasks/main.yml b/test/integration/targets/ansible-galaxy-collection/tasks/main.yml index 724c861e696..185bde8fbe8 100644 --- a/test/integration/targets/ansible-galaxy-collection/tasks/main.yml +++ b/test/integration/targets/ansible-galaxy-collection/tasks/main.yml @@ -135,6 +135,17 @@ loop_control: loop_var: resolvelib_version +- name: test choosing pinned pre-releases anywhere in the dependency tree + # This is a regression test for the case when the end-user does not + # explicitly allow installing pre-release collection versions, but their + # precise pins are still selected if met among the dependencies, either + # direct or transitive. + include_tasks: pinned_pre_releases_in_deptree.yml + +- name: test installing prereleases via scm direct requests + # In this test suite because the bug relies on the dep having versions on a Galaxy server + include_tasks: virtual_direct_requests.yml + - name: publish collection with a dep on another server setup_collections: server: secondary diff --git a/test/integration/targets/ansible-galaxy-collection/tasks/pinned_pre_releases_in_deptree.yml b/test/integration/targets/ansible-galaxy-collection/tasks/pinned_pre_releases_in_deptree.yml new file mode 100644 index 00000000000..60a0a796cc5 --- /dev/null +++ b/test/integration/targets/ansible-galaxy-collection/tasks/pinned_pre_releases_in_deptree.yml @@ -0,0 +1,79 @@ +--- + +- name: >- + test that the dependency resolver chooses pre-releases if they are pinned + environment: + ANSIBLE_COLLECTIONS_PATH: '{{ galaxy_dir }}' + ANSIBLE_CONFIG: '{{ galaxy_dir }}/ansible.cfg' + block: + - name: reset installation directory + file: + state: "{{ item }}" + path: "{{ galaxy_dir }}/ansible_collections" + loop: + - absent + - directory + + - name: >- + install collections with pre-release versions in the dependency tree + command: >- + ansible-galaxy collection install + meta_ns_with_trans_wildcard_dep.meta_name_w_trans_wildcard_dep + rc_meta_ns_with_trans_dev_dep.rc_meta_nm_w_transitive_dev_dep:=2.4.5-rc5 + {{ galaxy_verbosity }} + register: prioritize_direct_req + - assert: + that: + - >- + "rc_meta_ns_with_trans_dev_dep.rc_meta_nm_w_transitive_dev_dep:2.4.5-rc5 was installed successfully" + in prioritize_direct_req.stdout + - >- + "meta_ns_with_trans_wildcard_dep.meta_name_w_trans_wildcard_dep:4.5.6 was installed successfully" + in prioritize_direct_req.stdout + - >- + "ns_with_dev_dep.name_with_dev_dep:6.7.8 was installed successfully" + in prioritize_direct_req.stdout + - >- + "ns_with_wildcard_dep.name_with_wildcard_dep:5.6.7-beta.3 was installed successfully" + in prioritize_direct_req.stdout + - >- + "dev_and_stables_ns.dev_and_stables_name:1.2.3-dev0 was installed successfully" + in prioritize_direct_req.stdout + + - name: cleanup + file: + state: "{{ item }}" + path: "{{ galaxy_dir }}/ansible_collections" + loop: + - absent + - directory + + - name: >- + install collection that only has pre-release versions published + to the index + command: >- + ansible-galaxy collection install + rc_meta_ns_with_trans_dev_dep.rc_meta_nm_w_transitive_dev_dep:* + {{ galaxy_verbosity }} + register: select_pre_release_if_no_stable + - assert: + that: + - >- + "rc_meta_ns_with_trans_dev_dep.rc_meta_nm_w_transitive_dev_dep:2.4.5-rc5 was installed successfully" + in select_pre_release_if_no_stable.stdout + - >- + "ns_with_dev_dep.name_with_dev_dep:6.7.8 was installed successfully" + in select_pre_release_if_no_stable.stdout + - >- + "dev_and_stables_ns.dev_and_stables_name:1.2.3-dev0 was installed successfully" + in select_pre_release_if_no_stable.stdout + always: + - name: cleanup + file: + state: "{{ item }}" + path: "{{ galaxy_dir }}/ansible_collections" + loop: + - absent + - directory + +... diff --git a/test/integration/targets/ansible-galaxy-collection/tasks/virtual_direct_requests.yml b/test/integration/targets/ansible-galaxy-collection/tasks/virtual_direct_requests.yml new file mode 100644 index 00000000000..7b1931f0cc1 --- /dev/null +++ b/test/integration/targets/ansible-galaxy-collection/tasks/virtual_direct_requests.yml @@ -0,0 +1,77 @@ +- environment: + ANSIBLE_CONFIG: '{{ galaxy_dir }}/ansible.cfg' + vars: + scm_path: "{{ galaxy_dir }}/scms" + metadata: + collection1: |- + name: collection1 + version: "1.0.0" + dependencies: + test_prereleases.collection2: '*' + collection2: | + name: collection2 + version: "1.0.0-dev0" + dependencies: {} + namespace_boilerplate: |- + namespace: test_prereleases + readme: README.md + authors: + - "ansible-core" + description: test prerelease priority with virtual collections + license: + - GPL-2.0-or-later + license_file: '' + tags: [] + repository: https://github.com/ansible/ansible + documentation: https://github.com/ansible/ansible + homepage: https://github.com/ansible/ansible + issues: https://github.com/ansible/ansible + build_ignore: [] + block: + - name: Initialize git repository + command: 'git init {{ scm_path }}/test_prereleases' + + - name: Configure commiter for the repo + shell: git config user.email ansible-test@ansible.com && git config user.name ansible-test + args: + chdir: "{{ scm_path }}/test_prereleases" + + - name: Add collections to the repo + file: + path: "{{ scm_path }}/test_prereleases/{{ item }}" + state: directory + loop: + - collection1 + - collection2 + + - name: Add collection metadata + copy: + dest: "{{ scm_path }}/test_prereleases/{{ item }}/galaxy.yml" + content: "{{ metadata[item] + '\n' + metadata['namespace_boilerplate'] }}" + loop: + - collection1 + - collection2 + + - name: Save the changes + shell: git add . && git commit -m "Add collections to test installing a git repo directly takes priority over indirect Galaxy dep" + args: + chdir: '{{ scm_path }}/test_prereleases' + + - name: Validate the dependency also exists on Galaxy before test + command: "ansible-galaxy collection install test_prereleases.collection2" + register: prereq + failed_when: '"test_prereleases.collection2:1.0.0 was installed successfully" not in prereq.stdout' + + - name: Install collections from source + command: "ansible-galaxy collection install git+file://{{ scm_path }}/test_prereleases" + register: prioritize_direct_req + + - assert: + that: + - '"test_prereleases.collection2:1.0.0-dev0 was installed successfully" in prioritize_direct_req.stdout' + + always: + - name: Clean up test repos + file: + path: "{{ scm_path }}" + state: absent diff --git a/test/integration/targets/ansible-galaxy-collection/vars/main.yml b/test/integration/targets/ansible-galaxy-collection/vars/main.yml index d3f188b9061..9c1bb7619d8 100644 --- a/test/integration/targets/ansible-galaxy-collection/vars/main.yml +++ b/test/integration/targets/ansible-galaxy-collection/vars/main.yml @@ -164,3 +164,41 @@ collection_list: name: parent dependencies: namespace1.name1: '*' + + # non-prerelease is published to test that installing + # the pre-release from SCM doesn't accidentally prefer indirect + # dependencies from Galaxy + - namespace: test_prereleases + name: collection2 + version: 1.0.0 + + - namespace: dev_and_stables_ns + name: dev_and_stables_name + version: 1.2.3-dev0 + - namespace: dev_and_stables_ns + name: dev_and_stables_name + version: 1.2.4 + + - namespace: ns_with_wildcard_dep + name: name_with_wildcard_dep + version: 5.6.7-beta.3 + dependencies: + dev_and_stables_ns.dev_and_stables_name: >- + * + - namespace: ns_with_dev_dep + name: name_with_dev_dep + version: 6.7.8 + dependencies: + dev_and_stables_ns.dev_and_stables_name: 1.2.3-dev0 + + - namespace: rc_meta_ns_with_trans_dev_dep + name: rc_meta_nm_w_transitive_dev_dep + version: 2.4.5-rc5 + dependencies: + ns_with_dev_dep.name_with_dev_dep: >- + * + - namespace: meta_ns_with_trans_wildcard_dep + name: meta_name_w_trans_wildcard_dep + version: 4.5.6 + dependencies: + ns_with_wildcard_dep.name_with_wildcard_dep: 5.6.7-beta.3