diff --git a/app/__init__.py b/app/__init__.py index c412e4b8f..5f85ccad6 100644 --- a/app/__init__.py +++ b/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) diff --git a/app/celery/broadcast_message_tasks.py b/app/celery/broadcast_message_tasks.py index d13b76cd3..65fdceeba 100644 --- a/app/celery/broadcast_message_tasks.py +++ b/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) diff --git a/app/celery/scheduled_tasks.py b/app/celery/scheduled_tasks.py index 948c572aa..7fc16a08f 100644 --- a/app/celery/scheduled_tasks.py +++ b/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') diff --git a/app/clients/cbc_proxy.py b/app/clients/cbc_proxy.py index 841e6dad4..99174121b 100644 --- a/app/clients/cbc_proxy.py +++ b/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 -# where each identifier is a previous sent message -# ie a Cancel message would have a unique identifier but have the identifier of +# previous_provider_messages is a list of previous events (models.py::BroadcastProviderMessage) +# ie a Cancel message would have a unique event but have the event of # the preceeding Alert message in the previous_provider_messages field @@ -26,11 +26,31 @@ class CBCProxyException(Exception): pass -# Noop = no operation -class CBCProxyNoopClient: +class CBCProxyClient: + _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, ): - """ - canary - a specific lambda that does not connect to a provider, but just confirms the connectivity between - Notify and the CBC proxy AWS account - """ - self._invoke_lambda(function_name='canary', payload={'identifier': identifier}) + self._invoke_lambda(payload={'identifier': identifier}) + + +class CBCProxyEE(CBCProxyClientBase): + 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) - - # 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 + self._invoke_lambda(payload=payload) diff --git a/tests/app/celery/test_broadcast_message_tasks.py b/tests/app/celery/test_broadcast_message_tasks.py index 92dff4db6..097f940a0 100644 --- a/tests/app/celery/test_broadcast_message_tasks.py +++ b/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 diff --git a/tests/app/celery/test_scheduled_tasks.py b/tests/app/celery/test_scheduled_tasks.py index a5b4d9cd3..8ea0613a6 100644 --- a/tests/app/celery/test_scheduled_tasks.py +++ b/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() diff --git a/tests/app/clients/test_cbc_proxy.py b/tests/app/clients/test_cbc_proxy.py index d888d44b6..10534855b 100644 --- a/tests/app/clients/test_cbc_proxy.py +++ b/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): - assert cbc_proxy._lambda_client._client_config.region_name == 'eu-west-2' +@pytest.fixture +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): - key = cbc_proxy._lambda_client._request_signer._credentials.access_key - secret = cbc_proxy._lambda_client._request_signer._credentials.secret_key +@pytest.mark.parametrize('provider_name, expected_provider_class', [ + ('ee', CBCProxyEE), + ('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, - '_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, + cbc_proxy_ee, '_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, - )