Merge branch '3.4.x'

Closes gh-44890

Author: bclozel

spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/ssl/FileWatcher.java

@@ -35,7 +35,6 @@
 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,26 +85,18 @@ void watch(Set<Path> paths, Runnable action) {
 					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);
 			}
 		}
 	}
 
-	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 @@ public void close() throws IOException {
 		}
 	}
 
+	/**
+	 * 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 @@ private void onThreadException(Thread thread, Throwable throwable) {
 		}
 
 		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 @@ public void close() throws IOException {
 	 */
 	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);

spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/ssl/FileWatcherTests.java

@@ -18,8 +18,10 @@
 
 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 @@ void shouldTriggerOnConfigMapUpdates(@TempDir Path tempDir) throws Exception {
 		}
 	}
 
+	/**
+	 * 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 @@ Path createConfigMap(Path parentDir, String secretFileName) throws IOException {
 		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 @@ void waitForChanges(boolean fail) throws InterruptedException {
 			}
 		}
 
+		void reset() {
+			this.latch = new CountDownLatch(1);
+			this.changed = false;
+		}
+
 	}
 
 }