From e9a478c21a9fd3f30e33d0bb8232c7cb6ed3a038 Mon Sep 17 00:00:00 2001
From: Lukas Holecek <hluk@email.cz>
Date: Aug 14 2018 12:12:51 +0000
Subject: Improve error messages for Koji build retrieval


---

diff --git a/greenwave/resources.py b/greenwave/resources.py
index e57129c..06ab7fd 100644
--- a/greenwave/resources.py
+++ b/greenwave/resources.py
@@ -28,27 +28,43 @@ requests_session = requests.Session()
 @greenwave.utils.retry(wait_on=urllib3.exceptions.NewConnectionError)
 def retrieve_scm_from_koji(nvr):
     """ Retrieve cached rev and namespace from koji using the nvr """
-    proxy = xmlrpc.client.ServerProxy(current_app.config['KOJI_BASE_URL'])
+    koji_url = current_app.config['KOJI_BASE_URL']
+    proxy = xmlrpc.client.ServerProxy(koji_url)
     build = proxy.getBuild(nvr)
+    return retrieve_scm_from_koji_build(nvr, build, koji_url)
 
+
+def retrieve_scm_from_koji_build(nvr, build, koji_url):
     if not build:
-        raise BadGateway("Found %s when looking for %s at %s" % (
-            build, nvr, current_app.config['KOJI_BASE_URL']))
-
-    try:
-        url = urlparse(build['source'])
-
-        if not url.scheme.startswith('git'):
-            raise BadGateway('Unable to extract scm from koji.  '
-                             '%s doesn\'t begin with git://' % url)
-
-        rev = url.fragment
-        namespace = url.path.split('/')[-2]
-        return namespace, rev
-    except Exception:
-        error = 'Error occurred looking for the "rev" in koji.'
-        log.exception(error)
-        raise BadGateway(error)
+        raise BadGateway(
+            'Failed to find Koji build for "{}" at "{}"'.format(nvr, koji_url))
+
+    source = build.get('source')
+    if not source:
+        raise BadGateway(
+            'Failed to retrieve SCM URL from Koji build "{}" at "{}" '
+            '(expected SCM URL in "source" attribute)'
+            .format(nvr, koji_url))
+
+    url = urlparse(source)
+
+    path_components = url.path.rsplit('/', 2)
+    if len(path_components) < 3:
+        raise BadGateway(
+            'Failed to parse SCM URL "{}" from Koji build "{}" at "{}" '
+            '(expected second to last component to be namespace)'
+            .format(source, nvr, koji_url))
+
+    namespace = path_components[-2]
+
+    rev = url.fragment
+    if not rev:
+        raise BadGateway(
+            'Failed to parse SCM URL "{}" from Koji build "{}" at "{}" '
+            '(missing URL fragment with SCM revision information)'
+            .format(source, nvr, koji_url))
+
+    return namespace, rev
 
 
 @cached
diff --git a/greenwave/tests/test_retrieve_gating_yaml.py b/greenwave/tests/test_retrieve_gating_yaml.py
new file mode 100644
index 0000000..25d6e1b
--- /dev/null
+++ b/greenwave/tests/test_retrieve_gating_yaml.py
@@ -0,0 +1,71 @@
+# SPDX-License-Identifier: GPL-2.0+
+
+import pytest
+
+from werkzeug.exceptions import BadGateway
+
+from greenwave.resources import retrieve_scm_from_koji_build
+
+KOJI_URL = 'https://koji.fedoraproject.org/kojihub'
+
+
+def test_retrieve_scm_from_rpm_build():
+    nvr = 'nethack-3.6.1-3.fc29'
+    build = {
+        'nvr': nvr,
+        'source': 'git+https://src.fedoraproject.org/rpms/nethack.git#0c1a84e0e8a152897003bd7e27b3f407ff6ba040' # noqa
+    }
+    namespace, rev = retrieve_scm_from_koji_build(nvr, build, KOJI_URL)
+    assert namespace == 'rpms'
+    assert rev == '0c1a84e0e8a152897003bd7e27b3f407ff6ba040'
+
+
+def test_retrieve_scm_from_container_build():
+    nvr = 'golang-github-openshift-prometheus-alert-buffer-container-v3.10.0-0.34.0.0'
+    build = {
+        'nvr': nvr,
+        'source': 'git://pkgs.devel.redhat.com/containers/golang-github-openshift-prometheus-alert-buffer#46af2f8efbfb0a4e7e7d5676f4efb997f72d4b8c' # noqa
+    }
+    namespace, rev = retrieve_scm_from_koji_build(nvr, build, KOJI_URL)
+    assert namespace == 'containers'
+    assert rev == '46af2f8efbfb0a4e7e7d5676f4efb997f72d4b8c'
+
+
+def test_retrieve_scm_from_nonexistent_build():
+    nvr = 'foo-1.2.3-1.fc29'
+    build = {}
+    expected_error = 'Failed to find Koji build for "{}" at "{}"'.format(nvr, KOJI_URL)
+    with pytest.raises(BadGateway, match=expected_error):
+        retrieve_scm_from_koji_build(nvr, build, KOJI_URL)
+
+
+def test_retrieve_scm_from_build_with_missing_source():
+    nvr = 'foo-1.2.3-1.fc29'
+    build = {
+        'nvr': nvr
+    }
+    expected_error = 'expected SCM URL in "source" attribute'
+    with pytest.raises(BadGateway, match=expected_error):
+        retrieve_scm_from_koji_build(nvr, build, KOJI_URL)
+
+
+def test_retrieve_scm_from_build_with_bad_source():
+    nvr = 'foo-1.2.3-1.fc29'
+    build = {
+        'nvr': nvr,
+        'source': 'git+https://src.fedoraproject.org/foo.git#deadbeef',
+    }
+    expected_error = 'expected second to last component to be namespace'
+    with pytest.raises(BadGateway, match=expected_error):
+        retrieve_scm_from_koji_build(nvr, build, KOJI_URL)
+
+
+def test_retrieve_scm_from_build_with_missing_rev():
+    nvr = 'foo-1.2.3-1.fc29'
+    build = {
+        'nvr': nvr,
+        'source': 'git+https://src.fedoraproject.org/rpms/foo.git',
+    }
+    expected_error = 'missing URL fragment with SCM revision information'
+    with pytest.raises(BadGateway, match=expected_error):
+        retrieve_scm_from_koji_build(nvr, build, KOJI_URL)