better error for "ansible-galaxy collection verify" if there's no MANIFEST.json (#67498)

* Add a better error for "ansible-galaxy verify" if the MANIFEST.json has been deleted from the installed collection or if the collection hasn't been installed via normal means

* Fix unit tests for the remote collection

If there's something wrong with the local collection's version it will fail before the remote collection is sought

* Add a test for the new error msg

* Prevent the duplicate warning

Mock the new isfile call where needed

* Update lib/ansible/galaxy/collection.py

Co-Authored-By: Martin Krizek <martin.krizek@gmail.com>

Co-authored-by: Martin Krizek <martin.krizek@gmail.com>

lib/ansible/galaxy/collection.py

@@ -668,6 +668,11 @@ def verify_collections(collections, search_paths, apis, validate_certs, ignore_e
                     for search_path in search_paths:
                         b_search_path = to_bytes(os.path.join(search_path, namespace, name), errors='surrogate_or_strict')
                         if os.path.isdir(b_search_path):
+                            if not os.path.isfile(os.path.join(to_text(b_search_path, errors='surrogate_or_strict'), 'MANIFEST.json')):
+                                raise AnsibleError(
+                                    message="Collection %s does not appear to have a MANIFEST.json. " % collection_name +
+                                            "A MANIFEST.json is expected if the collection has been built and installed via ansible-galaxy."
+                                )
                             local_collection = CollectionRequirement.from_path(b_search_path, False)
                             break
                     if local_collection is None:

test/units/galaxy/test_collection.py

@@ -1154,6 +1154,25 @@ def test_verify_identical(monkeypatch, mock_collection, manifest_info, files_man
             assert mock_debug.call_args_list[-1][0][0] == success_msg
 
 
+@patch.object(os.path, 'isdir', return_value=True)
+def test_verify_collections_no_version(mock_isdir, mock_collection, monkeypatch):
+    namespace = 'ansible_namespace'
+    name = 'collection'
+    version = '*'  # Occurs if MANIFEST.json does not exist
+
+    local_collection = mock_collection(namespace=namespace, name=name, version=version)
+    monkeypatch.setattr(collection.CollectionRequirement, 'from_path', MagicMock(return_value=local_collection))
+
+    collections = [('%s.%s' % (namespace, name), version, None)]
+
+    with pytest.raises(AnsibleError) as err:
+        collection.verify_collections(collections, './', local_collection.api, False, False)
+
+    err_msg = 'Collection %s.%s does not appear to have a MANIFEST.json. ' % (namespace, name)
+    err_msg += 'A MANIFEST.json is expected if the collection has been built and installed via ansible-galaxy.'
+    assert err.value.message == err_msg
+
+
 @patch.object(collection.CollectionRequirement, 'verify')
 def test_verify_collections_not_installed(mock_verify, mock_collection, monkeypatch):
     namespace = 'ansible_namespace'
@@ -1208,11 +1227,14 @@ def test_verify_collections_not_installed_ignore_errors(mock_verify, mock_collec
 
 @patch.object(os.path, 'isdir', return_value=True)
 @patch.object(collection.CollectionRequirement, 'verify')
-def test_verify_collections_no_remote(mock_verify, mock_isdir, mock_collection):
+def test_verify_collections_no_remote(mock_verify, mock_isdir, mock_collection, monkeypatch):
     namespace = 'ansible_namespace'
     name = 'collection'
     version = '1.0.0'
 
+    monkeypatch.setattr(os.path, 'isfile', MagicMock(side_effect=[False, True]))
+    monkeypatch.setattr(collection.CollectionRequirement, 'from_path', MagicMock(return_value=mock_collection()))
+
     collections = [('%s.%s' % (namespace, name), version, None)]
     search_path = './'
     validate_certs = False
@@ -1227,12 +1249,13 @@ def test_verify_collections_no_remote(mock_verify, mock_isdir, mock_collection):
 
 @patch.object(os.path, 'isdir', return_value=True)
 @patch.object(collection.CollectionRequirement, 'verify')
-def test_verify_collections_no_remote_ignore_errors(mock_verify, mock_isdir, mock_collection):
+def test_verify_collections_no_remote_ignore_errors(mock_verify, mock_isdir, mock_collection, monkeypatch):
     namespace = 'ansible_namespace'
     name = 'collection'
     version = '1.0.0'
 
-    local_collection = mock_collection(local_installed=False)
+    monkeypatch.setattr(os.path, 'isfile', MagicMock(side_effect=[False, True]))
+    monkeypatch.setattr(collection.CollectionRequirement, 'from_path', MagicMock(return_value=mock_collection()))
 
     collections = [('%s.%s' % (namespace, name), version, None)]
     search_path = './'
@@ -1292,13 +1315,14 @@ def test_verify_collections_url(monkeypatch):
     assert err.value.message == msg
 
 
-@patch.object(os.path, 'isfile', return_value=False)
 @patch.object(os.path, 'isdir', return_value=True)
 @patch.object(collection.CollectionRequirement, 'verify')
-def test_verify_collections_name(mock_verify, mock_isdir, mock_isfile, mock_collection, monkeypatch):
+def test_verify_collections_name(mock_verify, mock_isdir, mock_collection, monkeypatch):
     local_collection = mock_collection()
     monkeypatch.setattr(collection.CollectionRequirement, 'from_path', MagicMock(return_value=local_collection))
 
+    monkeypatch.setattr(os.path, 'isfile', MagicMock(side_effect=[False, True, False]))
+
     located_remote_from_name = MagicMock(return_value=mock_collection(local=False))
     monkeypatch.setattr(collection.CollectionRequirement, 'from_name', located_remote_from_name)