Consolidate temp dir handling in packaging tests (#58292)

The packaging tests currently have a couple different ways of deciding
where temp files should be placed, and then sometimes used fixed file
or directory names within that dir. This commit conslidates some of that
temp dir handling by making it more compatible with the handling that
exists within the bats tests, where /tmp is not always appropriate due
to how systemd interacts with it. This commit also adds a utility
methhod for creating temp dirs, so as to ensure the new directory is
created as if a umask of 022 were used, which is not the case when using
Files.createTempDirectory without a set of permissions (it assumes 077).
This commit is contained in:
Ryan Ernst 2020-06-18 10:28:06 -07:00 committed by Ryan Ernst
parent 20abba8433
commit d702cb0ad9
No known key found for this signature in database
GPG Key ID: 5F7EA39E15F54DCE
7 changed files with 58 additions and 31 deletions

View File

@ -42,7 +42,6 @@ import static org.elasticsearch.packaging.util.FileExistenceMatchers.fileDoesNot
import static org.elasticsearch.packaging.util.FileExistenceMatchers.fileExists; import static org.elasticsearch.packaging.util.FileExistenceMatchers.fileExists;
import static org.elasticsearch.packaging.util.FileUtils.append; import static org.elasticsearch.packaging.util.FileUtils.append;
import static org.elasticsearch.packaging.util.FileUtils.cp; import static org.elasticsearch.packaging.util.FileUtils.cp;
import static org.elasticsearch.packaging.util.FileUtils.getTempDir;
import static org.elasticsearch.packaging.util.FileUtils.mkdir; import static org.elasticsearch.packaging.util.FileUtils.mkdir;
import static org.elasticsearch.packaging.util.FileUtils.mv; import static org.elasticsearch.packaging.util.FileUtils.mv;
import static org.elasticsearch.packaging.util.FileUtils.rm; import static org.elasticsearch.packaging.util.FileUtils.rm;
@ -252,7 +251,7 @@ public class ArchiveTests extends PackagingTestCase {
public void test70CustomPathConfAndJvmOptions() throws Exception { public void test70CustomPathConfAndJvmOptions() throws Exception {
final Path tempConf = getTempDir().resolve("esconf-alternate"); final Path tempConf = createTempDir("esconf-alternate");
try { try {
mkdir(tempConf); mkdir(tempConf);
@ -344,7 +343,7 @@ public class ArchiveTests extends PackagingTestCase {
public void test80RelativePathConf() throws Exception { public void test80RelativePathConf() throws Exception {
final Path temp = getTempDir().resolve("esconf-alternate"); final Path temp = createTempDir("esconf-alternate");
final Path tempConf = temp.resolve("config"); final Path tempConf = temp.resolve("config");
try { try {
@ -425,7 +424,7 @@ public class ArchiveTests extends PackagingTestCase {
Path relativeDataPath = installation.data.relativize(installation.home); Path relativeDataPath = installation.data.relativize(installation.home);
append(installation.config("elasticsearch.yml"), "path.data: " + relativeDataPath); append(installation.config("elasticsearch.yml"), "path.data: " + relativeDataPath);
sh.setWorkingDirectory(getTempDir()); sh.setWorkingDirectory(getRootTempDir());
startElasticsearch(); startElasticsearch();
stopElasticsearch(); stopElasticsearch();
@ -437,7 +436,7 @@ public class ArchiveTests extends PackagingTestCase {
public void test94ElasticsearchNodeExecuteCliNotEsHomeWorkDir() throws Exception { public void test94ElasticsearchNodeExecuteCliNotEsHomeWorkDir() throws Exception {
final Installation.Executables bin = installation.executables(); final Installation.Executables bin = installation.executables();
// Run the cli tools from the tmp dir // Run the cli tools from the tmp dir
sh.setWorkingDirectory(getTempDir()); sh.setWorkingDirectory(getRootTempDir());
Platforms.PlatformAction action = () -> { Platforms.PlatformAction action = () -> {
Result result = sh.run(bin.certutilTool + " -h"); Result result = sh.run(bin.certutilTool + " -h");

View File

@ -40,13 +40,12 @@ import static org.elasticsearch.packaging.util.FileMatcher.Fileness.File;
import static org.elasticsearch.packaging.util.FileMatcher.file; import static org.elasticsearch.packaging.util.FileMatcher.file;
import static org.elasticsearch.packaging.util.FileMatcher.p600; import static org.elasticsearch.packaging.util.FileMatcher.p600;
import static org.elasticsearch.packaging.util.FileUtils.escapePath; import static org.elasticsearch.packaging.util.FileUtils.escapePath;
import static org.elasticsearch.packaging.util.FileUtils.getTempDir;
import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.containsString;
import static org.junit.Assume.assumeTrue; import static org.junit.Assume.assumeTrue;
public class CertGenCliTests extends PackagingTestCase { public class CertGenCliTests extends PackagingTestCase {
private static final Path instancesFile = getTempDir().resolve("instances.yml"); private static final Path instancesFile = getRootTempDir().resolve("instances.yml");
private static final Path certificatesFile = getTempDir().resolve("certificates.zip"); private static final Path certificatesFile = getRootTempDir().resolve("certificates.zip");
@Before @Before
public void filterDistros() { public void filterDistros() {

View File

@ -56,7 +56,6 @@ import static org.elasticsearch.packaging.util.FileMatcher.p600;
import static org.elasticsearch.packaging.util.FileMatcher.p660; import static org.elasticsearch.packaging.util.FileMatcher.p660;
import static org.elasticsearch.packaging.util.FileMatcher.p775; import static org.elasticsearch.packaging.util.FileMatcher.p775;
import static org.elasticsearch.packaging.util.FileUtils.append; import static org.elasticsearch.packaging.util.FileUtils.append;
import static org.elasticsearch.packaging.util.FileUtils.getTempDir;
import static org.elasticsearch.packaging.util.FileUtils.rm; import static org.elasticsearch.packaging.util.FileUtils.rm;
import static org.hamcrest.Matchers.arrayWithSize; import static org.hamcrest.Matchers.arrayWithSize;
import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.containsString;
@ -88,7 +87,7 @@ public class DockerTests extends PackagingTestCase {
@Before @Before
public void setupTest() throws IOException { public void setupTest() throws IOException {
installation = runContainer(distribution()); installation = runContainer(distribution());
tempDir = Files.createTempDirectory(getTempDir(), DockerTests.class.getSimpleName()); tempDir = createTempDir(DockerTests.class.getSimpleName());
} }
@After @After

View File

@ -52,7 +52,6 @@ import static org.elasticsearch.packaging.util.FileMatcher.Fileness.File;
import static org.elasticsearch.packaging.util.FileMatcher.file; import static org.elasticsearch.packaging.util.FileMatcher.file;
import static org.elasticsearch.packaging.util.FileMatcher.p600; import static org.elasticsearch.packaging.util.FileMatcher.p600;
import static org.elasticsearch.packaging.util.FileMatcher.p660; import static org.elasticsearch.packaging.util.FileMatcher.p660;
import static org.elasticsearch.packaging.util.FileUtils.getTempDir;
import static org.elasticsearch.packaging.util.FileUtils.rm; import static org.elasticsearch.packaging.util.FileUtils.rm;
import static org.elasticsearch.packaging.util.Packages.assertInstalled; import static org.elasticsearch.packaging.util.Packages.assertInstalled;
import static org.elasticsearch.packaging.util.Packages.assertRemoved; import static org.elasticsearch.packaging.util.Packages.assertRemoved;
@ -306,7 +305,7 @@ public class KeystoreManagementTests extends PackagingTestCase {
Path tempDir = null; Path tempDir = null;
try { try {
tempDir = Files.createTempDirectory(getTempDir(), DockerTests.class.getSimpleName()); tempDir = createTempDir(DockerTests.class.getSimpleName());
String password = "password"; String password = "password";
String passwordFilename = "password.txt"; String passwordFilename = "password.txt";
@ -366,7 +365,7 @@ public class KeystoreManagementTests extends PackagingTestCase {
private Path getKeystoreFileFromDockerContainer(String password, Path dockerKeystore) throws IOException { private Path getKeystoreFileFromDockerContainer(String password, Path dockerKeystore) throws IOException {
// Mount a temporary directory for copying the keystore // Mount a temporary directory for copying the keystore
Path dockerTemp = Paths.get("/usr/tmp/keystore-tmp"); Path dockerTemp = Paths.get("/usr/tmp/keystore-tmp");
Path tempDirectory = Files.createTempDirectory(getTempDir(), KeystoreManagementTests.class.getSimpleName()); Path tempDirectory = createTempDir(KeystoreManagementTests.class.getSimpleName());
Map<Path, Path> volumes = new HashMap<>(); Map<Path, Path> volumes = new HashMap<>();
volumes.put(tempDirectory, dockerTemp); volumes.put(tempDirectory, dockerTemp);

View File

@ -47,9 +47,12 @@ import org.junit.rules.TestWatcher;
import org.junit.runner.Description; import org.junit.runner.Description;
import org.junit.runner.RunWith; import org.junit.runner.RunWith;
import java.io.IOException;
import java.nio.file.Files; import java.nio.file.Files;
import java.nio.file.Path; import java.nio.file.Path;
import java.nio.file.Paths; import java.nio.file.Paths;
import java.nio.file.attribute.FileAttribute;
import java.nio.file.attribute.PosixFilePermissions;
import java.util.Collections; import java.util.Collections;
import java.util.List; import java.util.List;
@ -112,18 +115,18 @@ public abstract class PackagingTestCase extends Assert {
public final TestName testNameRule = new TestName(); public final TestName testNameRule = new TestName();
@BeforeClass @BeforeClass
public static void filterCompatible() { public static void init() throws Exception {
assumeTrue("only compatible distributions", distribution.packaging.compatible); assumeTrue("only compatible distributions", distribution.packaging.compatible);
// make sure temp dir exists
if (Files.exists(getRootTempDir()) == false) {
Files.createDirectories(getRootTempDir());
} }
@BeforeClass // cleanup from previous test
public static void cleanup() throws Exception { cleanup();
installation = null;
cleanEverything();
}
@BeforeClass // create shell
public static void createShell() throws Exception {
if (distribution().isDocker()) { if (distribution().isDocker()) {
ensureImageIsLoaded(distribution); ensureImageIsLoaded(distribution);
sh = new Docker.DockerShell(); sh = new Docker.DockerShell();
@ -201,6 +204,11 @@ public abstract class PackagingTestCase extends Assert {
} }
} }
protected static void cleanup() throws Exception {
installation = null;
cleanEverything();
}
/** /**
* Starts and stops elasticsearch, and performs assertions while it is running. * Starts and stops elasticsearch, and performs assertions while it is running.
*/ */
@ -356,4 +364,29 @@ public abstract class PackagingTestCase extends Assert {
} }
} }
public static Path getRootTempDir() {
if (distribution().isPackage()) {
// The custom config directory is not under /tmp or /var/tmp because
// systemd's private temp directory functionally means different
// processes can have different views of what's in these directories
return Paths.get("/var/test-tmp").toAbsolutePath();
} else {
// vagrant creates /tmp for us in windows so we use that to avoid long paths
return Paths.get("/tmp").toAbsolutePath();
}
}
private static final FileAttribute<?>[] NEW_DIR_PERMS;
static {
if (Platforms.WINDOWS) {
NEW_DIR_PERMS = new FileAttribute<?>[0];
} else {
NEW_DIR_PERMS = new FileAttribute<?>[] { PosixFilePermissions.asFileAttribute(PosixFilePermissions.fromString("rwxr-xr-x")) };
}
}
public static Path createTempDir(String prefix) throws IOException {
return Files.createTempDirectory(getRootTempDir(), prefix, NEW_DIR_PERMS);
}
} }

View File

@ -27,7 +27,7 @@ import java.util.Collections;
import java.util.List; import java.util.List;
import java.util.function.Consumer; import java.util.function.Consumer;
import static org.elasticsearch.packaging.util.FileUtils.getTempDir; import static org.elasticsearch.packaging.test.PackagingTestCase.getRootTempDir;
import static org.elasticsearch.packaging.util.FileUtils.lsGlob; import static org.elasticsearch.packaging.util.FileUtils.lsGlob;
import static org.elasticsearch.packaging.util.Platforms.isDPKG; import static org.elasticsearch.packaging.util.Platforms.isDPKG;
import static org.elasticsearch.packaging.util.Platforms.isRPM; import static org.elasticsearch.packaging.util.Platforms.isRPM;
@ -83,7 +83,7 @@ public class Cleanup {
// when we run es as a role user on windows, add the equivalent here // when we run es as a role user on windows, add the equivalent here
// delete files that may still exist // delete files that may still exist
lsGlob(getTempDir(), "elasticsearch*").forEach(FileUtils::rm); lsGlob(getRootTempDir(), "elasticsearch*").forEach(FileUtils::rm);
final List<String> filesToDelete = Platforms.WINDOWS ? ELASTICSEARCH_FILES_WINDOWS : ELASTICSEARCH_FILES_LINUX; final List<String> filesToDelete = Platforms.WINDOWS ? ELASTICSEARCH_FILES_WINDOWS : ELASTICSEARCH_FILES_LINUX;
// windows needs leniency due to asinine releasing of file locking async from a process exiting // windows needs leniency due to asinine releasing of file locking async from a process exiting
Consumer<? super Path> rm = Platforms.WINDOWS ? FileUtils::rmWithRetries : FileUtils::rm; Consumer<? super Path> rm = Platforms.WINDOWS ? FileUtils::rmWithRetries : FileUtils::rm;

View File

@ -36,7 +36,6 @@ import java.nio.file.Files;
import java.nio.file.LinkOption; import java.nio.file.LinkOption;
import java.nio.file.OpenOption; import java.nio.file.OpenOption;
import java.nio.file.Path; import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.StandardOpenOption; import java.nio.file.StandardOpenOption;
import java.nio.file.attribute.BasicFileAttributes; import java.nio.file.attribute.BasicFileAttributes;
import java.nio.file.attribute.FileOwnerAttributeView; import java.nio.file.attribute.FileOwnerAttributeView;
@ -53,6 +52,7 @@ import java.util.stream.Stream;
import java.util.zip.GZIPInputStream; import java.util.zip.GZIPInputStream;
import java.util.zip.ZipException; import java.util.zip.ZipException;
import static org.elasticsearch.packaging.test.PackagingTestCase.getRootTempDir;
import static org.elasticsearch.packaging.util.FileExistenceMatchers.fileDoesNotExist; import static org.elasticsearch.packaging.util.FileExistenceMatchers.fileDoesNotExist;
import static org.elasticsearch.packaging.util.FileExistenceMatchers.fileExists; import static org.elasticsearch.packaging.util.FileExistenceMatchers.fileExists;
import static org.hamcrest.Matchers.emptyIterable; import static org.hamcrest.Matchers.emptyIterable;
@ -66,6 +66,9 @@ public class FileUtils {
public static List<Path> lsGlob(Path directory, String glob) { public static List<Path> lsGlob(Path directory, String glob) {
List<Path> paths = new ArrayList<>(); List<Path> paths = new ArrayList<>();
if (Files.exists(directory) == false) {
return org.elasticsearch.common.collect.List.of();
}
try (DirectoryStream<Path> stream = Files.newDirectoryStream(directory, glob)) { try (DirectoryStream<Path> stream = Files.newDirectoryStream(directory, glob)) {
for (Path path : stream) { for (Path path : stream) {
@ -297,13 +300,8 @@ public class FileUtils {
return numericPathOwnership; return numericPathOwnership;
} }
// vagrant creates /tmp for us in windows so we use that to avoid long paths
public static Path getTempDir() {
return Paths.get("/tmp").toAbsolutePath();
}
public static Path getDefaultArchiveInstallPath() { public static Path getDefaultArchiveInstallPath() {
return getTempDir().resolve("elasticsearch"); return getRootTempDir().resolve("elasticsearch");
} }
private static final Pattern VERSION_REGEX = Pattern.compile("(\\d+\\.\\d+\\.\\d+(-SNAPSHOT)?)"); private static final Pattern VERSION_REGEX = Pattern.compile("(\\d+\\.\\d+\\.\\d+(-SNAPSHOT)?)");