changes from review

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
This commit is contained in:
Lachlan Roberts 2020-07-14 16:10:07 +10:00
parent b03643961d
commit a83844df32
5 changed files with 38 additions and 61 deletions

View File

@ -1,12 +1,11 @@
<?xml version="1.0"?><!DOCTYPE Configure PUBLIC "-//Jetty//Configure//EN" "http://www.eclipse.org/jetty/configure_9_3.dtd">
<Configure id="Server" class="org.eclipse.jetty.server.Server">
<Call name="addBean">
<Arg>
<New id="sslKeyStoreScanner" class="org.eclipse.jetty.util.ssl.SslKeyStoreScanner">
<New id="sslKeyStoreScanner" class="org.eclipse.jetty.util.ssl.KeyStoreScanner">
<Arg><Ref refid="sslContextFactory"/></Arg>
<Set name="scanInterval"><Property name="jetty.ssl.reload.scanInterval" default="1"/></Set>
<Set name="scanInterval"><Property name="jetty.sslContext.reload.scanInterval" default="1"/></Set>
</New>
</Arg>
</Call>

View File

@ -10,12 +10,9 @@ ssl
[depend]
ssl
[lib]
lib/jetty-ssl-reload-${jetty.version}.jar
[xml]
etc/jetty-ssl-reload.xml
etc/jetty-ssl-context-reload.xml
[ini-template]
# Monitored directory scan period (seconds)
# jetty.ssl.reload.scanInterval=1
# jetty.sslContext.reload.scanInterval=1

View File

@ -19,74 +19,56 @@
package org.eclipse.jetty.util.ssl;
import java.io.File;
import java.net.URI;
import java.util.ArrayList;
import java.util.List;
import java.io.IOException;
import java.util.Collections;
import org.eclipse.jetty.util.Scanner;
import org.eclipse.jetty.util.annotation.ManagedAttribute;
import org.eclipse.jetty.util.annotation.ManagedOperation;
import org.eclipse.jetty.util.component.AbstractLifeCycle;
import org.eclipse.jetty.util.component.ContainerLifeCycle;
import org.eclipse.jetty.util.log.Log;
import org.eclipse.jetty.util.log.Logger;
public class SslKeyStoreScanner extends AbstractLifeCycle implements Scanner.DiscreteListener
public class KeyStoreScanner extends ContainerLifeCycle implements Scanner.DiscreteListener
{
private static final Logger LOG = Log.getLogger(SslKeyStoreScanner.class);
private static final Logger LOG = Log.getLogger(KeyStoreScanner.class);
private final SslContextFactory sslContextFactory;
private final File keystoreFile;
private final List<File> files = new ArrayList<>();
private Scanner _scanner;
private int _scanInterval = 1;
private final Scanner _scanner;
public SslKeyStoreScanner(SslContextFactory sslContextFactory)
public KeyStoreScanner(SslContextFactory sslContextFactory)
{
this.sslContextFactory = sslContextFactory;
this.keystoreFile = new File(URI.create(sslContextFactory.getKeyStorePath())); // getKeyStorePath is giving url instead of path?
if (!keystoreFile.exists())
throw new IllegalArgumentException("keystore file does not exist");
if (keystoreFile.isDirectory())
throw new IllegalArgumentException("expected keystore file not directory");
try
{
keystoreFile = sslContextFactory.getKeyStoreResource().getFile();
if (keystoreFile == null || !keystoreFile.exists())
throw new IllegalArgumentException("keystore file does not exist");
if (keystoreFile.isDirectory())
throw new IllegalArgumentException("expected keystore file not directory");
}
catch (IOException e)
{
throw new IllegalArgumentException("could not obtain keystore file", e);
}
File parentFile = keystoreFile.getParentFile();
if (!parentFile.exists() || !parentFile.isDirectory())
throw new IllegalArgumentException("error obtaining keystore dir");
files.add(parentFile);
if (LOG.isDebugEnabled())
LOG.debug("created {}", this);
}
@Override
protected void doStart() throws Exception
{
if (LOG.isDebugEnabled())
LOG.debug(this.getClass().getSimpleName() + ".doStart()");
_scanner = new Scanner();
_scanner.setScanDirs(files);
_scanner.setScanInterval(_scanInterval);
_scanner.setScanDirs(Collections.singletonList(parentFile));
_scanner.setScanInterval(1);
_scanner.setReportDirs(false);
_scanner.setReportExistingFilesOnStartup(false);
_scanner.setScanDepth(1);
_scanner.addListener(this);
_scanner.start();
addBean(_scanner);
}
@Override
protected void doStop() throws Exception
{
if (_scanner != null)
{
_scanner.stop();
_scanner.removeListener(this);
_scanner = null;
}
}
@Override
public void fileAdded(String filename) throws Exception
public void fileAdded(String filename)
{
if (LOG.isDebugEnabled())
LOG.debug("added {}", filename);
@ -96,7 +78,7 @@ public class SslKeyStoreScanner extends AbstractLifeCycle implements Scanner.Dis
}
@Override
public void fileChanged(String filename) throws Exception
public void fileChanged(String filename)
{
if (LOG.isDebugEnabled())
LOG.debug("changed {}", filename);
@ -106,18 +88,17 @@ public class SslKeyStoreScanner extends AbstractLifeCycle implements Scanner.Dis
}
@Override
public void fileRemoved(String filename) throws Exception
public void fileRemoved(String filename)
{
if (LOG.isDebugEnabled())
LOG.debug("removed {}", filename);
// TODO: do we want to do this?
if (keystoreFile.toPath().toString().equals(filename))
reload();
}
@ManagedOperation(value = "Reload the SSL Keystore", impact = "ACTION")
public void reload() throws Exception
public void reload()
{
if (LOG.isDebugEnabled())
LOG.debug("reloading keystore file {}", keystoreFile);
@ -135,11 +116,11 @@ public class SslKeyStoreScanner extends AbstractLifeCycle implements Scanner.Dis
@ManagedAttribute("scanning interval to detect changes which need reloaded")
public int getScanInterval()
{
return _scanInterval;
return _scanner.getScanInterval();
}
public void setScanInterval(int scanInterval)
{
_scanInterval = scanInterval;
_scanner.setScanInterval(scanInterval);
}
}

View File

@ -43,8 +43,8 @@ import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
import org.eclipse.jetty.toolchain.test.jupiter.WorkDir;
import org.eclipse.jetty.toolchain.test.jupiter.WorkDirExtension;
import org.eclipse.jetty.util.log.StacklessLogging;
import org.eclipse.jetty.util.ssl.KeyStoreScanner;
import org.eclipse.jetty.util.ssl.SslContextFactory;
import org.eclipse.jetty.util.ssl.SslKeyStoreScanner;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
@ -55,7 +55,7 @@ import static org.hamcrest.Matchers.is;
import static org.junit.jupiter.api.Assertions.assertThrows;
@ExtendWith(WorkDirExtension.class)
public class KeystoreReloadTest
public class KeyStoreScannerTest
{
private static final int scanInterval = 1;
public WorkDir testdir;
@ -99,7 +99,7 @@ public class KeystoreReloadTest
server.addConnector(connector);
// Configure Keystore Reload.
SslKeyStoreScanner keystoreScanner = new SslKeyStoreScanner(sslContextFactory);
KeyStoreScanner keystoreScanner = new KeyStoreScanner(sslContextFactory);
keystoreScanner.setScanInterval(scanInterval);
server.addBean(keystoreScanner);
@ -140,7 +140,7 @@ public class KeystoreReloadTest
assertThat(getExpiryYear(cert1), is(2015));
// Switch to use badKeystore which has the incorrect passwords.
try (StacklessLogging ignored = new StacklessLogging(SslKeyStoreScanner.class))
try (StacklessLogging ignored = new StacklessLogging(KeyStoreScanner.class))
{
useKeystore("badKeystore");
Thread.sleep(Duration.ofSeconds(scanInterval * 2).toMillis());
@ -160,7 +160,7 @@ public class KeystoreReloadTest
assertThat(getExpiryYear(cert1), is(2015));
// Delete the keystore.
try (StacklessLogging ignored = new StacklessLogging(SslKeyStoreScanner.class))
try (StacklessLogging ignored = new StacklessLogging(KeyStoreScanner.class))
{
useKeystore(null);
Thread.sleep(Duration.ofSeconds(scanInterval * 2).toMillis());

View File

@ -2,4 +2,4 @@ org.eclipse.jetty.util.log.class=org.eclipse.jetty.util.log.StdErrLog
#org.eclipse.jetty.util.log.class=org.eclipse.jetty.util.log.Slf4jLog
#org.eclipse.jetty.LEVEL=DEBUG
#org.eclipse.jetty.websocket.LEVEL=DEBUG
#org.eclipse.jetty.util.ssl.SslKeyStoreScanner.LEVEL=DEBUG
#org.eclipse.jetty.util.ssl.KeyStoreScanner.LEVEL=DEBUG