summaryrefslogtreecommitdiffstats
path: root/node-admin
diff options
context:
space:
mode:
authorValerij Fredriksen <freva@users.noreply.github.com>2022-04-21 13:01:33 +0200
committerGitHub <noreply@github.com>2022-04-21 13:01:33 +0200
commit26d7ba296e48d59647c0fc1567295ede87cdc0a3 (patch)
tree24cbbec6414e3bbdaaa30eba71de31257e9c12f4 /node-admin
parent888beccd43a3bb14febb0c2d2838e0dda852593c (diff)
Revert "Reapply "Set default permissions""
Diffstat (limited to 'node-admin')
-rw-r--r--node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java28
-rw-r--r--node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystemProvider.java25
-rw-r--r--node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystemTest.java24
3 files changed, 2 insertions, 75 deletions
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java
index 75977da369c..61e777a9576 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java
@@ -27,9 +27,6 @@ import com.yahoo.vespa.hosted.node.admin.maintenance.acl.AclMaintainer;
import com.yahoo.vespa.hosted.node.admin.maintenance.identity.CredentialsMaintainer;
import com.yahoo.vespa.hosted.node.admin.maintenance.servicedump.VespaServiceDumper;
import com.yahoo.vespa.hosted.node.admin.nodeadmin.ConvergenceException;
-import com.yahoo.vespa.hosted.node.admin.task.util.file.FileFinder;
-import com.yahoo.vespa.hosted.node.admin.task.util.file.UnixPath;
-import com.yahoo.vespa.hosted.node.admin.task.util.fs.ContainerPath;
import java.time.Clock;
import java.time.Duration;
@@ -140,31 +137,6 @@ public class NodeAgentImpl implements NodeAgent {
if (loopThread != null)
throw new IllegalStateException("Can not re-start a node agent.");
- // TODO: Remove after this has rolled out everywhere
- int[] stats = new int[]{0, 0, 0};
- ContainerPath vespaHome = initialContext.paths().underVespaHome("");
- FileFinder.files(initialContext.paths().of("/")).forEachPath(path -> {
- UnixPath unixPath = new UnixPath(path);
-
- String permissions = unixPath.getPermissions();
- if (!permissions.endsWith("---")) {
- unixPath.setPermissions(permissions.substring(0, 6) + "---");
- stats[0]++;
- }
-
- if (path.startsWith(vespaHome) && unixPath.getOwnerId() != initialContext.users().vespa().uid()) {
- unixPath.setOwnerId(initialContext.users().vespa().uid());
- stats[1]++;
- }
-
- if (path.startsWith(vespaHome) && unixPath.getGroupId() != initialContext.users().vespa().gid()) {
- unixPath.setGroupId(initialContext.users().vespa().gid());
- stats[2]++;
- }
- });
- if (stats[0] + stats[1] + stats[2] > 0)
- initialContext.log(logger, "chmod %d, chown UID %d, chown GID %d files", stats[0], stats[1], stats[2]);
-
loopThread = new Thread(() -> {
while (!terminated.get()) {
try {
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystemProvider.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystemProvider.java
index 2a2e3d611c9..964ed5e0e4d 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystemProvider.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystemProvider.java
@@ -26,8 +26,6 @@ import java.nio.file.attribute.FileAttributeView;
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;
import java.nio.file.spi.FileSystemProvider;
import java.util.HashMap;
@@ -46,12 +44,6 @@ import static com.yahoo.yolean.Exceptions.uncheck;
* @author freva
*/
class ContainerFileSystemProvider extends FileSystemProvider {
-
- private static final FileAttribute<?> DEFAULT_FILE_PERMISSIONS = PosixFilePermissions.asFileAttribute(Set.of( // 0640
- PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE, PosixFilePermission.GROUP_READ));
- private static final FileAttribute<?> DEFAULT_DIRECTORY_PERMISSIONS = PosixFilePermissions.asFileAttribute(Set.of( // 0750
- PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE, PosixFilePermission.OWNER_EXECUTE, PosixFilePermission.GROUP_READ, PosixFilePermission.GROUP_EXECUTE));
-
private final ContainerFileSystem containerFs;
private final ContainerUserPrincipalLookupService userPrincipalLookupService;
@@ -90,8 +82,7 @@ class ContainerFileSystemProvider extends FileSystemProvider {
Path pathOnHost = pathOnHost(path);
try (SecureDirectoryStream<Path> sds = leafDirectoryStream(pathOnHost)) {
boolean existedBefore = Files.exists(pathOnHost);
- SeekableByteChannel seekableByteChannel = sds.newByteChannel(
- pathOnHost.getFileName(), addNoFollow(options), addPermissions(DEFAULT_FILE_PERMISSIONS, attrs));
+ SeekableByteChannel seekableByteChannel = sds.newByteChannel(pathOnHost.getFileName(), addNoFollow(options), attrs);
if (!existedBefore) fixOwnerToContainerRoot(toContainerPath(path));
return seekableByteChannel;
}
@@ -108,7 +99,7 @@ class ContainerFileSystemProvider extends FileSystemProvider {
public void createDirectory(Path dir, FileAttribute<?>... attrs) throws IOException {
Path pathOnHost = pathOnHost(dir);
boolean existedBefore = Files.exists(pathOnHost);
- provider(pathOnHost).createDirectory(pathOnHost, addPermissions(DEFAULT_DIRECTORY_PERMISSIONS, attrs));
+ provider(pathOnHost).createDirectory(pathOnHost);
if (!existedBefore) fixOwnerToContainerRoot(toContainerPath(dir));
}
@@ -333,16 +324,4 @@ class ContainerFileSystemProvider extends FileSystemProvider {
copy[options.length] = LinkOption.NOFOLLOW_LINKS;
return copy;
}
-
- private static FileAttribute<?>[] addPermissions(FileAttribute<?> defaultPermissions, FileAttribute<?>... attrs) {
- for (FileAttribute<?> attr : attrs) {
- if (attr.name().equals("posix:permissions") || attr.name().equals("unix:permissions"))
- return attrs;
- }
-
- FileAttribute<?>[] copy = new FileAttribute<?>[attrs.length + 1];
- System.arraycopy(attrs, 0, copy, 0, attrs.length);
- copy[attrs.length] = defaultPermissions;
- return copy;
- }
}
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystemTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystemTest.java
index b26f0fe5bf8..c3affccc32b 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystemTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystemTest.java
@@ -16,8 +16,6 @@ import java.nio.file.LinkOption;
import java.nio.file.Path;
import java.nio.file.StandardCopyOption;
import java.nio.file.StandardOpenOption;
-import java.nio.file.attribute.FileAttribute;
-import java.nio.file.attribute.PosixFilePermissions;
import java.util.Map;
import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -176,19 +174,6 @@ class ContainerFileSystemTest {
Files.writeString(file.pathOnHost(), "hello"); // Writing through host FS works
}
- @Test
- public void permissions() throws IOException {
- assertPermissions(Files.createDirectory(containerFs.getPath("/dir1")), "rwxr-x---");
- assertPermissions(Files.createDirectory(containerFs.getPath("/dir2"), permissionsFromString("r-x-w-rw-")), "r-x-w-rw-");
-
- assertPermissions(Files.createDirectories(containerFs.getPath("/sub/dir/leaf"), permissionsFromString("r-x-w-rw-")), "r-x-w-rw-");
- assertPermissions(containerFs.getPath("/sub/dir"), "r-x-w-rw-"); // Non-leafs get the same permission as the leaf
-
- // TODO: Uncomment when JimFS forwards attributes for SecureDirectoryStream::newByteChannel
-// assertPermissions(Files.createFile(containerFs.getPath("/file1")), "rw-r-----");
-// assertPermissions(Files.createFile(containerFs.getPath("/file2"), permissionsFromString("r-x-w-rw-")), "r-x-w-rw-");
- }
-
private static void assertOwnership(ContainerPath path, int contUid, int contGid, int hostUid, int hostGid) throws IOException {
assertOwnership(path, contUid, contGid);
assertOwnership(path.pathOnHost(), hostUid, hostGid);
@@ -199,13 +184,4 @@ class ContainerFileSystemTest {
assertEquals(uid, attrs.get("uid"));
assertEquals(gid, attrs.get("gid"));
}
-
- private static void assertPermissions(Path path, String expected) throws IOException {
- String actual = PosixFilePermissions.toString(Files.getPosixFilePermissions(path));
- assertEquals(expected, actual);
- }
-
- private static FileAttribute<?> permissionsFromString(String permissions) {
- return PosixFilePermissions.asFileAttribute(PosixFilePermissions.fromString(permissions));
- }
}