From 203c0b76e0752bd44ee87961e1cdbcbd436c4e14 Mon Sep 17 00:00:00 2001 From: Kenny Lee Sin Cheong Date: Thu, 18 May 2017 23:19:30 -0400 Subject: [PATCH 1/4] Raise an APIRequestFailure exception when security scanner is unavailable Put worker to sleep for the duration of the default indexing interval when an APIRequestFailure occurs, when the API request fails due to a connection error, timeout, or other ambiguous errors, from analyze_layer or get_layer_data . --- test/test_secscan.py | 5 +++-- util/secscan/analyzer.py | 4 ++-- workers/securityworker.py | 4 +++- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/test/test_secscan.py b/test/test_secscan.py index b5da8e2b6..db466689a 100644 --- a/test/test_secscan.py +++ b/test/test_secscan.py @@ -8,7 +8,7 @@ from data.database import Image, IMAGE_NOT_SCANNED_ENGINE_VERSION from endpoints.notificationevent import VulnerabilityFoundEvent from endpoints.v2 import v2_bp from initdb import setup_database_for_testing, finished_database_for_testing -from util.secscan.api import SecurityScannerAPI +from util.secscan.api import SecurityScannerAPI, APIRequestFailure from util.secscan.analyzer import LayerAnalyzer from util.secscan.fake import fake_security_scanner from util.secscan.notifier import SecurityNotificationHandler, ProcessNotificationPageResult @@ -160,7 +160,8 @@ class TestSecurityScanner(unittest.TestCase): security_scanner.set_internal_error_layer_id(security_scanner.layer_id(layer)) analyzer = LayerAnalyzer(app.config, self.api) - analyzer.analyze_recursively(layer) + with self.assertRaises(APIRequestFailure) as ctx: + analyzer.analyze_recursively(layer) layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest') self.assertAnalyzed(layer, security_scanner, False, -1) diff --git a/util/secscan/analyzer.py b/util/secscan/analyzer.py index 3f0309a1e..cdfe99051 100644 --- a/util/secscan/analyzer.py +++ b/util/secscan/analyzer.py @@ -57,7 +57,7 @@ class LayerAnalyzer(object): except AnalyzeLayerRetryException: # Something went wrong when trying to analyze the layer, but we should retry, so leave # the layer unindexed. Another worker will come along and handle it. - pass + raise APIRequestFailure except MissingParentLayerException: # Pass upward, as missing parent is handled in the analyze_recursively method. raise @@ -145,7 +145,7 @@ class LayerAnalyzer(object): try: layer_data = self._api.get_layer_data(layer, include_vulnerabilities=True) except APIRequestFailure: - layer_data = None + raise if layer_data is not None: # Dispatch events for any detected vulnerabilities diff --git a/workers/securityworker.py b/workers/securityworker.py index f1907efd8..0eb6bbe4f 100644 --- a/workers/securityworker.py +++ b/workers/securityworker.py @@ -10,7 +10,7 @@ from workers.worker import Worker from data.database import UseThenDisconnect from data.model.image import (get_images_eligible_for_scan, get_image_pk_field, get_max_id_for_sec_scan, get_min_id_for_sec_scan) -from util.secscan.api import SecurityConfigValidator +from util.secscan.api import SecurityConfigValidator, APIRequestFailure from util.secscan.analyzer import LayerAnalyzer, PreemptedException from util.migrate.allocator import yield_random_entries from endpoints.v2 import v2_bp @@ -73,6 +73,8 @@ class SecurityWorker(Worker): except PreemptedException: logger.info('Another worker pre-empted us for layer: %s', candidate.id) abt.set() + except APIRequestFailure: + time.sleep(DEFAULT_INDEXING_INTERVAL) unscanned_images_gauge.Set(num_remaining) From b5f8e7e24d4169aa89665cf3df724c8fa3773221 Mon Sep 17 00:00:00 2001 From: Kenny Lee Sin Cheong Date: Fri, 2 Jun 2017 12:28:17 -0400 Subject: [PATCH 2/4] Returning from the method instead of calling sleep Simply returning from the method will give DEFAULT_INDEXING_INTERVAL seconds before the next scan operation. --- workers/securityworker.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/workers/securityworker.py b/workers/securityworker.py index 0eb6bbe4f..662f19c5c 100644 --- a/workers/securityworker.py +++ b/workers/securityworker.py @@ -74,7 +74,7 @@ class SecurityWorker(Worker): logger.info('Another worker pre-empted us for layer: %s', candidate.id) abt.set() except APIRequestFailure: - time.sleep(DEFAULT_INDEXING_INTERVAL) + return unscanned_images_gauge.Set(num_remaining) From 3302a96f882e11ef02e024428a29ebead9a1aae5 Mon Sep 17 00:00:00 2001 From: Kenny Lee Sin Cheong Date: Fri, 2 Jun 2017 14:40:15 -0400 Subject: [PATCH 3/4] Log the APIRequestFailure at ERROR level --- workers/securityworker.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/workers/securityworker.py b/workers/securityworker.py index 662f19c5c..a2eeda623 100644 --- a/workers/securityworker.py +++ b/workers/securityworker.py @@ -73,7 +73,8 @@ class SecurityWorker(Worker): except PreemptedException: logger.info('Another worker pre-empted us for layer: %s', candidate.id) abt.set() - except APIRequestFailure: + except APIRequestFailure as arf: + logger.exception('Security scanner service unavailable: %s', arf) return unscanned_images_gauge.Set(num_remaining) From ad1a0e08400a6c01b165a2c2a712d2988d98a1a4 Mon Sep 17 00:00:00 2001 From: Kenny Lee Sin Cheong Date: Fri, 2 Jun 2017 17:20:41 -0400 Subject: [PATCH 4/4] logger.exception dumps a stack trace by default --- workers/securityworker.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/workers/securityworker.py b/workers/securityworker.py index a2eeda623..d8b200178 100644 --- a/workers/securityworker.py +++ b/workers/securityworker.py @@ -73,8 +73,8 @@ class SecurityWorker(Worker): except PreemptedException: logger.info('Another worker pre-empted us for layer: %s', candidate.id) abt.set() - except APIRequestFailure as arf: - logger.exception('Security scanner service unavailable: %s', arf) + except APIRequestFailure: + logger.exception('Security scanner service unavailable') return unscanned_images_gauge.Set(num_remaining)