From 45c2de482c7f088a8ed42a7c471345077c5993ff Mon Sep 17 00:00:00 2001
From: Adam Williamson <awilliam@redhat.com>
Date: Dec 06 2018 20:30:21 +0000
Subject: Don't try and make a decision for pipeline msgs with empty NVR


While working on another PR, I noticed that the pipeline reports
a lot of results for testcase
'ci.pipeline.allpackages-build.package.ignored'. In all these
results, the 'original_spec_nvr' key is present, but its value
is an empty string. It looks to me like this code, when given
one of these messages, will try to publish a decision for a
package with the NVR "". I'm not sure how things go from there,
but I'm guessing either something later just bails on an empty
subject_identifier (best case) or it actually tries and comes
up with a nonsensical decision (medium case) or it causes some
kind of error or crash (worst case). Whichever of those it is,
it seems sensible to just avoid this by *not* yielding a subject
unless we actually have some content in the nvr field.

Here's an example of the fedmsg for one of these results:
https://apps.fedoraproject.org/datagrepper/id?id=2018-7d336210-10f9-4afa-88a2-2dcb8f32fb11&is_raw=true&size=extra-large

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

---

diff --git a/greenwave/consumers/resultsdb.py b/greenwave/consumers/resultsdb.py
index 07ad90a..0d397c9 100644
--- a/greenwave/consumers/resultsdb.py
+++ b/greenwave/consumers/resultsdb.py
@@ -181,7 +181,11 @@ class ResultsDBHandler(fedmsg.consumers.FedmsgConsumer):
                 nvr = _decode(data['item'])
             else:
                 nvr = _decode(data['original_spec_nvr'])
-            yield ('koji_build', nvr)
+            # when the pipeline ignores a package, which happens
+            # *a lot*, we get a message with an 'original_spec_nvr'
+            # key with an empty value; let's not try and handle this
+            if nvr:
+                yield ('koji_build', nvr)
 
     def consume(self, message):
         """
diff --git a/greenwave/tests/test_resultsdb_consumer.py b/greenwave/tests/test_resultsdb_consumer.py
index f5daaf6..a6038c9 100644
--- a/greenwave/tests/test_resultsdb_consumer.py
+++ b/greenwave/tests/test_resultsdb_consumer.py
@@ -19,6 +19,22 @@ def test_announcement_keys_decode_with_list():
     assert subjects == [('koji_build', 'glibc-1.0-1.fc27')]
 
 
+def test_no_announcement_subjects_for_empty_nvr():
+    """The CI pipeline submits a lot of results for the test
+    'org.centos.prod.ci.pipeline.allpackages-build.package.ignored'
+    with the 'original_spec_nvr' key present, but the value just an
+    empty string. To avoid unpredictable consequences, we should not
+    return any announcement subjects for such a message.
+    """
+    cls = greenwave.consumers.resultsdb.ResultsDBHandler
+    message = {'msg': {'data': {
+        'original_spec_nvr': [""],
+    }}}
+    subjects = list(cls.announcement_subjects(message))
+
+    assert subjects == []
+
+
 def test_announcement_subjects_for_brew_build():
     # The 'brew-build' type appears internally within Red Hat. We treat it as an
     # alias of 'koji_build'.