From 454c7e37eca922c2085c8e786b1a2e53f28b379a Mon Sep 17 00:00:00 2001 From: Matt Davis Date: Mon, 29 Mar 2021 13:06:02 -0700 Subject: [PATCH] nonzero exit code on `ansible galaxy collection verify` failures (#74051) --- .../fragments/galaxy_verify_exitcode.yml | 2 ++ lib/ansible/cli/galaxy.py | 7 +++-- lib/ansible/galaxy/collection/__init__.py | 29 ++++++++++++++++--- .../tasks/verify.yml | 23 ++++++++++----- 4 files changed, 48 insertions(+), 13 deletions(-) create mode 100644 changelogs/fragments/galaxy_verify_exitcode.yml diff --git a/changelogs/fragments/galaxy_verify_exitcode.yml b/changelogs/fragments/galaxy_verify_exitcode.yml new file mode 100644 index 00000000000..e3400bffd70 --- /dev/null +++ b/changelogs/fragments/galaxy_verify_exitcode.yml @@ -0,0 +1,2 @@ +minor_changes: +- ansible-galaxy CLI - ``collection verify`` command now exits with a non-zero exit code on verification failure diff --git a/lib/ansible/cli/galaxy.py b/lib/ansible/cli/galaxy.py index 071fdc25345..e8ba0a419e5 100644 --- a/lib/ansible/cli/galaxy.py +++ b/lib/ansible/cli/galaxy.py @@ -549,7 +549,7 @@ class GalaxyCLI(CLI): **galaxy_options )) - context.CLIARGS['func']() + return context.CLIARGS['func']() @property def api(self): @@ -1091,13 +1091,16 @@ class GalaxyCLI(CLI): resolved_paths = [validate_collection_path(GalaxyCLI._resolve_path(path)) for path in search_paths] - verify_collections( + results = verify_collections( requirements, resolved_paths, self.api_servers, ignore_errors, local_verify_only=local_verify_only, artifacts_manager=artifacts_manager, ) + if any(result for result in results if not result.success): + return 1 + return 0 @with_collection_artifacts_manager diff --git a/lib/ansible/galaxy/collection/__init__.py b/lib/ansible/galaxy/collection/__init__.py index ed0512927d9..ed99da34b78 100644 --- a/lib/ansible/galaxy/collection/__init__.py +++ b/lib/ansible/galaxy/collection/__init__.py @@ -131,16 +131,26 @@ MANIFEST_FILENAME = 'MANIFEST.json' ModifiedContent = namedtuple('ModifiedContent', ['filename', 'expected', 'installed']) +# FUTURE: expose actual verify result details for a collection on this object, maybe reimplement as dataclass on py3.8+ +class CollectionVerifyResult: + def __init__(self, collection_name): # type: (str) -> None + self.collection_name = collection_name # type: str + self.success = True # type: bool + + def verify_local_collection( local_collection, remote_collection, artifacts_manager, -): # type: (Candidate, Optional[Candidate], ConcreteArtifactsManager) -> None +): # type: (Candidate, Optional[Candidate], ConcreteArtifactsManager) -> CollectionVerifyResult """Verify integrity of the locally installed collection. :param local_collection: Collection being checked. :param remote_collection: Upstream collection (optional, if None, only verify local artifact) :param artifacts_manager: Artifacts manager. + :return: a collection verify result object. """ + result = CollectionVerifyResult(local_collection.fqcn) + b_collection_path = to_bytes( local_collection.src, errors='surrogate_or_strict', ) @@ -188,7 +198,8 @@ def verify_local_collection( ) ) display.display(err) - return + result.success = False + return result # Verify the downloaded manifest hash matches the installed copy before verifying the file manifest manifest_hash = get_hash_from_validation_source(MANIFEST_FILENAME) @@ -214,6 +225,7 @@ def verify_local_collection( _verify_file_hash(b_collection_path, manifest_data['name'], expected_hash, modified_content) if modified_content: + result.success = False display.display( 'Collection {fqcn!s} contains modified content ' 'in the following files:'. @@ -229,6 +241,8 @@ def verify_local_collection( format(coll=local_collection, what=what), ) + return result + def build_collection(u_collection_path, u_output_path, force): # type: (Text, Text, bool) -> Text @@ -574,7 +588,7 @@ def verify_collections( ignore_errors, # type: bool local_verify_only, # type: bool artifacts_manager, # type: ConcreteArtifactsManager -): # type: (...) -> None +): # type: (...) -> List[CollectionVerifyResult] r"""Verify the integrity of locally installed collections. :param collections: The collections to check. @@ -583,7 +597,10 @@ def verify_collections( :param ignore_errors: Whether to ignore any errors when verifying the collection. :param local_verify_only: When True, skip downloads and only verify local manifests. :param artifacts_manager: Artifacts manager. + :return: list of CollectionVerifyResult objects describing the results of each collection verification """ + results = [] # type: List[CollectionVerifyResult] + api_proxy = MultiGalaxyAPIProxy(apis, artifacts_manager) with _display_progress(): @@ -656,11 +673,13 @@ def verify_collections( ) raise - verify_local_collection( + result = verify_local_collection( local_collection, remote_collection, artifacts_manager, ) + results.append(result) + except AnsibleError as err: if ignore_errors: display.warning( @@ -672,6 +691,8 @@ def verify_collections( else: raise + return results + @contextmanager def _tempdir(): diff --git a/test/integration/targets/ansible-galaxy-collection/tasks/verify.yml b/test/integration/targets/ansible-galaxy-collection/tasks/verify.yml index f951c654cd6..eacb8d6b307 100644 --- a/test/integration/targets/ansible-galaxy-collection/tasks/verify.yml +++ b/test/integration/targets/ansible-galaxy-collection/tasks/verify.yml @@ -16,11 +16,11 @@ - name: test verifying a tarfile command: ansible-galaxy collection verify {{ galaxy_dir }}/ansible_test-verify-1.0.0.tar.gz register: verify - ignore_errors: yes + failed_when: verify.rc == 0 - assert: that: - - verify.failed + - verify.rc != 0 - >- "ERROR! 'file' type is not supported. The format namespace.name is expected." in verify.stderr @@ -48,11 +48,11 @@ - name: verify a collection that doesn't appear to be installed command: ansible-galaxy collection verify ansible_test.verify:1.0.0 register: verify - ignore_errors: true + failed_when: verify.rc == 0 - assert: that: - - verify.failed + - verify.rc != 0 - "'Collection ansible_test.verify is not installed in any of the collection paths.' in verify.stderr" - name: create a modules directory @@ -86,9 +86,11 @@ environment: ANSIBLE_COLLECTIONS_PATH: '{{ galaxy_dir }}' register: verify + failed_when: verify.rc == 0 - assert: that: + - verify.rc != 0 - '"ansible_test.verify has the version ''1.0.0'' but is being compared to ''2.0.0''" in verify.stdout' - name: install the new version from the server @@ -147,9 +149,11 @@ register: verify environment: ANSIBLE_COLLECTIONS_PATH: '{{ galaxy_dir }}' + failed_when: verify.rc == 0 - assert: that: + - verify.rc != 0 - "'Collection ansible_test.verify contains modified content in the following files:\n plugins/modules/test_module.py' in verify.stdout" - name: modify the FILES.json to match the new checksum @@ -165,9 +169,11 @@ register: verify environment: ANSIBLE_COLLECTIONS_PATH: '{{ galaxy_dir }}' + failed_when: verify.rc == 0 - assert: that: + - verify.rc != 0 - "'Collection ansible_test.verify contains modified content in the following files:\n FILES.json' in verify.stdout" - name: get the checksum of the FILES.json @@ -187,9 +193,11 @@ register: verify environment: ANSIBLE_COLLECTIONS_PATH: '{{ galaxy_dir }}' + failed_when: verify.rc == 0 - assert: that: + - verify.rc != 0 - "'Collection ansible_test.verify contains modified content in the following files:\n MANIFEST.json' in verify.stdout" - name: remove the artifact metadata to test verifying a collection without it @@ -215,11 +223,11 @@ register: verify environment: ANSIBLE_COLLECTIONS_PATH: '{{ galaxy_dir }}' - ignore_errors: yes + failed_when: verify.rc == 0 - assert: that: - - verify.failed + - verify.rc != 0 - "'Collection ansible_test.verify does not have a MANIFEST.json' in verify.stderr" - name: update the collection version to something not present on the server @@ -260,9 +268,10 @@ register: verify environment: ANSIBLE_COLLECTIONS_PATH: '{{ galaxy_dir }}' - ignore_errors: yes + failed_when: verify.rc == 0 - assert: that: + - verify.rc != 0 - "'Collection ansible_test.verify contains modified content in the following files:' in verify.stdout" - "'plugins/modules/test_module.py' in verify.stdout"