Conversation
src/main/java/com/ironcorelabs/tenantsecurity/kms/v1/CachedKeyDecryptor.java
Outdated
Show resolved
Hide resolved
src/main/java/com/ironcorelabs/tenantsecurity/kms/v1/CachedKeyDecryptor.java
Outdated
Show resolved
Hide resolved
src/main/java/com/ironcorelabs/tenantsecurity/kms/v1/CachedKeyDecryptor.java
Outdated
Show resolved
Hide resolved
src/main/java/com/ironcorelabs/tenantsecurity/kms/v1/TenantSecurityClient.java
Outdated
Show resolved
Hide resolved
coltfred
left a comment
coltfred
left a comment
There was a problem hiding this comment.
Couple things I noticed.
I was thinking maybe we should add an example (with a readme explaining that using it during a db transaction for instance would mean no calls to the external service). Thoughts?
| */ | ||
| public CompletableFuture<CachedDecryptor> createCachedDecryptor(String edek, | ||
| DocumentMetadata metadata) { | ||
| return newCachedKeyFromUnwrap(edek, metadata).thenApply(k -> k); |
There was a problem hiding this comment.
You should comment this so we understand why an identity map is needed.
|
|
||
| // === EDEK mismatch === | ||
|
|
||
| public void cachedDecryptorRejectsEdekMismatch() throws Exception { |
There was a problem hiding this comment.
All these tests can use a try resources for the client as well.
| * | ||
| * @return The total operation count | ||
| */ | ||
| int getOperationCount(); |
There was a problem hiding this comment.
I don't see this as having a ton of value. What do you see as the usecase for including it?
There was a problem hiding this comment.
Mostly for assertions in tests, but it doesn't seem bad in general
| * @param operation Function that takes the CachedDecryptor and returns a CompletableFuture | ||
| * @return CompletableFuture resolving to the operation's result | ||
| */ | ||
| public <T> CompletableFuture<T> withCachedDecryptor(EncryptedDocument encryptedDocument, |
There was a problem hiding this comment.
I was trying to figure out if this overload (and the sister ones below) are worth it. This api is getting fairly large.
What do you think?
src/test/java/com/ironcorelabs/tenantsecurity/kms/v1/CachedKeyOpsRoundTrip.java
Outdated
Show resolved
Hide resolved
giarc3
changed the title
giarc3
changed the title
2 participants
This is a strawman implementation of the idea of caching a DEK for reuse for a short period of time.
TODO:
Note that you'll have to do a local mvn install to test the example, since it depends on the new un-released classes