separate cbc proxy into separate clients

this is a pretty big and convoluted refactor unfortunately.

Previously:

There was one global `cbc_proxy_client` object in apps. This class has
the information about how to invoke the bt-ee lambda, and handles all
calls to lambda. This includes calls to the canary too (which is a
separate lambda).

The future:

There's one global `cbc_proxy_client`. This knows about the different
provider functions and lambdas, and you'll need to ask this client for a
proxy for your chosen provider. call cbc_proxy_client.get_proxy('ee')`
and it'll return you a proxy that knows what ee's lambda function is,
how to transform any content in a way that is exclusive to ee, and in
future how to parse any response from ee.

The present:

I also cleaned up some duplicate tests.
I'm really not sure about the names of some of these variables - in
particular `cbc_proxy_client` isn't a client - it's more of a java style
factory, where you call a function on it to get the client of your
choice.

app/__init__.py

@@ -23,7 +23,7 @@ from werkzeug.local import LocalProxy
 
 from app.celery.celery import NotifyCelery
 from app.clients import NotificationProviderClients
-from app.clients.cbc_proxy import CBCProxyClient, CBCProxyNoopClient
+from app.clients.cbc_proxy import CBCProxyClient
 from app.clients.document_download import DocumentDownloadClient
 from app.clients.email.aws_ses import AwsSesClient
 from app.clients.email.aws_ses_stub import AwsSesStubClient
@@ -61,7 +61,7 @@ zendesk_client = ZendeskClient()
 statsd_client = StatsdClient()
 redis_store = RedisClient()
 performance_platform_client = PerformancePlatformClient()
-cbc_proxy_client = CBCProxyNoopClient()
+cbc_proxy_client = CBCProxyClient()
 document_download_client = DocumentDownloadClient()
 metrics = GDSMetrics()
 
@@ -114,9 +114,6 @@ def create_app(application):
     performance_platform_client.init_app(application)
     document_download_client.init_app(application)
 
-    global cbc_proxy_client
-    if application.config['CBC_PROXY_AWS_ACCESS_KEY_ID']:
-        cbc_proxy_client = CBCProxyClient()
     cbc_proxy_client.init_app(application)
 
     register_blueprint(application)

app/celery/broadcast_message_tasks.py

@@ -39,8 +39,10 @@ def send_broadcast_provider_message(broadcast_event_id, provider):
         for polygon in broadcast_event.transmitted_areas["simple_polygons"]
     ]
 
+    cbc_proxy_provider_client = cbc_proxy_client.get_proxy(provider)
+
     if broadcast_event.message_type == BroadcastEventMessageType.ALERT:
-        cbc_proxy_client.create_and_send_broadcast(
+        cbc_proxy_provider_client.create_and_send_broadcast(
             identifier=str(broadcast_provider_message.id),
             headline="GOV.UK Notify Broadcast",
             description=broadcast_event.transmitted_content['body'],
@@ -49,7 +51,7 @@ def send_broadcast_provider_message(broadcast_event_id, provider):
             expires=broadcast_event.transmitted_finishes_at_as_cap_datetime_string,
         )
     elif broadcast_event.message_type == BroadcastEventMessageType.UPDATE:
-        cbc_proxy_client.update_and_send_broadcast(
+        cbc_proxy_provider_client.update_and_send_broadcast(
             identifier=str(broadcast_provider_message.id),
             headline="GOV.UK Notify Broadcast",
             description=broadcast_event.transmitted_content['body'],
@@ -59,7 +61,7 @@ def send_broadcast_provider_message(broadcast_event_id, provider):
             expires=broadcast_event.transmitted_finishes_at_as_cap_datetime_string,
         )
     elif broadcast_event.message_type == BroadcastEventMessageType.CANCEL:
-        cbc_proxy_client.cancel_broadcast(
+        cbc_proxy_provider_client.cancel_broadcast(
             identifier=str(broadcast_provider_message.id),
             headline="GOV.UK Notify Broadcast",
             description=broadcast_event.transmitted_content['body'],
@@ -88,4 +90,4 @@ def trigger_link_test(provider):
     identifier = str(uuid.uuid4())
     message = f"Sending a link test to CBC proxy for provider {provider} with ID {identifier}"
     current_app.logger.info(message)
-    cbc_proxy_client.send_link_test(identifier)
+    cbc_proxy_client.get_proxy(provider).send_link_test(identifier)

app/celery/scheduled_tasks.py

@@ -301,7 +301,7 @@ def send_canary_to_cbc_proxy():
     identifier = str(uuid.uuid4())
     message = f"Sending a canary message to CBC proxy with ID {identifier}"
     current_app.logger.info(message)
-    cbc_proxy_client.send_canary(identifier)
+    cbc_proxy_client.get_proxy('canary').send_canary(identifier)
 
 
 @notify_celery.task(name='trigger-link-tests')

app/clients/cbc_proxy.py

@@ -1,6 +1,7 @@
 import json
 
 import boto3
+from flask import current_app
 
 from app.config import BroadcastProvider
 
@@ -16,9 +17,8 @@ from app.config import BroadcastProvider
 # * description is a string which populates the areaDesc field
 # * polygon is a list of lat/long pairs
 #
-# previous_provider_messages is a whitespace separated list of message identifiers
+# previous_provider_messages is a list of previous events (models.py::BroadcastProviderMessage)
-# where each identifier is a previous sent message
+# ie a Cancel message would have a unique event but have the event of
-# ie a Cancel message would have a unique identifier but have the identifier of
 #    the preceeding Alert message in the previous_provider_messages field
 
 
@@ -26,11 +26,31 @@ class CBCProxyException(Exception):
     pass
 
 
-# Noop = no operation
+class CBCProxyClient:
-class CBCProxyNoopClient:
+    _lambda_client = None
 
     def init_app(self, app):
-        pass
+        if app.config.get('CBC_PROXY_AWS_ACCESS_KEY_ID'):
+            self._lambda_client = boto3.client(
+                'lambda',
+                region_name='eu-west-2',
+                aws_access_key_id=app.config['CBC_PROXY_AWS_ACCESS_KEY_ID'],
+                aws_secret_access_key=app.config['CBC_PROXY_AWS_SECRET_ACCESS_KEY'],
+            )
+
+    def get_proxy(self, provider):
+        proxy_classes = {
+            'canary': CBCProxyCanary,
+            BroadcastProvider.EE: CBCProxyEE,
+        }
+        return proxy_classes[provider](self._lambda_client)
+
+
+class CBCProxyClientBase:
+    lambda_name = None
+
+    def __init__(self, lambda_client):
+        self._lambda_client = lambda_client
 
     def send_canary(
         self,
@@ -67,25 +87,17 @@ class CBCProxyNoopClient:
     ):
         pass
 
+    def _invoke_lambda(self, payload):
+        if not self.lambda_name:
+            current_app.logger.warning(
+                '{self.__class__.__name__} tried to send {payload} but cbc proxy aws env vars not set'
+            )
+            return
 
-class CBCProxyClient:
-    provider_function_name_map = {
-        BroadcastProvider.EE: 'bt-ee-1-proxy',
-    }
-
-    def init_app(self, app):
-        self._lambda_client = boto3.client(
-            'lambda',
-            region_name='eu-west-2',
-            aws_access_key_id=app.config['CBC_PROXY_AWS_ACCESS_KEY_ID'],
-            aws_secret_access_key=app.config['CBC_PROXY_AWS_SECRET_ACCESS_KEY'],
-        )
-
-    def _invoke_lambda(self, function_name, payload):
         payload_bytes = bytes(json.dumps(payload), encoding='utf8')
 
         result = self._lambda_client.invoke(
-            FunctionName=function_name,
+            FunctionName=self.lambda_name,
             InvocationType='RequestResponse',
             Payload=payload_bytes,
         )
@@ -98,15 +110,23 @@ class CBCProxyClient:
 
         return result
 
+
+class CBCProxyCanary(CBCProxyClientBase):
+    """
+    The canary is a lambda which tests notify's connectivity to the Cell Broadcast AWS infrastructure. It calls the
+    canary, a specific lambda that does not open a vpn or connect to a provider but just responds from within AWS.
+    """
+    lambda_name = 'canary'
+
     def send_canary(
         self,
         identifier,
     ):
-        """
+        self._invoke_lambda(payload={'identifier': identifier})
-        canary - a specific lambda that does not connect to a provider, but just confirms the connectivity between
+
-        Notify and the CBC proxy AWS account
+
-        """
+class CBCProxyEE(CBCProxyClientBase):
-        self._invoke_lambda(function_name='canary', payload={'identifier': identifier})
+    lambda_name = 'bt-ee-1-proxy'
 
     def send_link_test(
         self,
@@ -118,7 +138,7 @@ class CBCProxyClient:
         """
         payload = {'message_type': 'test', 'identifier': identifier}
 
-        self._invoke_lambda(function_name='bt-ee-1-proxy', payload=payload)
+        self._invoke_lambda(payload=payload)
 
     def create_and_send_broadcast(
         self,
@@ -134,20 +154,4 @@ class CBCProxyClient:
             'sent': sent,
             'expires': expires,
         }
-        self._invoke_lambda(function_name='bt-ee-1-proxy', payload=payload)
+        self._invoke_lambda(payload=payload)
-
-    # We have not implementated updating a broadcast
-    def update_and_send_broadcast(
-        self,
-        identifier, previous_provider_messages, headline, description, areas,
-        sent, expires,
-    ):
-        pass
-
-    # We have not implemented cancelling a broadcast
-    def cancel_broadcast(
-        self,
-        identifier, previous_provider_messages, headline, description, areas,
-        sent, expires,
-    ):
-        pass

tests/app/celery/test_broadcast_message_tasks.py

@@ -48,7 +48,7 @@ def test_send_broadcast_provider_message_sends_data_correctly(mocker, sample_ser
     event = create_broadcast_event(broadcast_message)
 
     mock_create_broadcast = mocker.patch(
-        'app.cbc_proxy_client.create_and_send_broadcast',
+        'app.clients.cbc_proxy.CBCProxyEE.create_and_send_broadcast',
     )
 
     assert event.get_provider_message('ee') is None
@@ -95,7 +95,7 @@ def test_send_broadcast_provider_message_sends_update_with_references(mocker, sa
     update_event = create_broadcast_event(broadcast_message, message_type=BroadcastEventMessageType.UPDATE)
 
     mock_update_broadcast = mocker.patch(
-        'app.cbc_proxy_client.update_and_send_broadcast',
+        'app.clients.cbc_proxy.CBCProxyEE.update_and_send_broadcast',
     )
 
     send_broadcast_provider_message(provider='ee', broadcast_event_id=str(update_event.id))
@@ -140,7 +140,7 @@ def test_send_broadcast_provider_message_sends_cancel_with_references(mocker, sa
     create_broadcast_provider_message(update_event, 'ee')
 
     mock_cancel_broadcast = mocker.patch(
-        'app.cbc_proxy_client.cancel_broadcast',
+        'app.clients.cbc_proxy.CBCProxyEE.cancel_broadcast',
     )
 
     send_broadcast_provider_message(provider='ee', broadcast_event_id=str(cancel_event.id))
@@ -181,7 +181,7 @@ def test_send_broadcast_provider_message_errors(mocker, sample_service):
     event = create_broadcast_event(broadcast_message)
 
     mock_create_broadcast = mocker.patch(
-        'app.cbc_proxy_client.create_and_send_broadcast',
+        'app.clients.cbc_proxy.CBCProxyEE.create_and_send_broadcast',
         side_effect=Exception('oh no'),
     )
 
@@ -210,10 +210,10 @@ def test_trigger_link_tests_invokes_cbc_proxy_client(
     mocker,
 ):
     mock_send_link_test = mocker.patch(
-        'app.cbc_proxy_client.send_link_test',
+        'app.clients.cbc_proxy.CBCProxyEE.send_link_test',
     )
 
-    trigger_link_test('some-provider')
+    trigger_link_test('ee')
 
     assert mock_send_link_test.called
     # the 0th argument of the call to send_link_test

tests/app/celery/test_scheduled_tasks.py

@@ -561,9 +561,10 @@ def test_check_for_services_with_high_failure_rates_or_sending_to_tv_numbers(
 
 def test_send_canary_to_cbc_proxy_invokes_cbc_proxy_client(
     mocker,
+    notify_api
 ):
     mock_send_canary = mocker.patch(
-        'app.cbc_proxy_client.send_canary',
+        'app.clients.cbc_proxy.CBCProxyCanary.send_canary',
     )
 
     scheduled_tasks.send_canary_to_cbc_proxy()

tests/app/clients/test_cbc_proxy.py

@@ -1,13 +1,14 @@
 import json
 import uuid
+from unittest.mock import Mock
 
 import pytest
 
-from app.clients.cbc_proxy import CBCProxyClient, CBCProxyException
+from app.clients.cbc_proxy import CBCProxyClient, CBCProxyException, CBCProxyEE, CBCProxyCanary
 
 
 @pytest.fixture(scope='function')
-def cbc_proxy(client, mocker):
+def cbc_proxy_client(client, mocker):
     client = CBCProxyClient()
     current_app = mocker.Mock(config={
         'CBC_PROXY_AWS_ACCESS_KEY_ID': 'cbc-proxy-aws-access-key-id',
@@ -17,19 +18,39 @@ def cbc_proxy(client, mocker):
     return client
 
 
-def test_cbc_proxy_lambda_client_has_correct_region(cbc_proxy):
+@pytest.fixture
-    assert cbc_proxy._lambda_client._client_config.region_name == 'eu-west-2'
+def cbc_proxy_ee(cbc_proxy_client):
+    return cbc_proxy_client.get_proxy('ee')
 
 
-def test_cbc_proxy_lambda_client_has_correct_keys(cbc_proxy):
+@pytest.mark.parametrize('provider_name, expected_provider_class', [
-    key = cbc_proxy._lambda_client._request_signer._credentials.access_key
+    ('ee', CBCProxyEE),
-    secret = cbc_proxy._lambda_client._request_signer._credentials.secret_key
+    ('canary', CBCProxyCanary),
+])
+def test_cbc_proxy_client_returns_correct_client(provider_name, expected_provider_class):
+    mock_lambda = Mock()
+    cbc_proxy_client = CBCProxyClient()
+    cbc_proxy_client._lambda_client = mock_lambda
+
+    ret = cbc_proxy_client.get_proxy(provider_name)
+
+    assert type(ret) == expected_provider_class
+    assert ret._lambda_client == mock_lambda
+
+
+def test_cbc_proxy_lambda_client_has_correct_region(cbc_proxy_ee):
+    assert cbc_proxy_ee._lambda_client._client_config.region_name == 'eu-west-2'
+
+
+def test_cbc_proxy_lambda_client_has_correct_keys(cbc_proxy_ee):
+    key = cbc_proxy_ee._lambda_client._request_signer._credentials.access_key
+    secret = cbc_proxy_ee._lambda_client._request_signer._credentials.secret_key
 
     assert key == 'cbc-proxy-aws-access-key-id'
     assert secret == 'cbc-proxy-aws-secret-access-key'
 
 
-def test_cbc_proxy_create_and_send_invokes_function(mocker, cbc_proxy):
+def test_cbc_proxy_create_and_send_invokes_function(mocker, cbc_proxy_ee):
     identifier = 'my-identifier'
     headline = 'my-headline'
     description = 'my-description'
@@ -50,7 +71,7 @@ def test_cbc_proxy_create_and_send_invokes_function(mocker, cbc_proxy):
     }]
 
     ld_client_mock = mocker.patch.object(
-        cbc_proxy,
+        cbc_proxy_ee,
         '_lambda_client',
         create=True,
     )
@@ -59,7 +80,7 @@ def test_cbc_proxy_create_and_send_invokes_function(mocker, cbc_proxy):
         'StatusCode': 200,
     }
 
-    cbc_proxy.create_and_send_broadcast(
+    cbc_proxy_ee.create_and_send_broadcast(
         identifier=identifier,
         headline=headline,
         description=description,
@@ -86,7 +107,7 @@ def test_cbc_proxy_create_and_send_invokes_function(mocker, cbc_proxy):
     assert payload['expires'] == expires
 
 
-def test_cbc_proxy_create_and_send_handles_invoke_error(mocker, cbc_proxy):
+def test_cbc_proxy_create_and_send_handles_invoke_error(mocker, cbc_proxy_ee):
     identifier = 'my-identifier'
     headline = 'my-headline'
     description = 'my-description'
@@ -107,7 +128,7 @@ def test_cbc_proxy_create_and_send_handles_invoke_error(mocker, cbc_proxy):
     }]
 
     ld_client_mock = mocker.patch.object(
-        cbc_proxy,
+        cbc_proxy_ee,
         '_lambda_client',
         create=True,
     )
@@ -117,7 +138,7 @@ def test_cbc_proxy_create_and_send_handles_invoke_error(mocker, cbc_proxy):
     }
 
     with pytest.raises(CBCProxyException) as e:
-        cbc_proxy.create_and_send_broadcast(
+        cbc_proxy_ee.create_and_send_broadcast(
             identifier=identifier,
             headline=headline,
             description=description,
@@ -134,7 +155,7 @@ def test_cbc_proxy_create_and_send_handles_invoke_error(mocker, cbc_proxy):
     )
 
 
-def test_cbc_proxy_create_and_send_handles_function_error(mocker, cbc_proxy):
+def test_cbc_proxy_create_and_send_handles_function_error(mocker, cbc_proxy_ee):
     identifier = 'my-identifier'
     headline = 'my-headline'
     description = 'my-description'
@@ -155,7 +176,7 @@ def test_cbc_proxy_create_and_send_handles_function_error(mocker, cbc_proxy):
     }]
 
     ld_client_mock = mocker.patch.object(
-        cbc_proxy,
+        cbc_proxy_ee,
         '_lambda_client',
         create=True,
     )
@@ -166,7 +187,7 @@ def test_cbc_proxy_create_and_send_handles_function_error(mocker, cbc_proxy):
     }
 
     with pytest.raises(CBCProxyException) as e:
-        cbc_proxy.create_and_send_broadcast(
+        cbc_proxy_ee.create_and_send_broadcast(
             identifier=identifier,
             headline=headline,
             description=description,
@@ -183,11 +204,13 @@ def test_cbc_proxy_create_and_send_handles_function_error(mocker, cbc_proxy):
     )
 
 
-def test_cbc_proxy_send_canary_invokes_function(mocker, cbc_proxy):
+def test_cbc_proxy_send_canary_invokes_function(mocker, cbc_proxy_client):
     identifier = str(uuid.uuid4())
 
+    canary_client = cbc_proxy_client.get_proxy('canary')
+
     ld_client_mock = mocker.patch.object(
-        cbc_proxy,
+        canary_client,
         '_lambda_client',
         create=True,
     )
@@ -196,7 +219,7 @@ def test_cbc_proxy_send_canary_invokes_function(mocker, cbc_proxy):
         'StatusCode': 200,
     }
 
-    cbc_proxy.send_canary(
+    canary_client.send_canary(
         identifier=identifier,
     )
 
@@ -213,66 +236,11 @@ def test_cbc_proxy_send_canary_invokes_function(mocker, cbc_proxy):
     assert payload['identifier'] == identifier
 
 
-def test_cbc_proxy_send_canary_handles_invoke_error(mocker, cbc_proxy):
+def test_cbc_proxy_send_link_test_invokes_function(mocker, cbc_proxy_ee):
     identifier = str(uuid.uuid4())
 
     ld_client_mock = mocker.patch.object(
-        cbc_proxy,
+        cbc_proxy_ee,
-        '_lambda_client',
-        create=True,
-    )
-
-    ld_client_mock.invoke.return_value = {
-        'StatusCode': 400,
-    }
-
-    with pytest.raises(CBCProxyException) as e:
-        cbc_proxy.send_canary(
-            identifier=identifier,
-        )
-
-    assert e.match('Could not invoke lambda')
-
-    ld_client_mock.invoke.assert_called_once_with(
-        FunctionName='canary',
-        InvocationType='RequestResponse',
-        Payload=mocker.ANY,
-    )
-
-
-def test_cbc_proxy_send_canary_handles_function_error(mocker, cbc_proxy):
-    identifier = str(uuid.uuid4())
-
-    ld_client_mock = mocker.patch.object(
-        cbc_proxy,
-        '_lambda_client',
-        create=True,
-    )
-
-    ld_client_mock.invoke.return_value = {
-        'StatusCode': 200,
-        'FunctionError': 'something',
-    }
-
-    with pytest.raises(CBCProxyException) as e:
-        cbc_proxy.send_canary(
-            identifier=identifier,
-        )
-
-    assert e.match('Function exited with unhandled exception')
-
-    ld_client_mock.invoke.assert_called_once_with(
-        FunctionName='canary',
-        InvocationType='RequestResponse',
-        Payload=mocker.ANY,
-    )
-
-
-def test_cbc_proxy_send_link_test_invokes_function(mocker, cbc_proxy):
-    identifier = str(uuid.uuid4())
-
-    ld_client_mock = mocker.patch.object(
-        cbc_proxy,
         '_lambda_client',
         create=True,
     )
@@ -281,7 +249,7 @@ def test_cbc_proxy_send_link_test_invokes_function(mocker, cbc_proxy):
         'StatusCode': 200,
     }
 
-    cbc_proxy.send_link_test(
+    cbc_proxy_ee.send_link_test(
         identifier=identifier,
     )
 
@@ -297,58 +265,3 @@ def test_cbc_proxy_send_link_test_invokes_function(mocker, cbc_proxy):
 
     assert payload['identifier'] == identifier
     assert payload['message_type'] == 'test'
-
-
-def test_cbc_proxy_send_link_test_handles_invoke_error(mocker, cbc_proxy):
-    identifier = str(uuid.uuid4())
-
-    ld_client_mock = mocker.patch.object(
-        cbc_proxy,
-        '_lambda_client',
-        create=True,
-    )
-
-    ld_client_mock.invoke.return_value = {
-        'StatusCode': 400,
-    }
-
-    with pytest.raises(CBCProxyException) as e:
-        cbc_proxy.send_link_test(
-            identifier=identifier,
-        )
-
-    assert e.match('Could not invoke lambda')
-
-    ld_client_mock.invoke.assert_called_once_with(
-        FunctionName='bt-ee-1-proxy',
-        InvocationType='RequestResponse',
-        Payload=mocker.ANY,
-    )
-
-
-def test_cbc_proxy_send_link_test_handles_function_error(mocker, cbc_proxy):
-    identifier = str(uuid.uuid4())
-
-    ld_client_mock = mocker.patch.object(
-        cbc_proxy,
-        '_lambda_client',
-        create=True,
-    )
-
-    ld_client_mock.invoke.return_value = {
-        'StatusCode': 200,
-        'FunctionError': 'something',
-    }
-
-    with pytest.raises(CBCProxyException) as e:
-        cbc_proxy.send_link_test(
-            identifier=identifier,
-        )
-
-    assert e.match('Function exited with unhandled exception')
-
-    ld_client_mock.invoke.assert_called_once_with(
-        FunctionName='bt-ee-1-proxy',
-        InvocationType='RequestResponse',
-        Payload=mocker.ANY,
-    )