Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refactor SSL ReloadWatcher to use java.time and java.nio #80

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Refactor SSL ReloadWatcher to use java.time and java.nio #80

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 42 additions & 24 deletions common/src/main/java/com/zegelin/cassandra/exporter/netty/ssl/ReloadWatcher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,61 +5,66 @@
import org.slf4j.LoggerFactory;

import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.attribute.FileTime;
import java.time.Duration;
import java.time.Instant;
import java.util.Collection;
import java.util.Objects;
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;
import java.util.stream.Stream;

// TODO: Switch to using java.time.* classes (Instant, Duration, etc) and java.nio.Path
public class ReloadWatcher {
private static final Logger logger = LoggerFactory.getLogger(ReloadWatcher.class);

private static final long RELOAD_MARGIN_MILLIS = 1000;
private final long intervalInMs;
private final Collection<File> files;
private static final Duration RELOAD_MARGIN = Duration.ofMillis(1000);
private final Duration reloadInterval;
private final Collection<Path> files;

private long nextReloadAt;
private long reloadedAt;
private Instant nextReloadAt;
private Instant reloadedAt;

public ReloadWatcher(final HttpServerOptions httpServerOptions) {
intervalInMs = TimeUnit.SECONDS.toMillis(httpServerOptions.sslReloadIntervalInSeconds);
reloadInterval = Duration.ofSeconds(httpServerOptions.sslReloadIntervalInSeconds);
files = Stream.of(httpServerOptions.sslServerKeyFile,
httpServerOptions.sslServerKeyPasswordFile,
httpServerOptions.sslServerCertificateFile,
httpServerOptions.sslTrustedCertificateFile)
.filter(Objects::nonNull)
.map(File::toPath)
.collect(Collectors.toSet());

logger.info("Watching {} for changes every {} seconds.", this.files, httpServerOptions.sslReloadIntervalInSeconds);
reset(System.currentTimeMillis());
reset(Instant.now());
}

private void reset(final long now) {
// Create a 1 second margin to compensate for poor resolution of File.lastModified()
reloadedAt = now - RELOAD_MARGIN_MILLIS;
private void reset(final Instant now) {
// Create a 1 second margin to compensate for poor resolution of Files.getLastModifiedTime()
reloadedAt = now.minus(RELOAD_MARGIN);

nextReloadAt = now + intervalInMs;
nextReloadAt = now.plus(reloadInterval);
logger.debug("Next reload at {}.", nextReloadAt);
}

public synchronized void forceReload() {
if (!enabled()) {
if (disabled()) {
return;
}

logger.info("Forced reload of exporter certificates on next scrape.");

reloadedAt = 0L;
nextReloadAt = 0L;
reloadedAt = Instant.EPOCH;
nextReloadAt = Instant.EPOCH;
}

boolean needReload() {
if (!enabled()) {
if (disabled()) {
return false;
}

final long now = System.currentTimeMillis();
final Instant now = Instant.now();

if (timeToPoll(now)) {
return reallyNeedReload(now);
Expand All @@ -68,15 +73,15 @@ boolean needReload() {
return false;
}

private boolean enabled() {
return intervalInMs > 0;
private boolean disabled() {
return reloadInterval.isZero();
}

private boolean timeToPoll(final long now) {
return now > nextReloadAt;
private boolean timeToPoll(final Instant now) {
return now.isAfter(nextReloadAt);
}

private synchronized boolean reallyNeedReload(final long now) {
private synchronized boolean reallyNeedReload(final Instant now) {
if (timeToPoll(now)) {
try {
return anyFileModified();
Expand All @@ -88,6 +93,19 @@ private synchronized boolean reallyNeedReload(final long now) {
}

private boolean anyFileModified() {
return files.stream().anyMatch(f -> f.lastModified() > reloadedAt);
return files.stream()
.map(this::getLastModifiedTimeSafe)
.map(FileTime::toMillis)
.map(Instant::ofEpochMilli)
.anyMatch(lastModifiedAt -> lastModifiedAt.isAfter(reloadedAt));
}

private FileTime getLastModifiedTimeSafe(final Path path) {
try {
return Files.getLastModifiedTime(path);
} catch (IOException e) {
logger.warn("Unable to get modification time of file {} - forcing reload.", path, e);
return FileTime.from(Instant.MAX);
}
}
}
2 changes: 1 addition & 1 deletion common/src/main/java/com/zegelin/cassandra/exporter/netty/ssl/SslContextFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ private String getKeyPassword() {
}

private SslContextBuilder getSelfSignedContextBuilder() {
logger.warn("Running exporter in SSL mode with insecure self-signed certificate");
logger.warn("Running exporter in SSL mode with insecure self-signed certificate.");

try {
final SelfSignedCertificate ssc = new SelfSignedCertificate();
Expand Down
44 changes: 26 additions & 18 deletions common/src/test/java/com/zegelin/cassandra/exporter/netty/ssl/TestReloadWatcher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,14 @@
import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.attribute.FileTime;
import java.time.Instant;

import static org.assertj.core.api.Assertions.assertThat;

public class TestReloadWatcher {
public static final long INITIAL_FILE_AGE_MILLIS = 5000;
public static final FileTime INITIAL_FILE_MODIFICATION_TIME = FileTime.from(Instant.now().minusSeconds(5));
public static final long SLEEP_MILLIS = 1001;

private HttpServerOptions options;
Expand All @@ -26,16 +29,16 @@ public void before() throws IOException {
options.sslServerCertificateFile = givenTemporaryFile("server-cert");
options.sslTrustedCertificateFile = givenTemporaryFile("trusted-cert");

options.sslServerKeyFile.setLastModified(System.currentTimeMillis() - INITIAL_FILE_AGE_MILLIS);
options.sslServerCertificateFile.setLastModified(System.currentTimeMillis() - INITIAL_FILE_AGE_MILLIS);
options.sslTrustedCertificateFile.setLastModified(System.currentTimeMillis() - INITIAL_FILE_AGE_MILLIS);
Files.setLastModifiedTime(options.sslServerKeyFile.toPath(), INITIAL_FILE_MODIFICATION_TIME);
Files.setLastModifiedTime(options.sslServerCertificateFile.toPath(), INITIAL_FILE_MODIFICATION_TIME);
Files.setLastModifiedTime(options.sslTrustedCertificateFile.toPath(), INITIAL_FILE_MODIFICATION_TIME);

watcher = new ReloadWatcher(options);
}

@Test
public void testNoImmediateReload() {
options.sslServerKeyFile.setLastModified(System.currentTimeMillis());
public void testNoImmediateReload() throws IOException {
touch(options.sslServerKeyFile);

assertThat(watcher.needReload()).isFalse();
}
Expand All @@ -48,11 +51,11 @@ public void testNoReloadWhenFilesAreUntouched() throws InterruptedException {
}

@Test
public void testReloadOnceWhenFilesAreTouched() throws InterruptedException {
public void testReloadOnceWhenFilesAreTouched() throws Exception {
Thread.sleep(SLEEP_MILLIS);

options.sslServerKeyFile.setLastModified(System.currentTimeMillis());
options.sslServerCertificateFile.setLastModified(System.currentTimeMillis());
touch(options.sslServerKeyFile);
touch(options.sslServerCertificateFile);

Thread.sleep(SLEEP_MILLIS);

Expand All @@ -63,15 +66,16 @@ public void testReloadOnceWhenFilesAreTouched() throws InterruptedException {
assertThat(watcher.needReload()).isFalse();
}

// Verify that we reload certificates on next pass again in case files are modified
// Verify that we compensate for poor time resolution of Files.getLastModifiedTime().
// In other words, make sure that we reload certificates on next pass again in case files are modified
// just as we check for reload.
@Test
public void testReloadAgainWhenFilesAreTouchedJustAfterReload() throws InterruptedException {
public void testReloadAgainWhenFilesAreTouchedJustAfterReload() throws Exception {
Thread.sleep(SLEEP_MILLIS);

options.sslServerKeyFile.setLastModified(System.currentTimeMillis());
touch(options.sslServerKeyFile);
assertThat(watcher.needReload()).isTrue();
options.sslServerCertificateFile.setLastModified(System.currentTimeMillis());
touch(options.sslServerCertificateFile);

Thread.sleep(SLEEP_MILLIS);

Expand All @@ -88,20 +92,24 @@ public void testReloadWhenForced() throws InterruptedException {
}

@Test
public void testNoReloadWhenDisabled() throws InterruptedException {
public void testNoReloadWhenDisabled() throws Exception {
options.sslReloadIntervalInSeconds = 0;
watcher = new ReloadWatcher(options);

Thread.sleep(SLEEP_MILLIS);
options.sslServerKeyFile.setLastModified(System.currentTimeMillis());
touch(options.sslServerKeyFile);

assertThat(watcher.needReload()).isFalse();
}

private File givenTemporaryFile(String filename) throws IOException {
File file = File.createTempFile(filename, "tmp");
Files.write(file.toPath(), "dummy".getBytes());
Path file = Files.createTempFile(filename, "tmp");
Files.write(file, "dummy".getBytes());

return file;
return file.toFile();
}

private void touch(File file) throws IOException {
Files.setLastModifiedTime(file.toPath(), FileTime.from(Instant.now()));
}
}
20 changes: 14 additions & 6 deletions common/src/test/java/com/zegelin/cassandra/exporter/netty/ssl/TestSslSupport.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,12 @@
import org.testng.annotations.Test;

import java.io.File;
import java.io.IOException;
import java.net.InetSocketAddress;
import java.net.URL;
import java.nio.file.Files;
import java.nio.file.attribute.FileTime;
import java.time.Instant;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.any;
Expand Down Expand Up @@ -105,7 +109,7 @@ public void testSslOptionalWithValidation() {


@Test
public void testSslReload() throws InterruptedException {
public void testSslReload() throws Exception {
serverOptions.sslMode = SslMode.ENABLE;
serverOptions.sslServerKeyFile = givenResource("cert/key.pem");
serverOptions.sslServerCertificateFile = givenResource("cert/cert.pem");
Expand All @@ -116,15 +120,15 @@ public void testSslReload() throws InterruptedException {
SslContext unexpectedContext = sslSupport.getSslContext();

Thread.sleep(1001);
serverOptions.sslServerKeyFile.setLastModified(System.currentTimeMillis());
touch(serverOptions.sslServerKeyFile);

sslSupport.maybeAddHandler(socketChannel);

assertThat(sslSupport.getSslContext()).isNotSameAs(unexpectedContext);
}

@Test
public void testSslNoReloadOnFailure() throws InterruptedException {
public void testSslNoReloadOnFailure() throws Exception {
serverOptions.sslMode = SslMode.ENABLE;
serverOptions.sslServerKeyFile = givenResource("cert/key.pem");
serverOptions.sslServerCertificateFile = givenResource("cert/cert.pem");
Expand All @@ -135,7 +139,7 @@ public void testSslNoReloadOnFailure() throws InterruptedException {
SslContext expectedContext = sslSupport.getSslContext();

Thread.sleep(1001);
serverOptions.sslServerKeyFile.setLastModified(System.currentTimeMillis());
touch(serverOptions.sslServerKeyFile);
serverOptions.sslServerCertificateFile = null;

sslSupport.maybeAddHandler(socketChannel);
Expand All @@ -144,7 +148,7 @@ public void testSslNoReloadOnFailure() throws InterruptedException {
}

@Test
public void testSslReloadDisabled() throws InterruptedException {
public void testSslReloadDisabled() throws Exception {
serverOptions.sslMode = SslMode.ENABLE;
serverOptions.sslServerKeyFile = givenResource("cert/key.pem");
serverOptions.sslServerCertificateFile = givenResource("cert/cert.pem");
Expand All @@ -154,7 +158,7 @@ public void testSslReloadDisabled() throws InterruptedException {
SslContext expectedContext = sslSupport.getSslContext();

Thread.sleep(1001);
serverOptions.sslServerKeyFile.setLastModified(System.currentTimeMillis());
touch(serverOptions.sslServerKeyFile);

sslSupport.maybeAddHandler(socketChannel);

Expand All @@ -165,4 +169,8 @@ private File givenResource(String resource) {
URL url = this.getClass().getResource("/" + resource);
return new File(url.getFile());
}

private void touch(File file) throws IOException {
Files.setLastModifiedTime(file.toPath(), FileTime.from(Instant.now()));
}
}