Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix OPTIONS handling #1

Open
1 task
douglasnaphas opened this issue Jan 27, 2020 · 6 comments
Open
1 task

Fix OPTIONS handling #1

douglasnaphas opened this issue Jan 27, 2020 · 6 comments

Comments

@douglasnaphas
Copy link
Owner

Upstream issue 1434, PRs 1464 and 1649.

TODO

douglasnaphas added a commit that referenced this issue Jan 27, 2020
#1

I need to see how this stack looks and behaves once deployed.
douglasnaphas added a commit to douglasnaphas/play-sam that referenced this issue Feb 2, 2020
douglasnaphas/aws-sam-cli#1

To see the problem with OPTIONS described in this Issue:

  1. Make one directory where the branch for the PR addressing the issue
is checked out, from the awslabs/aws-sam-cli repo (the PR dir).
  1. Make another directory where the develop branch of the
awslabs/aws-sam-cli repo is checked out (the develop dir).
  1. In each of the above two dirs, activate a virtualenv for Python
3.7.4 and run `make init` to make a `samdev` executable available in
each virtualenv.
  1. In the PR dir, run `samdev local start-api --profile $PROFILE
--template $PLAY-SAM/dev-template.yml --port 5000 --host
localhost`
  1. In the develop dir, run `samdev local start-api --profile $PROFILE
--template $PLAY-SAM/dev-template.yml --port 5001 --host
localhost`
  1. In each dir, run `curl -X OPTIONS -i http://localhost:$PORT` where
`$PORT` is 5000 or 5001, depending on the dir.
  1. You should get a 200 with `"Output":"hello from options"}` as the
response data from the Express server in the PR dir, 200 with no data in
the develop dir. This illustrates the issue of OPTIONS not reaching the
handler.
douglasnaphas added a commit to douglasnaphas/play-sam that referenced this issue Feb 2, 2020
douglasnaphas/aws-sam-cli#1

Running the OPTIONS example template now works with

```
samdev local start-api --profile douglas --template \
~/repos/play-sam/opts-handler/opts-handler-template.yml --port 5000 \
--host localhost
```
douglasnaphas added a commit to douglasnaphas/play-sam that referenced this issue Feb 2, 2020
douglasnaphas/aws-sam-cli#1

This adds a template and handler that mimic the template
swagger-template.yml used by
tests/integration/local/start_api/test_start_api.py ->
TestServiceCorsSwaggerRequests.

Running this with similar steps to those listed a couple of commits ago
illustrates the failing integration test, when an OPTIONS method is
used. `curl -X OPTIONS -i http://localhost:$PORT/echobase64eventbody`
fails in the PR dir, but succeeds in the develop dir.
@douglasnaphas
Copy link
Owner Author

douglasnaphas commented Feb 2, 2020

Attempting to build and deploy to CloudFormation

Build

After touch requirements.txt to create a blank requirements.txt (since sam build [options] failed without one) with the virtualenv activated:

(venv) $ sam build --profile doug --template cors-swagger-template.yml --debug
Telemetry endpoint configured to be https://aws-serverless-tools-telemetry.us-west-2.amazonaws.com/metrics
'build' command is called
No Parameters detected in the template
2 resources found in the template
Found Serverless function with name='EchoBase64EventBodyFunction' and CodeUri='.'
Building resource 'EchoBase64EventBodyFunction'
Loading workflow module 'aws_lambda_builders.workflows'
Registering workflow 'PythonPipBuilder' with capability 'Capability(language='python', dependency_manager='pip', application_framework=None)'
Registering workflow 'NodejsNpmBuilder' with capability 'Capability(language='nodejs', dependency_manager='npm', application_framework=None)'
Registering workflow 'RubyBundlerBuilder' with capability 'Capability(language='ruby', dependency_manager='bundler', application_framework=None)'
Registering workflow 'GoDepBuilder' with capability 'Capability(language='go', dependency_manager='dep', application_framework=None)'
Registering workflow 'GoModulesBuilder' with capability 'Capability(language='go', dependency_manager='modules', application_framework=None)'
Registering workflow 'JavaGradleWorkflow' with capability 'Capability(language='java', dependency_manager='gradle', application_framework=None)'
Registering workflow 'JavaMavenWorkflow' with capability 'Capability(language='java', dependency_manager='maven', application_framework=None)'
Registering workflow 'DotnetCliPackageBuilder' with capability 'Capability(language='dotnet', dependency_manager='cli-package', application_framework=None)'
Found workflow 'PythonPipBuilder' to support capabilities 'Capability(language='python', dependency_manager='pip', application_framework=None)'
Running workflow 'PythonPipBuilder'
Running PythonPipBuilder:ResolveDependencies
PythonPipBuilder:ResolveDependencies succeeded
Running PythonPipBuilder:CopySource
PythonPipBuilder:CopySource succeeded

Build Succeeded

Built Artifacts  : .aws-sam/build
Built Template   : .aws-sam/build/template.yaml

Commands you can use next
=========================
[*] Invoke Function: sam local invoke
[*] Deploy: sam deploy --guided

Sending Telemetry: {'metrics': [{'commandRun': {'awsProfileProvided': True, 'debugFlagProvided': True, 'region': '', 'commandName': 'sam build', 'duration': 249, 'exitReason': 'success', 'exitCode': 0, 'requestId': '046123c6-dba7-41ac-bfad-a886b8a9737e', 'installationId': 'add6fd49-7d12-45c5-92b8-12c8210d5f59', 'sessionId': 'eb78df53-c504-47c2-abe8-1a501a042f2e', 'executionEnvironment': 'CLI', 'pyversion': '3.7.6', 'samcliVersion': '0.40.0'}}]}
HTTPSConnectionPool(host='aws-serverless-tools-telemetry.us-west-2.amazonaws.com', port=443): Read timed out. (read timeout=0.1)

Deploy

(venv) $ ls -a
.                         ..                        .aws-sam                  cors-swagger-template.yml main.py                   requirements.txt          venv

(venv) $ sam local invoke
Invoking main.echo_base64_event_body (python3.6)

Fetching lambci/lambda:python3.6 Docker container image......
Mounting /Users/dougnaphas/repos/play-sam/cors-swagger/.aws-sam/build/EchoBase64EventBodyFunction as /var/task:ro,delegated inside runtime container
START RequestId: 7fa61199-cea0-1215-c80a-99e4f4d0070e Version: $LATEST
'body': KeyError
Traceback (most recent call last):
  File "/var/task/main.py", line 15, in echo_base64_event_body
    "body": event["body"],
KeyError: 'body'

END RequestId: 7fa61199-cea0-1215-c80a-99e4f4d0070e
REPORT RequestId: 7fa61199-cea0-1215-c80a-99e4f4d0070e  Init Duration: 378.60 ms        Duration: 7.30 ms       Billed Duration: 100 ms Memory Size: 128 MB     Max Memory Used: 25 MB

{"errorType":"KeyError","errorMessage":"'body'","stackTrace":["  File \"/var/task/main.py\", line 15, in echo_base64_event_body\n    \"body\": event[\"body\"],\n"]}

(venv) $ sam deploy --profile doug --guided --template .aws-sam/build/template.yaml

Configuring SAM deploy
======================

        Looking for samconfig.toml :  Not found

        Setting default arguments for 'sam deploy'
        =========================================
        Stack Name [sam-app]: cors-swagger-1434-1
        AWS Region [us-east-1]: us-west-2
        #Shows you resources changes to be deployed and require a 'Y' to initiate deploy
        Confirm changes before deploy [y/N]: y
        #SAM needs permission to be able to create roles to connect to the resources in your template
        Allow SAM CLI IAM role creation [Y/n]: y
        Save arguments to samconfig.toml [Y/n]: y

        Looking for resources needed for deployment: Not found.
        Creating the required resources...
        Successfully created!

                Managed S3 bucket: aws-sam-cli-managed-default-samclisourcebucket-1hbwe4gjysnvb
                A different default S3 bucket can be set in samconfig.toml

        Saved arguments to config file
        Running 'sam deploy' for future deployments will use the parameters saved above.
        The above parameters can be changed by modifying samconfig.toml
        Learn more about samconfig.toml syntax at
        https://docs.aws.amazon.com/serverless-application-model/latest/developerguide/serverless-sam-cli-config.html

        Deploying with following values
        ===============================
        Stack name                 : cors-swagger-1434-1
        Region                     : us-west-2
        Confirm changeset          : True
        Deployment s3 bucket       : aws-sam-cli-managed-default-samclisourcebucket-1hbwe4gjysnvb
        Capabilities               : ["CAPABILITY_IAM"]
        Parameter overrides        : {}

Initiating deployment
=====================
Uploading to cors-swagger-1434-1/99dd519189e850f7ac7fb9a3d8c3398b  2861 / 2861.0  (100.00%)
Uploading to cors-swagger-1434-1/9d3a059a86b5bb2fd155b4d2913f32f5.template  1145 / 1145.0  (100.00%)

Waiting for changeset to be created..

CloudFormation stack changeset
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Operation                                                                      LogicalResourceId                                                              ResourceType
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ Add                                                                          EchoBase64EventBodyFunctionRole                                                AWS::IAM::Role
+ Add                                                                          EchoBase64EventBodyFunction                                                    AWS::Lambda::Function
+ Add                                                                          MyApiDeployment4bfb617884                                                      AWS::ApiGateway::Deployment
+ Add                                                                          MyApidevStage                                                                  AWS::ApiGateway::Stage
+ Add                                                                          MyApi                                                                          AWS::ApiGateway::RestApi
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

Changeset created successfully. arn:aws:cloudformation:us-west-2:403958634573:changeSet/samcli-deploy1580667346/2ab5eae5-3473-490c-9564-7855495b3909


Previewing CloudFormation changeset before deployment
======================================================
Deploy this changeset? [y/N]: y

2020-02-02 13:19:00 - Waiting for stack create/update to complete

CloudFormation events from changeset
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
ResourceStatus                                             ResourceType                                               LogicalResourceId                                          ResourceStatusReason
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
CREATE_IN_PROGRESS                                         AWS::IAM::Role                                             EchoBase64EventBodyFunctionRole                            -
CREATE_IN_PROGRESS                                         AWS::IAM::Role                                             EchoBase64EventBodyFunctionRole                            Resource creation Initiated
CREATE_COMPLETE                                            AWS::IAM::Role                                             EchoBase64EventBodyFunctionRole                            -
CREATE_IN_PROGRESS                                         AWS::Lambda::Function                                      EchoBase64EventBodyFunction                                -
CREATE_COMPLETE                                            AWS::Lambda::Function                                      EchoBase64EventBodyFunction                                -
CREATE_IN_PROGRESS                                         AWS::Lambda::Function                                      EchoBase64EventBodyFunction                                Resource creation Initiated
CREATE_IN_PROGRESS                                         AWS::ApiGateway::RestApi                                   MyApi                                                      -
CREATE_COMPLETE                                            AWS::ApiGateway::RestApi                                   MyApi                                                      -
CREATE_IN_PROGRESS                                         AWS::ApiGateway::RestApi                                   MyApi                                                      Resource creation Initiated
CREATE_IN_PROGRESS                                         AWS::ApiGateway::Deployment                                MyApiDeployment4bfb617884                                  -
CREATE_COMPLETE                                            AWS::ApiGateway::Deployment                                MyApiDeployment4bfb617884                                  -
CREATE_IN_PROGRESS                                         AWS::ApiGateway::Deployment                                MyApiDeployment4bfb617884                                  Resource creation Initiated
CREATE_IN_PROGRESS                                         AWS::ApiGateway::Stage                                     MyApidevStage                                              -
CREATE_IN_PROGRESS                                         AWS::ApiGateway::Stage                                     MyApidevStage                                              Resource creation Initiated
CREATE_COMPLETE                                            AWS::ApiGateway::Stage                                     MyApidevStage                                              -
CREATE_COMPLETE                                            AWS::CloudFormation::Stack                                 cors-swagger-1434-1                                        -
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

Successfully created/updated stack - cors-swagger-1434-1 in us-west-2

@douglasnaphas
Copy link
Owner Author

I think this will get the integration tests passing:

        if method == "OPTIONS" and (method not in route.methods or cors_headers["Access-Control-Allow-Origin"] == "*"):

Screen Shot 2020-02-05 at 9 50 37 PM

@douglasnaphas
Copy link
Owner Author

This is probably a better approach:

method, _ = self.get_request_methods_endpoints(request)
        if method == "OPTIONS" and self.api.cors:
            headers = Headers(cors_headers)
            return self.service_response("", headers, 200)

Screen Shot 2020-02-05 at 9 59 40 PM

I need to see if the PR's unit test passes, and probably add a test for the cors case.

douglasnaphas added a commit to douglasnaphas/play-sam that referenced this issue Feb 6, 2020
douglasnaphas/aws-sam-cli#1

API Gateway sends the revised header with lowercase letters. This was
confirmed when hitting the endpoint resulted in an internal server error
with title case letters, but a 200 once this change (to lowercase) was
deployed.
douglasnaphas added a commit to douglasnaphas/play-sam that referenced this issue Feb 6, 2020
douglasnaphas/aws-sam-cli#1

However, the explicity OPTIONS handlers I am adding here do not work
once deployed, it seems due to permissions issues.
@douglasnaphas
Copy link
Owner Author

This says something about how httpMethod must be POST.

@douglasnaphas
Copy link
Owner Author

OpenAPI path object rules: https://swagger.io/specification/#pathItemObject.

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

No branches or pull requests

1 participant