honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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

In Elasticsearch 5.3.0 a bug was introduced in the merging of default
settings when the target setting existed as an array. When this bug
concerns path.data and default.path.data, we ended up in a situation
where the paths specified in both settings would be used to write index
data. Since our packaging sets default.path.data, users that configure
multiple data paths via an array and use the packaging are subject to
having shards land in paths in default.path.data when that is very
likely not what they intended.

This commit is an attempt to rectify this situation. If path.data and
default.path.data are configured, we check for the presence of indices
there. If we find any, we log messages explaining the situation and fail
the node.

Relates #24099
This commit is contained in:
Jason Tedor 2017-04-17 07:03:46 -04:00 committed by GitHub
parent a8be0a5836
commit 8033c576b7
4 changed files with 213 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
core/src/main/java/org/elasticsearch/bootstrap/Security.java
View File

@ -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.NoSuchAlgorithmException;
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 @@ final class Security {
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 @@ final class Security {
}
}
@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.
*

44
core/src/main/java/org/elasticsearch/env/NodeEnvironment.java vendored
View File

@ -202,7 +202,7 @@ public final class NodeEnvironment implements Closeable {
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 final class NodeEnvironment implements Closeable {
}
}
/**
* 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 final class NodeEnvironment implements Closeable {
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 final class NodeEnvironment implements Closeable {
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,6 +785,25 @@ public final class NodeEnvironment implements Closeable {
assertEnvIsLocked();
Set<String> indexFolders = new HashSet<>();
for (NodePath nodePath : nodePaths) {
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)) {
@ -774,9 +814,7 @@ public final class NodeEnvironment implements Closeable {
}
}
}
}
return indexFolders;
}
/**

59
core/src/main/java/org/elasticsearch/node/Node.java
View File

@ -50,6 +50,7 @@ import org.elasticsearch.cluster.routing.RoutingService;
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.Key;
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.Arrays;
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 @@ public class Node implements Closeable {
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 @@ public class Node implements Closeable {
}
}
/**
* 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) {

77
test/framework/src/main/java/org/elasticsearch/node/NodeTests.java
View File

@ -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,6 +175,72 @@ public class NodeTests extends ESTestCase {
}
}
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()
@ -175,4 +251,5 @@ public class NodeTests extends ESTestCase {
.put(Node.NODE_DATA_SETTING.getKey(), true);
}
}