diff --git a/test/test_secscan.py b/test/test_secscan.py index 737b6ba9e..91d2616ca 100644 --- a/test/test_secscan.py +++ b/test/test_secscan.py @@ -10,7 +10,7 @@ from util.secscan.api import SecurityScannerAPI, AnalyzeLayerException from util.secscan.analyzer import LayerAnalyzer from util.secscan.notifier import process_notification_data from data import model -from data.database import Image +from data.database import Image, IMAGE_NOT_SCANNED_ENGINE_VERSION from workers.security_notification_worker import SecurityNotificationWorker from endpoints.v2 import v2_bp @@ -271,8 +271,7 @@ class TestSecurityScanner(unittest.TestCase): self.assertEquals(False, layer.security_indexed) self.assertEquals(1, layer.security_indexed_engine) - - def test_analyze_layer_success_events(self): + def assert_analyze_layer_notify(self, security_indexed_engine, security_indexed, expect_notification): 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) @@ -284,47 +283,11 @@ class TestSecurityScanner(unittest.TestCase): 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 an event was written for the tag. - time.sleep(1) - queue_item = notification_queue.get() - self.assertIsNotNone(queue_item) - - body = json.loads(queue_item.body) - self.assertEquals(set(['latest', 'prod']), set(body['event_data']['tags'])) - self.assertEquals('CVE-2014-9471', body['event_data']['vulnerability']['id']) - 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 + # Update the layer's state before analyzing. + layer.security_indexed_engine = security_indexed_engine + layer.security_indexed = security_indexed 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) @@ -332,15 +295,37 @@ class TestSecurityScanner(unittest.TestCase): 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. + # Ensure an event was written for the tag (if necessary). time.sleep(1) - self.assertIsNone(notification_queue.get()) + queue_item = notification_queue.get() + + if expect_notification: + self.assertIsNotNone(queue_item) + + body = json.loads(queue_item.body) + self.assertEquals(set(['latest', 'prod']), set(body['event_data']['tags'])) + self.assertEquals('CVE-2014-9471', body['event_data']['vulnerability']['id']) + self.assertEquals('Low', body['event_data']['vulnerability']['priority']) + self.assertTrue(body['event_data']['vulnerability']['has_fix']) + else: + self.assertIsNone(queue_item) # 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_events(self): + # Not previously indexed at all => Notification + self.assert_analyze_layer_notify(IMAGE_NOT_SCANNED_ENGINE_VERSION, False, True) + + def test_analyze_layer_success_no_notification(self): + # Previously successfully indexed => No notification + self.assert_analyze_layer_notify(0, True, False) + + def test_analyze_layer_failed_then_success_notification(self): + # Previously failed to index => Notification + self.assert_analyze_layer_notify(0, False, True) def _get_notification_data(self, new_layer_ids, old_layer_ids, new_severity='Low'): return { diff --git a/util/secscan/analyzer.py b/util/secscan/analyzer.py index f2d78336a..fdbba29be 100644 --- a/util/secscan/analyzer.py +++ b/util/secscan/analyzer.py @@ -68,8 +68,10 @@ class LayerAnalyzer(object): return True, set_secscan_status(layer, False, self._target_version) # Analyze the image. - logger.info('Analyzing layer %s', layer.docker_image_id) + previously_security_indexed_successfully = layer.security_indexed previous_security_indexed_engine = layer.security_indexed_engine + + logger.info('Analyzing layer %s', layer.docker_image_id) (analyzed_version, should_requeue) = self._api.analyze_layer(layer) # If analysis failed, then determine whether we need to requeue. @@ -89,13 +91,17 @@ 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, 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 we are the one who've done the job successfully first, then we need to decide if we should + # send notifications. Notifications are sent if: + # 1) This is a new layer + # 2) This is an existing layer that previously did not index properly + # We don't always send notifications as if we are re-indexing a successful layer for a newer + # feature set in the security scanner, notifications will be spammy. + is_new_image = previous_security_indexed_engine == IMAGE_NOT_SCANNED_ENGINE_VERSION + is_existing_image_unindexed = not is_new_image and not previously_security_indexed_successfully if (features.SECURITY_NOTIFICATIONS and set_status and - previous_security_indexed_engine == IMAGE_NOT_SCANNED_ENGINE_VERSION): + (is_new_image or is_existing_image_unindexed)): + # Get the tags of the layer we analyzed. repository_map = defaultdict(list) event = ExternalNotificationEvent.get(name='vulnerability_found')