Change security notification code to use the new stream diff reporters
This ensures that even if security scanner pagination sends Old and New layer IDs on different pages, they will properly be handled across the entire notification. Fixes https://www.pivotaltracker.com/story/show/136133657
This commit is contained in:
Joseph Schorr
parent
ced0149520
commit
5b3212ea0e
5 changed files with 301 additions and 190 deletions
|
|
@ -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.
|
||||
|
|
@ -634,7 +641,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,7 +649,7 @@ 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)
|
||||
|
|
@ -650,108 +657,98 @@ class TestSecurityScanner(unittest.TestCase):
|
|||
self.assertIsNotNone(notification_queue.get())
|
||||
|
||||
|
||||
def test_notification_worker_offset_pages(self):
|
||||
def test_notification_worker_offset_pages_not_indexed(self):
|
||||
# Try without indexes.
|
||||
self.assert_notification_worker_offset_pages(indexed=False)
|
||||
|
||||
def get_layer_id(repo_name, tag):
|
||||
# Create a repository notification for the repo, if it doesn't exist.
|
||||
has_notification = model.notification.list_repo_notifications(ADMIN_ACCESS_USER, repo_name,
|
||||
'vulnerability_found')
|
||||
if not list(has_notification):
|
||||
repo = model.repository.get_repository(ADMIN_ACCESS_USER, repo_name)
|
||||
model.notification.create_repo_notification(repo, 'vulnerability_found',
|
||||
'quay_notification', {}, {'level': 100})
|
||||
|
||||
layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, repo_name, tag, include_storage=True)
|
||||
return '%s.%s' % (layer.docker_image_id, layer.storage.uuid)
|
||||
def test_notification_worker_offset_pages_indexed(self):
|
||||
# Try with indexes.
|
||||
self.assert_notification_worker_offset_pages(indexed=True)
|
||||
|
||||
# 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. In practice, Clair will served these IDs
|
||||
# sorted in the same manner.
|
||||
new_layer_ids = [get_layer_id('simple', 'latest'), get_layer_id('complex', 'prod')]
|
||||
old_layer_ids = ['someid1', 'someid2', get_layer_id('simple', 'latest')]
|
||||
|
||||
apis_called = []
|
||||
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)
|
||||
|
||||
@urlmatch(netloc=r'(.*\.)?mockclairservice', path=r'/v1/layers/(.+)')
|
||||
def get_matching_layer_vulnerable(url, request):
|
||||
apis_called.append('VULN')
|
||||
return json.dumps({
|
||||
"Layer": {
|
||||
"Name": 'somelayerid',
|
||||
"Namespace": "debian:8",
|
||||
"IndexedByVersion": 1,
|
||||
"Features": [
|
||||
{
|
||||
"Name": "coreutils",
|
||||
"Namespace": "debian:8",
|
||||
"Version": "8.23-4",
|
||||
"Vulnerabilities": [
|
||||
{
|
||||
"Name": "CVE-TEST",
|
||||
"Namespace": "debian:8",
|
||||
"Severity": "Low",
|
||||
}
|
||||
],
|
||||
}
|
||||
]
|
||||
}
|
||||
})
|
||||
# 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)
|
||||
|
||||
@urlmatch(netloc=r'(.*\.)?mockclairservice', path=r'/v1/notifications/somenotification$', method='DELETE')
|
||||
def delete_notification(url, request):
|
||||
apis_called.append('DELETE')
|
||||
return {'status_code': 201, 'content': ''}
|
||||
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})
|
||||
|
||||
@urlmatch(netloc=r'(.*\.)?mockclairservice', path=r'/v1/notifications/somenotification$', method='GET')
|
||||
def get_notification(url, request):
|
||||
if url.query.find('page=nextpage') >= 0:
|
||||
apis_called.append('GET-2')
|
||||
|
||||
data = {
|
||||
'Notification': self._get_notification_data(new_layer_ids[2:], old_layer_ids[2:]),
|
||||
}
|
||||
|
||||
return json.dumps(data)
|
||||
else:
|
||||
apis_called.append('GET-1')
|
||||
|
||||
notification_data = self._get_notification_data(new_layer_ids[0:2], old_layer_ids[0:2])
|
||||
notification_data['NextPage'] = 'nextpage'
|
||||
|
||||
data = {
|
||||
'Notification': notification_data,
|
||||
}
|
||||
|
||||
return json.dumps(data)
|
||||
|
||||
# Ensure that there are no event queue items for any layers.
|
||||
# Ensure that there are no event queue items for the layer.
|
||||
self.assertIsNone(notification_queue.get())
|
||||
|
||||
# Test with a known notification with pages.
|
||||
data = {
|
||||
'Name': 'somenotification'
|
||||
}
|
||||
|
||||
with HTTMock(get_notification, delete_notification, get_matching_layer_vulnerable):
|
||||
with fake_security_scanner() as security_scanner:
|
||||
# Test with an unknown notification.
|
||||
worker = SecurityNotificationWorker(None)
|
||||
self.assertTrue(worker.perform_notification_work(data))
|
||||
self.assertFalse(worker.perform_notification_work({
|
||||
'Name': 'unknownnotification'
|
||||
}))
|
||||
|
||||
# Verify each of the expected API calls were made.
|
||||
self.assertEquals(set(['GET-1', 'GET-2', 'DELETE', 'VULN']), set(apis_called))
|
||||
# Add some analyzed layers.
|
||||
analyzer = LayerAnalyzer(app.config, self.api)
|
||||
analyzer.analyze_recursively(layer1)
|
||||
analyzer.analyze_recursively(layer2)
|
||||
|
||||
# Verify that we have notifications *just* for the New layer.
|
||||
expected_item = notification_queue.get()
|
||||
self.assertIsNotNone(expected_item)
|
||||
item_body = json.loads(expected_item['body'])
|
||||
self.assertEquals('devtable/complex', item_body['event_data']['repository'])
|
||||
self.assertEquals(['prod'], item_body['event_data']['tags'])
|
||||
# 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())
|
||||
|
||||
# Make sure we have no additional notifications.
|
||||
self.assertIsNone(notification_queue.get())
|
||||
|
||||
|
||||
if __name__ == '__main__':
|
||||
|
|
|
|||
Reference in a new issue