Skip to content

Commit 75e51f8

Browse files
committed
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 2f05af1 commit 75e51f8

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
@@ -48,6 +48,7 @@
4848
import java.util.Collections;
4949
import java.util.List;
5050
import java.util.concurrent.CountDownLatch;
51+
import java.util.concurrent.atomic.AtomicReference;
5152
import java.util.function.Consumer;
5253

5354
import static org.hamcrest.Matchers.containsString;
@@ -309,20 +310,31 @@ public void testReloadingKeyStoreException() throws Exception {
309310
Environment env = randomBoolean() ? null : TestEnvironment.newEnvironment(settings);
310311
final SSLService sslService = new SSLService(settings, env);
311312
final SSLConfiguration config = sslService.getSSLConfiguration("xpack.ssl");
313+
final AtomicReference<Exception> exceptionRef = new AtomicReference<>();
314+
final CountDownLatch latch = new CountDownLatch(1);
312315
new SSLConfigurationReloader(env, sslService, resourceWatcherService) {
313316
@Override
314317
void reloadSSLContext(SSLConfiguration configuration) {
315-
fail("reload should not be called! [keystore reload exception]");
318+
try {
319+
super.reloadSSLContext(configuration);
320+
} catch (Exception e) {
321+
exceptionRef.set(e);
322+
throw e;
323+
} finally {
324+
latch.countDown();
325+
}
316326
}
317327
};
318328

319329
final SSLContext context = sslService.sslContextHolder(config).sslContext();
320330

321331
// truncate the keystore
322-
try (OutputStream out = Files.newOutputStream(keystorePath, StandardOpenOption.TRUNCATE_EXISTING)) {
332+
try (OutputStream ignore = Files.newOutputStream(keystorePath, StandardOpenOption.TRUNCATE_EXISTING)) {
323333
}
324334

325-
// we intentionally don't wait here as we rely on concurrency to catch a failure
335+
latch.await();
336+
assertNotNull(exceptionRef.get());
337+
assertThat(exceptionRef.get().getMessage(), containsString("failed to initialize a KeyManagerFactory"));
326338
assertThat(sslService.sslContextHolder(config).sslContext(), sameInstance(context));
327339
}
328340

@@ -350,20 +362,31 @@ public void testReloadingPEMKeyConfigException() throws Exception {
350362
Environment env = randomBoolean() ? null : TestEnvironment.newEnvironment(settings);
351363
final SSLService sslService = new SSLService(settings, env);
352364
final SSLConfiguration config = sslService.getSSLConfiguration("xpack.ssl");
365+
final AtomicReference<Exception> exceptionRef = new AtomicReference<>();
366+
final CountDownLatch latch = new CountDownLatch(1);
353367
new SSLConfigurationReloader(env, sslService, resourceWatcherService) {
354368
@Override
355369
void reloadSSLContext(SSLConfiguration configuration) {
356-
fail("reload should not be called! [pem key reload exception]");
370+
try {
371+
super.reloadSSLContext(configuration);
372+
} catch (Exception e) {
373+
exceptionRef.set(e);
374+
throw e;
375+
} finally {
376+
latch.countDown();
377+
}
357378
}
358379
};
359380

360381
final SSLContext context = sslService.sslContextHolder(config).sslContext();
361382

362383
// truncate the file
363-
try (OutputStream os = Files.newOutputStream(keyPath, StandardOpenOption.TRUNCATE_EXISTING)) {
384+
try (OutputStream ignore = Files.newOutputStream(keyPath, StandardOpenOption.TRUNCATE_EXISTING)) {
364385
}
365386

366-
// we intentionally don't wait here as we rely on concurrency to catch a failure
387+
latch.await();
388+
assertNotNull(exceptionRef.get());
389+
assertThat(exceptionRef.get().getMessage(), containsString("Error parsing Private Key"));
367390
assertThat(sslService.sslContextHolder(config).sslContext(), sameInstance(context));
368391
}
369392

@@ -385,20 +408,31 @@ public void testTrustStoreReloadException() throws Exception {
385408
Environment env = randomBoolean() ? null : TestEnvironment.newEnvironment(settings);
386409
final SSLService sslService = new SSLService(settings, env);
387410
final SSLConfiguration config = sslService.getSSLConfiguration("xpack.ssl");
411+
final AtomicReference<Exception> exceptionRef = new AtomicReference<>();
412+
final CountDownLatch latch = new CountDownLatch(1);
388413
new SSLConfigurationReloader(env, sslService, resourceWatcherService) {
389414
@Override
390415
void reloadSSLContext(SSLConfiguration configuration) {
391-
fail("reload should not be called! [truststore reload exception]");
416+
try {
417+
super.reloadSSLContext(configuration);
418+
} catch (Exception e) {
419+
exceptionRef.set(e);
420+
throw e;
421+
} finally {
422+
latch.countDown();
423+
}
392424
}
393425
};
394426

395427
final SSLContext context = sslService.sslContextHolder(config).sslContext();
396428

397429
// truncate the truststore
398-
try (OutputStream os = Files.newOutputStream(trustStorePath, StandardOpenOption.TRUNCATE_EXISTING)) {
430+
try (OutputStream ignore = Files.newOutputStream(trustStorePath, StandardOpenOption.TRUNCATE_EXISTING)) {
399431
}
400432

401-
// we intentionally don't wait here as we rely on concurrency to catch a failure
433+
latch.await();
434+
assertNotNull(exceptionRef.get());
435+
assertThat(exceptionRef.get().getMessage(), containsString("failed to initialize a TrustManagerFactory"));
402436
assertThat(sslService.sslContextHolder(config).sslContext(), sameInstance(context));
403437
}
404438

@@ -417,10 +451,19 @@ public void testPEMTrustReloadException() throws Exception {
417451
Environment env = randomBoolean() ? null : TestEnvironment.newEnvironment(settings);
418452
final SSLService sslService = new SSLService(settings, env);
419453
final SSLConfiguration config = sslService.getSSLConfiguration("xpack.ssl");
454+
final AtomicReference<Exception> exceptionRef = new AtomicReference<>();
455+
final CountDownLatch latch = new CountDownLatch(1);
420456
new SSLConfigurationReloader(env, sslService, resourceWatcherService) {
421457
@Override
422458
void reloadSSLContext(SSLConfiguration configuration) {
423-
fail("reload should not be called! [pem trust reload exception]");
459+
try {
460+
super.reloadSSLContext(configuration);
461+
} catch (Exception e) {
462+
exceptionRef.set(e);
463+
throw e;
464+
} finally {
465+
latch.countDown();
466+
}
424467
}
425468
};
426469

@@ -433,9 +476,10 @@ void reloadSSLContext(SSLConfiguration configuration) {
433476
}
434477
atomicMoveIfPossible(updatedCert, clientCertPath);
435478

436-
// we intentionally don't wait here as we rely on concurrency to catch a failure
479+
latch.await();
480+
assertNotNull(exceptionRef.get());
481+
assertThat(exceptionRef.get().getMessage(), containsString("failed to initialize a TrustManagerFactory"));
437482
assertThat(sslService.sslContextHolder(config).sslContext(), sameInstance(context));
438-
439483
}
440484

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

0 commit comments

Comments
 (0)