New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Add support AWS-KMS on S3 for FTE #22529

Merged

Praveen2112 merged 1 commit into trinodb:master from marcinsbd:marcinsbd/FTE-to-support-self-managed-keys-on-S3

Aug 29, 2024

Contributor

marcinsbd commented Jun 27, 2024 •

edited by Praveen2112

Loading

Description

This PR adds support for encrypting the files stored in S3 bucket as a part of files generated by FTEs.

Additional context and related issues

Release notes

( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
(x) Release notes are required, with the following suggested text:

# Section
* Allow configuring encryption for S3 Filesystem when used for FTE

cla-bot bot added the cla-signed label

marcinsbd marked this pull request as draft

June 27, 2024 13:10

Praveen2112 reviewed

View reviewed changes

...system/src/main/java/io/trino/plugin/exchange/filesystem/s3/S3FileSystemExchangeStorage.java Outdated Show resolved Hide resolved

...change-filesystem/src/main/java/io/trino/plugin/exchange/filesystem/s3/ExchangeS3Config.java Outdated Show resolved Hide resolved

...change-filesystem/src/main/java/io/trino/plugin/exchange/filesystem/s3/ExchangeS3Config.java Show resolved Hide resolved

testing/trino-testing-containers/src/main/java/io/trino/testing/containers/Kes.java Outdated Show resolved Hide resolved

...ge-filesystem/src/test/java/io/trino/plugin/exchange/filesystem/containers/MinioStorage.java Show resolved Hide resolved

...ge-filesystem/src/test/java/io/trino/plugin/exchange/filesystem/containers/MinioStorage.java Outdated Show resolved Hide resolved

marcinsbd force-pushed the marcinsbd/FTE-to-support-self-managed-keys-on-S3 branch 3 times, most recently from b97d5f7 to 52f3752 Compare

July 23, 2024 19:45

marcinsbd marked this pull request as ready for review

July 23, 2024 20:51

marcinsbd force-pushed the marcinsbd/FTE-to-support-self-managed-keys-on-S3 branch 3 times, most recently from 2d96da6 to 016c10e Compare

July 26, 2024 10:40

Praveen2112 reviewed

View reviewed changes

...system/src/main/java/io/trino/plugin/exchange/filesystem/s3/S3FileSystemExchangeStorage.java Outdated Show resolved Hide resolved

...ge-filesystem/src/test/java/io/trino/plugin/exchange/filesystem/containers/MinioStorage.java Outdated Show resolved Hide resolved

...ge-filesystem/src/test/java/io/trino/plugin/exchange/filesystem/s3/TestExchangeS3Config.java Outdated Show resolved Hide resolved

Member

Praveen2112 commented Jul 31, 2024

@marcinsbd Can you please rebase the PR ?

marcinsbd force-pushed the marcinsbd/FTE-to-support-self-managed-keys-on-S3 branch 2 times, most recently from 6b5178b to 6d22b83 Compare

July 31, 2024 18:11

marcinsbd requested review from Praveen2112, electrum and losipiuk

July 31, 2024 20:49

Praveen2112 approved these changes

View reviewed changes

...system/src/main/java/io/trino/plugin/exchange/filesystem/s3/S3FileSystemExchangeStorage.java Outdated Show resolved Hide resolved

...system/src/main/java/io/trino/plugin/exchange/filesystem/s3/S3FileSystemExchangeStorage.java Outdated Show resolved Hide resolved

.../trino-testing-containers/src/main/java/io/trino/testing/containers/KeyManagementServer.java Show resolved Hide resolved

marcinsbd force-pushed the marcinsbd/FTE-to-support-self-managed-keys-on-S3 branch 2 times, most recently from 8f6cd1e to 0cc6d27 Compare

August 12, 2024 10:37

ssheikin reviewed

View reviewed changes

...change-filesystem/src/main/java/io/trino/plugin/exchange/filesystem/s3/ExchangeS3Config.java Outdated Show resolved Hide resolved

ssheikin reviewed

View reviewed changes

...system/src/main/java/io/trino/plugin/exchange/filesystem/s3/S3FileSystemExchangeStorage.java Outdated Show resolved Hide resolved

...ge-filesystem/src/test/java/io/trino/plugin/exchange/filesystem/containers/MinioStorage.java Outdated Show resolved Hide resolved

...ge-filesystem/src/test/java/io/trino/plugin/exchange/filesystem/containers/MinioStorage.java Outdated Show resolved Hide resolved

.../test/java/io/trino/plugin/exchange/filesystem/s3/TestS3FileSystemExchangeManagerSseKms.java Outdated Show resolved Hide resolved

...c/test/java/io/trino/plugin/exchange/filesystem/s3/TestS3FileSystemExchangeManagerSseS3.java Outdated Show resolved Hide resolved

...c/test/java/io/trino/plugin/exchange/filesystem/s3/TestS3FileSystemExchangeManagerSseS3.java Outdated Show resolved Hide resolved

.../test/java/io/trino/plugin/exchange/filesystem/s3/TestS3FileSystemExchangeManagerSseKms.java Outdated Show resolved Hide resolved

marcinsbd force-pushed the marcinsbd/FTE-to-support-self-managed-keys-on-S3 branch from 0cc6d27 to a64ae47 Compare

August 13, 2024 08:09

marcinsbd requested review from ssheikin and Praveen2112

August 13, 2024 10:19

ssheikin reviewed

View reviewed changes

...change-filesystem/src/main/java/io/trino/plugin/exchange/filesystem/s3/ExchangeS3Config.java Outdated Show resolved Hide resolved

marcinsbd force-pushed the marcinsbd/FTE-to-support-self-managed-keys-on-S3 branch 2 times, most recently from 5b312b8 to 9f28c49 Compare

August 14, 2024 11:59

ssheikin reviewed

View reviewed changes

...system/src/main/java/io/trino/plugin/exchange/filesystem/s3/S3FileSystemExchangeStorage.java Outdated Show resolved Hide resolved

marcinsbd force-pushed the marcinsbd/FTE-to-support-self-managed-keys-on-S3 branch from 9f28c49 to 3a7db93 Compare

August 14, 2024 12:42

marcinsbd requested a review from ssheikin

August 14, 2024 14:16

losipiuk reviewed

View reviewed changes

...change-filesystem/src/main/java/io/trino/plugin/exchange/filesystem/s3/ExchangeS3Config.java Outdated

Comment on lines 309 to 314

    
                  @AssertTrue(message = "Property value of exchange.s3.sse.kms-key-id needs to be provided only when exchange.s3.sse.type=KMS")

                  public boolean isSseConfigValid()

                  {

                      return sseType == KMS ^ getSseKmsKeyId().isEmpty();

                  }

Member

losipiuk Aug 28, 2024

nit: Would be a bit more readable to have two separate validations for

sseType != KMS && sskeKeyId not emtpy
sseType == KMS && sskKeyId empty

Then you could provide easier to understand error messages.

Contributor Author

marcinsbd Aug 28, 2024

done

losipiuk approved these changes

View reviewed changes


          Add support AWS-KMS on S3 for FTE

51bc447

marcinsbd force-pushed the marcinsbd/FTE-to-support-self-managed-keys-on-S3 branch from 3a7db93 to 51bc447 Compare

August 28, 2024 11:43

Praveen2112 approved these changes

View reviewed changes

Praveen2112 merged commit 739aeff into trinodb:master

94 checks passed

github-actions bot added this to the 455 milestone

mosabua mentioned this pull request

Add Trino 455 release notes #23096

Merged

soenkeliebau mentioned this pull request

chore(tracking): Release SDP 24.11.0 stackabletech/issues#647

Closed

# for free to join this conversation on GitHub. Already have an account? # to comment

Labels