From 4a6c32a32edab409af123cd923324b305a6a8d1e Mon Sep 17 00:00:00 2001 From: Gordon Leigh Date: Tue, 15 Oct 2019 16:35:20 +0100 Subject: [PATCH 1/4] fix: options endpoint does not invoke lambda (#1434) --- samcli/local/apigw/local_apigw_service.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/samcli/local/apigw/local_apigw_service.py b/samcli/local/apigw/local_apigw_service.py index 5af0c31277..e2a9689273 100644 --- a/samcli/local/apigw/local_apigw_service.py +++ b/samcli/local/apigw/local_apigw_service.py @@ -173,7 +173,7 @@ def _request_handler(self, **kwargs): cors_headers = Cors.cors_to_headers(self.api.cors) method, _ = self.get_request_methods_endpoints(request) - if method == "OPTIONS": + if method == "OPTIONS" and method not in route.methods: headers = Headers(cors_headers) return self.service_response("", headers, 200) From 613adba6ddb68147a45b6450b990e18a4c865430 Mon Sep 17 00:00:00 2001 From: Gordon Leigh Date: Tue, 22 Oct 2019 21:30:12 +0100 Subject: [PATCH 2/4] wip: add unit test --- .../local/apigw/test_local_apigw_service.py | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/unit/local/apigw/test_local_apigw_service.py b/tests/unit/local/apigw/test_local_apigw_service.py index 8545a29be5..532b7f3f6d 100644 --- a/tests/unit/local/apigw/test_local_apigw_service.py +++ b/tests/unit/local/apigw/test_local_apigw_service.py @@ -50,6 +50,30 @@ def test_request_must_invoke_lambda(self, request_mock): self.assertEqual(result, make_response_mock) self.lambda_runner.invoke.assert_called_with(ANY, ANY, stdout=ANY, stderr=self.stderr) + @patch.object(LocalApigwService, "get_request_methods_endpoints") + def test_options_request_must_invoke_lambda(self, request_mock): + make_response_mock = Mock() + + self.service.service_response = make_response_mock + self.service._get_current_route = MagicMock() + self.service._get_current_route.return_value.methods = ["OPTIONS"] + self.service._construct_event = Mock() + + parse_output_mock = Mock() + parse_output_mock.return_value = ("status_code", Headers({"headers": "headers"}), "body") + self.service._parse_lambda_output = parse_output_mock + + service_response_mock = Mock() + service_response_mock.return_value = make_response_mock + self.service.service_response = service_response_mock + + request_mock.return_value = ("OPTIONS", "test") + + result = self.service._request_handler() + + self.assertEqual(result, make_response_mock) + self.lambda_runner.invoke.assert_called_with(ANY, ANY, stdout=ANY, stderr=self.stderr) + @patch.object(LocalApigwService, "get_request_methods_endpoints") @patch("samcli.local.apigw.local_apigw_service.LambdaOutputParser") def test_request_handler_returns_process_stdout_when_making_response(self, lambda_output_parser_mock, request_mock): From 0835fa67d038ef1eae513be16c721e1712fc7df1 Mon Sep 17 00:00:00 2001 From: Douglas Naphas Date: Wed, 5 Feb 2020 22:11:00 -0500 Subject: [PATCH 3/4] Respond 200 when CORS is specified https://github.com/awslabs/aws-sam-cli/issues/1434 This is intended to get the following integration tests passing: tests/integration/local/start_api/test_start_api.py TestServiceCorsSwaggerRequests TestServiceCorsGlobalRequests Those tests use a [CorsConfiguration](https://docs.aws.amazon.com/serverless-application-model/latest/developerguide/sam-property-api-corsconfiguration.html) and a [Cors Global](https://docs.aws.amazon.com/serverless-application-model/latest/developerguide/sam-specification-template-anatomy-globals.html), respectively, to enable CORS. When CORS is enabled, OPTIONS requests should send a 200 with the specified CORS headers, even if no OPTIONS handler is explicitly stated in the pre-transformation SAM template. I confirmed this by deploying from a [SAM template](https://github.com/douglasnaphas/play-sam/tree/master/cors-swagger) similar to one of the failing integration tests (with Cors specified but with no explicit OPTIONS handler), and observing that it does indeed respond with a 200 and the CORS headers attached. --- samcli/local/apigw/local_apigw_service.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/samcli/local/apigw/local_apigw_service.py b/samcli/local/apigw/local_apigw_service.py index e2a9689273..9a09fe8566 100644 --- a/samcli/local/apigw/local_apigw_service.py +++ b/samcli/local/apigw/local_apigw_service.py @@ -173,7 +173,7 @@ def _request_handler(self, **kwargs): cors_headers = Cors.cors_to_headers(self.api.cors) method, _ = self.get_request_methods_endpoints(request) - if method == "OPTIONS" and method not in route.methods: + if method == "OPTIONS" and self.api.cors: headers = Headers(cors_headers) return self.service_response("", headers, 200) From bea9e46e8a8b4207acddc4b1ce1376b89a86c46c Mon Sep 17 00:00:00 2001 From: Douglas Naphas Date: Tue, 18 Feb 2020 13:28:30 -0500 Subject: [PATCH 4/4] Add an integration test on OPTIONS handling https://github.com/awslabs/aws-sam-cli/issues/1434 This adds an integration test to verify that templates with explicit OPTIONS handlers actually have those handlers invoked on OPTIONS requests. There has been an issue with OPTIONS requests returning 200 and not reaching an explicitly specified OPTIONS handler. https://github.com/awslabs/aws-sam-cli/pull/1649 fixes that, so if the integration test added in this commit passes (and fails on the develop branch), it could be added to that PR. --- .../local/start_api/test_start_api.py | 20 ++++++++++++++++ .../testdata/start_api/options_handler/app.js | 10 ++++++++ .../start_api/options_handler/index.js | 7 ++++++ .../options_handler/opts-handler-template.yml | 24 +++++++++++++++++++ .../start_api/options_handler/package.json | 10 ++++++++ 5 files changed, 71 insertions(+) create mode 100644 tests/integration/testdata/start_api/options_handler/app.js create mode 100644 tests/integration/testdata/start_api/options_handler/index.js create mode 100644 tests/integration/testdata/start_api/options_handler/opts-handler-template.yml create mode 100644 tests/integration/testdata/start_api/options_handler/package.json diff --git a/tests/integration/local/start_api/test_start_api.py b/tests/integration/local/start_api/test_start_api.py index deba58b4f9..c78ff90d68 100644 --- a/tests/integration/local/start_api/test_start_api.py +++ b/tests/integration/local/start_api/test_start_api.py @@ -801,6 +801,26 @@ def test_swagger_stage_variable(self): response_data = response.json() self.assertEqual(response_data.get("stageVariables"), {"VarName": "varValue"}) +class TestOptionsHandler(StartApiIntegBaseClass): + """ + Test to check that an OPTIONS handler is invoked + """ + + template_path = "/testdata/start_api/options_handler/swagger-template.yaml" + + def setUp(self): + self.url = "http://127.0.0.1:{}".format(self.port) + + @pytest.mark.flaky(reruns=3) + @pytest.mark.timeout(timeout=600, method="thread") + def test_options_handler(self): + """ + This tests that a template's OPTIONS handler is invoked + """ + response = requests.options(self.url + "/", timeout=300) + + self.assertEqual(response.status_code, 204) + class TestServiceCorsSwaggerRequests(StartApiIntegBaseClass): """ diff --git a/tests/integration/testdata/start_api/options_handler/app.js b/tests/integration/testdata/start_api/options_handler/app.js new file mode 100644 index 0000000000..133ba61d3b --- /dev/null +++ b/tests/integration/testdata/start_api/options_handler/app.js @@ -0,0 +1,10 @@ +const express = require("express"); +const app = express(); +app.get("/", (req, res, next) => { + return res.send({ Output: "hello from get" }); +}); +app.options("/", (req, res, next) => { + res.status(204).send({ Output: "hello from options" }); +}); + +module.exports = app; diff --git a/tests/integration/testdata/start_api/options_handler/index.js b/tests/integration/testdata/start_api/options_handler/index.js new file mode 100644 index 0000000000..85d091641a --- /dev/null +++ b/tests/integration/testdata/start_api/options_handler/index.js @@ -0,0 +1,7 @@ +'use strict'; + +const awsServerlessExpress = require('aws-serverless-express') +const app = require('./app') +const server = awsServerlessExpress.createServer(app) + +exports.handler = (event, context) => awsServerlessExpress.proxy(server, event, context); diff --git a/tests/integration/testdata/start_api/options_handler/opts-handler-template.yml b/tests/integration/testdata/start_api/options_handler/opts-handler-template.yml new file mode 100644 index 0000000000..8450140617 --- /dev/null +++ b/tests/integration/testdata/start_api/options_handler/opts-handler-template.yml @@ -0,0 +1,24 @@ +AWSTemplateFormatVersion: "2010-09-09" +Transform: AWS::Serverless-2016-10-31 + +Resources: + Assignments: + Properties: + Environment: + Variables: + NODE_ENV: development + Events: + GetEvent: + Properties: + Method: get + Path: / + Type: Api + OptionsEvent: + Properties: + Method: options + Path: / + Type: Api + Handler: index.handler + Runtime: nodejs12.x + Timeout: 6 + Type: AWS::Serverless::Function diff --git a/tests/integration/testdata/start_api/options_handler/package.json b/tests/integration/testdata/start_api/options_handler/package.json new file mode 100644 index 0000000000..a6b67440f7 --- /dev/null +++ b/tests/integration/testdata/start_api/options_handler/package.json @@ -0,0 +1,10 @@ +{ + "name": "options-handler", + "version": "1.0.0", + "description": "Handle OPTIONS requests", + "main": "index.js", + "dependencies": { + "aws-serverless-express": "^3.3.6", + "express": "^4.17.1" + } +}