ansible-galaxy - validate version for directories and collections in git repos (#76579)

* Ensure the version is valid for directories and collections in git repos before installing

Fix the error message for invalid semantic versions

* Make requested changes

* Add a test case for unhandled ValueError exception

* Add changelog

* Update lib/ansible/galaxy/collection/galaxy_api_proxy.py

Co-authored-by: Sviatoslav Sydorenko <wk.cvs.github@sydorenko.org.ua>

* Reword error message

Include link to learn how to compose a semver version

* Move version validation into the caller, find_matches

* Add tests for more invalid version types

* Remove unused import

Fix raising unexpected error

* Update lib/ansible/galaxy/collection/__init__.py

Co-authored-by: Sviatoslav Sydorenko <wk.cvs.github@sydorenko.org.ua>

* Update lib/ansible/galaxy/dependency_resolution/providers.py

Co-authored-by: Sviatoslav Sydorenko <wk.cvs.github@sydorenko.org.ua>

* Update lib/ansible/galaxy/dependency_resolution/providers.py

Co-authored-by: Sviatoslav Sydorenko <wk.cvs.github@sydorenko.org.ua>

Co-authored-by: Sviatoslav Sydorenko <wk.cvs.github@sydorenko.org.ua>

changelogs/fragments/76579-fix-ansible-galaxy-collection-version-error.yml

@@ -0,0 +1,2 @@
+bugfixes:
+  - ansible-galaxy - installing/downloading collections with invalid versions in git repositories and directories now gives a formatted error message (https://github.com/ansible/ansible/issues/76425, https://github.com/ansible/ansible/issues/75404).

lib/ansible/galaxy/collection/__init__.py

@@ -1383,3 +1383,5 @@ def _resolve_depenency_map(
             AnsibleError('\n'.join(error_msg_lines)),
             dep_exc,
         )
+    except ValueError as exc:
+        raise AnsibleError(to_native(exc)) from exc

lib/ansible/galaxy/dependency_resolution/providers.py

@@ -28,6 +28,7 @@ from ansible.galaxy.dependency_resolution.versioning import (
     is_pre_release,
     meets_requirements,
 )
+from ansible.module_utils.six import string_types
 from ansible.utils.version import SemanticVersion
 
 from resolvelib import AbstractProvider
@@ -211,11 +212,45 @@ class CollectionDependencyProvider(AbstractProvider):
         first_req = requirements[0]
         fqcn = first_req.fqcn
         # The fqcn is guaranteed to be the same
-        coll_versions = self._api_proxy.get_collection_versions(first_req)
+        version_req = "A SemVer-compliant version or '*' is required. See https://semver.org to learn how to compose it correctly. "
+        version_req += "This is an issue with the collection."
+        try:
+            coll_versions = self._api_proxy.get_collection_versions(first_req)
+        except TypeError as exc:
+            if first_req.is_concrete_artifact:
+                # Non hashable versions will cause a TypeError
+                raise ValueError(
+                    f"Invalid version found for the collection '{first_req}'. {version_req}"
+                ) from exc
+            # Unexpected error from a Galaxy server
+            raise
+
         if first_req.is_concrete_artifact:
             # FIXME: do we assume that all the following artifacts are also concrete?
             # FIXME: does using fqcn==None cause us problems here?
 
+            # Ensure the version found in the concrete artifact is SemVer-compliant
+            for version, req_src in coll_versions:
+                version_err = f"Invalid version found for the collection '{first_req}': {version} ({type(version)}). {version_req}"
+                # NOTE: The known cases causing the version to be a non-string object come from
+                # NOTE: the differences in how the YAML parser normalizes ambiguous values and
+                # NOTE: how the end-users sometimes expect them to be parsed. Unless the users
+                # NOTE: explicitly use the double quotes of one of the multiline string syntaxes
+                # NOTE: in the collection metadata file, PyYAML will parse a value containing
+                # NOTE: two dot-separated integers as `float`, a single integer as `int`, and 3+
+                # NOTE: integers as a `str`. In some cases, they may also use an empty value
+                # NOTE: which is normalized as `null` and turned into `None` in the Python-land.
+                # NOTE: Another known mistake is setting a minor part of the SemVer notation
+                # NOTE: skipping the "patch" bit like "1.0" which is assumed non-compliant even
+                # NOTE: after the conversion to string.
+                if not isinstance(version, string_types):
+                    raise ValueError(version_err)
+                elif version != '*':
+                    try:
+                        SemanticVersion(version)
+                    except ValueError as ex:
+                        raise ValueError(version_err) from ex
+
             return [
                 Candidate(fqcn, version, _none_src_server, first_req.type)
                 for version, _none_src_server in coll_versions

test/integration/targets/ansible-galaxy-collection-scm/tasks/main.yml

@@ -24,6 +24,8 @@
   - include_tasks: ./setup_recursive_scm_dependency.yml
   - include_tasks: ./scm_dependency_deduplication.yml
   - include_tasks: ./download.yml
+  - include_tasks: ./setup_collection_bad_version.yml
+  - include_tasks: ./test_invalid_version.yml
 
   always:
 

test/integration/targets/ansible-galaxy-collection-scm/tasks/setup_collection_bad_version.yml

@@ -0,0 +1,47 @@
+- name: Initialize a git repo
+  command: 'git init {{ test_error_repo_path }}'
+
+- stat:
+    path: "{{ test_error_repo_path }}"
+
+- name: Add a collection to the repository
+  command: 'ansible-galaxy collection init {{ item }}'
+  args:
+    chdir: '{{ scm_path }}'
+  loop:
+    - error_test.float_version_collection
+    - error_test.not_semantic_version_collection
+    - error_test.list_version_collection
+    - error_test.dict_version_collection
+
+- name: Add an invalid float version to a collection
+  lineinfile:
+    path: '{{ test_error_repo_path }}/float_version_collection/galaxy.yml'
+    regexp: '^version'
+    line: "version: 1.0"  # Version is a float, not a string as expected
+
+- name: Add an invalid non-semantic string version a collection
+  lineinfile:
+    path: '{{ test_error_repo_path }}/not_semantic_version_collection/galaxy.yml'
+    regexp: '^version'
+    line: "version: '1.0'"  # Version is a string, but not a semantic version as expected
+
+- name: Add an invalid list version to a collection
+  lineinfile:
+    path: '{{ test_error_repo_path }}/list_version_collection/galaxy.yml'
+    regexp: '^version'
+    line: "version: ['1.0.0']"  # Version is a list, not a string as expected
+
+- name: Add an invalid version to a collection
+  lineinfile:
+    path: '{{ test_error_repo_path }}/dict_version_collection/galaxy.yml'
+    regexp: '^version'
+    line: "version: {'broken': 'version'}"  # Version is a dict, not a string as expected
+
+- name: Commit the changes
+  command: '{{ item }}'
+  args:
+    chdir: '{{ test_error_repo_path }}'
+  loop:
+    - git add ./
+    - git commit -m 'add collections'

test/integration/targets/ansible-galaxy-collection-scm/tasks/test_invalid_version.yml

@@ -0,0 +1,58 @@
+- block:
+  - name: test installing a collection with an invalid float version value
+    command: 'ansible-galaxy collection install git+file://{{ test_error_repo_path }}/.git#float_version_collection -vvvvvv'
+    ignore_errors: yes
+    register: invalid_version_result
+
+  - assert:
+      that:
+        - invalid_version_result is failed
+        - msg in invalid_version_result.stderr
+    vars:
+      req: error_test.float_version_collection:1.0
+      ver: "1.0 (<class 'float'>)"
+      msg: "Invalid version found for the collection '{{ req }}': {{ ver }}. A SemVer-compliant version or '*' is required."
+
+  - name: test installing a collection with an invalid non-SemVer string version value
+    command: 'ansible-galaxy collection install git+file://{{ test_error_repo_path }}/.git#not_semantic_version_collection -vvvvvv'
+    ignore_errors: yes
+    register: invalid_version_result
+
+  - assert:
+      that:
+        - invalid_version_result is failed
+        - msg in invalid_version_result.stderr
+    vars:
+      req: error_test.not_semantic_version_collection:1.0
+      ver: "1.0 (<class 'str'>)"
+      msg: "Invalid version found for the collection '{{ req }}': {{ ver }}. A SemVer-compliant version or '*' is required."
+
+  - name: test installing a collection with an invalid list version value
+    command: 'ansible-galaxy collection install git+file://{{ test_error_repo_path }}/.git#list_version_collection -vvvvvv'
+    ignore_errors: yes
+    register: invalid_version_result
+
+  - assert:
+      that:
+        - invalid_version_result is failed
+        - msg in invalid_version_result.stderr
+    vars:
+      req: "error_test.list_version_collection:['1.0.0']"
+      msg: "Invalid version found for the collection '{{ req }}'. A SemVer-compliant version or '*' is required."
+
+  - name: test installing a collection with an invalid dict version value
+    command: 'ansible-galaxy collection install git+file://{{ test_error_repo_path }}/.git#dict_version_collection -vvvvvv'
+    ignore_errors: yes
+    register: invalid_version_result
+
+  - assert:
+      that:
+        - invalid_version_result is failed
+        - msg in invalid_version_result.stderr
+    vars:
+      req: "error_test.dict_version_collection:{'broken': 'version'}"
+      msg: "Invalid version found for the collection '{{ req }}'. A SemVer-compliant version or '*' is required."
+
+  always:
+  - include_tasks: ./empty_installed_collections.yml
+    when: cleanup

test/integration/targets/ansible-galaxy-collection-scm/vars/main.yml

@@ -2,3 +2,4 @@ install_path: "{{ galaxy_dir }}/collections/ansible_collections"
 alt_install_path: "{{ galaxy_dir }}/other_collections/ansible_collections"
 scm_path: "{{ galaxy_dir }}/development"
 test_repo_path: "{{ galaxy_dir }}/development/ansible_test"
+test_error_repo_path: "{{ galaxy_dir }}/development/error_test"