tsai
/
thingsboard
mirror of https://github.com/thingsboard/thingsboard.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity
Browse Source

CertificateReloadManager: detect content changes via checksum, not mtime

pull/15590/head
Andrii Landiak 5 days ago
parent
563f5394f0
commit
e998026de3
2 changed files with 54 additions and 84 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 74
      common/transport/transport-api/src/main/java/org/thingsboard/server/common/transport/service/CertificateReloadManager.java
  2. 64
      common/transport/transport-api/src/test/java/org/thingsboard/server/common/transport/service/CertificateReloadManagerTest.java

74
common/transport/transport-api/src/main/java/org/thingsboard/server/common/transport/service/CertificateReloadManager.java
View File

@ -27,7 +27,6 @@ import org.thingsboard.server.common.transport.config.ssl.SslCredentials;
import org.thingsboard.server.common.transport.config.ssl.SslCredentialsConfig;
import org.thingsboard.server.queue.util.TbTransportComponent;
import java.io.IOException;
import java.io.InputStream;
import java.nio.file.Files;
import java.nio.file.Path;
@ -165,7 +164,6 @@ public class CertificateReloadManager implements SmartInitializingSingleton, Dis
static class CertificateWatcher {
private final List<Path> paths;
private final Runnable reloadCallback;
private final Map<Path, Long> lastModifiedMap;
private final Map<Path, String> lastChecksumMap;
private int consecutiveFailures;
private String failedCombinedChecksum;
@ -173,60 +171,23 @@ public class CertificateReloadManager implements SmartInitializingSingleton, Dis
CertificateWatcher(List<Path> paths, Runnable reloadCallback) {
this.paths = paths;
this.reloadCallback = reloadCallback;
this.lastModifiedMap = new HashMap<>();
this.lastChecksumMap = new HashMap<>();
for (Path path : paths) {
lastModifiedMap.put(path, getLastModifiedTime(path));
lastChecksumMap.put(path, calculateChecksum(path));
}
this.consecutiveFailures = 0;
}
synchronized void checkAndReload(String name) {
boolean anyModifiedChanged = false;
for (Path path : paths) {
long currentModified = getLastModifiedTime(path);
Long lastModified = lastModifiedMap.getOrDefault(path, 0L);
if (currentModified != lastModified) {
anyModifiedChanged = true;
break;
}
}
if (!anyModifiedChanged) {
return;
}
// Capture mtimes and checksums together before the callback runs.
// Pairing a post-callback mtime with a pre-callback checksum would let a write-during-reload be missed on the next poll.
Map<Path, Long> currentModifiedTimes = new HashMap<>();
Map<Path, String> currentChecksums = new HashMap<>();
StringBuilder combined = new StringBuilder();
for (Path path : paths) {
currentModifiedTimes.put(path, getLastModifiedTime(path));
String checksum = calculateChecksum(path);
currentChecksums.put(path, checksum);
if (!combined.isEmpty()) {
combined.append("|");
}
combined.append(path).append("=").append(checksum);
currentChecksums.put(path, calculateChecksum(path));
}
String combinedChecksum = combined.toString();
// Build old combined checksum for comparison
StringBuilder oldCombined = new StringBuilder();
for (Path path : paths) {
if (!oldCombined.isEmpty()) {
oldCombined.append("|");
}
oldCombined.append(path).append("=").append(lastChecksumMap.getOrDefault(path, ""));
}
String oldCombinedChecksum = oldCombined.toString();
String combinedChecksum = combinedChecksum(currentChecksums);
String oldCombinedChecksum = combinedChecksum(lastChecksumMap);
if (combinedChecksum.equals(oldCombinedChecksum)) {
// Content unchanged, just update modification times
for (Path path : paths) {
lastModifiedMap.put(path, currentModifiedTimes.get(path));
}
// Content unchanged
return;
}
@ -237,41 +198,34 @@ public class CertificateReloadManager implements SmartInitializingSingleton, Dis
}
if (consecutiveFailures >= MAX_CONSECUTIVE_FAILURES) {
// Update modification times to avoid re-checking mtime and re-computing checksums every poll cycle
for (Path path : paths) {
lastModifiedMap.put(path, currentModifiedTimes.get(path));
}
return;
}
try {
log.info("Certificate change detected for: {}. Triggering reload...", name);
reloadCallback.run();
for (Path path : paths) {
lastModifiedMap.put(path, currentModifiedTimes.get(path));
lastChecksumMap.put(path, currentChecksums.get(path));
}
lastChecksumMap.putAll(currentChecksums);
consecutiveFailures = 0;
failedCombinedChecksum = null;
} catch (Exception e) {
consecutiveFailures++;
failedCombinedChecksum = combinedChecksum;
// Deliberately NOT updating the lastModifiedMap here, so the next poll cycle retries
// (mtime mismatch passes the early gate, checksum matches failedCombinedChecksum).
// Deliberately NOT updating lastChecksumMap here, so the next poll cycle still sees a differing
// checksum, re-enters this method, and retries the same content.
log.error("Failed to reload certificate for {} (attempt {}/{}): {}",
name, consecutiveFailures, MAX_CONSECUTIVE_FAILURES, e.getMessage(), e);
}
}
private long getLastModifiedTime(Path path) {
try {
if (!Files.exists(path)) {
return 0;
private String combinedChecksum(Map<Path, String> checksums) {
StringBuilder combined = new StringBuilder();
for (Path path : paths) {
if (!combined.isEmpty()) {
combined.append("|");
}
return Files.getLastModifiedTime(path).toMillis();
} catch (IOException e) {
return 0;
combined.append(path).append("=").append(checksums.getOrDefault(path, ""));
}
return combined.toString();
}
private String calculateChecksum(Path path) {

64
common/transport/transport-api/src/test/java/org/thingsboard/server/common/transport/service/CertificateReloadManagerTest.java
View File

@ -31,10 +31,7 @@ import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
import static java.util.concurrent.TimeUnit.MILLISECONDS;
import static java.util.concurrent.TimeUnit.SECONDS;
import static org.assertj.core.api.Assertions.assertThat;
import static org.awaitility.Awaitility.await;
public class CertificateReloadManagerTest {
@ -59,11 +56,10 @@ public class CertificateReloadManagerTest {
}
}
private void writeFileAndAwaitMtimeChange(Path path, String content, long baselineMtime) throws IOException {
private void writeFileAndBumpMtime(Path path, String content, long baselineMtime) throws IOException {
Files.writeString(path, content);
await().atMost(2, SECONDS)
.pollInterval(10, MILLISECONDS)
.until(() -> Files.getLastModifiedTime(path).toMillis() != baselineMtime);
// Force a strictly newer mtime: back-to-back writes can share a millisecond, hiding the change from the watcher.
Files.setLastModifiedTime(path, FileTime.fromMillis(baselineMtime + 1000));
}
private long mtime(Path path) throws IOException {
@ -77,7 +73,7 @@ public class CertificateReloadManagerTest {
certificateReloadManager.registerWatcher("test-cert", certFile, reloadCount::incrementAndGet);
long baseline = mtime(certFile);
writeFileAndAwaitMtimeChange(certFile, "-----BEGIN CERTIFICATE-----\nTEST_CERT_V2_MODIFIED\n-----END CERTIFICATE-----\n", baseline);
writeFileAndBumpMtime(certFile, "-----BEGIN CERTIFICATE-----\nTEST_CERT_V2_MODIFIED\n-----END CERTIFICATE-----\n", baseline);
ReflectionTestUtils.invokeMethod(certificateReloadManager, "checkCertificates");
@ -85,7 +81,7 @@ public class CertificateReloadManagerTest {
}
@Test
public void givenCertificateFileUnchanged_whenCheckForChanges_thenShouldNotTriggerReload() throws Exception {
public void givenCertificateFileUnchanged_whenCheckForChanges_thenShouldNotTriggerReload() {
AtomicInteger reloadCount = new AtomicInteger(0);
certificateReloadManager.registerWatcher("test-cert", certFile, reloadCount::incrementAndGet);
@ -147,7 +143,7 @@ public class CertificateReloadManagerTest {
certificateReloadManager.registerWatcher("test-key", keyFile, keyReloadCount::incrementAndGet);
long baseline = mtime(keyFile);
writeFileAndAwaitMtimeChange(keyFile, "-----BEGIN PRIVATE KEY-----\nTEST_KEY_V2_MODIFIED\n-----END PRIVATE KEY-----\n", baseline);
writeFileAndBumpMtime(keyFile, "-----BEGIN PRIVATE KEY-----\nTEST_KEY_V2_MODIFIED\n-----END PRIVATE KEY-----\n", baseline);
ReflectionTestUtils.invokeMethod(certificateReloadManager, "checkCertificates");
@ -168,8 +164,8 @@ public class CertificateReloadManagerTest {
long baseline1 = mtime(certFile);
long baseline2 = mtime(cert2File);
writeFileAndAwaitMtimeChange(certFile, "-----BEGIN CERTIFICATE-----\nMODIFIED1\n-----END CERTIFICATE-----\n", baseline1);
writeFileAndAwaitMtimeChange(cert2File, "-----BEGIN CERTIFICATE-----\nMODIFIED2\n-----END CERTIFICATE-----\n", baseline2);
writeFileAndBumpMtime(certFile, "-----BEGIN CERTIFICATE-----\nMODIFIED1\n-----END CERTIFICATE-----\n", baseline1);
writeFileAndBumpMtime(cert2File, "-----BEGIN CERTIFICATE-----\nMODIFIED2\n-----END CERTIFICATE-----\n", baseline2);
ReflectionTestUtils.invokeMethod(certificateReloadManager, "checkCertificates");
@ -191,8 +187,8 @@ public class CertificateReloadManagerTest {
long baseline1 = mtime(certFile);
long baseline2 = mtime(cert2File);
writeFileAndAwaitMtimeChange(certFile, "-----BEGIN CERTIFICATE-----\nMODIFIED1\n-----END CERTIFICATE-----\n", baseline1);
writeFileAndAwaitMtimeChange(cert2File, "-----BEGIN CERTIFICATE-----\nMODIFIED2\n-----END CERTIFICATE-----\n", baseline2);
writeFileAndBumpMtime(certFile, "-----BEGIN CERTIFICATE-----\nMODIFIED1\n-----END CERTIFICATE-----\n", baseline1);
writeFileAndBumpMtime(cert2File, "-----BEGIN CERTIFICATE-----\nMODIFIED2\n-----END CERTIFICATE-----\n", baseline2);
ReflectionTestUtils.invokeMethod(certificateReloadManager, "checkCertificates");
@ -224,9 +220,7 @@ public class CertificateReloadManagerTest {
for (int i = 0; i < 5; i++) {
Files.writeString(certFile, "-----BEGIN CERTIFICATE-----\nCERT_VERSION_" + i + "\n-----END CERTIFICATE-----\n");
}
await().atMost(2, SECONDS)
.pollInterval(10, MILLISECONDS)
.until(() -> mtime(certFile) != baseline);
Files.setLastModifiedTime(certFile, FileTime.fromMillis(baseline + 1000));
ReflectionTestUtils.invokeMethod(certificateReloadManager, "checkCertificates");
@ -242,7 +236,7 @@ public class CertificateReloadManagerTest {
certificateReloadManager.registerWatcher("test-cert", certFile, reloadCount::incrementAndGet);
long baseline = mtime(certFile);
writeFileAndAwaitMtimeChange(certFile, "-----BEGIN CERTIFICATE-----\nMODIFIED\n-----END CERTIFICATE-----\n", baseline);
writeFileAndBumpMtime(certFile, "-----BEGIN CERTIFICATE-----\nMODIFIED\n-----END CERTIFICATE-----\n", baseline);
for (int i = 0; i < 5; i++) {
new Thread(() -> {
@ -272,7 +266,7 @@ public class CertificateReloadManagerTest {
certificateReloadManager.registerWatcher("test-cert", certFile, reloadCount::incrementAndGet);
long baseline = mtime(certFile);
writeFileAndAwaitMtimeChange(certFile, originalContent, baseline);
writeFileAndBumpMtime(certFile, originalContent, baseline);
ReflectionTestUtils.invokeMethod(certificateReloadManager, "checkCertificates");
@ -289,7 +283,7 @@ public class CertificateReloadManagerTest {
});
long baseline = mtime(certFile);
writeFileAndAwaitMtimeChange(certFile, "-----BEGIN CERTIFICATE-----\nBAD_CERT\n-----END CERTIFICATE-----\n", baseline);
writeFileAndBumpMtime(certFile, "-----BEGIN CERTIFICATE-----\nBAD_CERT\n-----END CERTIFICATE-----\n", baseline);
for (int i = 0; i < 15; i++) {
ReflectionTestUtils.invokeMethod(certificateReloadManager, "checkCertificates");
@ -311,19 +305,41 @@ public class CertificateReloadManagerTest {
});
long baseline = mtime(certFile);
writeFileAndAwaitMtimeChange(certFile, "-----BEGIN CERTIFICATE-----\nBAD_CERT\n-----END CERTIFICATE-----\n", baseline);
writeFileAndBumpMtime(certFile, "-----BEGIN CERTIFICATE-----\nBAD_CERT\n-----END CERTIFICATE-----\n", baseline);
ReflectionTestUtils.invokeMethod(certificateReloadManager, "checkCertificates");
assertThat(reloadAttempts.get()).isEqualTo(1);
shouldFail.set(0);
long baseline2 = mtime(certFile);
writeFileAndAwaitMtimeChange(certFile, "-----BEGIN CERTIFICATE-----\nGOOD_CERT\n-----END CERTIFICATE-----\n", baseline2);
writeFileAndBumpMtime(certFile, "-----BEGIN CERTIFICATE-----\nGOOD_CERT\n-----END CERTIFICATE-----\n", baseline2);
ReflectionTestUtils.invokeMethod(certificateReloadManager, "checkCertificates");
assertThat(reloadAttempts.get()).isEqualTo(2);
}
@Test
public void givenContentChangedButMtimeUnchanged_whenCheckForChanges_thenShouldTriggerReload() throws Exception {
// Bug fingerprint: a cert-manager rotation that lands in the same wall-clock millisecond as the
// watcher's recorded baseline mtime. Files.getLastModifiedTime().toMillis() truncates to the ms,
// so the rotated content shares the baseline mtime and an mtime-only gate would never re-hash it.
AtomicInteger reloadCount = new AtomicInteger(0);
certificateReloadManager.registerWatcher("test-cert", certFile, reloadCount::incrementAndGet);
long baseline = mtime(certFile);
Files.writeString(certFile, "-----BEGIN CERTIFICATE-----\nROTATED_SAME_MS\n-----END CERTIFICATE-----\n");
// Force the mtime back to the exact baseline millisecond — content changed, timestamp did not.
Files.setLastModifiedTime(certFile, FileTime.fromMillis(baseline));
// Sanity guard: the watcher observes a timestamp identical to its recorded baseline.
assertThat(mtime(certFile)).isEqualTo(baseline);
ReflectionTestUtils.invokeMethod(certificateReloadManager, "checkCertificates");
assertThat(reloadCount.get()).isEqualTo(1);
}
@Test
public void givenCallbackHitMaxFailures_whenFileChangesToNewContent_thenShouldResetAndRetry() throws Exception {
AtomicInteger reloadAttempts = new AtomicInteger(0);
@ -337,7 +353,7 @@ public class CertificateReloadManagerTest {
});
long baseline = mtime(certFile);
writeFileAndAwaitMtimeChange(certFile, "-----BEGIN CERTIFICATE-----\nBAD_CERT\n-----END CERTIFICATE-----\n", baseline);
writeFileAndBumpMtime(certFile, "-----BEGIN CERTIFICATE-----\nBAD_CERT\n-----END CERTIFICATE-----\n", baseline);
for (int i = 0; i < 15; i++) {
ReflectionTestUtils.invokeMethod(certificateReloadManager, "checkCertificates");
@ -346,7 +362,7 @@ public class CertificateReloadManagerTest {
shouldFail.set(0);
long baseline2 = mtime(certFile);
writeFileAndAwaitMtimeChange(certFile, "-----BEGIN CERTIFICATE-----\nFIXED_CERT\n-----END CERTIFICATE-----\n", baseline2);
writeFileAndBumpMtime(certFile, "-----BEGIN CERTIFICATE-----\nFIXED_CERT\n-----END CERTIFICATE-----\n", baseline2);
ReflectionTestUtils.invokeMethod(certificateReloadManager, "checkCertificates");
assertThat(reloadAttempts.get()).isEqualTo(11);

Write Preview
Loading…
Cancel
Save