Skip to content

Commit 3b9b4ce

Browse files
authored
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
1 parent 68c57ad commit 3b9b4ce

File tree

1 file changed

+56
-12
lines changed

1 file changed

+56
-12
lines changed

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SSLConfigurationReloaderTests.java

Lines changed: 56 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@
6767
import java.util.List;
6868
import java.util.concurrent.CountDownLatch;
6969
import java.util.concurrent.TimeUnit;
70+
import java.util.concurrent.atomic.AtomicReference;
7071
import java.util.function.Consumer;
7172

7273
import static org.hamcrest.Matchers.containsString;
@@ -330,20 +331,31 @@ public void testReloadingKeyStoreException() throws Exception {
330331
Environment env = randomBoolean() ? null : TestEnvironment.newEnvironment(settings);
331332
final SSLService sslService = new SSLService(settings, env);
332333
final SSLConfiguration config = sslService.getSSLConfiguration("xpack.security.transport.ssl.");
334+
final AtomicReference<Exception> exceptionRef = new AtomicReference<>();
335+
final CountDownLatch latch = new CountDownLatch(1);
333336
new SSLConfigurationReloader(env, sslService, resourceWatcherService) {
334337
@Override
335338
void reloadSSLContext(SSLConfiguration configuration) {
336-
fail("reload should not be called! [keystore reload exception]");
339+
try {
340+
super.reloadSSLContext(configuration);
341+
} catch (Exception e) {
342+
exceptionRef.set(e);
343+
throw e;
344+
} finally {
345+
latch.countDown();
346+
}
337347
}
338348
};
339349

340350
final SSLContext context = sslService.sslContextHolder(config).sslContext();
341351

342352
// truncate the keystore
343-
try (OutputStream out = Files.newOutputStream(keystorePath, StandardOpenOption.TRUNCATE_EXISTING)) {
353+
try (OutputStream ignore = Files.newOutputStream(keystorePath, StandardOpenOption.TRUNCATE_EXISTING)) {
344354
}
345355

346-
// we intentionally don't wait here as we rely on concurrency to catch a failure
356+
latch.await();
357+
assertNotNull(exceptionRef.get());
358+
assertThat(exceptionRef.get().getMessage(), containsString("failed to initialize a KeyManagerFactory"));
347359
assertThat(sslService.sslContextHolder(config).sslContext(), sameInstance(context));
348360
}
349361

@@ -371,20 +383,31 @@ public void testReloadingPEMKeyConfigException() throws Exception {
371383
Environment env = randomBoolean() ? null : TestEnvironment.newEnvironment(settings);
372384
final SSLService sslService = new SSLService(settings, env);
373385
final SSLConfiguration config = sslService.getSSLConfiguration("xpack.security.transport.ssl.");
386+
final AtomicReference<Exception> exceptionRef = new AtomicReference<>();
387+
final CountDownLatch latch = new CountDownLatch(1);
374388
new SSLConfigurationReloader(env, sslService, resourceWatcherService) {
375389
@Override
376390
void reloadSSLContext(SSLConfiguration configuration) {
377-
fail("reload should not be called! [pem key reload exception]");
391+
try {
392+
super.reloadSSLContext(configuration);
393+
} catch (Exception e) {
394+
exceptionRef.set(e);
395+
throw e;
396+
} finally {
397+
latch.countDown();
398+
}
378399
}
379400
};
380401

381402
final SSLContext context = sslService.sslContextHolder(config).sslContext();
382403

383404
// truncate the file
384-
try (OutputStream os = Files.newOutputStream(keyPath, StandardOpenOption.TRUNCATE_EXISTING)) {
405+
try (OutputStream ignore = Files.newOutputStream(keyPath, StandardOpenOption.TRUNCATE_EXISTING)) {
385406
}
386407

387-
// we intentionally don't wait here as we rely on concurrency to catch a failure
408+
latch.await();
409+
assertNotNull(exceptionRef.get());
410+
assertThat(exceptionRef.get().getMessage(), containsString("Error parsing Private Key"));
388411
assertThat(sslService.sslContextHolder(config).sslContext(), sameInstance(context));
389412
}
390413

@@ -406,20 +429,31 @@ public void testTrustStoreReloadException() throws Exception {
406429
Environment env = randomBoolean() ? null : TestEnvironment.newEnvironment(settings);
407430
final SSLService sslService = new SSLService(settings, env);
408431
final SSLConfiguration config = sslService.getSSLConfiguration("xpack.security.transport.ssl.");
432+
final AtomicReference<Exception> exceptionRef = new AtomicReference<>();
433+
final CountDownLatch latch = new CountDownLatch(1);
409434
new SSLConfigurationReloader(env, sslService, resourceWatcherService) {
410435
@Override
411436
void reloadSSLContext(SSLConfiguration configuration) {
412-
fail("reload should not be called! [truststore reload exception]");
437+
try {
438+
super.reloadSSLContext(configuration);
439+
} catch (Exception e) {
440+
exceptionRef.set(e);
441+
throw e;
442+
} finally {
443+
latch.countDown();
444+
}
413445
}
414446
};
415447

416448
final SSLContext context = sslService.sslContextHolder(config).sslContext();
417449

418450
// truncate the truststore
419-
try (OutputStream os = Files.newOutputStream(trustStorePath, StandardOpenOption.TRUNCATE_EXISTING)) {
451+
try (OutputStream ignore = Files.newOutputStream(trustStorePath, StandardOpenOption.TRUNCATE_EXISTING)) {
420452
}
421453

422-
// we intentionally don't wait here as we rely on concurrency to catch a failure
454+
latch.await();
455+
assertNotNull(exceptionRef.get());
456+
assertThat(exceptionRef.get().getMessage(), containsString("failed to initialize a TrustManagerFactory"));
423457
assertThat(sslService.sslContextHolder(config).sslContext(), sameInstance(context));
424458
}
425459

@@ -438,10 +472,19 @@ public void testPEMTrustReloadException() throws Exception {
438472
Environment env = randomBoolean() ? null : TestEnvironment.newEnvironment(settings);
439473
final SSLService sslService = new SSLService(settings, env);
440474
final SSLConfiguration config = sslService.sslConfiguration(settings.getByPrefix("xpack.security.transport.ssl."));
475+
final AtomicReference<Exception> exceptionRef = new AtomicReference<>();
476+
final CountDownLatch latch = new CountDownLatch(1);
441477
new SSLConfigurationReloader(env, sslService, resourceWatcherService) {
442478
@Override
443479
void reloadSSLContext(SSLConfiguration configuration) {
444-
fail("reload should not be called! [pem trust reload exception]");
480+
try {
481+
super.reloadSSLContext(configuration);
482+
} catch (Exception e) {
483+
exceptionRef.set(e);
484+
throw e;
485+
} finally {
486+
latch.countDown();
487+
}
445488
}
446489
};
447490

@@ -454,9 +497,10 @@ void reloadSSLContext(SSLConfiguration configuration) {
454497
}
455498
atomicMoveIfPossible(updatedCert, clientCertPath);
456499

457-
// we intentionally don't wait here as we rely on concurrency to catch a failure
500+
latch.await();
501+
assertNotNull(exceptionRef.get());
502+
assertThat(exceptionRef.get().getMessage(), containsString("failed to initialize a TrustManagerFactory"));
458503
assertThat(sslService.sslContextHolder(config).sslContext(), sameInstance(context));
459-
460504
}
461505

462506
private void validateSSLConfigurationIsReloaded(Settings settings, Environment env, Consumer<SSLContext> preChecks,

0 commit comments

Comments
 (0)