feat: allow S3EncryptionClient and S3AsyncEncryption Client to be configured #328
Conversation
kessplas
changed the title
|
|
||
| // Instantiate the wrapped Async S3 client with the default credentials | ||
| // and the region to use with S3. | ||
| final S3AsyncClient wrappedAsyncClient = S3AsyncClient.builder() |
There was a problem hiding this comment.
Just for my understanding -- this example only uses the synchronous client, right? This is just demonstrating how to set the async client?
There was a problem hiding this comment.
The synchronous client uses the async client for cryptographic operations. I'll clarify this and add examples for async.
| // By passing the creds into the credentialsProvider parameter, | ||
| // the S3EC will use these creds for both S3 and KMS requests. | ||
| final S3Client s3Client = S3EncryptionClient.builder() | ||
| .credentialsProvider(creds) |
There was a problem hiding this comment.
This is more like a default credentials provider, right?
If I were to pass in my own S3/KMS clients and a credentials provider, the clients' creds would not be overridden -- correct?
I want to get a better sense of what happens if we pass both, and I wonder if we should explain that in docs/comments?
There was a problem hiding this comment.
good point,
the wrapped client takes precedence.
i'll make a note in the Javadoc comments.
| return this; | ||
| } | ||
|
|
||
| public Builder dualstackEnabled(Boolean isDualStackEnabled) { |
There was a problem hiding this comment.
What's going on with this and fipsEnabled? Do we use these? Or are we just acting as a passthrough to send these to SDK? How does SDK use these?
I might want some more Javadocs here just to explain how this is used, esp. on the client builder
There was a problem hiding this comment.
It's just a pass-through,
my understanding is that AWS services have both a "default" endpoint,
and a FIPS endpoint,
which only supports TLS ciphers that are FIPS-validated.
So this points the SDK to the FIPS one.
In that sense, it's similar to endpoint override, but for FIPS.
We should clarify that it does not mean that the S3EC will use FIPS for its encryption/decryption, I'll add a comment.
| String output = objectResponse.asUtf8String(); | ||
| assertEquals(input, output); | ||
|
|
||
| // Cleanup |
There was a problem hiding this comment.
(nit)
I like putting the cleanup at the beginning of tests
If a test fails, the cleanup won't happen since it happens at the end of the test
There was a problem hiding this comment.
The appendTestSuffix method ensures that object keys aren't reused. In the case of test failure, objects are cleaned up via the lifecycle rule configuration on the S3 bucket.
…a into top-level-creds
## [3.2.0](v3.1.3...v3.2.0) (2024-08-20) ### Features * add KMS Discovery Keyring ([#324](#324)) ([8d3c06a](8d3c06a)) * allow S3EncryptionClient and S3AsyncEncryption Client to be configured ([#328](#328)) ([11f25f6](11f25f6)) ### Maintenance * **deps-dev:** bump org.bouncycastle:bcprov-jdk18on ([#260](#260)) ([cd58967](cd58967)) * **deps:** bump software.amazon.awssdk.crt:aws-crt ([#303](#303)) ([cfe325e](cfe325e)) * update build scripts ([#341](#341)) ([8fa4266](8fa4266)) * Update Release CFN ([#343](#343)) ([81606b6](81606b6)), closes [#382](#382)
Issue #, if available: #226
Description of changes: Adds "top-level" client configuration options to the S3 Encryption Client. This lets customers set configuration, e.g. credentials or region in one place which is then propagated to each of the "inner" clients used by the S3EC.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Check any applicable: