From 0dcd271fdcd616e75e6baf16f911b5febd9c081c Mon Sep 17 00:00:00 2001 From: jaymode Date: Tue, 26 Feb 2019 10:41:23 -0700 Subject: [PATCH] Fix SSLConfigurationReloaderTests failure tests This change fixes the tests that expect the reload of a SSLConfiguration to fail after changes made in #30509. The tests relied on the behavior that an SSLConfiguration only had reload called on it after the key and trust managers had been created, but that is no longer 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 rather than relying on concurrency to catch a failure. Closes #39260 --- .../ssl/SSLConfigurationReloaderTests.java | 68 +++++++++++++++---- 1 file changed, 56 insertions(+), 12 deletions(-) diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SSLConfigurationReloaderTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SSLConfigurationReloaderTests.java index f8adc85cdf8d4..83b1d80f563a5 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SSLConfigurationReloaderTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SSLConfigurationReloaderTests.java @@ -67,6 +67,7 @@ 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 void testReloadingKeyStoreException() throws Exception { 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 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 void testReloadingPEMKeyConfigException() throws Exception { 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 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 void testTrustStoreReloadException() throws Exception { 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 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 void testPEMTrustReloadException() throws Exception { 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 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 @@ void reloadSSLContext(SSLConfiguration configuration) { } 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 preChecks,