diff options
author | Sloane Hertel <19572925+s-hertel@users.noreply.github.com> | 2022-08-03 14:20:58 -0400 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-08-03 11:20:58 -0700 |
commit | 2efb33d200cb899ec155c7f800cbce4e9309e37a (patch) | |
tree | 3bd4d74bbe75c1dd3f851dabad2017ec5f5dddef | |
parent | 253c30441b1ea42d1e58e532676da33d04c9c2a6 (diff) | |
download | ansible-2efb33d200cb899ec155c7f800cbce4e9309e37a.tar.gz |
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 05608b20e8f875d51866a184f8c579fe60498e05)
-rw-r--r-- | changelogs/fragments/70180-collection-list-more-robust.yml | 2 | ||||
-rwxr-xr-x | lib/ansible/cli/galaxy.py | 2 | ||||
-rw-r--r-- | lib/ansible/galaxy/collection/concrete_artifact_manager.py | 39 | ||||
-rw-r--r-- | test/integration/targets/ansible-galaxy-collection/tasks/list.yml | 25 | ||||
-rw-r--r-- | test/units/galaxy/test_collection.py | 17 |
5 files changed, 78 insertions, 7 deletions
diff --git a/changelogs/fragments/70180-collection-list-more-robust.yml b/changelogs/fragments/70180-collection-list-more-robust.yml new file mode 100644 index 0000000000..1ffd68ca03 --- /dev/null +++ b/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). diff --git a/lib/ansible/cli/galaxy.py b/lib/ansible/cli/galaxy.py index 2b1f0674f3..288318aa27 100755 --- a/lib/ansible/cli/galaxy.py +++ b/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']) diff --git a/lib/ansible/galaxy/collection/concrete_artifact_manager.py b/lib/ansible/galaxy/collection/concrete_artifact_manager.py index 5d2e61c949..1ef7eed28c 100644 --- a/lib/ansible/galaxy/collection/concrete_artifact_manager.py +++ b/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" - % (to_native(b_galaxy_yml_path), ", ".join(sorted(missing_keys)))) + msg = ( + "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( diff --git a/test/integration/targets/ansible-galaxy-collection/tasks/list.yml b/test/integration/targets/ansible-galaxy-collection/tasks/list.yml index f4d0f51bbe..b8d63492c6 100644 --- a/test/integration/targets/ansible-galaxy-collection/tasks/list.yml +++ b/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 diff --git a/test/units/galaxy/test_collection.py b/test/units/galaxy/test_collection.py index 4cfca39446..e486daf6df 100644 --- a/test/units/galaxy/test_collection.py +++ b/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 |