From e7d6e60d9777ae47b9bfe3f0deae00f03aaf6c8a Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Wed, 19 Jul 2017 11:05:50 -0400 Subject: [PATCH] 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)