Allow querying multiple decision contexts in a single decision (#95)

When Greenwave was designed we never really envisaged a use for
querying multiple decision contexts at once. We assumed it'd
only ever make sense to care about exactly one context for
whatever decision you're trying to make.

However, we wound up using multiple decision contexts to solve
the problem of wanting different policies for the same gating
point depending on the contents of the thing being gated.
For a specific example, when gating Fedora updates, we have
a `bodhi_update_push_testing` context that we always query for
any update being pushed to testing, but we also have a
`bodhi_update_push_testing_critpath` context that we only query
for critical path updates (because it requires some tests that
are only *run* on critical path updates). A pending Bodhi pull
request - fedora-infra/bodhi#4759 -
would make Bodhi query even more decision contexts for critical
path updates.

Currently when Bodhi wants to query multiple decision contexts,
it has to ask for one decision per context. This is pretty
inefficient. It actually turns out to be quite easy to just
allow the `decision_context` value to be a list of strings as
well as a single string; all that really needs to be done to
handle this is a fairly small tweak to `Policy.matches()`. Since
we already iterate over every policy, I don't think this even
has any significant performance impact, it should be about as
fast to check every policy against a list of decision contexts
as it is to check it against a single context.

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

functional-tests/test_api_v1.py

@@ -177,8 +177,8 @@ def test_404_for_invalid_product_version(requests_session, greenwave_server, tes
     }
     r = requests_session.post(greenwave_server + 'api/v1.0/decision', json=data)
     assert r.status_code == 404
-    expected = ('Cannot find any applicable policies for koji_build subjects '
-                'at gating point bodhi_push_update_stable in f26')
+    expected = ('Found no applicable policies for koji_build subjects '
+                'at gating point(s) bodhi_push_update_stable in f26')
     assert expected == r.json()['message']
 
 
@@ -193,8 +193,8 @@ def test_404_for_invalid_decision_context(requests_session, greenwave_server, te
     }
     r = requests_session.post(greenwave_server + 'api/v1.0/decision', json=data)
     assert r.status_code == 404
-    expected = ('Cannot find any applicable policies for koji_build subjects '
-                'at gating point bodhi_push_update in fedora-26')
+    expected = ('Found no applicable policies for koji_build subjects '
+                'at gating point(s) bodhi_push_update in fedora-26')
     assert expected == r.json()['message']
 
 

greenwave/api_v1.py

@@ -257,6 +257,25 @@ def make_decision():
            "verbose": true
        }
 
+    **Sample request 3**:
+
+    It is also possible to specify decision_context as a list, so you can query
+    multiple decision contexts at once.
+
+    .. sourcecode:: http
+
+       POST /api/v1.0/decision HTTP/1.1
+       Accept: application/json
+       Content-Type: application/json
+
+       {
+           "decision_context": ["bodhi_update_push_stable", "bodhi_update_push_stable_critpath"],
+           "product_version": "fedora-32",
+           "subject_type": "koji_build",
+           "subject_identifier": "bodhi-5.1.1-1.fc32",
+           "verbose": true
+       }
+
     **Sample On-demand policy request**:
 
     Note: Greenwave would not publish a message on the message bus when an on-demand
@@ -304,10 +323,10 @@ def make_decision():
        }
 
     :jsonparam string product_version: The product version string used for querying WaiverDB.
-    :jsonparam string decision_context: The decision context string, identified by a
-        free-form string label. It is to be named through coordination between policy
+    :jsonparam string decision_context: The decision context(s). Either a string or a list of
+        strings. These are free-form labels to be named through coordination between policy
         author and calling application, for example ``bodhi_update_push_stable``.
-        Do not use this parameter with `rules`.
+        Do not use this parameter together with `rules`.
     :jsonparam string subject_type: The type of software artefact we are
         making a decision about, for example ``koji_build``.
         See :ref:`subject-types` for a list of possible subject types.

greenwave/decision.py

@@ -51,6 +51,7 @@ class Decision:
     Collects answers from rules from policies.
     """
     def __init__(self, decision_context, product_version, verbose=False):
+        # this can be a single string or a list of strings
         self.decision_context = decision_context
         self.product_version = product_version
         self.verbose = verbose
@@ -74,9 +75,13 @@ def check(self, subject, policies, results_retriever):
             if subject.ignore_missing_policy:
                 return
 
+            dc = self.decision_context
+            if isinstance(dc, list):
+                dc = ' '.join(dc)
+
             raise NotFound(
-                'Cannot find any applicable policies for %s subjects at gating point %s in %s' % (
-                    subject.type, self.decision_context, self.product_version))
+                'Found no applicable policies for %s subjects at gating point(s) %s in %s' % (
+                    subject.type, dc, self.product_version))
 
         if self.verbose:
             # Retrieve test results and waivers for all items when verbose output is requested.
@@ -175,9 +180,12 @@ def make_decision(data, config):
     log.debug('New decision request for data: %s', data)
     product_version = data['product_version']
 
-    decision_context = data.get('decision_context', None)
+    decision_contexts = data.get('decision_context', [])
+    if not isinstance(decision_contexts, list):
+        # this will be a single context as a string
+        decision_contexts = [decision_contexts]
     rules = data.get('rules', [])
-    if decision_context and rules:
+    if decision_contexts and rules:
         raise BadRequest('Cannot have both decision_context and rules')
 
     on_demand_policies = []
@@ -213,7 +221,7 @@ def make_decision(data, config):
         **retriever_args)
 
     policies = on_demand_policies or config['policies']
-    decision = Decision(decision_context, product_version, verbose)
+    decision = Decision(decision_contexts, product_version, verbose)
     for subject in _decision_subjects_for_request(data):
         decision.check(subject, policies, results_retriever)
 

greenwave/policies.py

@@ -826,8 +826,11 @@ def matches(self, **attributes):
 
         There must be at least one matching rule or no rules in the policy.
         """
-        decision_context = attributes.get('decision_context')
-        if decision_context and (decision_context not in self.all_decision_contexts):
+        decision_contexts = attributes.get('decision_context')
+        if decision_contexts and not isinstance(decision_contexts, list):
+            decision_contexts = [decision_contexts]
+        if decision_contexts and not any(context in self.all_decision_contexts
+                                         for context in decision_contexts):
             return False
 
         product_version = attributes.get('product_version')

greenwave/tests/test_api_v1.py

@@ -194,8 +194,8 @@ def test_make_decision_with_no_tests_required_and_empty_remote_rules(mock_result
             response = make_decision(policies=policies)
             assert 404 == response.status_code
             print(response.json)
-            assert 'Cannot find any applicable policies for koji_build subjects at gating point ' \
-                   'test_policies in fedora-rawhide' == response.json['message']
+            assert 'Found no applicable policies for koji_build subjects at gating ' \
+                   'point(s) test_policies in fedora-rawhide' == response.json['message']
             mock_waivers.assert_not_called()
 
 
@@ -223,6 +223,44 @@ def test_make_decision_with_missing_required_gating_yaml(mock_results, mock_waiv
             mock_waivers.assert_called_once()
 
 
+def test_make_decision_multiple_contexts(mock_results, mock_waivers):
+    mock_results.return_value = [make_result(outcome='FAILED')]
+    mock_waivers.return_value = []
+    policies = """
+        --- !Policy
+        id: "test_policy"
+        product_versions:
+          - fedora-rawhide
+        decision_context: test_policies
+        subject_type: koji_build
+        rules:
+          - !PassingTestCaseRule {test_case_name: sometest}
+
+        --- !Policy
+        id: "test_policy_2"
+        product_versions:
+          - fedora-rawhide
+        decision_context: test_2
+        subject_type: koji_build
+        rules:
+          - !PassingTestCaseRule {test_case_name: sometest_2}
+
+        --- !Policy
+        id: "test_policy_3"
+        product_versions:
+          - fedora-rawhide
+        decision_context: test_3
+        subject_type: koji_build
+        rules:
+          - !PassingTestCaseRule {test_case_name: sometest_3}
+    """
+    response = make_decision(policies=policies, decision_context=["test_policies", "test_2"])
+    assert 200 == response.status_code
+    assert '2 of 2 required tests failed' == response.json['summary']
+    assert ['test_policy', 'test_policy_2'] == response.json['applicable_policies']
+    mock_waivers.assert_called_once()
+
+
 def test_subject_types(client):
     response = client.get('/api/v1.0/subject_types')
     assert response.status_code == 200

greenwave/tests/test_policies.py

@@ -1027,6 +1027,35 @@ def test_policy_all_decision_contexts(tmpdir):
     assert policy.all_decision_contexts == ['test4']
 
 
+def test_decision_multiple_contexts(tmpdir):
+    p = tmpdir.join('fedora.yaml')
+    p.write(dedent("""
+        --- !Policy
+        id: "some_policy"
+        product_versions:
+          - rhel-9000
+        decision_context: bodhi_update_push_stable
+        subject_type: kind-of-magic
+        rules:
+          - !PassingTestCaseRule {test_case_name: sometest}
+
+        --- !Policy
+        id: "some_other_policy"
+        product_versions:
+          - rhel-9000
+        decision_context: some_other_context
+        subject_type: kind-of-magic
+        rules:
+          - !PassingTestCaseRule {test_case_name: someothertest}
+        """))
+    policies = load_policies(tmpdir.strpath)
+    subject = create_subject('kind-of-magic', 'nethack-1.2.3-1.el9000')
+    results = DummyResultsRetriever(subject, 'sometest', 'PASSED')
+    decision = Decision(['bodhi_update_push_stable', 'some_other_context'], 'rhel-9000')
+    decision.check(subject, policies, results)
+    assert answer_types(decision.answers) == ['test-result-passed', 'test-result-missing']
+
+
 @pytest.mark.parametrize(('package', 'expected_answers'), [
     ('nethack', ['test-result-passed']),
     ('net*', ['test-result-passed']),