From 4cad7e479c922cd973015fd1cf4bd5e5890158de Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Wed, 16 Oct 2019 15:23:12 -0700 Subject: [PATCH] Galaxy publish fix (#63580) * Handle galaxy v2/v3 API diffs for artifact publish response For publishing a collection artifact (POST /v3/collections/artifacts/), the response format is different between v2 and v3. For v2 galaxy, the 'task' url returned is a full url with scheme: {"task": "https://galaxy-dev.ansible.com/api/v2/collection-imports/35573/"} For v3 galaxy, the task url is relative: {"task": "/api/automation-hub/v3/imports/collections/838d1308-a8f4-402c-95cb-7823f3806cd8/"} So check which API we are using and update the task url approriately. * Use full url for all wait_for_import messages Update unit tests to parameterize the expected responses and urls. * update explanatory comment * Rename n_url to full_url. * Fix issue with overwrite of the complete path * Fixes overwrite of the complete path in case there's extra path stored in self.api_sever * Normalizes the input to the wait_import_task function so it receives the same value on both v2 and v3 Builds on #63523 * Update unittests for new call signature * Add changelog for ansible-galaxy publish API fixes. --- ...-galaxy-handle-import-task-url-changes.yml | 3 + lib/ansible/galaxy/api.py | 27 ++++-- lib/ansible/galaxy/collection.py | 16 +++- test/units/galaxy/test_api.py | 95 +++++++++++-------- test/units/galaxy/test_collection.py | 2 +- 5 files changed, 93 insertions(+), 50 deletions(-) create mode 100644 changelogs/fragments/ansible-galaxy-handle-import-task-url-changes.yml diff --git a/changelogs/fragments/ansible-galaxy-handle-import-task-url-changes.yml b/changelogs/fragments/ansible-galaxy-handle-import-task-url-changes.yml new file mode 100644 index 00000000000..31552d2d0ee --- /dev/null +++ b/changelogs/fragments/ansible-galaxy-handle-import-task-url-changes.yml @@ -0,0 +1,3 @@ +bugfixes: + - ansible-galaxy - Handle the different task resource urls in API responses from publishing + collection artifacts to galaxy servers using v2 and v3 APIs. diff --git a/lib/ansible/galaxy/api.py b/lib/ansible/galaxy/api.py index 17b1df751de..6c04a215d35 100644 --- a/lib/ansible/galaxy/api.py +++ b/lib/ansible/galaxy/api.py @@ -15,7 +15,7 @@ from ansible import context from ansible.errors import AnsibleError from ansible.module_utils.six import string_types from ansible.module_utils.six.moves.urllib.error import HTTPError -from ansible.module_utils.six.moves.urllib.parse import quote as urlquote, urlencode +from ansible.module_utils.six.moves.urllib.parse import quote as urlquote, urlencode, urlparse from ansible.module_utils._text import to_bytes, to_native, to_text from ansible.module_utils.urls import open_url from ansible.utils.display import Display @@ -439,24 +439,35 @@ class GalaxyAPI: return resp['task'] @g_connect(['v2', 'v3']) - def wait_import_task(self, task_url, timeout=0): + def wait_import_task(self, task_id, timeout=0): """ Waits until the import process on the Galaxy server has completed or the timeout is reached. - :param task_url: The full URI of the import task to wait for, this is returned by publish_collection. + :param task_id: The id of the import task to wait for. This can be parsed out of the return + value for GalaxyAPI.publish_collection. :param timeout: The timeout in seconds, 0 is no timeout. """ # TODO: actually verify that v3 returns the same structure as v2, right now this is just an assumption. state = 'waiting' data = None - display.display("Waiting until Galaxy import task %s has completed" % task_url) + # Construct the appropriate URL per version + if 'v3' in self.available_api_versions: + full_url = _urljoin(self.api_server, 'automation-hub', self.available_api_versions['v3'], + 'imports/collections', task_id, '/') + else: + # TODO: Should we have a trailing slash here? I'm working with what the unittests ask + # for but a trailing slash may be more correct + full_url = _urljoin(self.api_server, self.available_api_versions['v2'], + 'collection-imports', task_id) + + display.display("Waiting until Galaxy import task %s has completed" % full_url) start = time.time() wait = 2 while timeout == 0 or (time.time() - start) < timeout: - data = self._call_galaxy(task_url, method='GET', auth_required=True, - error_context_msg='Error when getting import task results at %s' % task_url) + data = self._call_galaxy(full_url, method='GET', auth_required=True, + error_context_msg='Error when getting import task results at %s' % full_url) state = data.get('state', 'waiting') @@ -471,7 +482,7 @@ class GalaxyAPI: wait = min(30, wait * 1.5) if state == 'waiting': raise AnsibleError("Timeout while waiting for the Galaxy import process to finish, check progress at '%s'" - % to_native(task_url)) + % to_native(full_url)) for message in data.get('messages', []): level = message['level'] @@ -485,7 +496,7 @@ class GalaxyAPI: if state == 'failed': code = to_native(data['error'].get('code', 'UNKNOWN')) description = to_native( - data['error'].get('description', "Unknown error, see %s for more details" % task_url)) + data['error'].get('description', "Unknown error, see %s for more details" % full_url)) raise AnsibleError("Galaxy import process failed: %s (Code: %s)" % (description, code)) @g_connect(['v2', 'v3']) diff --git a/lib/ansible/galaxy/collection.py b/lib/ansible/galaxy/collection.py index cf53426428b..3eaca23fbbf 100644 --- a/lib/ansible/galaxy/collection.py +++ b/lib/ansible/galaxy/collection.py @@ -382,10 +382,24 @@ def publish_collection(collection_path, api, wait, timeout): :param timeout: The time in seconds to wait for the import process to finish, 0 is indefinite. """ import_uri = api.publish_collection(collection_path) + if wait: + # Galaxy returns a url fragment which differs between v2 and v3. The second to last entry is + # always the task_id, though. + # v2: {"task": "https://galaxy-dev.ansible.com/api/v2/collection-imports/35573/"} + # v3: {"task": "/api/automation-hub/v3/imports/collections/838d1308-a8f4-402c-95cb-7823f3806cd8/"} + task_id = None + for path_segment in reversed(import_uri.split('/')): + if path_segment: + task_id = path_segment + break + + if not task_id: + raise AnsibleError("Publishing the collection did not return valid task info. Cannot wait for task status. Returned task info: '%s'" % import_uri) + display.display("Collection has been published to the Galaxy server %s %s" % (api.name, api.api_server)) with _display_progress(): - api.wait_import_task(import_uri, timeout) + api.wait_import_task(task_id, timeout) display.display("Collection has been successfully published and imported to the Galaxy server %s %s" % (api.name, api.api_server)) else: diff --git a/test/units/galaxy/test_api.py b/test/units/galaxy/test_api.py index d557178696b..8c69489e035 100644 --- a/test/units/galaxy/test_api.py +++ b/test/units/galaxy/test_api.py @@ -332,13 +332,16 @@ def test_publish_failure(api_version, collection_url, response, expected, collec api.publish_collection(collection_artifact) -@pytest.mark.parametrize('api_version, token_type, token_ins', [ - ('v2', 'Token', GalaxyToken('my token')), - ('v3', 'Bearer', KeycloakToken(auth_url='https://api.test/')), +@pytest.mark.parametrize('api_version, token_type, token_ins, import_uri, full_import_uri', [ + ('v2', 'Token', GalaxyToken('my token'), + '1234', + 'https://galaxy.server.com/api/v2/collection-imports/1234'), + ('v3', 'Bearer', KeycloakToken(auth_url='https://api.test/'), + '1234', + 'https://galaxy.server.com/api/automation-hub/v3/imports/collections/1234/'), ]) -def test_wait_import_task(api_version, token_type, token_ins, monkeypatch): +def test_wait_import_task(api_version, token_type, token_ins, import_uri, full_import_uri, monkeypatch): api = get_test_galaxy_api('https://galaxy.server.com/api/', api_version, token_ins=token_ins) - import_uri = 'https://galaxy.server.com/api/%s/task/1234' % api_version if token_ins: mock_token_get = MagicMock() @@ -355,20 +358,23 @@ def test_wait_import_task(api_version, token_type, token_ins, monkeypatch): api.wait_import_task(import_uri) assert mock_open.call_count == 1 - assert mock_open.mock_calls[0][1][0] == import_uri + assert mock_open.mock_calls[0][1][0] == full_import_uri assert mock_open.mock_calls[0][2]['headers']['Authorization'] == '%s my token' % token_type assert mock_display.call_count == 1 - assert mock_display.mock_calls[0][1][0] == 'Waiting until Galaxy import task %s has completed' % import_uri + assert mock_display.mock_calls[0][1][0] == 'Waiting until Galaxy import task %s has completed' % full_import_uri -@pytest.mark.parametrize('api_version, token_type, token_ins', [ - ('v2', 'Token', GalaxyToken('my token')), - ('v3', 'Bearer', KeycloakToken(auth_url='https://api.test/')), +@pytest.mark.parametrize('api_version, token_type, token_ins, import_uri, full_import_uri', [ + ('v2', 'Token', GalaxyToken('my token'), + '1234', + 'https://galaxy.server.com/api/v2/collection-imports/1234'), + ('v3', 'Bearer', KeycloakToken(auth_url='https://api.test/'), + '1234', + 'https://galaxy.server.com/api/automation-hub/v3/imports/collections/1234/'), ]) -def test_wait_import_task_multiple_requests(api_version, token_type, token_ins, monkeypatch): +def test_wait_import_task_multiple_requests(api_version, token_type, token_ins, import_uri, full_import_uri, monkeypatch): api = get_test_galaxy_api('https://galaxy.server.com/api/', api_version, token_ins=token_ins) - import_uri = 'https://galaxy.server.com/api/%s/task/1234' % api_version if token_ins: mock_token_get = MagicMock() @@ -393,26 +399,29 @@ def test_wait_import_task_multiple_requests(api_version, token_type, token_ins, api.wait_import_task(import_uri) assert mock_open.call_count == 2 - assert mock_open.mock_calls[0][1][0] == import_uri + assert mock_open.mock_calls[0][1][0] == full_import_uri assert mock_open.mock_calls[0][2]['headers']['Authorization'] == '%s my token' % token_type - assert mock_open.mock_calls[1][1][0] == import_uri + assert mock_open.mock_calls[1][1][0] == full_import_uri assert mock_open.mock_calls[1][2]['headers']['Authorization'] == '%s my token' % token_type assert mock_display.call_count == 1 - assert mock_display.mock_calls[0][1][0] == 'Waiting until Galaxy import task %s has completed' % import_uri + assert mock_display.mock_calls[0][1][0] == 'Waiting until Galaxy import task %s has completed' % full_import_uri assert mock_vvv.call_count == 1 assert mock_vvv.mock_calls[0][1][0] == \ 'Galaxy import process has a status of test, wait 2 seconds before trying again' -@pytest.mark.parametrize('api_version, token_type, token_ins', [ - ('v2', 'Token', GalaxyToken('my token')), - ('v3', 'Bearer', KeycloakToken(auth_url='https://api.test/')), +@pytest.mark.parametrize('api_version, token_type, token_ins, import_uri, full_import_uri,', [ + ('v2', 'Token', GalaxyToken('my token'), + '1234', + 'https://galaxy.server.com/api/v2/collection-imports/1234'), + ('v3', 'Bearer', KeycloakToken(auth_url='https://api.test/'), + '1234', + 'https://galaxy.server.com/api/automation-hub/v3/imports/collections/1234/'), ]) -def test_wait_import_task_with_failure(api_version, token_type, token_ins, monkeypatch): +def test_wait_import_task_with_failure(api_version, token_type, token_ins, import_uri, full_import_uri, monkeypatch): api = get_test_galaxy_api('https://galaxy.server.com/api/', api_version, token_ins=token_ins) - import_uri = 'https://galaxy.server.com/api/%s/task/1234' % api_version if token_ins: mock_token_get = MagicMock() @@ -464,11 +473,11 @@ def test_wait_import_task_with_failure(api_version, token_type, token_ins, monke api.wait_import_task(import_uri) assert mock_open.call_count == 1 - assert mock_open.mock_calls[0][1][0] == import_uri + assert mock_open.mock_calls[0][1][0] == full_import_uri assert mock_open.mock_calls[0][2]['headers']['Authorization'] == '%s my token' % token_type assert mock_display.call_count == 1 - assert mock_display.mock_calls[0][1][0] == 'Waiting until Galaxy import task %s has completed' % import_uri + assert mock_display.mock_calls[0][1][0] == 'Waiting until Galaxy import task %s has completed' % full_import_uri assert mock_vvv.call_count == 1 assert mock_vvv.mock_calls[0][1][0] == u'Galaxy import message: info - Somé info' @@ -480,13 +489,16 @@ def test_wait_import_task_with_failure(api_version, token_type, token_ins, monke assert mock_err.mock_calls[0][1][0] == u'Galaxy import error message: Somé error' -@pytest.mark.parametrize('api_version, token_type, token_ins', [ - ('v2', 'Token', GalaxyToken('my_token')), - ('v3', 'Bearer', KeycloakToken(auth_url='https://api.test/')), +@pytest.mark.parametrize('api_version, token_type, token_ins, import_uri, full_import_uri', [ + ('v2', 'Token', GalaxyToken('my_token'), + '1234', + 'https://galaxy.server.com/api/v2/collection-imports/1234'), + ('v3', 'Bearer', KeycloakToken(auth_url='https://api.test/'), + '1234', + 'https://galaxy.server.com/api/automation-hub/v3/imports/collections/1234/'), ]) -def test_wait_import_task_with_failure_no_error(api_version, token_type, token_ins, monkeypatch): +def test_wait_import_task_with_failure_no_error(api_version, token_type, token_ins, import_uri, full_import_uri, monkeypatch): api = get_test_galaxy_api('https://galaxy.server.com/api/', api_version, token_ins=token_ins) - import_uri = 'https://galaxy.server.com/api/%s/task/1234' % api_version if token_ins: mock_token_get = MagicMock() @@ -529,16 +541,16 @@ def test_wait_import_task_with_failure_no_error(api_version, token_type, token_i mock_err = MagicMock() monkeypatch.setattr(Display, 'error', mock_err) - expected = 'Galaxy import process failed: Unknown error, see %s for more details (Code: UNKNOWN)' % import_uri - with pytest.raises(AnsibleError, match=re.escape(expected)): + expected = 'Galaxy import process failed: Unknown error, see %s for more details \\(Code: UNKNOWN\\)' % full_import_uri + with pytest.raises(AnsibleError, match=expected): api.wait_import_task(import_uri) assert mock_open.call_count == 1 - assert mock_open.mock_calls[0][1][0] == import_uri + assert mock_open.mock_calls[0][1][0] == full_import_uri assert mock_open.mock_calls[0][2]['headers']['Authorization'] == '%s my token' % token_type assert mock_display.call_count == 1 - assert mock_display.mock_calls[0][1][0] == 'Waiting until Galaxy import task %s has completed' % import_uri + assert mock_display.mock_calls[0][1][0] == 'Waiting until Galaxy import task %s has completed' % full_import_uri assert mock_vvv.call_count == 1 assert mock_vvv.mock_calls[0][1][0] == u'Galaxy import message: info - Somé info' @@ -550,13 +562,16 @@ def test_wait_import_task_with_failure_no_error(api_version, token_type, token_i assert mock_err.mock_calls[0][1][0] == u'Galaxy import error message: Somé error' -@pytest.mark.parametrize('api_version, token_type, token_ins', [ - ('v2', 'Token', GalaxyToken('my token')), - ('v3', 'Bearer', KeycloakToken(auth_url='https://api.test/')), +@pytest.mark.parametrize('api_version, token_type, token_ins, import_uri, full_import_uri', [ + ('v2', 'Token', GalaxyToken('my token'), + '1234', + 'https://galaxy.server.com/api/v2/collection-imports/1234'), + ('v3', 'Bearer', KeycloakToken(auth_url='https://api.test/'), + '1234', + 'https://galaxy.server.com/api/automation-hub/v3/imports/collections/1234/'), ]) -def test_wait_import_task_timeout(api_version, token_type, token_ins, monkeypatch): +def test_wait_import_task_timeout(api_version, token_type, token_ins, import_uri, full_import_uri, monkeypatch): api = get_test_galaxy_api('https://galaxy.server.com/api/', api_version, token_ins=token_ins) - import_uri = 'https://galaxy.server.com/api/%s/task/1234' % api_version if token_ins: mock_token_get = MagicMock() @@ -578,18 +593,18 @@ def test_wait_import_task_timeout(api_version, token_type, token_ins, monkeypatc monkeypatch.setattr(time, 'sleep', MagicMock()) - expected = "Timeout while waiting for the Galaxy import process to finish, check progress at '%s'" % import_uri + expected = "Timeout while waiting for the Galaxy import process to finish, check progress at '%s'" % full_import_uri with pytest.raises(AnsibleError, match=expected): api.wait_import_task(import_uri, 1) assert mock_open.call_count > 1 - assert mock_open.mock_calls[0][1][0] == import_uri + assert mock_open.mock_calls[0][1][0] == full_import_uri assert mock_open.mock_calls[0][2]['headers']['Authorization'] == '%s my token' % token_type - assert mock_open.mock_calls[1][1][0] == import_uri + assert mock_open.mock_calls[1][1][0] == full_import_uri assert mock_open.mock_calls[1][2]['headers']['Authorization'] == '%s my token' % token_type assert mock_display.call_count == 1 - assert mock_display.mock_calls[0][1][0] == 'Waiting until Galaxy import task %s has completed' % import_uri + assert mock_display.mock_calls[0][1][0] == 'Waiting until Galaxy import task %s has completed' % full_import_uri # expected_wait_msg = 'Galaxy import process has a status of waiting, wait {0} seconds before trying again' assert mock_vvv.call_count > 9 # 1st is opening Galaxy token file. diff --git a/test/units/galaxy/test_collection.py b/test/units/galaxy/test_collection.py index c86a524a090..18df8bbdcd6 100644 --- a/test/units/galaxy/test_collection.py +++ b/test/units/galaxy/test_collection.py @@ -439,7 +439,7 @@ def test_publish_with_wait(galaxy_server, collection_artifact, monkeypatch): assert mock_publish.mock_calls[0][1][0] == artifact_path assert mock_wait.call_count == 1 - assert mock_wait.mock_calls[0][1][0] == fake_import_uri + assert mock_wait.mock_calls[0][1][0] == '1234' assert mock_display.mock_calls[0][1][0] == "Collection has been published to the Galaxy server test_server %s" \ % galaxy_server.api_server