From f71613976fc406250c38cfe5153e7335f50383d9 Mon Sep 17 00:00:00 2001
From: Giulia Naponiello <gnaponie@redhat.com>
Date: Jun 18 2018 11:45:12 +0000
Subject: [PATCH 1/2] Tolerate invalid gating.yaml


Let's tollerate invalid gating.yaml files when they have:
* malformed yaml syntax
* RemoteRule inside the gating.yaml
...just skip the check and log a warning.

---

diff --git a/greenwave/policies.py b/greenwave/policies.py
index 014dfed..3f900fc 100644
--- a/greenwave/policies.py
+++ b/greenwave/policies.py
@@ -2,8 +2,11 @@
 
 from fnmatch import fnmatch
 import yaml
+import logging
 import greenwave.resources
 
+log = logging.getLogger(__name__)
+
 
 def validate_policies(policies, disallowed_rules=None):
     disallowed_rules = disallowed_rules or []
@@ -21,8 +24,10 @@ def validate_policies(policies, disallowed_rules=None):
                                    'is not an instance of Rule' % rule)
             for disallowed_rule in disallowed_rules:
                 if isinstance(rule, disallowed_rule):
-                    raise RuntimeError('Policies are not configured properly as rule %s '
-                                       'is an instance of %s' % (rule, disallowed_rule))
+                    log.warning('Policies are not configured properly as rule %s '
+                                'is an instance of %s' % (rule, disallowed_rule))
+                    return RuntimeError('Policies are not configured properly as rule %s '
+                                        'is an instance of %s' % (rule, disallowed_rule))
 
 
 def subject_type_identifier_to_item(subject_type, subject_identifier):
@@ -277,13 +282,24 @@ class RemoteRule(Rule):
             # greenwave extension file not found
             return []
 
-        policies = yaml.safe_load_all(response)
-        # policies is a generator, so listifying it
-        policies = list(policies)
+        try:
+            policies = yaml.safe_load_all(response)
+            # policies is a generator, so listifying it
+            policies = list(policies)
+        except yaml.parser.ParserError:
+            # if the yaml file is malformed we skip these policies
+            log.warning("Error parsing the founded gating.yaml file.")
+            return []
         # policies in dist-git are always about a package
         for policy in policies:
             policy.subject_type = 'koji_build'
-        validate_policies(policies, [RemoteRule])
+        r = validate_policies(policies, [RemoteRule])
+        if (r and type(r) == RuntimeError and
+           r.__str__().startswith("Policies are not configured properly as rule") and
+           "is an instance of <class 'greenwave.policies.RemoteRule'" in r.__str__()):
+            return []
+        else:
+            raise r
         answers = []
         for policy in policies:
             response = policy.check(subject_identifier, results, waivers)
diff --git a/greenwave/tests/test_policies.py b/greenwave/tests/test_policies.py
index b299896..bd16196 100644
--- a/greenwave/tests/test_policies.py
+++ b/greenwave/tests/test_policies.py
@@ -306,3 +306,64 @@ rules:
                 decision = policy.check(nvr, results, waivers)
                 assert len(decision) == 1
                 assert isinstance(decision[0], TestResultFailed)
+
+
+def test_remote_rule_malformed_yaml(tmpdir):
+    """ Testing the RemoteRule with a malformed gating.yaml file """
+
+    nvr = 'nethack-1.2.3-1.el9000'
+
+    serverside_fragment = """
+--- !Policy
+id: "taskotron_release_critical_tasks_with_remoterule"
+product_versions:
+  - fedora-26
+decision_context: bodhi_update_push_stable_with_remoterule
+subject_type: koji_build
+blacklist: []
+rules:
+  - !RemoteRule {}
+        """
+
+    remote_fragments = ["""
+--- !Policy
+   : "some-policy-from-a-random-packager"
+product_versions:
+  - fedora-26
+decision_context: bodhi_update_push_stable_with_remoterule
+blacklist: []
+rules:
+  - !PassingTestCaseRule {test_case_name: dist.upgradepath}
+        """, """
+--- !Policy
+id: "some-policy-from-a-random-packager"
+product_versions:
+  - fedora-26
+decision_context: bodhi_update_push_stable_with_remoterule
+blacklist: []
+rules:
+  - !RemoteRule {test_case_name: dist.upgradepath}
+        """]
+
+    for remote_fragment in remote_fragments:
+        p = tmpdir.join('gating.yaml')
+        p.write(serverside_fragment)
+        app = create_app('greenwave.config.TestingConfig')
+        with app.app_context():
+            with mock.patch('greenwave.resources.retrieve_rev_from_koji'):
+                with mock.patch('greenwave.resources.retrieve_yaml_remote_rule') as f:
+                    f.return_value = remote_fragment
+                    policies = load_policies(tmpdir.strpath)
+                    policy = policies[0]
+
+                    waivers = []
+
+                    # Ensure that presence of a result is success.
+                    results = [{
+                        "id": 12345,
+                        "data": {"original_spec_nvr": [nvr]},
+                        "testcase": {"name": "dist.upgradepath"},
+                        "outcome": "PASSED"
+                    }]
+                    decision = policy.check(nvr, results, waivers)
+                    assert len(decision) == 0

From d63b0ed2bc8fde1b97659a728dfb7279531fb142 Mon Sep 17 00:00:00 2001
From: Giulia Naponiello <gnaponie@redhat.com>
Date: Jun 18 2018 15:50:03 +0000
Subject: [PATCH 2/2] Add a fake result to inform the user of an error


Adding a fake result with outcome == NEEDS_INSPECTION when there is
a misconfigured gating.yaml file. The user will see this result in
the output of the decision API and will notice there is something
wrong with his/her configuration.

---

diff --git a/greenwave/policies.py b/greenwave/policies.py
index 3f900fc..a021792 100644
--- a/greenwave/policies.py
+++ b/greenwave/policies.py
@@ -45,6 +45,17 @@ def subject_type_identifier_to_item(subject_type, subject_identifier):
         raise RuntimeError('Unrecognised subject type: %s' % subject_type)
 
 
+def handle_remote_rule_misconfigured_yaml(results, subject_identifier, subject_type, note):
+    results.append(
+        {'data': {
+            'item': [subject_identifier],
+            'type': [subject_type],
+            'id': -1,
+            'outcome': 'NEEDS_INSPECTION',
+            'note': note}})
+    return results
+
+
 class Answer(object):
     """
     Represents the result of evaluating a policy rule against a particular
@@ -288,7 +299,10 @@ class RemoteRule(Rule):
             policies = list(policies)
         except yaml.parser.ParserError:
             # if the yaml file is malformed we skip these policies
-            log.warning("Error parsing the founded gating.yaml file.")
+            msg = "Error parsing the founded gating.yaml file."
+            log.warning(msg)
+            results = handle_remote_rule_misconfigured_yaml(results, subject_identifier,
+                                                            subject_type, msg)
             return []
         # policies in dist-git are always about a package
         for policy in policies:
@@ -297,9 +311,11 @@ class RemoteRule(Rule):
         if (r and type(r) == RuntimeError and
            r.__str__().startswith("Policies are not configured properly as rule") and
            "is an instance of <class 'greenwave.policies.RemoteRule'" in r.__str__()):
+            results = handle_remote_rule_misconfigured_yaml(results, subject_identifier,
+                                                            subject_type, r.__str__())
             return []
-        else:
-            raise r
+        elif r:
+            raise RuntimeError(r)
         answers = []
         for policy in policies:
             response = policy.check(subject_identifier, results, waivers)
diff --git a/greenwave/tests/test_policies.py b/greenwave/tests/test_policies.py
index bd16196..450e689 100644
--- a/greenwave/tests/test_policies.py
+++ b/greenwave/tests/test_policies.py
@@ -359,11 +359,13 @@ rules:
                     waivers = []
 
                     # Ensure that presence of a result is success.
-                    results = [{
-                        "id": 12345,
-                        "data": {"original_spec_nvr": [nvr]},
-                        "testcase": {"name": "dist.upgradepath"},
-                        "outcome": "PASSED"
-                    }]
+                    results = []
                     decision = policy.check(nvr, results, waivers)
+                    assert results[0]['data']['outcome'] == 'NEEDS_INSPECTION'
+                    assert (results[0]['data']['note'] ==
+                            'Error parsing the founded gating.yaml file.' or (
+                                results[0]['data']['note'].startswith(
+                                    "Policies are not configured properly as rule"
+                                ) and results[0]['data']['note'].endswith(
+                                    "is an instance of <class 'greenwave.policies.RemoteRule'>")))
                     assert len(decision) == 0