Merge pull request #2249 from coreos-inc/notifier-fixes

Security notification pagination fix
This commit is contained in:
josephschorr 2017-01-17 11:33:25 -05:00 committed by GitHub
commit eb2cafacd4
6 changed files with 810 additions and 113 deletions

View file

@ -0,0 +1,310 @@
import unittest
from util.morecollections import (FastIndexList, StreamingDiffTracker,
IndexedStreamingDiffTracker)
class FastIndexListTests(unittest.TestCase):
def test_basic_usage(self):
indexlist = FastIndexList()
# Add 1
indexlist.add(1)
self.assertEquals([1], indexlist.values())
self.assertEquals(0, indexlist.index(1))
# Add 2
indexlist.add(2)
self.assertEquals([1, 2], indexlist.values())
self.assertEquals(0, indexlist.index(1))
self.assertEquals(1, indexlist.index(2))
# Pop nothing.
indexlist.pop_until(-1)
self.assertEquals([1, 2], indexlist.values())
self.assertEquals(0, indexlist.index(1))
self.assertEquals(1, indexlist.index(2))
# Pop 1.
self.assertEquals([1], indexlist.pop_until(0))
self.assertEquals([2], indexlist.values())
self.assertIsNone(indexlist.index(1))
self.assertEquals(0, indexlist.index(2))
# Add 3.
indexlist.add(3)
self.assertEquals([2, 3], indexlist.values())
self.assertEquals(0, indexlist.index(2))
self.assertEquals(1, indexlist.index(3))
# Pop 2, 3.
self.assertEquals([2, 3], indexlist.pop_until(1))
self.assertEquals([], indexlist.values())
self.assertIsNone(indexlist.index(1))
self.assertIsNone(indexlist.index(2))
self.assertIsNone(indexlist.index(3))
def test_popping(self):
indexlist = FastIndexList()
indexlist.add('hello')
indexlist.add('world')
indexlist.add('you')
indexlist.add('rock')
self.assertEquals(0, indexlist.index('hello'))
self.assertEquals(1, indexlist.index('world'))
self.assertEquals(2, indexlist.index('you'))
self.assertEquals(3, indexlist.index('rock'))
indexlist.pop_until(1)
self.assertEquals(0, indexlist.index('you'))
self.assertEquals(1, indexlist.index('rock'))
class IndexedStreamingDiffTrackerTests(unittest.TestCase):
def test_basic(self):
added = []
tracker = IndexedStreamingDiffTracker(added.append, 3)
tracker.push_new([('a', 0), ('b', 1), ('c', 2)])
tracker.push_old([('b', 1)])
tracker.done()
self.assertEquals(['a', 'c'], added)
def test_multiple_done(self):
added = []
tracker = IndexedStreamingDiffTracker(added.append, 3)
tracker.push_new([('a', 0), ('b', 1), ('c', 2)])
tracker.push_old([('b', 1)])
tracker.done()
tracker.done()
self.assertEquals(['a', 'c'], added)
def test_same_streams(self):
added = []
tracker = IndexedStreamingDiffTracker(added.append, 3)
tracker.push_new([('a', 0), ('b', 1), ('c', 2)])
tracker.push_old([('a', 0), ('b', 1), ('c', 2)])
tracker.done()
self.assertEquals([], added)
def test_only_new(self):
added = []
tracker = IndexedStreamingDiffTracker(added.append, 3)
tracker.push_new([('a', 0), ('b', 1), ('c', 2)])
tracker.push_old([])
tracker.done()
self.assertEquals(['a', 'b', 'c'], added)
def test_pagination(self):
added = []
tracker = IndexedStreamingDiffTracker(added.append, 2)
tracker.push_new([('a', 0), ('b', 1)])
tracker.push_old([])
tracker.push_new([('c', 2)])
tracker.push_old([])
tracker.done()
self.assertEquals(['a', 'b', 'c'], added)
def test_old_pagination_no_repeat(self):
added = []
tracker = IndexedStreamingDiffTracker(added.append, 2)
tracker.push_new([('new1', 3), ('new2', 4)])
tracker.push_old([('old1', 1), ('old2', 2)])
tracker.push_new([])
tracker.push_old([('new1', 3)])
tracker.done()
self.assertEquals(['new2'], added)
def test_old_pagination(self):
added = []
tracker = IndexedStreamingDiffTracker(added.append, 2)
tracker.push_new([('a', 10), ('b', 11)])
tracker.push_old([('z', 1), ('y', 2)])
tracker.push_new([('c', 12)])
tracker.push_old([('a', 10)])
tracker.done()
self.assertEquals(['b', 'c'], added)
def test_very_offset(self):
added = []
tracker = IndexedStreamingDiffTracker(added.append, 2)
tracker.push_new([('a', 10), ('b', 11)])
tracker.push_old([('z', 1), ('y', 2)])
tracker.push_new([('c', 12), ('d', 13)])
tracker.push_old([('x', 3), ('w', 4)])
tracker.push_new([('e', 14)])
tracker.push_old([('a', 10), ('d', 13)])
tracker.done()
self.assertEquals(['b', 'c', 'e'], added)
def test_many_old(self):
added = []
tracker = IndexedStreamingDiffTracker(added.append, 2)
tracker.push_new([('z', 26), ('hello', 100)])
tracker.push_old([('a', 1), ('b', 2)])
tracker.push_new([])
tracker.push_old([('c', 1), ('d', 2)])
tracker.push_new([])
tracker.push_old([('e', 3), ('f', 4)])
tracker.push_new([])
tracker.push_old([('g', 5), ('z', 26)])
tracker.done()
self.assertEquals(['hello'], added)
def test_high_old_bound(self):
added = []
tracker = IndexedStreamingDiffTracker(added.append, 2)
tracker.push_new([('z', 26), ('hello', 100)])
tracker.push_old([('end1', 999), ('end2', 1000)])
tracker.push_new([])
tracker.push_old([])
tracker.done()
self.assertEquals(['z', 'hello'], added)
class StreamingDiffTrackerTests(unittest.TestCase):
def test_basic(self):
added = []
tracker = StreamingDiffTracker(added.append, 3)
tracker.push_new(['a', 'b', 'c'])
tracker.push_old(['b'])
tracker.done()
self.assertEquals(['a', 'c'], added)
def test_same_streams(self):
added = []
tracker = StreamingDiffTracker(added.append, 3)
tracker.push_new(['a', 'b', 'c'])
tracker.push_old(['a', 'b', 'c'])
tracker.done()
self.assertEquals([], added)
def test_some_new(self):
added = []
tracker = StreamingDiffTracker(added.append, 5)
tracker.push_new(['a', 'b', 'c', 'd', 'e'])
tracker.push_old(['a', 'b', 'c'])
tracker.done()
self.assertEquals(['d', 'e'], added)
def test_offset_new(self):
added = []
tracker = StreamingDiffTracker(added.append, 5)
tracker.push_new(['b', 'c', 'd', 'e'])
tracker.push_old(['a', 'b', 'c'])
tracker.done()
self.assertEquals(['d', 'e'], added)
def test_multiple_calls(self):
added = []
tracker = StreamingDiffTracker(added.append, 3)
tracker.push_new(['a', 'b', 'c'])
tracker.push_old(['b', 'd', 'e'])
tracker.push_new(['f', 'g', 'h'])
tracker.push_old(['g', 'h'])
tracker.done()
self.assertEquals(['a', 'c', 'f'], added)
def test_empty_old(self):
added = []
tracker = StreamingDiffTracker(added.append, 3)
tracker.push_new(['a', 'b', 'c'])
tracker.push_old([])
tracker.push_new(['f', 'g', 'h'])
tracker.push_old([])
tracker.done()
self.assertEquals(['a', 'b', 'c', 'f', 'g', 'h'], added)
def test_more_old(self):
added = []
tracker = StreamingDiffTracker(added.append, 2)
tracker.push_new(['c', 'd'])
tracker.push_old(['a', 'b'])
tracker.push_new([])
tracker.push_old(['c'])
tracker.done()
self.assertEquals(['d'], added)
def test_more_new(self):
added = []
tracker = StreamingDiffTracker(added.append, 4)
tracker.push_new(['a', 'b', 'c', 'd'])
tracker.push_old(['r'])
tracker.push_new(['e', 'f', 'r', 'z'])
tracker.push_old([])
tracker.done()
self.assertEquals(['a', 'b', 'c', 'd', 'e', 'f', 'z'], added)
def test_more_new2(self):
added = []
tracker = StreamingDiffTracker(added.append, 4)
tracker.push_new(['a', 'b', 'c', 'd'])
tracker.push_old(['r'])
tracker.push_new(['e', 'f', 'g', 'h'])
tracker.push_old([])
tracker.push_new(['i', 'j', 'r', 'z'])
tracker.push_old([])
tracker.done()
self.assertEquals(['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'z'], added)
if __name__ == '__main__':
unittest.main()

View file

@ -11,7 +11,7 @@ from initdb import setup_database_for_testing, finished_database_for_testing
from util.secscan.api import SecurityScannerAPI
from util.secscan.analyzer import LayerAnalyzer
from util.secscan.fake import fake_security_scanner
from util.secscan.notifier import process_notification_data
from util.secscan.notifier import SecurityNotificationHandler, ProcessNotificationPageResult
from workers.security_notification_worker import SecurityNotificationWorker
@ -20,6 +20,13 @@ SIMPLE_REPO = 'simple'
COMPLEX_REPO = 'complex'
def process_notification_data(notification_data):
handler = SecurityNotificationHandler(100)
result = handler.process_notification_page_data(notification_data)
handler.send_notifications()
return result == ProcessNotificationPageResult.FINISHED_PROCESSING
class TestSecurityScanner(unittest.TestCase):
def setUp(self):
# Enable direct download in fake storage.
@ -57,7 +64,6 @@ class TestSecurityScanner(unittest.TestCase):
self.assertEquals(engineVersion, parent.security_indexed_engine)
self.assertTrue(security_scanner.has_layer(security_scanner.layer_id(parent)))
def test_get_layer(self):
""" Test for basic retrieval of layers from the security scanner. """
layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest', include_storage=True)
@ -75,7 +81,6 @@ class TestSecurityScanner(unittest.TestCase):
self.assertIsNotNone(result)
self.assertEquals(result['Layer']['Name'], security_scanner.layer_id(layer))
def test_analyze_layer_nodirectdownload_success(self):
""" Tests analyzing a layer when direct download is disabled. """
@ -114,7 +119,6 @@ class TestSecurityScanner(unittest.TestCase):
layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest')
self.assertAnalyzed(layer, security_scanner, True, 1)
def test_analyze_layer_success(self):
""" Tests that analyzing a layer successfully marks it as analyzed. """
@ -129,7 +133,6 @@ class TestSecurityScanner(unittest.TestCase):
layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest')
self.assertAnalyzed(layer, security_scanner, True, 1)
def test_analyze_layer_failure(self):
""" Tests that failing to analyze a layer (because it 422s) marks it as analyzed but failed. """
@ -146,7 +149,6 @@ class TestSecurityScanner(unittest.TestCase):
layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest')
self.assertAnalyzed(layer, security_scanner, False, 1)
def test_analyze_layer_internal_error(self):
""" Tests that failing to analyze a layer (because it 500s) marks it as not analyzed. """
@ -163,7 +165,6 @@ class TestSecurityScanner(unittest.TestCase):
layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest')
self.assertAnalyzed(layer, security_scanner, False, -1)
def test_analyze_layer_error(self):
""" Tests that failing to analyze a layer (because it 400s) marks it as analyzed but failed. """
@ -183,7 +184,6 @@ class TestSecurityScanner(unittest.TestCase):
layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest')
self.assertAnalyzed(layer, security_scanner, False, 1)
def test_analyze_layer_missing_parent_handled(self):
""" Tests that a missing parent causes an automatic reanalysis, which succeeds. """
@ -214,7 +214,6 @@ class TestSecurityScanner(unittest.TestCase):
layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest')
self.assertAnalyzed(layer, security_scanner, True, 1)
def test_analyze_layer_invalid_parent(self):
""" Tests that trying to reanalyze a parent that is invalid causes the layer to be marked
as analyzed, but failed.
@ -250,7 +249,6 @@ class TestSecurityScanner(unittest.TestCase):
layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest')
self.assertAnalyzed(layer, security_scanner, False, 1)
def test_analyze_layer_unsupported_parent(self):
""" Tests that attempting to analyze a layer whose parent is unanalyzable, results in the layer
being marked as analyzed, but failed.
@ -271,7 +269,6 @@ class TestSecurityScanner(unittest.TestCase):
layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest')
self.assertAnalyzed(layer, security_scanner, False, 1)
def test_analyze_layer_missing_storage(self):
""" Tests trying to analyze a layer with missing storage. """
@ -292,7 +289,6 @@ class TestSecurityScanner(unittest.TestCase):
layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest')
self.assertAnalyzed(layer, security_scanner, False, 1)
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)
@ -350,22 +346,18 @@ class TestSecurityScanner(unittest.TestCase):
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 test_notification_new_layers_not_vulnerable(self):
layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest', include_storage=True)
layer_id = '%s.%s' % (layer.docker_image_id, layer.storage.uuid)
@ -395,7 +387,6 @@ class TestSecurityScanner(unittest.TestCase):
# Ensure that there are no event queue items for the layer.
self.assertIsNone(notification_queue.get())
def test_notification_delete(self):
layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest', include_storage=True)
layer_id = '%s.%s' % (layer.docker_image_id, layer.storage.uuid)
@ -425,7 +416,6 @@ class TestSecurityScanner(unittest.TestCase):
# Ensure that there are no event queue items for the layer.
self.assertIsNone(notification_queue.get())
def test_notification_new_layers(self):
layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest', include_storage=True)
layer_id = '%s.%s' % (layer.docker_image_id, layer.storage.uuid)
@ -452,7 +442,7 @@ class TestSecurityScanner(unittest.TestCase):
"Description": "Some service",
"Link": "https://security-tracker.debian.org/tracker/CVE-2014-9471",
"Severity": "Low",
"FixedIn": {'Version': "9.23-5"},
"FixedIn": {"Version": "9.23-5"},
}
security_scanner.set_vulns(layer_id, [vuln_info])
@ -473,7 +463,6 @@ class TestSecurityScanner(unittest.TestCase):
self.assertEquals('Low', item_body['event_data']['vulnerability']['priority'])
self.assertTrue(item_body['event_data']['vulnerability']['has_fix'])
def test_notification_no_new_layers(self):
layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest', include_storage=True)
@ -502,7 +491,6 @@ class TestSecurityScanner(unittest.TestCase):
# Ensure that there are no event queue items for the layer.
self.assertIsNone(notification_queue.get())
def test_notification_no_new_layers_increased_severity(self):
layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest', include_storage=True)
layer_id = '%s.%s' % (layer.docker_image_id, layer.storage.uuid)
@ -577,7 +565,6 @@ class TestSecurityScanner(unittest.TestCase):
{'level': 0})
self.assertFalse(VulnerabilityFoundEvent().should_perform(event_data, notification))
def test_select_images_to_scan(self):
# Set all images to have a security index of a version to that of the config.
expected_version = app.config['SECURITY_SCANNER_ENGINE_VERSION_TARGET']
@ -591,7 +578,6 @@ class TestSecurityScanner(unittest.TestCase):
self.assertIsNotNone(model.image.get_min_id_for_sec_scan(expected_version + 1))
self.assertTrue(len(model.image.get_images_eligible_for_scan(expected_version + 1)) > 0)
def test_notification_worker(self):
layer1 = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest', include_storage=True)
layer2 = model.tag.get_tag_image(ADMIN_ACCESS_USER, COMPLEX_REPO, 'prod', include_storage=True)
@ -634,7 +620,7 @@ class TestSecurityScanner(unittest.TestCase):
security_scanner.set_vulns(security_scanner.layer_id(layer2), [new_vuln_info])
layer_ids = [security_scanner.layer_id(layer1), security_scanner.layer_id(layer2)]
notification_data = security_scanner.add_notification([], layer_ids, {}, new_vuln_info)
notification_data = security_scanner.add_notification([], layer_ids, None, new_vuln_info)
# Test with a known notification with pages.
data = {
@ -642,13 +628,103 @@ class TestSecurityScanner(unittest.TestCase):
}
worker = SecurityNotificationWorker(None)
self.assertTrue(worker.perform_notification_work(data, layer_limit=1))
self.assertTrue(worker.perform_notification_work(data, layer_limit=2))
# Make sure all pages were processed by ensuring we have two notifications.
time.sleep(1)
self.assertIsNotNone(notification_queue.get())
self.assertIsNotNone(notification_queue.get())
def test_notification_worker_offset_pages_not_indexed(self):
# Try without indexes.
self.assert_notification_worker_offset_pages(indexed=False)
def test_notification_worker_offset_pages_indexed(self):
# Try with indexes.
self.assert_notification_worker_offset_pages(indexed=True)
def assert_notification_worker_offset_pages(self, indexed=False):
layer1 = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest', include_storage=True)
layer2 = model.tag.get_tag_image(ADMIN_ACCESS_USER, COMPLEX_REPO, 'prod', include_storage=True)
# Add a repo events for the layers.
simple_repo = model.repository.get_repository(ADMIN_ACCESS_USER, SIMPLE_REPO)
complex_repo = model.repository.get_repository(ADMIN_ACCESS_USER, COMPLEX_REPO)
model.notification.create_repo_notification(simple_repo, 'vulnerability_found',
'quay_notification', {}, {'level': 100})
model.notification.create_repo_notification(complex_repo, 'vulnerability_found',
'quay_notification', {}, {'level': 100})
# Ensure that there are no event queue items for the layer.
self.assertIsNone(notification_queue.get())
with fake_security_scanner() as security_scanner:
# Test with an unknown notification.
worker = SecurityNotificationWorker(None)
self.assertFalse(worker.perform_notification_work({
'Name': 'unknownnotification'
}))
# Add some analyzed layers.
analyzer = LayerAnalyzer(app.config, self.api)
analyzer.analyze_recursively(layer1)
analyzer.analyze_recursively(layer2)
# Add a notification with pages of data.
new_vuln_info = {
"Name": "CVE-TEST",
"Namespace": "debian:8",
"Description": "Some service",
"Link": "https://security-tracker.debian.org/tracker/CVE-2014-9471",
"Severity": "Critical",
"FixedIn": {'Version': "9.23-5"},
}
security_scanner.set_vulns(security_scanner.layer_id(layer1), [new_vuln_info])
security_scanner.set_vulns(security_scanner.layer_id(layer2), [new_vuln_info])
# Define offsetting sets of layer IDs, to test cross-pagination support. In this test, we
# will only serve 2 layer IDs per page: the first page will serve both of the 'New' layer IDs,
# but since the first 2 'Old' layer IDs are "earlier" than the shared ID of
# `devtable/simple:latest`, they won't get served in the 'New' list until the *second* page.
# The notification handling system should correctly not notify for this layer, even though it
# is marked 'New' on page 1 and marked 'Old' on page 2. Clair will served these
# IDs sorted in the same manner.
idx_old_layer_ids = [{'LayerName': 'old1', 'Index': 1},
{'LayerName': 'old2', 'Index': 2},
{'LayerName': security_scanner.layer_id(layer1), 'Index': 3}]
idx_new_layer_ids = [{'LayerName': security_scanner.layer_id(layer1), 'Index': 3},
{'LayerName': security_scanner.layer_id(layer2), 'Index': 4}]
old_layer_ids = [t['LayerName'] for t in idx_old_layer_ids]
new_layer_ids = [t['LayerName'] for t in idx_new_layer_ids]
if not indexed:
idx_old_layer_ids = None
idx_new_layer_ids = None
notification_data = security_scanner.add_notification(old_layer_ids, new_layer_ids, None,
new_vuln_info, max_per_page=2,
indexed_old_layer_ids=idx_old_layer_ids,
indexed_new_layer_ids=idx_new_layer_ids)
# Test with a known notification with pages.
data = {
'Name': notification_data['Name'],
}
worker = SecurityNotificationWorker(None)
self.assertTrue(worker.perform_notification_work(data, layer_limit=2))
# Make sure all pages were processed by ensuring we have only one notification. If the second
# page was not processed, then the `Old` entry for layer1 will not be found, and we'd get two
# notifications.
time.sleep(1)
self.assertIsNotNone(notification_queue.get())
self.assertIsNone(notification_queue.get())
if __name__ == '__main__':
unittest.main()