Have security scanner analyze only send notifications for *new* layers

Following this change, anytime a layer is indexed by the security scanner, we only send notifications out if the layer previously had a security_indexed_engine value of `-1`, thus ensuring it has *never* been indexed previously. This will allow us to change to version of the security scanner upwards, and have all the images be re-indexed, without firing off notifications in a spammy manner.

data/database.py

@@ -30,6 +30,10 @@ logger = logging.getLogger(__name__)
 
 DEFAULT_DB_CONNECT_TIMEOUT = 10 # seconds
 
+# IMAGE_NOT_SCANNED_ENGINE_VERSION is the version found in security_indexed_engine when the
+# image has not yet been scanned.
+IMAGE_NOT_SCANNED_ENGINE_VERSION = -1
+
 _SCHEME_DRIVERS = {
   'mysql': MySQLDatabase,
   'mysql+pymysql': MySQLDatabase,
@@ -665,7 +669,7 @@ class Image(BaseModel):
   v1_checksum = CharField(null=True)
 
   security_indexed = BooleanField(default=False, index=True)
-  security_indexed_engine = IntegerField(default=-1, index=True)
+  security_indexed_engine = IntegerField(default=IMAGE_NOT_SCANNED_ENGINE_VERSION, index=True)
 
   # We use a proxy here instead of 'self' in order to disable the foreign key constraint
   parent = ForeignKeyField(_ImageProxy, null=True, related_name='children')

test/test_secscan.py

@@ -301,6 +301,45 @@ class TestSecurityScanner(unittest.TestCase):
     self.assertEquals('Low', body['event_data']['vulnerability']['priority'])
     self.assertTrue(body['event_data']['vulnerability']['has_fix'])
 
+    # Ensure its security indexed engine was updated.
+    updated_layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest')
+    self.assertEquals(updated_layer.id, layer.id)
+    self.assertTrue(updated_layer.security_indexed_engine > 0)
+
+
+  def test_analyze_layer_success_no_notification(self):
+    layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest', include_storage=True)
+    self.assertFalse(layer.security_indexed)
+    self.assertEquals(-1, layer.security_indexed_engine)
+
+    # Ensure there are no existing events.
+    self.assertIsNone(notification_queue.get())
+
+    # Set the security_indexed_engine of the layer to 0 to ensure it is marked as having been
+    # indexed (in some form) before this call.
+    layer.security_indexed_engine = 0
+    layer.save()
+
+    # Add a repo event for the layer.
+    repo = model.repository.get_repository(ADMIN_ACCESS_USER, SIMPLE_REPO)
+    model.notification.create_repo_notification(repo, 'vulnerability_found', 'quay_notification', {}, {'level': 100})
+
+    with HTTMock(analyze_layer_success_mock, get_layer_success_mock, response_content):
+      analyzer = LayerAnalyzer(app.config, self.api)
+      analyzer.analyze_recursively(layer)
+
+      layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest')
+      self.assertAnalyzed(layer, True, 1)
+
+    # Ensure no event was written for the tag, as the layer was being re-indexed.
+    time.sleep(1)
+    self.assertIsNone(notification_queue.get())
+
+    # Ensure its security indexed engine was updated.
+    updated_layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest')
+    self.assertEquals(updated_layer.id, layer.id)
+    self.assertTrue(updated_layer.security_indexed_engine > 0)
+
 
   def _get_notification_data(self, new_layer_ids, old_layer_ids, new_severity='Low'):
     return {

util/secscan/analyzer.py

@@ -6,7 +6,7 @@ import features
 from collections import defaultdict
 
 from endpoints.notificationhelper import spawn_notification
-from data.database import Image, ExternalNotificationEvent
+from data.database import Image, ExternalNotificationEvent, IMAGE_NOT_SCANNED_ENGINE_VERSION
 from data.model.tag import filter_tags_have_repository_event, get_tags_for_image
 from data.model.image import set_secscan_status, get_image_with_storage_and_parent_base
 from util.secscan.api import APIRequestFailure
@@ -69,6 +69,7 @@ class LayerAnalyzer(object):
 
     # Analyze the image.
     logger.info('Analyzing layer %s', layer.docker_image_id)
+    previous_security_indexed_engine = layer.security_indexed_engine
     (analyzed_version, should_requeue) = self._api.analyze_layer(layer)
 
     # If analysis failed, then determine whether we need to requeue.
@@ -88,9 +89,13 @@ class LayerAnalyzer(object):
                 analyzed_version)
     set_status = set_secscan_status(layer, True, analyzed_version)
 
-    # If we are the one who've done the job successfully first, get the vulnerabilities and
-    # send notifications to the repos that have a tag on that layer.
-    if features.SECURITY_NOTIFICATIONS and set_status:
+    # If we are the one who've done the job successfully first, and this is a *new* layer,
+    # as indicated by having a version of -1, get the vulnerabilities and
+    # send notifications to the repos that have a tag on that layer. We don't always send
+    # notifications as if we are re-indexing a layer for a newer feature set in the security
+    # scanner, notifications will be spammy.
+    if (features.SECURITY_NOTIFICATIONS and set_status and
+        previous_security_indexed_engine == IMAGE_NOT_SCANNED_ENGINE_VERSION):
       # Get the tags of the layer we analyzed.
       repository_map = defaultdict(list)
       event = ExternalNotificationEvent.get(name='vulnerability_found')