Make random UUIDs reproducible in tests
Today we use a random source of UUIDs for assigning allocation IDs, cluster IDs, etc. Yet, the source of randomness for this is not reproducible in tests. Since allocation IDs end up as keys in hash maps, this means allocation decisions and not reproducible in tests and this leads to non-reproducible test failures. This commit modifies the behavior of random UUIDs so that they are reproducible under tests. The behavior for production code is not changed, we still use a true source of secure randomness but under tests we just use a reproducible source of non-secure randomness. It is important to note that there is a test, UUIDTests#testThreadedRandomUUID that relies on the UUIDs being truly random. Thus, we have to modify the setup for this test to use a true source of randomness. Thus, this is one test that will never be reproducible but it is intentionally so. Relates #18808
This commit is contained in:
parent
43e07c0c88
commit
a25b8ee1bf
|
@ -19,8 +19,6 @@
|
|||
|
||||
package org.elasticsearch.common;
|
||||
|
||||
|
||||
import java.io.IOException;
|
||||
import java.util.Base64;
|
||||
import java.util.Random;
|
||||
|
||||
|
@ -32,7 +30,7 @@ class RandomBasedUUIDGenerator implements UUIDGenerator {
|
|||
*/
|
||||
@Override
|
||||
public String getBase64UUID() {
|
||||
return getBase64UUID(SecureRandomHolder.INSTANCE);
|
||||
return getBase64UUID(Randomness.getSecure());
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -57,4 +55,5 @@ class RandomBasedUUIDGenerator implements UUIDGenerator {
|
|||
randomBytes[8] |= 0x80; /* set the variant (MSB is set)*/
|
||||
return Base64.getUrlEncoder().withoutPadding().encodeToString(randomBytes);
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
@ -23,6 +23,9 @@ import org.elasticsearch.common.settings.Setting;
|
|||
import org.elasticsearch.common.settings.Settings;
|
||||
|
||||
import java.lang.reflect.Method;
|
||||
import java.security.NoSuchAlgorithmException;
|
||||
import java.security.NoSuchProviderException;
|
||||
import java.security.SecureRandom;
|
||||
import java.util.Collections;
|
||||
import java.util.List;
|
||||
import java.util.Random;
|
||||
|
@ -44,6 +47,7 @@ import java.util.concurrent.ThreadLocalRandom;
|
|||
* DiscoveryService#NODE_ID_SEED_SETTING)).
|
||||
*/
|
||||
public final class Randomness {
|
||||
|
||||
private static final Method currentMethod;
|
||||
private static final Method getRandomMethod;
|
||||
|
||||
|
@ -72,7 +76,7 @@ public final class Randomness {
|
|||
* @param setting the setting to access the seed
|
||||
* @return a reproducible source of randomness
|
||||
*/
|
||||
public static Random get(Settings settings, Setting<Long> setting) {
|
||||
public static Random get(final Settings settings, final Setting<Long> setting) {
|
||||
if (setting.exists(settings)) {
|
||||
return new Random(setting.get(settings));
|
||||
} else {
|
||||
|
@ -98,7 +102,7 @@ public final class Randomness {
|
|||
public static Random get() {
|
||||
if (currentMethod != null && getRandomMethod != null) {
|
||||
try {
|
||||
Object randomizedContext = currentMethod.invoke(null);
|
||||
final Object randomizedContext = currentMethod.invoke(null);
|
||||
return (Random) getRandomMethod.invoke(randomizedContext);
|
||||
} catch (ReflectiveOperationException e) {
|
||||
// unexpected, bail
|
||||
|
@ -109,13 +113,42 @@ public final class Randomness {
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Provides a source of randomness that is reproducible when
|
||||
* running under the Elasticsearch test suite, and otherwise
|
||||
* produces a non-reproducible source of secure randomness.
|
||||
* Reproducible sources of randomness are created when the system
|
||||
* property "tests.seed" is set and the security policy allows
|
||||
* reading this system property. Otherwise, non-reproducible
|
||||
* sources of secure randomness are created.
|
||||
*
|
||||
* @return a source of randomness
|
||||
* @throws IllegalStateException if running tests but was not able
|
||||
* to acquire an instance of Random from
|
||||
* RandomizedContext or tests are
|
||||
* running but tests.seed is not set
|
||||
*/
|
||||
public static Random getSecure() {
|
||||
if (currentMethod != null && getRandomMethod != null) {
|
||||
return get();
|
||||
} else {
|
||||
return getSecureRandomWithoutSeed();
|
||||
}
|
||||
}
|
||||
|
||||
@SuppressForbidden(reason = "ThreadLocalRandom is okay when not running tests")
|
||||
private static Random getWithoutSeed() {
|
||||
assert currentMethod == null && getRandomMethod == null : "running under tests but tried to create non-reproducible random";
|
||||
return ThreadLocalRandom.current();
|
||||
}
|
||||
|
||||
public static void shuffle(List<?> list) {
|
||||
private static SecureRandom getSecureRandomWithoutSeed() {
|
||||
assert currentMethod == null && getRandomMethod == null : "running under tests but tried to create non-reproducible random";
|
||||
return SecureRandomHolder.INSTANCE;
|
||||
}
|
||||
|
||||
public static void shuffle(final List<?> list) {
|
||||
Collections.shuffle(list, get());
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
@ -34,6 +34,7 @@ import org.elasticsearch.common.transport.DummyTransportAddress;
|
|||
import org.elasticsearch.gateway.GatewayService;
|
||||
import org.elasticsearch.index.Index;
|
||||
import org.elasticsearch.test.ESTestCase;
|
||||
import org.junit.BeforeClass;
|
||||
|
||||
import java.util.ArrayList;
|
||||
import java.util.Arrays;
|
||||
|
@ -54,11 +55,17 @@ public class ClusterChangedEventTests extends ESTestCase {
|
|||
|
||||
private static final ClusterName TEST_CLUSTER_NAME = new ClusterName("test");
|
||||
private static final String NODE_ID_PREFIX = "node_";
|
||||
private static final String INITIAL_CLUSTER_ID = UUIDs.randomBase64UUID();
|
||||
private static String INITIAL_CLUSTER_ID;
|
||||
private static List<Index> initialIndices;
|
||||
|
||||
@BeforeClass
|
||||
public static void beforeClass() {
|
||||
INITIAL_CLUSTER_ID = UUIDs.randomBase64UUID();
|
||||
// the initial indices which every cluster state test starts out with
|
||||
private static final List<Index> initialIndices = Arrays.asList(new Index("idx1", UUIDs.randomBase64UUID()),
|
||||
initialIndices = Arrays.asList(new Index("idx1", UUIDs.randomBase64UUID()),
|
||||
new Index("idx2", UUIDs.randomBase64UUID()),
|
||||
new Index("idx3", UUIDs.randomBase64UUID()));
|
||||
}
|
||||
|
||||
/**
|
||||
* Test basic properties of the ClusterChangedEvent class:
|
||||
|
|
|
@ -20,7 +20,9 @@ package org.elasticsearch.common;
|
|||
|
||||
import org.elasticsearch.test.ESTestCase;
|
||||
|
||||
import java.security.SecureRandom;
|
||||
import java.util.HashSet;
|
||||
import java.util.Random;
|
||||
import java.util.Set;
|
||||
|
||||
public class UUIDTests extends ESTestCase {
|
||||
|
@ -41,7 +43,18 @@ public class UUIDTests extends ESTestCase {
|
|||
}
|
||||
|
||||
public void testThreadedRandomUUID() {
|
||||
testUUIDThreaded(randomUUIDGen);
|
||||
// we can not use a reproducible source of randomness for this
|
||||
// test, the test explicitly relies on each thread having a
|
||||
// unique source of randomness; thus, we fake what production
|
||||
// code does when using a RandomBasedUUIDGenerator
|
||||
testUUIDThreaded(new RandomBasedUUIDGenerator() {
|
||||
private final SecureRandom sr = SecureRandomHolder.INSTANCE;
|
||||
|
||||
@Override
|
||||
public String getBase64UUID() {
|
||||
return getBase64UUID(sr);
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
Set<String> verifyUUIDSet(int count, UUIDGenerator uuidSource) {
|
||||
|
@ -98,6 +111,6 @@ public class UUIDTests extends ESTestCase {
|
|||
for (UUIDGenRunner runner : runners) {
|
||||
globalSet.addAll(runner.uuidSet);
|
||||
}
|
||||
assertEquals(count*uuids, globalSet.size());
|
||||
assertEquals(count * uuids, globalSet.size());
|
||||
}
|
||||
}
|
||||
|
|
|
@ -32,6 +32,7 @@ import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
|
|||
import org.elasticsearch.cluster.metadata.MetaData;
|
||||
import org.elasticsearch.common.settings.Settings;
|
||||
import org.elasticsearch.test.ESTestCase;
|
||||
import org.junit.BeforeClass;
|
||||
|
||||
import static org.hamcrest.Matchers.containsString;
|
||||
|
||||
|
@ -39,7 +40,14 @@ import static org.hamcrest.Matchers.containsString;
|
|||
* Tests that indexing from an index back into itself fails the request.
|
||||
*/
|
||||
public class ReindexSameIndexTests extends ESTestCase {
|
||||
private static final ClusterState STATE = ClusterState.builder(new ClusterName("test")).metaData(MetaData.builder()
|
||||
|
||||
private static ClusterState STATE;
|
||||
private static final IndexNameExpressionResolver INDEX_NAME_EXPRESSION_RESOLVER = new IndexNameExpressionResolver(Settings.EMPTY);
|
||||
private static AutoCreateIndex AUTO_CREATE_INDEX = new AutoCreateIndex(Settings.EMPTY, INDEX_NAME_EXPRESSION_RESOLVER);
|
||||
|
||||
@BeforeClass
|
||||
public static void beforeClass() {
|
||||
STATE = ClusterState.builder(new ClusterName("test")).metaData(MetaData.builder()
|
||||
.put(index("target", "target_alias", "target_multi"), true)
|
||||
.put(index("target2", "target_multi"), true)
|
||||
.put(index("foo"), true)
|
||||
|
@ -47,8 +55,7 @@ public class ReindexSameIndexTests extends ESTestCase {
|
|||
.put(index("baz"), true)
|
||||
.put(index("source", "source_multi"), true)
|
||||
.put(index("source2", "source_multi"), true)).build();
|
||||
private static final IndexNameExpressionResolver INDEX_NAME_EXPRESSION_RESOLVER = new IndexNameExpressionResolver(Settings.EMPTY);
|
||||
private static final AutoCreateIndex AUTO_CREATE_INDEX = new AutoCreateIndex(Settings.EMPTY, INDEX_NAME_EXPRESSION_RESOLVER);
|
||||
}
|
||||
|
||||
public void testObviousCases() throws Exception {
|
||||
fails("target", "target");
|
||||
|
|
Loading…
Reference in New Issue