-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Add RetryingClientEncryption and use it in one prose test #1365
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,22 +17,15 @@ | |
package com.mongodb.reactivestreams.client; | ||
|
||
import com.mongodb.ClientEncryptionSettings; | ||
import com.mongodb.MongoClientSettings; | ||
import com.mongodb.client.AbstractClientEncryptionRewrapManyDataKeyProseTest; | ||
import com.mongodb.client.MongoClient; | ||
import com.mongodb.client.vault.ClientEncryption; | ||
import com.mongodb.client.vault.ClientEncryptions; | ||
import com.mongodb.reactivestreams.client.syncadapter.SyncMongoClient; | ||
import com.mongodb.reactivestreams.client.syncadapter.SyncClientEncryption; | ||
import com.mongodb.reactivestreams.client.vault.ClientEncryptions; | ||
|
||
public class ClientEncryptionRewrapManyDataKeyProseTest extends AbstractClientEncryptionRewrapManyDataKeyProseTest { | ||
|
||
@Override | ||
protected MongoClient createMongoClient(final MongoClientSettings settings) { | ||
return new SyncMongoClient(MongoClients.create(settings)); | ||
} | ||
|
||
@Override | ||
public ClientEncryption getClientEncryption(final ClientEncryptionSettings settings) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is actually a bug in the test, as this should be creating a reactive ClientEncryption and wrapping it in a SyncClientEncryption. |
||
return ClientEncryptions.create(settings); | ||
public ClientEncryption createClientEncryption(final ClientEncryptionSettings settings) { | ||
return new SyncClientEncryption(ClientEncryptions.create(settings)); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,7 +18,6 @@ | |
|
||
import com.mongodb.ClientEncryptionSettings; | ||
import com.mongodb.MongoClientException; | ||
import com.mongodb.MongoClientSettings; | ||
import com.mongodb.client.model.vault.DataKeyOptions; | ||
import com.mongodb.client.model.vault.EncryptOptions; | ||
import com.mongodb.client.model.vault.RewrapManyDataKeyOptions; | ||
|
@@ -29,6 +28,7 @@ | |
import org.bson.BsonString; | ||
import org.junit.jupiter.api.AfterEach; | ||
import org.junit.jupiter.api.Assumptions; | ||
import org.junit.jupiter.api.BeforeEach; | ||
import org.junit.jupiter.api.Test; | ||
import org.junit.jupiter.api.function.Executable; | ||
import org.junit.jupiter.params.ParameterizedTest; | ||
|
@@ -47,6 +47,7 @@ | |
import static com.mongodb.ClusterFixture.serverVersionAtLeast; | ||
import static com.mongodb.client.Fixture.getMongoClient; | ||
import static com.mongodb.client.Fixture.getMongoClientSettingsBuilder; | ||
import static java.util.Objects.requireNonNull; | ||
import static org.junit.jupiter.api.Assertions.assertEquals; | ||
import static org.junit.jupiter.api.Assertions.assertThrows; | ||
|
||
|
@@ -101,8 +102,9 @@ public abstract class AbstractClientEncryptionRewrapManyDataKeyProseTest { | |
}}); | ||
}}; | ||
|
||
protected abstract MongoClient createMongoClient(MongoClientSettings settings); | ||
public abstract ClientEncryption getClientEncryption(ClientEncryptionSettings settings); | ||
private ClientEncryption clientEncryption; | ||
|
||
protected abstract ClientEncryption createClientEncryption(ClientEncryptionSettings settings); | ||
|
||
public static Collection<Arguments> data() { | ||
List<Arguments> data = new ArrayList<>(); | ||
|
@@ -120,59 +122,50 @@ protected AbstractClientEncryptionRewrapManyDataKeyProseTest() { | |
Assumptions.assumeTrue(hasEncryptionTestsEnabled(), "Custom Endpoint tests disables"); | ||
} | ||
|
||
@BeforeEach | ||
void beforeEach() { | ||
clientEncryption = createClientEncryption(ClientEncryptionSettings.builder() | ||
.keyVaultMongoClientSettings(getMongoClientSettingsBuilder().build()) | ||
.keyVaultNamespace("keyvault.datakeys") | ||
.kmsProviders(KMS_PROVIDERS) | ||
.build()); | ||
getMongoClient().getDatabase("keyvault").getCollection("datakeys").drop(); | ||
} | ||
|
||
@AfterEach | ||
void cleanUp(){ | ||
getMongoClient().getDatabase("keyvault").getCollection("datakeys").drop(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No need to drop on cleanup, since we drop on setup. |
||
clientEncryption.close(); | ||
} | ||
|
||
@ParameterizedTest | ||
@MethodSource("data") | ||
public void rewrapWithSeparateClientEncryption(final String srcProvider, final String dstProvider) { | ||
// Interactions with KMS are unreliable, so retry all operations | ||
ClientEncryption retryingClientEncryption = new RetryingClientEncryption(clientEncryption); | ||
|
||
BsonDocument srcKey = MASTER_KEYS_BY_PROVIDER.get(srcProvider); | ||
BsonDocument dstKey = MASTER_KEYS_BY_PROVIDER.get(dstProvider); | ||
BsonString testString = new BsonString("test"); | ||
|
||
getMongoClient().getDatabase("keyvault").getCollection("datakeys").drop(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moved to setup |
||
|
||
ClientEncryption clientEncryption1 = getClientEncryption(ClientEncryptionSettings.builder() | ||
.keyVaultMongoClientSettings(getMongoClientSettingsBuilder().build()) | ||
.keyVaultNamespace("keyvault.datakeys") | ||
.kmsProviders(KMS_PROVIDERS) | ||
.build()); | ||
|
||
BsonBinary keyId = clientEncryption1.createDataKey(srcProvider, new DataKeyOptions().masterKey(srcKey)); | ||
BsonBinary keyId = retryingClientEncryption.createDataKey(srcProvider, new DataKeyOptions().masterKey(srcKey)); | ||
|
||
BsonBinary ciphertext = clientEncryption1.encrypt( | ||
BsonBinary ciphertext = retryingClientEncryption.encrypt( | ||
testString, | ||
new EncryptOptions("AEAD_AES_256_CBC_HMAC_SHA_512-Deterministic").keyId(keyId)); | ||
|
||
ClientEncryption clientEncryption2 = getClientEncryption(ClientEncryptionSettings.builder() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These two ClientEncryption instances are identical. Is there any actual need to have two of them? |
||
.keyVaultMongoClientSettings(getMongoClientSettingsBuilder().build()) | ||
.keyVaultNamespace("keyvault.datakeys") | ||
.kmsProviders(KMS_PROVIDERS) | ||
.build()); | ||
|
||
RewrapManyDataKeyResult result = clientEncryption2.rewrapManyDataKey( | ||
RewrapManyDataKeyResult result = retryingClientEncryption.rewrapManyDataKey( | ||
new BsonDocument(), | ||
new RewrapManyDataKeyOptions().provider(dstProvider).masterKey(dstKey)); | ||
assertEquals(1, result.getBulkWriteResult().getModifiedCount()); | ||
assertEquals(1, requireNonNull(result.getBulkWriteResult()).getModifiedCount()); | ||
|
||
assertEquals(testString, clientEncryption1.decrypt(ciphertext)); | ||
assertEquals(testString, clientEncryption2.decrypt(ciphertext)); | ||
assertEquals(testString, retryingClientEncryption.decrypt(ciphertext)); | ||
assertEquals(testString, retryingClientEncryption.decrypt(ciphertext)); | ||
} | ||
|
||
@Test | ||
public void shouldThrowClientErrorWhenProviderIsNotSpecified() { | ||
//given | ||
ClientEncryption clientEncryption = getClientEncryption(ClientEncryptionSettings.builder() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now created in setup. But we don't want to wrap with RetryingClientEncryption because we are actually testing that the operation throws an exception. |
||
.keyVaultMongoClientSettings(getMongoClientSettingsBuilder().build()) | ||
.keyVaultNamespace("keyvault.datakeys") | ||
.kmsProviders(KMS_PROVIDERS) | ||
.build()); | ||
|
||
RewrapManyDataKeyOptions rewrapManyDataKeyOptions = new RewrapManyDataKeyOptions().masterKey(BsonDocument.parse("{}")); | ||
|
||
//when | ||
RewrapManyDataKeyOptions rewrapManyDataKeyOptions = new RewrapManyDataKeyOptions().masterKey(BsonDocument.parse("{}")); | ||
Executable executable = () -> clientEncryption.rewrapManyDataKey(new BsonDocument(), rewrapManyDataKeyOptions); | ||
|
||
//then | ||
|
@@ -181,4 +174,6 @@ public void shouldThrowClientErrorWhenProviderIsNotSpecified() { | |
assertEquals("Missing the provider but supplied a master key in the RewrapManyDataKeyOptions", | ||
mongoClientException.getMessage()); | ||
} | ||
|
||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,160 @@ | ||
/* | ||
* Copyright 2008-present MongoDB, Inc. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
package com.mongodb.client; | ||
|
||
import com.mongodb.client.model.CreateCollectionOptions; | ||
import com.mongodb.client.model.CreateEncryptedCollectionParams; | ||
import com.mongodb.client.model.vault.DataKeyOptions; | ||
import com.mongodb.client.model.vault.EncryptOptions; | ||
import com.mongodb.client.model.vault.RewrapManyDataKeyOptions; | ||
import com.mongodb.client.model.vault.RewrapManyDataKeyResult; | ||
import com.mongodb.client.result.DeleteResult; | ||
import com.mongodb.client.vault.ClientEncryption; | ||
import com.mongodb.internal.diagnostics.logging.Logger; | ||
import com.mongodb.internal.diagnostics.logging.Loggers; | ||
import com.mongodb.lang.Nullable; | ||
import org.bson.BsonBinary; | ||
import org.bson.BsonDocument; | ||
import org.bson.BsonValue; | ||
import org.bson.conversions.Bson; | ||
|
||
import java.util.function.Supplier; | ||
|
||
/** | ||
* An implementation of {@link ClientEncryption} used for testing, that retries any operation that interacts with the Key Management | ||
* Service (KMS), in order to handle transient errors from the KMS (e.g. socket timeouts) that are encountered occasionally in CI. | ||
* | ||
* <p> | ||
* Each operation will be retried three times, and sleep for one second before each retry. If the operation fails after three attempts, | ||
* the last exception will be thrown. | ||
* </p> | ||
* | ||
* <p> | ||
* Note that this class is not appropriate for testing operations that the test expects to throw exceptions, as there is no way to | ||
* configure it to not retry when an exception is expected. | ||
* </p> | ||
* | ||
* <p> | ||
* Flaky tests like this can wrap their {@code ClientEncryption} in a {@code RetryingClientEncryption}. | ||
* </p> | ||
*/ | ||
public class RetryingClientEncryption implements ClientEncryption { | ||
private static final Logger LOGGER = Loggers.getLogger("test"); | ||
private final ClientEncryption wrapped; | ||
|
||
RetryingClientEncryption(final ClientEncryption wrapped) { | ||
this.wrapped = wrapped; | ||
} | ||
|
||
<T> T retryOperation(final Supplier<T> supplier) { | ||
RuntimeException lastException = null; | ||
for (int i = 0; i < 3; i++) { | ||
try { | ||
return supplier.get(); | ||
} catch (RuntimeException e) { | ||
lastException = e; | ||
LOGGER.info("Exception in ClientEncryption. Retrying in 1 second...", e); | ||
try { | ||
Thread.sleep(1000); | ||
} catch (InterruptedException ex) { | ||
// ignore | ||
} | ||
} | ||
} | ||
throw lastException; | ||
} | ||
|
||
|
||
@Override | ||
public BsonBinary createDataKey(final String kmsProvider) { | ||
return retryOperation(() -> wrapped.createDataKey(kmsProvider)); | ||
} | ||
|
||
@Override | ||
public BsonBinary createDataKey(final String kmsProvider, final DataKeyOptions dataKeyOptions) { | ||
return retryOperation(() -> wrapped.createDataKey(kmsProvider, dataKeyOptions)); | ||
} | ||
|
||
@Override | ||
public BsonBinary encrypt(final BsonValue value, final EncryptOptions options) { | ||
return retryOperation(() -> wrapped.encrypt(value, options)); | ||
} | ||
|
||
@Override | ||
public BsonDocument encryptExpression(final Bson expression, final EncryptOptions options) { | ||
return retryOperation(() -> wrapped.encryptExpression(expression, options)); | ||
} | ||
|
||
@Override | ||
public BsonValue decrypt(final BsonBinary value) { | ||
return retryOperation(() -> wrapped.decrypt(value)); | ||
} | ||
|
||
@Override | ||
public DeleteResult deleteKey(final BsonBinary id) { | ||
return wrapped.deleteKey(id); | ||
} | ||
|
||
@Nullable | ||
@Override | ||
public BsonDocument getKey(final BsonBinary id) { | ||
return wrapped.getKey(id); | ||
} | ||
|
||
@Override | ||
public FindIterable<BsonDocument> getKeys() { | ||
return wrapped.getKeys(); | ||
} | ||
|
||
@Nullable | ||
@Override | ||
public BsonDocument addKeyAltName(final BsonBinary id, final String keyAltName) { | ||
return wrapped.addKeyAltName(id, keyAltName); | ||
} | ||
|
||
@Nullable | ||
@Override | ||
public BsonDocument removeKeyAltName(final BsonBinary id, final String keyAltName) { | ||
return wrapped.removeKeyAltName(id, keyAltName); | ||
} | ||
|
||
@Nullable | ||
@Override | ||
public BsonDocument getKeyByAltName(final String keyAltName) { | ||
return wrapped.getKeyByAltName(keyAltName); | ||
} | ||
|
||
@Override | ||
public RewrapManyDataKeyResult rewrapManyDataKey(final Bson filter) { | ||
return retryOperation(() -> wrapped.rewrapManyDataKey(filter)); | ||
} | ||
|
||
@Override | ||
public RewrapManyDataKeyResult rewrapManyDataKey(final Bson filter, final RewrapManyDataKeyOptions options) { | ||
return retryOperation(() -> wrapped.rewrapManyDataKey(filter, options)); | ||
} | ||
|
||
@Override | ||
public BsonDocument createEncryptedCollection(final MongoDatabase database, final String collectionName, final CreateCollectionOptions createCollectionOptions, final CreateEncryptedCollectionParams createEncryptedCollectionParams) { | ||
return wrapped.createEncryptedCollection(database, collectionName, createCollectionOptions, createEncryptedCollectionParams); | ||
} | ||
|
||
@Override | ||
public void close() { | ||
wrapped.close(); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Never actually used, so removed it.