From a5cab2a91b4c5b478287f48691f0d4aed0b556a5 Mon Sep 17 00:00:00 2001 From: Jimmy Royer Date: Tue, 14 Nov 2023 16:26:39 -0500 Subject: [PATCH 1/8] Move some verbose celery INFO-level logs to reduce log size (#2019) * Remove celery task time execution log * Moved celery task time execution log to DEBUG, sending as statsd metrics * Moved info level log into debug * Formatting * Removed types; moved some more info log into debug * Fixed errors + more info logs into debug --- app/celery/celery.py | 10 ++++++++-- app/celery/provider_tasks.py | 2 +- app/celery/tasks.py | 12 ++++++------ 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/app/celery/celery.py b/app/celery/celery.py index b1758f73c0..21e78e7949 100644 --- a/app/celery/celery.py +++ b/app/celery/celery.py @@ -12,13 +12,19 @@ def worker_process_shutdown(sender, signal, pid, exitcode, **kwargs): def make_task(app): + from app import statsd_client + class NotifyTask(Task): abstract = True start = None def on_success(self, retval, task_id, args, kwargs): - elapsed_time = time.time() - self.start - app.logger.info("{task_name} took {time}".format(task_name=self.name, time="{0:.4f}".format(elapsed_time))) + task_name = self.name + now = time.time() + statsd_client.timing_with_dates(f"celery-task.{task_name}.total-time", now, self.start) + + elapsed_time = now - self.start + app.logger.debug("{task_name} took {time}".format(task_name=task_name, time="{0:.4f}".format(elapsed_time))) def on_failure(self, exc, task_id, args, kwargs, einfo): # ensure task will log exceptions to correct handlers diff --git a/app/celery/provider_tasks.py b/app/celery/provider_tasks.py index bfdc671a2e..f150bb2b5e 100644 --- a/app/celery/provider_tasks.py +++ b/app/celery/provider_tasks.py @@ -63,7 +63,7 @@ def deliver_sms(self, notification_id): @statsd(namespace="tasks") def deliver_email(self, notification_id): try: - current_app.logger.info("Start sending email for notification id: {}".format(notification_id)) + current_app.logger.debug("Start sending email for notification id: {}".format(notification_id)) notification = notifications_dao.get_notification_by_id(notification_id) if not notification: raise NoResultFound() diff --git a/app/celery/tasks.py b/app/celery/tasks.py index d917c7cc9a..ae79b688d0 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -277,12 +277,12 @@ def save_smss(self, service_id: Optional[str], signed_notifications: List[Signed try: # If the data is not present in the encrypted data then fallback on whats needed for process_job. saved_notifications = persist_notifications(verified_notifications) - current_app.logger.info( + current_app.logger.debug( f"Saved following notifications into db: {notification_id_queue.keys()} associated with receipt {receipt}" ) if receipt: acknowledge_receipt(SMS_TYPE, process_type, receipt) - current_app.logger.info( + current_app.logger.debug( f"Batch saving: receipt_id {receipt} removed from buffer queue for notification_id {notification_id} for process_type {process_type}" ) else: @@ -297,7 +297,7 @@ def save_smss(self, service_id: Optional[str], signed_notifications: List[Signed signed_and_verified = list(zip(signed_notifications, verified_notifications)) handle_batch_error_and_forward(self, signed_and_verified, SMS_TYPE, e, receipt, template) - current_app.logger.info(f"Sending following sms notifications to AWS: {notification_id_queue.keys()}") + current_app.logger.debug(f"Sending following sms notifications to AWS: {notification_id_queue.keys()}") for notification_obj in saved_notifications: try: if not current_app.config["FF_EMAIL_DAILY_LIMIT"]: @@ -383,7 +383,7 @@ def save_emails(self, _service_id: Optional[str], signed_notifications: List[Sig try: # If the data is not present in the encrypted data then fallback on whats needed for process_job saved_notifications = persist_notifications(verified_notifications) - current_app.logger.info( + current_app.logger.debug( f"Saved following notifications into db: {notification_id_queue.keys()} associated with receipt {receipt}" ) if receipt: @@ -392,7 +392,7 @@ def save_emails(self, _service_id: Optional[str], signed_notifications: List[Sig # at this point in the code we have a list of notifications (saved_notifications) # which could use multiple templates acknowledge_receipt(EMAIL_TYPE, process_type, receipt) - current_app.logger.info( + current_app.logger.debug( f"Batch saving: receipt_id {receipt} removed from buffer queue for notification_id {notification_id} for process_type {process_type}" ) else: @@ -415,7 +415,7 @@ def try_to_send_notifications_to_queue(notification_id_queue, service, saved_not Loop through saved_notifications, check if the service has hit their daily rate limit, and if not, call send_notification_to_queue on notification """ - current_app.logger.info(f"Sending following email notifications to AWS: {notification_id_queue.keys()}") + current_app.logger.debug(f"Sending following email notifications to AWS: {notification_id_queue.keys()}") # todo: fix this potential bug # service is whatever it was set to last in the for loop above. # at this point in the code we have a list of notifications (saved_notifications) From 069af465c2a1c1d6d789a873c1d4baa7dd252877 Mon Sep 17 00:00:00 2001 From: Steve Astels Date: Tue, 14 Nov 2023 17:10:59 -0500 Subject: [PATCH 2/8] Revert "Move some verbose celery INFO-level logs to reduce log size (#2019)" (#2021) This reverts commit a5cab2a91b4c5b478287f48691f0d4aed0b556a5. --- app/celery/celery.py | 10 ++-------- app/celery/provider_tasks.py | 2 +- app/celery/tasks.py | 12 ++++++------ 3 files changed, 9 insertions(+), 15 deletions(-) diff --git a/app/celery/celery.py b/app/celery/celery.py index 21e78e7949..b1758f73c0 100644 --- a/app/celery/celery.py +++ b/app/celery/celery.py @@ -12,19 +12,13 @@ def worker_process_shutdown(sender, signal, pid, exitcode, **kwargs): def make_task(app): - from app import statsd_client - class NotifyTask(Task): abstract = True start = None def on_success(self, retval, task_id, args, kwargs): - task_name = self.name - now = time.time() - statsd_client.timing_with_dates(f"celery-task.{task_name}.total-time", now, self.start) - - elapsed_time = now - self.start - app.logger.debug("{task_name} took {time}".format(task_name=task_name, time="{0:.4f}".format(elapsed_time))) + elapsed_time = time.time() - self.start + app.logger.info("{task_name} took {time}".format(task_name=self.name, time="{0:.4f}".format(elapsed_time))) def on_failure(self, exc, task_id, args, kwargs, einfo): # ensure task will log exceptions to correct handlers diff --git a/app/celery/provider_tasks.py b/app/celery/provider_tasks.py index f150bb2b5e..bfdc671a2e 100644 --- a/app/celery/provider_tasks.py +++ b/app/celery/provider_tasks.py @@ -63,7 +63,7 @@ def deliver_sms(self, notification_id): @statsd(namespace="tasks") def deliver_email(self, notification_id): try: - current_app.logger.debug("Start sending email for notification id: {}".format(notification_id)) + current_app.logger.info("Start sending email for notification id: {}".format(notification_id)) notification = notifications_dao.get_notification_by_id(notification_id) if not notification: raise NoResultFound() diff --git a/app/celery/tasks.py b/app/celery/tasks.py index ae79b688d0..d917c7cc9a 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -277,12 +277,12 @@ def save_smss(self, service_id: Optional[str], signed_notifications: List[Signed try: # If the data is not present in the encrypted data then fallback on whats needed for process_job. saved_notifications = persist_notifications(verified_notifications) - current_app.logger.debug( + current_app.logger.info( f"Saved following notifications into db: {notification_id_queue.keys()} associated with receipt {receipt}" ) if receipt: acknowledge_receipt(SMS_TYPE, process_type, receipt) - current_app.logger.debug( + current_app.logger.info( f"Batch saving: receipt_id {receipt} removed from buffer queue for notification_id {notification_id} for process_type {process_type}" ) else: @@ -297,7 +297,7 @@ def save_smss(self, service_id: Optional[str], signed_notifications: List[Signed signed_and_verified = list(zip(signed_notifications, verified_notifications)) handle_batch_error_and_forward(self, signed_and_verified, SMS_TYPE, e, receipt, template) - current_app.logger.debug(f"Sending following sms notifications to AWS: {notification_id_queue.keys()}") + current_app.logger.info(f"Sending following sms notifications to AWS: {notification_id_queue.keys()}") for notification_obj in saved_notifications: try: if not current_app.config["FF_EMAIL_DAILY_LIMIT"]: @@ -383,7 +383,7 @@ def save_emails(self, _service_id: Optional[str], signed_notifications: List[Sig try: # If the data is not present in the encrypted data then fallback on whats needed for process_job saved_notifications = persist_notifications(verified_notifications) - current_app.logger.debug( + current_app.logger.info( f"Saved following notifications into db: {notification_id_queue.keys()} associated with receipt {receipt}" ) if receipt: @@ -392,7 +392,7 @@ def save_emails(self, _service_id: Optional[str], signed_notifications: List[Sig # at this point in the code we have a list of notifications (saved_notifications) # which could use multiple templates acknowledge_receipt(EMAIL_TYPE, process_type, receipt) - current_app.logger.debug( + current_app.logger.info( f"Batch saving: receipt_id {receipt} removed from buffer queue for notification_id {notification_id} for process_type {process_type}" ) else: @@ -415,7 +415,7 @@ def try_to_send_notifications_to_queue(notification_id_queue, service, saved_not Loop through saved_notifications, check if the service has hit their daily rate limit, and if not, call send_notification_to_queue on notification """ - current_app.logger.debug(f"Sending following email notifications to AWS: {notification_id_queue.keys()}") + current_app.logger.info(f"Sending following email notifications to AWS: {notification_id_queue.keys()}") # todo: fix this potential bug # service is whatever it was set to last in the for loop above. # at this point in the code we have a list of notifications (saved_notifications) From c3df334dedaafc0f4729b6cadefb65673bac4ce3 Mon Sep 17 00:00:00 2001 From: Jimmy Royer Date: Tue, 14 Nov 2023 17:52:01 -0500 Subject: [PATCH 3/8] Enable low retry period for SMS high queue in production (#2020) --- app/config.py | 1 - 1 file changed, 1 deletion(-) diff --git a/app/config.py b/app/config.py index 5b2453a094..0e56f6110d 100644 --- a/app/config.py +++ b/app/config.py @@ -709,7 +709,6 @@ class Production(Config): API_RATE_LIMIT_ENABLED = True CHECK_PROXY_HEADER = False CRONITOR_ENABLED = False - FF_CELERY_CUSTOM_TASK_PARAMS = False class Staging(Production): From 9cf34043b1a99136132926c2588f5fc908af55f0 Mon Sep 17 00:00:00 2001 From: Andrew Date: Wed, 15 Nov 2023 09:29:19 -0400 Subject: [PATCH 4/8] Feat/secure api revocation route (#2015) * feat(api key revoke): add new auth type to protect revocation route * feat(api key revoke): protect api key revocation route with new auth * test(api key revoke): add scenarios to test for happy path and missing data, invalid auth * cypress(api key revocation): create a key using admin auth and then revoke it using the API with SRE auth * chore: formatting * chore: remove unused var * chore: remove unused import * chore: remove unused vars * task: add SRE_CLIENT_SECRET to list of sensitive keys, add var to example * chore: add default value for new env var * test(api key revoke): ensure method cannot be called using admin auth * chore: set cypress default environment back to staging * chore: update comments and example to make them clearer * chore: fix comment --------- Co-authored-by: Jumana B --- .env.example | 1 + app/__init__.py | 5 +- app/api_key/rest.py | 97 +++++++++++-------- app/authentication/auth.py | 15 +++ app/config.py | 6 ++ tests/__init__.py | 8 ++ tests/app/api_key/test_rest.py | 74 +++++++++++++- tests_cypress/cypress/Notify/NotifyAPI.js | 67 ++++++++++++- tests_cypress/cypress/e2e/api/sre_tools.cy.js | 43 ++++++++ 9 files changed, 264 insertions(+), 52 deletions(-) create mode 100644 tests_cypress/cypress/e2e/api/sre_tools.cy.js diff --git a/.env.example b/.env.example index e58e05bbb4..8e60a9b5ae 100644 --- a/.env.example +++ b/.env.example @@ -1,6 +1,7 @@ NOTIFY_ENVIRONMENT=development ADMIN_CLIENT_SECRET=dev-notify-secret-key +SRE_CLIENT_SECRET=dev-notify-secret-key SECRET_KEY=dev-notify-secret-key DANGEROUS_SALT=dev-notify-salt diff --git a/app/__init__.py b/app/__init__.py index 2a9cf36b0e..77a2a7d545 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -169,11 +169,12 @@ def register_notify_blueprint(application, blueprint, auth_function, prefix=None def register_blueprint(application): from app.accept_invite.rest import accept_invite - from app.api_key.rest import api_key_blueprint + from app.api_key.rest import api_key_blueprint, sre_tools_blueprint from app.authentication.auth import ( requires_admin_auth, requires_auth, requires_no_auth, + requires_sre_auth, ) from app.billing.rest import billing_blueprint from app.complaint.complaint_rest import complaint_blueprint @@ -233,6 +234,8 @@ def register_blueprint(application): register_notify_blueprint(application, api_key_blueprint, requires_admin_auth, "/api-key") + register_notify_blueprint(application, sre_tools_blueprint, requires_sre_auth, "/sre-tools") + register_notify_blueprint(application, letter_job, requires_admin_auth) register_notify_blueprint(application, letter_callback_blueprint, requires_no_auth) diff --git a/app/api_key/rest.py b/app/api_key/rest.py index 6d8fdd4301..9f0959ad0e 100644 --- a/app/api_key/rest.py +++ b/app/api_key/rest.py @@ -19,6 +19,9 @@ api_key_blueprint = Blueprint("api_key", __name__) register_errors(api_key_blueprint) +sre_tools_blueprint = Blueprint("sre_tools", __name__) +register_errors(sre_tools_blueprint) + @api_key_blueprint.route("//summary-statistics", methods=["GET"]) def get_api_key_stats(api_key_id): @@ -103,58 +106,68 @@ def send_api_key_revokation_email(service_id, api_key_name, api_key_information) return -@api_key_blueprint.route("/revoke-api-keys", methods=["POST"]) +@sre_tools_blueprint.route("/api-key-revoke", methods=["POST"]) def revoke_api_keys(): """ - We take a list of api keys and revoke them. The data is of the form: - [ - { - "token": "NMIfyYncKcRALEXAMPLE", - "type": "mycompany_api_token", - "url": "https://github.com/octocat/Hello-World/blob/12345600b9cbe38a219f39a9941c9319b600c002/foo/bar.txt", - "source": "content", - } - ] - - The function does 3 things: - 1. Finds the api key by the token - 2. Revokes the api key + This method accepts a single api key and revokes it. The data is of the form: + { + "token": "gcntfy-key-name-uuid-uuid", + "type": "mycompany_api_token", + "url": "https://github.com/octocat/Hello-World/blob/12345600b9cbe38a219f39a9941c9319b600c002/foo/bar.txt", + "source": "content", + } + + The function does 4 things: + 1. Finds the api key by API key itself + 2. Revokes the API key 3. Saves the source and url into the compromised_key_info field - 4. Sends the service owners of the api key an email notification indicating that the key has been revoked + 4. TODO: Sends the service owners of the api key an email notification indicating that the key has been revoked """ try: - data = request.get_json() + api_key_data = request.get_json() + # check for correct payload + if ( + isinstance(api_key_data, list) + or api_key_data.get("token") is None + or api_key_data.get("type") is None + or api_key_data.get("url") is None + or api_key_data.get("source") is None + ): + raise InvalidRequest("Invalid payload", status_code=400) except werkzeug.exceptions.BadRequest as errors: raise InvalidRequest(errors, status_code=400) # Step 1 - for api_key_data in data: - try: - # take last 36 chars of string so that it works even if the full key is provided. - api_key_token = api_key_data["token"][-36:] - api_key = get_api_key_by_secret(api_key_token) - except Exception: - current_app.logger.error(f"API key not found for token {api_key_data['type']}") - continue # skip to next api key - - # Step 2 - expire_api_key(api_key.service_id, api_key.id) - - current_app.logger.info("Expired api key {} for service {}".format(api_key.id, api_key.service_id)) - - # Step 3 - update_compromised_api_key_info( - api_key.service_id, - api_key.id, - { - "time_of_revocation": str(datetime.utcnow()), - "type": api_key_data["type"], - "url": api_key_data["url"], - "source": api_key_data["source"], - }, + try: + # take last 36 chars of string so that it works even if the full key is provided. + api_key_token = api_key_data["token"][-36:] + api_key = get_api_key_by_secret(api_key_token) + except Exception: + current_app.logger.error( + "Revoke api key: API key not found for token {}".format(api_key_data["token"]) + if api_key_data.get("token") + else "Revoke api key: no token provided" ) + raise InvalidRequest("Invalid request", status_code=400) + + # Step 2 + expire_api_key(api_key.service_id, api_key.id) - # Step 4 - send_api_key_revokation_email(api_key.service_id, api_key.name, api_key_data) + current_app.logger.info("Expired api key {} for service {}".format(api_key.id, api_key.service_id)) + + # Step 3 + update_compromised_api_key_info( + api_key.service_id, + api_key.id, + { + "time_of_revocation": str(datetime.utcnow()), + "type": api_key_data["type"], + "url": api_key_data["url"], + "source": api_key_data["source"], + }, + ) + + # Step 4 + send_api_key_revokation_email(api_key.service_id, api_key.name, api_key_data) return jsonify(result="ok"), 201 diff --git a/app/authentication/auth.py b/app/authentication/auth.py index 2fb200a323..144c89079f 100644 --- a/app/authentication/auth.py +++ b/app/authentication/auth.py @@ -93,6 +93,21 @@ def requires_admin_auth(): raise AuthError("Unauthorized, admin authentication token required", 401) +def requires_sre_auth(): + request_helper.check_proxy_header_before_request() + + auth_type, auth_token = get_auth_token(request) + if auth_type != JWT_AUTH_TYPE: + raise AuthError("Invalid scheme: can only use JWT for sre authentication", 401) + client = __get_token_issuer(auth_token) + + if client == current_app.config.get("SRE_USER_NAME"): + g.service_id = current_app.config.get("SRE_USER_NAME") + return handle_admin_key(auth_token, current_app.config.get("SRE_CLIENT_SECRET")) + else: + raise AuthError("Unauthorized, sre authentication token required", 401) + + def requires_auth(): request_helper.check_proxy_header_before_request() diff --git a/app/config.py b/app/config.py index 0e56f6110d..2e4a06059b 100644 --- a/app/config.py +++ b/app/config.py @@ -592,6 +592,10 @@ class Config(object): FF_EMAIL_DAILY_LIMIT = env.bool("FF_EMAIL_DAILY_LIMIT", False) FF_SALESFORCE_CONTACT = env.bool("FF_SALESFORCE_CONTACT", False) + # SRE Tools auth keys + SRE_USER_NAME = "SRE_CLIENT_USER" + SRE_CLIENT_SECRET = os.getenv("SRE_CLIENT_SECRET") + @classmethod def get_sensitive_config(cls) -> list[str]: "List of config keys that contain sensitive information" @@ -612,6 +616,7 @@ def get_sensitive_config(cls) -> list[str]: "SALESFORCE_SECURITY_TOKEN", "TEMPLATE_PREVIEW_API_KEY", "DOCUMENT_DOWNLOAD_API_KEY", + "SRE_CLIENT_SECRET", ] @classmethod @@ -639,6 +644,7 @@ class Development(Config): ADMIN_CLIENT_SECRET = os.getenv("ADMIN_CLIENT_SECRET", "dev-notify-secret-key") SECRET_KEY = env.list("SECRET_KEY", ["dev-notify-secret-key"]) DANGEROUS_SALT = os.getenv("DANGEROUS_SALT", "dev-notify-salt ") + SRE_CLIENT_SECRET = os.getenv("SRE_CLIENT_SECRET", "dev-notify-secret-key") NOTIFY_ENVIRONMENT = "development" NOTIFICATION_QUEUE_PREFIX = os.getenv("NOTIFICATION_QUEUE_PREFIX", "notification-canada-ca") diff --git a/tests/__init__.py b/tests/__init__.py index a1debdd999..5cf5c9313d 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -34,6 +34,14 @@ def create_authorization_header(service_id=None, key_type=KEY_TYPE_NORMAL): return "Authorization", "Bearer {}".format(token) +def create_sre_authorization_header(): + client_id = current_app.config["SRE_USER_NAME"] + secret = current_app.config["SRE_CLIENT_SECRET"] + + token = create_jwt_token(secret=secret, client_id=client_id) + return "Authorization", "Bearer {}".format(token) + + def unwrap_function(fn): """ Given a function, returns its undecorated original. diff --git a/tests/app/api_key/test_rest.py b/tests/app/api_key/test_rest.py index 669d2a32ee..2452b94f2b 100644 --- a/tests/app/api_key/test_rest.py +++ b/tests/app/api_key/test_rest.py @@ -1,8 +1,12 @@ from datetime import datetime +import pytest +from flask import url_for + from app import DATETIME_FORMAT from app.dao.api_key_dao import get_api_key_by_secret, get_unsigned_secret from app.models import KEY_TYPE_NORMAL +from tests import create_sre_authorization_header from tests.app.db import ( create_api_key, create_notification, @@ -79,21 +83,81 @@ def test_get_api_keys_ranked(admin_request, notify_db, notify_db_session): class TestApiKeyRevocation: - def test_revoke_api_keys(self, admin_request, notify_db, notify_db_session): + def test_revoke_api_keys_with_valid_auth(self, client, notify_db, notify_db_session, mocker): service = create_service(service_name="Service 1") api_key_1 = create_api_key(service, key_type=KEY_TYPE_NORMAL, key_name="Key 1") unsigned_secret = get_unsigned_secret(api_key_1.id) - admin_request.post( - "api_key.revoke_api_keys", - _data=[{"token": unsigned_secret, "type": "cds-tester", "url": "https://example.com", "source": "cds-tester"}], - _expected_status=201, + sre_auth_header = create_sre_authorization_header() + response = client.post( + url_for("sre_tools.revoke_api_keys"), + headers=[sre_auth_header], + json={"token": unsigned_secret, "type": "cds-tester", "url": "https://example.com", "source": "cds-tester"}, ) # Get api key from DB api_key_1 = get_api_key_by_secret(api_key_1.secret) + assert response.status_code == 201 assert api_key_1.expiry_date is not None assert api_key_1.compromised_key_info["type"] == "cds-tester" assert api_key_1.compromised_key_info["url"] == "https://example.com" assert api_key_1.compromised_key_info["source"] == "cds-tester" assert api_key_1.compromised_key_info["time_of_revocation"] + + def test_revoke_api_keys_fails_with_no_auth(self, client, notify_db, notify_db_session, mocker): + service = create_service(service_name="Service 1") + api_key_1 = create_api_key(service, key_type=KEY_TYPE_NORMAL, key_name="Key 1") + unsigned_secret = get_unsigned_secret(api_key_1.id) + + response = client.post( + url_for("sre_tools.revoke_api_keys"), + headers=[], + json={"token": unsigned_secret, "type": "cds-tester", "url": "https://example.com", "source": "cds-tester"}, + ) + + assert response.status_code == 401 + + @pytest.mark.parametrize( + "payload", + ( + { + # no token + "type": "cds-tester", + "url": "https://example.com", + "source": "cds-tester", + }, + { + "token": "token", + # no type + "url": "https://example.com", + "source": "cds-tester", + }, + { + "token": "token", + "type": "cds-tester", + # no url + "source": "cds-tester", + }, + { + "token": "token", + "type": "cds-tester", + "url": "https://example.com", + # no source + }, + { + # no anything + }, + {"token": "token", "type": "cds-tester", "url": "https://example.com", "source": "cds-tester"}, # invalid token + ), + ) + def test_revoke_api_keys_fails_with_400_missing_or_invalid_payload( + self, client, notify_db, notify_db_session, mocker, payload + ): + sre_auth_header = create_sre_authorization_header() + response = client.post( + url_for("sre_tools.revoke_api_keys"), + headers=[sre_auth_header], + json=payload, + ) + + assert response.status_code == 400 diff --git a/tests_cypress/cypress/Notify/NotifyAPI.js b/tests_cypress/cypress/Notify/NotifyAPI.js index 714a6c7450..02ec5afbe5 100644 --- a/tests_cypress/cypress/Notify/NotifyAPI.js +++ b/tests_cypress/cypress/Notify/NotifyAPI.js @@ -2,21 +2,32 @@ import jwt from "jsonwebtoken"; import config from "../../config"; const Utilities = { - CreateJWT: () => { + CreateJWT: (user, secret) => { const claims = { - 'iss': Cypress.env('ADMIN_USERNAME'), + 'iss': user, 'iat': Math.round(Date.now() / 1000) } - var token = jwt.sign(claims, Cypress.env('ADMIN_SECRET')); + var token = jwt.sign(claims, secret); return token; }, + GenerateID: (length=10) => { + let result = ''; + const characters = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789'; + const charactersLength = characters.length; + let counter = 0; + while (counter < length) { + result += characters.charAt(Math.floor(Math.random() * charactersLength)); + counter += 1; + } + return result; + } }; const Admin = { SendOneOff: ({to, template_id}) => { - var token = Utilities.CreateJWT(); + var token = Utilities.CreateJWT(Cypress.env('ADMIN_USERNAME'), Cypress.env('ADMIN_SECRET')); return cy.request({ url: `/service/${config.Services.Cypress}/send-notification`, method: 'POST', @@ -105,6 +116,54 @@ const API = { } }); }, + CreateAPIKey: ({ service_id, key_type, name }) => { + var token = Utilities.CreateJWT(Cypress.env('ADMIN_USERNAME'), Cypress.env('ADMIN_SECRET')); + return cy.request({ + url: `/service/${service_id}/api-key`, + method: 'POST', + headers: { + Authorization: `Bearer ${token}`, + }, + body: { + key_type: key_type, + name: name, + created_by: Cypress.env('NOTIFY_USER_ID'), + } + }); + }, + RevokeAPIKey: ({ token, type, url, source, failOnStatusCode = true }) => { + var jwt_token = Utilities.CreateJWT(Cypress.env('SRE_USERNAME'), Cypress.env('SRE_SECRET')); + cy.request({ + url: `/sre-tools/api-key-revoke`, + method: 'POST', + headers: { + Authorization: `Bearer ${jwt_token}`, + }, + body: { + "token": token, + "type": type, + "url": url, + "source": source + } + }); + }, + RevokeAPIKeyWithAdminAuth: ({ token, type, url, source, failOnStatusCode = true }) => { + var jwt_token = Utilities.CreateJWT(Cypress.env('ADMIN_USERNAME'), Cypress.env('ADMIN_SECRET')); + return cy.request({ + url: `/sre-tools/api-key-revoke`, + method: 'POST', + headers: { + Authorization: `Bearer ${jwt_token}`, + }, + body: { + "token": token, + "type": type, + "url": url, + "source": source + }, + failOnStatusCode: failOnStatusCode + }); + } } diff --git a/tests_cypress/cypress/e2e/api/sre_tools.cy.js b/tests_cypress/cypress/e2e/api/sre_tools.cy.js new file mode 100644 index 0000000000..01e24d0c52 --- /dev/null +++ b/tests_cypress/cypress/e2e/api/sre_tools.cy.js @@ -0,0 +1,43 @@ +/// + +import config from '../../../config'; +import Notify from "../../Notify/NotifyAPI"; + +describe('SRE Tools', () => { + it('can revoke an API key using SRE auth', () => { + let key_name = 'api-revoke-test-' + Notify.Utilities.GenerateID(); + + Notify.API.CreateAPIKey({ + service_id: config.Services.Cypress, + key_type: 'normal', + name: key_name + }).as('APIKey'); + + cy.log("Generated API KEY: " + key_name); + + cy.get('@APIKey').then((response) => { + let api_key = response.body.data.key_name + "-" + config.Services.Cypress + "-" + response.body.data.key; + + Notify.API.RevokeAPIKey({ + token: api_key, + type: 'normal', + url:'https://example.com', + source: 'Cypress Test' + }); + cy.log("Revoked API KEY: " + key_name); + }); + }); + it('cannot revoke an API key using admin auth', () => { + Notify.API.RevokeAPIKeyWithAdminAuth({ + token: "fake-key", + type: 'normal', + url:'https://example.com', + source: 'Cypress Test', + failOnStatusCode: false + }).as('revokeRequest'); + + cy.get('@revokeRequest').then(response => { + expect(response.status).to.eq(401); + }); + }); +}); From 7b81e420634a3c2eacc47cfa963a5505a8c6550e Mon Sep 17 00:00:00 2001 From: Steve Astels Date: Wed, 15 Nov 2023 16:33:16 -0500 Subject: [PATCH 5/8] make perf test longer (#2023) --- tests-perf/locust/locust.conf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests-perf/locust/locust.conf b/tests-perf/locust/locust.conf index c1eba3b220..76aa3d2273 100644 --- a/tests-perf/locust/locust.conf +++ b/tests-perf/locust/locust.conf @@ -3,7 +3,7 @@ locustfile = tests-perf/locust/locust-notifications.py host = https://api.staging.notification.cdssandbox.xyz users = 3000 spawn-rate = 20 -run-time = 5m +run-time = 10m # headless = true # master = true From 07f2ed5c8ec8fdbbfb25507415b67afc63baeb22 Mon Sep 17 00:00:00 2001 From: Steve Astels Date: Thu, 16 Nov 2023 16:04:54 -0500 Subject: [PATCH 6/8] 10m -> 20m perf test (#2025) --- tests-perf/locust/locust.conf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests-perf/locust/locust.conf b/tests-perf/locust/locust.conf index 76aa3d2273..0f958e2abb 100644 --- a/tests-perf/locust/locust.conf +++ b/tests-perf/locust/locust.conf @@ -3,7 +3,7 @@ locustfile = tests-perf/locust/locust-notifications.py host = https://api.staging.notification.cdssandbox.xyz users = 3000 spawn-rate = 20 -run-time = 10m +run-time = 20m # headless = true # master = true From 203307f69724cdb863e9eab83e80c581443a98d3 Mon Sep 17 00:00:00 2001 From: Jimmy Royer Date: Thu, 16 Nov 2023 17:26:52 -0500 Subject: [PATCH 7/8] Fix startup scripts to POSIX standards / add timeout for cwagent waiting (#2016) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Added timeout to cwagents waiting + shellcheck addition * Setting +x permission on run-shellcheck.sh * Fix Shellcheck errors by doing as little work as possible (#2022) # Summary | Résumé These fixes a bunch of shellcheck errors and ignores a few I didn't test, actually I didn't test any of this, most of the fixes were pretty straightforward ones, there was one I fixed with co-pilot that should be tested and the others I didn't fix I just assumed they worked as is since they were in the existing codebase. - **Fixes**: https://www.shellcheck.net/wiki/SC1091 Couldn't find the file that was being referenced (`environment_test.sh`) in `scripts/run_single_test.sh` so I just the source to /dev/null - **Fixes**: https://www.shellcheck.net/wiki/SC2028 Replaces some echo's where a control character was passed "\n" with a printf so it should work the same and not complain - **Fixes the read -p error**: https://www.shellcheck.net/wiki/SC3045 by asking co-pilot to fix it, I can't guarantee this one will work, **YOU SHOULD TEST THIS FILE: scripts/run_celery_purge.sh** - **Ignores**: https://www.shellcheck.net/wiki/SC2009 I'm assuming it worked and I didn't want to find the pgrep way of doing it, also it's not posix - **Ignores**: https://www.shellcheck.net/wiki/SC2219 I'm assuming it worked and I didn't want to test if the following works ```bash ((wait_time++)) || true ``` - **Fixes**: https://www.shellcheck.net/wiki/SC2105 By removing the break from the case, they don't need it. - **Fixes**: https://www.shellcheck.net/wiki/SC3046 By replacing `source` with `.` - **Fixes**: https://www.shellcheck.net/wiki/SC2068 by wrapping array Expansion with double quotes - **Fixes**: https://www.shellcheck.net/wiki/SC2086 By wrapping the variable in double quotes --------- Co-authored-by: Calvin Rodo --- .github/workflows/scripts/run-shellcheck.sh | 3 +++ .github/workflows/shellcheck.yml | 14 +++++++++++ scripts/run_celery.sh | 25 +++++++++++-------- scripts/run_celery_beat.sh | 23 +++++++++++------- scripts/run_celery_core_tasks.sh | 26 ++++++++++++-------- scripts/run_celery_exit.sh | 8 +++--- scripts/run_celery_local.sh | 2 +- scripts/run_celery_no_sms_sending.sh | 27 ++++++++++++--------- scripts/run_celery_purge.sh | 19 ++++++++------- scripts/run_celery_send_email.sh | 27 ++++++++++++--------- scripts/run_celery_send_sms.sh | 24 ++++++++++++------ scripts/run_celery_sms.sh | 22 +++++++++++------ scripts/run_single_test.sh | 5 ++-- scripts/run_tests.sh | 4 +-- 14 files changed, 145 insertions(+), 84 deletions(-) create mode 100755 .github/workflows/scripts/run-shellcheck.sh create mode 100644 .github/workflows/shellcheck.yml diff --git a/.github/workflows/scripts/run-shellcheck.sh b/.github/workflows/scripts/run-shellcheck.sh new file mode 100755 index 0000000000..0826c1b0d9 --- /dev/null +++ b/.github/workflows/scripts/run-shellcheck.sh @@ -0,0 +1,3 @@ +#!/bin/bash + +docker run --rm -v "$PWD:/mnt" koalaman/shellcheck:v0.9.0 -P ./bin/ -x ./scripts/*.sh \ No newline at end of file diff --git a/.github/workflows/shellcheck.yml b/.github/workflows/shellcheck.yml new file mode 100644 index 0000000000..3ad9d66ef2 --- /dev/null +++ b/.github/workflows/shellcheck.yml @@ -0,0 +1,14 @@ +name: Shellcheck +on: + push: + paths: + - "**/*.sh" + +jobs: + shellcheck: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - name: Shellcheck + run: | + .github/workflows/scripts/run-shellcheck.sh diff --git a/scripts/run_celery.sh b/scripts/run_celery.sh index 46146203ac..612ec22c16 100755 --- a/scripts/run_celery.sh +++ b/scripts/run_celery.sh @@ -1,25 +1,30 @@ #!/bin/sh -# runs celery with all celery queues except the throtted sms queue - set -e -# Check and see if this is running in K8s and if so, wait for cloudwatch agent -if [[ ! -z "${STATSD_HOST}" ]]; then +# Runs celery with all celery queues except the throtted sms queue. - echo "Initializing... Waiting for CWAgent to become ready." - while : - do - if nc -vz $STATSD_HOST 25888; then +# Check and see if this is running in K8s and if so, wait for cloudwatch agent +if [ -n "${STATSD_HOST}" ]; then + echo "Initializing... Waiting for CWAgent to become ready within the next 30 seconds." + timeout=30 + while [ $timeout -gt 0 ]; do + if nc -vz "$STATSD_HOST" 25888; then echo "CWAgent is Ready." - break; + break else echo "Waiting for CWAgent to become ready." sleep 1 + timeout=$((timeout - 1)) fi done + + if [ $timeout -eq 0 ]; then + echo "Timeout reached. CWAgent did not become ready in 30 seconds." + exit 1 + fi fi echo "Start celery, concurrency: ${CELERY_CONCURRENCY-4}" -celery -A run_celery.notify_celery worker --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency=${CELERY_CONCURRENCY-4} -Q database-tasks,-priority-database-tasks.fifo,-normal-database-tasks,-bulk-database-tasks,job-tasks,notify-internal-tasks,periodic-tasks,priority-tasks,normal-tasks,bulk-tasks,reporting-tasks,research-mode-tasks,retry-tasks,send-sms-tasks,send-sms-high,send-sms-medium,send-sms-low,send-email-tasks,service-callbacks,delivery-receipts +celery -A run_celery.notify_celery worker --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency="${CELERY_CONCURRENCY-4}" -Q database-tasks,-priority-database-tasks.fifo,-normal-database-tasks,-bulk-database-tasks,job-tasks,notify-internal-tasks,periodic-tasks,priority-tasks,normal-tasks,bulk-tasks,reporting-tasks,research-mode-tasks,retry-tasks,send-sms-tasks,send-sms-high,send-sms-medium,send-sms-low,send-email-tasks,service-callbacks,delivery-receipts diff --git a/scripts/run_celery_beat.sh b/scripts/run_celery_beat.sh index 61731f6ad2..7a2684102e 100755 --- a/scripts/run_celery_beat.sh +++ b/scripts/run_celery_beat.sh @@ -1,23 +1,28 @@ #!/bin/sh -# runs the celery beat process. This runs the periodic tasks - set -e -# Check and see if this is running in K8s and if so, wait for cloudwatch agent -if [[ ! -z "${STATSD_HOST}" ]]; then +# Runs the celery beat process, i.e the Celery periodic tasks. - echo "Initializing... Waiting for CWAgent to become ready." - while : - do - if nc -vz $STATSD_HOST 25888; then +# Check and see if this is running in K8s and if so, wait for cloudwatch agent +if [ -n "${STATSD_HOST}" ]; then + echo "Initializing... Waiting for CWAgent to become ready within the next 30 seconds." + timeout=30 + while [ $timeout -gt 0 ]; do + if nc -vz "$STATSD_HOST" 25888; then echo "CWAgent is Ready." - break; + break else echo "Waiting for CWAgent to become ready." sleep 1 + timeout=$((timeout - 1)) fi done + + if [ $timeout -eq 0 ]; then + echo "Timeout reached. CWAgent did not become ready in 30 seconds." + exit 1 + fi fi celery -A run_celery.notify_celery beat --loglevel=INFO diff --git a/scripts/run_celery_core_tasks.sh b/scripts/run_celery_core_tasks.sh index 1f3e8decea..3ff390e922 100755 --- a/scripts/run_celery_core_tasks.sh +++ b/scripts/run_celery_core_tasks.sh @@ -1,25 +1,31 @@ #!/bin/sh -# runs celery with all celery queues except send-throttled-sms-tasks, send-sms-* and send-email-* - set -e -# Check and see if this is running in K8s and if so, wait for cloudwatch agent -if [[ ! -z "${STATSD_HOST}" ]]; then +# Runs celery with all celery queues except send-throttled-sms-tasks, +# send-sms-* and send-email-*. - echo "Initializing... Waiting for CWAgent to become ready." - while : - do - if nc -vz $STATSD_HOST 25888; then +# Check and see if this is running in K8s and if so, wait for cloudwatch agent +if [ -n "${STATSD_HOST}" ]; then + echo "Initializing... Waiting for CWAgent to become ready within the next 30 seconds." + timeout=30 + while [ $timeout -gt 0 ]; do + if nc -vz "$STATSD_HOST" 25888; then echo "CWAgent is Ready." - break; + break else echo "Waiting for CWAgent to become ready." sleep 1 + timeout=$((timeout - 1)) fi done + + if [ $timeout -eq 0 ]; then + echo "Timeout reached. CWAgent did not become ready in 30 seconds." + exit 1 + fi fi echo "Start celery, concurrency: ${CELERY_CONCURRENCY-4}" -celery -A run_celery.notify_celery worker --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency=${CELERY_CONCURRENCY-4} -Q database-tasks,-priority-database-tasks.fifo,-normal-database-tasks,-bulk-database-tasks,job-tasks,notify-internal-tasks,periodic-tasks,priority-tasks,normal-tasks,bulk-tasks,reporting-tasks,research-mode-tasks,retry-tasks,service-callbacks,delivery-receipts +celery -A run_celery.notify_celery worker --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency="${CELERY_CONCURRENCY-4}" -Q database-tasks,-priority-database-tasks.fifo,-normal-database-tasks,-bulk-database-tasks,job-tasks,notify-internal-tasks,periodic-tasks,priority-tasks,normal-tasks,bulk-tasks,reporting-tasks,research-mode-tasks,retry-tasks,service-callbacks,delivery-receipts diff --git a/scripts/run_celery_exit.sh b/scripts/run_celery_exit.sh index e34fdf7495..97ed326f56 100755 --- a/scripts/run_celery_exit.sh +++ b/scripts/run_celery_exit.sh @@ -10,6 +10,7 @@ function get_celery_pids { # and keep only these PIDs set +o pipefail # so grep returning no matches does not premature fail pipe + # shellcheck disable=SC2009 # We don't want to bother re-writing this to use pgrep APP_PIDS=$(ps aux --sort=start_time | grep 'celery worker' | grep 'bin/celery' | head -1 | awk '{print $2}') set -o pipefail # pipefail should be set everywhere else } @@ -18,12 +19,12 @@ function send_signal_to_celery_processes { # refresh pids to account for the case that some workers may have terminated but others not get_celery_pids # send signal to all remaining apps - echo ${APP_PIDS} | tr -d '\n' | tr -s ' ' | xargs echo "Sending signal ${1} to processes with pids: " >> /proc/1/fd/1 + echo "${APP_PIDS}" | tr -d '\n' | tr -s ' ' | xargs echo "Sending signal ${1} to processes with pids: " >> /proc/1/fd/1 echo "We will send ${1} signal" >> /proc/1/fd/1 for value in ${APP_PIDS} do - echo kill -s ${1} $value - kill -s ${1} $value + echo kill -s "${1}" "$value" + kill -s "${1}" "$value" done #echo ${APP_PIDS} | xargs kill -s ${1} } @@ -59,6 +60,7 @@ function on_exit { echo "exit function is running with wait time of 9s" >> /proc/1/fd/1 get_celery_pids ensure_celery_is_running + # shellcheck disable=SC2219 # We could probably rewrite it as `((wait_time++)) || true` but I haven't tested and I assume this works as is let wait_time=wait_time+1 sleep 1 done diff --git a/scripts/run_celery_local.sh b/scripts/run_celery_local.sh index 069387e4b0..4b400747f4 100755 --- a/scripts/run_celery_local.sh +++ b/scripts/run_celery_local.sh @@ -7,4 +7,4 @@ set -e echo "Start celery, concurrency: ${CELERY_CONCURRENCY-4}" -celery -A run_celery.notify_celery worker --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency=${CELERY_CONCURRENCY-4} -Q database-tasks,-priority-database-tasks.fifo,-normal-database-tasks,-bulk-database-tasks,job-tasks,notify-internal-tasks,periodic-tasks,priority-tasks,normal-tasks,bulk-tasks,reporting-tasks,research-mode-tasks,retry-tasks,send-sms-tasks,send-sms-high,send-sms-medium,send-sms-low,send-throttled-sms-tasks,send-email-high,send-email-medium,send-email-low,send-email-tasks,service-callbacks,delivery-receipts +celery -A run_celery.notify_celery worker --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency="${CELERY_CONCURRENCY-4}" -Q database-tasks,-priority-database-tasks.fifo,-normal-database-tasks,-bulk-database-tasks,job-tasks,notify-internal-tasks,periodic-tasks,priority-tasks,normal-tasks,bulk-tasks,reporting-tasks,research-mode-tasks,retry-tasks,send-sms-tasks,send-sms-high,send-sms-medium,send-sms-low,send-throttled-sms-tasks,send-email-high,send-email-medium,send-email-low,send-email-tasks,service-callbacks,delivery-receipts diff --git a/scripts/run_celery_no_sms_sending.sh b/scripts/run_celery_no_sms_sending.sh index 081f7174ef..cebefa7435 100755 --- a/scripts/run_celery_no_sms_sending.sh +++ b/scripts/run_celery_no_sms_sending.sh @@ -1,26 +1,31 @@ #!/bin/sh -# runs celery with only the throttled sms sending queue - -# runs celery with all celery queues except send-throttled-sms-tasks, send-sms-tasks, send-sms-high, send-sms-medium, or send-sms-low - set -e +# Runs celery with all celery queues except send-throttled-sms-tasks, +# send-sms-tasks, send-sms-high, send-sms-medium, or send-sms-low. + # Check and see if this is running in K8s and if so, wait for cloudwatch agent -if [[ ! -z "${STATSD_HOST}" ]]; then - echo "Initializing... Waiting for CWAgent to become ready." - while : - do - if nc -vz $STATSD_HOST 25888; then +if [ -n "${STATSD_HOST}" ]; then + echo "Initializing... Waiting for CWAgent to become ready within the next 30 seconds." + timeout=30 + while [ $timeout -gt 0 ]; do + if nc -vz "$STATSD_HOST" 25888; then echo "CWAgent is Ready." - break; + break else echo "Waiting for CWAgent to become ready." sleep 1 + timeout=$((timeout - 1)) fi done + + if [ $timeout -eq 0 ]; then + echo "Timeout reached. CWAgent did not become ready in 30 seconds." + exit 1 + fi fi echo "Start celery, concurrency: ${CELERY_CONCURRENCY-4}" -celery -A run_celery.notify_celery worker --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency=${CELERY_CONCURRENCY-4} -Q database-tasks,-priority-database-tasks.fifo,-normal-database-tasks,-bulk-database-tasks,job-tasks,notify-internal-tasks,periodic-tasks,priority-tasks,normal-tasks,bulk-tasks,reporting-tasks,research-mode-tasks,retry-tasks,send-email-tasks,send-email-high,send-email-medium,send-email-low,service-callbacks,delivery-receipts +celery -A run_celery.notify_celery worker --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency="${CELERY_CONCURRENCY-4}" -Q database-tasks,-priority-database-tasks.fifo,-normal-database-tasks,-bulk-database-tasks,job-tasks,notify-internal-tasks,periodic-tasks,priority-tasks,normal-tasks,bulk-tasks,reporting-tasks,research-mode-tasks,retry-tasks,send-email-tasks,send-email-high,send-email-medium,send-email-low,service-callbacks,delivery-receipts diff --git a/scripts/run_celery_purge.sh b/scripts/run_celery_purge.sh index 3ad1eb1509..ea67a3c807 100755 --- a/scripts/run_celery_purge.sh +++ b/scripts/run_celery_purge.sh @@ -2,15 +2,16 @@ set -e -echo "\n--------------------------------------------------\n" -echo " WARNING!!!!\n" -echo " This script is for local development only!\n" -echo " It will delete everything in the celery queues.\n" -echo "--------------------------------------------------\n" -echo "Are you sure you want to continue?" -read -p "If so, type 'purge'> " check +printf "\n--------------------------------------------------\n" +printf " WARNING!!!!\n" +printf " This script is for local development only!\n" +printf " It will delete everything in the celery queues.\n" +printf "\n--------------------------------------------------\n" +printf "Are you sure you want to continue?" +echo "If so, type 'purge'> \c" +read -r check case $check in - purge ) echo "purging!"; celery -A run_celery.notify_celery purge -f; break;; - * ) echo "\nNot purging\n";; + purge ) echo "purging!"; celery -A run_celery.notify_celery purge -f;; + * ) printf "\nNot purging\n";; esac diff --git a/scripts/run_celery_send_email.sh b/scripts/run_celery_send_email.sh index 6ae32a9d81..98e2eed691 100755 --- a/scripts/run_celery_send_email.sh +++ b/scripts/run_celery_send_email.sh @@ -1,26 +1,31 @@ #!/bin/sh -# runs celery with only the send-email-* queues - set -e -# Check and see if this is running in K8s and if so, wait for cloudwatch agent -if [[ ! -z "${STATSD_HOST}" ]]; then +# Runs celery with only the send-email-* queues. - echo "Initializing... Waiting for CWAgent to become ready." - while : - do - if nc -vz $STATSD_HOST 25888; then +# Check and see if this is running in K8s and if so, wait for cloudwatch agent +if [ -n "${STATSD_HOST}" ]; then + echo "Initializing... Waiting for CWAgent to become ready within the next 30 seconds." + timeout=30 + while [ $timeout -gt 0 ]; do + if nc -vz "$STATSD_HOST" 25888; then echo "CWAgent is Ready." - break; + break else echo "Waiting for CWAgent to become ready." sleep 1 + timeout=$((timeout - 1)) fi done + + if [ $timeout -eq 0 ]; then + echo "Timeout reached. CWAgent did not become ready in 30 seconds." + exit 1 + fi fi echo "Start celery, concurrency: ${CELERY_CONCURRENCY-4}" -# TODO: we shouldn't be using the send-email-tasks queue anymore - once we verify this we can remove it -celery -A run_celery.notify_celery worker --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency=${CELERY_CONCURRENCY-4} -Q send-email-tasks,send-email-high,send-email-medium,send-email-low +# TODO: we shouldn't be using the send-email-tasks queue anymore, once we verify this we can remove it +celery -A run_celery.notify_celery worker --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency="${CELERY_CONCURRENCY-4}" -Q send-email-tasks,send-email-high,send-email-medium,send-email-low diff --git a/scripts/run_celery_send_sms.sh b/scripts/run_celery_send_sms.sh index 7146d46b0c..ffdbbfa54a 100755 --- a/scripts/run_celery_send_sms.sh +++ b/scripts/run_celery_send_sms.sh @@ -1,23 +1,31 @@ #!/bin/sh -# runs celery with only the send-sms-* queues set -e -if [[ ! -z "${STATSD_HOST}" ]]; then - echo "Initializing... Waiting for CWAgent to become ready." - while : - do - if nc -vz $STATSD_HOST 25888; then +# Runs celery with only the send-sms-* queues. + +# Check and see if this is running in K8s and if so, wait for cloudwatch agent +if [ -n "${STATSD_HOST}" ]; then + echo "Initializing... Waiting for CWAgent to become ready within the next 30 seconds." + timeout=30 + while [ $timeout -gt 0 ]; do + if nc -vz "$STATSD_HOST" 25888; then echo "CWAgent is Ready." - break; + break else echo "Waiting for CWAgent to become ready." sleep 1 + timeout=$((timeout - 1)) fi done + + if [ $timeout -eq 0 ]; then + echo "Timeout reached. CWAgent did not become ready in 30 seconds." + exit 1 + fi fi echo "Start celery, concurrency: ${CELERY_CONCURRENCY-4}" # TODO: we shouldn't be using the send-sms-tasks queue anymore - once we verify this we can remove it -celery -A run_celery.notify_celery worker --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency=${CELERY_CONCURRENCY-4} -Q send-sms-tasks,send-sms-high,send-sms-medium,send-sms-low +celery -A run_celery.notify_celery worker --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency="${CELERY_CONCURRENCY-4}" -Q send-sms-tasks,send-sms-high,send-sms-medium,send-sms-low diff --git a/scripts/run_celery_sms.sh b/scripts/run_celery_sms.sh index 0eac65942a..795e7c7d81 100755 --- a/scripts/run_celery_sms.sh +++ b/scripts/run_celery_sms.sh @@ -1,22 +1,28 @@ #!/bin/sh -# runs celery with only the throttled sms sending queue - set -e +# Runs celery with only the throttled sms sending queue. -if [[ ! -z "${STATSD_HOST}" ]]; then - echo "Initializing... Waiting for CWAgent to become ready." - while : - do - if nc -vz $STATSD_HOST 25888; then +# Check and see if this is running in K8s and if so, wait for cloudwatch agent +if [ -n "${STATSD_HOST}" ]; then + echo "Initializing... Waiting for CWAgent to become ready within the next 30 seconds." + timeout=30 + while [ $timeout -gt 0 ]; do + if nc -vz "$STATSD_HOST" 25888; then echo "CWAgent is Ready." - break; + break else echo "Waiting for CWAgent to become ready." sleep 1 + timeout=$((timeout - 1)) fi done + + if [ $timeout -eq 0 ]; then + echo "Timeout reached. CWAgent did not become ready in 30 seconds." + exit 1 + fi fi celery -A run_celery.notify_celery worker --loglevel=INFO --concurrency=1 -Q send-throttled-sms-tasks diff --git a/scripts/run_single_test.sh b/scripts/run_single_test.sh index 3d05f4bb0d..ce20b8318d 100755 --- a/scripts/run_single_test.sh +++ b/scripts/run_single_test.sh @@ -1,4 +1,5 @@ #!/bin/sh # run a single unit test, pass in the unit test name for example: tests/app/service/test_rest.py::test_get_template_list -source environment_test.sh -py.test $@ +# shellcheck source=/dev/null # Not finding this file in code base +. environment_test.sh +py.test "$@" diff --git a/scripts/run_tests.sh b/scripts/run_tests.sh index 453ac7844e..53ada3e0d9 100755 --- a/scripts/run_tests.sh +++ b/scripts/run_tests.sh @@ -12,9 +12,9 @@ function display_result { EXIT_STATUS=$2 TEST=$3 - if [ $RESULT -ne 0 ]; then + if [ "$RESULT" -ne 0 ]; then echo -e "\033[31m$TEST failed\033[0m" - exit $EXIT_STATUS + exit "$EXIT_STATUS" else echo -e "\033[32m$TEST passed\033[0m" fi From a7b67264be0d866bb5f42d98e3768db3cc853a64 Mon Sep 17 00:00:00 2001 From: Ben Larabie Date: Mon, 20 Nov 2023 08:59:36 -0500 Subject: [PATCH 8/8] Moving cwagent check outside of run celery (#2027) * Moving cwagent check outside of run celery * making cwagent check executable * added shebang --- scripts/cwcheck.sh | 21 +++++++++++++++++++++ scripts/run_celery.sh | 21 --------------------- scripts/run_celery_beat.sh | 21 --------------------- scripts/run_celery_core_tasks.sh | 21 --------------------- scripts/run_celery_send_email.sh | 21 --------------------- scripts/run_celery_send_sms.sh | 21 --------------------- scripts/run_celery_sms.sh | 21 --------------------- 7 files changed, 21 insertions(+), 126 deletions(-) create mode 100755 scripts/cwcheck.sh diff --git a/scripts/cwcheck.sh b/scripts/cwcheck.sh new file mode 100755 index 0000000000..6adf6f23bb --- /dev/null +++ b/scripts/cwcheck.sh @@ -0,0 +1,21 @@ +#!/bin/bash +# Check and see if this is running in K8s and if so, wait for cloudwatch agent +if [ -n "${STATSD_HOST}" ]; then + echo "Initializing... Waiting for CWAgent to become ready within the next 30 seconds." + timeout=30 + while [ $timeout -gt 0 ]; do + if nc -vz "$STATSD_HOST" 25888; then + echo "CWAgent is Ready." + break + else + echo "Waiting for CWAgent to become ready." + sleep 1 + timeout=$((timeout - 1)) + fi + done + + if [ $timeout -eq 0 ]; then + echo "Timeout reached. CWAgent did not become ready in 30 seconds." + exit 1 + fi +fi diff --git a/scripts/run_celery.sh b/scripts/run_celery.sh index 612ec22c16..99f09ac9bb 100755 --- a/scripts/run_celery.sh +++ b/scripts/run_celery.sh @@ -4,27 +4,6 @@ set -e # Runs celery with all celery queues except the throtted sms queue. -# Check and see if this is running in K8s and if so, wait for cloudwatch agent -if [ -n "${STATSD_HOST}" ]; then - echo "Initializing... Waiting for CWAgent to become ready within the next 30 seconds." - timeout=30 - while [ $timeout -gt 0 ]; do - if nc -vz "$STATSD_HOST" 25888; then - echo "CWAgent is Ready." - break - else - echo "Waiting for CWAgent to become ready." - sleep 1 - timeout=$((timeout - 1)) - fi - done - - if [ $timeout -eq 0 ]; then - echo "Timeout reached. CWAgent did not become ready in 30 seconds." - exit 1 - fi -fi - echo "Start celery, concurrency: ${CELERY_CONCURRENCY-4}" celery -A run_celery.notify_celery worker --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency="${CELERY_CONCURRENCY-4}" -Q database-tasks,-priority-database-tasks.fifo,-normal-database-tasks,-bulk-database-tasks,job-tasks,notify-internal-tasks,periodic-tasks,priority-tasks,normal-tasks,bulk-tasks,reporting-tasks,research-mode-tasks,retry-tasks,send-sms-tasks,send-sms-high,send-sms-medium,send-sms-low,send-email-tasks,service-callbacks,delivery-receipts diff --git a/scripts/run_celery_beat.sh b/scripts/run_celery_beat.sh index 7a2684102e..2479d725ad 100755 --- a/scripts/run_celery_beat.sh +++ b/scripts/run_celery_beat.sh @@ -4,25 +4,4 @@ set -e # Runs the celery beat process, i.e the Celery periodic tasks. -# Check and see if this is running in K8s and if so, wait for cloudwatch agent -if [ -n "${STATSD_HOST}" ]; then - echo "Initializing... Waiting for CWAgent to become ready within the next 30 seconds." - timeout=30 - while [ $timeout -gt 0 ]; do - if nc -vz "$STATSD_HOST" 25888; then - echo "CWAgent is Ready." - break - else - echo "Waiting for CWAgent to become ready." - sleep 1 - timeout=$((timeout - 1)) - fi - done - - if [ $timeout -eq 0 ]; then - echo "Timeout reached. CWAgent did not become ready in 30 seconds." - exit 1 - fi -fi - celery -A run_celery.notify_celery beat --loglevel=INFO diff --git a/scripts/run_celery_core_tasks.sh b/scripts/run_celery_core_tasks.sh index 3ff390e922..e109bce78d 100755 --- a/scripts/run_celery_core_tasks.sh +++ b/scripts/run_celery_core_tasks.sh @@ -5,27 +5,6 @@ set -e # Runs celery with all celery queues except send-throttled-sms-tasks, # send-sms-* and send-email-*. -# Check and see if this is running in K8s and if so, wait for cloudwatch agent -if [ -n "${STATSD_HOST}" ]; then - echo "Initializing... Waiting for CWAgent to become ready within the next 30 seconds." - timeout=30 - while [ $timeout -gt 0 ]; do - if nc -vz "$STATSD_HOST" 25888; then - echo "CWAgent is Ready." - break - else - echo "Waiting for CWAgent to become ready." - sleep 1 - timeout=$((timeout - 1)) - fi - done - - if [ $timeout -eq 0 ]; then - echo "Timeout reached. CWAgent did not become ready in 30 seconds." - exit 1 - fi -fi - echo "Start celery, concurrency: ${CELERY_CONCURRENCY-4}" celery -A run_celery.notify_celery worker --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency="${CELERY_CONCURRENCY-4}" -Q database-tasks,-priority-database-tasks.fifo,-normal-database-tasks,-bulk-database-tasks,job-tasks,notify-internal-tasks,periodic-tasks,priority-tasks,normal-tasks,bulk-tasks,reporting-tasks,research-mode-tasks,retry-tasks,service-callbacks,delivery-receipts diff --git a/scripts/run_celery_send_email.sh b/scripts/run_celery_send_email.sh index 98e2eed691..98fda14a68 100755 --- a/scripts/run_celery_send_email.sh +++ b/scripts/run_celery_send_email.sh @@ -4,27 +4,6 @@ set -e # Runs celery with only the send-email-* queues. -# Check and see if this is running in K8s and if so, wait for cloudwatch agent -if [ -n "${STATSD_HOST}" ]; then - echo "Initializing... Waiting for CWAgent to become ready within the next 30 seconds." - timeout=30 - while [ $timeout -gt 0 ]; do - if nc -vz "$STATSD_HOST" 25888; then - echo "CWAgent is Ready." - break - else - echo "Waiting for CWAgent to become ready." - sleep 1 - timeout=$((timeout - 1)) - fi - done - - if [ $timeout -eq 0 ]; then - echo "Timeout reached. CWAgent did not become ready in 30 seconds." - exit 1 - fi -fi - echo "Start celery, concurrency: ${CELERY_CONCURRENCY-4}" # TODO: we shouldn't be using the send-email-tasks queue anymore, once we verify this we can remove it diff --git a/scripts/run_celery_send_sms.sh b/scripts/run_celery_send_sms.sh index ffdbbfa54a..5f7865b62c 100755 --- a/scripts/run_celery_send_sms.sh +++ b/scripts/run_celery_send_sms.sh @@ -4,27 +4,6 @@ set -e # Runs celery with only the send-sms-* queues. -# Check and see if this is running in K8s and if so, wait for cloudwatch agent -if [ -n "${STATSD_HOST}" ]; then - echo "Initializing... Waiting for CWAgent to become ready within the next 30 seconds." - timeout=30 - while [ $timeout -gt 0 ]; do - if nc -vz "$STATSD_HOST" 25888; then - echo "CWAgent is Ready." - break - else - echo "Waiting for CWAgent to become ready." - sleep 1 - timeout=$((timeout - 1)) - fi - done - - if [ $timeout -eq 0 ]; then - echo "Timeout reached. CWAgent did not become ready in 30 seconds." - exit 1 - fi -fi - echo "Start celery, concurrency: ${CELERY_CONCURRENCY-4}" # TODO: we shouldn't be using the send-sms-tasks queue anymore - once we verify this we can remove it diff --git a/scripts/run_celery_sms.sh b/scripts/run_celery_sms.sh index 795e7c7d81..302ecc5f15 100755 --- a/scripts/run_celery_sms.sh +++ b/scripts/run_celery_sms.sh @@ -4,25 +4,4 @@ set -e # Runs celery with only the throttled sms sending queue. -# Check and see if this is running in K8s and if so, wait for cloudwatch agent -if [ -n "${STATSD_HOST}" ]; then - echo "Initializing... Waiting for CWAgent to become ready within the next 30 seconds." - timeout=30 - while [ $timeout -gt 0 ]; do - if nc -vz "$STATSD_HOST" 25888; then - echo "CWAgent is Ready." - break - else - echo "Waiting for CWAgent to become ready." - sleep 1 - timeout=$((timeout - 1)) - fi - done - - if [ $timeout -eq 0 ]; then - echo "Timeout reached. CWAgent did not become ready in 30 seconds." - exit 1 - fi -fi - celery -A run_celery.notify_celery worker --loglevel=INFO --concurrency=1 -Q send-throttled-sms-tasks