If we can't parse SCM URL from Koji, send an error result (#218)

If Greenwave can't parse the SCM URL for a Koji build (which can
happen if the build is run from a .src.rpm, which Koji admins
can do), it raises an exception in `_get_sub_policies`, which
ultimately results in the query returning 502. This doesn't give
a great experience in Bodhi - it'll just show no automated test
results and the gating status will be stuck at 'waiting' forever.
Using browser developer tools you can determine that it's getting
a 502 from Greenwave, but you can't tell why (we had to find that
out from greenwave's logs, in the real-world case here).

If we have Greenwave instead handle the exception and send a
response that includes an unsatisfied requirement indicating the
error, that should result in a better experience in Bodhi. The
gating status will be failed, test results will be shown, and the
UI will give at least some indication that there was an error
trying to retrieve remote policies. With developer tools you
should see the exact error in the Greenwave response JSON.

Signed-off-by: Adam Williamson <awilliam@redhat.com>

greenwave/policies.py

@@ -606,6 +606,8 @@ def _get_sub_policies(self, policy, subject):
             logging.exception('Unexpected Koji XMLRPC fault with code: %s', err.faultCode)
             error = f'Koji XMLRPC fault due to: \'{err.faultString}\''
             raise BadGateway(error)
+        except greenwave.resources.KojiScmUrlParseError as err:
+            return [], [FailedFetchRemoteRuleYaml(subject, remote_policies_urls, err.description)]
         except Exception:
             logging.exception('Failed to retrieve policies for %r', subject)
             error = 'Unexpected error while fetching remote policies'

greenwave/resources.py

@@ -169,6 +169,13 @@ class NoSourceException(RuntimeError):
     pass
 
 
+class KojiScmUrlParseError(BadGateway):
+    """
+    Exception raised when parsing SCM revision from Koji build fails.
+    """
+    pass
+
+
 @cached
 def retrieve_koji_build_target(task_id, koji_url: str):
     log.debug('Getting Koji task request ID %r', task_id)
@@ -256,7 +263,7 @@ def retrieve_scm_from_koji_build(nvr: str, source: str, koji_url: str):
 
     rev = url.fragment
     if not rev:
-        raise BadGateway(
+        raise KojiScmUrlParseError(
             'Failed to parse SCM URL "{}" from Koji build "{}" at "{}" '
             '(missing URL fragment with SCM revision information)'.format(source, nvr, koji_url)
         )

greenwave/tests/test_policies.py

@@ -21,7 +21,7 @@
     TestResultFailed,
     OnDemandPolicy
 )
-from greenwave.resources import ResultsRetriever
+from greenwave.resources import ResultsRetriever, KojiScmUrlParseError
 from greenwave.safe_yaml import SafeYAMLError
 from greenwave.subjects.factory import create_subject
 from greenwave.waivers import waive_answers
@@ -747,6 +747,43 @@ def test_get_sub_policies_multiple_urls(tmpdir):
                 assert decision.answers[0].subject.identifier == subject.identifier
 
 
+def test_get_sub_policies_scm_error(tmpdir):
+    """
+    Test that _get_sub_policies correctly returns an error to go in
+    the response - but doesn't raise an exception - when SCM URL parse
+    fails.
+    """
+
+    nvr = '389-ds-1.4-820181127205924.9edba152'
+    subject = create_subject('redhat-container-image', nvr)
+
+    serverside_fragment = dedent("""
+        --- !Policy
+        id: "taskotron_release_critical_tasks_with_remoterule"
+        product_versions:
+          - rhel-8
+        decision_contexts:
+          - osci_compose_gate1
+          - osci_compose_gate2
+        subject_type: redhat-container-image
+        rules:
+          - !RemoteRule {}
+        """)
+
+    p = tmpdir.join('gating.yaml')
+    p.write(serverside_fragment)
+    with mock.patch('greenwave.resources.retrieve_scm_from_koji') as scm:
+        scm.side_effect = KojiScmUrlParseError("Failed to parse SCM URL")
+        policies = load_policies(tmpdir.strpath)
+        results = DummyResultsRetriever(
+            subject, 'baseos-ci.redhat-container-image.tier0.functional')
+        decision = Decision('osci_compose_gate1', 'rhel-8')
+        decision.check(subject, policies, results)
+        assert answer_types(decision.answers) == ['failed-fetch-gating-yaml']
+        assert not decision.answers[0].is_satisfied
+        assert decision.answers[0].subject.identifier == subject.identifier
+
+
 def test_redhat_container_image_subject_type():
     nvr = '389-ds-1.4-820181127205924.9edba152'
     rdb_url = 'http://results.db'

greenwave/tests/test_retrieve_gating_yaml.py

@@ -5,10 +5,11 @@
 
 import pytest
 import mock
-from werkzeug.exceptions import BadGateway, NotFound
+from werkzeug.exceptions import NotFound
 
 from greenwave.resources import (
     NoSourceException,
+    KojiScmUrlParseError,
     retrieve_scm_from_koji,
     retrieve_yaml_remote_rule,
 )
@@ -114,7 +115,7 @@ def test_retrieve_scm_from_build_with_missing_rev(app, koji_proxy):
         }
     }
     expected_error = 'missing URL fragment with SCM revision information'
-    with pytest.raises(BadGateway, match=expected_error):
+    with pytest.raises(KojiScmUrlParseError, match=expected_error):
         retrieve_scm_from_koji(nvr)