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

Merge pull request #15590 from AndriiLandiak/certificates-test-4.2 

[lts-4.2] CertificateReloadManagerTest: force deterministic mtime bump instead of polling
pull/15709/head
Viacheslav Klimov 4 months ago
committed by GitHub
parent
27d34eead0 e998026de3
commit
fe157bde66
No known key found for this signature in database GPG Key ID: B5690EEEBB952194
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
Unified 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.common.transport.config.ssl.SslCredentialsConfig;
import org.thingsboard.server.queue.util.TbTransportComponent; import org.thingsboard.server.queue.util.TbTransportComponent;
import java.io.IOException;
import java.io.InputStream; import java.io.InputStream;
import java.nio.file.Files; import java.nio.file.Files;
import java.nio.file.Path; import java.nio.file.Path;
@ -165,7 +164,6 @@ public class CertificateReloadManager implements SmartInitializingSingleton, Dis
static class CertificateWatcher { static class CertificateWatcher {
private final List<Path> paths; private final List<Path> paths;
private final Runnable reloadCallback; private final Runnable reloadCallback;
private final Map<Path, Long> lastModifiedMap;
private final Map<Path, String> lastChecksumMap; private final Map<Path, String> lastChecksumMap;
private int consecutiveFailures; private int consecutiveFailures;
private String failedCombinedChecksum; private String failedCombinedChecksum;
@ -173,60 +171,23 @@ public class CertificateReloadManager implements SmartInitializingSingleton, Dis
CertificateWatcher(List<Path> paths, Runnable reloadCallback) { CertificateWatcher(List<Path> paths, Runnable reloadCallback) {
this.paths = paths; this.paths = paths;
this.reloadCallback = reloadCallback; this.reloadCallback = reloadCallback;
this.lastModifiedMap = new HashMap<>();
this.lastChecksumMap = new HashMap<>(); this.lastChecksumMap = new HashMap<>();
for (Path path : paths) { for (Path path : paths) {
lastModifiedMap.put(path, getLastModifiedTime(path));
lastChecksumMap.put(path, calculateChecksum(path)); lastChecksumMap.put(path, calculateChecksum(path));
} }
this.consecutiveFailures = 0; this.consecutiveFailures = 0;
} }
synchronized void checkAndReload(String name) { 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<>(); Map<Path, String> currentChecksums = new HashMap<>();
StringBuilder combined = new StringBuilder();
for (Path path : paths) { for (Path path : paths) {
currentModifiedTimes.put(path, getLastModifiedTime(path)); currentChecksums.put(path, calculateChecksum(path));
String checksum = calculateChecksum(path);
currentChecksums.put(path, checksum);
if (!combined.isEmpty()) {
combined.append("|");
}
combined.append(path).append("=").append(checksum);
} }
String combinedChecksum = combined.toString(); String combinedChecksum = combinedChecksum(currentChecksums);
String oldCombinedChecksum = combinedChecksum(lastChecksumMap);
// 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();
if (combinedChecksum.equals(oldCombinedChecksum)) { if (combinedChecksum.equals(oldCombinedChecksum)) {
// Content unchanged, just update modification times // Content unchanged
for (Path path : paths) {
lastModifiedMap.put(path, currentModifiedTimes.get(path));
}
return; return;
} }
@ -237,41 +198,34 @@ public class CertificateReloadManager implements SmartInitializingSingleton, Dis
} }
if (consecutiveFailures >= MAX_CONSECUTIVE_FAILURES) { 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; return;
} }
try { try {
log.info("Certificate change detected for: {}. Triggering reload...", name); log.info("Certificate change detected for: {}. Triggering reload...", name);
reloadCallback.run(); reloadCallback.run();
for (Path path : paths) { lastChecksumMap.putAll(currentChecksums);
lastModifiedMap.put(path, currentModifiedTimes.get(path));
lastChecksumMap.put(path, currentChecksums.get(path));
}
consecutiveFailures = 0; consecutiveFailures = 0;
failedCombinedChecksum = null; failedCombinedChecksum = null;
} catch (Exception e) { } catch (Exception e) {
consecutiveFailures++; consecutiveFailures++;
failedCombinedChecksum = combinedChecksum; failedCombinedChecksum = combinedChecksum;
// Deliberately NOT updating the lastModifiedMap here, so the next poll cycle retries // Deliberately NOT updating lastChecksumMap here, so the next poll cycle still sees a differing
// (mtime mismatch passes the early gate, checksum matches failedCombinedChecksum). // checksum, re-enters this method, and retries the same content.
log.error("Failed to reload certificate for {} (attempt {}/{}): {}", log.error("Failed to reload certificate for {} (attempt {}/{}): {}",
name, consecutiveFailures, MAX_CONSECUTIVE_FAILURES, e.getMessage(), e); name, consecutiveFailures, MAX_CONSECUTIVE_FAILURES, e.getMessage(), e);
} }
} }
private long getLastModifiedTime(Path path) { private String combinedChecksum(Map<Path, String> checksums) {
try { StringBuilder combined = new StringBuilder();
if (!Files.exists(path)) { for (Path path : paths) {
return 0; if (!combined.isEmpty()) {
combined.append("|");
} }
return Files.getLastModifiedTime(path).toMillis(); combined.append(path).append("=").append(checksums.getOrDefault(path, ""));
} catch (IOException e) {
return 0;
} }
return combined.toString();
} }
private String calculateChecksum(Path path) { 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.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger; 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.assertj.core.api.Assertions.assertThat;
import static org.awaitility.Awaitility.await;
public class CertificateReloadManagerTest { 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); Files.writeString(path, content);
await().atMost(2, SECONDS) // Force a strictly newer mtime: back-to-back writes can share a millisecond, hiding the change from the watcher.
.pollInterval(10, MILLISECONDS) Files.setLastModifiedTime(path, FileTime.fromMillis(baselineMtime + 1000));
.until(() -> Files.getLastModifiedTime(path).toMillis() != baselineMtime);
} }
private long mtime(Path path) throws IOException { private long mtime(Path path) throws IOException {
@ -77,7 +73,7 @@ public class CertificateReloadManagerTest {
certificateReloadManager.registerWatcher("test-cert", certFile, reloadCount::incrementAndGet); certificateReloadManager.registerWatcher("test-cert", certFile, reloadCount::incrementAndGet);
long baseline = mtime(certFile); 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"); ReflectionTestUtils.invokeMethod(certificateReloadManager, "checkCertificates");
@ -85,7 +81,7 @@ public class CertificateReloadManagerTest {
} }
@Test @Test
public void givenCertificateFileUnchanged_whenCheckForChanges_thenShouldNotTriggerReload() throws Exception { public void givenCertificateFileUnchanged_whenCheckForChanges_thenShouldNotTriggerReload() {
AtomicInteger reloadCount = new AtomicInteger(0); AtomicInteger reloadCount = new AtomicInteger(0);
certificateReloadManager.registerWatcher("test-cert", certFile, reloadCount::incrementAndGet); certificateReloadManager.registerWatcher("test-cert", certFile, reloadCount::incrementAndGet);
@ -147,7 +143,7 @@ public class CertificateReloadManagerTest {
certificateReloadManager.registerWatcher("test-key", keyFile, keyReloadCount::incrementAndGet); certificateReloadManager.registerWatcher("test-key", keyFile, keyReloadCount::incrementAndGet);
long baseline = mtime(keyFile); 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"); ReflectionTestUtils.invokeMethod(certificateReloadManager, "checkCertificates");
@ -168,8 +164,8 @@ public class CertificateReloadManagerTest {
long baseline1 = mtime(certFile); long baseline1 = mtime(certFile);
long baseline2 = mtime(cert2File); long baseline2 = mtime(cert2File);
writeFileAndAwaitMtimeChange(certFile, "-----BEGIN CERTIFICATE-----\nMODIFIED1\n-----END CERTIFICATE-----\n", baseline1); writeFileAndBumpMtime(certFile, "-----BEGIN CERTIFICATE-----\nMODIFIED1\n-----END CERTIFICATE-----\n", baseline1);
writeFileAndAwaitMtimeChange(cert2File, "-----BEGIN CERTIFICATE-----\nMODIFIED2\n-----END CERTIFICATE-----\n", baseline2); writeFileAndBumpMtime(cert2File, "-----BEGIN CERTIFICATE-----\nMODIFIED2\n-----END CERTIFICATE-----\n", baseline2);
ReflectionTestUtils.invokeMethod(certificateReloadManager, "checkCertificates"); ReflectionTestUtils.invokeMethod(certificateReloadManager, "checkCertificates");
@ -191,8 +187,8 @@ public class CertificateReloadManagerTest {
long baseline1 = mtime(certFile); long baseline1 = mtime(certFile);
long baseline2 = mtime(cert2File); long baseline2 = mtime(cert2File);
writeFileAndAwaitMtimeChange(certFile, "-----BEGIN CERTIFICATE-----\nMODIFIED1\n-----END CERTIFICATE-----\n", baseline1); writeFileAndBumpMtime(certFile, "-----BEGIN CERTIFICATE-----\nMODIFIED1\n-----END CERTIFICATE-----\n", baseline1);
writeFileAndAwaitMtimeChange(cert2File, "-----BEGIN CERTIFICATE-----\nMODIFIED2\n-----END CERTIFICATE-----\n", baseline2); writeFileAndBumpMtime(cert2File, "-----BEGIN CERTIFICATE-----\nMODIFIED2\n-----END CERTIFICATE-----\n", baseline2);
ReflectionTestUtils.invokeMethod(certificateReloadManager, "checkCertificates"); ReflectionTestUtils.invokeMethod(certificateReloadManager, "checkCertificates");
@ -224,9 +220,7 @@ public class CertificateReloadManagerTest {
for (int i = 0; i < 5; i++) { for (int i = 0; i < 5; i++) {
Files.writeString(certFile, "-----BEGIN CERTIFICATE-----\nCERT_VERSION_" + i + "\n-----END CERTIFICATE-----\n"); Files.writeString(certFile, "-----BEGIN CERTIFICATE-----\nCERT_VERSION_" + i + "\n-----END CERTIFICATE-----\n");
} }
await().atMost(2, SECONDS) Files.setLastModifiedTime(certFile, FileTime.fromMillis(baseline + 1000));
.pollInterval(10, MILLISECONDS)
.until(() -> mtime(certFile) != baseline);
ReflectionTestUtils.invokeMethod(certificateReloadManager, "checkCertificates"); ReflectionTestUtils.invokeMethod(certificateReloadManager, "checkCertificates");
@ -242,7 +236,7 @@ public class CertificateReloadManagerTest {
certificateReloadManager.registerWatcher("test-cert", certFile, reloadCount::incrementAndGet); certificateReloadManager.registerWatcher("test-cert", certFile, reloadCount::incrementAndGet);
long baseline = mtime(certFile); 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++) { for (int i = 0; i < 5; i++) {
new Thread(() -> { new Thread(() -> {
@ -272,7 +266,7 @@ public class CertificateReloadManagerTest {
certificateReloadManager.registerWatcher("test-cert", certFile, reloadCount::incrementAndGet); certificateReloadManager.registerWatcher("test-cert", certFile, reloadCount::incrementAndGet);
long baseline = mtime(certFile); long baseline = mtime(certFile);
writeFileAndAwaitMtimeChange(certFile, originalContent, baseline); writeFileAndBumpMtime(certFile, originalContent, baseline);
ReflectionTestUtils.invokeMethod(certificateReloadManager, "checkCertificates"); ReflectionTestUtils.invokeMethod(certificateReloadManager, "checkCertificates");
@ -289,7 +283,7 @@ public class CertificateReloadManagerTest {
}); });
long baseline = mtime(certFile); 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++) { for (int i = 0; i < 15; i++) {
ReflectionTestUtils.invokeMethod(certificateReloadManager, "checkCertificates"); ReflectionTestUtils.invokeMethod(certificateReloadManager, "checkCertificates");
@ -311,19 +305,41 @@ public class CertificateReloadManagerTest {
}); });
long baseline = mtime(certFile); 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"); ReflectionTestUtils.invokeMethod(certificateReloadManager, "checkCertificates");
assertThat(reloadAttempts.get()).isEqualTo(1); assertThat(reloadAttempts.get()).isEqualTo(1);
shouldFail.set(0); shouldFail.set(0);
long baseline2 = mtime(certFile); 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"); ReflectionTestUtils.invokeMethod(certificateReloadManager, "checkCertificates");
assertThat(reloadAttempts.get()).isEqualTo(2); 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 @Test
public void givenCallbackHitMaxFailures_whenFileChangesToNewContent_thenShouldResetAndRetry() throws Exception { public void givenCallbackHitMaxFailures_whenFileChangesToNewContent_thenShouldResetAndRetry() throws Exception {
AtomicInteger reloadAttempts = new AtomicInteger(0); AtomicInteger reloadAttempts = new AtomicInteger(0);
@ -337,7 +353,7 @@ public class CertificateReloadManagerTest {
}); });
long baseline = mtime(certFile); 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++) { for (int i = 0; i < 15; i++) {
ReflectionTestUtils.invokeMethod(certificateReloadManager, "checkCertificates"); ReflectionTestUtils.invokeMethod(certificateReloadManager, "checkCertificates");
@ -346,7 +362,7 @@ public class CertificateReloadManagerTest {
shouldFail.set(0); shouldFail.set(0);
long baseline2 = mtime(certFile); 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"); ReflectionTestUtils.invokeMethod(certificateReloadManager, "checkCertificates");
assertThat(reloadAttempts.get()).isEqualTo(11); assertThat(reloadAttempts.get()).isEqualTo(11);

Write Preview
Loading…
Cancel
Save