Detect remnants of path.data/default.path.data bug

core/src/main/java/org/elasticsearch/bootstrap/Security.java

@@ -20,7 +20,6 @@
 package org.elasticsearch.bootstrap;
 
 import org.elasticsearch.SecureSM;
-import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.SuppressForbidden;
 import org.elasticsearch.common.io.PathUtils;
 import org.elasticsearch.common.network.NetworkModule;
@@ -45,11 +44,9 @@
 import java.security.Permissions;
 import java.security.Policy;
 import java.security.URIParameter;
-import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.LinkedHashSet;
-import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
@@ -269,6 +266,26 @@ static void addFilePermissions(Permissions policy, Environment environment) {
         for (Path path : environment.dataFiles()) {
             addPath(policy, Environment.PATH_DATA_SETTING.getKey(), path, "read,readlink,write,delete");
         }
+        /*
+         * If path.data and default.path.data are set, we need read access to the paths in default.path.data to check for the existence of
+         * index directories there that could have arisen from a bug in the handling of simultaneous configuration of path.data and
+         * default.path.data that was introduced in Elasticsearch 5.3.0.
+         *
+         * If path.data is not set then default.path.data would take precedence in setting the data paths for the environment and
+         * permissions would have been granted above.
+         *
+         * If path.data is not set and default.path.data is not set, then we would fallback to the default data directory under
+         * Elasticsearch home and again permissions would have been granted above.
+         *
+         * If path.data is set and default.path.data is not set, there is nothing to do here.
+         */
+        if (Environment.PATH_DATA_SETTING.exists(environment.settings())
+                && Environment.DEFAULT_PATH_DATA_SETTING.exists(environment.settings())) {
+            for (final String path : Environment.DEFAULT_PATH_DATA_SETTING.get(environment.settings())) {
+                // write permissions are not needed here, we are not going to be writing to any paths here
+                addPath(policy, Environment.DEFAULT_PATH_DATA_SETTING.getKey(), getPath(path), "read,readlink");
+            }
+        }
         for (Path path : environment.repoFiles()) {
             addPath(policy, Environment.PATH_REPO_SETTING.getKey(), path, "read,readlink,write,delete");
         }
@@ -278,6 +295,11 @@ static void addFilePermissions(Permissions policy, Environment environment) {
         }
     }
 
+    @SuppressForbidden(reason = "read path that is not configured in environment")
+    private static Path getPath(final String path) {
+        return PathUtils.get(path);
+    }
+
     /**
      * Add dynamic {@link SocketPermission}s based on HTTP and transport settings.
      *

core/src/main/java/org/elasticsearch/env/NodeEnvironment.java

@@ -202,7 +202,7 @@ public NodeEnvironment(Settings settings, Environment environment) throws IOExce
                 for (int dirIndex = 0; dirIndex < environment.dataFiles().length; dirIndex++) {
                     Path dataDirWithClusterName = environment.dataWithClusterFiles()[dirIndex];
                     Path dataDir = environment.dataFiles()[dirIndex];
-                    Path dir = dataDir.resolve(NODES_FOLDER).resolve(Integer.toString(possibleLockId));
+                    Path dir = resolveNodePath(dataDir, possibleLockId);
                     Files.createDirectories(dir);
 
                     try (Directory luceneDir = FSDirectory.open(dir, NativeFSLockFactory.INSTANCE)) {
@@ -268,6 +268,17 @@ public NodeEnvironment(Settings settings, Environment environment) throws IOExce
         }
     }
 
+    /**
+     * Resolve a specific nodes/{node.id} path for the specified path and node lock id.
+     *
+     * @param path       the path
+     * @param nodeLockId the node lock id
+     * @return the resolved path
+     */
+    public static Path resolveNodePath(final Path path, final int nodeLockId) {
+        return path.resolve(NODES_FOLDER).resolve(Integer.toString(nodeLockId));
+    }
+
     /** Returns true if the directory is empty */
     private static boolean dirEmpty(final Path path) throws IOException {
         try (DirectoryStream<Path> stream = Files.newDirectoryStream(path)) {
@@ -724,6 +735,14 @@ public NodePath[] nodePaths() {
         return nodePaths;
     }
 
+    public int getNodeLockId() {
+        assertEnvIsLocked();
+        if (nodePaths == null || locks == null) {
+            throw new IllegalStateException("node is not configured to store local location");
+        }
+        return nodeLockId;
+    }
+
     /**
      * Returns all index paths.
      */
@@ -736,6 +755,8 @@ public Path[] indexPaths(Index index) {
         return indexPaths;
     }
 
+
+
     /**
      * Returns all shard paths excluding custom shard path. Note: Shards are only allocated on one of the
      * returned paths. The returned array may contain paths to non-existing directories.
@@ -764,19 +785,36 @@ public Set<String> availableIndexFolders() throws IOException {
         assertEnvIsLocked();
         Set<String> indexFolders = new HashSet<>();
         for (NodePath nodePath : nodePaths) {
-            Path indicesLocation = nodePath.indicesPath;
-            if (Files.isDirectory(indicesLocation)) {
-                try (DirectoryStream<Path> stream = Files.newDirectoryStream(indicesLocation)) {
-                    for (Path index : stream) {
-                        if (Files.isDirectory(index)) {
-                            indexFolders.add(index.getFileName().toString());
-                        }
+            indexFolders.addAll(availableIndexFoldersForPath(nodePath));
+        }
+        return indexFolders;
+
+    }
+
+    /**
+     * Return all directory names in the nodes/{node.id}/indices directory for the given node path.
+     *
+     * @param nodePath the path
+     * @return all directories that could be indices for the given node path.
+     * @throws IOException if an I/O exception occurs traversing the filesystem
+     */
+    public Set<String> availableIndexFoldersForPath(final NodePath nodePath) throws IOException {
+        if (nodePaths == null || locks == null) {
+            throw new IllegalStateException("node is not configured to store local location");
+        }
+        assertEnvIsLocked();
+        final Set<String> indexFolders = new HashSet<>();
+        Path indicesLocation = nodePath.indicesPath;
+        if (Files.isDirectory(indicesLocation)) {
+            try (DirectoryStream<Path> stream = Files.newDirectoryStream(indicesLocation)) {
+                for (Path index : stream) {
+                    if (Files.isDirectory(index)) {
+                        indexFolders.add(index.getFileName().toString());
                     }
                 }
             }
         }
         return indexFolders;
-
     }
 
     /**

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

@@ -50,6 +50,7 @@
 import org.elasticsearch.cluster.routing.allocation.AllocationService;
 import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.StopWatch;
+import org.elasticsearch.common.SuppressForbidden;
 import org.elasticsearch.common.component.Lifecycle;
 import org.elasticsearch.common.component.LifecycleComponent;
 import org.elasticsearch.common.inject.Binder;
@@ -58,6 +59,7 @@
 import org.elasticsearch.common.inject.Module;
 import org.elasticsearch.common.inject.ModulesBuilder;
 import org.elasticsearch.common.inject.util.Providers;
+import org.elasticsearch.common.io.PathUtils;
 import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
 import org.elasticsearch.common.lease.Releasables;
 import org.elasticsearch.common.logging.DeprecationLogger;
@@ -146,7 +148,9 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
+import java.util.Locale;
 import java.util.Map;
+import java.util.Set;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 import java.util.function.Consumer;
@@ -262,6 +266,9 @@ protected Node(final Environment environment, Collection<Class<? extends Plugin>
             Logger logger = Loggers.getLogger(Node.class, tmpSettings);
             final String nodeId = nodeEnvironment.nodeId();
             tmpSettings = addNodeNameIfNeeded(tmpSettings, nodeId);
+            if (DiscoveryNode.nodeRequiresLocalStorage(tmpSettings)) {
+                checkForIndexDataInDefaultPathData(tmpSettings, nodeEnvironment, logger);
+            }
             // this must be captured after the node name is possibly added to the settings
             final String nodeName = NODE_NAME_SETTING.get(tmpSettings);
             if (hadPredefinedNodeName == false) {
@@ -500,6 +507,58 @@ protected Node(final Environment environment, Collection<Class<? extends Plugin>
         }
     }
 
+    /**
+     * Checks for path.data and default.path.data being configured, and there being index data in any of the paths in default.path.data.
+     *
+     * @param settings the settings to check for path.data and default.path.data
+     * @param nodeEnv  the current node environment
+     * @param logger   a logger where messages regarding the detection will be logged
+     * @throws IOException if an I/O exception occurs reading the directory structure
+     */
+    static void checkForIndexDataInDefaultPathData(
+            final Settings settings, final NodeEnvironment nodeEnv, final Logger logger) throws IOException {
+        if (!Environment.PATH_DATA_SETTING.exists(settings) || !Environment.DEFAULT_PATH_DATA_SETTING.exists(settings)) {
+            return;
+        }
+
+        boolean clean = true;
+        for (final String defaultPathData : Environment.DEFAULT_PATH_DATA_SETTING.get(settings)) {
+            final Path nodeDirectory = NodeEnvironment.resolveNodePath(getPath(defaultPathData), nodeEnv.getNodeLockId());
+            if (Files.exists(nodeDirectory) == false) {
+                continue;
+            }
+            final NodeEnvironment.NodePath nodePath = new NodeEnvironment.NodePath(nodeDirectory);
+            final Set<String> availableIndexFolders = nodeEnv.availableIndexFoldersForPath(nodePath);
+            if (availableIndexFolders.isEmpty()) {
+                continue;
+            }
+            clean = false;
+            logger.error("detected index data in default.path.data [{}] where there should not be any", nodePath.indicesPath);
+            for (final String availableIndexFolder : availableIndexFolders) {
+                logger.info(
+                        "index folder [{}] in default.path.data [{}] must be moved to any of {}",
+                        availableIndexFolder,
+                        nodePath.indicesPath,
+                        Arrays.stream(nodeEnv.nodePaths()).map(np -> np.indicesPath).collect(Collectors.toList()));
+            }
+        }
+
+        if (clean) {
+            return;
+        }
+
+        final String message = String.format(
+                Locale.ROOT,
+                "detected index data in default.path.data %s where there should not be any; check the logs for details",
+                Environment.DEFAULT_PATH_DATA_SETTING.get(settings));
+        throw new IllegalStateException(message);
+    }
+
+    @SuppressForbidden(reason = "read path that is not configured in environment")
+    private static Path getPath(final String path) {
+        return PathUtils.get(path);
+    }
+
     // visible for testing
     static void warnIfPreRelease(final Version version, final boolean isSnapshot, final Logger logger) {
         if (!version.isRelease() || isSnapshot) {

test/framework/src/main/java/org/elasticsearch/node/NodeTests.java

@@ -19,31 +19,41 @@
 package org.elasticsearch.node;
 
 import org.apache.logging.log4j.Logger;
+import org.apache.lucene.util.LuceneTestCase;
 import org.elasticsearch.Version;
 import org.elasticsearch.bootstrap.BootstrapCheck;
 import org.elasticsearch.cluster.ClusterName;
+import org.elasticsearch.common.UUIDs;
 import org.elasticsearch.common.network.NetworkModule;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.transport.BoundTransportAddress;
 import org.elasticsearch.env.Environment;
+import org.elasticsearch.env.NodeEnvironment;
 import org.elasticsearch.plugins.Plugin;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.test.InternalTestCluster;
 import org.elasticsearch.transport.MockTcpTransportPlugin;
 
 import java.io.IOException;
+import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
+import java.util.Locale;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
 
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.hasToString;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.reset;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoMoreInteractions;
 
+@LuceneTestCase.SuppressFileSystems(value = "ExtrasFS")
 public class NodeTests extends ESTestCase {
 
     public void testNodeName() throws IOException {
@@ -165,14 +175,81 @@ public void testNodeAttributes() throws IOException {
         }
     }
 
+    public void testDefaultPathDataSet() throws IOException {
+        final Path zero = createTempDir().toAbsolutePath();
+        final Path one = createTempDir().toAbsolutePath();
+        final Path defaultPathData = createTempDir().toAbsolutePath();
+        final Settings settings = Settings.builder()
+                .put("path.home", "/home")
+                .put("path.data.0", zero)
+                .put("path.data.1", one)
+                .put("default.path.data", defaultPathData)
+                .build();
+        try (NodeEnvironment nodeEnv = new NodeEnvironment(settings, new Environment(settings))) {
+            final Path defaultPathDataWithNodesAndId = defaultPathData.resolve("nodes/0");
+            Files.createDirectories(defaultPathDataWithNodesAndId);
+            final NodeEnvironment.NodePath defaultNodePath = new NodeEnvironment.NodePath(defaultPathDataWithNodesAndId);
+            final boolean indexExists = randomBoolean();
+            final List<String> indices;
+            if (indexExists) {
+                indices = IntStream.range(0, randomIntBetween(1, 3)).mapToObj(i -> UUIDs.randomBase64UUID()).collect(Collectors.toList());
+                for (final String index : indices) {
+                    Files.createDirectories(defaultNodePath.indicesPath.resolve(index));
+                }
+            } else {
+                indices = Collections.emptyList();
+            }
+            final Logger mock = mock(Logger.class);
+            if (indexExists) {
+                final IllegalStateException e = expectThrows(
+                        IllegalStateException.class,
+                        () -> Node.checkForIndexDataInDefaultPathData(settings, nodeEnv, mock));
+                final String message = String.format(
+                        Locale.ROOT,
+                        "detected index data in default.path.data [%s] where there should not be any; check the logs for details",
+                        defaultPathData);
+                assertThat(e, hasToString(containsString(message)));
+                verify(mock)
+                        .error("detected index data in default.path.data [{}] where there should not be any", defaultNodePath.indicesPath);
+                for (final String index : indices) {
+                    verify(mock).info(
+                            "index folder [{}] in default.path.data [{}] must be moved to any of {}",
+                            index,
+                            defaultNodePath.indicesPath,
+                            Arrays.stream(nodeEnv.nodePaths()).map(np -> np.indicesPath).collect(Collectors.toList()));
+                }
+                verifyNoMoreInteractions(mock);
+            } else {
+                Node.checkForIndexDataInDefaultPathData(settings, nodeEnv, mock);
+                verifyNoMoreInteractions(mock);
+            }
+        }
+    }
+
+    public void testDefaultPathDataNotSet() throws IOException {
+        final Path zero = createTempDir().toAbsolutePath();
+        final Path one = createTempDir().toAbsolutePath();
+        final Settings settings = Settings.builder()
+                .put("path.home", "/home")
+                .put("path.data.0", zero)
+                .put("path.data.1", one)
+                .build();
+        try (NodeEnvironment nodeEnv = new NodeEnvironment(settings, new Environment(settings))) {
+            final Logger mock = mock(Logger.class);
+            Node.checkForIndexDataInDefaultPathData(settings, nodeEnv, mock);
+            verifyNoMoreInteractions(mock);
+        }
+    }
+
     private static Settings.Builder baseSettings() {
         final Path tempDir = createTempDir();
         return Settings.builder()
-            .put(ClusterName.CLUSTER_NAME_SETTING.getKey(), InternalTestCluster.clusterName("single-node-cluster", randomLong()))
-            .put(Environment.PATH_HOME_SETTING.getKey(), tempDir)
-            .put(NetworkModule.HTTP_ENABLED.getKey(), false)
-            .put("transport.type", "mock-socket-network")
-            .put(Node.NODE_DATA_SETTING.getKey(), true);
+                .put(ClusterName.CLUSTER_NAME_SETTING.getKey(), InternalTestCluster.clusterName("single-node-cluster", randomLong()))
+                .put(Environment.PATH_HOME_SETTING.getKey(), tempDir)
+                .put(NetworkModule.HTTP_ENABLED.getKey(), false)
+                .put("transport.type", "mock-socket-network")
+                .put(Node.NODE_DATA_SETTING.getKey(), true);
     }
 
+
 }