From 8ec72463b2a7e4ae4a86704a607e42175c2d5bf8 Mon Sep 17 00:00:00 2001 From: Gary Tempus Date: Thu, 21 Nov 2024 14:19:25 -0500 Subject: [PATCH] :bug: fix: implementation replacement on redirect (#176) (#178) * :bug: fix: Only replace implementation with `dynamic` Supports FLAG-1126 * :white_check_mark: test: add initial pytest suite * :art: refactor: create more declarative methods and tests * :memo: docs: add docstring to handler * :memo: docs: add implementation note regarding relative URLs (cherry picked from commit 8d79ee75271bd3b57879144722e4fa0f0cfcecef) (cherry picked from commit bc6a6c77c306d5c63212f61bee44db9bc2ef5968) --- .../redirect_s3_404/src/lambda_function.py | 88 +++++++--- tests/lambda/test_redirect_s3_404.py | 165 ++++++++++++++++++ 2 files changed, 226 insertions(+), 27 deletions(-) create mode 100644 tests/lambda/test_redirect_s3_404.py diff --git a/terraform/modules/content_delivery_network/lambda_functions/redirect_s3_404/src/lambda_function.py b/terraform/modules/content_delivery_network/lambda_functions/redirect_s3_404/src/lambda_function.py index 7af0e28e..8c0cb8b1 100644 --- a/terraform/modules/content_delivery_network/lambda_functions/redirect_s3_404/src/lambda_function.py +++ b/terraform/modules/content_delivery_network/lambda_functions/redirect_s3_404/src/lambda_function.py @@ -1,12 +1,28 @@ # mypy: ignore-errors +from urllib.parse import urlparse, urlunparse def handler(event, context): - """ - This function updates the HTTP status code in the response to 307, to redirect to another - path (cache behavior) that has a different origin configured. Note the following: + """This function updates the HTTP status code in the response to 307, to + redirect to another path (cache behavior) that has a different origin + configured. + + Note the following: 1. The function is triggered in an origin response 2. The response status from the origin server is an error status code (404) + + The pattern for the incoming request uri: + + /{dataset}/{version}/{implementation}/{z}/{x}/{y}.(png|pbf) + + results in a redirect response like: + + /{dataset}/{version}/dynamic/{z}/{x}/{y}.(png|pbf)?implementation={implementation} + + *Implementation Note: The request URI and redirect URL are relative (with a leading '/'). When python `splits` the string + of a relative URL, the first element of the list is the empty string (''). Therefore, + + /{dataset}/{version}/{implementation}/{z}/{x}/{y}.(png|pbf) has seven (7) elements after splitting. """ response = event["Records"][0]["cf"]["response"] @@ -17,38 +33,56 @@ def handler(event, context): # custom origin is tile cache app. URL is passed via custom header set in cloud front # (env variables are not support for Lambda@Edge) - if int(response["status"]) == 404 and is_tile(request["uri"]): + parsed_url = urlparse(request["uri"]) + path_parts = parsed_url.path.split("/") - implementation = get_implementation(request["uri"]) + if int(response["status"]) == 404 and is_tile(path_parts): + implementation = replace_implementation_in_path(path_parts) + querystring = add_implementation_to_query_params( + implementation, request["querystring"] + ) + updated_url = urlunparse( + parsed_url._replace(path="/".join(path_parts), query=querystring) + ) + update_headers_for_redirect(headers, updated_url) + return build_redirect_response(headers) - redirect_path = request["uri"].replace(implementation, "dynamic") + return response - if request["querystring"]: - querystring = f"{request['querystring']}&implementation={implementation}" - else: - querystring = f"implementation={implementation}" - redirect_path += f"?{querystring}" +def is_tile(uri): + """The resource is a tile if its last path element ends in .png or .pbf.""" + print("REQUEST URI", "/".join(uri)) + return len(uri) == 7 and uri[6][-4:] in [".png", ".pbf"] - headers["location"] = [{"key": "Location", "value": redirect_path}] - headers["content-type"] = [{"key": "Content-Type", "value": "application/json"}] - headers["content-encoding"] = [{"key": "Content-Encoding", "value": "UTF-8"}] - response = { - "status": "307", - "statusDescription": "Temporary Redirect", - "headers": headers, - } +def replace_implementation_in_path(path_parts): + """Replace the implementation path segment with "dynamic" and return the + original implementation.""" + implementation = path_parts[3] + path_parts[3] = "dynamic" + return implementation - return response +def add_implementation_to_query_params(implementation, query_string): + implementation_param = f"implementation={implementation}" + if query_string: + querystring = f"{query_string}&{implementation_param}" + else: + querystring = implementation_param + return querystring -def is_tile(uri): - print("REQUEST URI", uri) - parts = uri.split("/") - return len(parts) == 7 and parts[6][-4:] in [".png", ".pbf"] + +def update_headers_for_redirect(headers, updated_url): + headers["location"] = [{"key": "Location", "value": updated_url}] + headers["content-type"] = [{"key": "Content-Type", "value": "application/json"}] + headers["content-encoding"] = [{"key": "Content-Encoding", "value": "UTF-8"}] -def get_implementation(uri): - parts = uri.split("/") - return parts[3] +def build_redirect_response(headers): + response = { + "status": "307", + "statusDescription": "Temporary Redirect", + "headers": headers, + } + return response diff --git a/tests/lambda/test_redirect_s3_404.py b/tests/lambda/test_redirect_s3_404.py new file mode 100644 index 00000000..79c04179 --- /dev/null +++ b/tests/lambda/test_redirect_s3_404.py @@ -0,0 +1,165 @@ +from terraform.modules.content_delivery_network.lambda_functions.redirect_s3_404.src.lambda_function import ( + handler, +) + + +def create_event(status="404", uri="not important for this test", querystring=""): + """Helper method to create the base event dictionary with customizable + status, URI, and query string.""" + return { + "Records": [ + { + "cf": { + "response": { + "status": status, + "headers": { + "content-type": [ + {"key": "Content-Type", "value": "application/json"} + ] + }, + }, + "request": { + "uri": uri, + "querystring": querystring, + }, + } + } + ] + } + + +class TestRedirectOnlyTileRequestsThatAreNotFound: + def test_handler_does_not_modify_response_if_status_is_something_other_than_404( + self, + ): + event = create_event(status="200") + + response = handler(event, {}) + + assert response == { + "status": "200", + "headers": { + "content-type": [{"key": "Content-Type", "value": "application/json"}] + }, + } + + def test_handler_creates_a_redirect_response_if_status_is_404_and_is_a_png_tile( + self, + ): + event = create_event( + uri="/sbtn_natural_forests_map/v202310/natural_forest/10/20/30.png" + ) + + response = handler(event, {}) + + assert ( + response.items() + >= { + "status": "307", + "statusDescription": "Temporary Redirect", + }.items() + ) + + def test_handler_creates_a_redirect_response_if_status_is_404_and_is_a_pbf_tile( + self, + ): + event = create_event( + uri="/sbtn_natural_forests_map/v202310/natural_forest/10/20/30.pbf" + ) + + response = handler(event, {}) + + assert ( + response.items() + >= { + "status": "307", + "statusDescription": "Temporary Redirect", + }.items() + ) + + def test_handler_does_not_modify_response_if_request_is_a_resource_other_than_a_tile( + self, + ): + event = create_event( + uri="/sbtn_natural_forests_map/v202310/natural_forest/10/20/30.txt" + ) + + response = handler(event, {}) + + assert response == { + "status": "404", + "headers": { + "content-type": [{"key": "Content-Type", "value": "application/json"}] + }, + } + + +class TestRedirectsToADynamicTileResource: + def test_original_implementation_is_replaced_with_dynamic(self): + implementation = "natural_forest" + event = create_event( + uri=f"/sbtn_natural_forests_map/v202310/{implementation}/10/20/30.png" + ) + + response = handler(event, {}) + + assert response["headers"]["location"][0]["key"] == "Location" + assert ( + "/sbtn_natural_forests_map/v202310/dynamic/10/20/30.png" + in response["headers"]["location"][0]["value"] + ) + + +class TestAddsOriginalImplementationToTheExistingQueryParams: + def test_original_implementation_is_added_to_the_list_of_query_parameters(self): + implementation = "natural_forest" + event = create_event( + uri=f"/sbtn_natural_forests_map/v202310/{implementation}/10/20/30.png", + querystring="some_param=30", + ) + + response = handler(event, {}) + + assert response["headers"]["location"][0]["key"] == "Location" + assert ( + "?some_param=30&implementation=natural_forest" + in response["headers"]["location"][0]["value"] + ) + + def test_original_implementation_is_added_as_a_query_parameter(self): + implementation = "natural_forest" + event = create_event( + uri=f"/sbtn_natural_forests_map/v202310/{implementation}/10/20/30.png" + ) + + response = handler(event, {}) + + assert response["headers"]["location"][0]["key"] == "Location" + assert ( + "?implementation=natural_forest" + in response["headers"]["location"][0]["value"] + ) + + +class TestStandardHeaderInfoIsAddedToRedirect: + def test_content_type_is_set(self): + event = create_event( + uri="/sbtn_natural_forests_map/v202310/default/10/20/30.png" + ) + + response = handler(event, {}) + + assert response["headers"]["content-type"] == [ + {"key": "Content-Type", "value": "application/json"} + ] + + def test_content_encoding_is_set(self): + event = create_event( + uri="/sbtn_natural_forests_map/v202310/default/10/20/30.png" + ) + + response = handler(event, {}) + + assert response["headers"]["content-encoding"] == [ + {"key": "Content-Encoding", "value": "UTF-8"} + ]