Skip to content

add conditions for cur s3 policies #825

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

Merged
merged 9 commits into from
Jun 20, 2024
Merged

add conditions for cur s3 policies #825

merged 9 commits into from
Jun 20, 2024

Conversation

iakov-aws
Copy link
Contributor

Issue #, if available:

Description of changes:

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

Unverified

This user has not yet uploaded their public signing key.
@iakov-aws
Copy link
Contributor Author

@sean-nixon can you have a look at TF part?

Copy link
Contributor

@sean-nixon sean-nixon left a comment

Choose a reason for hiding this comment

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

I've left some inline comments, but at a higher level, all of these changes should be made to the cur-setup-destination module, not the cur-setup-source module.

iakov-aws added 2 commits June 3, 2024 20:24

Unverified

This user has not yet uploaded their public signing key.

Unverified

This user has not yet uploaded their public signing key.
@iakov-aws
Copy link
Contributor Author

I've left some inline comments, but at a higher level, all of these changes should be made to the cur-setup-destination module, not the cur-setup-source module.

Great feedback. Please can you approve the new version?

Copy link
Contributor

@sean-nixon sean-nixon left a comment

Choose a reason for hiding this comment

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

I run bats tests locally and they failed. You're using current in your data source references but the original code uses this. Added inline code suggestions to move everything to this to be consistent.

@sean-nixon
Copy link
Contributor

@iakov-aws Let me know if I can do anything else to support this

iakov-aws and others added 6 commits June 20, 2024 21:39
Co-authored-by: Sean Nixon <sean-nixon@users.noreply.github.com>
Co-authored-by: Sean Nixon <sean-nixon@users.noreply.github.com>
Co-authored-by: Sean Nixon <sean-nixon@users.noreply.github.com>
Co-authored-by: Sean Nixon <sean-nixon@users.noreply.github.com>
Co-authored-by: Sean Nixon <sean-nixon@users.noreply.github.com>
Co-authored-by: Sean Nixon <sean-nixon@users.noreply.github.com>
@iakov-aws iakov-aws merged commit 2630d13 into main Jun 20, 2024
15 checks passed
@iakov-aws iakov-aws deleted the add-s3-restrictions branch June 20, 2024 20:47
eriweb added a commit to eriweb/aws-cudos-framework-deployment that referenced this pull request Jul 8, 2024
iakov-aws pushed a commit that referenced this pull request Jul 8, 2024
@vivien-ramahandry-fox
Copy link

Hello,
I created an issue with these conditions added on the bucket policy of the cur
#989
It doesn't work for me

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

4 participants