Use actual files in application package to validate file/directory in user config

4 files changed, 54 insertions, 20 deletions

diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/ApplicationFile.java b/config-model-api/src/main/java/com/yahoo/config/application/api/ApplicationFile.java
index 36d6efdf59b..d262c7bc862 100644
--- a/config-model-api/src/main/java/com/yahoo/config/application/api/ApplicationFile.java
+++ b/config-model-api/src/main/java/com/yahoo/config/application/api/ApplicationFile.java
@@ -27,21 +27,21 @@ public abstract class ApplicationFile implements Comparable<ApplicationFile> {
     }
 
     /**
-     * Check whether or not this file is a directory.
+     * Checks whether this file is a directory.
      *
      * @return true if it is, false if not.
      */
     public abstract boolean isDirectory();
 
     /**
-     * Test whether or not this file exists.
+     * Tests whether this file exists.
      *
      * @return true if it exists, false if not.
      */
     public abstract boolean exists();
 
     /**
-     * Create a {@link Reader} for the contents of this file.
+     * Creates a {@link Reader} for the contents of this file.
      *
      * @return A {@link Reader} that should be closed after use.
      * @throws FileNotFoundException if the file is not found.
@@ -50,7 +50,7 @@ public abstract class ApplicationFile implements Comparable<ApplicationFile> {
 
 
     /**
-     * Create an {@link InputStream} for the contents of this file.
+     * Creates an {@link InputStream} for the contents of this file.
      *
      * @return An {@link InputStream} that should be closed after use.
      * @throws FileNotFoundException if the file is not found.
@@ -58,7 +58,7 @@ public abstract class ApplicationFile implements Comparable<ApplicationFile> {
     public abstract InputStream createInputStream() throws FileNotFoundException;
 
     /**
-     * Create a directory at the path represented by this file. Parent directories will
+     * Creates a directory at the path represented by this file. Parent directories will
      * be automatically created.
      *
      * @return this
@@ -67,7 +67,7 @@ public abstract class ApplicationFile implements Comparable<ApplicationFile> {
     public abstract ApplicationFile createDirectory();
 
     /**
-     * Write the contents from this reader to this file. Any existing content will be overwritten!
+     * Writes the contents from supplied reader to this file. Any existing content will be overwritten!
      *
      * @param input A reader pointing to the content that should be written.
      * @return this
@@ -82,7 +82,7 @@ public abstract class ApplicationFile implements Comparable<ApplicationFile> {
     public abstract ApplicationFile appendFile(String value);
 
     /**
-     * List the files under this directory. If this is file, an empty list is returned.
+     * Lists the files under this directory. If this is file, an empty list is returned.
      * Only immediate files/subdirectories are returned.
      *
      * @return a list of files in this directory.
@@ -92,7 +92,7 @@ public abstract class ApplicationFile implements Comparable<ApplicationFile> {
     }
 
     /**
-     * List the files under this directory. If this is file, an empty list is returned.
+     * Lists the files under this directory. If this is a file, an empty list is returned.
      * Only immediate files/subdirectories are returned.
      *
      * @param filter A filter functor for filtering path names
@@ -101,7 +101,7 @@ public abstract class ApplicationFile implements Comparable<ApplicationFile> {
     public abstract List<ApplicationFile> listFiles(PathFilter filter);
 
     /**
-     * List the files in this directory, optionally list files for subdirectories recursively as well.
+     * Lists the files in this directory, optionally lists files for subdirectories recursively as well.
      *
      * @param recurse Set to true if all files in the directory tree should be returned.
      * @return a list of files in this directory.
@@ -121,7 +121,7 @@ public abstract class ApplicationFile implements Comparable<ApplicationFile> {
     }
 
     /**
-     * Delete the file pointed to by this. If it is a non-empty directory, the operation will throw.
+     * Deletes the file pointed to by this. If this is a non-empty directory, the operation will throw.
      *
      * @return this.
      * @throws RuntimeException if the file is a directory and not empty.
@@ -129,7 +129,7 @@ public abstract class ApplicationFile implements Comparable<ApplicationFile> {
     public abstract ApplicationFile delete();
 
     /**
-     * Get the path that this file represents.
+     * Gets the path that this file represents.
      *
      * @return a Path
      */
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java
index 9821f3b9568..f434d056bfc 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java
@@ -166,7 +166,8 @@ public final class ApplicationContainerCluster extends ContainerCluster<Applicat
         UserConfiguredFiles files = new UserConfiguredFiles(deployState.getFileRegistry(),
                                                             deployState.getDeployLogger(),
                                                             deployState.featureFlags(),
-                                                            userConfiguredUrls);
+                                                            userConfiguredUrls,
+                                                            deployState.getApplicationPackage());
         for (Component<?, ?> component : getAllComponents()) {
             files.register(component);
         }
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/UserConfiguredFiles.java b/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/UserConfiguredFiles.java
index e4eaa02acd5..8b098f9fc16 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/UserConfiguredFiles.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/UserConfiguredFiles.java
@@ -3,6 +3,8 @@ package com.yahoo.vespa.model.filedistribution;
 
 import com.yahoo.config.FileReference;
 import com.yahoo.config.ModelReference;
+import com.yahoo.config.application.api.ApplicationFile;
+import com.yahoo.config.application.api.ApplicationPackage;
 import com.yahoo.config.application.api.DeployLogger;
 import com.yahoo.config.application.api.FileRegistry;
 import com.yahoo.config.model.api.ModelContext;
@@ -37,14 +39,17 @@ public class UserConfiguredFiles implements Serializable {
     private final DeployLogger logger;
     private final UserConfiguredUrls userConfiguredUrls;
     private final String unknownConfigDefinition;
+    private final ApplicationPackage applicationPackage;
 
     public UserConfiguredFiles(FileRegistry fileRegistry, DeployLogger logger,
                                ModelContext.FeatureFlags featureFlags,
-                               UserConfiguredUrls userConfiguredUrls) {
+                               UserConfiguredUrls userConfiguredUrls,
+                               ApplicationPackage applicationPackage) {
         this.fileRegistry = fileRegistry;
         this.logger = logger;
         this.userConfiguredUrls = userConfiguredUrls;
         this.unknownConfigDefinition = featureFlags.unknownConfigDefinition();
+        this.applicationPackage = applicationPackage;
     }
 
     /**
@@ -156,8 +161,8 @@ public class UserConfiguredFiles implements Serializable {
             path = Path.fromString(builder.getValue());
         }
 
-        File file = path.toFile();
-        if (file.isDirectory() && (file.listFiles() == null || file.listFiles().length == 0))
+        ApplicationFile file = applicationPackage.getFile(path);
+        if (file.isDirectory() && (file.listFiles() == null || file.listFiles().isEmpty()))
             throw new IllegalArgumentException("Directory '" + path.getRelative() + "' is empty");
 
         FileReference reference = registeredFiles.get(path);
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/filedistribution/UserConfiguredFilesTest.java b/config-model/src/test/java/com/yahoo/vespa/model/filedistribution/UserConfiguredFilesTest.java
index 523b0e74be1..92fb89a5c4c 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/filedistribution/UserConfiguredFilesTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/filedistribution/UserConfiguredFilesTest.java
@@ -9,6 +9,7 @@ import com.yahoo.config.application.api.FileRegistry;
 import com.yahoo.config.model.application.provider.BaseDeployLogger;
 import com.yahoo.config.model.deploy.TestProperties;
 import com.yahoo.config.model.producer.UserConfigRepo;
+import com.yahoo.config.model.test.MockApplicationPackage;
 import com.yahoo.config.model.test.MockRoot;
 import com.yahoo.vespa.config.ConfigDefinition;
 import com.yahoo.vespa.config.ConfigDefinitionKey;
@@ -19,6 +20,7 @@ import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.io.TempDir;
 
+import java.io.File;
 import java.nio.ByteBuffer;
 import java.nio.file.Path;
 import java.util.HashMap;
@@ -69,12 +71,23 @@ public class UserConfiguredFilesTest {
         public String toString() { return export().toString(); }
 
     }
-
     private UserConfiguredFiles userConfiguredFiles() {
         return new UserConfiguredFiles(fileRegistry,
                                        new BaseDeployLogger(),
                                        new TestProperties(),
-                                       new ApplicationContainerCluster.UserConfiguredUrls());
+                                       new ApplicationContainerCluster.UserConfiguredUrls(),
+                                       new MockApplicationPackage.Builder().build());
+    }
+
+    private UserConfiguredFiles userConfiguredFiles(File root, com.yahoo.path.Path path) {
+        return new UserConfiguredFiles(fileRegistry,
+                                       new BaseDeployLogger(),
+                                       new TestProperties(),
+                                       new ApplicationContainerCluster.UserConfiguredUrls(),
+                                       new MockApplicationPackage.Builder()
+                                               .withRoot(root)
+                                               .withFiles(Map.of(path, ""))
+                                               .build());
     }
 
     @BeforeEach
@@ -289,13 +302,14 @@ public class UserConfiguredFilesTest {
     }
 
     @Test
-    void require_that_using_empty_dir_gives_sane_error_message(@TempDir Path tempDir) {
-        String relativeTempDir = tempDir.toString().substring(tempDir.toString().lastIndexOf("target"));
+    void require_that_using_empty_dir_fails(@TempDir Path tempDir) {
+        String relativeTempDir = tempDir.toString().substring(tempDir.toString().lastIndexOf("target") + 7);
         try {
             def.addPathDef("pathVal");
             builder.setField("pathVal", relativeTempDir);
             fileRegistry.pathToRef.put(relativeTempDir, new FileReference("bazshash"));
-            userConfiguredFiles().register(producer);
+            userConfiguredFiles(tempDir.toFile().getParentFile(),
+                                com.yahoo.path.Path.fromString(tempDir.toFile().getAbsolutePath())).register(producer);
             fail("Should have thrown exception");
         } catch (IllegalArgumentException e) {
             assertEquals("Invalid config in services.xml for 'mynamespace.myname': Directory '" + relativeTempDir + "' is empty",
@@ -303,4 +317,18 @@ public class UserConfiguredFilesTest {
         }
     }
 
+    @Test
+    void require_that_using_non_existing_dir_fails() {
+        String relativeTempDir = "non-existing";
+        try {
+            def.addPathDef("pathVal");
+            builder.setField("pathVal", relativeTempDir);
+            userConfiguredFiles().register(producer);
+            fail("Should have thrown exception");
+        } catch (IllegalArgumentException e) {
+            assertEquals("Invalid config in services.xml for 'mynamespace.myname': No such file or directory '" + relativeTempDir + "'",
+                         e.getMessage());
+        }
+    }
+
 }