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.

Sign up for GitHub

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

Jump to bottom

feat(kinesis): grantRead now allows the ListShards action and grant is now public #6141

Merged
merged 22 commits into from
Apr 8, 2020
Merged

feat(kinesis): grantRead now allows the ListShards action and grant is now public #6141

merged 22 commits into from
Apr 8, 2020

Conversation

reillykw
Copy link
Contributor

@reillykw reillykw commented Feb 6, 2020 •
edited by shivlaks
Loading

Commit Message

grant() method on the class is private, preventing custom actions on attached policy. This makes that public.

grantRead() API now has ListShards, DescribeStreamSummary, or SubscribeToShard permissions

grantWrite() API no longer has DescribeStream permissions as it has been replaced by ListShards for shard discovery.
Rationale: Align with the Kinesis Producer Library

Closes #3357

BREAKING CHANGE:
grantWrite() API no longer has DescribeStream permissions as it has been replaced by ListShards for shard discovery

End Commit Message

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

@reillykw reillykw mentioned this pull request Feb 6, 2020
Closed
1 task
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@reillykw reillykw requested a review from rix0rrr February 6, 2020 17:25
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@reillykw
Copy link
Contributor Author

reillykw commented Feb 24, 2020 •
edited
Loading

@rix0rrr @iliapolo @eladb Would it be possible to get a review on this in the near term?

@iliapolo
Copy link
Contributor

iliapolo commented Mar 9, 2020

@reillykw Thanks for this PR!

Yes I will get to it soon promptly 👍

@iliapolo iliapolo changed the title feat(kinesis): Add ListShards to actions in grant - Closes #3357 feat(kinesis): Add ListShards to actions in grantRead Mar 12, 2020
@iliapolo iliapolo changed the title feat(kinesis): Add ListShards to actions in grantRead feat(kinesis): Add ListShards to actions in grantRead and make grant public Mar 12, 2020
@iliapolo
Copy link
Contributor

@reillykw You mentioned in the PR description that this adds ListShards to grantWrite as well - where exactly is this reflected in the change?

(Also notice the changes I made to the PR description and title)

@iliapolo iliapolo changed the title feat(kinesis): Add ListShards to actions in grantRead and make grant public feat(kinesis): add ListShards to actions in grantRead and make grant public Mar 17, 2020
@iliapolo iliapolo changed the title feat(kinesis): add ListShards to actions in grantRead and make grant public feat(kinesis): grantRead now allows the ListShards action and grant is now public Mar 17, 2020
@iliapolo
Copy link
Contributor

@reillykw Can you please add a small section in the package README about the grant methods? I just noticed we don't actually have it and it would be nice to list out the various actions we are allowing with those methods.

@reillykw
Copy link
Contributor Author

I reverted that change locally as write doesn't need listShards for write capabilities. I'll add a section to the README soon, thanks.

@reillykw
Copy link
Contributor Author

@iliapolo Added documentation to the README, let me know if that is sufficient. Thanks!

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: 15a1db2
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: 066b6e9
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: 8c415ff
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@shivlaks shivlaks added the @aws-cdk/aws-kinesis Related to Amazon Kinesis label Apr 6, 2020
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: 165b7a2
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

eladb
eladb previously approved these changes Apr 6, 2020
View reviewed changes
@mergify
Copy link
Contributor

mergify bot commented Apr 6, 2020

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: 9363df4
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify mergify bot dismissed eladb’s stale review April 6, 2020 20:24

Pull request has been modified.

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: 4a898b7
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@shivlaks shivlaks added the pr/do-not-merge This PR should not be merged at this time. label Apr 6, 2020
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: f0b05b8
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: 59dd286
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@shivlaks shivlaks removed the pr/do-not-merge This PR should not be merged at this time. label Apr 7, 2020
@shivlaks shivlaks requested review from iliapolo and eladb April 7, 2020 05:15
iliapolo
iliapolo previously requested changes Apr 8, 2020
View reviewed changes
Copy link
Contributor

@iliapolo iliapolo left a comment

Choose a reason for hiding this comment

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

Just want to make sure about the removal of DescribeStream from the grantRead. Other than that im good with all the changes.

@mergify mergify bot dismissed iliapolo’s stale review April 8, 2020 16:36

Pull request has been modified.

@shivlaks shivlaks requested a review from iliapolo April 8, 2020 16:38
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: a7c79bc
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: 4eddd0c
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify
Copy link
Contributor

mergify bot commented Apr 8, 2020

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@mergify mergify bot merged commit 563fba4 into aws:master Apr 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shivlaks shivlaks shivlaks left review comments

@iliapolo iliapolo iliapolo approved these changes

@rix0rrr rix0rrr Awaiting requested review from rix0rrr

@eladb eladb Awaiting requested review from eladb

Assignees

@iliapolo iliapolo

Labels
@aws-cdk/aws-kinesis Related to Amazon Kinesis
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Kinesis Stream permissions missing ListShards
5 participants
@reillykw @aws-cdk-automation @iliapolo @shivlaks @eladb