Fix SSLConfigurationReloaderTests failure tests (#39408)

This change fixes the tests that expect the reload of a
SSLConfiguration to fail. The tests relied on an incorrect assumption
that the reloader only called reload on for an SSLConfiguration if the
key and trust managers were successfully reloaded, but that is not the
case. This change removes the fail call with a wrapped call to the
original method and captures the exception and counts down a latch to
make these tests consistently tested.

Closes #39260
This commit is contained in:
Jay Modi 2019-02-27 08:17:44 -07:00 committed by jaymode
parent 2ccba18809
commit 995144b197
No known key found for this signature in database
GPG Key ID: D859847567B3493D
1 changed files with 56 additions and 12 deletions

View File

@ -67,6 +67,7 @@ import java.util.Collections;
import java.util.List;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Consumer;
import static org.hamcrest.Matchers.containsString;
@ -330,20 +331,31 @@ public class SSLConfigurationReloaderTests extends ESTestCase {
Environment env = randomBoolean() ? null : TestEnvironment.newEnvironment(settings);
final SSLService sslService = new SSLService(settings, env);
final SSLConfiguration config = sslService.getSSLConfiguration("xpack.security.transport.ssl.");
final AtomicReference<Exception> exceptionRef = new AtomicReference<>();
final CountDownLatch latch = new CountDownLatch(1);
new SSLConfigurationReloader(env, sslService, resourceWatcherService) {
@Override
void reloadSSLContext(SSLConfiguration configuration) {
fail("reload should not be called! [keystore reload exception]");
try {
super.reloadSSLContext(configuration);
} catch (Exception e) {
exceptionRef.set(e);
throw e;
} finally {
latch.countDown();
}
}
};
final SSLContext context = sslService.sslContextHolder(config).sslContext();
// truncate the keystore
try (OutputStream out = Files.newOutputStream(keystorePath, StandardOpenOption.TRUNCATE_EXISTING)) {
try (OutputStream ignore = Files.newOutputStream(keystorePath, StandardOpenOption.TRUNCATE_EXISTING)) {
}
// we intentionally don't wait here as we rely on concurrency to catch a failure
latch.await();
assertNotNull(exceptionRef.get());
assertThat(exceptionRef.get().getMessage(), containsString("failed to initialize a KeyManagerFactory"));
assertThat(sslService.sslContextHolder(config).sslContext(), sameInstance(context));
}
@ -371,20 +383,31 @@ public class SSLConfigurationReloaderTests extends ESTestCase {
Environment env = randomBoolean() ? null : TestEnvironment.newEnvironment(settings);
final SSLService sslService = new SSLService(settings, env);
final SSLConfiguration config = sslService.getSSLConfiguration("xpack.security.transport.ssl.");
final AtomicReference<Exception> exceptionRef = new AtomicReference<>();
final CountDownLatch latch = new CountDownLatch(1);
new SSLConfigurationReloader(env, sslService, resourceWatcherService) {
@Override
void reloadSSLContext(SSLConfiguration configuration) {
fail("reload should not be called! [pem key reload exception]");
try {
super.reloadSSLContext(configuration);
} catch (Exception e) {
exceptionRef.set(e);
throw e;
} finally {
latch.countDown();
}
}
};
final SSLContext context = sslService.sslContextHolder(config).sslContext();
// truncate the file
try (OutputStream os = Files.newOutputStream(keyPath, StandardOpenOption.TRUNCATE_EXISTING)) {
try (OutputStream ignore = Files.newOutputStream(keyPath, StandardOpenOption.TRUNCATE_EXISTING)) {
}
// we intentionally don't wait here as we rely on concurrency to catch a failure
latch.await();
assertNotNull(exceptionRef.get());
assertThat(exceptionRef.get().getMessage(), containsString("Error parsing Private Key"));
assertThat(sslService.sslContextHolder(config).sslContext(), sameInstance(context));
}
@ -406,20 +429,31 @@ public class SSLConfigurationReloaderTests extends ESTestCase {
Environment env = randomBoolean() ? null : TestEnvironment.newEnvironment(settings);
final SSLService sslService = new SSLService(settings, env);
final SSLConfiguration config = sslService.getSSLConfiguration("xpack.security.transport.ssl.");
final AtomicReference<Exception> exceptionRef = new AtomicReference<>();
final CountDownLatch latch = new CountDownLatch(1);
new SSLConfigurationReloader(env, sslService, resourceWatcherService) {
@Override
void reloadSSLContext(SSLConfiguration configuration) {
fail("reload should not be called! [truststore reload exception]");
try {
super.reloadSSLContext(configuration);
} catch (Exception e) {
exceptionRef.set(e);
throw e;
} finally {
latch.countDown();
}
}
};
final SSLContext context = sslService.sslContextHolder(config).sslContext();
// truncate the truststore
try (OutputStream os = Files.newOutputStream(trustStorePath, StandardOpenOption.TRUNCATE_EXISTING)) {
try (OutputStream ignore = Files.newOutputStream(trustStorePath, StandardOpenOption.TRUNCATE_EXISTING)) {
}
// we intentionally don't wait here as we rely on concurrency to catch a failure
latch.await();
assertNotNull(exceptionRef.get());
assertThat(exceptionRef.get().getMessage(), containsString("failed to initialize a TrustManagerFactory"));
assertThat(sslService.sslContextHolder(config).sslContext(), sameInstance(context));
}
@ -438,10 +472,19 @@ public class SSLConfigurationReloaderTests extends ESTestCase {
Environment env = randomBoolean() ? null : TestEnvironment.newEnvironment(settings);
final SSLService sslService = new SSLService(settings, env);
final SSLConfiguration config = sslService.sslConfiguration(settings.getByPrefix("xpack.security.transport.ssl."));
final AtomicReference<Exception> exceptionRef = new AtomicReference<>();
final CountDownLatch latch = new CountDownLatch(1);
new SSLConfigurationReloader(env, sslService, resourceWatcherService) {
@Override
void reloadSSLContext(SSLConfiguration configuration) {
fail("reload should not be called! [pem trust reload exception]");
try {
super.reloadSSLContext(configuration);
} catch (Exception e) {
exceptionRef.set(e);
throw e;
} finally {
latch.countDown();
}
}
};
@ -454,9 +497,10 @@ public class SSLConfigurationReloaderTests extends ESTestCase {
}
atomicMoveIfPossible(updatedCert, clientCertPath);
// we intentionally don't wait here as we rely on concurrency to catch a failure
latch.await();
assertNotNull(exceptionRef.get());
assertThat(exceptionRef.get().getMessage(), containsString("failed to initialize a TrustManagerFactory"));
assertThat(sslService.sslContextHolder(config).sslContext(), sameInstance(context));
}
private void validateSSLConfigurationIsReloaded(Settings settings, Environment env, Consumer<SSLContext> preChecks,