Operate on user/group ID in node-admin file utils

10 files changed, 93 insertions, 96 deletions

diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/AttributeSync.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/AttributeSync.java
index b006aa0f404..f3ac4b7912d 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/AttributeSync.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/AttributeSync.java
@@ -22,8 +22,8 @@ public class AttributeSync {
 
     private final UnixPath path;
 
-    private Optional<String> owner = Optional.empty();
-    private Optional<String> group = Optional.empty();
+    private Optional<Integer> ownerId = Optional.empty();
+    private Optional<Integer> groupId = Optional.empty();
     private Optional<String> permissions = Optional.empty();
 
     public AttributeSync(Path path) {
@@ -39,27 +39,27 @@ public class AttributeSync {
         return this;
     }
 
-    public Optional<String> getOwner() {
-        return owner;
+    public Optional<Integer> ownerId() {
+        return ownerId;
     }
 
-    public AttributeSync withOwner(String owner) {
-        this.owner = Optional.of(owner);
+    public AttributeSync withOwnerId(int ownerId) {
+        this.ownerId = Optional.of(ownerId);
         return this;
     }
 
-    public Optional<String> getGroup() {
-        return group;
+    public Optional<Integer> groupId() {
+        return groupId;
     }
 
-    public AttributeSync withGroup(String group) {
-        this.group = Optional.of(group);
+    public AttributeSync withGroupId(int groupId) {
+        this.groupId = Optional.of(groupId);
         return this;
     }
 
     public AttributeSync with(PartialFileData fileData) {
-        owner = fileData.getOwner();
-        group = fileData.getGroup();
+        ownerId = fileData.getOwnerId();
+        groupId = fileData.getGroupId();
         permissions = fileData.getPermissions();
         return this;
     }
@@ -74,17 +74,17 @@ public class AttributeSync {
     public boolean converge(TaskContext context, FileAttributesCache currentAttributes) {
         boolean systemModified = updateAttribute(
                 context,
-                "owner",
-                owner,
-                () -> currentAttributes.getOrThrow().owner(),
-                path::setOwner);
+                "user ID",
+                ownerId,
+                () -> currentAttributes.getOrThrow().ownerId(),
+                path::setOwnerId);
 
         systemModified |= updateAttribute(
                 context,
-                "group",
-                group,
-                () -> currentAttributes.getOrThrow().group(),
-                path::setGroup);
+                "group ID",
+                groupId,
+                () -> currentAttributes.getOrThrow().groupId(),
+                path::setGroupId);
 
         systemModified |= updateAttribute(
                 context,
@@ -96,16 +96,16 @@ public class AttributeSync {
         return systemModified;
     }
 
-    private boolean updateAttribute(TaskContext context,
+    private <T> boolean updateAttribute(TaskContext context,
                                     String attributeName,
-                                    Optional<String> wantedValue,
-                                    Supplier<String> currentValueSupplier,
-                                    Consumer<String> valueSetter) {
+                                    Optional<T> wantedValue,
+                                    Supplier<T> currentValueSupplier,
+                                    Consumer<T> valueSetter) {
         if (wantedValue.isEmpty()) {
             return false;
         }
 
-        String currentValue = currentValueSupplier.get();
+        T currentValue = currentValueSupplier.get();
         if (Objects.equals(currentValue, wantedValue.get())) {
             return false;
         }
@@ -114,7 +114,7 @@ public class AttributeSync {
                 logger,
                 String.format("Changing %s of %s from %s to %s",
                         attributeName,
-                        path.toString(),
+                        path,
                         currentValue,
                         wantedValue.get()));
 
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileAttributes.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileAttributes.java
index 6d4d2f1fad5..ca99d9d99f4 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileAttributes.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileAttributes.java
@@ -1,5 +1,4 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-
 package com.yahoo.vespa.hosted.node.admin.task.util.file;
 
 import java.nio.file.attribute.PosixFileAttributes;
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileWriter.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileWriter.java
index b12179041f6..162a5fbbaec 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileWriter.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileWriter.java
@@ -43,13 +43,13 @@ public class FileWriter {
 
     public Path path() { return path; }
 
-    public FileWriter withOwner(String owner) {
-        fileDataBuilder.withOwner(owner);
+    public FileWriter withOwnerId(int ownerId) {
+        fileDataBuilder.withOwnerId(ownerId);
         return this;
     }
 
-    public FileWriter withGroup(String group) {
-        fileDataBuilder.withGroup(group);
+    public FileWriter withGroupId(int groupId) {
+        fileDataBuilder.withGroupId(groupId);
         return this;
     }
 
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/MakeDirectory.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/MakeDirectory.java
index 6b19493308c..61c4e7475a6 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/MakeDirectory.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/MakeDirectory.java
@@ -34,8 +34,8 @@ public class MakeDirectory {
      */
     public MakeDirectory createParents() { this.createParents = true; return this; }
 
-    public MakeDirectory withOwner(String owner) { attributeSync.withOwner(owner); return this; }
-    public MakeDirectory withGroup(String group) { attributeSync.withGroup(group); return this; }
+    public MakeDirectory withOwnerId(int ownerId) { attributeSync.withOwnerId(ownerId); return this; }
+    public MakeDirectory withGroupId(int groupId) { attributeSync.withGroupId(groupId); return this; }
     public MakeDirectory withPermissions(String permissions) {
         attributeSync.withPermissions(permissions);
         return this;
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/PartialFileData.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/PartialFileData.java
index 311644982c2..2ce663f80da 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/PartialFileData.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/PartialFileData.java
@@ -1,5 +1,4 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-
 package com.yahoo.vespa.hosted.node.admin.task.util.file;
 
 import java.nio.charset.Charset;
@@ -14,8 +13,8 @@ import java.util.Optional;
 // @Immutable
 public class PartialFileData {
     private final Optional<byte[]> content;
-    private final Optional<String> owner;
-    private final Optional<String> group;
+    private final Optional<Integer> ownerId;
+    private final Optional<Integer> groupId;
     private final Optional<String> permissions;
 
     public static Builder builder() {
@@ -23,12 +22,12 @@ public class PartialFileData {
     }
 
     private PartialFileData(Optional<byte[]> content,
-                            Optional<String> owner,
-                            Optional<String> group,
+                            Optional<Integer> ownerId,
+                            Optional<Integer> groupId,
                             Optional<String> permissions) {
         this.content = content;
-        this.owner = owner;
-        this.group = group;
+        this.ownerId = ownerId;
+        this.groupId = groupId;
         this.permissions = permissions;
     }
 
@@ -36,12 +35,12 @@ public class PartialFileData {
         return content;
     }
 
-    public Optional<String> getOwner() {
-        return owner;
+    public Optional<Integer> getOwnerId() {
+        return ownerId;
     }
 
-    public Optional<String> getGroup() {
-        return group;
+    public Optional<Integer> getGroupId() {
+        return groupId;
     }
 
     public Optional<String> getPermissions() {
@@ -50,19 +49,19 @@ public class PartialFileData {
 
     public static class Builder {
         private Optional<byte[]> content = Optional.empty();
-        private Optional<String> owner = Optional.empty();
-        private Optional<String> group = Optional.empty();
+        private Optional<Integer> ownerId = Optional.empty();
+        private Optional<Integer> groupId = Optional.empty();
         private Optional<String> permissions = Optional.empty();
 
         public Builder withContent(byte[] content) { this.content = Optional.of(content); return this; }
         public Builder withContent(String content, Charset charset) { return withContent(content.getBytes(charset)); }
         public Builder withContent(String content) { return withContent(content, StandardCharsets.UTF_8); }
-        public Builder withOwner(String owner) { this.owner = Optional.of(owner); return this; }
-        public Builder withGroup(String group) { this.group = Optional.of(group); return this; }
+        public Builder withOwnerId(int ownerId) { this.ownerId = Optional.of(ownerId); return this; }
+        public Builder withGroupId(int groupId) { this.groupId = Optional.of(groupId); return this; }
         public Builder withPermissions(String permissions) { this.permissions = Optional.of(permissions); return this; }
 
         public PartialFileData create() {
-            return new PartialFileData(content, owner, group, permissions);
+            return new PartialFileData(content, ownerId, groupId, permissions);
         }
     }
 }
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/UnixPath.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/UnixPath.java
index 431ff81dfad..1121db99399 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/UnixPath.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/UnixPath.java
@@ -16,7 +16,6 @@ import java.nio.file.attribute.FileAttribute;
 import java.nio.file.attribute.FileTime;
 import java.nio.file.attribute.GroupPrincipal;
 import java.nio.file.attribute.PosixFileAttributeView;
-import java.nio.file.attribute.PosixFileAttributes;
 import java.nio.file.attribute.PosixFilePermission;
 import java.nio.file.attribute.PosixFilePermissions;
 import java.nio.file.attribute.UserPrincipal;
@@ -57,7 +56,7 @@ public class UnixPath {
         Path filename = path.getFileName();
         if (filename == null) {
             // E.g. "/".
-            throw new IllegalStateException("Path has no filename: '" + path.toString() + "'");
+            throw new IllegalStateException("Path has no filename: '" + path + "'");
         }
 
         return filename.toString();
@@ -129,28 +128,32 @@ public class UnixPath {
         return this;
     }
 
-    public String getOwner() {
-        return getAttributes().owner();
+    public int getOwnerId() {
+        return getAttributes().ownerId();
     }
 
-    public UnixPath setOwner(String owner) {
+    public UnixPath setOwner(String user) { return setOwner(user, "user"); }
+    public UnixPath setOwnerId(int uid) { return setOwner(String.valueOf(uid), "uid"); }
+    private UnixPath setOwner(String owner, String type) {
         UserPrincipalLookupService service = path.getFileSystem().getUserPrincipalLookupService();
         UserPrincipal principal = uncheck(
                 () -> service.lookupPrincipalByName(owner),
-                "While looking up user %s", owner);
+                "While looking up %s %s", type, owner);
         uncheck(() -> Files.setOwner(path, principal));
         return this;
     }
 
-    public String getGroup() {
-        return getAttributes().group();
+    public int getGroupId() {
+        return getAttributes().groupId();
     }
 
-    public UnixPath setGroup(String group) {
+    public UnixPath setGroup(String group) { return setGroup(group, "group"); }
+    public UnixPath setGroupId(int gid) { return setGroup(String.valueOf(gid), "gid"); }
+    public UnixPath setGroup(String group, String type) {
         UserPrincipalLookupService service = path.getFileSystem().getUserPrincipalLookupService();
         GroupPrincipal principal = uncheck(
                 () -> service.lookupPrincipalByGroupName(group),
-                "while looking up group %s", group);
+                "While looking up group %s %s", type, group);
         uncheck(() -> Files.getFileAttributeView(path, PosixFileAttributeView.class).setGroup(principal));
         return this;
     }
@@ -169,9 +172,7 @@ public class UnixPath {
     }
 
     public FileAttributes getAttributes() {
-        PosixFileAttributes attributes = uncheck(() ->
-                Files.getFileAttributeView(path, PosixFileAttributeView.class).readAttributes());
-        return new FileAttributes(attributes);
+        return uncheck(() -> FileAttributes.fromAttributes(Files.readAttributes(path, "unix:*")));
     }
 
     public Optional<FileAttributes> getAttributesIfExists() {
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 36e7482c24e..2355f047b00 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
@@ -1,5 +1,4 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-
 package com.yahoo.vespa.hosted.node.admin.task.util.file;
 
 import com.yahoo.vespa.hosted.node.admin.component.TestTaskContext;
@@ -27,26 +26,25 @@ public class FileSyncTest {
     private final FileSync fileSync = new FileSync(path);
 
     private String content = "content";
-    private String owner = "owner"; // default is user
-    private String group = "group1"; // default is group
+    private int ownerId = 123; // default is 1
+    private int groupId = 456; // default is 2
     private String permissions = "rw-r-xr--";
 
     @Test
     public void trivial() {
         assertConvergence("Creating file /dir/file.txt",
-                "Changing owner of /dir/file.txt from user to owner",
-                "Changing group of /dir/file.txt from group to group1",
+                "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--");
 
         content = "new-content";
         assertConvergence("Patching file /dir/file.txt");
 
-        owner = "new-owner";
-        assertConvergence("Changing owner of /dir/file.txt from owner to " +
-                        owner);
+        ownerId = 124;
+        assertConvergence("Changing user ID of /dir/file.txt from 123 to 124");
 
-        group = "new-group1";
-        assertConvergence("Changing group of /dir/file.txt from group1 to new-group1");
+        groupId = 457;
+        assertConvergence("Changing group ID of /dir/file.txt from 456 to 457");
 
         permissions = "rwxr--rwx";
         assertConvergence("Changing permissions of /dir/file.txt from rw-r-xr-- to " +
@@ -56,8 +54,8 @@ public class FileSyncTest {
     private void assertConvergence(String... systemModificationMessages) {
         PartialFileData fileData = PartialFileData.builder()
                 .withContent(content)
-                .withOwner(owner)
-                .withGroup(group)
+                .withOwnerId(ownerId)
+                .withGroupId(groupId)
                 .withPermissions(permissions)
                 .create();
         taskContext.clearSystemModificationLog();
@@ -65,8 +63,8 @@ public class FileSyncTest {
 
         assertTrue(Files.isRegularFile(path));
         fileData.getContent().ifPresent(content -> assertArrayEquals(content, unixPath.readBytes()));
-        fileData.getOwner().ifPresent(owner -> assertEquals(owner, unixPath.getOwner()));
-        fileData.getGroup().ifPresent(group -> assertEquals(group, unixPath.getGroup()));
+        fileData.getOwnerId().ifPresent(owner -> assertEquals((int) owner, unixPath.getOwnerId()));
+        fileData.getGroupId().ifPresent(group -> assertEquals((int) group, unixPath.getGroupId()));
         fileData.getPermissions().ifPresent(permissions -> assertEquals(permissions, unixPath.getPermissions()));
 
         List<String> actualMods = taskContext.getSystemModificationLog();
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 8d7c3aae354..0f11a4c9a7a 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
@@ -27,14 +27,14 @@ public class FileWriterTest {
     public void testWrite() {
         final String content = "content";
         final String permissions = "rwxr-xr-x";
-        final String owner = "owner";
-        final String group = "group";
+        final int owner = 123;
+        final int group = 456;
 
         Path path = fileSystem.getPath("/opt/vespa/tmp/file.txt");
         FileWriter writer = new FileWriter(path, () -> content)
                 .withPermissions(permissions)
-                .withOwner(owner)
-                .withGroup(group)
+                .withOwnerId(owner)
+                .withGroupId(group)
                 .onlyIfFileDoesNotAlreadyExist();
         assertTrue(writer.converge(context));
         verify(context, times(1)).recordSystemModification(any(), eq("Creating file " + path));
@@ -42,8 +42,8 @@ public class FileWriterTest {
         UnixPath unixPath = new UnixPath(path);
         assertEquals(content, unixPath.readUtf8File());
         assertEquals(permissions, unixPath.getPermissions());
-        assertEquals(owner, unixPath.getOwner());
-        assertEquals(group, unixPath.getGroup());
+        assertEquals(owner, unixPath.getOwnerId());
+        assertEquals(group, unixPath.getGroupId());
         Instant fileTime = unixPath.getLastModifiedTime();
 
         // Second time is a no-op.
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/MakeDirectoryTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/MakeDirectoryTest.java
index b5a4d7475f3..b2c02690443 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/MakeDirectoryTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/MakeDirectoryTest.java
@@ -25,21 +25,21 @@ public class MakeDirectoryTest {
 
     private final String path = "/parent/dir";
     private String permissions = "rwxr----x";
-    private String owner = "test-owner";
-    private String group = "test-group";
+    private int ownerId = 123;
+    private int groupId = 456;
 
     @Test
     public void newDirectory() {
         verifySystemModifications(
                 "Creating directory " + path,
-                "Changing owner of /parent/dir from user to test-owner",
-                "Changing group of /parent/dir from group to test-group");
+                "Changing user ID of /parent/dir from 1 to 123",
+                "Changing group ID of /parent/dir from 2 to 456");
 
-        owner = "new-owner";
-        verifySystemModifications("Changing owner of /parent/dir from test-owner to new-owner");
+        ownerId = 124;
+        verifySystemModifications("Changing user ID of /parent/dir from 123 to 124");
 
-        group = "new-group";
-        verifySystemModifications("Changing group of /parent/dir from test-group to new-group");
+        groupId = 457;
+        verifySystemModifications("Changing group ID of /parent/dir from 456 to 457");
 
         permissions = "--x---r--";
         verifySystemModifications("Changing permissions of /parent/dir from rwxr----x to --x---r--");
@@ -50,8 +50,8 @@ public class MakeDirectoryTest {
         MakeDirectory makeDirectory = new MakeDirectory(fileSystem.getPath(path))
                 .createParents()
                 .withPermissions(permissions)
-                .withOwner(owner)
-                .withGroup(group);
+                .withOwnerId(ownerId)
+                .withGroupId(groupId);
         assertTrue(makeDirectory.converge(context));
 
         assertEquals(List.of(modifications), context.getSystemModificationLog());
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/UnixPathTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/UnixPathTest.java
index 37bd92a8606..952bf04620f 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/UnixPathTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/UnixPathTest.java
@@ -63,11 +63,11 @@ public class UnixPathTest {
         UnixPath unixPath = new UnixPath(path);
         unixPath.writeUtf8File("foo");
 
-        unixPath.setOwner("owner");
-        assertEquals("owner", unixPath.getOwner());
+        unixPath.setOwnerId(123);
+        assertEquals(123, unixPath.getOwnerId());
 
-        unixPath.setGroup("group");
-        assertEquals("group", unixPath.getGroup());
+        unixPath.setGroupId(456);
+        assertEquals(456, unixPath.getGroupId());
     }
 
     @Test