Skip to content

Improve incorrect access policy in s3api example #6482

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

Closed
wants to merge 1 commit into from

Conversation

bdavs3
Copy link

@bdavs3 bdavs3 commented Oct 14, 2021

Description of changes: Fix the "Resource" key in an SNS Access Policy so that it correctly reflects an SNS topic ARN, not an S3 bucket.

To confirm that this is correct, see the relevant AWS docs.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Copy link
Member

@kdaily kdaily left a comment

Choose a reason for hiding this comment

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

Thanks @bdavs3!

In this case, wouldn't the SNS topic need to be the same as in the example command (arn:aws:sns:us-west-2:123456789012:s3-notification-topic)?

@kdaily kdaily added the documentation This is a problem with documentation. label Nov 6, 2021
• In put-bucket-notification and put-bucket-notification-configuration, update the "Resource" key in the SNS Access Policy to correctly reflect that it is an SNS topic ARN, not an S3 bucket.
• To confirm that this is correct, see https://docs.aws.amazon.com/sns/latest/dg/sns-access-policy-use-cases.html#sns-grant-aws-account-access-to-topic
@bdavs3 bdavs3 force-pushed the s3api-documentation branch from f4bff47 to dd17373 Compare November 6, 2021 00:19
@bdavs3
Copy link
Author

bdavs3 commented Nov 6, 2021

@kdaily Yep, I agree. I amended my change.

@bdavs3
Copy link
Author

bdavs3 commented Feb 14, 2022

@kdaily Any movement on this, or other requests? I'm not sure if the force push was too aggressive. As a dev, do you want to preserve all of history no matter what?

@justindho
Copy link
Contributor

Hi @bdavs3, thanks for pointing out this issue initially. It looks like this was fixed in #6866, which was merged a few weeks ago. As a result, I will be closing this PR.

@justindho justindho closed this May 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation This is a problem with documentation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants