- Sponsor
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Add configuration options for AWS S3 server access logging #3006
Add configuration options for AWS S3 server access logging #3006
Conversation
SkipAccessLoggingBucketAcl bool `mapstructure:"skip_accesslogging_bucket_acl"` | ||
SkipAccessLoggingBucketEnforcedTLS bool `mapstructure:"skip_accesslogging_bucket_enforced_tls"` | ||
SkipAccessLoggingBucketPublicAccessBlocking bool `mapstructure:"skip_accesslogging_bucket_public_access_blocking"` | ||
SkipAccessLoggingBucketSSEncryption bool `mapstructure:"skip_accesslogging_bucket_ssencryption"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please consider adding tests to ensure that the newly added fields are functioning as expected?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @denis256, Thx for your feedback. I tried to updated existing existing tests and added one new for method createS3LoggingInput
. Please let me know if I missed anything else.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @denis256 , Can I politely ask you for re-review once you have some free time please? Thx.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @denis256, do you think you'll have time to take a look on this PR in the upcoming days?
675daef
to
e4b533b
Compare
Hey @findmyname666 , are you willing to rebase to get this PR up to speed? Also, are you able to run the tests locally to confirm that they work as expected? |
Hi @yhakbar , I will try to do it this week. Thx. |
e4b533b
to
d6f82a1
Compare
|
Hi @yhakbar, I re-based the code and re-tested
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re-tested and it passed. Thank you so much, @findmyname666 !
Great work. I really appreciate the effort and time you put into testing this.
[](https://renovatebot.com) This PR contains the following updates: | Package | Update | Change | |---|---|---| | [gruntwork-io/terragrunt](https://togithub.com/gruntwork-io/terragrunt) | patch | `v0.66.3` -> `v0.66.5` | | [gruntwork-io/terragrunt](https://togithub.com/gruntwork-io/terragrunt) | patch | `0.66.3` -> `0.66.5` | --- ### Release Notes <details> <summary>gruntwork-io/terragrunt (gruntwork-io/terragrunt)</summary> ### [`v0.66.5`](https://togithub.com/gruntwork-io/terragrunt/releases/tag/v0.66.5) [Compare Source](https://togithub.com/gruntwork-io/terragrunt/compare/v0.66.4...v0.66.5) #### What's Changed - chore: Adding `bugs` Lint Preset by [@​yhakbar](https://togithub.com/yhakbar) in [https://github.com/gruntwork-io/terragrunt/pull/3334](https://togithub.com/gruntwork-io/terragrunt/pull/3334) - chore: Adding `performance` Lint Preset by [@​yhakbar](https://togithub.com/yhakbar) in [https://github.com/gruntwork-io/terragrunt/pull/3336](https://togithub.com/gruntwork-io/terragrunt/pull/3336) **Full Changelog**: gruntwork-io/terragrunt@v0.66.4...v0.66.5 ### [`v0.66.4`](https://togithub.com/gruntwork-io/terragrunt/releases/tag/v0.66.4) [Compare Source](https://togithub.com/gruntwork-io/terragrunt/compare/v0.66.3...v0.66.4) #### What's Changed - feat: Add stale issue workflow by [@​yhakbar](https://togithub.com/yhakbar) in [https://github.com/gruntwork-io/terragrunt/pull/3329](https://togithub.com/gruntwork-io/terragrunt/pull/3329) - feat: Add configuration options for AWS S3 server access logging by [@​findmyname666](https://togithub.com/findmyname666) in [https://github.com/gruntwork-io/terragrunt/pull/3006](https://togithub.com/gruntwork-io/terragrunt/pull/3006) - fix: Fixing command not found error by [@​yhakbar](https://togithub.com/yhakbar) in [https://github.com/gruntwork-io/terragrunt/pull/3330](https://togithub.com/gruntwork-io/terragrunt/pull/3330) - fix: Fixing stale action by [@​yhakbar](https://togithub.com/yhakbar) in [https://github.com/gruntwork-io/terragrunt/pull/3332](https://togithub.com/gruntwork-io/terragrunt/pull/3332) - fix: Fix CICD tests for mocks by [@​denis256](https://togithub.com/denis256) in [https://github.com/gruntwork-io/terragrunt/pull/3335](https://togithub.com/gruntwork-io/terragrunt/pull/3335) - chore: Bumping `golangci-lint` to `v1.59.1` by [@​yhakbar](https://togithub.com/yhakbar) in [https://github.com/gruntwork-io/terragrunt/pull/3333](https://togithub.com/gruntwork-io/terragrunt/pull/3333) - chore: Hiding Mocks Tests by [@​yhakbar](https://togithub.com/yhakbar) in [https://github.com/gruntwork-io/terragrunt/pull/3331](https://togithub.com/gruntwork-io/terragrunt/pull/3331) #### New Contributors - [@​findmyname666](https://togithub.com/findmyname666) made their first contribution in [https://github.com/gruntwork-io/terragrunt/pull/3006](https://togithub.com/gruntwork-io/terragrunt/pull/3006) **Full Changelog**: gruntwork-io/terragrunt@v0.66.3...v0.66.4 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR was generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View the [repository job log](https://developer.mend.io/github/akrantz01/homelab). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC4yMC4xIiwidXBkYXRlZEluVmVyIjoiMzguMjYuMSIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOltdfQ==--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Description
Fixes #3003.
In our environment the S3 bucket where should be stored S3 access logs is pre-created by "central" team and we don't have permissions to do any changes. In such case we don't have to:
PartitionDateSource
.Additionally 1) adding error logging into the function
configureAccessLogBucket
. Without it I wasn't able to detect why the logging isn't configured on the source bucket. The error is returned but ignored. 2) Adding code to detect if the logging policy / configuration was change. The previous code just check if some policy is enabled.TODOs
Read the Gruntwork contribution guidelines.
Release Notes (draft)
Added / Removed / Updated [X].
Migration Guide
N/A.