fix(s3): KMS encryption works fine for server access logging target buckets

[zdf1230]

The previous changes(#23514 & #23385) about failing early when certain encryption type being used is not correct. In fact, KMS encryption works fine for server access logging target buckets with proper permission being setup.

So this change is removing the condition failing for the SSE-KMS with customized encryption key case.

However, it is not possible to know which encryption type for the server access logging bucket, so the only checking can be applied after this change merged is failing when logging to self case using BucketEncryption.KMS_MANAGED.

After this fix, the only condition would be failed pre-checking is Log to self and using KMS_MANAGED encryption type

This change only fix the checking condition, so this change won't affect snapshot at all. Hence, Exemption Request.

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

[gitpod-io]

[aws-cdk-automation]

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

[zdf1230]

As described in the description, this change only fix the checking condition, and won't affect snapshot generation at all. Hence, Exemption Request.

[corymhall]

I think since we've had a couple of PRs that assert different behavior we need some more documentation/testing. Can you provide links to documentation the states that you can set up logging with a KMS encrypted bucket?

If it is indeed possible then can we somehow automatically setup the correct permissions to allow it? Can we also add an integration test to assert that it works?

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

Pull request has been modified.

[zdf1230]

I think since we've had a couple of PRs that assert different behavior we need some more documentation/testing. Can you provide links to documentation the states that you can set up logging with a KMS encrypted bucket?

If it is indeed possible then can we somehow automatically setup the correct permissions to allow it? Can we also add an integration test to assert that it works?

Thanks for the response. It is a very good point that we can automatically setup the correct permissions. Added that and the integration tests as well. Deployed in my personal account and verified it works.

Though I cannot find any doc shown that SSE-KMS works. But the reason I know it works fine because we original use SSE-KMS approach in our services. The previous change caused error when we build with newer version of aws-cdk. Hence I am here to fix this to enable us using latest version of aws-cdk

[mergify]

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
• Commit ID: b547f75
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[mergify]

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).