From 14bbceb03836b6284f5177e17734060d4681a7d7 Mon Sep 17 00:00:00 2001
From: Dmytro Nosan <dimanosan@gmail.com>
Date: Sun, 23 Mar 2025 23:06:25 +0200
Subject: [PATCH] Watch symlinks for changes for SSL hot reloading

Prior to this commit, the SSL file hot reloading feature would
create file watchers for all configured locations and would resolve
symbolic links if they're found as registrations.

This arrangement would work for typical Let's Encrypt setups, but
would not get notified of consecutive changes for k8s setups.
Kubernetes uses a mix of symlinks and atomic file moves to update
secrets.

This commit not only tracks changes to symlinks targets, but also
to the original symlink.

Closes gh-44807

Signed-off-by: Dmytro Nosan <dimanosan@gmail.com>
[brian.clozel@broadcom.com: apply code conventions]
Signed-off-by: Brian Clozel <brian.clozel@broadcom.com>
---
 .../boot/autoconfigure/ssl/FileWatcher.java   | 61 +++++++++++----
 .../autoconfigure/ssl/FileWatcherTests.java   | 75 ++++++++++++++++++-
 2 files changed, 119 insertions(+), 17 deletions(-)

diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/ssl/FileWatcher.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/ssl/FileWatcher.java
index ba05ecd28dd..a6f9db6467b 100644
--- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/ssl/FileWatcher.java
+++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/ssl/FileWatcher.java
@@ -35,7 +35,6 @@ import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.TimeUnit;
-import java.util.stream.Collectors;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -86,11 +85,11 @@ class FileWatcher implements Closeable {
 					this.thread = new WatcherThread();
 					this.thread.start();
 				}
-				Set<Path> actualPaths = new HashSet<>();
+				Set<Path> registrationPaths = new HashSet<>();
 				for (Path path : paths) {
-					actualPaths.add(resolveSymlinkIfNecessary(path));
+					registrationPaths.addAll(getRegistrationPaths(path));
 				}
-				this.thread.register(new Registration(actualPaths, action));
+				this.thread.register(new Registration(registrationPaths, action));
 			}
 			catch (IOException ex) {
 				throw new UncheckedIOException("Failed to register paths for watching: " + paths, ex);
@@ -98,14 +97,6 @@ class FileWatcher implements Closeable {
 		}
 	}
 
-	private static Path resolveSymlinkIfNecessary(Path path) throws IOException {
-		if (Files.isSymbolicLink(path)) {
-			Path target = path.resolveSibling(Files.readSymbolicLink(path));
-			return resolveSymlinkIfNecessary(target);
-		}
-		return path;
-	}
-
 	@Override
 	public void close() throws IOException {
 		synchronized (this.lock) {
@@ -123,6 +114,44 @@ class FileWatcher implements Closeable {
 		}
 	}
 
+	/**
+	 * Retrieves all {@link Path Paths} that should be registered for the specified
+	 * {@link Path}. If the path is a symlink, changes to the symlink should be monitored,
+	 * not just the file it points to. For example, for the given {@code keystore.jks}
+	 * path in the following directory structure:<pre>
+	 * .
+	 * ├── ..a72e81ff-f0e1-41d8-a19b-068d3d1d4e2f
+	 * │   ├── keystore.jks
+	 * ├── ..data -> ..a72e81ff-f0e1-41d8-a19b-068d3d1d4e2f
+	 * ├── keystore.jks -> ..data/keystore.jks
+	 * </pre> the resulting paths would include:
+	 * <ul>
+	 * <li><b>keystore.jks</b></li>
+	 * <li><b>..data/keystore.jks</b></li>
+	 * <li><b>..data</b></li>
+	 * <li><b>..a72e81ff-f0e1-41d8-a19b-068d3d1d4e2f/keystore.jks</b></li>
+	 * </ul>
+	 * @param path the path
+	 * @return all possible {@link Path} instances to be registered
+	 * @throws IOException if an I/O error occurs
+	 */
+	private static Set<Path> getRegistrationPaths(Path path) throws IOException {
+		path = path.toAbsolutePath();
+		Set<Path> result = new HashSet<>();
+		result.add(path);
+		Path parent = path.getParent();
+		if (parent != null && Files.isSymbolicLink(parent)) {
+			result.add(parent);
+			Path target = parent.resolveSibling(Files.readSymbolicLink(parent));
+			result.addAll(getRegistrationPaths(target.resolve(path.getFileName())));
+		}
+		else if (Files.isSymbolicLink(path)) {
+			Path target = path.resolveSibling(Files.readSymbolicLink(path));
+			result.addAll(getRegistrationPaths(target));
+		}
+		return result;
+	}
+
 	/**
 	 * The watcher thread used to check for changes.
 	 */
@@ -145,11 +174,15 @@ class FileWatcher implements Closeable {
 		}
 
 		void register(Registration registration) throws IOException {
+			Set<Path> directories = new HashSet<>();
 			for (Path path : registration.paths()) {
 				if (!Files.isRegularFile(path) && !Files.isDirectory(path)) {
 					throw new IOException("'%s' is neither a file nor a directory".formatted(path));
 				}
 				Path directory = Files.isDirectory(path) ? path : path.getParent();
+				directories.add(directory);
+			}
+			for (Path directory : directories) {
 				WatchKey watchKey = register(directory);
 				this.registrations.computeIfAbsent(watchKey, (key) -> new CopyOnWriteArrayList<>()).add(registration);
 			}
@@ -224,10 +257,6 @@ class FileWatcher implements Closeable {
 	 */
 	private record Registration(Set<Path> paths, Runnable action) {
 
-		Registration {
-			paths = paths.stream().map(Path::toAbsolutePath).collect(Collectors.toSet());
-		}
-
 		boolean manages(Path file) {
 			Path absolutePath = file.toAbsolutePath();
 			return this.paths.contains(absolutePath) || isInDirectories(absolutePath);
diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/ssl/FileWatcherTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/ssl/FileWatcherTests.java
index 7a4d4061d14..63bb2d65133 100644
--- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/ssl/FileWatcherTests.java
+++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/ssl/FileWatcherTests.java
@@ -18,8 +18,10 @@ package org.springframework.boot.autoconfigure.ssl;
 
 import java.io.IOException;
 import java.io.UncheckedIOException;
+import java.nio.file.AccessDeniedException;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.nio.file.StandardCopyOption;
 import java.time.Duration;
 import java.util.Set;
 import java.util.UUID;
@@ -254,6 +256,62 @@ class FileWatcherTests {
 		}
 	}
 
+	/**
+	 * Updates many times K8s ConfigMap/Secret with atomic move. <pre>
+	 * .
+	 * ├── ..a72e81ff-f0e1-41d8-a19b-068d3d1d4e2f
+	 * │   ├── keystore.jks
+	 * ├── ..data -> ..a72e81ff-f0e1-41d8-a19b-068d3d1d4e2f
+	 * ├── keystore.jks -> ..data/keystore.jks
+	 * </pre>
+	 *
+	 * After a first a ConfigMap/Secret update, this will look like: <pre>
+	 * .
+	 * ├── ..bba2a61f-ce04-4c35-93aa-e455110d4487
+	 * │   ├── keystore.jks
+	 * ├── ..data -> ..bba2a61f-ce04-4c35-93aa-e455110d4487
+	 * ├── keystore.jks -> ..data/keystore.jks
+	 * </pre> After a second a ConfigMap/Secret update, this will look like: <pre>
+	 * .
+	 * ├── ..134887f0-df8f-4433-b70c-7784d2a33bd1
+	 * │   ├── keystore.jks
+	 * ├── ..data -> ..134887f0-df8f-4433-b70c-7784d2a33bd1
+	 * ├── keystore.jks -> ..data/keystore.jks
+	 *</pre>
+	 * <p>
+	 * When Kubernetes updates either the ConfigMap or Secret, it performs the following
+	 * steps:
+	 * <ul>
+	 * <li>Creates a new unique directory.</li>
+	 * <li>Writes the ConfigMap/Secret content to the newly created directory.</li>
+	 * <li>Creates a symlink {@code ..data_tmp} pointing to the newly created
+	 * directory.</li>
+	 * <li>Performs an atomic rename of {@code ..data_tmp} to {@code ..data}.</li>
+	 * <li>Deletes the old ConfigMap/Secret directory.</li>
+	 * </ul>
+	 */
+	@Test
+	void shouldTriggerOnConfigMapAtomicMoveUpdates(@TempDir Path tempDir) throws Exception {
+		Path configMap1 = createConfigMap(tempDir, "keystore.jks");
+		Path data = Files.createSymbolicLink(tempDir.resolve("..data"), configMap1);
+		Files.createSymbolicLink(tempDir.resolve("keystore.jks"), data.resolve("keystore.jks"));
+		WaitingCallback callback = new WaitingCallback();
+		this.fileWatcher.watch(Set.of(tempDir.resolve("keystore.jks")), callback);
+		// First update
+		Path configMap2 = createConfigMap(tempDir, "keystore.jks");
+		Path dataTmp = Files.createSymbolicLink(tempDir.resolve("..data_tmp"), configMap2);
+		move(dataTmp, data);
+		FileSystemUtils.deleteRecursively(configMap1);
+		callback.expectChanges();
+		callback.reset();
+		// Second update
+		Path configMap3 = createConfigMap(tempDir, "keystore.jks");
+		dataTmp = Files.createSymbolicLink(tempDir.resolve("..data_tmp"), configMap3);
+		move(dataTmp, data);
+		FileSystemUtils.deleteRecursively(configMap2);
+		callback.expectChanges();
+	}
+
 	Path createConfigMap(Path parentDir, String secretFileName) throws IOException {
 		Path configMapFolder = parentDir.resolve(".." + UUID.randomUUID());
 		Files.createDirectory(configMapFolder);
@@ -262,9 +320,19 @@ class FileWatcherTests {
 		return configMapFolder;
 	}
 
+	private void move(Path source, Path target) throws IOException {
+		try {
+			Files.move(source, target, StandardCopyOption.ATOMIC_MOVE);
+		}
+		catch (AccessDeniedException ex) {
+			// Windows
+			Files.move(source, target, StandardCopyOption.REPLACE_EXISTING);
+		}
+	}
+
 	private static final class WaitingCallback implements Runnable {
 
-		private final CountDownLatch latch = new CountDownLatch(1);
+		private CountDownLatch latch = new CountDownLatch(1);
 
 		volatile boolean changed = false;
 
@@ -292,6 +360,11 @@ class FileWatcherTests {
 			}
 		}
 
+		void reset() {
+			this.latch = new CountDownLatch(1);
+			this.changed = false;
+		}
+
 	}
 
 }