From ce560318462d720b1f56cb23b308b842bca3a7fd Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Fri, 14 Jul 2017 16:09:56 +0300 Subject: [PATCH 01/12] Move notifications into its own package --- buildman/jobutil/buildjob.py | 2 +- endpoints/api/repositorynotification.py | 29 +++++---- endpoints/building.py | 2 +- endpoints/v1/index.py | 1 + endpoints/v2/manifest.py | 1 + notifications/__init__.py | 0 notifications/models_interface.py | 15 +++++ .../notificationevent.py | 10 ++- .../notificationhelper.py | 0 .../notificationmethod.py | 65 ++++++++----------- .../test/test_notificationevent.py | 4 +- test/test_notifications.py | 4 +- test/test_secscan.py | 2 +- util/secscan/analyzer.py | 2 +- util/secscan/notifier.py | 2 +- .../notificationworker/notificationworker.py | 4 +- 16 files changed, 73 insertions(+), 70 deletions(-) create mode 100644 notifications/__init__.py create mode 100644 notifications/models_interface.py rename {endpoints => notifications}/notificationevent.py (98%) rename {endpoints => notifications}/notificationhelper.py (100%) rename {endpoints => notifications}/notificationmethod.py (83%) rename {endpoints => notifications}/test/test_notificationevent.py (92%) diff --git a/buildman/jobutil/buildjob.py b/buildman/jobutil/buildjob.py index 02199898f..2f94b9cc6 100644 --- a/buildman/jobutil/buildjob.py +++ b/buildman/jobutil/buildjob.py @@ -2,7 +2,7 @@ import json import logging from cachetools import lru_cache -from endpoints.notificationhelper import spawn_notification +from notifications.notificationhelper import spawn_notification from data import model from util.imagetree import ImageTree from util.morecollections import AttrDict diff --git a/endpoints/api/repositorynotification.py b/endpoints/api/repositorynotification.py index 10192479b..34826fdb2 100644 --- a/endpoints/api/repositorynotification.py +++ b/endpoints/api/repositorynotification.py @@ -6,10 +6,11 @@ from flask import request from endpoints.api import (RepositoryParamResource, nickname, resource, require_repo_admin, log_action, validate_json_request, request_error, path_param, disallow_for_app_repositories) -from endpoints.exception import NotFound, InvalidRequest -from endpoints.notificationmethod import (NotificationMethod, - CannotValidateNotificationMethodException) -from endpoints.notificationhelper import build_notification_data +from endpoints.exception import NotFound +from notifications.notificationevent import NotificationEvent +from notifications.notificationmethod import (NotificationMethod, + CannotValidateNotificationMethodException) +from notifications.notificationhelper import build_notification_data from workers.notificationworker.models_pre_oci import notification from endpoints.api.repositorynotification_models_pre_oci import pre_oci_model as model @@ -61,18 +62,18 @@ class RepositoryNotificationList(RepositoryParamResource): @validate_json_request('NotificationCreateRequest') def post(self, namespace_name, repository_name): parsed = request.get_json() - + method_handler = NotificationMethod.get_method(parsed['method']) try: method_handler.validate(namespace_name, repository_name, parsed['config']) except CannotValidateNotificationMethodException as ex: raise request_error(message=ex.message) - - new_notification = model.create_repo_notification(namespace_name, repository_name, - parsed['event'], - parsed['method'], - parsed['config'], - parsed['eventConfig'], + + new_notification = model.create_repo_notification(namespace_name, repository_name, + parsed['event'], + parsed['method'], + parsed['config'], + parsed['eventConfig'], parsed.get('title')) log_action('add_repo_notification', namespace_name, @@ -116,7 +117,7 @@ class RepositoryNotification(RepositoryParamResource): deleted = model.delete_repo_notification(namespace_name, repository_name, uuid) if not deleted: raise InvalidRequest("No repository notification found for: %s, %s, %s" % (namespace_name, repository_name, uuid)) - + log_action('delete_repo_notification', namespace_name, {'repo': repository_name, 'namespace': namespace_name, 'notification_id': uuid, 'event': deleted.event_name, 'method': deleted.method_name}, @@ -132,7 +133,7 @@ class RepositoryNotification(RepositoryParamResource): reset = model.reset_notification_number_of_failures(namespace_name, repository_name, uuid) if not reset: raise InvalidRequest("No repository notification found for: %s, %s, %s" % (namespace_name, repository_name, uuid)) - + log_action('reset_repo_notification', namespace_name, {'repo': repository_name, 'namespace': namespace_name, 'notification_id': uuid, 'event': reset.event_name, 'method': reset.method_name}, @@ -155,5 +156,5 @@ class TestRepositoryNotification(RepositoryParamResource): if not test_note: raise InvalidRequest("No repository notification found for: %s, %s, %s" % (namespace_name, repository_name, uuid)) - + return {}, 200 diff --git a/endpoints/building.py b/endpoints/building.py index 74a611841..470a1ab84 100644 --- a/endpoints/building.py +++ b/endpoints/building.py @@ -9,7 +9,7 @@ from app import app, dockerfile_build_queue, metric_queue from data import model from data.database import db from auth.auth_context import get_authenticated_user -from endpoints.notificationhelper import spawn_notification +from notifications.notificationhelper import spawn_notification from util.names import escape_tag from util.morecollections import AttrDict diff --git a/endpoints/v1/index.py b/endpoints/v1/index.py index 877fb90c5..25678a924 100644 --- a/endpoints/v1/index.py +++ b/endpoints/v1/index.py @@ -17,6 +17,7 @@ from endpoints.decorators import anon_protect, anon_allowed, parse_repository_na from endpoints.notificationhelper import spawn_notification from endpoints.v1 import v1_bp from endpoints.v1.models_pre_oci import pre_oci_model as model +from notifications.notificationhelper import spawn_notification from util.audit import track_and_log from util.http import abort from util.names import REPOSITORY_NAME_REGEX diff --git a/endpoints/v2/manifest.py b/endpoints/v2/manifest.py index b4970e2f6..ae04524dd 100644 --- a/endpoints/v2/manifest.py +++ b/endpoints/v2/manifest.py @@ -20,6 +20,7 @@ from endpoints.v2.labelhandlers import handle_label from image.docker import ManifestException from image.docker.schema1 import DockerSchema1Manifest, DockerSchema1ManifestBuilder from image.docker.schema2 import DOCKER_SCHEMA2_CONTENT_TYPES +from notifications.notificationhelper import spawn_notification from util.audit import track_and_log from util.names import VALID_TAG_PATTERN from util.registry.replication import queue_replication_batch diff --git a/notifications/__init__.py b/notifications/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/notifications/models_interface.py b/notifications/models_interface.py new file mode 100644 index 000000000..4c5dac3bd --- /dev/null +++ b/notifications/models_interface.py @@ -0,0 +1,15 @@ +from collections import namedtuple + +class Repository(namedtuple('Repository', ['namespace_name', 'name'])): + """ + Repository represents a repository. + """ + + +class Notification( + namedtuple('Notification', [ + 'uuid', 'event_name', 'method_name', 'event_config_dict', 'method_config_dict', + 'repository'])): + """ + Notification represents a registered notification of some kind. + """ diff --git a/endpoints/notificationevent.py b/notifications/notificationevent.py similarity index 98% rename from endpoints/notificationevent.py rename to notifications/notificationevent.py index 750e84caf..0401e1b05 100644 --- a/endpoints/notificationevent.py +++ b/notifications/notificationevent.py @@ -1,17 +1,16 @@ import logging import time -import json import re from datetime import datetime -from endpoints.notificationhelper import build_event_data +from notifications.notificationhelper import build_event_data from util.jinjautil import get_template_env -from util.morecollections import AttrDict from util.secscan import PRIORITY_LEVELS, get_priority_for_index -template_env = get_template_env("events") logger = logging.getLogger(__name__) +TEMPLATE_ENV = get_template_env("events") + class InvalidNotificationEventException(Exception): pass @@ -36,7 +35,7 @@ class NotificationEvent(object): """ Returns a human readable HTML message for the given notification data. """ - return template_env.get_template(self.event_name() + '.html').render({ + return TEMPLATE_ENV.get_template(self.event_name() + '.html').render({ 'event_data': event_data, 'notification_data': notification_data }) @@ -363,4 +362,3 @@ class BuildCancelledEvent(BaseBuildEvent): def get_summary(self, event_data, notification_data): return 'Build cancelled ' + _build_summary(event_data) - diff --git a/endpoints/notificationhelper.py b/notifications/notificationhelper.py similarity index 100% rename from endpoints/notificationhelper.py rename to notifications/notificationhelper.py diff --git a/endpoints/notificationmethod.py b/notifications/notificationmethod.py similarity index 83% rename from endpoints/notificationmethod.py rename to notifications/notificationmethod.py index 19c6ad9a5..2a426cfa6 100644 --- a/endpoints/notificationmethod.py +++ b/notifications/notificationmethod.py @@ -1,6 +1,6 @@ -import json import logging import re +import json import requests from flask_mail import Message @@ -27,22 +27,11 @@ class NotificationMethodPerformException(JobException): pass -def _get_namespace_name_from(repository): - # TODO Charlie 2017-07-14: This is hack for a bug in production - # because in some places have started calling this method with - # pre oci models and in some we have started calling with non pre oci models. We should - # remove this when we have switched over to database interfaces. - if hasattr(repository, 'namespace_name'): - namespace_name = repository.namespace_name - else: - namespace_name = repository.namespace_user.username - return namespace_name +def _ssl_cert(): + if app.config['PREFERRED_URL_SCHEME'] == 'https': + return [OVERRIDE_CONFIG_DIRECTORY + f for f in SSL_FILENAMES] - -SSLClientCert = None -if app.config['PREFERRED_URL_SCHEME'] == 'https': - # TODO(jschorr): move this into the config provider library - SSLClientCert = [OVERRIDE_CONFIG_DIRECTORY + f for f in SSL_FILENAMES] + return None class NotificationMethod(object): @@ -56,7 +45,7 @@ class NotificationMethod(object): """ raise NotImplementedError - def validate(self, namespace_name, repository_name, config_data): + def validate(self, repository, config_data): """ Validates that the notification can be created with the given data. Throws a CannotValidateNotificationMethodException on failure. @@ -88,12 +77,12 @@ class QuayNotificationMethod(NotificationMethod): def method_name(cls): return 'quay_notification' - def validate(self, namespace_name, repository_name, config_data): - status, err_message, target_users = self.find_targets(namespace_name, repository_name, config_data) + def validate(self, repository, config_data): + _, err_message, _ = self.find_targets(repository, config_data) if err_message: raise CannotValidateNotificationMethodException(err_message) - def find_targets(self, namespace_name, repository_name, config_data): + def find_targets(self, repository, config_data): target_info = config_data['target'] if target_info['kind'] == 'user': @@ -110,7 +99,7 @@ class QuayNotificationMethod(NotificationMethod): return (True, 'Unknown organization %s' % target_info['name'], None) # Only repositories under the organization can cause notifications to that org. - if target_info['name'] != _get_namespace_name_from(repository): + if target_info['name'] != repository.namespace_name: return (False, 'Organization name must match repository namespace') return (True, None, [target]) @@ -118,7 +107,7 @@ class QuayNotificationMethod(NotificationMethod): # Lookup the team. org_team = None try: - org_team = model.team.get_organization_team(_get_namespace_name_from(repository), target_info['name']) + org_team = model.team.get_organization_team(repository.namespace_name, target_info['name']) except model.InvalidTeamException: # Probably deleted. return (True, 'Unknown team %s' % target_info['name'], None) @@ -134,7 +123,7 @@ class QuayNotificationMethod(NotificationMethod): # Lookup the target user or team to which we'll send the notification. config_data = notification_obj.method_config_dict - status, err_message, target_users = self.find_targets(_get_namespace_name_from(repository), repository.name, config_data) + status, err_message, target_users = self.find_targets(repository, config_data) if not status: raise NotificationMethodPerformException(err_message) @@ -149,12 +138,12 @@ class EmailMethod(NotificationMethod): def method_name(cls): return 'email' - def validate(self, namespace_name, repository_name, config_data): + def validate(self, repository, config_data): email = config_data.get('email', '') if not email: raise CannotValidateNotificationMethodException('Missing e-mail address') - record = model.repository.get_email_authorized_for_repo(_get_namespace_name_from(repository), + record = model.repository.get_email_authorized_for_repo(repository.namespace_name, repository.name, email) if not record or not record.confirmed: raise CannotValidateNotificationMethodException('The specified e-mail address ' @@ -175,7 +164,7 @@ class EmailMethod(NotificationMethod): try: mail.send(msg) except Exception as ex: - logger.exception('Email was unable to be sent: %s' % ex.message) + logger.exception('Email was unable to be sent') raise NotificationMethodPerformException(ex.message) @@ -184,7 +173,7 @@ class WebhookMethod(NotificationMethod): def method_name(cls): return 'webhook' - def validate(self, namespace_name, repository_name, config_data): + def validate(self, repository, config_data): url = config_data.get('url', '') if not url: raise CannotValidateNotificationMethodException('Missing webhook URL') @@ -199,7 +188,7 @@ class WebhookMethod(NotificationMethod): headers = {'Content-type': 'application/json'} try: - resp = requests.post(url, data=json.dumps(payload), headers=headers, cert=SSLClientCert, + resp = requests.post(url, data=json.dumps(payload), headers=headers, cert=_ssl_cert(), timeout=METHOD_TIMEOUT) if resp.status_code / 100 != 2: error_message = '%s response for webhook to url: %s' % (resp.status_code, url) @@ -208,7 +197,7 @@ class WebhookMethod(NotificationMethod): raise NotificationMethodPerformException(error_message) except requests.exceptions.RequestException as ex: - logger.exception('Webhook was unable to be sent: %s' % ex.message) + logger.exception('Webhook was unable to be sent') raise NotificationMethodPerformException(ex.message) @@ -221,7 +210,7 @@ class FlowdockMethod(NotificationMethod): def method_name(cls): return 'flowdock' - def validate(self, namespace_name, repository_name, config_data): + def validate(self, repository, config_data): token = config_data.get('flow_api_token', '') if not token: raise CannotValidateNotificationMethodException('Missing Flowdock API Token') @@ -232,7 +221,7 @@ class FlowdockMethod(NotificationMethod): if not token: return - owner = model.user.get_user_or_org(_get_namespace_name_from(notification_obj.repository)) + owner = model.user.get_user_or_org(notification_obj.repository.namespace_name) if not owner: # Something went wrong. return @@ -245,7 +234,7 @@ class FlowdockMethod(NotificationMethod): 'subject': event_handler.get_summary(notification_data['event_data'], notification_data), 'content': event_handler.get_message(notification_data['event_data'], notification_data), 'from_name': owner.username, - 'project': (_get_namespace_name_from(notification_obj.repository)+ ' ' + + 'project': (notification_obj.repository.namespace_name + ' ' + notification_obj.repository.name), 'tags': ['#' + event_handler.event_name()], 'link': notification_data['event_data']['homepage'] @@ -260,7 +249,7 @@ class FlowdockMethod(NotificationMethod): raise NotificationMethodPerformException(error_message) except requests.exceptions.RequestException as ex: - logger.exception('Flowdock method was unable to be sent: %s' % ex.message) + logger.exception('Flowdock method was unable to be sent') raise NotificationMethodPerformException(ex.message) @@ -273,7 +262,7 @@ class HipchatMethod(NotificationMethod): def method_name(cls): return 'hipchat' - def validate(self, namespace_name, repository_name, config_data): + def validate(self, repository, config_data): if not config_data.get('notification_token', ''): raise CannotValidateNotificationMethodException('Missing Hipchat Room Notification Token') @@ -288,7 +277,7 @@ class HipchatMethod(NotificationMethod): if not token or not room_id: return - owner = model.user.get_user_or_org(_get_namespace_name_from(notification_obj.repository)) + owner = model.user.get_user_or_org(notification_obj.repository.namespace_name) if not owner: # Something went wrong. return @@ -321,7 +310,7 @@ class HipchatMethod(NotificationMethod): raise NotificationMethodPerformException(error_message) except requests.exceptions.RequestException as ex: - logger.exception('Hipchat method was unable to be sent: %s' % ex.message) + logger.exception('Hipchat method was unable to be sent') raise NotificationMethodPerformException(ex.message) @@ -384,7 +373,7 @@ class SlackMethod(NotificationMethod): def method_name(cls): return 'slack' - def validate(self, namespace_name, repository_name, config_data): + def validate(self, repository, config_data): if not config_data.get('url', ''): raise CannotValidateNotificationMethodException('Missing Slack Callback URL') @@ -400,7 +389,7 @@ class SlackMethod(NotificationMethod): if not url: return - owner = model.user.get_user_or_org(_get_namespace_name_from(notification_obj.repository)) + owner = model.user.get_user_or_org(notification_obj.repository.namespace_name) if not owner: # Something went wrong. return diff --git a/endpoints/test/test_notificationevent.py b/notifications/test/test_notificationevent.py similarity index 92% rename from endpoints/test/test_notificationevent.py rename to notifications/test/test_notificationevent.py index fd4d81c12..7429ef0a6 100644 --- a/endpoints/test/test_notificationevent.py +++ b/notifications/test/test_notificationevent.py @@ -1,6 +1,4 @@ -import json - -from endpoints.notificationevent import NotificationEvent +from notifications.notificationevent import NotificationEvent from util.morecollections import AttrDict from test.fixtures import * diff --git a/test/test_notifications.py b/test/test_notifications.py index 25ad26ae7..486bef19d 100644 --- a/test/test_notifications.py +++ b/test/test_notifications.py @@ -1,7 +1,7 @@ import unittest -from endpoints.notificationevent import (BuildSuccessEvent, NotificationEvent, - VulnerabilityFoundEvent) +from notifications.notificationevent import (BuildSuccessEvent, NotificationEvent, + VulnerabilityFoundEvent) from util.morecollections import AttrDict class TestCreate(unittest.TestCase): diff --git a/test/test_secscan.py b/test/test_secscan.py index 1e312e00d..8ba559d95 100644 --- a/test/test_secscan.py +++ b/test/test_secscan.py @@ -5,9 +5,9 @@ import unittest from app import app, storage, notification_queue from data import model 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 notifications.notificationevent import VulnerabilityFoundEvent from util.morecollections import AttrDict from util.secscan.api import SecurityScannerAPI, APIRequestFailure from util.secscan.analyzer import LayerAnalyzer diff --git a/util/secscan/analyzer.py b/util/secscan/analyzer.py index cdfe99051..0b4698c46 100644 --- a/util/secscan/analyzer.py +++ b/util/secscan/analyzer.py @@ -5,10 +5,10 @@ from collections import defaultdict import features -from endpoints.notificationhelper import spawn_notification from data.database import ExternalNotificationEvent, IMAGE_NOT_SCANNED_ENGINE_VERSION, Image 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 notifications.notificationhelper import spawn_notification from util.secscan import PRIORITY_LEVELS from util.secscan.api import (APIRequestFailure, AnalyzeLayerException, MissingParentLayerException, InvalidLayerException, AnalyzeLayerRetryException) diff --git a/util/secscan/notifier.py b/util/secscan/notifier.py index c71209ebe..bbd6525be 100644 --- a/util/secscan/notifier.py +++ b/util/secscan/notifier.py @@ -10,7 +10,7 @@ from data.model.tag import (filter_has_repository_event, filter_tags_have_reposi from data.database import (Image, ImageStorage, ExternalNotificationEvent, Repository, RepositoryTag) -from endpoints.notificationhelper import notification_batch +from notifications.notificationhelper import notification_batch from util.secscan import PRIORITY_LEVELS from util.secscan.api import APIRequestFailure from util.morecollections import AttrDict, StreamingDiffTracker, IndexedStreamingDiffTracker diff --git a/workers/notificationworker/notificationworker.py b/workers/notificationworker/notificationworker.py index 9a478f758..c4c0ba907 100644 --- a/workers/notificationworker/notificationworker.py +++ b/workers/notificationworker/notificationworker.py @@ -1,8 +1,8 @@ import logging from app import notification_queue -from endpoints.notificationmethod import NotificationMethod, InvalidNotificationMethodException -from endpoints.notificationevent import NotificationEvent, InvalidNotificationEventException +from notifications.notificationmethod import NotificationMethod, InvalidNotificationMethodException +from notifications.notificationevent import NotificationEvent, InvalidNotificationEventException from workers.notificationworker.models_pre_oci import pre_oci_model as model from workers.queueworker import QueueWorker, JobException From 5739e2ef4db179157fc1a396ae3c68a341fb4e6c Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Fri, 14 Jul 2017 16:16:37 +0300 Subject: [PATCH 02/12] Move notifications test into notifications package --- notifications/notificationevent.py | 5 + notifications/test/test_notifications.py | 171 +++++++++++++++++++++ test/test_notifications.py | 184 ----------------------- 3 files changed, 176 insertions(+), 184 deletions(-) create mode 100644 notifications/test/test_notifications.py delete mode 100644 test/test_notifications.py diff --git a/notifications/notificationevent.py b/notifications/notificationevent.py index 0401e1b05..7de742624 100644 --- a/notifications/notificationevent.py +++ b/notifications/notificationevent.py @@ -67,6 +67,11 @@ class NotificationEvent(object): raise InvalidNotificationEventException('Unable to find event: %s' % eventname) + @classmethod + def event_names(cls): + for subc in cls.__subclasses__(): + if subc.event_name() is not None: + yield subc.event_name() @staticmethod def _get_event(cls, eventname): diff --git a/notifications/test/test_notifications.py b/notifications/test/test_notifications.py new file mode 100644 index 000000000..a075ce30e --- /dev/null +++ b/notifications/test/test_notifications.py @@ -0,0 +1,171 @@ +import pytest + +from notifications.notificationevent import (BuildSuccessEvent, NotificationEvent, + VulnerabilityFoundEvent) +from util.morecollections import AttrDict + + +@pytest.mark.parametrize('event_kind', NotificationEvent.event_names()) +def test_create_notifications(event_kind): + assert NotificationEvent.get_event(event_kind) is not None + + +def test_build_emptyjson(): + notification_data = AttrDict({ + 'event_config_dict': None, + }) + + # No build data at all. + assert BuildSuccessEvent().should_perform({}, notification_data) + +def test_build_nofilter(): + notification_data = AttrDict({ + 'event_config_dict': {}, + }) + + # No build data at all. + assert BuildSuccessEvent().should_perform({}, notification_data) + + # With trigger metadata but no ref. + assert BuildSuccessEvent().should_perform({ + 'trigger_metadata': {}, + }, notification_data) + + # With trigger metadata and a ref. + assert BuildSuccessEvent().should_perform({ + 'trigger_metadata': { + 'ref': 'refs/heads/somebranch', + }, + }, notification_data) + + +def test_build_emptyfilter(): + notification_data = AttrDict({ + 'event_config_dict': {"ref-regex": ""}, + }) + + # No build data at all. + assert BuildSuccessEvent().should_perform({}, notification_data) + + # With trigger metadata but no ref. + assert BuildSuccessEvent().should_perform({ + 'trigger_metadata': {}, + }, notification_data) + + # With trigger metadata and a ref. + assert BuildSuccessEvent().should_perform({ + 'trigger_metadata': { + 'ref': 'refs/heads/somebranch', + }, + }, notification_data) + + +def test_build_invalidfilter(): + notification_data = AttrDict({ + 'event_config_dict': {"ref-regex": "]["}, + }) + + # No build data at all. + assert not BuildSuccessEvent().should_perform({}, notification_data) + + # With trigger metadata but no ref. + assert not BuildSuccessEvent().should_perform({ + 'trigger_metadata': {}, + }, notification_data) + + # With trigger metadata and a ref. + assert not BuildSuccessEvent().should_perform({ + 'trigger_metadata': { + 'ref': 'refs/heads/somebranch', + }, + }, notification_data) + + +def test_build_withfilter(): + notification_data = AttrDict({ + 'event_config_dict': {"ref-regex": "refs/heads/master"}, + }) + + # No build data at all. + assert not BuildSuccessEvent().should_perform({}, notification_data) + + # With trigger metadata but no ref. + assert not BuildSuccessEvent().should_perform({ + 'trigger_metadata': {}, + }, notification_data) + + # With trigger metadata and a not-matching ref. + assert not BuildSuccessEvent().should_perform({ + 'trigger_metadata': { + 'ref': 'refs/heads/somebranch', + }, + }, notification_data) + + # With trigger metadata and a matching ref. + assert BuildSuccessEvent().should_perform({ + 'trigger_metadata': { + 'ref': 'refs/heads/master', + }, + }, notification_data) + + +def test_build_withwildcardfilter(): + notification_data = AttrDict({ + 'event_config_dict': {"ref-regex": "refs/heads/.+"}, + }) + + # No build data at all. + assert not BuildSuccessEvent().should_perform({}, notification_data) + + # With trigger metadata but no ref. + assert not BuildSuccessEvent().should_perform({ + 'trigger_metadata': {}, + }, notification_data) + + # With trigger metadata and a not-matching ref. + assert not BuildSuccessEvent().should_perform({ + 'trigger_metadata': { + 'ref': 'refs/tags/sometag', + }, + }, notification_data) + + # With trigger metadata and a matching ref. + assert BuildSuccessEvent().should_perform({ + 'trigger_metadata': { + 'ref': 'refs/heads/master', + }, + }, notification_data) + + # With trigger metadata and another matching ref. + assert BuildSuccessEvent().should_perform({ + 'trigger_metadata': { + 'ref': 'refs/heads/somebranch', + }, + }, notification_data) + + +def test_vulnerability_notification_nolevel(): + notification_data = AttrDict({ + 'event_config_dict': {}, + }) + + # No level specified. + assert VulnerabilityFoundEvent().should_perform({}, notification_data) + + +def test_vulnerability_notification_nopvulninfo(): + notification_data = AttrDict({ + 'event_config_dict': {"level": 3}, + }) + + # No vuln info. + assert not VulnerabilityFoundEvent().should_perform({}, notification_data) + + +def test_vulnerability_notification_normal(): + notification_data = AttrDict({ + 'event_config_dict': {"level": 3}, + }) + + info = {"vulnerability": {"priority": "Critical"}} + assert VulnerabilityFoundEvent().should_perform(info, notification_data) diff --git a/test/test_notifications.py b/test/test_notifications.py deleted file mode 100644 index 486bef19d..000000000 --- a/test/test_notifications.py +++ /dev/null @@ -1,184 +0,0 @@ -import unittest - -from notifications.notificationevent import (BuildSuccessEvent, NotificationEvent, - VulnerabilityFoundEvent) -from util.morecollections import AttrDict - -class TestCreate(unittest.TestCase): - def test_create_notifications(self): - self.assertIsNotNone(NotificationEvent.get_event('repo_push')) - self.assertIsNotNone(NotificationEvent.get_event('build_queued')) - self.assertIsNotNone(NotificationEvent.get_event('build_success')) - self.assertIsNotNone(NotificationEvent.get_event('build_failure')) - self.assertIsNotNone(NotificationEvent.get_event('build_start')) - self.assertIsNotNone(NotificationEvent.get_event('build_cancelled')) - self.assertIsNotNone(NotificationEvent.get_event('vulnerability_found')) - - -class TestShouldPerform(unittest.TestCase): - def test_build_emptyjson(self): - notification_data = AttrDict({ - 'event_config_dict': None, - }) - - # No build data at all. - self.assertTrue(BuildSuccessEvent().should_perform({}, notification_data)) - - def test_build_nofilter(self): - notification_data = AttrDict({ - 'event_config_dict': {}, - }) - - # No build data at all. - self.assertTrue(BuildSuccessEvent().should_perform({}, notification_data)) - - # With trigger metadata but no ref. - self.assertTrue(BuildSuccessEvent().should_perform({ - 'trigger_metadata': {}, - }, notification_data)) - - # With trigger metadata and a ref. - self.assertTrue(BuildSuccessEvent().should_perform({ - 'trigger_metadata': { - 'ref': 'refs/heads/somebranch', - }, - }, notification_data)) - - - def test_build_emptyfilter(self): - notification_data = AttrDict({ - 'event_config_dict': {"ref-regex": ""}, - }) - - # No build data at all. - self.assertTrue(BuildSuccessEvent().should_perform({}, notification_data)) - - # With trigger metadata but no ref. - self.assertTrue(BuildSuccessEvent().should_perform({ - 'trigger_metadata': {}, - }, notification_data)) - - # With trigger metadata and a ref. - self.assertTrue(BuildSuccessEvent().should_perform({ - 'trigger_metadata': { - 'ref': 'refs/heads/somebranch', - }, - }, notification_data)) - - - def test_build_invalidfilter(self): - notification_data = AttrDict({ - 'event_config_dict': {"ref-regex": "]["}, - }) - - # No build data at all. - self.assertFalse(BuildSuccessEvent().should_perform({}, notification_data)) - - # With trigger metadata but no ref. - self.assertFalse(BuildSuccessEvent().should_perform({ - 'trigger_metadata': {}, - }, notification_data)) - - # With trigger metadata and a ref. - self.assertFalse(BuildSuccessEvent().should_perform({ - 'trigger_metadata': { - 'ref': 'refs/heads/somebranch', - }, - }, notification_data)) - - - def test_build_withfilter(self): - notification_data = AttrDict({ - 'event_config_dict': {"ref-regex": "refs/heads/master"}, - }) - - # No build data at all. - self.assertFalse(BuildSuccessEvent().should_perform({}, notification_data)) - - # With trigger metadata but no ref. - self.assertFalse(BuildSuccessEvent().should_perform({ - 'trigger_metadata': {}, - }, notification_data)) - - # With trigger metadata and a not-matching ref. - self.assertFalse(BuildSuccessEvent().should_perform({ - 'trigger_metadata': { - 'ref': 'refs/heads/somebranch', - }, - }, notification_data)) - - # With trigger metadata and a matching ref. - self.assertTrue(BuildSuccessEvent().should_perform({ - 'trigger_metadata': { - 'ref': 'refs/heads/master', - }, - }, notification_data)) - - - def test_build_withwildcardfilter(self): - notification_data = AttrDict({ - 'event_config_dict': {"ref-regex": "refs/heads/.+"}, - }) - - # No build data at all. - self.assertFalse(BuildSuccessEvent().should_perform({}, notification_data)) - - # With trigger metadata but no ref. - self.assertFalse(BuildSuccessEvent().should_perform({ - 'trigger_metadata': {}, - }, notification_data)) - - # With trigger metadata and a not-matching ref. - self.assertFalse(BuildSuccessEvent().should_perform({ - 'trigger_metadata': { - 'ref': 'refs/tags/sometag', - }, - }, notification_data)) - - # With trigger metadata and a matching ref. - self.assertTrue(BuildSuccessEvent().should_perform({ - 'trigger_metadata': { - 'ref': 'refs/heads/master', - }, - }, notification_data)) - - # With trigger metadata and another matching ref. - self.assertTrue(BuildSuccessEvent().should_perform({ - 'trigger_metadata': { - 'ref': 'refs/heads/somebranch', - }, - }, notification_data)) - - - def test_vulnerability_notification_nolevel(self): - notification_data = AttrDict({ - 'event_config_dict': {}, - }) - - # No level specified. - self.assertTrue(VulnerabilityFoundEvent().should_perform({}, notification_data)) - - - def test_vulnerability_notification_nopvulninfo(self): - notification_data = AttrDict({ - 'event_config_dict': {"level": 3}, - }) - - # No vuln info. - self.assertFalse(VulnerabilityFoundEvent().should_perform({}, notification_data)) - - - def test_vulnerability_notification_normal(self): - notification_data = AttrDict({ - 'event_config_dict': {"level": 3}, - }) - - info = {"vulnerability": {"priority": "Critical"}} - self.assertTrue(VulnerabilityFoundEvent().should_perform(info, notification_data)) - - - - -if __name__ == '__main__': - unittest.main() - From e7dbc4ee91e564709e842c4d267966c585836ec2 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Fri, 14 Jul 2017 16:29:59 +0300 Subject: [PATCH 03/12] Move notification helper code into the root module --- buildman/jobutil/buildjob.py | 2 +- endpoints/api/repositorynotification.py | 2 +- endpoints/building.py | 2 +- endpoints/v1/index.py | 2 +- endpoints/v2/manifest.py | 2 +- notifications/__init__.py | 84 +++++++++++++++++++++++++ notifications/notificationevent.py | 2 +- util/secscan/analyzer.py | 2 +- util/secscan/notifier.py | 2 +- 9 files changed, 92 insertions(+), 8 deletions(-) diff --git a/buildman/jobutil/buildjob.py b/buildman/jobutil/buildjob.py index 2f94b9cc6..18381b0f2 100644 --- a/buildman/jobutil/buildjob.py +++ b/buildman/jobutil/buildjob.py @@ -2,7 +2,7 @@ import json import logging from cachetools import lru_cache -from notifications.notificationhelper import spawn_notification +from notifications import spawn_notification from data import model from util.imagetree import ImageTree from util.morecollections import AttrDict diff --git a/endpoints/api/repositorynotification.py b/endpoints/api/repositorynotification.py index 34826fdb2..ff91972c1 100644 --- a/endpoints/api/repositorynotification.py +++ b/endpoints/api/repositorynotification.py @@ -7,10 +7,10 @@ from endpoints.api import (RepositoryParamResource, nickname, resource, require_ log_action, validate_json_request, request_error, path_param, disallow_for_app_repositories) from endpoints.exception import NotFound +from notifications import build_notification_data from notifications.notificationevent import NotificationEvent from notifications.notificationmethod import (NotificationMethod, CannotValidateNotificationMethodException) -from notifications.notificationhelper import build_notification_data from workers.notificationworker.models_pre_oci import notification from endpoints.api.repositorynotification_models_pre_oci import pre_oci_model as model diff --git a/endpoints/building.py b/endpoints/building.py index 470a1ab84..81bf984b8 100644 --- a/endpoints/building.py +++ b/endpoints/building.py @@ -9,7 +9,7 @@ from app import app, dockerfile_build_queue, metric_queue from data import model from data.database import db from auth.auth_context import get_authenticated_user -from notifications.notificationhelper import spawn_notification +from notifications import spawn_notification from util.names import escape_tag from util.morecollections import AttrDict diff --git a/endpoints/v1/index.py b/endpoints/v1/index.py index 25678a924..3e34498f4 100644 --- a/endpoints/v1/index.py +++ b/endpoints/v1/index.py @@ -17,7 +17,7 @@ from endpoints.decorators import anon_protect, anon_allowed, parse_repository_na from endpoints.notificationhelper import spawn_notification from endpoints.v1 import v1_bp from endpoints.v1.models_pre_oci import pre_oci_model as model -from notifications.notificationhelper import spawn_notification +from notifications import spawn_notification from util.audit import track_and_log from util.http import abort from util.names import REPOSITORY_NAME_REGEX diff --git a/endpoints/v2/manifest.py b/endpoints/v2/manifest.py index ae04524dd..1ef0646f5 100644 --- a/endpoints/v2/manifest.py +++ b/endpoints/v2/manifest.py @@ -20,7 +20,7 @@ from endpoints.v2.labelhandlers import handle_label from image.docker import ManifestException from image.docker.schema1 import DockerSchema1Manifest, DockerSchema1ManifestBuilder from image.docker.schema2 import DOCKER_SCHEMA2_CONTENT_TYPES -from notifications.notificationhelper import spawn_notification +from notifications import spawn_notification from util.audit import track_and_log from util.names import VALID_TAG_PATTERN from util.registry.replication import queue_replication_batch diff --git a/notifications/__init__.py b/notifications/__init__.py index e69de29bb..37e5bc56b 100644 --- a/notifications/__init__.py +++ b/notifications/__init__.py @@ -0,0 +1,84 @@ +import json + +from contextlib import contextmanager + +from app import app, notification_queue +from data import model +from auth.auth_context import get_authenticated_user, get_validated_oauth_token + + +DEFAULT_BATCH_SIZE = 1000 + + +def build_event_data(repo, extra_data=None, subpage=None): + repo_string = '%s/%s' % (repo.namespace_name, repo.name) + homepage = '%s://%s/repository/%s' % (app.config['PREFERRED_URL_SCHEME'], + app.config['SERVER_HOSTNAME'], + repo_string) + + if subpage: + if not subpage.startswith('/'): + subpage = '/' + subpage + + homepage = homepage + subpage + + event_data = { + 'repository': repo_string, + 'namespace': repo.namespace_name, + 'name': repo.name, + 'docker_url': '%s/%s' % (app.config['SERVER_HOSTNAME'], repo_string), + 'homepage': homepage, + } + + event_data.update(extra_data or {}) + return event_data + +def build_notification_data(notification, event_data, performer_data=None): + if not performer_data: + performer_data = {} + + oauth_token = get_validated_oauth_token() + if oauth_token: + performer_data['oauth_token_id'] = oauth_token.id + performer_data['oauth_token_application_id'] = oauth_token.application.client_id + performer_data['oauth_token_application'] = oauth_token.application.name + + performer_user = get_authenticated_user() + if performer_user: + performer_data['entity_id'] = performer_user.id + performer_data['entity_name'] = performer_user.username + + return { + 'notification_uuid': notification.uuid, + 'event_data': event_data, + 'performer_data': performer_data, + } + + +@contextmanager +def notification_batch(batch_size=DEFAULT_BATCH_SIZE): + """ + Context manager implementation which returns a target callable with the same signature + as spawn_notification. When the the context block exits the notifications generated by + the callable will be bulk inserted into the queue with the specified batch size. + """ + with notification_queue.batch_insert(batch_size) as queue_put: + def spawn_notification_batch(repo, event_name, extra_data=None, subpage=None, pathargs=None, + performer_data=None): + event_data = build_event_data(repo, extra_data=extra_data, subpage=subpage) + + notifications = model.notification.list_repo_notifications(repo.namespace_name, + repo.name, + event_name=event_name) + path = [repo.namespace_name, repo.name, event_name] + (pathargs or []) + for notification in list(notifications): + notification_data = build_notification_data(notification, event_data, performer_data) + queue_put(path, json.dumps(notification_data)) + + yield spawn_notification_batch + + +def spawn_notification(repo, event_name, extra_data=None, subpage=None, pathargs=None, + performer_data=None): + with notification_batch(1) as batch_spawn: + batch_spawn(repo, event_name, extra_data, subpage, pathargs, performer_data) diff --git a/notifications/notificationevent.py b/notifications/notificationevent.py index 7de742624..6b5b7fb56 100644 --- a/notifications/notificationevent.py +++ b/notifications/notificationevent.py @@ -3,7 +3,7 @@ import time import re from datetime import datetime -from notifications.notificationhelper import build_event_data +from notifications import build_event_data from util.jinjautil import get_template_env from util.secscan import PRIORITY_LEVELS, get_priority_for_index diff --git a/util/secscan/analyzer.py b/util/secscan/analyzer.py index 0b4698c46..97ab81950 100644 --- a/util/secscan/analyzer.py +++ b/util/secscan/analyzer.py @@ -8,7 +8,7 @@ import features from data.database import ExternalNotificationEvent, IMAGE_NOT_SCANNED_ENGINE_VERSION, Image 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 notifications.notificationhelper import spawn_notification +from notifications import spawn_notification from util.secscan import PRIORITY_LEVELS from util.secscan.api import (APIRequestFailure, AnalyzeLayerException, MissingParentLayerException, InvalidLayerException, AnalyzeLayerRetryException) diff --git a/util/secscan/notifier.py b/util/secscan/notifier.py index bbd6525be..db2b481eb 100644 --- a/util/secscan/notifier.py +++ b/util/secscan/notifier.py @@ -10,7 +10,7 @@ from data.model.tag import (filter_has_repository_event, filter_tags_have_reposi from data.database import (Image, ImageStorage, ExternalNotificationEvent, Repository, RepositoryTag) -from notifications.notificationhelper import notification_batch +from notifications import notification_batch from util.secscan import PRIORITY_LEVELS from util.secscan.api import APIRequestFailure from util.morecollections import AttrDict, StreamingDiffTracker, IndexedStreamingDiffTracker From e7fec9dd20652ddfa47e544e824ca4f343aa8ec5 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Fri, 14 Jul 2017 16:48:31 +0300 Subject: [PATCH 04/12] Change get_sample_data API to not require the custom notification tuple --- endpoints/api/repositorynotification.py | 4 ++- notifications/notificationevent.py | 31 ++++++++++---------- notifications/test/test_notificationevent.py | 17 ++--------- 3 files changed, 21 insertions(+), 31 deletions(-) diff --git a/endpoints/api/repositorynotification.py b/endpoints/api/repositorynotification.py index ff91972c1..22634d235 100644 --- a/endpoints/api/repositorynotification.py +++ b/endpoints/api/repositorynotification.py @@ -8,10 +8,10 @@ from endpoints.api import (RepositoryParamResource, nickname, resource, require_ path_param, disallow_for_app_repositories) from endpoints.exception import NotFound from notifications import build_notification_data +from notifications.models_interface import Repository from notifications.notificationevent import NotificationEvent from notifications.notificationmethod import (NotificationMethod, CannotValidateNotificationMethodException) -from workers.notificationworker.models_pre_oci import notification from endpoints.api.repositorynotification_models_pre_oci import pre_oci_model as model logger = logging.getLogger(__name__) @@ -153,6 +153,8 @@ class TestRepositoryNotification(RepositoryParamResource): def post(self, namespace_name, repository_name, uuid): """ Queues a test notification for this repository. """ test_note = model.queue_test_notification(uuid) + event_info = NotificationEvent.get_event(test_note.event_name) + sample_data = event_info.get_sample_data(Repository(namespace, repository), event_config) if not test_note: raise InvalidRequest("No repository notification found for: %s, %s, %s" % (namespace_name, repository_name, uuid)) diff --git a/notifications/notificationevent.py b/notifications/notificationevent.py index 6b5b7fb56..87a736878 100644 --- a/notifications/notificationevent.py +++ b/notifications/notificationevent.py @@ -40,7 +40,7 @@ class NotificationEvent(object): 'notification_data': notification_data }) - def get_sample_data(self, notification): + def get_sample_data(self, repository, event_config): """ Returns sample data for testing the raising of this notification, with an example notification. """ @@ -95,8 +95,8 @@ class RepoPushEvent(NotificationEvent): def get_summary(self, event_data, notification_data): return 'Repository %s updated' % (event_data['repository']) - def get_sample_data(self, notification): - return build_event_data(notification.repository, { + def get_sample_data(self, repository, event_config): + return build_event_data(repository, { 'updated_tags': {'latest': 'someimageid', 'foo': 'anotherimage'}, 'pruned_image_count': 3 }) @@ -129,10 +129,9 @@ class VulnerabilityFoundEvent(NotificationEvent): return 'info' - def get_sample_data(self, notification): - event_config = notification.event_config_dict + def get_sample_data(self, repository, event_config): level = event_config.get(VulnerabilityFoundEvent.CONFIG_LEVEL, 'Critical') - return build_event_data(notification.repository, { + return build_event_data(repository, { 'tags': ['latest', 'prod', 'foo', 'bar', 'baz'], 'image': 'some-image-id', 'vulnerability': { @@ -217,9 +216,9 @@ class BuildQueueEvent(BaseBuildEvent): def get_level(self, event_data, notification_data): return 'info' - def get_sample_data(self, notification): + def get_sample_data(self, repository, event_config): build_uuid = 'fake-build-id' - return build_event_data(notification.repository, { + return build_event_data(repository, { 'is_manual': False, 'build_id': build_uuid, 'build_name': 'some-fake-build', @@ -255,9 +254,9 @@ class BuildStartEvent(BaseBuildEvent): def get_level(self, event_data, notification_data): return 'info' - def get_sample_data(self, notification): + def get_sample_data(self, repository, event_config): build_uuid = 'fake-build-id' - return build_event_data(notification.repository, { + return build_event_data(repository, { 'build_id': build_uuid, 'build_name': 'some-fake-build', 'docker_tags': ['latest', 'foo', 'bar'], @@ -282,9 +281,9 @@ class BuildSuccessEvent(BaseBuildEvent): def get_level(self, event_data, notification_data): return 'success' - def get_sample_data(self, notification): + def get_sample_data(self, repository, event_config): build_uuid = 'fake-build-id' - return build_event_data(notification.repository, { + return build_event_data(repository, { 'build_id': build_uuid, 'build_name': 'some-fake-build', 'docker_tags': ['latest', 'foo', 'bar'], @@ -310,9 +309,9 @@ class BuildFailureEvent(BaseBuildEvent): def get_level(self, event_data, notification_data): return 'error' - def get_sample_data(self, notification): + def get_sample_data(self, repository, event_config): build_uuid = 'fake-build-id' - return build_event_data(notification.repository, { + return build_event_data(repository, { 'build_id': build_uuid, 'build_name': 'some-fake-build', 'docker_tags': ['latest', 'foo', 'bar'], @@ -349,9 +348,9 @@ class BuildCancelledEvent(BaseBuildEvent): def get_level(self, event_data, notification_data): return 'info' - def get_sample_data(self, notification): + def get_sample_data(self, repository, event_config): build_uuid = 'fake-build-id' - return build_event_data(notification.repository, { + return build_event_data(repository, { 'build_id': build_uuid, 'build_name': 'some-fake-build', 'docker_tags': ['latest', 'foo', 'bar'], diff --git a/notifications/test/test_notificationevent.py b/notifications/test/test_notificationevent.py index 7429ef0a6..a5f83648d 100644 --- a/notifications/test/test_notificationevent.py +++ b/notifications/test/test_notificationevent.py @@ -1,25 +1,14 @@ +from notifications.models_interface import Repository from notifications.notificationevent import NotificationEvent -from util.morecollections import AttrDict from test.fixtures import * -def test_all_notifications(app): - # Create a test notification. - test_notification = AttrDict({ - 'repository': AttrDict({ - 'namespace_name': AttrDict(dict(username='foo')), - 'name': 'bar', - }), - 'event_config_dict': { - 'level': 'low', - }, - }) - +def test_all_notifications(initialized_db): for subc in NotificationEvent.__subclasses__(): if subc.event_name() is not None: # Create the notification event. found = NotificationEvent.get_event(subc.event_name()) - sample_data = found.get_sample_data(test_notification) + sample_data = found.get_sample_data(Repository('foo', 'bar'), {'level': 'low'}) # Make sure all calls succeed. notification_data = { From be49dda75620cec38211de34c0899de72773f45c Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Fri, 14 Jul 2017 16:49:44 +0300 Subject: [PATCH 05/12] Change test to use pytest --- notifications/test/test_notificationevent.py | 27 ++++++++++---------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/notifications/test/test_notificationevent.py b/notifications/test/test_notificationevent.py index a5f83648d..0ec01bf9c 100644 --- a/notifications/test/test_notificationevent.py +++ b/notifications/test/test_notificationevent.py @@ -1,20 +1,21 @@ +import pytest + from notifications.models_interface import Repository from notifications.notificationevent import NotificationEvent from test.fixtures import * -def test_all_notifications(initialized_db): - for subc in NotificationEvent.__subclasses__(): - if subc.event_name() is not None: - # Create the notification event. - found = NotificationEvent.get_event(subc.event_name()) - sample_data = found.get_sample_data(Repository('foo', 'bar'), {'level': 'low'}) +@pytest.mark.parametrize('event_name', NotificationEvent.event_names()) +def test_build_notification(event_name, initialized_db): + # Create the notification event. + found = NotificationEvent.get_event(event_name) + sample_data = found.get_sample_data(Repository('foo', 'bar'), {'level': 'low'}) - # Make sure all calls succeed. - notification_data = { - 'performer_data': {}, - } + # Make sure all calls succeed. + notification_data = { + 'performer_data': {}, + } - found.get_level(sample_data, notification_data) - found.get_summary(sample_data, notification_data) - found.get_message(sample_data, notification_data) + found.get_level(sample_data, notification_data) + found.get_summary(sample_data, notification_data) + found.get_message(sample_data, notification_data) From 875a303762755ead7f4fe41938e702d3705f71ae Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Fri, 14 Jul 2017 16:52:44 +0300 Subject: [PATCH 06/12] Add missing build event template --- events/build_cancelled.html | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 events/build_cancelled.html diff --git a/events/build_cancelled.html b/events/build_cancelled.html new file mode 100644 index 000000000..91c00b519 --- /dev/null +++ b/events/build_cancelled.html @@ -0,0 +1,2 @@ +{% extends "build_event.html" %} +{% block eventkind %}canceled{% endblock %} From b1d5990ab379a07cc08b08a3405cea36f7880958 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Fri, 14 Jul 2017 16:52:52 +0300 Subject: [PATCH 07/12] Make sure event_names returns *all* events --- notifications/notificationevent.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/notifications/notificationevent.py b/notifications/notificationevent.py index 87a736878..ecc6a40a5 100644 --- a/notifications/notificationevent.py +++ b/notifications/notificationevent.py @@ -70,7 +70,10 @@ class NotificationEvent(object): @classmethod def event_names(cls): for subc in cls.__subclasses__(): - if subc.event_name() is not None: + if subc.event_name() is None: + for subsubc in subc.__subclasses__(): + yield subsubc.event_name() + else: yield subc.event_name() @staticmethod From 5e5e191f87a1407723328f89cb44a96441e6fd5c Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Fri, 14 Jul 2017 16:55:03 +0300 Subject: [PATCH 08/12] Merge tests into a single suite --- notifications/test/test_notificationevent.py | 171 ++++++++++++++++++- notifications/test/test_notifications.py | 171 ------------------- 2 files changed, 170 insertions(+), 172 deletions(-) delete mode 100644 notifications/test/test_notifications.py diff --git a/notifications/test/test_notificationevent.py b/notifications/test/test_notificationevent.py index 0ec01bf9c..1e9673cca 100644 --- a/notifications/test/test_notificationevent.py +++ b/notifications/test/test_notificationevent.py @@ -1,10 +1,18 @@ import pytest from notifications.models_interface import Repository -from notifications.notificationevent import NotificationEvent +from notifications.notificationevent import (BuildSuccessEvent, NotificationEvent, + VulnerabilityFoundEvent) +from util.morecollections import AttrDict from test.fixtures import * + +@pytest.mark.parametrize('event_kind', NotificationEvent.event_names()) +def test_create_notifications(event_kind): + assert NotificationEvent.get_event(event_kind) is not None + + @pytest.mark.parametrize('event_name', NotificationEvent.event_names()) def test_build_notification(event_name, initialized_db): # Create the notification event. @@ -19,3 +27,164 @@ def test_build_notification(event_name, initialized_db): found.get_level(sample_data, notification_data) found.get_summary(sample_data, notification_data) found.get_message(sample_data, notification_data) + + +def test_build_emptyjson(): + notification_data = AttrDict({ + 'event_config_dict': None, + }) + + # No build data at all. + assert BuildSuccessEvent().should_perform({}, notification_data) + +def test_build_nofilter(): + notification_data = AttrDict({ + 'event_config_dict': {}, + }) + + # No build data at all. + assert BuildSuccessEvent().should_perform({}, notification_data) + + # With trigger metadata but no ref. + assert BuildSuccessEvent().should_perform({ + 'trigger_metadata': {}, + }, notification_data) + + # With trigger metadata and a ref. + assert BuildSuccessEvent().should_perform({ + 'trigger_metadata': { + 'ref': 'refs/heads/somebranch', + }, + }, notification_data) + + +def test_build_emptyfilter(): + notification_data = AttrDict({ + 'event_config_dict': {"ref-regex": ""}, + }) + + # No build data at all. + assert BuildSuccessEvent().should_perform({}, notification_data) + + # With trigger metadata but no ref. + assert BuildSuccessEvent().should_perform({ + 'trigger_metadata': {}, + }, notification_data) + + # With trigger metadata and a ref. + assert BuildSuccessEvent().should_perform({ + 'trigger_metadata': { + 'ref': 'refs/heads/somebranch', + }, + }, notification_data) + + +def test_build_invalidfilter(): + notification_data = AttrDict({ + 'event_config_dict': {"ref-regex": "]["}, + }) + + # No build data at all. + assert not BuildSuccessEvent().should_perform({}, notification_data) + + # With trigger metadata but no ref. + assert not BuildSuccessEvent().should_perform({ + 'trigger_metadata': {}, + }, notification_data) + + # With trigger metadata and a ref. + assert not BuildSuccessEvent().should_perform({ + 'trigger_metadata': { + 'ref': 'refs/heads/somebranch', + }, + }, notification_data) + + +def test_build_withfilter(): + notification_data = AttrDict({ + 'event_config_dict': {"ref-regex": "refs/heads/master"}, + }) + + # No build data at all. + assert not BuildSuccessEvent().should_perform({}, notification_data) + + # With trigger metadata but no ref. + assert not BuildSuccessEvent().should_perform({ + 'trigger_metadata': {}, + }, notification_data) + + # With trigger metadata and a not-matching ref. + assert not BuildSuccessEvent().should_perform({ + 'trigger_metadata': { + 'ref': 'refs/heads/somebranch', + }, + }, notification_data) + + # With trigger metadata and a matching ref. + assert BuildSuccessEvent().should_perform({ + 'trigger_metadata': { + 'ref': 'refs/heads/master', + }, + }, notification_data) + + +def test_build_withwildcardfilter(): + notification_data = AttrDict({ + 'event_config_dict': {"ref-regex": "refs/heads/.+"}, + }) + + # No build data at all. + assert not BuildSuccessEvent().should_perform({}, notification_data) + + # With trigger metadata but no ref. + assert not BuildSuccessEvent().should_perform({ + 'trigger_metadata': {}, + }, notification_data) + + # With trigger metadata and a not-matching ref. + assert not BuildSuccessEvent().should_perform({ + 'trigger_metadata': { + 'ref': 'refs/tags/sometag', + }, + }, notification_data) + + # With trigger metadata and a matching ref. + assert BuildSuccessEvent().should_perform({ + 'trigger_metadata': { + 'ref': 'refs/heads/master', + }, + }, notification_data) + + # With trigger metadata and another matching ref. + assert BuildSuccessEvent().should_perform({ + 'trigger_metadata': { + 'ref': 'refs/heads/somebranch', + }, + }, notification_data) + + +def test_vulnerability_notification_nolevel(): + notification_data = AttrDict({ + 'event_config_dict': {}, + }) + + # No level specified. + assert VulnerabilityFoundEvent().should_perform({}, notification_data) + + +def test_vulnerability_notification_nopvulninfo(): + notification_data = AttrDict({ + 'event_config_dict': {"level": 3}, + }) + + # No vuln info. + assert not VulnerabilityFoundEvent().should_perform({}, notification_data) + + +def test_vulnerability_notification_normal(): + notification_data = AttrDict({ + 'event_config_dict': {"level": 3}, + }) + + info = {"vulnerability": {"priority": "Critical"}} + assert VulnerabilityFoundEvent().should_perform(info, notification_data) diff --git a/notifications/test/test_notifications.py b/notifications/test/test_notifications.py deleted file mode 100644 index a075ce30e..000000000 --- a/notifications/test/test_notifications.py +++ /dev/null @@ -1,171 +0,0 @@ -import pytest - -from notifications.notificationevent import (BuildSuccessEvent, NotificationEvent, - VulnerabilityFoundEvent) -from util.morecollections import AttrDict - - -@pytest.mark.parametrize('event_kind', NotificationEvent.event_names()) -def test_create_notifications(event_kind): - assert NotificationEvent.get_event(event_kind) is not None - - -def test_build_emptyjson(): - notification_data = AttrDict({ - 'event_config_dict': None, - }) - - # No build data at all. - assert BuildSuccessEvent().should_perform({}, notification_data) - -def test_build_nofilter(): - notification_data = AttrDict({ - 'event_config_dict': {}, - }) - - # No build data at all. - assert BuildSuccessEvent().should_perform({}, notification_data) - - # With trigger metadata but no ref. - assert BuildSuccessEvent().should_perform({ - 'trigger_metadata': {}, - }, notification_data) - - # With trigger metadata and a ref. - assert BuildSuccessEvent().should_perform({ - 'trigger_metadata': { - 'ref': 'refs/heads/somebranch', - }, - }, notification_data) - - -def test_build_emptyfilter(): - notification_data = AttrDict({ - 'event_config_dict': {"ref-regex": ""}, - }) - - # No build data at all. - assert BuildSuccessEvent().should_perform({}, notification_data) - - # With trigger metadata but no ref. - assert BuildSuccessEvent().should_perform({ - 'trigger_metadata': {}, - }, notification_data) - - # With trigger metadata and a ref. - assert BuildSuccessEvent().should_perform({ - 'trigger_metadata': { - 'ref': 'refs/heads/somebranch', - }, - }, notification_data) - - -def test_build_invalidfilter(): - notification_data = AttrDict({ - 'event_config_dict': {"ref-regex": "]["}, - }) - - # No build data at all. - assert not BuildSuccessEvent().should_perform({}, notification_data) - - # With trigger metadata but no ref. - assert not BuildSuccessEvent().should_perform({ - 'trigger_metadata': {}, - }, notification_data) - - # With trigger metadata and a ref. - assert not BuildSuccessEvent().should_perform({ - 'trigger_metadata': { - 'ref': 'refs/heads/somebranch', - }, - }, notification_data) - - -def test_build_withfilter(): - notification_data = AttrDict({ - 'event_config_dict': {"ref-regex": "refs/heads/master"}, - }) - - # No build data at all. - assert not BuildSuccessEvent().should_perform({}, notification_data) - - # With trigger metadata but no ref. - assert not BuildSuccessEvent().should_perform({ - 'trigger_metadata': {}, - }, notification_data) - - # With trigger metadata and a not-matching ref. - assert not BuildSuccessEvent().should_perform({ - 'trigger_metadata': { - 'ref': 'refs/heads/somebranch', - }, - }, notification_data) - - # With trigger metadata and a matching ref. - assert BuildSuccessEvent().should_perform({ - 'trigger_metadata': { - 'ref': 'refs/heads/master', - }, - }, notification_data) - - -def test_build_withwildcardfilter(): - notification_data = AttrDict({ - 'event_config_dict': {"ref-regex": "refs/heads/.+"}, - }) - - # No build data at all. - assert not BuildSuccessEvent().should_perform({}, notification_data) - - # With trigger metadata but no ref. - assert not BuildSuccessEvent().should_perform({ - 'trigger_metadata': {}, - }, notification_data) - - # With trigger metadata and a not-matching ref. - assert not BuildSuccessEvent().should_perform({ - 'trigger_metadata': { - 'ref': 'refs/tags/sometag', - }, - }, notification_data) - - # With trigger metadata and a matching ref. - assert BuildSuccessEvent().should_perform({ - 'trigger_metadata': { - 'ref': 'refs/heads/master', - }, - }, notification_data) - - # With trigger metadata and another matching ref. - assert BuildSuccessEvent().should_perform({ - 'trigger_metadata': { - 'ref': 'refs/heads/somebranch', - }, - }, notification_data) - - -def test_vulnerability_notification_nolevel(): - notification_data = AttrDict({ - 'event_config_dict': {}, - }) - - # No level specified. - assert VulnerabilityFoundEvent().should_perform({}, notification_data) - - -def test_vulnerability_notification_nopvulninfo(): - notification_data = AttrDict({ - 'event_config_dict': {"level": 3}, - }) - - # No vuln info. - assert not VulnerabilityFoundEvent().should_perform({}, notification_data) - - -def test_vulnerability_notification_normal(): - notification_data = AttrDict({ - 'event_config_dict': {"level": 3}, - }) - - info = {"vulnerability": {"priority": "Critical"}} - assert VulnerabilityFoundEvent().should_perform(info, notification_data) From 348b544f232c1f7873a81e5931e1445fbd837c47 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Fri, 14 Jul 2017 20:01:14 +0300 Subject: [PATCH 09/12] Notification method tests --- notifications/notificationmethod.py | 12 +- notifications/test/test_notificationmethod.py | 158 ++++++++++++++++++ 2 files changed, 165 insertions(+), 5 deletions(-) create mode 100644 notifications/test/test_notificationmethod.py diff --git a/notifications/notificationmethod.py b/notifications/notificationmethod.py index 2a426cfa6..ff2887976 100644 --- a/notifications/notificationmethod.py +++ b/notifications/notificationmethod.py @@ -57,7 +57,7 @@ class NotificationMethod(object): """ Performs the notification method. - notification_obj: The noticication namedtuple. + notification_obj: The notification namedtuple. event_handler: The NotificationEvent handler. notification_data: The dict of notification data placed in the queue. """ @@ -83,7 +83,9 @@ class QuayNotificationMethod(NotificationMethod): raise CannotValidateNotificationMethodException(err_message) def find_targets(self, repository, config_data): - target_info = config_data['target'] + target_info = config_data.get('target', None) + if not target_info or not target_info.get('kind'): + return (True, 'Missing target', []) if target_info['kind'] == 'user': target = model.user.get_nonrobot_user(target_info['name']) @@ -93,9 +95,9 @@ class QuayNotificationMethod(NotificationMethod): return (True, None, [target]) elif target_info['kind'] == 'org': - target = model.organization.get_organization(target_info['name']) - if not target: - # Just to be safe. + try: + target = model.organization.get_organization(target_info['name']) + except model.organization.InvalidOrganizationException: return (True, 'Unknown organization %s' % target_info['name'], None) # Only repositories under the organization can cause notifications to that org. diff --git a/notifications/test/test_notificationmethod.py b/notifications/test/test_notificationmethod.py new file mode 100644 index 000000000..602cfad91 --- /dev/null +++ b/notifications/test/test_notificationmethod.py @@ -0,0 +1,158 @@ +import pytest + +from mock import patch, Mock +from httmock import urlmatch, HTTMock + +from data import model +from notifications.notificationmethod import (QuayNotificationMethod, EmailMethod, WebhookMethod, + FlowdockMethod, HipchatMethod, SlackMethod, + CannotValidateNotificationMethodException) +from notifications.notificationevent import NotificationEvent +from notifications.models_interface import Repository, Notification + +from test.fixtures import * + +def assert_validated(method, method_config, error_message, namespace_name, repo_name): + if error_message is None: + method.validate(Repository(namespace_name, repo_name), method_config) + else: + with pytest.raises(CannotValidateNotificationMethodException) as ipe: + method.validate(Repository(namespace_name, repo_name), method_config) + assert ipe.value.message == error_message + + +@pytest.mark.parametrize('method_config,error_message', [ + ({}, 'Missing target'), + ({'target': {'name': 'invaliduser', 'kind': 'user'}}, 'Unknown user invaliduser'), + ({'target': {'name': 'invalidorg', 'kind': 'org'}}, 'Unknown organization invalidorg'), + ({'target': {'name': 'invalidteam', 'kind': 'team'}}, 'Unknown team invalidteam'), + + ({'target': {'name': 'devtable', 'kind': 'user'}}, None), + ({'target': {'name': 'buynlarge', 'kind': 'org'}}, None), + ({'target': {'name': 'owners', 'kind': 'team'}}, None), +]) +def test_validate_quay_notification(method_config, error_message, initialized_db): + method = QuayNotificationMethod() + assert_validated(method, method_config, error_message, 'buynlarge', 'orgrepo') + + +@pytest.mark.parametrize('method_config,error_message', [ + ({}, 'Missing e-mail address'), + ({'email': 'a@b.com'}, 'The specified e-mail address is not authorized to receive ' + 'notifications for this repository'), + + ({'email': 'jschorr@devtable.com'}, None), +]) +def test_validate_email(method_config, error_message, initialized_db): + method = EmailMethod() + assert_validated(method, method_config, error_message, 'devtable', 'simple') + + +@pytest.mark.parametrize('method_config,error_message', [ + ({}, 'Missing webhook URL'), + ({'url': 'http://example.com'}, None), +]) +def test_validate_webhook(method_config, error_message, initialized_db): + method = WebhookMethod() + assert_validated(method, method_config, error_message, 'devtable', 'simple') + + +@pytest.mark.parametrize('method_config,error_message', [ + ({}, 'Missing Flowdock API Token'), + ({'flow_api_token': 'sometoken'}, None), +]) +def test_validate_flowdock(method_config, error_message, initialized_db): + method = FlowdockMethod() + assert_validated(method, method_config, error_message, 'devtable', 'simple') + + +@pytest.mark.parametrize('method_config,error_message', [ + ({}, 'Missing Hipchat Room Notification Token'), + ({'notification_token': 'sometoken'}, 'Missing Hipchat Room ID'), + ({'notification_token': 'sometoken', 'room_id': 'foo'}, None), +]) +def test_validate_hipchat(method_config, error_message, initialized_db): + method = HipchatMethod() + assert_validated(method, method_config, error_message, 'devtable', 'simple') + + +@pytest.mark.parametrize('method_config,error_message', [ + ({}, 'Missing Slack Callback URL'), + ({'url': 'http://example.com'}, None), +]) +def test_validate_slack(method_config, error_message, initialized_db): + method = SlackMethod() + assert_validated(method, method_config, error_message, 'devtable', 'simple') + + +@pytest.mark.parametrize('target,expected_users', [ + ({'name': 'devtable', 'kind': 'user'}, ['devtable']), + ({'name': 'buynlarge', 'kind': 'org'}, ['buynlarge']), + ({'name': 'creators', 'kind': 'team'}, ['creator']), +]) +def test_perform_quay_notification(target, expected_users, initialized_db): + repository = Repository('buynlarge', 'orgrepo') + notification = Notification(uuid='fake', event_name='repo_push', method_name='quay', + event_config_dict={}, method_config_dict={'target': target}, + repository=repository) + + event_handler = NotificationEvent.get_event('repo_push') + + sample_data = event_handler.get_sample_data(repository, {}) + + method = QuayNotificationMethod() + method.perform(notification, event_handler, {'event_data': sample_data}) + + # Ensure that the notification was written for all the expected users. + if target['kind'] != 'team': + user = model.user.get_namespace_user(target['name']) + assert len(model.notification.list_notifications(user, kind_name='repo_push')) > 0 + + +def test_perform_email(initialized_db): + repository = Repository('buynlarge', 'orgrepo') + notification = Notification(uuid='fake', event_name='repo_push', method_name='email', + event_config_dict={}, method_config_dict={'email': 'test@example.com'}, + repository=repository) + + event_handler = NotificationEvent.get_event('repo_push') + + sample_data = event_handler.get_sample_data(repository, {}) + + mock = Mock() + def get_mock(*args, **kwargs): + return mock + + with patch('notifications.notificationmethod.Message', get_mock): + method = EmailMethod() + method.perform(notification, event_handler, {'event_data': sample_data, 'performer_data': {}}) + + mock.send.assert_called_once() + + +@pytest.mark.parametrize('method, method_config, netloc', [ + (WebhookMethod, {'url': 'http://testurl'}, 'testurl'), + (FlowdockMethod, {'flow_api_token': 'token'}, 'api.flowdock.com'), + (HipchatMethod, {'notification_token': 'token', 'room_id': 'foo'}, 'api.hipchat.com'), + (SlackMethod, {'url': 'http://example.com'}, 'example.com'), +]) +def test_perform_http_call(method, method_config, netloc, initialized_db): + repository = Repository('buynlarge', 'orgrepo') + notification = Notification(uuid='fake', event_name='repo_push', method_name=method.method_name(), + event_config_dict={}, method_config_dict=method_config, + repository=repository) + + event_handler = NotificationEvent.get_event('repo_push') + + sample_data = event_handler.get_sample_data(repository, {}) + + url_hit = [False] + @urlmatch(netloc=netloc) + def url_handler(_, __): + url_hit[0] = True + return '' + + with HTTMock(url_handler): + method().perform(notification, event_handler, {'event_data': sample_data, 'performer_data': {}}) + + assert url_hit[0] From 543cba352bb93a355a2615d5dc65f74a81df41ec Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Fri, 14 Jul 2017 20:51:08 +0300 Subject: [PATCH 10/12] Add end-to-end notification worker tests for all notification methods --- .../notificationworker/models_interface.py | 2 +- workers/notificationworker/models_pre_oci.py | 9 ++-- .../test/test_notificationworker.py | 51 ++++++++++++++++++- 3 files changed, 56 insertions(+), 6 deletions(-) diff --git a/workers/notificationworker/models_interface.py b/workers/notificationworker/models_interface.py index c0caefa94..f2c54902b 100644 --- a/workers/notificationworker/models_interface.py +++ b/workers/notificationworker/models_interface.py @@ -40,7 +40,7 @@ class NotificationWorkerDataInterface(object): pass @abstractmethod - def create_notification_for_testing(self, target_username): + def create_notification_for_testing(self, target_username, method_name=None, method_config=None): """ Creates a notification for testing. """ pass diff --git a/workers/notificationworker/models_pre_oci.py b/workers/notificationworker/models_pre_oci.py index 9e77f2e55..388c698f7 100644 --- a/workers/notificationworker/models_pre_oci.py +++ b/workers/notificationworker/models_pre_oci.py @@ -29,16 +29,17 @@ class PreOCIModel(NotificationWorkerDataInterface): def increment_notification_failure_count(self, notification): model.notification.increment_notification_failure_count(notification.uuid) - def create_notification_for_testing(self, target_username): + def create_notification_for_testing(self, target_username, method_name='quay_notification', + method_config=None): repo = model.repository.get_repository('devtable', 'simple') - method_data = { + method_data = method_config or { 'target': { 'kind': 'user', 'name': target_username, } } - notification = model.notification.create_repo_notification(repo, 'build_success', - 'quay_notification', method_data, {}) + notification = model.notification.create_repo_notification(repo, 'repo_push', + method_name, method_data, {}) return notification.uuid def user_has_local_notifications(self, target_username): diff --git a/workers/notificationworker/test/test_notificationworker.py b/workers/notificationworker/test/test_notificationworker.py index 92d218aca..76fd47532 100644 --- a/workers/notificationworker/test/test_notificationworker.py +++ b/workers/notificationworker/test/test_notificationworker.py @@ -1,10 +1,20 @@ +import pytest + +from mock import patch, Mock +from httmock import urlmatch, HTTMock + +from notifications.notificationmethod import (QuayNotificationMethod, EmailMethod, WebhookMethod, + FlowdockMethod, HipchatMethod, SlackMethod, + CannotValidateNotificationMethodException) +from notifications.notificationevent import RepoPushEvent +from notifications.models_interface import Repository from workers.notificationworker.notificationworker import NotificationWorker from test.fixtures import * from workers.notificationworker.models_pre_oci import pre_oci_model as model -def test_basic_notification(initialized_db): +def test_basic_notification_endtoend(initialized_db): # Ensure the public user doesn't have any notifications. assert not model.user_has_local_notifications('public') @@ -21,3 +31,42 @@ def test_basic_notification(initialized_db): # Ensure the notification was handled. assert model.user_has_local_notifications('public') + + +@pytest.mark.parametrize('method,method_config,netloc', [ + (QuayNotificationMethod, {'target': {'name': 'devtable', 'kind': 'user'}}, None), + (EmailMethod, {'email': 'jschorr@devtable.com'}, None), + (WebhookMethod, {'url': 'http://example.com'}, 'example.com'), + (FlowdockMethod, {'flow_api_token': 'sometoken'}, 'api.flowdock.com'), + (HipchatMethod, {'notification_token': 'token', 'room_id': 'foo'}, 'api.hipchat.com'), + (SlackMethod, {'url': 'http://example.com'}, 'example.com'), +]) +def test_notifications(method, method_config, netloc, initialized_db): + url_hit = [False] + @urlmatch(netloc=netloc) + def url_handler(_, __): + url_hit[0] = True + return '' + + mock = Mock() + def get_mock(*args, **kwargs): + return mock + + with patch('notifications.notificationmethod.Message', get_mock): + with HTTMock(url_handler): + # Add a basic build notification. + notification_uuid = model.create_notification_for_testing('public', + method_name=method.method_name(), + method_config=method_config) + event_data = RepoPushEvent().get_sample_data(Repository('devtable', 'simple'), {}) + + # Fire off the queue processing. + worker = NotificationWorker(None) + worker.process_queue_item({ + 'notification_uuid': notification_uuid, + 'event_data': event_data, + 'performer_data': {}, + }) + + if netloc is not None: + assert url_hit[0] From e7d6e60d9777ae47b9bfe3f0deae00f03aaf6c8a Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Wed, 19 Jul 2017 11:05:50 -0400 Subject: [PATCH 11/12] Update for merge and make additional interface improvements --- endpoints/api/repositorynotification.py | 6 +- ...repositorynotification_models_interface.py | 44 +++++----- .../repositorynotification_models_pre_oci.py | 29 ++++--- notifications/__init__.py | 15 ++-- notifications/notificationevent.py | 32 +++---- notifications/notificationhelper.py | 85 ------------------- notifications/notificationmethod.py | 27 +++--- notifications/test/test_notificationevent.py | 3 +- notifications/test/test_notificationmethod.py | 12 ++- .../test/test_notificationworker.py | 2 +- 10 files changed, 85 insertions(+), 170 deletions(-) delete mode 100644 notifications/notificationhelper.py diff --git a/endpoints/api/repositorynotification.py b/endpoints/api/repositorynotification.py index 22634d235..cecff5d2e 100644 --- a/endpoints/api/repositorynotification.py +++ b/endpoints/api/repositorynotification.py @@ -5,9 +5,8 @@ from flask import request from endpoints.api import (RepositoryParamResource, nickname, resource, require_repo_admin, log_action, validate_json_request, request_error, - path_param, disallow_for_app_repositories) + path_param, disallow_for_app_repositories, InvalidRequest) from endpoints.exception import NotFound -from notifications import build_notification_data from notifications.models_interface import Repository from notifications.notificationevent import NotificationEvent from notifications.notificationmethod import (NotificationMethod, @@ -153,9 +152,6 @@ class TestRepositoryNotification(RepositoryParamResource): def post(self, namespace_name, repository_name, uuid): """ Queues a test notification for this repository. """ test_note = model.queue_test_notification(uuid) - event_info = NotificationEvent.get_event(test_note.event_name) - sample_data = event_info.get_sample_data(Repository(namespace, repository), event_config) - if not test_note: raise InvalidRequest("No repository notification found for: %s, %s, %s" % (namespace_name, repository_name, uuid)) diff --git a/endpoints/api/repositorynotification_models_interface.py b/endpoints/api/repositorynotification_models_interface.py index 4d8154735..52c1214ff 100644 --- a/endpoints/api/repositorynotification_models_interface.py +++ b/endpoints/api/repositorynotification_models_interface.py @@ -19,12 +19,12 @@ class RepositoryNotification( """ RepositoryNotification represents a notification for a repository. :type uuid: string - :type event: string - :type method: string - :type config: string - :type title: string + :type event: string + :type method: string + :type config: string + :type title: string :type event_config: string - :type number_of_failures: int + :type number_of_failures: int """ def to_dict(self): try: @@ -53,11 +53,11 @@ class RepoNotificationInterface(object): """ Interface that represents all data store interactions required by the RepositoryNotification API """ - + @abstractmethod def create_repo_notification(self, namespace_name, repository_name, event_name, method_name, method_config, event_config, title=None): """ - + Args: namespace_name: namespace of repository repository_name: name of repository @@ -72,11 +72,11 @@ class RepoNotificationInterface(object): """ pass - + @abstractmethod def list_repo_notifications(self, namespace_name, repository_name, event_name=None): """ - + Args: namespace_name: namespace of repository repository_name: name of repository @@ -86,11 +86,11 @@ class RepoNotificationInterface(object): list(RepositoryNotification) """ pass - + @abstractmethod def get_repo_notification(self, uuid): """ - + Args: uuid: uuid of notification @@ -98,47 +98,47 @@ class RepoNotificationInterface(object): RepositoryNotification or None """ - pass - + pass + @abstractmethod def delete_repo_notification(self, namespace_name, repository_name, uuid): """ - + Args: namespace_name: namespace of repository repository_name: name of repository uuid: uuid of notification - + Returns: RepositoryNotification or None - + """ pass @abstractmethod def reset_notification_number_of_failures(self, namespace_name, repository_name, uuid): """ - + Args: namespace_name: namespace of repository repository_name: name of repository uuid: uuid of notification - + Returns: RepositoryNotification - + """ pass @abstractmethod def queue_test_notification(self, uuid): """ - + Args: uuid: uuid of notification - + Returns: RepositoryNotification or None - + """ pass diff --git a/endpoints/api/repositorynotification_models_pre_oci.py b/endpoints/api/repositorynotification_models_pre_oci.py index e1d4c37e9..8f1a1a4d8 100644 --- a/endpoints/api/repositorynotification_models_pre_oci.py +++ b/endpoints/api/repositorynotification_models_pre_oci.py @@ -4,23 +4,23 @@ from app import notification_queue from data import model from data.model import InvalidNotificationException from endpoints.api.repositorynotification_models_interface import RepoNotificationInterface, RepositoryNotification -from endpoints.notificationevent import NotificationEvent -from endpoints.notificationhelper import build_notification_data +from notifications import build_notification_data +from notifications.notificationevent import NotificationEvent class RepoNotificationPreOCIModel(RepoNotificationInterface): def create_repo_notification(self, namespace_name, repository_name, event_name, method_name, method_config, event_config, title=None): repository = model.repository.get_repository(namespace_name, repository_name) - return self._notification(model.notification.create_repo_notification(repository, - event_name, - method_name, - method_config, - event_config, + return self._notification(model.notification.create_repo_notification(repository, + event_name, + method_name, + method_config, + event_config, title)) def list_repo_notifications(self, namespace_name, repository_name, event_name=None): - return [self._notification(n) + return [self._notification(n) for n in model.notification.list_repo_notifications(namespace_name, repository_name, event_name)] def get_repo_notification(self, uuid): @@ -40,15 +40,17 @@ class RepoNotificationPreOCIModel(RepoNotificationInterface): def reset_notification_number_of_failures(self, namespace_name, repository_name, uuid): return self._notification( model.notification.reset_notification_number_of_failures(namespace_name, repository_name, uuid)) - + def queue_test_notification(self, uuid): try: notification = model.notification.get_repo_notification(uuid) except InvalidNotificationException: return None - + + event_config = json.loads(notification.event_config_json or '{}') event_info = NotificationEvent.get_event(notification.event.name) - sample_data = event_info.get_sample_data(notification) + sample_data = event_info.get_sample_data(notification.repository.namespace_user.username, + notification.repository.name, event_config) notification_data = build_notification_data(notification, sample_data) notification_queue.put([notification.repository.namespace_user.username, notification.uuid, notification.event.name], json.dumps(notification_data)) @@ -58,6 +60,7 @@ class RepoNotificationPreOCIModel(RepoNotificationInterface): def _notification(self, notification): if not notification: return None + return RepositoryNotification(uuid=notification.uuid, title=notification.title, event_name=notification.event.name, @@ -65,7 +68,7 @@ class RepoNotificationPreOCIModel(RepoNotificationInterface): config_json=notification.config_json, event_config_json=notification.event_config_json, number_of_failures=notification.number_of_failures) - - + + pre_oci_model = RepoNotificationPreOCIModel() \ No newline at end of file diff --git a/notifications/__init__.py b/notifications/__init__.py index 37e5bc56b..6264958e9 100644 --- a/notifications/__init__.py +++ b/notifications/__init__.py @@ -10,8 +10,12 @@ from auth.auth_context import get_authenticated_user, get_validated_oauth_token DEFAULT_BATCH_SIZE = 1000 -def build_event_data(repo, extra_data=None, subpage=None): - repo_string = '%s/%s' % (repo.namespace_name, repo.name) +def build_repository_event_data(namespace_name, repo_name, extra_data=None, subpage=None): + """ Builds the basic repository data for an event, including the repository's name, Docker URL + and homepage. If extra_data is specified, it is appended to the dictionary before it is + returned. + """ + repo_string = '%s/%s' % (namespace_name, repo_name) homepage = '%s://%s/repository/%s' % (app.config['PREFERRED_URL_SCHEME'], app.config['SERVER_HOSTNAME'], repo_string) @@ -24,8 +28,8 @@ def build_event_data(repo, extra_data=None, subpage=None): event_data = { 'repository': repo_string, - 'namespace': repo.namespace_name, - 'name': repo.name, + 'namespace': namespace_name, + 'name': repo_name, 'docker_url': '%s/%s' % (app.config['SERVER_HOSTNAME'], repo_string), 'homepage': homepage, } @@ -65,7 +69,8 @@ def notification_batch(batch_size=DEFAULT_BATCH_SIZE): with notification_queue.batch_insert(batch_size) as queue_put: def spawn_notification_batch(repo, event_name, extra_data=None, subpage=None, pathargs=None, performer_data=None): - event_data = build_event_data(repo, extra_data=extra_data, subpage=subpage) + event_data = build_repository_event_data(repo.namespace_name, repo.name, + extra_data=extra_data, subpage=subpage) notifications = model.notification.list_repo_notifications(repo.namespace_name, repo.name, diff --git a/notifications/notificationevent.py b/notifications/notificationevent.py index ecc6a40a5..d7a321aaf 100644 --- a/notifications/notificationevent.py +++ b/notifications/notificationevent.py @@ -3,7 +3,7 @@ import time import re from datetime import datetime -from notifications import build_event_data +from notifications import build_repository_event_data from util.jinjautil import get_template_env from util.secscan import PRIORITY_LEVELS, get_priority_for_index @@ -40,7 +40,7 @@ class NotificationEvent(object): 'notification_data': notification_data }) - def get_sample_data(self, repository, event_config): + def get_sample_data(self, namespace_name, repo_name, event_config): """ Returns sample data for testing the raising of this notification, with an example notification. """ @@ -98,8 +98,8 @@ class RepoPushEvent(NotificationEvent): def get_summary(self, event_data, notification_data): return 'Repository %s updated' % (event_data['repository']) - def get_sample_data(self, repository, event_config): - return build_event_data(repository, { + def get_sample_data(self, namespace_name, repo_name, event_config): + return build_repository_event_data(namespace_name, repo_name, { 'updated_tags': {'latest': 'someimageid', 'foo': 'anotherimage'}, 'pruned_image_count': 3 }) @@ -132,9 +132,9 @@ class VulnerabilityFoundEvent(NotificationEvent): return 'info' - def get_sample_data(self, repository, event_config): + def get_sample_data(self, namespace_name, repo_name, event_config): level = event_config.get(VulnerabilityFoundEvent.CONFIG_LEVEL, 'Critical') - return build_event_data(repository, { + return build_repository_event_data(namespace_name, repo_name, { 'tags': ['latest', 'prod', 'foo', 'bar', 'baz'], 'image': 'some-image-id', 'vulnerability': { @@ -219,9 +219,9 @@ class BuildQueueEvent(BaseBuildEvent): def get_level(self, event_data, notification_data): return 'info' - def get_sample_data(self, repository, event_config): + def get_sample_data(self, namespace_name, repo_name, event_config): build_uuid = 'fake-build-id' - return build_event_data(repository, { + return build_repository_event_data(namespace_name, repo_name, { 'is_manual': False, 'build_id': build_uuid, 'build_name': 'some-fake-build', @@ -257,9 +257,9 @@ class BuildStartEvent(BaseBuildEvent): def get_level(self, event_data, notification_data): return 'info' - def get_sample_data(self, repository, event_config): + def get_sample_data(self, namespace_name, repo_name, event_config): build_uuid = 'fake-build-id' - return build_event_data(repository, { + return build_repository_event_data(namespace_name, repo_name, { 'build_id': build_uuid, 'build_name': 'some-fake-build', 'docker_tags': ['latest', 'foo', 'bar'], @@ -284,9 +284,9 @@ class BuildSuccessEvent(BaseBuildEvent): def get_level(self, event_data, notification_data): return 'success' - def get_sample_data(self, repository, event_config): + def get_sample_data(self, namespace_name, repo_name, event_config): build_uuid = 'fake-build-id' - return build_event_data(repository, { + return build_repository_event_data(namespace_name, repo_name, { 'build_id': build_uuid, 'build_name': 'some-fake-build', 'docker_tags': ['latest', 'foo', 'bar'], @@ -312,9 +312,9 @@ class BuildFailureEvent(BaseBuildEvent): def get_level(self, event_data, notification_data): return 'error' - def get_sample_data(self, repository, event_config): + def get_sample_data(self, namespace_name, repo_name, event_config): build_uuid = 'fake-build-id' - return build_event_data(repository, { + return build_repository_event_data(namespace_name, repo_name, { 'build_id': build_uuid, 'build_name': 'some-fake-build', 'docker_tags': ['latest', 'foo', 'bar'], @@ -351,9 +351,9 @@ class BuildCancelledEvent(BaseBuildEvent): def get_level(self, event_data, notification_data): return 'info' - def get_sample_data(self, repository, event_config): + def get_sample_data(self, namespace_name, repo_name, event_config): build_uuid = 'fake-build-id' - return build_event_data(repository, { + return build_repository_event_data(namespace_name, repo_name, { 'build_id': build_uuid, 'build_name': 'some-fake-build', 'docker_tags': ['latest', 'foo', 'bar'], diff --git a/notifications/notificationhelper.py b/notifications/notificationhelper.py deleted file mode 100644 index 18e313b8a..000000000 --- a/notifications/notificationhelper.py +++ /dev/null @@ -1,85 +0,0 @@ -import json - -from contextlib import contextmanager - -from app import app, notification_queue -from data import model -from auth.auth_context import get_authenticated_user, get_validated_oauth_token -from endpoints.notificationmethod import _get_namespace_name_from - - -DEFAULT_BATCH_SIZE = 1000 - - -def build_event_data(repo, extra_data=None, subpage=None): - repo_string = '%s/%s' % (_get_namespace_name_from(repo), repo.name) - homepage = '%s://%s/repository/%s' % (app.config['PREFERRED_URL_SCHEME'], - app.config['SERVER_HOSTNAME'], - repo_string) - - if subpage: - if not subpage.startswith('/'): - subpage = '/' + subpage - - homepage = homepage + subpage - - event_data = { - 'repository': repo_string, - 'namespace': _get_namespace_name_from(repo), - 'name': repo.name, - 'docker_url': '%s/%s' % (app.config['SERVER_HOSTNAME'], repo_string), - 'homepage': homepage, - } - - event_data.update(extra_data or {}) - return event_data - -def build_notification_data(notification, event_data, performer_data=None): - if not performer_data: - performer_data = {} - - oauth_token = get_validated_oauth_token() - if oauth_token: - performer_data['oauth_token_id'] = oauth_token.id - performer_data['oauth_token_application_id'] = oauth_token.application.client_id - performer_data['oauth_token_application'] = oauth_token.application.name - - performer_user = get_authenticated_user() - if performer_user: - performer_data['entity_id'] = performer_user.id - performer_data['entity_name'] = performer_user.username - - return { - 'notification_uuid': notification.uuid, - 'event_data': event_data, - 'performer_data': performer_data, - } - - -@contextmanager -def notification_batch(batch_size=DEFAULT_BATCH_SIZE): - """ - Context manager implementation which returns a target callable with the same signature - as spawn_notification. When the the context block exits the notifications generated by - the callable will be bulk inserted into the queue with the specified batch size. - """ - with notification_queue.batch_insert(batch_size) as queue_put: - def spawn_notification_batch(repo, event_name, extra_data=None, subpage=None, pathargs=None, - performer_data=None): - event_data = build_event_data(repo, extra_data=extra_data, subpage=subpage) - - notifications = model.notification.list_repo_notifications(repo.namespace_name, - repo.name, - event_name=event_name) - path = [repo.namespace_name, repo.name, event_name] + (pathargs or []) - for notification in list(notifications): - notification_data = build_notification_data(notification, event_data, performer_data) - queue_put(path, json.dumps(notification_data)) - - yield spawn_notification_batch - - -def spawn_notification(repo, event_name, extra_data=None, subpage=None, pathargs=None, - performer_data=None): - with notification_batch(1) as batch_spawn: - batch_spawn(repo, event_name, extra_data, subpage, pathargs, performer_data) diff --git a/notifications/notificationmethod.py b/notifications/notificationmethod.py index ff2887976..73ac9d77e 100644 --- a/notifications/notificationmethod.py +++ b/notifications/notificationmethod.py @@ -45,7 +45,7 @@ class NotificationMethod(object): """ raise NotImplementedError - def validate(self, repository, config_data): + def validate(self, namespace_name, repo_name, config_data): """ Validates that the notification can be created with the given data. Throws a CannotValidateNotificationMethodException on failure. @@ -77,12 +77,12 @@ class QuayNotificationMethod(NotificationMethod): def method_name(cls): return 'quay_notification' - def validate(self, repository, config_data): - _, err_message, _ = self.find_targets(repository, config_data) + def validate(self, namespace_name, repo_name, config_data): + _, err_message, _ = self.find_targets(namespace_name, config_data) if err_message: raise CannotValidateNotificationMethodException(err_message) - def find_targets(self, repository, config_data): + def find_targets(self, namespace_name, config_data): target_info = config_data.get('target', None) if not target_info or not target_info.get('kind'): return (True, 'Missing target', []) @@ -101,7 +101,7 @@ class QuayNotificationMethod(NotificationMethod): return (True, 'Unknown organization %s' % target_info['name'], None) # Only repositories under the organization can cause notifications to that org. - if target_info['name'] != repository.namespace_name: + if target_info['name'] != namespace_name: return (False, 'Organization name must match repository namespace') return (True, None, [target]) @@ -109,7 +109,7 @@ class QuayNotificationMethod(NotificationMethod): # Lookup the team. org_team = None try: - org_team = model.team.get_organization_team(repository.namespace_name, target_info['name']) + org_team = model.team.get_organization_team(namespace_name, target_info['name']) except model.InvalidTeamException: # Probably deleted. return (True, 'Unknown team %s' % target_info['name'], None) @@ -125,7 +125,7 @@ class QuayNotificationMethod(NotificationMethod): # Lookup the target user or team to which we'll send the notification. config_data = notification_obj.method_config_dict - status, err_message, target_users = self.find_targets(repository, config_data) + status, err_message, target_users = self.find_targets(repository.namespace_name, config_data) if not status: raise NotificationMethodPerformException(err_message) @@ -140,13 +140,12 @@ class EmailMethod(NotificationMethod): def method_name(cls): return 'email' - def validate(self, repository, config_data): + def validate(self, namespace_name, repo_name, config_data): email = config_data.get('email', '') if not email: raise CannotValidateNotificationMethodException('Missing e-mail address') - record = model.repository.get_email_authorized_for_repo(repository.namespace_name, - repository.name, email) + record = model.repository.get_email_authorized_for_repo(namespace_name, repo_name, email) if not record or not record.confirmed: raise CannotValidateNotificationMethodException('The specified e-mail address ' 'is not authorized to receive ' @@ -175,7 +174,7 @@ class WebhookMethod(NotificationMethod): def method_name(cls): return 'webhook' - def validate(self, repository, config_data): + def validate(self, namespace_name, repo_name, config_data): url = config_data.get('url', '') if not url: raise CannotValidateNotificationMethodException('Missing webhook URL') @@ -212,7 +211,7 @@ class FlowdockMethod(NotificationMethod): def method_name(cls): return 'flowdock' - def validate(self, repository, config_data): + def validate(self, namespace_name, repo_name, config_data): token = config_data.get('flow_api_token', '') if not token: raise CannotValidateNotificationMethodException('Missing Flowdock API Token') @@ -264,7 +263,7 @@ class HipchatMethod(NotificationMethod): def method_name(cls): return 'hipchat' - def validate(self, repository, config_data): + def validate(self, namespace_name, repo_name, config_data): if not config_data.get('notification_token', ''): raise CannotValidateNotificationMethodException('Missing Hipchat Room Notification Token') @@ -375,7 +374,7 @@ class SlackMethod(NotificationMethod): def method_name(cls): return 'slack' - def validate(self, repository, config_data): + def validate(self, namespace_name, repo_name, config_data): if not config_data.get('url', ''): raise CannotValidateNotificationMethodException('Missing Slack Callback URL') diff --git a/notifications/test/test_notificationevent.py b/notifications/test/test_notificationevent.py index 1e9673cca..a3d533820 100644 --- a/notifications/test/test_notificationevent.py +++ b/notifications/test/test_notificationevent.py @@ -1,6 +1,5 @@ import pytest -from notifications.models_interface import Repository from notifications.notificationevent import (BuildSuccessEvent, NotificationEvent, VulnerabilityFoundEvent) from util.morecollections import AttrDict @@ -17,7 +16,7 @@ def test_create_notifications(event_kind): def test_build_notification(event_name, initialized_db): # Create the notification event. found = NotificationEvent.get_event(event_name) - sample_data = found.get_sample_data(Repository('foo', 'bar'), {'level': 'low'}) + sample_data = found.get_sample_data('foo', 'bar', {'level': 'low'}) # Make sure all calls succeed. notification_data = { diff --git a/notifications/test/test_notificationmethod.py b/notifications/test/test_notificationmethod.py index 602cfad91..8b6eeeef7 100644 --- a/notifications/test/test_notificationmethod.py +++ b/notifications/test/test_notificationmethod.py @@ -14,10 +14,10 @@ from test.fixtures import * def assert_validated(method, method_config, error_message, namespace_name, repo_name): if error_message is None: - method.validate(Repository(namespace_name, repo_name), method_config) + method.validate(namespace_name, repo_name, method_config) else: with pytest.raises(CannotValidateNotificationMethodException) as ipe: - method.validate(Repository(namespace_name, repo_name), method_config) + method.validate(namespace_name, repo_name, method_config) assert ipe.value.message == error_message @@ -98,7 +98,7 @@ def test_perform_quay_notification(target, expected_users, initialized_db): event_handler = NotificationEvent.get_event('repo_push') - sample_data = event_handler.get_sample_data(repository, {}) + sample_data = event_handler.get_sample_data(repository.namespace_name, repository.name, {}) method = QuayNotificationMethod() method.perform(notification, event_handler, {'event_data': sample_data}) @@ -116,8 +116,7 @@ def test_perform_email(initialized_db): repository=repository) event_handler = NotificationEvent.get_event('repo_push') - - sample_data = event_handler.get_sample_data(repository, {}) + sample_data = event_handler.get_sample_data(repository.namespace_name, repository.name, {}) mock = Mock() def get_mock(*args, **kwargs): @@ -143,8 +142,7 @@ def test_perform_http_call(method, method_config, netloc, initialized_db): repository=repository) event_handler = NotificationEvent.get_event('repo_push') - - sample_data = event_handler.get_sample_data(repository, {}) + sample_data = event_handler.get_sample_data(repository.namespace_name, repository.name, {}) url_hit = [False] @urlmatch(netloc=netloc) diff --git a/workers/notificationworker/test/test_notificationworker.py b/workers/notificationworker/test/test_notificationworker.py index 76fd47532..c414b52e1 100644 --- a/workers/notificationworker/test/test_notificationworker.py +++ b/workers/notificationworker/test/test_notificationworker.py @@ -58,7 +58,7 @@ def test_notifications(method, method_config, netloc, initialized_db): notification_uuid = model.create_notification_for_testing('public', method_name=method.method_name(), method_config=method_config) - event_data = RepoPushEvent().get_sample_data(Repository('devtable', 'simple'), {}) + event_data = RepoPushEvent().get_sample_data('devtable', 'simple', {}) # Fire off the queue processing. worker = NotificationWorker(None) From 48c79003c694ed06a8e604164dab6b50396cdcba Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Wed, 19 Jul 2017 11:08:33 -0400 Subject: [PATCH 12/12] yap --- endpoints/api/repositorynotification.py | 62 ++++++++++--------- ...repositorynotification_models_interface.py | 4 +- .../repositorynotification_models_pre_oci.py | 46 +++++++------- endpoints/v1/index.py | 1 - endpoints/v2/manifest.py | 1 - notifications/__init__.py | 9 ++- notifications/models_interface.py | 1 + 7 files changed, 64 insertions(+), 60 deletions(-) diff --git a/endpoints/api/repositorynotification.py b/endpoints/api/repositorynotification.py index cecff5d2e..c34cbc553 100644 --- a/endpoints/api/repositorynotification.py +++ b/endpoints/api/repositorynotification.py @@ -3,14 +3,14 @@ import logging from flask import request -from endpoints.api import (RepositoryParamResource, nickname, resource, require_repo_admin, - log_action, validate_json_request, request_error, - path_param, disallow_for_app_repositories, InvalidRequest) +from endpoints.api import ( + RepositoryParamResource, nickname, resource, require_repo_admin, log_action, + validate_json_request, request_error, path_param, disallow_for_app_repositories, InvalidRequest) from endpoints.exception import NotFound from notifications.models_interface import Repository from notifications.notificationevent import NotificationEvent -from notifications.notificationmethod import (NotificationMethod, - CannotValidateNotificationMethodException) +from notifications.notificationmethod import ( + NotificationMethod, CannotValidateNotificationMethodException) from endpoints.api.repositorynotification_models_pre_oci import pre_oci_model as model logger = logging.getLogger(__name__) @@ -69,17 +69,16 @@ class RepositoryNotificationList(RepositoryParamResource): raise request_error(message=ex.message) new_notification = model.create_repo_notification(namespace_name, repository_name, - parsed['event'], - parsed['method'], - parsed['config'], - parsed['eventConfig'], + parsed['event'], parsed['method'], + parsed['config'], parsed['eventConfig'], parsed.get('title')) - log_action('add_repo_notification', namespace_name, - {'repo': repository_name, 'namespace': namespace_name, - 'notification_id': new_notification.uuid, - 'event': new_notification.event_name, 'method': new_notification.method_name}, - repo_name=repository_name) + log_action('add_repo_notification', namespace_name, { + 'repo': repository_name, + 'namespace': namespace_name, + 'notification_id': new_notification.uuid, + 'event': new_notification.event_name, + 'method': new_notification.method_name}, repo_name=repository_name) return new_notification.to_dict(), 201 @require_repo_admin @@ -88,9 +87,7 @@ class RepositoryNotificationList(RepositoryParamResource): def get(self, namespace_name, repository_name): """ List the notifications for the specified repository. """ notifications = model.list_repo_notifications(namespace_name, repository_name) - return { - 'notifications': [n.to_dict() for n in notifications] - } + return {'notifications': [n.to_dict() for n in notifications]} @resource('/v1/repository//notification/') @@ -98,6 +95,7 @@ class RepositoryNotificationList(RepositoryParamResource): @path_param('uuid', 'The UUID of the notification') class RepositoryNotification(RepositoryParamResource): """ Resource for dealing with specific notifications. """ + @require_repo_admin @nickname('getRepoNotification') @disallow_for_app_repositories @@ -115,12 +113,15 @@ class RepositoryNotification(RepositoryParamResource): """ Deletes the specified notification. """ deleted = model.delete_repo_notification(namespace_name, repository_name, uuid) if not deleted: - raise InvalidRequest("No repository notification found for: %s, %s, %s" % (namespace_name, repository_name, uuid)) + raise InvalidRequest("No repository notification found for: %s, %s, %s" % + (namespace_name, repository_name, uuid)) - log_action('delete_repo_notification', namespace_name, - {'repo': repository_name, 'namespace': namespace_name, 'notification_id': uuid, - 'event': deleted.event_name, 'method': deleted.method_name}, - repo_name=repository_name) + log_action('delete_repo_notification', namespace_name, { + 'repo': repository_name, + 'namespace': namespace_name, + 'notification_id': uuid, + 'event': deleted.event_name, + 'method': deleted.method_name}, repo_name=repository_name) return 'No Content', 204 @@ -131,12 +132,15 @@ class RepositoryNotification(RepositoryParamResource): """ Resets repository notification to 0 failures. """ reset = model.reset_notification_number_of_failures(namespace_name, repository_name, uuid) if not reset: - raise InvalidRequest("No repository notification found for: %s, %s, %s" % (namespace_name, repository_name, uuid)) + raise InvalidRequest("No repository notification found for: %s, %s, %s" % + (namespace_name, repository_name, uuid)) - log_action('reset_repo_notification', namespace_name, - {'repo': repository_name, 'namespace': namespace_name, 'notification_id': uuid, - 'event': reset.event_name, 'method': reset.method_name}, - repo_name=repository_name) + log_action('reset_repo_notification', namespace_name, { + 'repo': repository_name, + 'namespace': namespace_name, + 'notification_id': uuid, + 'event': reset.event_name, + 'method': reset.method_name}, repo_name=repository_name) return 'No Content', 204 @@ -146,6 +150,7 @@ class RepositoryNotification(RepositoryParamResource): @path_param('uuid', 'The UUID of the notification') class TestRepositoryNotification(RepositoryParamResource): """ Resource for queuing a test of a notification. """ + @require_repo_admin @nickname('testRepoNotification') @disallow_for_app_repositories @@ -153,6 +158,7 @@ class TestRepositoryNotification(RepositoryParamResource): """ Queues a test notification for this repository. """ test_note = model.queue_test_notification(uuid) if not test_note: - raise InvalidRequest("No repository notification found for: %s, %s, %s" % (namespace_name, repository_name, uuid)) + raise InvalidRequest("No repository notification found for: %s, %s, %s" % + (namespace_name, repository_name, uuid)) return {}, 200 diff --git a/endpoints/api/repositorynotification_models_interface.py b/endpoints/api/repositorynotification_models_interface.py index 52c1214ff..ed0ebd2f7 100644 --- a/endpoints/api/repositorynotification_models_interface.py +++ b/endpoints/api/repositorynotification_models_interface.py @@ -26,6 +26,7 @@ class RepositoryNotification( :type event_config: string :type number_of_failures: int """ + def to_dict(self): try: config = json.loads(self.config_json) @@ -55,7 +56,8 @@ class RepoNotificationInterface(object): """ @abstractmethod - def create_repo_notification(self, namespace_name, repository_name, event_name, method_name, method_config, event_config, title=None): + def create_repo_notification(self, namespace_name, repository_name, event_name, method_name, + method_config, event_config, title=None): """ Args: diff --git a/endpoints/api/repositorynotification_models_pre_oci.py b/endpoints/api/repositorynotification_models_pre_oci.py index 8f1a1a4d8..b3edf43ae 100644 --- a/endpoints/api/repositorynotification_models_pre_oci.py +++ b/endpoints/api/repositorynotification_models_pre_oci.py @@ -3,25 +3,25 @@ import json from app import notification_queue from data import model from data.model import InvalidNotificationException -from endpoints.api.repositorynotification_models_interface import RepoNotificationInterface, RepositoryNotification +from endpoints.api.repositorynotification_models_interface import (RepoNotificationInterface, + RepositoryNotification) from notifications import build_notification_data from notifications.notificationevent import NotificationEvent class RepoNotificationPreOCIModel(RepoNotificationInterface): - - def create_repo_notification(self, namespace_name, repository_name, event_name, method_name, method_config, event_config, title=None): + def create_repo_notification(self, namespace_name, repository_name, event_name, method_name, + method_config, event_config, title=None): repository = model.repository.get_repository(namespace_name, repository_name) - return self._notification(model.notification.create_repo_notification(repository, - event_name, - method_name, - method_config, - event_config, - title)) + return self._notification( + model.notification.create_repo_notification(repository, event_name, method_name, + method_config, event_config, title)) def list_repo_notifications(self, namespace_name, repository_name, event_name=None): - return [self._notification(n) - for n in model.notification.list_repo_notifications(namespace_name, repository_name, event_name)] + return [ + self._notification(n) + for n in model.notification.list_repo_notifications(namespace_name, repository_name, + event_name)] def get_repo_notification(self, uuid): try: @@ -39,7 +39,8 @@ class RepoNotificationPreOCIModel(RepoNotificationInterface): def reset_notification_number_of_failures(self, namespace_name, repository_name, uuid): return self._notification( - model.notification.reset_notification_number_of_failures(namespace_name, repository_name, uuid)) + model.notification.reset_notification_number_of_failures(namespace_name, repository_name, + uuid)) def queue_test_notification(self, uuid): try: @@ -52,23 +53,20 @@ class RepoNotificationPreOCIModel(RepoNotificationInterface): sample_data = event_info.get_sample_data(notification.repository.namespace_user.username, notification.repository.name, event_config) notification_data = build_notification_data(notification, sample_data) - notification_queue.put([notification.repository.namespace_user.username, notification.uuid, - notification.event.name], json.dumps(notification_data)) + notification_queue.put([ + notification.repository.namespace_user.username, notification.uuid, notification.event.name], + json.dumps(notification_data)) return self._notification(notification) - def _notification(self, notification): if not notification: return None - return RepositoryNotification(uuid=notification.uuid, - title=notification.title, - event_name=notification.event.name, - method_name=notification.method.name, - config_json=notification.config_json, - event_config_json=notification.event_config_json, - number_of_failures=notification.number_of_failures) + return RepositoryNotification( + uuid=notification.uuid, title=notification.title, event_name=notification.event.name, + method_name=notification.method.name, config_json=notification.config_json, + event_config_json=notification.event_config_json, + number_of_failures=notification.number_of_failures) - -pre_oci_model = RepoNotificationPreOCIModel() \ No newline at end of file +pre_oci_model = RepoNotificationPreOCIModel() diff --git a/endpoints/v1/index.py b/endpoints/v1/index.py index 3e34498f4..07aa00e67 100644 --- a/endpoints/v1/index.py +++ b/endpoints/v1/index.py @@ -14,7 +14,6 @@ from auth.permissions import ( CreateRepositoryPermission, repository_read_grant, repository_write_grant) from auth.signedgrant import generate_signed_token from endpoints.decorators import anon_protect, anon_allowed, parse_repository_name -from endpoints.notificationhelper import spawn_notification from endpoints.v1 import v1_bp from endpoints.v1.models_pre_oci import pre_oci_model as model from notifications import spawn_notification diff --git a/endpoints/v2/manifest.py b/endpoints/v2/manifest.py index 1ef0646f5..c5e33662b 100644 --- a/endpoints/v2/manifest.py +++ b/endpoints/v2/manifest.py @@ -10,7 +10,6 @@ from app import docker_v2_signing_key, app, metric_queue from auth.registry_jwt_auth import process_registry_jwt_auth from digest import digest_tools from endpoints.decorators import anon_protect, parse_repository_name -from endpoints.notificationhelper import spawn_notification from endpoints.v2 import v2_bp, require_repo_read, require_repo_write from endpoints.v2.models_interface import Label from endpoints.v2.models_pre_oci import data_model as model diff --git a/notifications/__init__.py b/notifications/__init__.py index 6264958e9..04903167d 100644 --- a/notifications/__init__.py +++ b/notifications/__init__.py @@ -6,7 +6,6 @@ from app import app, notification_queue from data import model from auth.auth_context import get_authenticated_user, get_validated_oauth_token - DEFAULT_BATCH_SIZE = 1000 @@ -17,8 +16,7 @@ def build_repository_event_data(namespace_name, repo_name, extra_data=None, subp """ repo_string = '%s/%s' % (namespace_name, repo_name) homepage = '%s://%s/repository/%s' % (app.config['PREFERRED_URL_SCHEME'], - app.config['SERVER_HOSTNAME'], - repo_string) + app.config['SERVER_HOSTNAME'], repo_string) if subpage: if not subpage.startswith('/'): @@ -37,6 +35,7 @@ def build_repository_event_data(namespace_name, repo_name, extra_data=None, subp event_data.update(extra_data or {}) return event_data + def build_notification_data(notification, event_data, performer_data=None): if not performer_data: performer_data = {} @@ -67,13 +66,13 @@ def notification_batch(batch_size=DEFAULT_BATCH_SIZE): the callable will be bulk inserted into the queue with the specified batch size. """ with notification_queue.batch_insert(batch_size) as queue_put: + def spawn_notification_batch(repo, event_name, extra_data=None, subpage=None, pathargs=None, performer_data=None): event_data = build_repository_event_data(repo.namespace_name, repo.name, extra_data=extra_data, subpage=subpage) - notifications = model.notification.list_repo_notifications(repo.namespace_name, - repo.name, + notifications = model.notification.list_repo_notifications(repo.namespace_name, repo.name, event_name=event_name) path = [repo.namespace_name, repo.name, event_name] + (pathargs or []) for notification in list(notifications): diff --git a/notifications/models_interface.py b/notifications/models_interface.py index 4c5dac3bd..734977c81 100644 --- a/notifications/models_interface.py +++ b/notifications/models_interface.py @@ -1,5 +1,6 @@ from collections import namedtuple + class Repository(namedtuple('Repository', ['namespace_name', 'name'])): """ Repository represents a repository.