Fix listing collections that are missing the metadata required by build (#76596) (#78411)

* Rethread pr/70185 through the dependency resolver

Hang optional metadata toggle on the ConcreteArtifactsManager instead of threading it through whole list codepath

Don't error while listing collections if a collection's metadata is missing keys required for building a collection.

Give an informative warning if metadata has been badly formatted.

Co-authored-by: Sam Doran <sdoran@redhat.com>
(cherry picked from commit 05608b20e8)

changelogs/fragments/70180-collection-list-more-robust.yml

@@ -0,0 +1,2 @@
+bugfixes:
+  - ansible-galaxy - do not require mandatory keys in the ``galaxy.yml`` of source collections when listing them (https://github.com/ansible/ansible/issues/70180).

lib/ansible/cli/galaxy.py

@@ -1515,6 +1515,8 @@ class GalaxyCLI(CLI):
 
         :param artifacts_manager: Artifacts manager.
         """
+        if artifacts_manager is not None:
+            artifacts_manager.require_build_metadata = False
 
         output_format = context.CLIARGS['output_format']
         collections_search_paths = set(context.CLIARGS['collections_path'])

lib/ansible/galaxy/collection/concrete_artifact_manager.py

@@ -31,6 +31,7 @@ from ansible.galaxy.dependency_resolution.dataclasses import _GALAXY_YAML
 from ansible.galaxy.user_agent import user_agent
 from ansible.module_utils._text import to_bytes, to_native, to_text
 from ansible.module_utils.common.process import get_bin_path
+from ansible.module_utils.common._collections_compat import MutableMapping
 from ansible.module_utils.common.yaml import yaml_load
 from ansible.module_utils.six import raise_from
 from ansible.module_utils.urls import open_url
@@ -72,6 +73,7 @@ class ConcreteArtifactsManager:
         self.timeout = timeout  # type: int
         self._required_signature_count = required_signature_count  # type: str
         self._ignore_signature_errors = ignore_signature_errors  # type: list[str]
+        self._require_build_metadata = True  # type: bool
 
     @property
     def keyring(self):
@@ -87,6 +89,16 @@ class ConcreteArtifactsManager:
             return []
         return self._ignore_signature_errors
 
+    @property
+    def require_build_metadata(self):
+        # type: () -> bool
+        return self._require_build_metadata
+
+    @require_build_metadata.setter
+    def require_build_metadata(self, value):
+        # type: (bool) -> None
+        self._require_build_metadata = value
+
     def get_galaxy_artifact_source_info(self, collection):
         # type: (Candidate) -> dict[str, str | list[dict[str, str]]]
         server = collection.src.api_server
@@ -283,7 +295,7 @@ class ConcreteArtifactsManager:
         elif collection.is_dir:  # should we just build a coll instead?
             # FIXME: what if there's subdirs?
             try:
-                collection_meta = _get_meta_from_dir(b_artifact_path)
+                collection_meta = _get_meta_from_dir(b_artifact_path, self.require_build_metadata)
             except LookupError as lookup_err:
                 raise_from(
                     AnsibleError(
@@ -335,6 +347,7 @@ class ConcreteArtifactsManager:
             keyring=None,  # type: str
             required_signature_count=None,  # type: str
             ignore_signature_errors=None,  # type: list[str]
+            require_build_metadata=True,  # type: bool
     ):  # type: (...) -> t.Iterator[ConcreteArtifactsManager]
         """Custom ConcreteArtifactsManager constructor with temp dir.
 
@@ -502,6 +515,7 @@ def _consume_file(read_from, write_to=None):
 def _normalize_galaxy_yml_manifest(
         galaxy_yml,  # type: dict[str, str | list[str] | dict[str, str] | None]
         b_galaxy_yml_path,  # type: bytes
+        require_build_metadata=True,  # type: bool
 ):
     # type: (...) -> dict[str, str | list[str] | dict[str, str] | None]
     galaxy_yml_schema = (
@@ -530,8 +544,14 @@ def _normalize_galaxy_yml_manifest(
     set_keys = set(galaxy_yml.keys())
     missing_keys = mandatory_keys.difference(set_keys)
     if missing_keys:
-        raise AnsibleError("The collection galaxy.yml at '%s' is missing the following mandatory keys: %s"
+        msg = (
-                           % (to_native(b_galaxy_yml_path), ", ".join(sorted(missing_keys))))
+            "The collection galaxy.yml at '%s' is missing the following mandatory keys: %s"
+            % (to_native(b_galaxy_yml_path), ", ".join(sorted(missing_keys)))
+        )
+        if require_build_metadata:
+            raise AnsibleError(msg)
+        display.warning(msg)
+        raise ValueError(msg)
 
     extra_keys = set_keys.difference(all_keys)
     if len(extra_keys) > 0:
@@ -567,15 +587,17 @@ def _normalize_galaxy_yml_manifest(
 
 def _get_meta_from_dir(
         b_path,  # type: bytes
+        require_build_metadata=True,  # type: bool
 ):  # type: (...) -> dict[str, str | list[str] | dict[str, str] | None]
     try:
         return _get_meta_from_installed_dir(b_path)
     except LookupError:
-        return _get_meta_from_src_dir(b_path)
+        return _get_meta_from_src_dir(b_path, require_build_metadata)
 
 
 def _get_meta_from_src_dir(
         b_path,  # type: bytes
+        require_build_metadata=True,  # type: bool
 ):  # type: (...) -> dict[str, str | list[str] | dict[str, str] | None]
     galaxy_yml = os.path.join(b_path, _GALAXY_YAML)
     if not os.path.isfile(galaxy_yml):
@@ -600,7 +622,14 @@ def _get_meta_from_src_dir(
                 yaml_err,
             )
 
-    return _normalize_galaxy_yml_manifest(manifest, galaxy_yml)
+    if not isinstance(manifest, dict):
+        if require_build_metadata:
+            raise AnsibleError(f"The collection galaxy.yml at '{to_native(galaxy_yml)}' is incorrectly formatted.")
+        # Valid build metadata is not required by ansible-galaxy list. Raise ValueError to fall back to implicit metadata.
+        display.warning(f"The collection galaxy.yml at '{to_native(galaxy_yml)}' is incorrectly formatted.")
+        raise ValueError(f"The collection galaxy.yml at '{to_native(galaxy_yml)}' is incorrectly formatted.")
+
+    return _normalize_galaxy_yml_manifest(manifest, galaxy_yml, require_build_metadata)
 
 
 def _get_json_from_installed_dir(

test/integration/targets/ansible-galaxy-collection/tasks/list.yml

@@ -5,6 +5,8 @@
     - 'dev.collection2'
     - 'dev.collection3'
     - 'dev.collection4'
+    - 'dev.collection5'
+    - 'dev.collection6'
 
 - name: replace the default version of the collections
   lineinfile:
@@ -32,6 +34,17 @@
   - before: "^version: 1.0.0"
     after: "version:"
 
+- name: replace galaxy.yml content with a string
+  copy:
+    content: "invalid"
+    dest: "{{ galaxy_dir }}/dev/ansible_collections/dev/collection5/galaxy.yml"
+
+- name: remove galaxy.yml key required by build
+  lineinfile:
+    path: "{{ galaxy_dir }}/dev/ansible_collections/dev/collection6/galaxy.yml"
+    line: "version: 1.0.0"
+    state: absent
+
 - name: list collections in development without semver versions
   command: ansible-galaxy collection list {{ galaxy_verbosity }}
   register: list_result
@@ -45,6 +58,8 @@
       - "'dev.collection2 placeholder' in list_result.stdout"
       - "'dev.collection3 *' in list_result.stdout"
       - "'dev.collection4 *' in list_result.stdout"
+      - "'dev.collection5 *' in list_result.stdout"
+      - "'dev.collection6 *' in list_result.stdout"
 
 - name: list collections in human format
   command: ansible-galaxy collection list --format human
@@ -58,6 +73,8 @@
       # Note the version displayed is the 'placeholder' string rather than "*" since it is not falsey
       - "'dev.collection2 placeholder' in list_result_human.stdout"
       - "'dev.collection3 *' in list_result_human.stdout"
+      - "'dev.collection5 *' in list_result.stdout"
+      - "'dev.collection6 *' in list_result.stdout"
 
 - name: list collections in yaml format
   command: ansible-galaxy collection list --format yaml
@@ -67,10 +84,12 @@
 
 - assert:
     that:
-      - "item.value | length == 4"
+      - "item.value | length == 6"
       - "item.value['dev.collection1'].version == '*'"
       - "item.value['dev.collection2'].version == 'placeholder'"
       - "item.value['dev.collection3'].version == '*'"
+      - "item.value['dev.collection5'].version == '*'"
+      - "item.value['dev.collection6'].version == '*'"
   with_dict: "{{ list_result_yaml.stdout | from_yaml }}"
 
 - name: list collections in json format
@@ -81,10 +100,12 @@
 
 - assert:
     that:
-      - "item.value | length == 4"
+      - "item.value | length == 6"
       - "item.value['dev.collection1'].version == '*'"
       - "item.value['dev.collection2'].version == 'placeholder'"
       - "item.value['dev.collection3'].version == '*'"
+      - "item.value['dev.collection5'].version == '*'"
+      - "item.value['dev.collection6'].version == '*'"
   with_dict: "{{ list_result_json.stdout | from_json }}"
 
 - name: list single collection in json format

test/units/galaxy/test_collection.py

@@ -425,6 +425,23 @@ def test_missing_required_galaxy_key(galaxy_yml_dir):
         collection.concrete_artifact_manager._get_meta_from_src_dir(galaxy_yml_dir)
 
 
+@pytest.mark.parametrize('galaxy_yml_dir', [b'namespace: test_namespace'], indirect=True)
+def test_galaxy_yaml_no_mandatory_keys(galaxy_yml_dir):
+    expected = "The collection galaxy.yml at '%s/galaxy.yml' is missing the " \
+               "following mandatory keys: authors, name, readme, version" % to_native(galaxy_yml_dir)
+
+    with pytest.raises(ValueError, match=expected):
+        assert collection.concrete_artifact_manager._get_meta_from_src_dir(galaxy_yml_dir, require_build_metadata=False) == expected
+
+
+@pytest.mark.parametrize('galaxy_yml_dir', [b'My life story is so very interesting'], indirect=True)
+def test_galaxy_yaml_no_mandatory_keys_bad_yaml(galaxy_yml_dir):
+    expected = "The collection galaxy.yml at '%s/galaxy.yml' is incorrectly formatted." % to_native(galaxy_yml_dir)
+
+    with pytest.raises(AnsibleError, match=expected):
+        collection.concrete_artifact_manager._get_meta_from_src_dir(galaxy_yml_dir)
+
+
 @pytest.mark.parametrize('galaxy_yml_dir', [b"""
 namespace: namespace
 name: collection