From cb7f98c960579f7ec9cf17c19f727d3ca12318f5 Mon Sep 17 00:00:00 2001 From: Helen Bailey Date: Tue, 17 May 2022 15:58:49 -0400 Subject: [PATCH] Add GET request handler Why these changes are being introduced: The function needs to check for a `challenge` parameter in incoming GET requests and respond with that parameter's value. This is how Alma makes the initial webhook connection during configuration. How this addresses that need: * Adds functionality and tests to check the request method, respond appropriately to GET requests, and provide error responses to other request methods and incorrectly supplied parameters * Updates the README local development instructions to reflect changes * Also updates the Dockerfile and Makefile to handle pipenv's creation of a Requirements.txt file within the container Side effects of this change: GET requests to the URL function must now include a `challenge` parameter to receive a success response. Relevant ticket(s): * https://mitlibraries.atlassian.net/browse/ENSY-66 --- Dockerfile | 9 ++++---- Makefile | 1 - README.md | 14 ++++++++++--- conftest.py | 9 -------- lambdas/webhook.py | 37 ++++++++++++++++++++++++++++++--- tests/__init__.py | 0 tests/conftest.py | 34 ++++++++++++++++++++++++++++++ tests/test_webhook.py | 48 +++++++++++++++++++++++++++++++++---------- 8 files changed, 121 insertions(+), 31 deletions(-) delete mode 100644 conftest.py create mode 100644 tests/__init__.py create mode 100644 tests/conftest.py diff --git a/Dockerfile b/Dockerfile index 0e3d734..e7de511 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,12 +1,13 @@ FROM public.ecr.aws/lambda/python:3.9 # Copy function code -COPY lambdas ${LAMBDA_TASK_ROOT}/lambdas -COPY lambdas ${LAMBDA_TASK_ROOT}/lambdas +COPY . ${LAMBDA_TASK_ROOT}/ +COPY . ${LAMBDA_TASK_ROOT}/ # Install dependencies -COPY requirements.txt . -RUN pip3 install -r requirements.txt --target "${LAMBDA_TASK_ROOT}" +RUN pip3 install pipenv +RUN pipenv requirements +RUN pip3 install -r requirements.txt --target "${LAMBDA_TASK_ROOT}" # Default handler. See README for how to override to a different handler. CMD [ "lambdas.webhook.lambda_handler" ] diff --git a/Makefile b/Makefile index 7995841..060913b 100644 --- a/Makefile +++ b/Makefile @@ -43,7 +43,6 @@ mypy: ### Container commands ### dist-dev: ## Build docker container - pipenv requirements docker build --platform linux/amd64 \ -t $(ECR_REGISTRY_DEV)/alma-webhook-lambdas-dev:latest \ -t $(ECR_REGISTRY_DEV)/alma-webhook-lambdas-dev:`git describe --always` \ diff --git a/README.md b/README.md index d06eff7..aab2b47 100644 --- a/README.md +++ b/README.md @@ -24,11 +24,19 @@ Required env variables: ``` - Post data to the container: ```bash - curl -XPOST "http://localhost:9000/2015-03-31/functions/function/invocations" -d "{}" + curl -XPOST "http://localhost:9000/2015-03-31/functions/function/invocations" -H \ + 'Content-Type: application/json' -d \ + '{"queryStringParameters": + {"challenge": "challenge-accepted"}, "requestContext": {"http": {"method": "GET"}}}' ``` - Observe output: - ``` - Hello lambda! + ```json + { + "headers": {"Content-Type": "text/plain"}, + "isBase64Encoded": false, + "statusCode": 200, + "body": "challenge-accepted" + } ``` ### To run a different handler in the container diff --git a/conftest.py b/conftest.py deleted file mode 100644 index 152eaf0..0000000 --- a/conftest.py +++ /dev/null @@ -1,9 +0,0 @@ -import os - -import pytest - - -@pytest.fixture(autouse=True) -def test_env(): - os.environ = {"WORKSPACE": "test"} - yield diff --git a/lambdas/webhook.py b/lambdas/webhook.py index d87f425..7f365fa 100644 --- a/lambdas/webhook.py +++ b/lambdas/webhook.py @@ -1,12 +1,13 @@ +import json import logging import os import sentry_sdk -def lambda_handler(event: dict, context: object) -> str: +def lambda_handler(event: dict, context: object) -> dict[str, object]: logger = logging.getLogger(__name__) - logging.basicConfig(level=logging.INFO) + logging.basicConfig(level=logging.DEBUG) env = os.environ["WORKSPACE"] if sentry_dsn := os.getenv("SENTRY_DSN"): @@ -15,4 +16,34 @@ def lambda_handler(event: dict, context: object) -> str: "Sentry DSN found, exceptions will be sent to Sentry with env=%s", env ) - return "Hello lambda!" + logger.debug(json.dumps(event)) + + base_response = { + "headers": {"Content-Type": "text/plain"}, + "isBase64Encoded": False, + } + + request_type = event["requestContext"]["http"]["method"] + if request_type == "GET": + result = handle_get_request(event) + else: + result = { + "statusCode": 405, + "body": "HTTP method not allowed. Only GET requests are supported.", + } + + return base_response | result + + +def handle_get_request(event: dict) -> dict[str, object]: + try: + challenge_secret = event["queryStringParameters"]["challenge"] + except KeyError: + return { + "statusCode": 400, + "body": "Malformed request, 'challenge' query parameter required", + } + return { + "statusCode": 200, + "body": challenge_secret, + } diff --git a/tests/__init__.py b/tests/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 0000000..e6dc3cc --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,34 @@ +import os + +import pytest + + +@pytest.fixture(autouse=True) +def test_env(): + os.environ = {"WORKSPACE": "test"} + yield + + +@pytest.fixture() +def get_request(): + request_data = { + "queryStringParameters": {"challenge": "challenge-accepted"}, + "requestContext": { + "http": { + "method": "GET", + }, + }, + } + yield request_data + + +@pytest.fixture() +def get_request_missing_parameter(): + request_data = { + "requestContext": { + "http": { + "method": "GET", + }, + }, + } + yield request_data diff --git a/tests/test_webhook.py b/tests/test_webhook.py index 3447682..3028c50 100644 --- a/tests/test_webhook.py +++ b/tests/test_webhook.py @@ -3,25 +3,51 @@ from lambdas.webhook import lambda_handler -def test_webhook_returns_expected_value(): - expected_output = "Hello lambda!" - assert lambda_handler({}, {}) == expected_output - - -def test_webhook_configures_sentry_if_dsn_present(caplog, monkeypatch): - monkeypatch.setenv("WORKSPACE", "test") +def test_webhook_configures_sentry_if_dsn_present(caplog, get_request, monkeypatch): monkeypatch.setenv("SENTRY_DSN", "https://1234567890@00000.ingest.sentry.io/123456") caplog.set_level(logging.INFO) - lambda_handler({}, {}) + lambda_handler(get_request, {}) assert ( "Sentry DSN found, exceptions will be sent to Sentry with env=test" in caplog.text ) -def test_webhook_doesnt_configure_sentry_if_dsn_not_present(caplog, monkeypatch): - monkeypatch.setenv("WORKSPACE", "test") +def test_webhook_doesnt_configure_sentry_if_dsn_not_present( + caplog, get_request, monkeypatch +): monkeypatch.delenv("SENTRY_DSN", raising=False) caplog.set_level(logging.INFO) - lambda_handler({}, {}) + lambda_handler(get_request, {}) assert "Sentry DSN found" not in caplog.text + + +def test_webhook_handles_invalid_request_type(): + request_data = {"requestContext": {"http": {"method": "DELETE"}}} + expected_output = { + "headers": {"Content-Type": "text/plain"}, + "isBase64Encoded": False, + "statusCode": 405, + "body": "HTTP method not allowed. Only GET requests are supported.", + } + assert lambda_handler(request_data, {}) == expected_output + + +def test_webhook_handles_get_request_success(get_request): + expected_output = { + "headers": {"Content-Type": "text/plain"}, + "isBase64Encoded": False, + "statusCode": 200, + "body": "challenge-accepted", + } + assert lambda_handler(get_request, {}) == expected_output + + +def test_webhook_handles_get_request_missing_parameter(get_request_missing_parameter): + expected_output = { + "headers": {"Content-Type": "text/plain"}, + "isBase64Encoded": False, + "statusCode": 400, + "body": "Malformed request, 'challenge' query parameter required", + } + assert lambda_handler(get_request_missing_parameter, {}) == expected_output