[stable-2.9] 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.
(cherry picked from commit 4cad7e4)

Co-authored-by: Toshio Kuratomi <a.badger@gmail.com>

5 files changed, 93 insertions, 50 deletions

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 0000000000..31552d2d0e
--- /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 17b1df751d..6c04a215d3 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 caa22308eb..81f09c50ab 100644
--- a/lib/ansible/galaxy/collection.py
+++ b/lib/ansible/galaxy/collection.py
@@ -381,10 +381,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 d557178696..8c69489e03 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 c86a524a09..18df8bbdcd 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