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: "Invalid permissions on Lambda function" on path parameter #992

Merged
merged 3 commits into from
Jul 26, 2019

Conversation

53ningen
Copy link
Contributor

@53ningen 53ningen commented Jun 22, 2019

Issue #, if available:
#110

Description of changes:
fix the issue #110

Description of how you validated changes:
Deployed the following template successfully on CloudFormation.
Also I confirmed deployed APIs works well.

Resources:
  HelloWorldFunction:
    Type: AWS::Serverless::Function
    Properties:
      CodeUri: hello_world/
      Handler: app.lambda_handler
      Events:
        HttpGetUserExampleExample:
          Type: Api
          Properties:
            Path: '/users/example/example'
            Method: GET
        HttpGetUserGroupIdUserId:
          Type: Api
          Properties:
            Path: '/users/{groupId}/{userId}'
            Method: GET
        HttpGetGroupAster:
          Type: Api
          Properties:
            Path: '/groups/{proxy+}'
            Method: GET

Checklist:

  • Write/update tests
    • If this pull request is on the right path, I will add some tests
  • make pr passes
  • Update documentation
  • Verify transformed template deploys and application functions as expected
  • Add/update example to examples/2016-10-31

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

@codecov-io
Copy link

codecov-io commented Jun 22, 2019

Codecov Report

❗ No coverage uploaded for pull request base (develop@275e461). Click here to learn what that means.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##             develop     #992   +/-   ##
==========================================
  Coverage           ?   94.81%           
==========================================
  Files              ?       69           
  Lines              ?     3240           
  Branches           ?      631           
==========================================
  Hits               ?     3072           
  Misses             ?       87           
  Partials           ?       81
Impacted Files Coverage Δ
samtranslator/model/eventsources/push.py 88.46% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 275e461...9208fde. Read the comment docs.

@@ -492,7 +492,7 @@ def _get_permission(self, resources_to_link, stage, suffix):
if not stage or not suffix:
raise RuntimeError("Could not add permission to lambda function.")

path = path.replace('{proxy+}', '*')
path = re.sub(r'{([a-zA-Z0-9._-]+|proxy\+)}', '*', path)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resource's path part only allow a-zA-Z0-9._- and curly braces at the beginning and the end.
2019-06-23 2 19 29

@praneetap
Copy link
Contributor

Thanks for this contribution! This pr is against develop which is correct, could you please add tests as well?

@53ningen
Copy link
Contributor Author

53ningen commented Jul 4, 2019

Thank you for your response.
I will add some tests soon.

Copy link
Contributor Author

@53ningen 53ningen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@praneetap

I've added tests.

@53ningen
Copy link
Contributor Author

rebased on current develop branch

@brettstack
Copy link
Contributor

Thanks! Could you also add an input/output test?

@53ningen
Copy link
Contributor Author

53ningen commented Jul 16, 2019

@brettstack
Thank you for your response.
Does an input/output test mean like this...? 9208fde
Let me know if it is wrong.

Thanks.

@jlhood jlhood merged commit 5f886df into aws:develop Jul 26, 2019
@53ningen 53ningen deleted the bugfix/api-permission branch July 26, 2019 17:48
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.

None yet

5 participants