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

Add support for bucket notifications #60

Merged
merged 10 commits into from
Sep 27, 2021
Merged

Add support for bucket notifications #60

merged 10 commits into from
Sep 27, 2021

Conversation

max-lobur
Copy link
Contributor

@max-lobur max-lobur commented Sep 17, 2021 •
edited

what

  • Add support for bucket notifications
  • Only SQS destination for now

why

  • To enable log collection

@max-lobur max-lobur requested review from a team as code owners September 17, 2021 19:32
@max-lobur max-lobur added the wip Work in Progress: Not ready for final review or merge label Sep 17, 2021
bridgecrew[bot]
bridgecrew bot reviewed Sep 17, 2021
View reviewed changes
Copy link

@bridgecrew bridgecrew bot left a comment

Choose a reason for hiding this comment

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

Bridgecrew has found 1 infrastructure configuration error in this PR ⬇️

sqs_notifications.tf Show resolved Hide resolved
@max-lobur max-lobur requested a review from a team as a code owner September 17, 2021 19:33
nitrocode
nitrocode reviewed Sep 18, 2021
View reviewed changes
sqs_notifications.tf Outdated Show resolved Hide resolved
@nitrocode
Copy link
Member

@max-lobur can you fix the nitpick I pointed out and the bridgecrew kms_master_key_id suggestion ?

@nitrocode
Copy link
Member

/test all

@max-lobur max-lobur removed the wip Work in Progress: Not ready for final review or merge label Sep 22, 2021
@max-lobur
Copy link
Contributor Author

/test all

@max-lobur
Copy link
Contributor Author

/test all

mcalhoun
mcalhoun previously approved these changes Sep 22, 2021
View reviewed changes
Copy link
Member

@mcalhoun mcalhoun left a comment

Choose a reason for hiding this comment

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

LGTM! Just left a small nit comment

examples/complete/variables.tf Outdated Show resolved Hide resolved
mcalhoun
mcalhoun previously approved these changes Sep 22, 2021
View reviewed changes
Copy link
Member

@mcalhoun mcalhoun left a comment

Choose a reason for hiding this comment

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

LGTM!

@max-lobur
Copy link
Contributor Author

/test all

@max-lobur
Copy link
Contributor Author

/test all

@max-lobur max-lobur merged commit 2c2fac8 into cloudposse:master Sep 27, 2021
@max-lobur max-lobur mentioned this pull request Sep 27, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nitrocode nitrocode nitrocode left review comments

@bridgecrew bridgecrew[bot] bridgecrew left review comments

@Benbentwo Benbentwo Benbentwo approved these changes

@mcalhoun mcalhoun mcalhoun left review comments

@srhopkins srhopkins Awaiting requested review from srhopkins

@3h4x 3h4x Awaiting requested review from 3h4x

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@max-lobur @nitrocode @mcalhoun @Benbentwo @cloudpossebot