Skip to content

Conversation

@douglasnaphas
Copy link
Contributor

I am raising this to test out an integration test that I want to add to #1649, since I can't get the integration tests to run locally.

Issue #, if available:

Why is this change necessary?

How does it address the issue?

What side effects does this change have?

Checklist:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Gordon Leigh and others added 4 commits February 6, 2020 14:00
aws#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.
aws#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.
aws#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.
@jfuss
Copy link
Contributor

jfuss commented Feb 18, 2020

This was applied to #1649 and will be reviewing that PR.

@jfuss jfuss closed this Feb 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants