Blob Blame History Raw
From c3b3415feac73906887405419d1435a0a13be935 Mon Sep 17 00:00:00 2001
From: Ralph Bean <rbean@redhat.com>
Date: Mon, 21 Aug 2017 21:08:21 -0400
Subject: [PATCH 2/2] Special relevance.

I'm not totally happy with this approach.  First, I'll describe the bug,
then the solution, then what I'm not happy with.

First, the bug is that when we changed the format of the `subject` to a
list, we didn't think about how policies would map to subjects of
different types in a single query.  Bodhi would like to make a single
query with all the artifacts it care about, and have greenwave do the
heavy lifting.  It will provide a query with a ``koji_build``, and a
``bodhi_update`` alias, and a new ``original_spec_nvr`` nvr from the
atomic ci pipeline.  The problem is that all of our policies apply
indiscriminately to all of those subjects - which can't make sense.
There is no `rpmlint` run on an "update".

The approach taken here is to introduce new attributes to our policies
that let them define what subjects they are relevant to.  They can
specify a relevance_key or a relevance_value which makes them only
narrowly applicable to certain subjects.

I don't like that we have to have relevance_key *and* relevance_value
(in order to handle the strange looking ``original_spec_nvr``).  I also
don't like that "is relevance to" and "is applicable to" mean almost
exactly the same thing in english, but here in greenwave they are
defined at different scopes of the query.  "applicable" applies to the
query as a whole while "relevant" applies to each item in the query's
subject.

Perhaps this can be rethought and thrown out.. but I wanted to post
something to have something to argue about.

I'll also apply this to the greenwave "stg" instance so we can poke at
it there.

Lastly, this is on top of the `disjunction`, branch from #62.
---
 conf/policies/fedora.yaml                    |  6 +-
 functional-tests/consumers/test_resultsdb.py |  2 +-
 functional-tests/consumers/test_waiverdb.py  |  2 +-
 functional-tests/test_api_v1.py              | 84 +++++++++++++++++++++++++++-
 greenwave/api_v1.py                          |  4 +-
 greenwave/policies.py                        | 22 +++++++-
 6 files changed, 112 insertions(+), 8 deletions(-)

diff --git a/conf/policies/fedora.yaml b/conf/policies/fedora.yaml
index acda872..cb3b1f6 100644
--- a/conf/policies/fedora.yaml
+++ b/conf/policies/fedora.yaml
@@ -9,6 +9,8 @@ id: "taskotron_release_critical_tasks"
 product_versions:
   - fedora-26
 decision_context: bodhi_update_push_stable
+# This policy is applicable to fedora-26, but is only relevant to koji_builds
+relevance_value: koji_build
 rules:
   - !PassingTestCaseRule {test_case_name: dist.abicheck}
   - !PassingTestCaseRule {test_case_name: dist.rpmdeplint}
@@ -20,8 +22,10 @@ id: "atomic_ci_pipeline_results"
 product_versions:
   - fedora-26
 decision_context: bodhi_update_push_stable
+# This policy is applicable to fedora-26, but is only relevant to original_spec_nvrs.
+relevance_key: original_spec_nvr
 rules:
   - !FedoraAtomicCi {
     test_case_name: org.centos.prod.ci.pipeline.complete,
-    repos: ['kernel', 'rpm-ostree'],
+    repos: ['kernel', 'rpm-ostree', 'glibc'],
     }
diff --git a/greenwave/api_v1.py b/greenwave/api_v1.py
index 912fd1a..fe3e127 100644
--- a/greenwave/api_v1.py
+++ b/greenwave/api_v1.py
@@ -207,7 +207,9 @@ def make_decision():
         else:
             waivers = []
         waivers = [w for w in waivers if w['id'] not in ignore_waivers]
-        for policy in applicable_policies:
+        relevant_policies = [policy for policy in applicable_policies
+                             if policy.is_relevant_to(item)]
+        for policy in relevant_policies:
             answers.extend(policy.check(item, results, waivers))
     res = {
         'policies_satisified': all(answer.is_satisfied for answer in answers),
diff --git a/greenwave/policies.py b/greenwave/policies.py
index 2d01d08..8cb5e40 100644
--- a/greenwave/policies.py
+++ b/greenwave/policies.py
@@ -227,13 +227,27 @@ def applies_to(self, decision_context, product_version):
         return (decision_context == self.decision_context and
                 product_version in self.product_versions)
 
+    def is_relevant_to(self, item):
+        relevance_key = getattr(self, 'relevance_key', None)
+        relevance_value = getattr(self, 'relevance_value', None)
+        if relevance_key and relevance_value:
+            return item.get(relevance_key) == relevance_value
+        if relevance_key:
+            return relevance_key in item
+        if relevance_value:
+            return relevance_value in item.values()
+        return True
+
     def check(self, item, results, waivers):
         return [rule.check(item, results, waivers) for rule in self.rules]
 
     def __repr__(self):
-        return "%s(id=%r, product_versions=%r, decision_context=%r, rules=%r)" % (
-            self.__class__.__name__, self.id, self.product_versions, self.decision_context,
-            self.rules)
+        return ("%s(id=%r, product_versions=%r, decision_context=%r, rules=%r,"
+                " relevance_key=%r, relevance_value=%r)" % (
+                    self.__class__.__name__, self.id, self.product_versions,
+                    self.decision_context, self.rules,
+                    getattr(self, 'relevance_key', None),
+                    getattr(self, 'relevance_value', None)))
 
     def to_json(self):
         return {
@@ -241,4 +255,6 @@ def to_json(self):
             'product_versions': self.product_versions,
             'decision_context': self.decision_context,
             'rules': [rule.to_json() for rule in self.rules],
+            'relevance_key': getattr(self, 'relevance_key', None),
+            'relevance_value': getattr(self, 'relevance_value', None),
         }
-- 
2.9.4