FISH-788 Support sub-directories for MPCONFIG SecretDirConfigSource. #5006

nucleus/payara-modules/nucleus-microprofile/config-service/src/main/java/fish/payara/nucleus/microprofile/config/source/SecretsDirConfigSource.java

@@ -43,10 +43,15 @@
 
 import java.io.File;
 import java.io.IOException;
+import java.nio.file.FileVisitResult;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.nio.file.SimpleFileVisitor;
+import java.nio.file.attribute.BasicFileAttributes;
 import java.nio.file.attribute.FileTime;
+import java.util.Arrays;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
@@ -65,9 +70,10 @@ public class SecretsDirConfigSource extends PayaraConfigSource implements Config
     private Path secretsDir;
     private ConcurrentHashMap<String, String> properties;
     private ConcurrentHashMap<String, FileTime> storedModifiedTimes;
+    private ConcurrentHashMap<String, Path> storedPaths;
 
     public SecretsDirConfigSource() {
-        findFile();
+        findDir();
         loadProperties();
     }
 
@@ -97,28 +103,32 @@ public String getValue(String property) {
         String result = properties.get(property);
         if (result != null) {
             try {
-                // check the last modified time
+                // check existence (secret mount might have gone away) and the last modified time
                 FileTime ft = storedModifiedTimes.get(property);
-                Path path = Paths.get(secretsDir.toString(), property);
+                Path path = storedPaths.get(property);
                 if (Files.exists(path) && Files.getLastModifiedTime(path).compareTo(ft) > 0) {
-                    // file  has been modified since last check
-                    result = readFile(property);
+                    // file has been modified since last check, re-read content
+                    result = readFile(path);
                     storedModifiedTimes.put(property, Files.getLastModifiedTime(path));
                     properties.put(property, result);
                 }
             } catch (IOException ex) {
-                Logger.getLogger(SecretsDirConfigSource.class.getName()).log(Level.SEVERE, null, ex);
+                Logger.getLogger(SecretsDirConfigSource.class.getName()).log(Level.SEVERE, "Unable to read file in the directory", ex);
             }
         } else {
             // check whether there is a file there now as there wasn't before
-            Path path = Paths.get(secretsDir.toString(), property);
-            if (Files.exists(path) && Files.isRegularFile(path) && Files.isReadable(path)) {
+            // --> the list of possible paths is used "first match, first serve".
+            List<Path> paths = Arrays.asList(Paths.get(secretsDir.toString(), property),
+                                             Paths.get(secretsDir.toString(), property.replace('.', File.separatorChar)));
+            for (Path path : paths) {
                 try {
-                    result = readFile(property);
+                    result = readFile(path);
                     storedModifiedTimes.put(property, Files.getLastModifiedTime(path));
+                    storedPaths.put(property, path);
                     properties.put(property, result);
+                    break;
                 } catch (IOException ex) {
-                    Logger.getLogger(SecretsDirConfigSource.class.getName()).log(Level.SEVERE, null, ex);
+                    Logger.getLogger(SecretsDirConfigSource.class.getName()).log(Level.SEVERE, "Unable to read file in the directory", ex);
                 }
             }
         }
@@ -130,7 +140,7 @@ public String getName() {
         return "Secrets Directory";
     }
 
-    private void findFile() {
+    private void findDir() {
         secretsDir = Paths.get(configService.getMPConfig().getSecretDir());
 
         if (!Files.exists(secretsDir) || !Files.isDirectory(secretsDir) || !Files.isReadable(secretsDir)) {
@@ -143,39 +153,49 @@ private void findFile() {
         }
     }
 
-    private String readFile(String name) {
+    private String readFile(Path file) throws IOException {
         String result = null;
-        if (Files.exists(secretsDir) && Files.isDirectory(secretsDir) && Files.isReadable(secretsDir)) {
-            try {
-                Path file = Paths.get(secretsDir.toString(), name);
-                if (Files.exists(file) && Files.isReadable(file)) {
-                    StringBuilder collector = new StringBuilder();
-                    for (String line : Files.readAllLines(file)) {
-                        collector.append(line);
-                    }
-                    result = collector.toString();
-                }
-            } catch (IOException ex) {
-                Logger.getLogger(SecretsDirConfigSource.class.getName()).log(Level.SEVERE, null, ex);
+        if (Files.exists(file) && Files.isRegularFile(file) && Files.isReadable(file)) {
+            StringBuilder collector = new StringBuilder();
+            for (String line : Files.readAllLines(file)) {
+                collector.append(line);
             }
+            result = collector.toString();
         }
         return result;
     }
 
     private void loadProperties() {
         properties = new ConcurrentHashMap<>();
         storedModifiedTimes = new ConcurrentHashMap<>();
+        storedPaths = new ConcurrentHashMap<>();
         if (Files.exists(secretsDir) && Files.isDirectory(secretsDir) && Files.isReadable(secretsDir)) {
-            File files[] = secretsDir.toFile().listFiles();
-            for (File file : files) {
-                try {
-                    if (file.isFile() && file.canRead()) {
-                        properties.put(file.getName(), readFile(file.getName()));
-                        storedModifiedTimes.put(file.getName(), Files.getLastModifiedTime(file.toPath()));
+            try {
+                Files.walkFileTree(secretsDir, new SimpleFileVisitor<Path>() {
+                    @Override
+                    public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) throws IOException {
+                        return dir.toFile().isHidden() ? FileVisitResult.SKIP_SUBTREE : FileVisitResult.CONTINUE;
                     }
-                } catch (IOException ex) {
-                    Logger.getLogger(SecretsDirConfigSource.class.getName()).log(Level.SEVERE, "Unable to read file in the directory", ex);
-                }
+
+                    @Override
+                    public FileVisitResult visitFile(Path path, BasicFileAttributes mainAtts) throws IOException {
+                        File file = path.toFile();
+                        // do not read hidden files, as K8s Secret filenames are symlinks to hidden files with data.
+                        if (file.isFile() && ! file.isHidden() && file.canRead()) {
+                            // 1. get relative path based on the secrets dir ("/foobar"),
+                            // 2. replace all path seps with a ".",
+                            // so "/foobar/test/foo/bar" becomes "test/foo/bar" becomes "test.foo.bar" property name
+                            String property = secretsDir.relativize(path).toString().replace(File.separatorChar, '.');
+
+                            properties.put(property, readFile(path));
+                            storedModifiedTimes.put(property, mainAtts.lastModifiedTime());
+                            storedPaths.put(property, path);
+                        }
+                        return FileVisitResult.CONTINUE;
+                    }
+                });
+            } catch (IOException ex) {
+                Logger.getLogger(SecretsDirConfigSource.class.getName()).log(Level.SEVERE, "Unable to read file in the directory", ex);
             }
         }
     }

nucleus/payara-modules/nucleus-microprofile/config-service/src/test/java/fish/payara/nucleus/microprofile/config/source/SecretsDirConfigSourceTest.java

@@ -45,6 +45,7 @@
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.nio.file.attribute.FileTime;
+import java.util.Comparator;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
@@ -69,22 +70,44 @@ public class SecretsDirConfigSourceTest {
     @Before
     public void setUp() throws IOException {
         testDirectory = Files.createTempDirectory("microprofile-config-test");
-        // create a couple of test files
+
+        // create a couple of test simple files
         Path file1 = Paths.get(testDirectory.toString(), "property1");
         Path file2 = Paths.get(testDirectory.toString(), "property2");
+        Path fileHidden = Paths.get(testDirectory.toString(), ".hidden-property");
         file1 = Files.createFile(file1);
         Files.write(file1, "value1".getBytes());
         file2 = Files.createFile(file2);
         Files.write(file2, "value2".getBytes());
+        fileHidden = Files.createFile(fileHidden);
+
+        // create a subdirectory structure with test files
+        Path mounted = Paths.get(testDirectory.toString(), "foo", "bar");
+        Files.createDirectories(mounted);
+
+        Path fileMounted = Paths.get(mounted.toString(), "property3");
+        fileMounted = Files.createFile(fileMounted);
+        Files.write(fileMounted, "value3".getBytes());
+
+        // create "foo/bar/..data/property4" and symlink from "foo/bar/property4" as done on K8s
+        Path mountedK8s = Paths.get(mounted.toString(), "..data");
+        Files.createDirectories(mountedK8s);
+        Path fileK8sMounted = Paths.get(mountedK8s.toString(), "property4");
+        fileK8sMounted = Files.createFile(fileK8sMounted);
+        Files.write(fileK8sMounted, "value4".getBytes());
+        Path fileK8sSymlink = Paths.get(mounted.toString(), "property4");
+        fileK8sSymlink = Files.createSymbolicLink(fileK8sSymlink, fileK8sMounted);
+
+        // create & load
         source = new SecretsDirConfigSource(testDirectory);
     }
 
     @After
     public void tearDown() throws IOException {
-        for (File file : testDirectory.toFile().listFiles()) {
-            file.delete();
-        }
-        Files.delete(testDirectory);
+        Files.walk(testDirectory)
+            .sorted(Comparator.reverseOrder())
+            .map(Path::toFile)
+            .forEach(File::delete);
     }
 
     /**
@@ -95,6 +118,8 @@ public void testGetProperties() {
         Map<String, String> expected = new HashMap<>();
         expected.put("property1", "value1");
         expected.put("property2", "value2");
+        expected.put("foo.bar.property3", "value3");
+        expected.put("foo.bar.property4", "value4");
         assertEquals(expected, source.getProperties());
     }
 
@@ -103,7 +128,7 @@ public void testGetProperties() {
      */
     @Test
     public void testGetPropertyNames() {
-        assertEquals(new HashSet<>(asList("property1", "property2")), source.getPropertyNames());
+        assertEquals(new HashSet<>(asList("property1", "property2", "foo.bar.property3", "foo.bar.property4")), source.getPropertyNames());
     }
 
     /**
@@ -113,6 +138,8 @@ public void testGetPropertyNames() {
     public void testGetValue() {
         assertEquals("value1", source.getValue("property1"));
         assertEquals("value2", source.getValue("property2"));
+        assertEquals("value3", source.getValue("foo.bar.property3"));
+        assertEquals("value4", source.getValue("foo.bar.property4"));
     }
 
     /**
@@ -142,7 +169,27 @@ public void testChangeProperty() throws IOException {
             Files.write(file1, "value1".getBytes());
         }
     }
-
+
+    /**
+     * Test the changed Property in subdirectories
+     * @throws java.io.IOException
+     */
+    @Test
+    public void testChangePropertyInSubdir() throws IOException {
+        assertEquals("value4", source.getValue("foo.bar.property4"));
+        // change the file
+        Path file = Paths.get(testDirectory.toString(), "foo", "bar", "..data", "property4");
+        Files.write(file, "value-changed".getBytes());
+        try {
+            FileTime nowplus1sec = FileTime.fromMillis(System.currentTimeMillis() + TimeUnit.SECONDS.toMillis(1));
+            Files.setLastModifiedTime(file, nowplus1sec);
+            assertEquals("value-changed", source.getValue("foo.bar.property4"));
+        } finally {
+            // clean up
+            Files.write(file, "value4".getBytes());
+        }
+    }
+
     /**
      * Tests getting a new property as the file has now appeared
      */
@@ -159,7 +206,27 @@ public void testNewFile() throws IOException {
             Files.delete(file1);
         }
     }
-
+
+    /**
+     * Tests getting a new property as the file has now appeared in a subdirectory
+     */
+    @Test
+    public void testNewFileInSubdir() throws IOException {
+        assertNull(source.getValue("foo.bar.property-new"));
+        // change the file
+        Path file = Paths.get(testDirectory.toString(), "foo", "bar", "..data", "property-new");
+        Files.write(file, "newValue".getBytes());
+        Path fileSymlink = Paths.get(testDirectory.toString(), "foo", "bar", "property-new");
+        Files.createSymbolicLink(fileSymlink, file);
+        try {
+            assertEquals("newValue", source.getValue("foo.bar.property-new"));
+        } finally {
+            // clean up
+            Files.delete(file);
+            Files.delete(fileSymlink);
+        }
+    }
+
     @Test
     public void testBadDirectoryNoBlowUp() {
         assertNull(new SecretsDirConfigSource(Paths.get(testDirectory.toString(), "FOOBLE")).getValue("BILLY"));