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

Added dynamodb:DescribeTable to DynamoDBCrudPolicy and DynamoDBReadPo… #511

Merged
merged 5 commits into from
Sep 17, 2018
Merged

Conversation

sliedig
Copy link
Contributor

@sliedig sliedig commented Jul 15, 2018

Issue #, if available:
#509

Description of changes:
Added dynamodb:DescribeTable to DynamoDBCrudPolicy and DynamoDBReadPolicy policy templates

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

@brettstack
Copy link
Contributor

@sliedig could you rebase this on latest develop?

@sliedig
Copy link
Contributor Author

sliedig commented Sep 16, 2018

Done

@brettstack
Copy link
Contributor

Thanks. Unfortunately there's still a couple of issues:

  1. You've revived the docs/ JSON file which has been removed. You'll need to delete the file.
  2. You'll need to update the policy template test so that the expected output includes your changes.

@sliedig
Copy link
Contributor Author

sliedig commented Sep 17, 2018

I see that. Apologies, let me sort that out. Thanks.

Copy link
Contributor

@brettstack brettstack left a comment

Choose a reason for hiding this comment

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

Thanks! 🎉

@brettstack brettstack merged commit c8a0bd2 into aws:develop Sep 17, 2018
@turjachaudhuri
Copy link

Any idea when this will be merged into the master branch . I faced the same issue , and for now , I have to manually add the policy from IAM to get it working after my app is deployed from the serverless app repository?
Thanks in advance.

@keetonian
Copy link
Contributor

Per company policy I can't give you an exact date, but we are currently rolling out SAM v1.7.0 and are planning another release very soon for SAM that will have these changes (among other recent items) in it.

@sliedig
Copy link
Contributor Author

sliedig commented Oct 17, 2018

I take it this PR did not make it into the v1.7.0 release?

@jlhood
Copy link
Contributor

jlhood commented Oct 17, 2018

It was not in the v1.7.0 release, but it is currently in the v1.8.0 candidate release branch.

@MattiAstedrone
Copy link

MattiAstedrone commented Oct 22, 2018

Should the DynamoDBReadPolicy policy also give permission to the table's Global Secondary Indexes? I was hoping it would but that doesn't seem to be the case.

Using the policy, my lambda throws this error:
"User: arn:aws:sts::1234:assumed-role/CodeStarWorker-Lambda/awscodestar-lambda-1K9 is not authorized to perform: dynamodb:Query on resource: arn:aws:dynamodb:eu-west-1:1234:table/awscodestar-lambda-TokenTable-1VL/index/token-index"

I can fix it manually by adding * to the end of the IAM policy resource (tablename), like "awscodestar-lambda-TokenTable-1VL" -> "awscodestar-lambda-TokenTable-1VL*". But if I update the lambda, the change gets overwritten.

I tried to add the * in the SAM template but it didn't work out. Like this:

  • DynamoDBReadPolicy:
    TableName: !Join ['', [!Ref TokenTable, '*' ]]

@keetonian
Copy link
Contributor

This feature has now been released with 1.8.0

@sliedig
Copy link
Contributor Author

sliedig commented Oct 27, 2018

Thank you! :-)

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

6 participants