Create file with correct permissions

3 files changed, 26 insertions, 16 deletions

diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileSync.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileSync.java
index 63abce87487..d860cf8595b 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileSync.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileSync.java
@@ -46,8 +46,9 @@ public class FileSync {
     public boolean convergeTo(TaskContext taskContext, PartialFileData partialFileData, boolean atomicWrite) {
         boolean modifiedSystem = false;
 
-        if (partialFileData.getContent().isPresent())
-            modifiedSystem |= convergeTo(taskContext, partialFileData.getContent().get(), atomicWrite);
+        if (partialFileData.getContent().isPresent()) {
+            modifiedSystem |= convergeTo(taskContext, partialFileData.getContent().get(), atomicWrite, partialFileData.getPermissions());
+        }
 
         AttributeSync attributeSync = new AttributeSync(path.toPath()).with(partialFileData);
         modifiedSystem |= attributeSync.converge(taskContext, this.attributesCache);
@@ -60,15 +61,17 @@ public class FileSync {
      *
      * @param atomicWrite Whether to write updates to a temporary file in the same directory, and atomically move it
      *                    to path. Ensures the file cannot be read while in the middle of writing it.
+     * @param permissions Permissions if the file is created.
      * @return true if the content was written. Only modified if necessary (different).
      */
-    public boolean convergeTo(TaskContext taskContext, byte[] content, boolean atomicWrite) {
+    public boolean convergeTo(TaskContext taskContext, byte[] content, boolean atomicWrite, Optional<String> permissions) {
         Optional<Instant> lastModifiedTime = attributesCache.forceGet().map(FileAttributes::lastModifiedTime);
 
         if (lastModifiedTime.isEmpty()) {
-            taskContext.recordSystemModification(logger, "Creating file " + path);
+            taskContext.recordSystemModification(logger, "Creating file " + path +
+                                                         permissions.map(p -> " with permissions " + p).orElse(""));
             path.createParents();
-            writeBytes(content, atomicWrite);
+            writeBytes(content, atomicWrite, permissions);
             contentCache.updateWith(content, attributesCache.forceGet().orElseThrow().lastModifiedTime());
             return true;
         }
@@ -77,20 +80,28 @@ public class FileSync {
             return false;
         } else {
             taskContext.recordSystemModification(logger, "Patching file " + path);
-            writeBytes(content, atomicWrite);
+            // empty permissions here, because the file already exists and won't be applied anyway
+            writeBytes(content, atomicWrite, Optional.empty());
             contentCache.updateWith(content, attributesCache.forceGet().orElseThrow().lastModifiedTime());
             return true;
         }
     }
 
-    private void writeBytes(byte[] content, boolean atomic) {
+    private void writeBytes(byte[] content, boolean atomic, Optional<String> permissions) {
         if (atomic) {
-            String tmpPath = path.toPath().toString() + ".FileSyncTmp";
-            new UnixPath(path.toPath().getFileSystem().getPath(tmpPath))
-                    .writeBytes(content)
-                    .atomicMove(path.toPath());
+            UnixPath tmpPath = new UnixPath(path.toPath().getFileSystem().getPath(path.toPath().toString() + ".FileSyncTmp"));
+            if (permissions.isPresent()) {
+                tmpPath.writeBytes(content, permissions.get());
+            } else {
+                tmpPath.writeBytes(content);
+            }
+            tmpPath.atomicMove(path.toPath());
         } else {
-            path.writeBytes(content);
+            if (permissions.isPresent()) {
+                path.writeBytes(content, permissions.get());
+            } else {
+                path.writeBytes(content);
+            }
         }
     }
 }
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileSyncTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileSyncTest.java
index 5c7becdb9f1..de14168a14e 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileSyncTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileSyncTest.java
@@ -29,10 +29,9 @@ public class FileSyncTest {
 
     @Test
     void trivial() {
-        assertConvergence("Creating file /dir/file.txt",
+        assertConvergence("Creating file /dir/file.txt with permissions rw-r-xr--",
                 "Changing user ID of /dir/file.txt from 1 to 123",
-                "Changing group ID of /dir/file.txt from 2 to 456",
-                "Changing permissions of /dir/file.txt from rw-r--r-- to rw-r-xr--");
+                "Changing group ID of /dir/file.txt from 2 to 456");
 
         content = "new-content";
         assertConvergence("Patching file /dir/file.txt");
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileWriterTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileWriterTest.java
index 03f91c5d48a..159185a2c0c 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileWriterTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileWriterTest.java
@@ -35,7 +35,7 @@ public class FileWriterTest {
                 .withGroupId(group)
                 .onlyIfFileDoesNotAlreadyExist();
         assertTrue(writer.converge(context));
-        verify(context, times(1)).recordSystemModification(any(), eq("Creating file " + path));
+        verify(context, times(1)).recordSystemModification(any(), eq("Creating file " + path + " with permissions rwxr-xr-x"));
 
         UnixPath unixPath = new UnixPath(path);
         assertEquals(content, unixPath.readUtf8File());