Merge pull request #2678 from Uninett/pr/johanna-blacklisted-reason

 Add blacklisted reason field to alert sender

bin/alertengine.py

@@ -51,7 +51,7 @@
 
 # These have to be imported after the envrionment is setup
 from django.db import DatabaseError, connection
-from nav.alertengine.base import check_alerts
+from nav.alertengine.base import check_alerts, clear_blacklisted_status_of_alert_senders
 from nav.config import NAV_CONFIG
 
 #
@@ -140,6 +140,8 @@ def main():
     signal.signal(signal.SIGTERM, signalhandler)
     signal.signal(signal.SIGINT, signalhandler)
 
+    clear_blacklisted_status_of_alert_senders()
+
     # Loop forever
     _logger.info('Starting alertengine loop.')
     while True:

python/nav/alertengine/base.py

@@ -29,6 +29,7 @@
 from nav.models.profiles import (
     Account,
     AccountAlertQueue,
+    AlertSender,
     AlertSubscription,
     AlertAddress,
     FilterGroup,
@@ -649,3 +650,14 @@ def check_alert_against_filtergroupcontents(alert, filtergroupcontents, atype):
             )
 
     return matches
+
+
+def clear_blacklisted_status_of_alert_senders():
+    blacklisted_alert_senders = AlertSender.objects.exclude(
+        blacklisted_reason__isnull=True
+    )
+    for sender in blacklisted_alert_senders:
+        sender.blacklisted_reason = None
+    AlertSender.objects.bulk_update(
+        objs=blacklisted_alert_senders, fields=["blacklisted_reason"]
+    )

python/nav/alertengine/dispatchers/__init__.py

@@ -101,6 +101,13 @@ class FatalDispatcherException(DispatcherException):
     pass
 
 
+class InvalidAlertAddressError(Exception):
+    """Raised when an alert address is invalid, which is determined by the
+    is_valid_address method of each dispatcher"""
+
+    pass
+
+
 def is_valid_email(address):
     """Validates a string as an e-mail address"""
     try:

python/nav/models/profiles.py

@@ -35,8 +35,11 @@
 import nav.buildconf
 import nav.pwhash
 from nav.config import getconfig as get_alertengine_config
-from nav.alertengine.dispatchers import DispatcherException
-from nav.alertengine.dispatchers import FatalDispatcherException
+from nav.alertengine.dispatchers import (
+    DispatcherException,
+    FatalDispatcherException,
+    InvalidAlertAddressError,
+)
 
 from nav.models.event import AlertQueue, AlertType, EventType
 from nav.models.manage import Arp, Cam, Category, Device, Location
@@ -429,6 +432,24 @@ class Meta(object):
     def __str__(self):
         return self.type.scheme() + self.address
 
+    def has_valid_address(self):
+        from nav.alertengine.dispatchers.email_dispatcher import Email
+        from nav.alertengine.dispatchers.slack_dispatcher import Slack
+        from nav.alertengine.dispatchers.sms_dispatcher import Sms
+
+        if not self.type.supported:
+            return False
+        elif self.type.handler == 'sms':
+            if not Sms.is_valid_address(self.address):
+                return False
+        elif self.type.handler == 'email':
+            if not Email.is_valid_address(self.address):
+                return False
+        elif self.type.handler == 'slack':
+            if not Slack.is_valid_address(self.address):
+                return False
+        return True
+
     @transaction.atomic
     def send(self, alert, subscription):
         """Handles sending of alerts to with defined alert notification types
@@ -440,10 +461,10 @@ def send(self, alert, subscription):
         # Determine the right language for the user.
         lang = self.account.preferences.get(Account.PREFERENCE_KEY_LANGUAGE, 'en')
 
-        if not (self.address or '').strip():
+        if not self.has_valid_address():
             _logger.error(
-                'Ignoring alert %d (%s: %s)! Account %s does not have an '
-                'address set for the alertaddress with id %d, this needs '
+                'Ignoring alert %d (%s: %s)! Account %s does not have a '
+                'valid address for the alertaddress with id %d, this needs '
                 'to be fixed before the user will recieve any alerts.',
                 alert.id,
                 alert,
@@ -452,15 +473,15 @@ def send(self, alert, subscription):
                 self.id,
             )
 
-            return True
+            raise InvalidAlertAddressError
 
-        if self.type.is_blacklisted():
+        if self.type.blacklisted_reason:
             _logger.debug(
                 'Not sending alert %s to %s as handler %s is blacklisted: %s',
                 alert.id,
                 self.address,
                 self.type,
-                self.type.blacklist_reason(),
+                self.type.blacklisted_reason,
             )
             return False
 
@@ -497,7 +518,7 @@ def send(self, alert, subscription):
             _logger.exception(
                 'Unhandled error from %s (the handler has been blacklisted)', self.type
             )
-            self.type.blacklist(error)
+            self.type.blacklist(str(error))
             return False
 
         return True
@@ -509,8 +530,8 @@ class AlertSender(models.Model):
     name = models.CharField(max_length=100)
     handler = models.CharField(max_length=100)
     supported = models.BooleanField(default=True)
+    blacklisted_reason = models.CharField(max_length=100, blank=True)
 
-    _blacklist = {}
     _handlers = {}
 
     EMAIL = u'Email'
@@ -558,15 +579,8 @@ def _load_dispatcher_class(self):
 
     def blacklist(self, reason=None):
         """Blacklists this sender/medium from further alert dispatch."""
-        self.__class__._blacklist[self.handler] = reason
-
-    def is_blacklisted(self):
-        """Gets the blacklist status of this sender/medium."""
-        return self.handler in self.__class__._blacklist
-
-    def blacklist_reason(self):
-        """Gets the reason for a blacklist for this sender/medium"""
-        return self.__class__._blacklist.get(self.handler, 'Unknown reason')
+        self.blacklisted_reason = reason
+        self.save()
 
     def scheme(self):
         return self.SCHEMES.get(self.name, u'')
@@ -1432,7 +1446,7 @@ def send(self):
 
             super(AccountAlertQueue, self).delete()
             return False
-        except FatalDispatcherException:
+        except (FatalDispatcherException, InvalidAlertAddressError):
             self.delete()
             return False
 

python/nav/models/sql/changes/sc.05.07.0002.sql

@@ -0,0 +1 @@
+ALTER TABLE alertsender ADD COLUMN blacklisted_reason VARCHAR; 

tests/integration/models/alert_test.py

@@ -0,0 +1,148 @@
+from datetime import datetime
+import logging
+from mock import patch
+
+from nav.alertengine.base import clear_blacklisted_status_of_alert_senders
+from nav.alertengine.dispatchers import InvalidAlertAddressError
+from nav.models.profiles import (
+    Account,
+    AccountAlertQueue,
+    AlertAddress,
+    AlertProfile,
+    AlertSender,
+    AlertSubscription,
+    FilterGroup,
+    TimePeriod,
+)
+from nav.models.event import AlertQueue, Subsystem
+
+import pytest
+
+
+def test_delete_alert_subscription(db, alert, alertsub, account_alert_queue):
+    alertsub.delete()
+    assert not AccountAlertQueue.objects.filter(pk=account_alert_queue.pk).exists()
+    assert not AlertQueue.objects.filter(pk=alert.pk).exists()
+
+
+def test_sending_alert_to_alert_address_with_invalid_address_will_raise_error(
+    db, alert_address, alert, alertsub
+):
+    with pytest.raises(InvalidAlertAddressError):
+        alert_address.send(alert, alertsub)
+
+
+def test_sending_alert_to_alert_address_with_invalid_address_will_delete_alert_and_fail(
+    db, alert, account_alert_queue
+):
+    assert not account_alert_queue.send()
+    assert not AlertQueue.objects.filter(pk=alert.pk).exists()
+
+
+def test_sending_alert_via_blacklisted_sender_will_fail_but_not_delete_alert(
+    db, alert, alert_address, account_alert_queue
+):
+    alert_address.address = "47474747"
+    alert_address.save()
+    alert_address.type.blacklisted_reason = "This has been blacklisted because of x."
+    alert_address.type.save()
+    assert not account_alert_queue.send()
+    assert AlertQueue.objects.filter(pk=alert.pk).exists()
+
+
+@patch("nav.alertengine.dispatchers.sms_dispatcher.Sms.send")
+def test_error_when_sending_alert_will_blacklist_sender(
+    mocked_send_function, db, alert_address, account_alert_queue
+):
+    exception_reason = "Exception reason"
+    mocked_send_function.side_effect = ValueError(exception_reason)
+    alert_address.address = "47474747"
+    alert_address.save()
+
+    assert not account_alert_queue.send()
+    assert alert_address.type.blacklisted_reason == exception_reason
+
+
+def test_clearing_blacklisted_status_of_alert_senders_will_succeed(db, alert_sender):
+    alert_sender.blacklisted_reason = "This has been blacklisted because of x."
+    clear_blacklisted_status_of_alert_senders()
+    alert_sender.refresh_from_db()
+
+    assert not alert_sender.blacklisted_reason
+
+
+@pytest.fixture
+def account():
+    return Account.objects.get(pk=Account.ADMIN_ACCOUNT)
+
+
+@pytest.fixture
+def alert_address(account, alert_sender):
+    addr = AlertAddress(
+        account=account,
+        type=alert_sender,
+    )
+    addr.save()
+    yield addr
+    if addr.pk:
+        addr.delete()
+
+
+@pytest.fixture
+def alert_profile(account):
+    profile = AlertProfile(account=account)
+    profile.save()
+    yield profile
+    if profile.pk:
+        profile.delete()
+
+
+@pytest.fixture
+def time_period(alert_profile):
+    time_period = TimePeriod(profile=alert_profile)
+    time_period.save()
+    yield time_period
+    if time_period.pk:
+        time_period.delete()
+
+
+@pytest.fixture
+def alertsub(alert_address, time_period):
+    alertsub = AlertSubscription(
+        alert_address=alert_address,
+        time_period=time_period,
+        filter_group=FilterGroup.objects.first(),
+    )
+    alertsub.save()
+    yield alertsub
+    if alertsub.pk:
+        alertsub.delete()
+
+
+@pytest.fixture
+def alert():
+    alert = AlertQueue(
+        source=Subsystem.objects.first(), time=datetime.now(), value=1, severity=3
+    )
+    alert.save()
+    yield alert
+    if alert.pk:
+        alert.delete()
+
+
+@pytest.fixture
+def account_alert_queue(alert, alertsub):
+    account_queue = AccountAlertQueue(alert=alert, subscription=alertsub)
+    account_queue.save()
+    yield account_queue
+    if account_queue.pk:
+        account_queue.delete()
+
+
+@pytest.fixture
+def alert_sender(db):
+    alert_sender = AlertSender.objects.get(name=AlertSender.SMS)
+    yield alert_sender
+    if alert_sender.pk:
+        alert_sender.blacklisted_reason = None
+        alert_sender.save()