From 343ffaa18b63c92e182b16c3ad84b8d81ca4df69 Mon Sep 17 00:00:00 2001 From: Sloane Hertel Date: Thu, 14 May 2020 12:25:19 -0400 Subject: [PATCH] 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 Co-authored-by: Martin Krizek --- lib/ansible/galaxy/collection.py | 5 ++++ test/units/galaxy/test_collection.py | 34 ++++++++++++++++++++++++---- 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/lib/ansible/galaxy/collection.py b/lib/ansible/galaxy/collection.py index a055b08e718..856e54666f6 100644 --- a/lib/ansible/galaxy/collection.py +++ b/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: diff --git a/test/units/galaxy/test_collection.py b/test/units/galaxy/test_collection.py index f6b285cca03..1fced769339 100644 --- a/test/units/galaxy/test_collection.py +++ b/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)