Test: remove outdated logic for file writing in security tests (elastic/x-pack-elasticsearch#3947)

This commit removes some outdated logic in the SecurityTestUtils class
for writing files and creating directories. A long, long time ago there
was a global cluster for tests that was used across suites and because
of this there were calls to delete files if they already existed. The
global cluster has been removed, so we no longer need the code that
deletes the content of a directory if it already exists.

Additionally, the file writing used in SecurityTestUtils did not use
atomic moves when possible and this commit changes the code such that a
temp file is written and we try to atomically move it to the correct
path; if atomic moves are not supported a regular move is performed.

relates elastic/x-pack-elasticsearch#3912

Original commit: elastic/x-pack-elasticsearch@973fcfe2e1
This commit is contained in:
Jay Modi 2018-02-20 09:34:17 -07:00 committed by GitHub
parent 6728912c87
commit 0fc0034509
2 changed files with 35 additions and 30 deletions

View File

@ -33,6 +33,7 @@ import org.elasticsearch.xpack.security.test.SecurityTestUtils;
import java.io.IOException;
import java.io.InputStream;
import java.io.UncheckedIOException;
import java.net.URISyntaxException;
import java.net.URL;
import java.nio.file.FileSystemNotFoundException;
@ -116,19 +117,20 @@ public class SecuritySettingsSource extends ClusterDiscoveryConfiguration.Unicas
@Override
public Settings nodeSettings(int nodeOrdinal) {
final Path home = nodePath(parentFolder, subfolderPrefix, nodeOrdinal);
SecurityTestUtils.createFolder(home);
final Path xpackConf = home.resolve("config").resolve(XPackField.NAME);
SecurityTestUtils.createFolder(xpackConf);
try {
Files.createDirectories(xpackConf);
} catch (IOException e) {
throw new UncheckedIOException(e);
}
writeFile(xpackConf, "roles.yml", configRoles());
writeFile(xpackConf, "users", configUsers());
writeFile(xpackConf, "users_roles", configUsersRoles());
writeFile(xpackConf, "roles.yml", configRoles());
Settings.Builder builder = Settings.builder().put(super.nodeSettings(nodeOrdinal))
//TODO: for now isolate security tests from watcher & monitoring (randomize this later)
.put(XPackSettings.WATCHER_ENABLED.getKey(), false)
.put(XPackSettings.MONITORING_ENABLED.getKey(), false)
// .put(XPackSettings.MACHINE_LEARNING_ENABLED.getKey(), false)
// .put(MachineLearningField.AUTODETECT_PROCESS.getKey(), false)
.put(XPackSettings.AUDIT_ENABLED.getKey(), randomBoolean())
.put(LoggingAuditTrail.HOST_ADDRESS_SETTING.getKey(), randomBoolean())
.put(LoggingAuditTrail.HOST_NAME_SETTING.getKey(), randomBoolean())

View File

@ -5,7 +5,7 @@
*/
package org.elasticsearch.xpack.security.test;
import org.elasticsearch.ElasticsearchException;
import org.apache.lucene.util.IOUtils;
import org.elasticsearch.Version;
import org.elasticsearch.cluster.health.ClusterHealthStatus;
import org.elasticsearch.cluster.health.ClusterIndexHealth;
@ -18,7 +18,6 @@ import org.elasticsearch.cluster.routing.RecoverySource;
import org.elasticsearch.cluster.routing.RoutingTable;
import org.elasticsearch.cluster.routing.ShardRouting;
import org.elasticsearch.cluster.routing.UnassignedInfo;
import org.elasticsearch.common.io.FileSystemUtils;
import org.elasticsearch.common.io.Streams;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.index.Index;
@ -27,42 +26,46 @@ import org.elasticsearch.test.ESTestCase;
import java.io.IOException;
import java.io.OutputStream;
import java.io.UncheckedIOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.AtomicMoveNotSupportedException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Locale;
import java.util.UUID;
import static java.nio.file.StandardCopyOption.ATOMIC_MOVE;
import static java.nio.file.StandardCopyOption.REPLACE_EXISTING;
import static java.nio.file.StandardOpenOption.CREATE;
import static java.nio.file.StandardOpenOption.TRUNCATE_EXISTING;
import static java.nio.file.StandardOpenOption.WRITE;
import static org.elasticsearch.cluster.routing.RecoverySource.StoreRecoverySource.EXISTING_STORE_INSTANCE;
import static org.elasticsearch.xpack.security.SecurityLifecycleService.SECURITY_INDEX_NAME;
import static org.junit.Assert.assertEquals;
public class SecurityTestUtils {
public static void createFolder(Path path) {
//the directory might exist e.g. if the global cluster gets restarted, then we recreate the directory as well
if (Files.exists(path)) {
try {
FileSystemUtils.deleteSubDirectories(path);
} catch (IOException e) {
throw new RuntimeException("could not delete existing temporary folder: " + path.toAbsolutePath(), e);
}
} else {
try {
Files.createDirectories(path);
} catch (IOException e) {
throw new RuntimeException("could not create temporary folder: " + path.toAbsolutePath());
}
}
}
public static String writeFile(Path folder, String name, byte[] content) {
Path file = folder.resolve(name);
try (OutputStream os = Files.newOutputStream(file)) {
Streams.copy(content, os);
} catch (IOException e) {
throw new ElasticsearchException("error writing file in test", e);
final Path path = folder.resolve(name);
Path tempFile = null;
try {
tempFile = Files.createTempFile(path.getParent(), path.getFileName().toString(), "tmp");
try (OutputStream os = Files.newOutputStream(tempFile, CREATE, TRUNCATE_EXISTING, WRITE)) {
Streams.copy(content, os);
}
try {
Files.move(tempFile, path, REPLACE_EXISTING, ATOMIC_MOVE);
} catch (final AtomicMoveNotSupportedException e) {
Files.move(tempFile, path, REPLACE_EXISTING);
}
} catch (final IOException e) {
throw new UncheckedIOException(String.format(Locale.ROOT, "could not write file [%s]", path.toAbsolutePath()), e);
} finally {
// we are ignoring exceptions here, so we do not need handle whether or not tempFile was initialized nor if the file exists
IOUtils.deleteFilesIgnoringExceptions(tempFile);
}
return file.toAbsolutePath().toString();
return path.toAbsolutePath().toString();
}
public static String writeFile(Path folder, String name, String content) {