fixup! [R] Route SNS notifications through a Lambda function (#5246)

lambdas/indexer/app.py

@@ -35,7 +35,7 @@
     LogForwardingController,
 )
 from azul.indexer.notification_controller import (
-    NotificationController,
+    MonitoringController,
 )
 from azul.logging import (
     configure_app_logging,
@@ -78,8 +78,8 @@ def health_controller(self):
         return self._controller(HealthController, lambda_name='indexer')
 
     @cached_property
-    def notification_controller(self):
-        return self._controller(NotificationController)
+    def monitoring_controller(self):
+        return self._controller(MonitoringController)
 
     @cached_property
     def index_controller(self) -> IndexController:
@@ -105,9 +105,9 @@ def log_forwarder(self, prefix: str):
             return lambda func: func
 
     @property
-    def monitoring(self):
+    def monitoring_sns_handler(self):
         if config.enable_monitoring:
-            return self.on_sns_message(topic=config.qualified_resource_name('monitoring'))
+            return self.on_sns_message(topic=aws.monitoring_topic_name)
         else:
             return lambda func: func
 
@@ -365,6 +365,6 @@ def forward_s3_logs(event: chalice.app.S3Event):
     app.log_controller.forward_s3_access_logs(event)
 
 
-@app.monitoring
+@app.monitoring_sns_handler
 def notify(event: chalice.app.SNSEvent):
-    app.notification_controller.notify_group(event)
+    app.monitoring_controller.notify_group(event)

src/azul/indexer/notification_controller.py

@@ -7,15 +7,15 @@
     AppController,
 )
 from azul.indexer.notify_service import (
-    AzulEmailNotificationService,
+    EmailService,
 )
 
 
-class NotificationController(AppController):
+class MonitoringController(AppController):
 
     @cached_property
-    def service(self):
-        return AzulEmailNotificationService()
+    def email_service(self):
+        return EmailService()
 
     def notify_group(self, event: chalice.app.SNSEvent) -> None:
-        self.service.notify_group(event.subject, event.message)
+        self.email_service.send_message(event.subject, event.message)

src/azul/indexer/notify_service.py

@@ -2,7 +2,6 @@
 import logging
 
 from azul import (
-    JSON,
     config,
 )
 from azul.deployment import (
@@ -15,34 +14,31 @@
 log = logging.getLogger(__name__)
 
 
-class AzulEmailNotificationService:
+class EmailService:
 
-    def notify_group(self, subject: str, message: str) -> None:
-        log.info('Notifying group of event %r', trunc_ellipses(message, 256))
-        # Try to improve readability by adding indent
+    @property
+    def to_email(self):
+        return config.monitoring_email
+
+    @property
+    def from_email(self):
+        return ' '.join([
+            'Azul',
+            config.deployment_stage,
+            'Monitoring',
+            '<monitoring@' + config.api_lambda_domain('indexer') + '>'
+        ])
+
+    def send_message(self, subject: str, body: str) -> None:
+        log.info('Sending message %r with body %r',
+                 subject, trunc_ellipses(body, 256))
         try:
-            body = json.loads(message)
+            body = json.loads(body)
         except json.decoder.JSONDecodeError:
-            log.warning('Not a JSON serializable event, sending as received.')
-            body = message
+            log.warning('Not a JSON serializable event, sending as is')
         else:
             body = json.dumps(body, indent=4)
-        response = aws.ses.send_email(
-            FromEmailAddress=' '.join([
-                'Azul',
-                config.deployment_stage,
-                'Monitoring',
-                '<monitoring@' + config.api_lambda_domain('indexer') + '>'
-            ]),
-            Destination={
-                'ToAddresses': [config.monitoring_email]
-            },
-            Content=self._content(subject, body)
-        )
-        log.info('Sent notification %r', response['MessageId'])
-
-    def _content(self, subject: str, body: str) -> JSON:
-        return {
+        content = {
             'Simple': {
                 'Subject': {
                     'Data': subject
@@ -54,3 +50,8 @@ def _content(self, subject: str, body: str) -> JSON:
                 }
             }
         }
+        response = aws.ses.send_email(FromEmailAddress=self.from_email,
+                                      Destination=dict(ToAddresses=[self.to_email]),
+                                      Content=content)
+        log.info('Successfully sent message %r, message ID is %r',
+                 subject, response['MessageId'])

terraform/api_gateway.tf.json.template.py

@@ -351,6 +351,7 @@ def for_domain(cls, domain):
                     'function_name': '${aws_lambda_function.indexer_%s.function_name}' % function_name,
                     'maximum_retry_attempts': 0
                 }
+                # REVIEW: see my comments on your other PR that modifies this section
                 for function_name in [
                     *(
                         ('forward_alb_logs', 'forward_s3_logs')
@@ -502,14 +503,15 @@ def for_domain(cls, domain):
                     },
                     **(
                         {
-                            'notify_ses': {
+                            'notify': {
                                 'zone_id': '${data.aws_route53_zone.%s.id}' % zones_by_domain[app.domains[0]].slug,
-                                'name': '_amazonses.' + config.api_lambda_domain(app.name),
+                                'name': '_amazonses.' + app.domains[0],
                                 'type': 'TXT',
                                 'ttl': '600',
                                 'records': ['${aws_ses_domain_identity.notify.verification_token}']
                             }
-                        } if app.name == 'indexer' and config.enable_monitoring else
+                        }
+                        if app.name == 'indexer' and config.enable_monitoring else
                         {}
                     )
                 },
@@ -637,7 +639,7 @@ def for_domain(cls, domain):
                     {
                         'aws_ses_domain_identity': {
                             'notify': {
-                                'domain': config.api_lambda_domain(app.name)
+                                'domain': app.domains[0]
                             }
                         },
                         'aws_ses_identity_policy': {
@@ -650,6 +652,8 @@ def for_domain(cls, domain):
                                         {
                                             'Effect': 'Allow',
                                             'Principal': {
+                                                # REVIEW: Who or what creates the role this ARN is referring to? And
+                                                #         what does the part after the last slash in the ARN signify?
                                                 'AWS': 'arn:aws:sts::'
                                                        + aws.account
                                                        + ':assumed-role/'