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.
pull/63614/head
Toshio Kuratomi 5 years ago committed by GitHub
parent 82ee341fe0
commit 4cad7e479c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

@ -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.

@ -15,7 +15,7 @@ from ansible import context
from ansible.errors import AnsibleError from ansible.errors import AnsibleError
from ansible.module_utils.six import string_types 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.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._text import to_bytes, to_native, to_text
from ansible.module_utils.urls import open_url from ansible.module_utils.urls import open_url
from ansible.utils.display import Display from ansible.utils.display import Display
@ -439,24 +439,35 @@ class GalaxyAPI:
return resp['task'] return resp['task']
@g_connect(['v2', 'v3']) @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. 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. :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. # TODO: actually verify that v3 returns the same structure as v2, right now this is just an assumption.
state = 'waiting' state = 'waiting'
data = None 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() start = time.time()
wait = 2 wait = 2
while timeout == 0 or (time.time() - start) < timeout: while timeout == 0 or (time.time() - start) < timeout:
data = self._call_galaxy(task_url, method='GET', auth_required=True, data = self._call_galaxy(full_url, method='GET', auth_required=True,
error_context_msg='Error when getting import task results at %s' % task_url) error_context_msg='Error when getting import task results at %s' % full_url)
state = data.get('state', 'waiting') state = data.get('state', 'waiting')
@ -471,7 +482,7 @@ class GalaxyAPI:
wait = min(30, wait * 1.5) wait = min(30, wait * 1.5)
if state == 'waiting': if state == 'waiting':
raise AnsibleError("Timeout while waiting for the Galaxy import process to finish, check progress at '%s'" 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', []): for message in data.get('messages', []):
level = message['level'] level = message['level']
@ -485,7 +496,7 @@ class GalaxyAPI:
if state == 'failed': if state == 'failed':
code = to_native(data['error'].get('code', 'UNKNOWN')) code = to_native(data['error'].get('code', 'UNKNOWN'))
description = to_native( 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)) raise AnsibleError("Galaxy import process failed: %s (Code: %s)" % (description, code))
@g_connect(['v2', 'v3']) @g_connect(['v2', 'v3'])

@ -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. :param timeout: The time in seconds to wait for the import process to finish, 0 is indefinite.
""" """
import_uri = api.publish_collection(collection_path) import_uri = api.publish_collection(collection_path)
if wait: 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)) display.display("Collection has been published to the Galaxy server %s %s" % (api.name, api.api_server))
with _display_progress(): 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" display.display("Collection has been successfully published and imported to the Galaxy server %s %s"
% (api.name, api.api_server)) % (api.name, api.api_server))
else: else:

@ -332,13 +332,16 @@ def test_publish_failure(api_version, collection_url, response, expected, collec
api.publish_collection(collection_artifact) api.publish_collection(collection_artifact)
@pytest.mark.parametrize('api_version, token_type, token_ins', [ @pytest.mark.parametrize('api_version, token_type, token_ins, import_uri, full_import_uri', [
('v2', 'Token', GalaxyToken('my token')), ('v2', 'Token', GalaxyToken('my token'),
('v3', 'Bearer', KeycloakToken(auth_url='https://api.test/')), '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) 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: if token_ins:
mock_token_get = MagicMock() 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) api.wait_import_task(import_uri)
assert mock_open.call_count == 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[0][2]['headers']['Authorization'] == '%s my token' % token_type
assert mock_display.call_count == 1 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', [ @pytest.mark.parametrize('api_version, token_type, token_ins, import_uri, full_import_uri', [
('v2', 'Token', GalaxyToken('my token')), ('v2', 'Token', GalaxyToken('my token'),
('v3', 'Bearer', KeycloakToken(auth_url='https://api.test/')), '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) 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: if token_ins:
mock_token_get = MagicMock() 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) api.wait_import_task(import_uri)
assert mock_open.call_count == 2 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[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_open.mock_calls[1][2]['headers']['Authorization'] == '%s my token' % token_type
assert mock_display.call_count == 1 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.call_count == 1
assert mock_vvv.mock_calls[0][1][0] == \ assert mock_vvv.mock_calls[0][1][0] == \
'Galaxy import process has a status of test, wait 2 seconds before trying again' 'Galaxy import process has a status of test, wait 2 seconds before trying again'
@pytest.mark.parametrize('api_version, token_type, token_ins', [ @pytest.mark.parametrize('api_version, token_type, token_ins, import_uri, full_import_uri,', [
('v2', 'Token', GalaxyToken('my token')), ('v2', 'Token', GalaxyToken('my token'),
('v3', 'Bearer', KeycloakToken(auth_url='https://api.test/')), '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) 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: if token_ins:
mock_token_get = MagicMock() 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) api.wait_import_task(import_uri)
assert mock_open.call_count == 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[0][2]['headers']['Authorization'] == '%s my token' % token_type
assert mock_display.call_count == 1 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.call_count == 1
assert mock_vvv.mock_calls[0][1][0] == u'Galaxy import message: info - Somé info' 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' assert mock_err.mock_calls[0][1][0] == u'Galaxy import error message: Somé error'
@pytest.mark.parametrize('api_version, token_type, token_ins', [ @pytest.mark.parametrize('api_version, token_type, token_ins, import_uri, full_import_uri', [
('v2', 'Token', GalaxyToken('my_token')), ('v2', 'Token', GalaxyToken('my_token'),
('v3', 'Bearer', KeycloakToken(auth_url='https://api.test/')), '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) 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: if token_ins:
mock_token_get = MagicMock() 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() mock_err = MagicMock()
monkeypatch.setattr(Display, 'error', mock_err) monkeypatch.setattr(Display, 'error', mock_err)
expected = 'Galaxy import process failed: Unknown error, see %s for more details (Code: UNKNOWN)' % import_uri expected = 'Galaxy import process failed: Unknown error, see %s for more details \\(Code: UNKNOWN\\)' % full_import_uri
with pytest.raises(AnsibleError, match=re.escape(expected)): with pytest.raises(AnsibleError, match=expected):
api.wait_import_task(import_uri) api.wait_import_task(import_uri)
assert mock_open.call_count == 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[0][2]['headers']['Authorization'] == '%s my token' % token_type
assert mock_display.call_count == 1 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.call_count == 1
assert mock_vvv.mock_calls[0][1][0] == u'Galaxy import message: info - Somé info' 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' assert mock_err.mock_calls[0][1][0] == u'Galaxy import error message: Somé error'
@pytest.mark.parametrize('api_version, token_type, token_ins', [ @pytest.mark.parametrize('api_version, token_type, token_ins, import_uri, full_import_uri', [
('v2', 'Token', GalaxyToken('my token')), ('v2', 'Token', GalaxyToken('my token'),
('v3', 'Bearer', KeycloakToken(auth_url='https://api.test/')), '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) 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: if token_ins:
mock_token_get = MagicMock() 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()) 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): with pytest.raises(AnsibleError, match=expected):
api.wait_import_task(import_uri, 1) api.wait_import_task(import_uri, 1)
assert mock_open.call_count > 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[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_open.mock_calls[1][2]['headers']['Authorization'] == '%s my token' % token_type
assert mock_display.call_count == 1 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' # 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. assert mock_vvv.call_count > 9 # 1st is opening Galaxy token file.

@ -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_publish.mock_calls[0][1][0] == artifact_path
assert mock_wait.call_count == 1 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" \ assert mock_display.mock_calls[0][1][0] == "Collection has been published to the Galaxy server test_server %s" \
% galaxy_server.api_server % galaxy_server.api_server

Loading…
Cancel
Save