nonzero exit code on `ansible galaxy collection verify` failures (#74051)

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

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

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():

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"