Convert path.data to String setting instead of List (#72282)

Since multiple data path support has been removed, the Setting no longer
needs to support multiple values. This commit converts the
PATH_DATA_SETTING to a String setting from List<String>.

relates #71205

benchmarks/src/main/java/org/elasticsearch/benchmark/fs/AvailableIndexFoldersBenchmark.java

@@ -44,13 +44,12 @@ public class AvailableIndexFoldersBenchmark {
     @Setup
     public void setup() throws IOException {
         Path path = Files.createTempDirectory("test");
-        String[] paths = new String[] { path.toString() };
         nodePath = new NodeEnvironment.NodePath(path);
 
         LogConfigurator.setNodeName("test");
         Settings settings = Settings.builder()
             .put(Environment.PATH_HOME_SETTING.getKey(), path)
-            .putList(Environment.PATH_DATA_SETTING.getKey(), paths)
+            .put(Environment.PATH_DATA_SETTING.getKey(), path.resolve("data"))
             .build();
         nodeEnv = new NodeEnvironment(settings, new Environment(settings, null));
 

plugins/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureRepositorySettingsTests.java

@@ -30,7 +30,7 @@ public class AzureRepositorySettingsTests extends ESTestCase {
     private AzureRepository azureRepository(Settings settings) {
         Settings internalSettings = Settings.builder()
             .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toAbsolutePath())
-            .putList(Environment.PATH_DATA_SETTING.getKey(), tmpPaths())
+            .put(Environment.PATH_DATA_SETTING.getKey(), createTempDir().toAbsolutePath())
             .put(settings)
             .build();
         final AzureRepository azureRepository = new AzureRepository(new RepositoryMetadata("foo", "azure", internalSettings),

qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/EvilSecurityTests.java

@@ -24,9 +24,6 @@
 import java.security.Permissions;
 import java.util.Set;
 
-import static org.hamcrest.Matchers.containsString;
-import static org.hamcrest.Matchers.hasToString;
-
 @SuppressForbidden(reason = "modifies system properties and attempts to create symbolic links intentionally")
 public class EvilSecurityTests extends ESTestCase {
 
@@ -70,8 +67,7 @@ public void testEnvironmentPaths() throws Exception {
 
         Settings.Builder settingsBuilder = Settings.builder();
         settingsBuilder.put(Environment.PATH_HOME_SETTING.getKey(), esHome.resolve("home").toString());
-        settingsBuilder.putList(Environment.PATH_DATA_SETTING.getKey(), esHome.resolve("data1").toString(),
-                esHome.resolve("data2").toString());
+        settingsBuilder.put(Environment.PATH_DATA_SETTING.getKey(), esHome.resolve("data1").toString());
         settingsBuilder.put(Environment.PATH_SHARED_DATA_SETTING.getKey(), esHome.resolve("custom").toString());
         settingsBuilder.put(Environment.PATH_LOGS_SETTING.getKey(), esHome.resolve("logs").toString());
         settingsBuilder.put(Environment.NODE_PIDFILE_SETTING.getKey(), esHome.resolve("test.pid").toString());
@@ -124,31 +120,6 @@ public void testEnvironmentPaths() throws Exception {
         assertExactPermissions(new FilePermission(environment.pidFile().toString(), "delete"), permissions);
     }
 
-    public void testDuplicateDataPaths() throws IOException {
-        assumeFalse("https://github.com/elastic/elasticsearch/issues/44558", Constants.WINDOWS);
-        final Path path = createTempDir();
-        final Path home = path.resolve("home");
-        final Path data = path.resolve("data");
-        final Path duplicate;
-        if (randomBoolean()) {
-            duplicate = data;
-        } else {
-            duplicate = createTempDir().toAbsolutePath().resolve("link");
-            Files.createSymbolicLink(duplicate, data);
-        }
-
-        final Settings settings =
-                Settings
-                        .builder()
-                        .put(Environment.PATH_HOME_SETTING.getKey(), home.toString())
-                        .putList(Environment.PATH_DATA_SETTING.getKey(), data.toString(), duplicate.toString())
-                        .build();
-
-        final Environment environment = TestEnvironment.newEnvironment(settings);
-        final IllegalStateException e = expectThrows(IllegalStateException.class, () -> Security.createPermissions(environment));
-        assertThat(e, hasToString(containsString("path [" + duplicate.toRealPath() + "] is duplicated by [" + duplicate + "]")));
-    }
-
     public void testEnsureSymlink() throws IOException {
         Path p = createTempDir();
 

qa/evil-tests/src/test/java/org/elasticsearch/env/NodeEnvironmentEvilTests.java

@@ -7,7 +7,6 @@
  */
 package org.elasticsearch.env;
 
-import org.elasticsearch.common.io.PathUtils;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.test.PosixPermissionsResetter;
@@ -32,14 +31,13 @@ public static void checkPosix() throws IOException {
 
     public void testMissingWritePermission() throws IOException {
         assumeTrue("posix filesystem", isPosix);
-        final String[] tempPaths = tmpPaths();
-        Path path = PathUtils.get(randomFrom(tempPaths));
+        Path path = createTempDir();
         try (PosixPermissionsResetter attr = new PosixPermissionsResetter(path)) {
             attr.setPermissions(new HashSet<>(Arrays.asList(PosixFilePermission.OTHERS_READ, PosixFilePermission.GROUP_READ,
                 PosixFilePermission.OWNER_READ)));
             Settings build = Settings.builder()
                     .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toAbsolutePath().toString())
-                    .putList(Environment.PATH_DATA_SETTING.getKey(), tempPaths).build();
+                    .put(Environment.PATH_DATA_SETTING.getKey(), path).build();
             IllegalStateException exception = expectThrows(IllegalStateException.class, () -> {
                 new NodeEnvironment(build, TestEnvironment.newEnvironment(build));
             });
@@ -50,8 +48,7 @@ public void testMissingWritePermission() throws IOException {
 
     public void testMissingWritePermissionOnIndex() throws IOException {
         assumeTrue("posix filesystem", isPosix);
-        final String[] tempPaths = tmpPaths();
-        Path path = PathUtils.get(randomFrom(tempPaths));
+        Path path = createTempDir();
         Path fooIndex = path.resolve(NodeEnvironment.INDICES_FOLDER)
             .resolve("foo");
         Files.createDirectories(fooIndex);
@@ -60,7 +57,7 @@ public void testMissingWritePermissionOnIndex() throws IOException {
                 PosixFilePermission.OWNER_READ)));
             Settings build = Settings.builder()
                 .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toAbsolutePath().toString())
-                .putList(Environment.PATH_DATA_SETTING.getKey(), tempPaths).build();
+                .put(Environment.PATH_DATA_SETTING.getKey(), path).build();
             IOException ioException = expectThrows(IOException.class, () -> {
                 new NodeEnvironment(build, TestEnvironment.newEnvironment(build));
             });
@@ -70,8 +67,7 @@ public void testMissingWritePermissionOnIndex() throws IOException {
 
     public void testMissingWritePermissionOnShard() throws IOException {
         assumeTrue("posix filesystem", isPosix);
-        final String[] tempPaths = tmpPaths();
-        Path path = PathUtils.get(randomFrom(tempPaths));
+        Path path = createTempDir();
         Path fooIndex = path.resolve(NodeEnvironment.INDICES_FOLDER)
             .resolve("foo");
         Path fooShard = fooIndex.resolve("0");
@@ -85,7 +81,7 @@ public void testMissingWritePermissionOnShard() throws IOException {
                 PosixFilePermission.OWNER_READ)));
             Settings build = Settings.builder()
                 .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toAbsolutePath().toString())
-                .putList(Environment.PATH_DATA_SETTING.getKey(), tempPaths).build();
+                .put(Environment.PATH_DATA_SETTING.getKey(), path).build();
             IOException ioException = expectThrows(IOException.class, () -> {
                 new NodeEnvironment(build, TestEnvironment.newEnvironment(build));
             });

server/src/internalClusterTest/java/org/elasticsearch/env/NodeEnvironmentIT.java

@@ -28,7 +28,6 @@
 import java.nio.file.StandardCopyOption;
 import java.util.List;
 import java.util.Set;
-import java.util.stream.Collectors;
 
 import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery;
 import static org.elasticsearch.test.NodeRoles.nonDataNode;
@@ -137,8 +136,7 @@ public void testUpgradeDataFolder() throws IOException, InterruptedException {
         internalCluster().stopRandomDataNode();
 
         // simulate older data path layout by moving data under "nodes/0" folder
-        final List<Path> dataPaths = Environment.PATH_DATA_SETTING.get(dataPathSettings)
-            .stream().map(PathUtils::get).collect(Collectors.toList());
+        final List<Path> dataPaths = List.of(PathUtils.get(Environment.PATH_DATA_SETTING.get(dataPathSettings)));
         dataPaths.forEach(path -> {
                 final Path targetPath = path.resolve("nodes").resolve("0");
                 try {

server/src/internalClusterTest/java/org/elasticsearch/plugins/IndexFoldersDeletionListenerIT.java

@@ -235,7 +235,7 @@ public void testListenersInvokedWhenIndexHasLeftOverShard() throws Exception {
         final Path dataDirWithLeftOverShards = createTempDir();
         String dataNode = internalCluster().startDataOnlyNode(
             Settings.builder()
-                .putList(Environment.PATH_DATA_SETTING.getKey(), List.of(dataDirWithLeftOverShards.toAbsolutePath().toString()))
+                .put(Environment.PATH_DATA_SETTING.getKey(), dataDirWithLeftOverShards.toAbsolutePath().toString())
                 .putNull(Environment.PATH_SHARED_DATA_SETTING.getKey())
                 .build()
         );

server/src/main/java/org/elasticsearch/cluster/coordination/JoinHelper.java

@@ -123,7 +123,7 @@ public ClusterTasksResult<JoinTaskExecutor.Task> execute(ClusterState currentSta
                 channel.sendResponse(Empty.INSTANCE);
             });
 
-        final List<String> dataPaths = Environment.PATH_DATA_SETTING.get(settings);
+        final String dataPath = Environment.PATH_DATA_SETTING.get(settings);
         transportService.registerRequestHandler(VALIDATE_JOIN_ACTION_NAME,
             ThreadPool.Names.GENERIC, ValidateJoinRequest::new,
             (request, channel, task) -> {
@@ -134,9 +134,8 @@ public ClusterTasksResult<JoinTaskExecutor.Task> execute(ClusterState currentSta
                             localState.metadata().clusterUUID() + "] and is now trying to join a different cluster with UUID [" +
                             request.getState().metadata().clusterUUID() + "]. This is forbidden and usually indicates an incorrect " +
                             "discovery or cluster bootstrapping configuration. Note that the cluster UUID persists across restarts and " +
-                            "can only be changed by deleting the contents of the node's data " +
-                            (dataPaths.size() == 1 ? "path " : "paths ") + dataPaths +
-                            " which will also remove any data held by this node.");
+                            "can only be changed by deleting the contents of the node's data path [" + dataPath +
+                            "] which will also remove any data held by this node.");
                 }
                 joinValidators.forEach(action -> action.accept(transportService.getLocalNode(), request.getState()));
                 channel.sendResponse(Empty.INSTANCE);

server/src/main/java/org/elasticsearch/env/Environment.java

@@ -40,8 +40,8 @@ public class Environment {
     private static final Path[] EMPTY_PATH_ARRAY = new Path[0];
 
     public static final Setting<String> PATH_HOME_SETTING = Setting.simpleString("path.home", Property.NodeScope);
-    public static final Setting<List<String>> PATH_DATA_SETTING =
-            Setting.listSetting("path.data", Collections.emptyList(), Function.identity(), Property.NodeScope);
+    public static final Setting<String> PATH_DATA_SETTING =
+            Setting.simpleString("path.data", Property.NodeScope);
     public static final Setting<String> PATH_LOGS_SETTING =
             new Setting<>("path.logs", "", Function.identity(), Property.NodeScope);
     public static final Setting<List<String>> PATH_REPO_SETTING =
@@ -51,7 +51,7 @@ public class Environment {
 
     private final Settings settings;
 
-    private final Path[] dataFiles;
+    private final Path dataFile;
 
     private final Path[] repoFiles;
 
@@ -100,14 +100,10 @@ public Environment(final Settings settings, final Path configPath) {
 
         pluginsFile = homeFile.resolve("plugins");
 
-        List<String> dataPaths = PATH_DATA_SETTING.get(settings);
-        if (dataPaths.isEmpty() == false) {
-            dataFiles = new Path[dataPaths.size()];
-            for (int i = 0; i < dataPaths.size(); i++) {
-                dataFiles[i] = PathUtils.get(dataPaths.get(i)).toAbsolutePath().normalize();
-            }
+        if (PATH_DATA_SETTING.exists(settings)) {
+            dataFile = PathUtils.get(PATH_DATA_SETTING.get(settings)).toAbsolutePath().normalize();
         } else {
-            dataFiles = new Path[]{homeFile.resolve("data")};
+            dataFile = homeFile.resolve("data");
         }
         if (PATH_SHARED_DATA_SETTING.exists(settings)) {
             sharedDataFile = PathUtils.get(PATH_SHARED_DATA_SETTING.get(settings)).toAbsolutePath().normalize();
@@ -143,12 +139,7 @@ public Environment(final Settings settings, final Path configPath) {
 
         final Settings.Builder finalSettings = Settings.builder().put(settings);
         if (PATH_DATA_SETTING.exists(settings)) {
-            if (dataFiles.length == 1) {
-                finalSettings.put(PATH_DATA_SETTING.getKey(), dataFiles[0].toString());
-            } else {
-                finalSettings.putList(PATH_DATA_SETTING.getKey(),
-                    Arrays.stream(dataFiles).map(Path::toString).collect(Collectors.toList()));
-            }
+            finalSettings.put(PATH_DATA_SETTING.getKey(), dataFile.toString());
         }
         finalSettings.put(PATH_HOME_SETTING.getKey(), homeFile);
         finalSettings.put(PATH_LOGS_SETTING.getKey(), logsFile.toString());
@@ -179,7 +170,7 @@ public Settings settings() {
      * The data location.
      */
     public Path[] dataFiles() {
-        return dataFiles;
+        return new Path[] { dataFile };
     }
 
     /**

server/src/main/java/org/elasticsearch/node/Node.java

@@ -306,9 +306,7 @@ protected Node(final Environment initialEnvironment,
                     Build.CURRENT.getQualifiedVersion());
             }
 
-            if (initialEnvironment.dataFiles().length > 1) {
-                throw new IllegalArgumentException("Multiple [path.data] values found. Specify a single data path.");
-            } else if (Environment.dataPathUsesList(tmpSettings)) {
+            if (Environment.dataPathUsesList(tmpSettings)) {
                 throw new IllegalArgumentException("[path.data] is a list. Specify as a string value.");
             }
 

server/src/test/java/org/elasticsearch/env/EnvironmentTests.java

@@ -140,9 +140,8 @@ public void testPathNormalization() throws IOException {
 
         final Path home = PathUtils.get(homePath);
 
-        final List<String> dataPaths = Environment.PATH_DATA_SETTING.get(environment.settings());
-        assertThat(dataPaths, hasSize(1));
-        assertPath(dataPaths.get(0), home.resolve("data"));
+        final String dataPath = Environment.PATH_DATA_SETTING.get(environment.settings());
+        assertPath(dataPath, home.resolve("data"));
 
         final String logPath = Environment.PATH_LOGS_SETTING.get(environment.settings());
         assertPath(logPath, home.resolve("logs"));