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

Set default bucket encryption during bucket creation #8478

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

Conversation

vh05
Copy link
Contributor

@vh05 vh05 commented Oct 23, 2024

All S3 buckets have encryption configured by default, and objects are automatically encrypted by using server side encryption. When we do get-bucker-encryption on any bucket we get the the default encryption configuration.

With this patch we set default encryption on bucket while creating the bucket and follow the behavior of S3 bucket

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2318715

@vh05 vh05 force-pushed the get_bucket_encryption branch from 2ad2f5c to 0ae78a6 Compare November 5, 2024 08:46
@vh05 vh05 force-pushed the get_bucket_encryption branch 3 times, most recently from 0cdb765 to a9b6309 Compare November 12, 2024 11:56
@vh05 vh05 force-pushed the get_bucket_encryption branch from a9b6309 to e8b1651 Compare November 12, 2024 12:04
All S3 buckets have encryption configured by default,
and objects are automatically encrypted by using server
side encryption. When we do get-bucker-encryption on
any bucket we get the the default encryption configuration.

With this patch we set default encryption on bucket while
creating the bucket and follow the behavior of S3 bucket

Signed-off-by: Vinayakswami Hariharmath <[email protected]>
@vh05 vh05 force-pushed the get_bucket_encryption branch from e8b1651 to ad54e15 Compare November 13, 2024 05:55
@pull-request-size pull-request-size bot added size/M and removed size/S labels Nov 13, 2024

// Set default server side bucket encryption
// More details: https://docs.aws.amazon.com/AmazonS3/latest/userguide/bucket-encryption.html
await req.object_sdk.put_bucket_encryption({
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be set for any bucket, even namespace buckets (S3, blob)? It's worth checking the current support for bucket encryption in NS buckets and mapping the gaps. I think that if there are any gaps, we should at least make sure we are not setting a configuration that might conflict with the target configuration (e.g. S3 bucket)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tested it on aws-s3 namespace bucket. The bucket created out of aws-s3 namespace, is not applied with default server side encryption. Only the parent bucket is encrypted.

The bucket created here is through obc. Do we need to apply the encryption here as well and check ?

Copy link
Contributor

Choose a reason for hiding this comment

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

For NS S3, since we don't create the bucket on AWS itself, we should rely on the default of AWS, which should be exactly this unless someone else has set it.
When receiving putBucketEncryption for NS S3 we should simply pass it through to the target bucket and return the result, same for getBucketEncryption, simply return it from the target.

Azure, we might want to skip if it's not the same for now.

For NS FS, I would also not touch for now. Especially not as a default.

That leaves our data buckets, for them, we encrypt all the objects anyway, it's part of the pipeline. So we simply want to set this config so we can return it on a get operation.

@dannyzaken @vh05 wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay. For NS S3 we will show the underlying bucket's encryption configuration.

@liranmauda liranmauda requested a review from jackyalbo November 18, 2024 09:58
@nimrod-becker nimrod-becker self-requested a review November 19, 2024 10:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants