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

updated documenation regarding multiple bucket access #574

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ilich
Copy link

@ilich ilich commented Sep 12, 2024

Issue #, if available:
The documentation does not mention that it is possible to provide access to multiple buckets

Description of changes:

  • Added the note about multiple bucket access
  • Added note regarding S3 bucket resource policy

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

@simonkrol
Copy link
Member

Hi @ilich,

That seems reasonable to me, I'll add a task to have this included in an upcoming release. While you won't see this PR get merged due to our internal policy, once the changes have been released, you'll see the PR get closed and a link to this PR and your Github profile included in the External Contributors section at the bottom of the readme.

Thanks for your contributions to SIH,
Simon

@simonkrol simonkrol self-requested a review September 18, 2024 13:31
@simonkrol simonkrol self-assigned this Sep 18, 2024
@ilich
Copy link
Author

ilich commented Sep 19, 2024

Hi Simon,

Thank you for the update and for the useful architecture pattern.

@simonkrol
Copy link
Member

Hi @ilich,

I'm working on including this internally for an upcoming release, but wanted to get a few more details from you.

When you updated the source buckets, and had to manually update the role, were you changing things through the Lambda environment variables? If so, if you update that value through the template parameters(by updating the stack with the same template), does the role get updated properly?

Thanks,
Simon

@ilich
Copy link
Author

ilich commented Oct 19, 2024

The comma-separated list of buckets works well. The policy created by the CDK scripts handled all of them.

The resource policy is an existing policy we had. It was created manually, and I had to update it manually as well. I added that note more for debugging purposes because the original error we had in runtime was a bit misleading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants