diff --git a/Makefile b/Makefile index ea6bb8c41..3b2d46d16 100644 --- a/Makefile +++ b/Makefile @@ -41,32 +41,25 @@ venv/bin/activate: .PHONY: check-env-vars check-env-vars: ## Check mandatory environment variables $(if ${DEPLOY_ENV},,$(error Must specify DEPLOY_ENV)) - $(if ${DNS_NAME},,$(error Must specify DNS_NAME)) - $(if ${AWS_ACCESS_KEY_ID},,$(error Must specify AWS_ACCESS_KEY_ID)) - $(if ${AWS_SECRET_ACCESS_KEY},,$(error Must specify AWS_SECRET_ACCESS_KEY)) .PHONY: sandbox sandbox: ## Set environment to sandbox $(eval export DEPLOY_ENV=sandbox) - $(eval export DNS_NAME="cloudapps.digital") @true .PHONY: preview preview: ## Set environment to preview $(eval export DEPLOY_ENV=preview) - $(eval export DNS_NAME="notify.works") @true .PHONY: staging staging: ## Set environment to staging $(eval export DEPLOY_ENV=staging) - $(eval export DNS_NAME="staging-notify.works") @true .PHONY: production production: ## Set environment to production $(eval export DEPLOY_ENV=production) - $(eval export DNS_NAME="notifications.service.gov.uk") @true .PHONY: dependencies @@ -82,31 +75,12 @@ generate-version-file: ## Generates the app version file build: dependencies generate-version-file ## Build project . venv/bin/activate && PIP_ACCEL_CACHE=${PIP_ACCEL_CACHE} pip-accel install -r requirements.txt -.PHONY: cf-build -cf-build: dependencies generate-version-file ## Build project for PAAS - -.PHONY: build-codedeploy-artifact -build-codedeploy-artifact: ## Build the deploy artifact for CodeDeploy +.PHONY: build-paas-artifact +build-paas-artifact: ## Build the deploy artifact for PaaS rm -rf target mkdir -p target zip -y -q -r -x@deploy-exclude.lst target/notifications-api.zip ./ - rm -rf build/db-migration-codedeploy - mkdir -p build/db-migration-codedeploy - unzip target/notifications-api.zip -d build/db-migration-codedeploy - cd build/db-migration-codedeploy && \ - mv -f appspec-db-migration.yml appspec.yml && \ - zip -y -q -r -x@deploy-exclude.lst ../../target/notifications-api-db-migration.zip ./ - -.PHONY: upload-codedeploy-artifact ## Upload the deploy artifact for CodeDeploy -upload-codedeploy-artifact: check-env-vars - $(if ${DEPLOY_BUILD_NUMBER},,$(error Must specify DEPLOY_BUILD_NUMBER)) - aws s3 cp --region eu-west-1 --sse AES256 target/notifications-api.zip s3://${DNS_NAME}-codedeploy/notifications-api-${DEPLOY_BUILD_NUMBER}.zip - aws s3 cp --region eu-west-1 --sse AES256 target/notifications-api-db-migration.zip s3://${DNS_NAME}-codedeploy/notifications-api-db-migration-${DEPLOY_BUILD_NUMBER}.zip - -.PHONY: build-paas-artifact -build-paas-artifact: build-codedeploy-artifact ## Build the deploy artifact for PaaS - .PHONY: upload-paas-artifact ## Upload the deploy artifact for PaaS upload-paas-artifact: $(if ${DEPLOY_BUILD_NUMBER},,$(error Must specify DEPLOY_BUILD_NUMBER)) @@ -117,42 +91,6 @@ upload-paas-artifact: test: venv generate-version-file ## Run tests ./scripts/run_tests.sh -.PHONY: deploy-api -deploy-api: check-env-vars ## Trigger CodeDeploy for the api - aws deploy create-deployment --application-name notify-api --deployment-config-name CodeDeployDefault.OneAtATime --deployment-group-name notify-api --s3-location bucket=${DNS_NAME}-codedeploy,key=notifications-api-${DEPLOY_BUILD_NUMBER}.zip,bundleType=zip --region eu-west-1 - -.PHONY: deploy-api -deploy-api-db-migration: check-env-vars ## Trigger CodeDeploy for the api db migration - aws deploy create-deployment --application-name notify-api-db-migration --deployment-config-name CodeDeployDefault.OneAtATime --deployment-group-name notify-api-db-migration --s3-location bucket=${DNS_NAME}-codedeploy,key=notifications-api-db-migration-${DEPLOY_BUILD_NUMBER}.zip,bundleType=zip --region eu-west-1 - -.PHONY: deploy-admin-api -deploy-admin-api: check-env-vars ## Trigger CodeDeploy for the admin api - aws deploy create-deployment --application-name notify-admin-api --deployment-config-name CodeDeployDefault.OneAtATime --deployment-group-name notify-admin-api --s3-location bucket=${DNS_NAME}-codedeploy,key=notifications-api-${DEPLOY_BUILD_NUMBER}.zip,bundleType=zip --region eu-west-1 - -.PHONY: deploy-delivery -deploy-delivery: check-env-vars ## Trigger CodeDeploy for the delivery app - aws deploy create-deployment --application-name notify-delivery --deployment-config-name CodeDeployDefault.OneAtATime --deployment-group-name notify-delivery --s3-location bucket=${DNS_NAME}-codedeploy,key=notifications-api-${DEPLOY_BUILD_NUMBER}.zip,bundleType=zip --region eu-west-1 - -.PHONY: check-aws-vars -check-aws-vars: ## Check if AWS access keys are set - $(if ${AWS_ACCESS_KEY_ID},,$(error Must specify AWS_ACCESS_KEY_ID)) - $(if ${AWS_SECRET_ACCESS_KEY},,$(error Must specify AWS_SECRET_ACCESS_KEY)) - -.PHONY: deploy-suspend-autoscaling-processes -deploy-suspend-autoscaling-processes: check-aws-vars ## Suspend launch and terminate processes for the auto-scaling group - $(if ${CODEDEPLOY_APP_NAME},,$(error Must specify CODEDEPLOY_APP_NAME)) - aws autoscaling suspend-processes --region eu-west-1 --auto-scaling-group-name ${CODEDEPLOY_APP_NAME} --scaling-processes "Launch" "Terminate" - -.PHONY: deploy-resume-autoscaling-processes -deploy-resume-autoscaling-processes: check-aws-vars ## Resume launch and terminate processes for the auto-scaling group - $(if ${CODEDEPLOY_APP_NAME},,$(error Must specify CODEDEPLOY_APP_NAME)) - aws autoscaling resume-processes --region eu-west-1 --auto-scaling-group-name ${CODEDEPLOY_APP_NAME} --scaling-processes "Launch" "Terminate" - -.PHONY: deploy-check-autoscaling-processes -deploy-check-autoscaling-processes: check-aws-vars ## Returns with the number of instances with active autoscaling events - $(if ${CODEDEPLOY_APP_NAME},,$(error Must specify CODEDEPLOY_APP_NAME)) - @aws autoscaling describe-auto-scaling-groups --region eu-west-1 --auto-scaling-group-names ${CODEDEPLOY_APP_NAME} | jq '.AutoScalingGroups[0].Instances|map(select(.LifecycleState != "InService"))|length' - .PHONY: coverage coverage: venv ## Create coverage report . venv/bin/activate && coveralls @@ -181,25 +119,6 @@ build-with-docker: prepare-docker-build-image ## Build inside a Docker container ${DOCKER_BUILDER_IMAGE_NAME} \ gosu hostuser make build -.PHONY: cf-build-with-docker -cf-build-with-docker: prepare-docker-build-image ## Build inside a Docker container - @docker run -i${DOCKER_TTY} --rm \ - --name "${DOCKER_CONTAINER_PREFIX}-build" \ - -v "`pwd`:/var/project" \ - -v "${PIP_ACCEL_CACHE}:/var/project/cache/pip-accel" \ - -e UID=$(shell id -u) \ - -e GID=$(shell id -g) \ - -e GIT_COMMIT=${GIT_COMMIT} \ - -e BUILD_NUMBER=${BUILD_NUMBER} \ - -e BUILD_URL=${BUILD_URL} \ - -e http_proxy="${HTTP_PROXY}" \ - -e HTTP_PROXY="${HTTP_PROXY}" \ - -e https_proxy="${HTTPS_PROXY}" \ - -e HTTPS_PROXY="${HTTPS_PROXY}" \ - -e NO_PROXY="${NO_PROXY}" \ - ${DOCKER_BUILDER_IMAGE_NAME} \ - gosu hostuser make cf-build - .PHONY: test-with-docker test-with-docker: prepare-docker-build-image create-docker-test-db ## Run tests inside a Docker container @docker run -i${DOCKER_TTY} --rm \ @@ -220,7 +139,7 @@ test-with-docker: prepare-docker-build-image create-docker-test-db ## Run tests ${DOCKER_BUILDER_IMAGE_NAME} \ gosu hostuser make test -.PHONY: test-with-docker +.PHONY: create-docker-test-db create-docker-test-db: ## Start the test database in a Docker container docker rm -f ${DOCKER_CONTAINER_PREFIX}-db 2> /dev/null || true @docker run -d \ diff --git a/app/celery/letters_pdf_tasks.py b/app/celery/letters_pdf_tasks.py index ec08666b4..7096f281a 100644 --- a/app/celery/letters_pdf_tasks.py +++ b/app/celery/letters_pdf_tasks.py @@ -98,7 +98,8 @@ def collate_letter_pdfs_for_day(date): notify_celery.send_task( name=TaskNames.ZIP_AND_SEND_LETTER_PDFS, kwargs={'filenames_to_zip': filenames}, - queue=QueueNames.PROCESS_FTP + queue=QueueNames.PROCESS_FTP, + compression='zlib' ) @@ -112,7 +113,10 @@ def group_letters(letter_pdfs): list_of_files = [] for letter in letter_pdfs: if letter['Key'].lower().endswith('.pdf'): - if running_filesize + letter['Size'] > current_app.config['MAX_LETTER_PDF_ZIP_FILESIZE']: + if ( + running_filesize + letter['Size'] > current_app.config['MAX_LETTER_PDF_ZIP_FILESIZE'] or + len(list_of_files) >= current_app.config['MAX_LETTER_PDF_COUNT_PER_ZIP'] + ): yield list_of_files running_filesize = 0 list_of_files = [] diff --git a/app/celery/tasks.py b/app/celery/tasks.py index 7b5717b66..57a7c5bc8 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -31,7 +31,6 @@ from app import ( from app.aws import s3 from app.celery import provider_tasks from app.celery import letters_pdf_tasks -from app.celery.service_callback_tasks import send_delivery_status_to_service from app.config import QueueNames from app.dao.inbound_sms_dao import dao_get_inbound_sms_by_id from app.dao.jobs_dao import ( @@ -46,13 +45,11 @@ from app.dao.notifications_dao import ( dao_update_notifications_for_job_to_sent_to_dvla, dao_update_notifications_by_reference, dao_get_last_notification_added_for_job_id, - dao_get_notifications_by_references, ) from app.dao.provider_details_dao import get_current_provider from app.dao.service_inbound_api_dao import get_service_inbound_api_for_service from app.dao.services_dao import dao_fetch_service_by_id, fetch_todays_total_message_count from app.dao.templates_dao import dao_get_template_by_id -from app.dao.service_callback_api_dao import get_service_callback_api_for_service from app.models import ( DVLA_RESPONSE_STATUS_SENT, EMAIL_TYPE, @@ -416,12 +413,6 @@ def update_letter_notifications_to_error(self, notification_references): ) current_app.logger.info("Updated {} letter notifications to technical-failure".format(updated_count)) - notifications = dao_get_notifications_by_references(references=notification_references) - # queue callback task only if the service_callback_api exists - service_callback_api = get_service_callback_api_for_service(service_id=notifications[0].service_id) - if service_callback_api: - for notification in notifications: - send_delivery_status_to_service.apply_async([str(notification.id)], queue=QueueNames.CALLBACKS) def create_dvla_file_contents_for_job(job_id): @@ -503,12 +494,6 @@ def update_letter_notifications_statuses(self, filename): current_app.logger.info( 'DVLA file: {filename}, notification updated to {status}: {reference}'.format( filename=filename, status=status, reference=str(update.reference))) - notifications = dao_get_notifications_by_references(references=[update.reference]) - # queue callback task only if the service_callback_api exists - service_callback_api = get_service_callback_api_for_service(service_id=notifications[0].service_id) - if service_callback_api: - for notification in notifications: - send_delivery_status_to_service.apply_async([str(notification.id)], queue=QueueNames.CALLBACKS) def process_updates_from_file(response_file): diff --git a/app/config.py b/app/config.py index e42a08c3b..ba24b4a5a 100644 --- a/app/config.py +++ b/app/config.py @@ -129,6 +129,7 @@ class Config(object): MAX_VERIFY_CODE_COUNT = 10 MAX_LETTER_PDF_ZIP_FILESIZE = 500 * 1024 * 1024 # 500mb + MAX_LETTER_PDF_COUNT_PER_ZIP = 5000 CHECK_PROXY_HEADER = False diff --git a/app/invite/rest.py b/app/invite/rest.py index ba263e1b3..c234dd5d1 100644 --- a/app/invite/rest.py +++ b/app/invite/rest.py @@ -23,7 +23,8 @@ register_errors(invite) @invite.route('', methods=['POST']) def create_invited_user(service_id): - invited_user, errors = invited_user_schema.load(request.get_json()) + request_json = request.get_json() + invited_user, errors = invited_user_schema.load(request_json) save_invited_user(invited_user) template = dao_get_template_by_id(current_app.config['INVITATION_EMAIL_TEMPLATE_ID']) @@ -37,7 +38,10 @@ def create_invited_user(service_id): personalisation={ 'user_name': invited_user.from_user.name, 'service_name': invited_user.service.name, - 'url': invited_user_url(invited_user.id) + 'url': invited_user_url( + invited_user.id, + request_json.get('invite_link_host'), + ), }, notification_type=EMAIL_TYPE, api_key_id=None, @@ -74,8 +78,11 @@ def update_invited_user(service_id, invited_user_id): return jsonify(data=invited_user_schema.dump(fetched).data), 200 -def invited_user_url(invited_user_id): +def invited_user_url(invited_user_id, invite_link_host=None): from notifications_utils.url_safe_token import generate_token token = generate_token(str(invited_user_id), current_app.config['SECRET_KEY'], current_app.config['DANGEROUS_SALT']) - return '{0}/invitation/{1}'.format(current_app.config['ADMIN_BASE_URL'], token) + if invite_link_host is None: + invite_link_host = current_app.config['ADMIN_BASE_URL'] + + return '{0}/invitation/{1}'.format(invite_link_host, token) diff --git a/app/models.py b/app/models.py index 51af29f90..4f4a6cafc 100644 --- a/app/models.py +++ b/app/models.py @@ -14,6 +14,7 @@ from sqlalchemy import UniqueConstraint, CheckConstraint from notifications_utils.recipients import ( validate_email_address, validate_phone_number, + try_validate_and_format_phone_number, InvalidPhoneError, InvalidEmailError ) @@ -366,6 +367,9 @@ class ServiceSmsSender(db.Model): created_at = db.Column(db.DateTime, default=datetime.datetime.utcnow, nullable=False) updated_at = db.Column(db.DateTime, nullable=True, onupdate=datetime.datetime.utcnow) + def get_reply_to_text(self): + return try_validate_and_format_phone_number(self.sms_sender) + def serialize(self): return { "id": str(self.id), @@ -640,7 +644,7 @@ class TemplateBase(db.Model): elif self.template_type == EMAIL_TYPE: return self.service.get_default_reply_to_email_address() elif self.template_type == SMS_TYPE: - return self.service.get_default_sms_sender() + return try_validate_and_format_phone_number(self.service.get_default_sms_sender()) else: return None diff --git a/app/service/send_notification.py b/app/service/send_notification.py index 53cbb2b6c..2b9be3de7 100644 --- a/app/service/send_notification.py +++ b/app/service/send_notification.py @@ -90,7 +90,7 @@ def get_reply_to_text(notification_type, sender_id, service, template): if notification_type == EMAIL_TYPE: reply_to = dao_get_reply_to_by_id(service.id, sender_id).email_address elif notification_type == SMS_TYPE: - reply_to = dao_get_service_sms_senders_by_id(service.id, sender_id).sms_sender + reply_to = dao_get_service_sms_senders_by_id(service.id, sender_id).get_reply_to_text() else: reply_to = template.get_reply_to_text() diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index 5466be234..bcb2b4b7e 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -2,6 +2,8 @@ import functools from flask import request, jsonify, current_app, abort +from notifications_utils.recipients import try_validate_and_format_phone_number + from app import api_user, authenticated_service from app.config import QueueNames from app.models import ( @@ -208,9 +210,13 @@ def get_reply_to_text(notification_type, form, template): elif notification_type == SMS_TYPE: service_sms_sender_id = form.get("sms_sender_id", None) - reply_to = check_service_sms_sender_id( + sms_sender_id = check_service_sms_sender_id( str(authenticated_service.id), service_sms_sender_id, notification_type - ) or template.get_reply_to_text() + ) + if sms_sender_id: + reply_to = try_validate_and_format_phone_number(sms_sender_id) + else: + reply_to = template.get_reply_to_text() elif notification_type == LETTER_TYPE: reply_to = template.get_reply_to_text() diff --git a/appspec-db-migration.yml b/appspec-db-migration.yml deleted file mode 100644 index b840f4432..000000000 --- a/appspec-db-migration.yml +++ /dev/null @@ -1,18 +0,0 @@ ---- -os: linux -version: 0.0 -files: - - destination: /home/notify-app/notifications-api - source: / -hooks: - AfterInstall: - - location: scripts/aws_install_dependencies.sh - runas: root - timeout: 1000 - - location: scripts/aws_change_ownership.sh - runas: root - timeout: 300 - ApplicationStart: - - location: scripts/aws_run_db_migrations.sh - runas: root - timeout: 300 diff --git a/appspec.yml b/appspec.yml deleted file mode 100644 index 87a0f944a..000000000 --- a/appspec.yml +++ /dev/null @@ -1,32 +0,0 @@ ---- -os: linux -version: 0.0 -files: - - destination: /home/notify-app/notifications-api - source: / -hooks: - BeforeInstall: - - location: scripts/aws_clear_instance.sh - runas: root - timeout: 1000 - AfterInstall: - - location: scripts/aws_install_dependencies.sh - runas: root - timeout: 1000 - - location: scripts/aws_change_ownership.sh - runas: root - timeout: 300 - ApplicationStart: - - location: scripts/aws_start_app.sh - runas: root - timeout: 300 - - location: scripts/register_with_elb.sh - runas: ubuntu - timeout: 300 - ApplicationStop: - - location: scripts/deregister_from_elb.sh - runas: ubuntu - timeout: 300 - - location: scripts/aws_stop_app.sh - runas: root - timeout: 300 diff --git a/scripts/aws_change_ownership.sh b/scripts/aws_change_ownership.sh deleted file mode 100755 index ad0d438a7..000000000 --- a/scripts/aws_change_ownership.sh +++ /dev/null @@ -1,8 +0,0 @@ -#!/bin/bash - -set -eo pipefail - -echo "Chown application to be owned by notify-app" - -cd /home/notify-app/; -chown -R notify-app:govuk-notify-applications notifications-api diff --git a/scripts/aws_clear_instance.sh b/scripts/aws_clear_instance.sh deleted file mode 100755 index 9cdc14568..000000000 --- a/scripts/aws_clear_instance.sh +++ /dev/null @@ -1,10 +0,0 @@ -#!/bin/bash - -echo "Removing application and dependencies" - -if [ -d "/home/notify-app/notifications-api" ]; then - # Remove and re-create the directory - rm -rf /home/notify-app/notifications-api - mkdir -p /home/notify-app/notifications-api -fi - diff --git a/scripts/aws_install_dependencies.sh b/scripts/aws_install_dependencies.sh deleted file mode 100755 index b82881d45..000000000 --- a/scripts/aws_install_dependencies.sh +++ /dev/null @@ -1,8 +0,0 @@ -#!/bin/bash - -set -eo pipefail - -echo "Install dependencies" - -cd /home/notify-app/notifications-api; -pip3 install -r /home/notify-app/notifications-api/requirements.txt diff --git a/scripts/aws_run_db_migrations.sh b/scripts/aws_run_db_migrations.sh deleted file mode 100755 index 9667839d5..000000000 --- a/scripts/aws_run_db_migrations.sh +++ /dev/null @@ -1,8 +0,0 @@ -#!/usr/bin/env bash - -set -eo pipefail - -echo "Run database migrations" - -cd /home/notify-app/notifications-api; -python3 db.py db upgrade diff --git a/scripts/aws_start_app.sh b/scripts/aws_start_app.sh deleted file mode 100755 index 0c48d4859..000000000 --- a/scripts/aws_start_app.sh +++ /dev/null @@ -1,20 +0,0 @@ -#!/usr/bin/env bash - -set -eo pipefail - -function start -{ - service=$1 - if [ -e "/etc/init/${service}.conf" ] - then - echo "Starting ${service}" - service ${service} start - fi -} - -start "notifications-api" -start "notifications-api-celery-worker" -start "notifications-api-celery-worker-sender" -start "notifications-api-celery-worker-research" -start "notifications-api-celery-worker-db" -start "notifications-api-celery-beat" diff --git a/scripts/aws_stop_app.sh b/scripts/aws_stop_app.sh deleted file mode 100755 index 4ccc4ebc9..000000000 --- a/scripts/aws_stop_app.sh +++ /dev/null @@ -1,23 +0,0 @@ -#!/usr/bin/env bash - -set -eo pipefail - -function stop -{ - service=$1 - if [ -e "/etc/init/${service}.conf" ]; then - echo "stopping ${service}" - if service ${service} stop; then - echo "${service} stopped" - else - >&2 echo "Could not stop ${service}" - fi - fi -} - -stop "notifications-api" -stop "notifications-api-celery-beat" -stop "notifications-api-celery-worker" -stop "notifications-api-celery-worker-sender" -stop "notifications-api-celery-worker-research" -stop "notifications-api-celery-worker-db" diff --git a/scripts/deregister_from_elb.sh b/scripts/deregister_from_elb.sh deleted file mode 100755 index a539f0cac..000000000 --- a/scripts/deregister_from_elb.sh +++ /dev/null @@ -1,77 +0,0 @@ -#!/bin/bash -# -# Copyright 2014 Amazon.com, Inc. or its affiliates. All Rights Reserved. -# -# Licensed under the Apache License, Version 2.0 (the "License"). -# You may not use this file except in compliance with the License. -# A copy of the License is located at -# -# http://aws.amazon.com/apache2.0 -# -# or in the "license" file accompanying this file. This file is distributed -# on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either -# express or implied. See the License for the specific language governing -# permissions and limitations under the License. - -. $(dirname $0)/common_functions.sh - -msg "Running AWS CLI with region: $(get_instance_region)" - -# get this instance's ID -INSTANCE_ID=$(get_instance_id) -if [ $? != 0 -o -z "$INSTANCE_ID" ]; then - error_exit "Unable to get this instance's ID; cannot continue." -fi - -# Get current time -msg "Started $(basename $0) at $(/bin/date "+%F %T")" -start_sec=$(/bin/date +%s.%N) - -msg "Getting relevant load balancer" -INSTANCE_NAME=$(get_instance_name_from_tags $INSTANCE_ID) - -if [[ "$(tr [:upper:] [:lower:] <<< "${INSTANCE_NAME}")" =~ 'delivery' ]]; then - msg "NO ELBs for delivery" - exit 0 -fi - -get_elb_name_for_instance_name $INSTANCE_NAME -get_elb_list $INSTANCE_ID $ELB_NAME - -msg "Checking that user set at least one load balancer" -if test -z "$ELB_LIST"; then - error_exit "Must have at least one load balancer to deregister from" -fi - -# Loop through all LBs the user set, and attempt to deregister this instance from them. -for elb in $ELB_LIST; do - msg "Checking validity of load balancer named '$elb'" - validate_elb $INSTANCE_ID $elb - if [ $? != 0 ]; then - msg "Error validating $elb; cannot continue with this LB" - continue - fi - - msg "Deregistering $INSTANCE_ID from $elb" - deregister_instance $INSTANCE_ID $elb - - if [ $? != 0 ]; then - error_exit "Failed to deregister instance $INSTANCE_ID from ELB $elb" - fi -done - -# Wait for all Deregistrations to finish -msg "Waiting for instance to de-register from its load balancers" -for elb in $ELB_LIST; do - wait_for_state "elb" $INSTANCE_ID "OutOfService" $elb - if [ $? != 0 ]; then - error_exit "Failed waiting for $INSTANCE_ID to leave $elb" - fi -done - -msg "Finished $(basename $0) at $(/bin/date "+%F %T")" - -end_sec=$(/bin/date +%s.%N) -elapsed_seconds=$(echo "$end_sec - $start_sec" | /usr/bin/bc) - -msg "Elapsed time: $elapsed_seconds" diff --git a/scripts/register_with_elb.sh b/scripts/register_with_elb.sh deleted file mode 100755 index 7e6fb80ac..000000000 --- a/scripts/register_with_elb.sh +++ /dev/null @@ -1,78 +0,0 @@ -#!/bin/bash -# -# Copyright 2014 Amazon.com, Inc. or its affiliates. All Rights Reserved. -# -# Licensed under the Apache License, Version 2.0 (the "License"). -# You may not use this file except in compliance with the License. -# A copy of the License is located at -# -# http://aws.amazon.com/apache2.0 -# -# or in the "license" file accompanying this file. This file is distributed -# on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either -# express or implied. See the License for the specific language governing -# permissions and limitations under the License. - -. $(dirname $0)/common_functions.sh - -msg "Running AWS CLI with region: $(get_instance_region)" - -# get this instance's ID -INSTANCE_ID=$(get_instance_id) -if [ $? != 0 -o -z "$INSTANCE_ID" ]; then - error_exit "Unable to get this instance's ID; cannot continue." -fi - -# Get current time -msg "Started $(basename $0) at $(/bin/date "+%F %T")" -start_sec=$(/bin/date +%s.%N) - -msg "Getting relevant load balancer" -INSTANCE_NAME=$(get_instance_name_from_tags $INSTANCE_ID) - -if [[ "$(tr [:upper:] [:lower:] <<< "${INSTANCE_NAME}")" =~ 'delivery' ]]; then - msg "NO ELBs for delivery" - exit 0 -fi - -get_elb_name_for_instance_name $INSTANCE_NAME -ELB_LIST=$ELB_NAME -get_elb_list $INSTANCE_ID $ELB_NAME - -msg "Checking that user set at least one load balancer" -if test -z "$ELB_LIST"; then - error_exit "Must have at least one load balancer to register to" -fi - -# Loop through all LBs the user set, and attempt to register this instance to them. -for elb in $ELB_LIST; do - msg "Checking validity of load balancer named '$elb'" - validate_elb $INSTANCE_ID $elb - if [ $? != 0 ]; then - msg "Error validating $elb; cannot continue with this LB" - continue - fi - - msg "Registering $INSTANCE_ID to $elb" - register_instance $INSTANCE_ID $elb - - if [ $? != 0 ]; then - error_exit "Failed to register instance $INSTANCE_ID from ELB $elb" - fi -done - -# Wait for all Registrations to finish -msg "Waiting for instance to register to its load balancers" -for elb in $ELB_LIST; do - wait_for_state "elb" $INSTANCE_ID "InService" $elb - if [ $? != 0 ]; then - error_exit "Failed waiting for $INSTANCE_ID to return to $elb" - fi -done - -msg "Finished $(basename $0) at $(/bin/date "+%F %T")" - -end_sec=$(/bin/date +%s.%N) -elapsed_seconds=$(echo "$end_sec - $start_sec" | /usr/bin/bc) - -msg "Elapsed time: $elapsed_seconds" diff --git a/scripts/stop_celery.py b/scripts/stop_celery.py deleted file mode 100755 index 71b10ec03..000000000 --- a/scripts/stop_celery.py +++ /dev/null @@ -1,82 +0,0 @@ -#!/usr/bin/env python3 -""" - -Scipt used to stop celery in AWS environments. -This is used from upstart to issue a TERM signal to the master celery process. - -This will then allow the worker threads to stop, after completing -whatever tasks that are in flight. - -Note the script blocks for up to 15minutes, which is long enough to allow our -longest possible task to complete. If it can return quicker it will. - -Usage: - ./stop_celery.py - -Example: - ./stop_celery.py /tmp/celery.pid -""" - -import os -from docopt import docopt -import re -import subprocess -from time import sleep - - -def strip_white_space(from_this): - return re.sub(r'\s+', '', from_this) - - -def get_pid_from_file(filename): - """ - Open the file which MUST contain only the PID of the master celery process. - This is written to disk by the start celery command issued by upstart - """ - with open(filename) as f: - celery_pid = f.read() - return strip_white_space(celery_pid) - - -def issue_term_signal_to_pid(pid, celery_pid_file): - """ - Issues a TERM signal (15) to the master celery process. - - This method attempts to print out any response from this subprocess call. However this call is generally silent. - """ - print("Trying to stop ", celery_pid_file) - result = subprocess.Popen(['kill', '-15', pid], stdout=subprocess.PIPE, stderr=subprocess.PIPE) - for line in result.stdout.readlines(): - print(line.rstrip()) - for line in result.stderr.readlines(): - print(line.rstrip()) - - -def pid_still_running(pid): - """ - uses the proc filesystem to identify if the celery master pid is still around. - - Once the process stops this file no longer exists. Slim possibilty of a race condition here. - """ - return os.path.exists("/proc/" + pid) - - -if __name__ == "__main__": - arguments = docopt(__doc__) - celery_pid_file = arguments[''] - - celery_pid = get_pid_from_file(celery_pid_file) - - issue_term_signal_to_pid(celery_pid, celery_pid_file) - - """ - Blocking loop to check for the still running process. - 5 seconds between loops - 180 loops - Maximum block time of 900 seconds (15 minutes) - """ - iteration = 0 - while pid_still_running(celery_pid) and iteration < 180: - print("[", celery_pid_file, "] waited for ", iteration * 5, " secs") - sleep(5) - iteration += 1 diff --git a/tests/app/celery/test_ftp_update_tasks.py b/tests/app/celery/test_ftp_update_tasks.py index cf3e90ade..880189297 100644 --- a/tests/app/celery/test_ftp_update_tasks.py +++ b/tests/app/celery/test_ftp_update_tasks.py @@ -23,7 +23,6 @@ from app.celery.tasks import ( from tests.app.db import create_notification, create_service_callback_api from tests.conftest import set_config -from unittest.mock import call def test_update_job_to_sent_to_dvla(sample_letter_template, sample_letter_job): @@ -103,10 +102,6 @@ def test_update_letter_notifications_statuses_persisted(notify_api, mocker, samp sent_letter.reference, failed_letter.reference) mocker.patch('app.celery.tasks.s3.get_s3_file', return_value=valid_file) - send_mock = mocker.patch( - 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' - ) - update_letter_notifications_statuses(filename='foo.txt') assert sent_letter.status == NOTIFICATION_DELIVERED @@ -116,10 +111,6 @@ def test_update_letter_notifications_statuses_persisted(notify_api, mocker, samp assert failed_letter.billable_units == 2 assert failed_letter.updated_at - calls = [call([str(failed_letter.id)], queue="service-callbacks"), - call([str(sent_letter.id)], queue="service-callbacks")] - send_mock.assert_has_calls(calls, any_order=True) - def test_update_letter_notifications_does_not_call_send_callback_if_no_db_entry(notify_api, mocker, sample_letter_template): @@ -159,9 +150,6 @@ def test_update_letter_notifications_to_error_updates_based_on_notification_refe sample_letter_template, mocker ): - send_mock = mocker.patch( - 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' - ) first = create_notification(sample_letter_template, reference='first ref') second = create_notification(sample_letter_template, reference='second ref') create_service_callback_api(service=sample_letter_template.service, url="https://original_url.com") @@ -174,4 +162,3 @@ def test_update_letter_notifications_to_error_updates_based_on_notification_refe assert first.sent_at is None assert first.updated_at == dt assert second.status == NOTIFICATION_CREATED - assert send_mock.called diff --git a/tests/app/celery/test_letters_pdf_tasks.py b/tests/app/celery/test_letters_pdf_tasks.py index 78a58306c..5c2532934 100644 --- a/tests/app/celery/test_letters_pdf_tasks.py +++ b/tests/app/celery/test_letters_pdf_tasks.py @@ -155,16 +155,18 @@ def test_collate_letter_pdfs_for_day(notify_api, mocker): assert mock_celery.call_args_list[0] == call( name='zip-and-send-letter-pdfs', kwargs={'filenames_to_zip': ['A.PDF', 'B.pDf']}, - queue='process-ftp-tasks' + queue='process-ftp-tasks', + compression='zlib' ) assert mock_celery.call_args_list[1] == call( name='zip-and-send-letter-pdfs', kwargs={'filenames_to_zip': ['C.pdf']}, - queue='process-ftp-tasks' + queue='process-ftp-tasks', + compression='zlib' ) -def test_group_letters(notify_api): +def test_group_letters_splits_on_file_size(notify_api): letters = [ # ends under max but next one is too big {'Key': 'A.pdf', 'Size': 1}, {'Key': 'B.pdf', 'Size': 2}, @@ -190,6 +192,63 @@ def test_group_letters(notify_api): assert next(x, None) is None +def test_group_letters_splits_on_file_count(notify_api): + letters = [ + {'Key': 'A.pdf', 'Size': 1}, + {'Key': 'B.pdf', 'Size': 2}, + {'Key': 'C.pdf', 'Size': 3}, + {'Key': 'D.pdf', 'Size': 1}, + {'Key': 'E.pdf', 'Size': 1}, + {'Key': 'F.pdf', 'Size': 5}, + {'Key': 'G.pdf', 'Size': 6}, + {'Key': 'H.pdf', 'Size': 1}, + {'Key': 'I.pdf', 'Size': 1}, + ] + + with set_config_values(notify_api, {'MAX_LETTER_PDF_COUNT_PER_ZIP': 3}): + x = group_letters(letters) + + assert next(x) == [{'Key': 'A.pdf', 'Size': 1}, {'Key': 'B.pdf', 'Size': 2}, {'Key': 'C.pdf', 'Size': 3}] + assert next(x) == [{'Key': 'D.pdf', 'Size': 1}, {'Key': 'E.pdf', 'Size': 1}, {'Key': 'F.pdf', 'Size': 5}] + assert next(x) == [{'Key': 'G.pdf', 'Size': 6}, {'Key': 'H.pdf', 'Size': 1}, {'Key': 'I.pdf', 'Size': 1}] + # make sure iterator is exhausted + assert next(x, None) is None + + +def test_group_letters_splits_on_file_size_and_file_count(notify_api): + letters = [ + # ends under max file size but next file is too big + {'Key': 'A.pdf', 'Size': 1}, + {'Key': 'B.pdf', 'Size': 2}, + # ends on exactly max number of files and file size + {'Key': 'C.pdf', 'Size': 3}, + {'Key': 'D.pdf', 'Size': 1}, + {'Key': 'E.pdf', 'Size': 1}, + # exactly max file size goes in next file + {'Key': 'F.pdf', 'Size': 5}, + # file size is within max but number of files reaches limit + {'Key': 'G.pdf', 'Size': 1}, + {'Key': 'H.pdf', 'Size': 1}, + {'Key': 'I.pdf', 'Size': 1}, + # whatever's left goes in last list + {'Key': 'J.pdf', 'Size': 1}, + ] + + with set_config_values(notify_api, { + 'MAX_LETTER_PDF_ZIP_FILESIZE': 5, + 'MAX_LETTER_PDF_COUNT_PER_ZIP': 3 + }): + x = group_letters(letters) + + assert next(x) == [{'Key': 'A.pdf', 'Size': 1}, {'Key': 'B.pdf', 'Size': 2}] + assert next(x) == [{'Key': 'C.pdf', 'Size': 3}, {'Key': 'D.pdf', 'Size': 1}, {'Key': 'E.pdf', 'Size': 1}] + assert next(x) == [{'Key': 'F.pdf', 'Size': 5}] + assert next(x) == [{'Key': 'G.pdf', 'Size': 1}, {'Key': 'H.pdf', 'Size': 1}, {'Key': 'I.pdf', 'Size': 1}] + assert next(x) == [{'Key': 'J.pdf', 'Size': 1}] + # make sure iterator is exhausted + assert next(x, None) is None + + def test_group_letters_ignores_non_pdfs(notify_api): letters = [{'Key': 'A.zip'}] assert list(group_letters(letters)) == [] diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index 882fc5b9f..18f1fa9e5 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -1081,6 +1081,25 @@ def test_save_letter_uses_template_reply_to_text(mocker, notify_db_session): assert notification_db.reply_to_text == "Template address contact" +def test_save_sms_uses_sms_sender_reply_to_text(mocker, notify_db_session): + service = create_service_with_defined_sms_sender(sms_sender_value='07123123123') + template = create_template(service=service) + + notification = _notification_json(template, to="07700 900205") + mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') + + notification_id = uuid.uuid4() + save_sms( + service.id, + notification_id, + encryption.encrypt(notification), + key_type=KEY_TYPE_TEST + ) + + persisted_notification = Notification.query.one() + assert persisted_notification.reply_to_text == '447123123123' + + def test_save_letter_calls_update_noti_to_sent_task_with_letters_as_pdf_permission_in_research_mode( mocker, notify_db_session, sample_letter_job): sample_letter_job.service.research_mode = True diff --git a/tests/app/invite/test_invite_rest.py b/tests/app/invite/test_invite_rest.py index ebeeb3492..2071651a3 100644 --- a/tests/app/invite/test_invite_rest.py +++ b/tests/app/invite/test_invite_rest.py @@ -1,22 +1,41 @@ import json +import pytest import uuid from app.models import Notification, SMS_AUTH_TYPE, EMAIL_AUTH_TYPE from tests import create_authorization_header -def test_create_invited_user(admin_request, sample_service, mocker, invitation_email_template): +@pytest.mark.parametrize('extra_args, expected_start_of_invite_url', [ + ( + {}, + 'http://localhost:6012/invitation/' + ), + ( + {'invite_link_host': 'https://www.example.com'}, + 'https://www.example.com/invitation/' + ), +]) +def test_create_invited_user( + admin_request, + sample_service, + mocker, + invitation_email_template, + extra_args, + expected_start_of_invite_url, +): mocked = mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') email_address = 'invited_user@service.gov.uk' invite_from = sample_service.users[0] - data = { - 'service': str(sample_service.id), - 'email_address': email_address, - 'from_user': str(invite_from.id), - 'permissions': 'send_messages,manage_service,manage_api_keys', - 'auth_type': EMAIL_AUTH_TYPE - } + data = dict( + service=str(sample_service.id), + email_address=email_address, + from_user=str(invite_from.id), + permissions='send_messages,manage_service,manage_api_keys', + auth_type=EMAIL_AUTH_TYPE, + **extra_args + ) json_resp = admin_request.post( 'invite.create_invited_user', @@ -33,7 +52,15 @@ def test_create_invited_user(admin_request, sample_service, mocker, invitation_e assert json_resp['data']['id'] notification = Notification.query.first() + assert notification.reply_to_text == invite_from.email_address + + assert len(notification.personalisation.keys()) == 3 + assert notification.personalisation['service_name'] == 'Sample service' + assert notification.personalisation['user_name'] == 'Test User' + assert notification.personalisation['url'].startswith(expected_start_of_invite_url) + assert len(notification.personalisation['url']) > len(expected_start_of_invite_url) + mocked.assert_called_once_with([(str(notification.id))], queue="notify-internal-tasks") diff --git a/tests/app/service/test_send_one_off_notification.py b/tests/app/service/test_send_one_off_notification.py index e64102ec1..cbbe8f7dd 100644 --- a/tests/app/service/test_send_one_off_notification.py +++ b/tests/app/service/test_send_one_off_notification.py @@ -19,6 +19,7 @@ from tests.app.db import ( create_user, create_reply_to_email, create_letter_contact, + create_service_sms_sender, create_service, create_template ) @@ -238,9 +239,62 @@ def test_send_one_off_letter_notification_should_use_template_reply_to_text(samp research_mode=False, queue=None ) + assert notification.reply_to_text == "Edinburgh, ED1 1AA" +def test_send_one_off_sms_notification_should_use_sms_sender_reply_to_text(sample_service, celery_mock): + template = create_template(service=sample_service, template_type=SMS_TYPE) + sms_sender = create_service_sms_sender( + service=sample_service, + sms_sender='07123123123', + is_default=False + ) + + data = { + 'to': '07111111111', + 'template_id': str(template.id), + 'created_by': str(sample_service.created_by_id), + 'sender_id': str(sms_sender.id), + } + + notification_id = send_one_off_notification(service_id=sample_service.id, post_data=data) + notification = Notification.query.get(notification_id['id']) + celery_mock.assert_called_once_with( + notification=notification, + research_mode=False, + queue=None + ) + + assert notification.reply_to_text == "447123123123" + + +def test_send_one_off_sms_notification_should_use_default_service_reply_to_text(sample_service, celery_mock): + template = create_template(service=sample_service, template_type=SMS_TYPE) + sample_service.service_sms_senders[0].is_default = False + create_service_sms_sender( + service=sample_service, + sms_sender='07123123456', + is_default=True + ) + + data = { + 'to': '07111111111', + 'template_id': str(template.id), + 'created_by': str(sample_service.created_by_id), + } + + notification_id = send_one_off_notification(service_id=sample_service.id, post_data=data) + notification = Notification.query.get(notification_id['id']) + celery_mock.assert_called_once_with( + notification=notification, + research_mode=False, + queue=None + ) + + assert notification.reply_to_text == "447123123456" + + def test_send_one_off_notification_should_throw_exception_if_reply_to_id_doesnot_exist( sample_email_template ): diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index cba00ff9b..df669d258 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -98,6 +98,34 @@ def test_post_sms_notification_uses_inbound_number_as_sender(client, notify_db_s mocked.assert_called_once_with([str(notification_id)], queue='send-sms-tasks') +def test_post_sms_notification_uses_inbound_number_reply_to_as_sender(client, notify_db_session, mocker): + service = create_service_with_inbound_number(inbound_number='07123123123') + + template = create_template(service=service, content="Hello (( Name))\nYour thing is due soon") + mocked = mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') + data = { + 'phone_number': '+447700900855', + 'template_id': str(template.id), + 'personalisation': {' Name': 'Jo'} + } + auth_header = create_authorization_header(service_id=service.id) + + response = client.post( + path='/v2/notifications/sms', + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header]) + assert response.status_code == 201 + resp_json = json.loads(response.get_data(as_text=True)) + assert validate(resp_json, post_sms_response) == resp_json + notifications = Notification.query.all() + assert len(notifications) == 1 + notification_id = notifications[0].id + assert resp_json['id'] == str(notification_id) + assert resp_json['content']['from_number'] == '447123123123' + assert notifications[0].reply_to_text == '447123123123' + mocked.assert_called_once_with([str(notification_id)], queue='send-sms-tasks') + + def test_post_sms_notification_returns_201_with_sms_sender_id( client, sample_template_with_placeholders, mocker ): @@ -125,6 +153,33 @@ def test_post_sms_notification_returns_201_with_sms_sender_id( mocked.assert_called_once_with([resp_json['id']], queue='send-sms-tasks') +def test_post_sms_notification_uses_sms_sender_id_reply_to( + client, sample_template_with_placeholders, mocker +): + sms_sender = create_service_sms_sender(service=sample_template_with_placeholders.service, sms_sender='07123123123') + mocked = mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') + data = { + 'phone_number': '+447700900855', + 'template_id': str(sample_template_with_placeholders.id), + 'personalisation': {' Name': 'Jo'}, + 'sms_sender_id': str(sms_sender.id) + } + auth_header = create_authorization_header(service_id=sample_template_with_placeholders.service_id) + + response = client.post( + path='/v2/notifications/sms', + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header]) + assert response.status_code == 201 + resp_json = json.loads(response.get_data(as_text=True)) + assert validate(resp_json, post_sms_response) == resp_json + assert resp_json['content']['from_number'] == '447123123123' + notifications = Notification.query.all() + assert len(notifications) == 1 + assert notifications[0].reply_to_text == '447123123123' + mocked.assert_called_once_with([resp_json['id']], queue='send-sms-tasks') + + def test_notification_reply_to_text_is_original_value_if_sender_is_changed_after_post_notification( client, sample_template, mocker ):