Normalize environment paths (#45179)

This commit applies a normalization process to environment paths, both
in how they are stored internally, also their settings values. This
normalization is done via two means:
 - we make the paths absolute
 - we remove redundant name elements from the path (what Java calls
   "normalization")

This change ensures that when we compare and refer to these paths within
the system, we are using a common ground. For example, prior to the
change if the data path was relative, we would not compare it correctly
to paths from disk usage. This is because the paths in disk usage were
being made absolute.
This commit is contained in:
Jason Tedor 2019-08-06 06:02:28 -04:00
parent 7aeb2fe73c
commit 5b1b146099
No known key found for this signature in database
GPG Key ID: FA89F05560F16BC5
10 changed files with 103 additions and 32 deletions

View File

@ -712,7 +712,7 @@ class InstallPluginCommand extends EnvironmentAwareCommand {
Locale.ROOT,
"plugin directory [%s] already exists; if you need to update the plugin, " +
"uninstall it first using command 'remove %s'",
destination.toAbsolutePath(),
destination,
pluginName);
throw new UserException(PLUGIN_EXISTS, message);
}

View File

@ -63,7 +63,7 @@ class ListPluginsCommand extends EnvironmentAwareCommand {
private void printPlugin(Environment env, Terminal terminal, Path plugin, String prefix) throws IOException {
terminal.println(Terminal.Verbosity.SILENT, prefix + plugin.getFileName().toString());
PluginInfo info = PluginInfo.readFromProperties(env.pluginsFile().resolve(plugin.toAbsolutePath()));
PluginInfo info = PluginInfo.readFromProperties(env.pluginsFile().resolve(plugin));
terminal.println(Terminal.Verbosity.VERBOSE, info.toString(prefix));
if (info.getElasticsearchVersion().equals(Version.CURRENT) == false) {
terminal.println("WARNING: plugin [" + info.getName() + "] was built for Elasticsearch version " + info.getVersion() +

View File

@ -22,9 +22,12 @@ package org.elasticsearch.bootstrap;
import org.elasticsearch.cli.ExitCodes;
import org.elasticsearch.common.SuppressForbidden;
import org.elasticsearch.common.io.PathUtils;
import org.elasticsearch.common.settings.Settings;
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.Matchers.hasItem;
import static org.hamcrest.Matchers.hasSize;
public class EvilElasticsearchCliTests extends ESElasticsearchCliTestCase {
@ -40,9 +43,11 @@ public class EvilElasticsearchCliTests extends ESElasticsearchCliTestCase {
output -> {},
(foreground, pidFile, quiet, esSettings) -> {
Settings settings = esSettings.settings();
assertThat(settings.size(), equalTo(2));
assertEquals(value, settings.get("path.home"));
assertTrue(settings.keySet().contains("path.logs")); // added by env initialization
assertThat(settings.keySet(), hasSize(2));
assertThat(
settings.get("path.home"),
equalTo(PathUtils.get(System.getProperty("user.dir")).resolve(value).toString()));
assertThat(settings.keySet(), hasItem("path.logs")); // added by env initialization
});
System.clearProperty("es.path.home");
@ -53,9 +58,11 @@ public class EvilElasticsearchCliTests extends ESElasticsearchCliTestCase {
output -> {},
(foreground, pidFile, quiet, esSettings) -> {
Settings settings = esSettings.settings();
assertThat(settings.size(), equalTo(2));
assertEquals(commandLineValue, settings.get("path.home"));
assertTrue(settings.keySet().contains("path.logs")); // added by env initialization
assertThat(settings.keySet(), hasSize(2));
assertThat(
settings.get("path.home"),
equalTo(PathUtils.get(System.getProperty("user.dir")).resolve(commandLineValue).toString()));
assertThat(settings.keySet(), hasItem("path.logs")); // added by env initialization
},
"-Epath.home=" + commandLineValue);

View File

@ -35,10 +35,12 @@ import java.net.URL;
import java.nio.file.FileStore;
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.Objects;
import java.util.function.Function;
import java.util.stream.Collectors;
/**
* The environment of where things exists.
@ -96,13 +98,13 @@ public class Environment {
Environment(final Settings settings, final Path configPath, final Path tmpPath) {
final Path homeFile;
if (PATH_HOME_SETTING.exists(settings)) {
homeFile = PathUtils.get(PATH_HOME_SETTING.get(settings)).normalize();
homeFile = PathUtils.get(PATH_HOME_SETTING.get(settings)).toAbsolutePath().normalize();
} else {
throw new IllegalStateException(PATH_HOME_SETTING.getKey() + " is not configured");
}
if (configPath != null) {
configFile = configPath.normalize();
configFile = configPath.toAbsolutePath().normalize();
} else {
configFile = homeFile.resolve("config");
}
@ -117,7 +119,7 @@ public class Environment {
if (dataPaths.isEmpty() == false) {
dataFiles = new Path[dataPaths.size()];
for (int i = 0; i < dataPaths.size(); i++) {
dataFiles[i] = PathUtils.get(dataPaths.get(i));
dataFiles[i] = PathUtils.get(dataPaths.get(i)).toAbsolutePath().normalize();
}
} else {
dataFiles = new Path[]{homeFile.resolve("data")};
@ -131,7 +133,7 @@ public class Environment {
}
}
if (PATH_SHARED_DATA_SETTING.exists(settings)) {
sharedDataFile = PathUtils.get(PATH_SHARED_DATA_SETTING.get(settings)).normalize();
sharedDataFile = PathUtils.get(PATH_SHARED_DATA_SETTING.get(settings)).toAbsolutePath().normalize();
} else {
sharedDataFile = null;
}
@ -141,19 +143,19 @@ public class Environment {
} else {
repoFiles = new Path[repoPaths.size()];
for (int i = 0; i < repoPaths.size(); i++) {
repoFiles[i] = PathUtils.get(repoPaths.get(i));
repoFiles[i] = PathUtils.get(repoPaths.get(i)).toAbsolutePath().normalize();
}
}
// this is trappy, Setting#get(Settings) will get a fallback setting yet return false for Settings#exists(Settings)
if (PATH_LOGS_SETTING.exists(settings)) {
logsFile = PathUtils.get(PATH_LOGS_SETTING.get(settings)).normalize();
logsFile = PathUtils.get(PATH_LOGS_SETTING.get(settings)).toAbsolutePath().normalize();
} else {
logsFile = homeFile.resolve("logs");
}
if (PIDFILE_SETTING.exists(settings)) {
pidFile = PathUtils.get(PIDFILE_SETTING.get(settings)).normalize();
pidFile = PathUtils.get(PIDFILE_SETTING.get(settings)).toAbsolutePath().normalize();
} else {
pidFile = null;
}
@ -162,12 +164,25 @@ public class Environment {
libFile = homeFile.resolve("lib");
modulesFile = homeFile.resolve("modules");
Settings.Builder finalSettings = Settings.builder().put(settings);
finalSettings.put(PATH_HOME_SETTING.getKey(), homeFile);
final Settings.Builder finalSettings = Settings.builder().put(settings);
if (PATH_DATA_SETTING.exists(settings)) {
finalSettings.putList(PATH_DATA_SETTING.getKey(), dataPaths);
finalSettings.putList(PATH_DATA_SETTING.getKey(), Arrays.stream(dataFiles).map(Path::toString).collect(Collectors.toList()));
}
finalSettings.put(PATH_HOME_SETTING.getKey(), homeFile);
finalSettings.put(PATH_LOGS_SETTING.getKey(), logsFile.toString());
if (PATH_REPO_SETTING.exists(settings)) {
finalSettings.putList(
Environment.PATH_REPO_SETTING.getKey(),
Arrays.stream(repoFiles).map(Path::toString).collect(Collectors.toList()));
}
if (PATH_SHARED_DATA_SETTING.exists(settings)) {
assert sharedDataFile != null;
finalSettings.put(Environment.PATH_SHARED_DATA_SETTING.getKey(), sharedDataFile.toString());
}
if (PIDFILE_SETTING.exists(settings)) {
assert pidFile != null;
finalSettings.put(Environment.PIDFILE_SETTING.getKey(), pidFile.toString());
}
this.settings = finalSettings.build();
}

View File

@ -405,9 +405,9 @@ public abstract class MetaDataStateFormat<T> {
logger.trace("generation id [{}] read from [{}]", generation, stateFile.getFileName());
return state;
} catch (Exception e) {
exceptions.add(new IOException("failed to read " + stateFile.toAbsolutePath(), e));
exceptions.add(new IOException("failed to read " + stateFile, e));
logger.debug(() -> new ParameterizedMessage(
"{}: failed to read [{}], ignoring...", stateFile.toAbsolutePath(), prefix), e);
"{}: failed to read [{}], ignoring...", stateFile, prefix), e);
}
}
// if we reach this something went wrong
@ -415,7 +415,7 @@ public abstract class MetaDataStateFormat<T> {
if (stateFiles.size() > 0) {
// We have some state files but none of them gave us a usable state
throw new IllegalStateException("Could not find a state file to recover from among " +
stateFiles.stream().map(Path::toAbsolutePath).map(Object::toString).collect(Collectors.joining(", ")));
stateFiles.stream().map(Object::toString).collect(Collectors.joining(", ")));
}
return null;
}
@ -435,7 +435,7 @@ public abstract class MetaDataStateFormat<T> {
if (generation > -1 && state == null) {
throw new IllegalStateException("unable to find state files with generation id " + generation +
" returned by findMaxGenerationId function, in data folders [" +
Arrays.stream(dataLocations).map(Path::toAbsolutePath).
Arrays.stream(dataLocations).
map(Object::toString).collect(Collectors.joining(", ")) +
"], concurrent writes?");
}

View File

@ -148,7 +148,7 @@ public class FsProbe {
public static FsInfo.Path getFSInfo(NodePath nodePath) throws IOException {
FsInfo.Path fsPath = new FsInfo.Path();
fsPath.path = nodePath.path.toAbsolutePath().toString();
fsPath.path = nodePath.path.toString();
// NOTE: we use already cached (on node startup) FileStore and spins
// since recomputing these once per second (default) could be costly,

View File

@ -91,10 +91,6 @@ public class InternalSettingsPreparer {
checkSettingsForTerminalDeprecation(output);
finalizeSettings(output, defaultNodeName);
environment = new Environment(output.build(), configPath);
// we put back the path.logs so we can use it in the logging configuration file
output.put(Environment.PATH_LOGS_SETTING.getKey(), environment.logsFile().toAbsolutePath().normalize().toString());
return new Environment(output.build(), configPath);
}

View File

@ -18,6 +18,7 @@
*/
package org.elasticsearch.env;
import org.elasticsearch.common.io.PathUtils;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.test.ESTestCase;
@ -25,6 +26,7 @@ import java.io.FileNotFoundException;
import java.io.IOException;
import java.net.URL;
import java.nio.file.Path;
import java.util.List;
import static org.hamcrest.CoreMatchers.endsWith;
import static org.hamcrest.CoreMatchers.notNullValue;
@ -33,6 +35,7 @@ import static org.hamcrest.CoreMatchers.startsWith;
import static org.hamcrest.Matchers.arrayWithSize;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.hasToString;
/**
@ -168,4 +171,54 @@ public class EnvironmentTests extends ESTestCase {
assertThat(e.getMessage(), startsWith("Configured temporary file directory ["));
assertThat(e.getMessage(), endsWith(".test] is not a directory"));
}
// test that environment paths are absolute and normalized
public void testPathNormalization() throws IOException {
final Settings settings = Settings.builder()
.put(Environment.PATH_HOME_SETTING.getKey(), "home")
.put(Environment.PATH_DATA_SETTING.getKey(), "./home/../home/data")
.put(Environment.PATH_LOGS_SETTING.getKey(), "./home/../home/logs")
.put(Environment.PATH_REPO_SETTING.getKey(), "./home/../home/repo")
.put(Environment.PATH_SHARED_DATA_SETTING.getKey(), "./home/../home/shared_data")
.put(Environment.PIDFILE_SETTING.getKey(), "./home/../home/pidfile")
.build();
// the above paths will be treated as relative to the working directory
final Path workingDirectory = PathUtils.get(System.getProperty("user.dir"));
final Environment environment = new Environment(settings, null, createTempDir());
final String homePath = Environment.PATH_HOME_SETTING.get(environment.settings());
assertPath(homePath, workingDirectory.resolve("home"));
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 logPath = Environment.PATH_LOGS_SETTING.get(environment.settings());
assertPath(logPath, home.resolve("logs"));
final List<String> repoPaths = Environment.PATH_REPO_SETTING.get(environment.settings());
assertThat(repoPaths, hasSize(1));
assertPath(repoPaths.get(0), home.resolve("repo"));
final String sharedDataPath = Environment.PATH_SHARED_DATA_SETTING.get(environment.settings());
assertPath(sharedDataPath, home.resolve("shared_data"));
}
private void assertPath(final String actual, final Path expected) {
assertIsAbsolute(actual);
assertIsNormalized(actual);
assertThat(PathUtils.get(actual), equalTo(expected));
}
private void assertIsAbsolute(final String path) {
assertTrue("path [" + path + "] is not absolute", PathUtils.get(path).isAbsolute());
}
private void assertIsNormalized(final String path) {
assertThat("path [" + path + "] is not normalized", PathUtils.get(path), equalTo(PathUtils.get(path).normalize()));
}
}

View File

@ -949,7 +949,7 @@ public abstract class ESTestCase extends LuceneTestCase {
// because some code is buggy when it comes to multiple nio.2 filesystems
// (e.g. FileSystemUtils, and likely some tests)
try {
return PathUtils.get(getClass().getResource(relativePath).toURI());
return PathUtils.get(getClass().getResource(relativePath).toURI()).toAbsolutePath().normalize();
} catch (Exception e) {
throw new RuntimeException("resource not found: " + relativePath, e);
}

View File

@ -502,7 +502,7 @@ public class UsersToolTests extends CommandTestCase {
execute("useradd", pathHomeParameter, fileOrderParameter, "username", "-p", SecuritySettingsSourceField.TEST_PASSWORD);
});
assertEquals(ExitCodes.CONFIG, e.exitCode);
assertThat(e.getMessage(), containsString("Configuration file [eshome/config/users] is missing"));
assertThat(e.getMessage(), containsString("Configuration file [/work/eshome/config/users] is missing"));
}
public void testUserListNoConfig() throws Exception {
@ -514,7 +514,7 @@ public class UsersToolTests extends CommandTestCase {
execute("list", pathHomeParameter, fileOrderParameter);
});
assertEquals(ExitCodes.CONFIG, e.exitCode);
assertThat(e.getMessage(), containsString("Configuration file [eshome/config/users] is missing"));
assertThat(e.getMessage(), containsString("Configuration file [/work/eshome/config/users] is missing"));
}
public void testUserDelNoConfig() throws Exception {
@ -526,7 +526,7 @@ public class UsersToolTests extends CommandTestCase {
execute("userdel", pathHomeParameter, fileOrderParameter, "username");
});
assertEquals(ExitCodes.CONFIG, e.exitCode);
assertThat(e.getMessage(), containsString("Configuration file [eshome/config/users] is missing"));
assertThat(e.getMessage(), containsString("Configuration file [/work/eshome/config/users] is missing"));
}
public void testListUserRolesNoConfig() throws Exception {
@ -538,6 +538,6 @@ public class UsersToolTests extends CommandTestCase {
execute("roles", pathHomeParameter, fileOrderParameter, "username");
});
assertEquals(ExitCodes.CONFIG, e.exitCode);
assertThat(e.getMessage(), containsString("Configuration file [eshome/config/users_roles] is missing"));
assertThat(e.getMessage(), containsString("Configuration file [/work/eshome/config/users_roles] is missing"));
}
}