From 220f14eb3ba5c76afcbb794574aa75d5986d0697 Mon Sep 17 00:00:00 2001 From: Tomas Dvorak Date: Thu, 2 Nov 2023 08:36:35 +0100 Subject: [PATCH] Fix opensearch configuration sync permissions (#17059) * Fix opensearch configuration sync permissions * added changelog --- changelog/unreleased/pr-17059.toml | 4 ++++ .../bootstrap/preflight/FullDirSync.java | 21 ++++++++++++++----- .../OpensearchCommandLineProcess.java | 5 ++--- .../management/OpensearchProcessImpl.java | 7 +++---- .../bootstrap/preflight/FullDirSyncTest.java | 18 ++++++++++++++++ 5 files changed, 43 insertions(+), 12 deletions(-) create mode 100644 changelog/unreleased/pr-17059.toml diff --git a/changelog/unreleased/pr-17059.toml b/changelog/unreleased/pr-17059.toml new file mode 100644 index 0000000000..81cbc644bd --- /dev/null +++ b/changelog/unreleased/pr-17059.toml @@ -0,0 +1,4 @@ +type = "f" +message = "Fixed opensearch configuration sync permissions" + +pulls = ["17059"] diff --git a/data-node/src/main/java/org/graylog/datanode/bootstrap/preflight/FullDirSync.java b/data-node/src/main/java/org/graylog/datanode/bootstrap/preflight/FullDirSync.java index 6cbf37c388..9826f73c33 100644 --- a/data-node/src/main/java/org/graylog/datanode/bootstrap/preflight/FullDirSync.java +++ b/data-node/src/main/java/org/graylog/datanode/bootstrap/preflight/FullDirSync.java @@ -27,19 +27,30 @@ import java.nio.file.Path; import java.nio.file.SimpleFileVisitor; import java.nio.file.StandardCopyOption; import java.nio.file.attribute.BasicFileAttributes; +import java.nio.file.attribute.PosixFilePermission; +import java.nio.file.attribute.PosixFilePermissions; import java.util.LinkedList; import java.util.List; +import java.util.Set; public class FullDirSync { private static final Logger LOG = LoggerFactory.getLogger(FullDirSync.class); + /** + * The execute bit for directories means that owner can traverse these and access their content. + */ + protected static final Set DIRECTORY_PERMISSIONS = Set.of(PosixFilePermission.OWNER_WRITE, PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_EXECUTE); + protected static final Set FILE_PERMISSIONS = Set.of(PosixFilePermission.OWNER_WRITE, PosixFilePermission.OWNER_READ); + public static void run(Path source, Path target) throws IOException { final List existingPaths = collectExistingPaths(target); - final List copiedPaths = copyFiles(source, target); - - existingPaths.removeAll(copiedPaths); + existingPaths.remove(target); // exclude the target, we don't want to remove it in following step + deletePaths(existingPaths); + copyFiles(source, target); + } + private static void deletePaths(List existingPaths) throws IOException { for (Path path : existingPaths) { if (Files.isDirectory(path)) { LOG.info("Deleting obsolete directory " + path); @@ -54,7 +65,6 @@ public class FullDirSync { } } - } private static List copyFiles(Path source, Path target) throws IOException { @@ -65,7 +75,7 @@ public class FullDirSync { @Override public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) throws IOException { Path currentTarget = target.resolve(source.relativize(dir).toString()); - Files.createDirectories(currentTarget); + Files.createDirectories(currentTarget, PosixFilePermissions.asFileAttribute((DIRECTORY_PERMISSIONS))); copiedPaths.add(currentTarget); LOG.info("Synchronizing directory {}", currentTarget); return FileVisitResult.CONTINUE; @@ -75,6 +85,7 @@ public class FullDirSync { public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException { final Path currentTarget = target.resolve(source.relativize(file).toString()); Files.copy(file, currentTarget, StandardCopyOption.REPLACE_EXISTING); + Files.setPosixFilePermissions(currentTarget, FILE_PERMISSIONS); copiedPaths.add(currentTarget); LOG.info("Synchronizing file {}", currentTarget); return FileVisitResult.CONTINUE; diff --git a/data-node/src/main/java/org/graylog/datanode/management/OpensearchCommandLineProcess.java b/data-node/src/main/java/org/graylog/datanode/management/OpensearchCommandLineProcess.java index fbc3c7c1d1..48414c456e 100644 --- a/data-node/src/main/java/org/graylog/datanode/management/OpensearchCommandLineProcess.java +++ b/data-node/src/main/java/org/graylog/datanode/management/OpensearchCommandLineProcess.java @@ -42,7 +42,7 @@ public class OpensearchCommandLineProcess implements Closeable { private final CommandLineProcess commandLineProcess; private final CommandLineProcessListener resultHandler; private final ObjectMapper mapper = new ObjectMapper(new YAMLFactory()); - private static final String CONFIG = "opensearch.yml"; + private static final Path CONFIG = Path.of("opensearch.yml"); /** * as long as OpenSearch is not supported on macOS, we have to fix the jdk path if we want to @@ -79,8 +79,7 @@ public class OpensearchCommandLineProcess implements Closeable { private void writeOpenSearchConfig(final OpensearchConfiguration config) { try { - final var configFile = config.datanodeDirectories().getOpensearchProcessConfigurationDir().resolve(CONFIG); - Files.deleteIfExists(configFile); + final Path configFile = config.datanodeDirectories().createOpensearchProcessConfigurationFile(CONFIG); mapper.writeValue(configFile.toFile(), getOpensearchConfigurationArguments(config)); } catch (IOException e) { throw new RuntimeException("Could not generate OpenSearch config: " + e.getMessage(), e); diff --git a/data-node/src/main/java/org/graylog/datanode/management/OpensearchProcessImpl.java b/data-node/src/main/java/org/graylog/datanode/management/OpensearchProcessImpl.java index fab6275f8a..f05088cffb 100644 --- a/data-node/src/main/java/org/graylog/datanode/management/OpensearchProcessImpl.java +++ b/data-node/src/main/java/org/graylog/datanode/management/OpensearchProcessImpl.java @@ -22,7 +22,6 @@ import org.apache.commons.exec.ExecuteException; import org.apache.http.client.utils.URIBuilder; import org.graylog.datanode.Configuration; import org.graylog.datanode.configuration.DatanodeConfiguration; -import org.graylog.datanode.configuration.DatanodeConfigurationProvider; import org.graylog.datanode.process.OpensearchConfiguration; import org.graylog.datanode.process.OpensearchInfo; import org.graylog.datanode.process.ProcessEvent; @@ -53,6 +52,7 @@ import java.util.stream.Collectors; class OpensearchProcessImpl implements OpensearchProcess, ProcessListener { private static final Logger LOG = LoggerFactory.getLogger(OpensearchProcessImpl.class); + public static final Path UNICAST_HOSTS_FILE = Path.of("unicast_hosts.txt"); private final StateMachineTracerAggregator tracerAggregator; @SuppressWarnings("OptionalUsedAsFieldOrParameterType") @@ -70,7 +70,6 @@ class OpensearchProcessImpl implements OpensearchProcess, ProcessListener { private final Queue stdout; private final Queue stderr; private final CustomCAX509TrustManager trustManager; - private final Path hostsfile; private final NodeService nodeService; private final Configuration configuration; @@ -85,7 +84,6 @@ class OpensearchProcessImpl implements OpensearchProcess, ProcessListener { this.trustManager = trustManager; this.nodeService = nodeService; this.configuration = configuration; - this.hostsfile = datanodeConfiguration.datanodeDirectories().getOpensearchProcessConfigurationDir().resolve("unicast_hosts.txt"); } private RestHighLevelClient createRestClient(OpensearchConfiguration configuration) { @@ -157,10 +155,11 @@ class OpensearchProcessImpl implements OpensearchProcess, ProcessListener { private void writeSeedHostsList() { try { + final Path hostsfile = datanodeConfiguration.datanodeDirectories().createOpensearchProcessConfigurationFile(UNICAST_HOSTS_FILE); final Set current = nodeService.allActive(Node.Type.DATANODE).values().stream().map(Node::getClusterAddress).filter(Objects::nonNull).collect(Collectors.toSet()); Files.write(hostsfile, current, Charset.defaultCharset(), StandardOpenOption.WRITE, StandardOpenOption.TRUNCATE_EXISTING, StandardOpenOption.CREATE); } catch (IOException iox) { - LOG.error("Could not write to file: {} - {}", hostsfile, iox.getMessage()); + LOG.error("Could not write to file: {} - {}", UNICAST_HOSTS_FILE, iox.getMessage()); } } diff --git a/data-node/src/test/java/org/graylog/datanode/bootstrap/preflight/FullDirSyncTest.java b/data-node/src/test/java/org/graylog/datanode/bootstrap/preflight/FullDirSyncTest.java index cbaa5deeff..0f31b18ca3 100644 --- a/data-node/src/test/java/org/graylog/datanode/bootstrap/preflight/FullDirSyncTest.java +++ b/data-node/src/test/java/org/graylog/datanode/bootstrap/preflight/FullDirSyncTest.java @@ -27,8 +27,10 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.SimpleFileVisitor; import java.nio.file.attribute.BasicFileAttributes; +import java.nio.file.attribute.PosixFilePermission; import java.util.ArrayList; import java.util.List; +import java.util.Set; import static org.junit.jupiter.api.Assertions.*; @@ -102,5 +104,21 @@ class FullDirSyncTest { .hasSize(4) .contains("a.txt", "b.txt", "subdir", "subdir/c.txt"); + Assertions.assertThat(afterSyncState) + .filteredOn(Files::isDirectory) + .allSatisfy(p -> { + final Set permission = Files.getPosixFilePermissions(p); + Assertions.assertThat(permission).containsExactlyInAnyOrderElementsOf(FullDirSync.DIRECTORY_PERMISSIONS); + }); + + Assertions.assertThat(afterSyncState) + .filteredOn(Files::isRegularFile) + .allSatisfy(p -> { + final Set permission = Files.getPosixFilePermissions(p); + Assertions.assertThat(permission).containsExactlyInAnyOrderElementsOf(FullDirSync.FILE_PERMISSIONS); + }); + + + } }