From 356c209120027c893fccfc22613d551d30fc8ced Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hugo=20Ar=C3=A8s?= Date: Mon, 8 Jul 2019 13:14:41 -0400 Subject: [PATCH] Fix CompressArchiveUtil unit tests The tar method's contract is very clear, it creates a tar archive from files in a folder. The unit tests should validate that the tar archive was created and that the content is what it is supposed to be. Same apply to the archiveTARFiles method. Before, in order to validate that the tar archive was created, the unit tests were un-archiving the tar. This is wrong as you end up implicitly testing the extraction code which could be faulty. In addition, the 2 tests for symlinks were not even checking if the extracted file was a symlink, they were only checking if the file or folder were readable. Rewrite the tests to assert directly the content of the archive. Also fix the 2 symlinks tests to make sure the tar preserve them. Fixing the unit tests exposed that CompressArchiveUtil methods do not preserve the symlinks which is a bug. Add @Ignore annotation for those tests until the bug is fixed. This change is a preparation change to fix the archive creation to preserve symlinks so add tests to increase archive creation code coverage close to 100%. This will allow to fix the code while making sure no regression are introduced. Related to issue https://github.com/docker-java/docker-java/issues/532 --- .../core/util/CompressArchiveUtilTest.java | 352 +++++++++++++++--- 1 file changed, 292 insertions(+), 60 deletions(-) diff --git a/src/test/java/com/github/dockerjava/core/util/CompressArchiveUtilTest.java b/src/test/java/com/github/dockerjava/core/util/CompressArchiveUtilTest.java index 896fd9415..c2b4a6f4b 100644 --- a/src/test/java/com/github/dockerjava/core/util/CompressArchiveUtilTest.java +++ b/src/test/java/com/github/dockerjava/core/util/CompressArchiveUtilTest.java @@ -2,23 +2,27 @@ import org.apache.commons.compress.archivers.tar.TarArchiveEntry; import org.apache.commons.compress.archivers.tar.TarArchiveInputStream; -import org.apache.commons.io.IOUtils; +import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; import java.io.BufferedInputStream; +import java.io.ByteArrayInputStream; import java.io.File; import java.io.FileInputStream; -import java.io.FileOutputStream; import java.io.IOException; +import java.io.InputStream; import java.nio.file.Files; import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; import java.util.zip.GZIPInputStream; import static java.util.Arrays.asList; -import static org.hamcrest.CoreMatchers.is; -import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; public class CompressArchiveUtilTest { @@ -26,69 +30,297 @@ public class CompressArchiveUtilTest { public TemporaryFolder tempFolder = new TemporaryFolder(); @Test - public void testExecutableFlagIsPreserved() throws Exception { + public void tarWithRegularFileAsInput() throws Exception { + Path archiveSourceFile = tempFolder.getRoot().toPath().resolve("sourceFile"); + createFileWithContent(archiveSourceFile); + + // ChildrenOnly = false + Path tarGzFile = tempFolder.newFile("archive.tar.gz").toPath(); + CompressArchiveUtil.tar(archiveSourceFile, tarGzFile, true, false); + assertEquals(1, getNumberOfEntryInArchive(tarGzFile.toFile())); + assertTarArchiveEntryIsNonEmptyFile(tarGzFile.toFile(), "sourceFile"); + + // ChildrenOnly = true, this option make no sense when input is a file but still, let's test it + // to make sure it behaves as expected + tarGzFile = tempFolder.newFile("archiveChildrenOnly.tar.gz").toPath(); + CompressArchiveUtil.tar(archiveSourceFile, tarGzFile, true, false); + assertEquals(1, getNumberOfEntryInArchive(tarGzFile.toFile())); + assertTarArchiveEntryIsNonEmptyFile(tarGzFile.toFile(), "sourceFile"); + } + + @Test + public void tarWithExecutableFileAsInput() throws Exception { + Path archiveSourceFile = tempFolder.getRoot().toPath().resolve("executableFile.sh"); + createFileWithContent(archiveSourceFile); + archiveSourceFile.toFile().setExecutable(true); + + // ChildrenOnly = false + Path tarGzFile = tempFolder.newFile("archive.tar.gz").toPath(); + CompressArchiveUtil.tar(archiveSourceFile, tarGzFile, true, false); + assertEquals(1, getNumberOfEntryInArchive(tarGzFile.toFile())); + assertTarArchiveEntryIsExecutableFile(tarGzFile.toFile(), "executableFile.sh"); + + // ChildrenOnly = true, this option make no sense when input is a file but still, let's test it + // to make sure it behaves as expected + tarGzFile = tempFolder.newFile("archiveChildrenOnly.tar.gz").toPath(); + CompressArchiveUtil.tar(archiveSourceFile, tarGzFile, true, false); + assertEquals(1, getNumberOfEntryInArchive(tarGzFile.toFile())); + assertTarArchiveEntryIsExecutableFile(tarGzFile.toFile(), "executableFile.sh"); + } + + @Test + @Ignore("Symlink creation is broken so test do not pass") + public void tarWithSymbolicLinkFileAsInput() throws IOException { + Path archiveSourceFile = tempFolder.getRoot().toPath().resolve("symlinkFile"); + Path linkTargetFile = tempFolder.newFile("link-target").toPath(); + Files.createSymbolicLink(archiveSourceFile, linkTargetFile); + + // ChildrenOnly = false + Path tarGzFile = tempFolder.newFile("archive.tar.gz").toPath(); + CompressArchiveUtil.tar(archiveSourceFile, tarGzFile, true, false); + assertEquals(1, getNumberOfEntryInArchive(tarGzFile.toFile())); + assertTarArchiveEntryIsSymlink(tarGzFile.toFile(), "symlinkFile", linkTargetFile.toString()); + + // ChildrenOnly = true, this option make no sense when input is a file but still, let's test it + // to make sure it behaves as expected + tarGzFile = tempFolder.newFile("archiveChildrenOnly.tar.gz").toPath(); + CompressArchiveUtil.tar(archiveSourceFile, tarGzFile, true, false); + assertEquals(1, getNumberOfEntryInArchive(tarGzFile.toFile())); + assertTarArchiveEntryIsSymlink(tarGzFile.toFile(), "symlinkFile", linkTargetFile.toString()); + } + + @Test + public void tarWithfolderAsInput() throws Exception { + Path archiveSourceDir = tempFolder.newFolder("archive-source").toPath(); + createFoldersAndSubFolderWithFiles(archiveSourceDir); + + // ChildrenOnly = false + Path tarGzFile = tempFolder.newFile("archive.tar.gz").toPath(); + CompressArchiveUtil.tar(archiveSourceDir, tarGzFile, true, false); + assertEquals(7, getNumberOfEntryInArchive(tarGzFile.toFile())); + assertTarArchiveEntryIsDirectory(tarGzFile.toFile(), "archive-source"); + assertTarArchiveEntryIsDirectory(tarGzFile.toFile(), "folderA"); + assertTarArchiveEntryIsDirectory(tarGzFile.toFile(), "folderB"); + assertTarArchiveEntryIsDirectory(tarGzFile.toFile(), "subFolderB"); + assertTarArchiveEntryIsNonEmptyFile(tarGzFile.toFile(), "fileA"); + assertTarArchiveEntryIsNonEmptyFile(tarGzFile.toFile(), "fileB"); + assertTarArchiveEntryIsNonEmptyFile(tarGzFile.toFile(), "subFileB"); + + // ChildrenOnly = true + tarGzFile = tempFolder.newFile("archiveChildrenOnly.tar.gz").toPath(); + CompressArchiveUtil.tar(archiveSourceDir, tarGzFile, true, true); + assertEquals(6, getNumberOfEntryInArchive(tarGzFile.toFile())); + assertTarArchiveEntryIsDirectory(tarGzFile.toFile(), "folderA"); + assertTarArchiveEntryIsDirectory(tarGzFile.toFile(), "folderB"); + assertTarArchiveEntryIsDirectory(tarGzFile.toFile(), "subFolderB"); + assertTarArchiveEntryIsNonEmptyFile(tarGzFile.toFile(), "fileA"); + assertTarArchiveEntryIsNonEmptyFile(tarGzFile.toFile(), "fileB"); + assertTarArchiveEntryIsNonEmptyFile(tarGzFile.toFile(), "subFileB"); + } + + @Test + public void tarWithfolderAsInputAndNestedExecutableFile() throws Exception { + Path archiveSourceDir = tempFolder.newFolder("archive-source").toPath(); + Path executableFile = archiveSourceDir.resolve("executableFile.sh"); + createFileWithContent(executableFile); + executableFile.toFile().setExecutable(true); + + // ChildrenOnly = false + Path tarGzFile = tempFolder.newFile("archive.tar.gz").toPath(); + CompressArchiveUtil.tar(archiveSourceDir, tarGzFile, true, false); + assertEquals(2, getNumberOfEntryInArchive(tarGzFile.toFile())); + assertTarArchiveEntryIsDirectory(tarGzFile.toFile(), "archive-source"); + assertTarArchiveEntryIsExecutableFile(tarGzFile.toFile(), "executableFile.sh"); + + // ChildrenOnly = true + tarGzFile = tempFolder.newFile("archiveChildrenOnly.tar.gz").toPath(); + CompressArchiveUtil.tar(archiveSourceDir, tarGzFile, true, true); + assertEquals(1, getNumberOfEntryInArchive(tarGzFile.toFile())); + assertTarArchiveEntryIsExecutableFile(tarGzFile.toFile(), "executableFile.sh"); + } + + @Test + @Ignore("Symlink creation is broken so test do not pass") + public void tarWithfolderAsInputAndNestedSymbolicLinkFile() throws Exception { + Path archiveSourceDir = tempFolder.newFolder("archive-source").toPath(); + Path linkTargetFile = tempFolder.newFile("link-target").toPath(); + Path symlinkFile = archiveSourceDir.resolve("symlinkFile"); + Files.createSymbolicLink(symlinkFile, linkTargetFile); + + // ChildrenOnly = false + Path tarGzFile = tempFolder.newFile("archive.tar.gz").toPath(); + CompressArchiveUtil.tar(archiveSourceDir, tarGzFile, true, false); + assertEquals(2, getNumberOfEntryInArchive(tarGzFile.toFile())); + assertTarArchiveEntryIsDirectory(tarGzFile.toFile(), "archive-source"); + assertTarArchiveEntryIsSymlink(tarGzFile.toFile(), "symlinkFile", linkTargetFile.toString()); + + // ChildrenOnly = true + tarGzFile = tempFolder.newFile("archiveChildrenOnly.tar.gz").toPath(); + CompressArchiveUtil.tar(archiveSourceDir, tarGzFile, true, true); + assertEquals(1, getNumberOfEntryInArchive(tarGzFile.toFile())); + assertTarArchiveEntryIsSymlink(tarGzFile.toFile(), "symlinkFile", linkTargetFile.toString()); + } + + @Test + @Ignore("Symlink creation is broken so test do not pass") + public void tarWithfolderAsInputAndNestedSymbolicLinkDir() throws Exception { + Path archiveSourceDir = tempFolder.newFolder("archive-source").toPath(); + Path linkTargetDir = tempFolder.newFolder("link-target").toPath(); + Path symlinkFile = archiveSourceDir.resolve("symlinkFile"); + Files.createSymbolicLink(symlinkFile, linkTargetDir); + + // ChildrenOnly = false + Path tarGzFile = tempFolder.newFile("archive.tar.gz").toPath(); + CompressArchiveUtil.tar(archiveSourceDir, tarGzFile, true, false); + assertEquals(2, getNumberOfEntryInArchive(tarGzFile.toFile())); + assertTarArchiveEntryIsDirectory(tarGzFile.toFile(), "archive-source"); + assertTarArchiveEntryIsSymlink(tarGzFile.toFile(), "symlinkFile", linkTargetDir.toString()); + + // ChildrenOnly = true + tarGzFile = tempFolder.newFile("archiveChildrenOnly.tar.gz").toPath(); + CompressArchiveUtil.tar(archiveSourceDir, tarGzFile, true, true); + assertEquals(1, getNumberOfEntryInArchive(tarGzFile.toFile())); + assertTarArchiveEntryIsSymlink(tarGzFile.toFile(), "symlinkFile", linkTargetDir.toString()); + } + + @Test + public void archiveTARFilesWithFolderAndFiles() throws Exception { + File archive = CompressArchiveUtil.archiveTARFiles(tempFolder.getRoot(), + createFoldersAndSubFolderWithFiles(tempFolder.getRoot().toPath()), "archive"); + assertEquals(6, getNumberOfEntryInArchive(archive)); + assertTarArchiveEntryIsDirectory(archive, "folderA"); + assertTarArchiveEntryIsDirectory(archive, "folderB"); + assertTarArchiveEntryIsDirectory(archive, "subFolderB"); + assertTarArchiveEntryIsNonEmptyFile(archive, "fileA"); + assertTarArchiveEntryIsNonEmptyFile(archive, "fileB"); + assertTarArchiveEntryIsNonEmptyFile(archive, "subFileB"); + } + + @Test + public void archiveTARFilesWithExecutableFile() throws Exception { File executableFile = tempFolder.newFile("executableFile.sh"); executableFile.setExecutable(true); - assertThat(executableFile.canExecute(), is(true)); - - File archive = CompressArchiveUtil.archiveTARFiles(tempFolder.getRoot(), asList(executableFile), - "archive"); - File expectedFile = extractFileByName(archive, "executableFile.sh", "executableFile.sh.result"); - - assertThat("should be executable", expectedFile.canExecute()); - } - - private File extractFileByName(File archive, String filenameToExtract, String outputName) throws IOException { - File expectedFile = new File(tempFolder.newFolder(), outputName); - expectedFile.delete(); - assertThat(expectedFile.exists(), is(false)); - - TarArchiveInputStream tarArchiveInputStream = new TarArchiveInputStream(new GZIPInputStream( - new BufferedInputStream(new FileInputStream(archive)))); - TarArchiveEntry entry; - boolean found = false; - while ((entry = tarArchiveInputStream.getNextTarEntry()) != null) { - String individualFiles = entry.getName(); - if (individualFiles.equals(filenameToExtract) || individualFiles.endsWith("/" + filenameToExtract)) { - found = true; - IOUtils.copy(tarArchiveInputStream, new FileOutputStream(expectedFile)); - if ((entry.getMode() & 0755) == 0755) { - expectedFile.setExecutable(true); - } - break; - } - } - assertThat("should extracted the file", found); - tarArchiveInputStream.close(); - return expectedFile; + + File archive = CompressArchiveUtil.archiveTARFiles(tempFolder.getRoot(), asList(executableFile), "archive"); + assertEquals(1, getNumberOfEntryInArchive(archive)); + assertTarArchiveEntryIsExecutableFile(archive, "executableFile.sh"); } @Test - public void testSymbolicLinkDir() throws IOException { - Path uploadDir = tempFolder.newFolder("upload").toPath(); - Path linkTarget = tempFolder.newFolder("link-target").toPath(); - Path tmpFile = Files.createTempFile(linkTarget, "link-dir", "rand"); - Files.createSymbolicLink(uploadDir.resolve("link-folder"), linkTarget); - Path tarGzFile = tempFolder.newFile("docker-java.tar.gz").toPath(); - //follow link only works for childrenOnly=false - CompressArchiveUtil.tar(uploadDir, tarGzFile, true, false); - File expectedFile = extractFileByName(tarGzFile.toFile(), tmpFile.toFile().getName(), "foo1"); - assertThat(expectedFile.canRead(), is(true)); + @Ignore("Symlink creation is broken so test do not pass") + public void archiveTARFilesWithSymbolicLinkFile() throws Exception { + Path linkTargetFile = tempFolder.newFile("link-target").toPath(); + Path symlinkFile = tempFolder.getRoot().toPath().resolve("symlinkFile"); + Files.createSymbolicLink(symlinkFile, linkTargetFile); + + File archive = CompressArchiveUtil.archiveTARFiles(tempFolder.getRoot(), asList(symlinkFile.toFile()), "archive"); + assertEquals(1, getNumberOfEntryInArchive(archive)); + assertTarArchiveEntryIsSymlink(archive, "symlinkFile", linkTargetFile.toString()); } @Test - public void testSymbolicLinkFile() throws IOException { - Path uploadDir = tempFolder.newFolder("upload").toPath(); - Path tmpFile = tempFolder.newFile("src").toPath(); - Files.createSymbolicLink(uploadDir.resolve("link-file"), tmpFile); - Path tarGzFile = tempFolder.newFile("docker-java.tar.gz").toPath(); - boolean childrenOnly = false; - CompressArchiveUtil.tar(uploadDir, tarGzFile, true, childrenOnly); - File expectedFile = extractFileByName(tarGzFile.toFile(), "link-file", "foo1"); - assertThat(expectedFile.canRead(), is(true)); - childrenOnly = true; - CompressArchiveUtil.tar(uploadDir, tarGzFile, true, childrenOnly); - extractFileByName(tarGzFile.toFile(), "link-file", "foo1"); - assertThat(expectedFile.canRead(), is(true)); + @Ignore("Symlink creation is broken so test do not pass") + public void archiveTARFilesWithSymbolicLinkDir() throws Exception { + Path linkTargetDir = tempFolder.newFolder("link-target").toPath(); + Path symlinkFile = tempFolder.getRoot().toPath().resolve("symlinkFile"); + Files.createSymbolicLink(symlinkFile, linkTargetDir); + + File archive = CompressArchiveUtil.archiveTARFiles(tempFolder.getRoot(), asList(symlinkFile.toFile()), "archive"); + assertEquals(1, getNumberOfEntryInArchive(archive)); + assertTarArchiveEntryIsSymlink(archive, "symlinkFile", linkTargetDir.toString()); + } + + private static void assertTarArchiveEntryIsDirectory(File archive, String directoryName) throws IOException { + TarArchiveEntry tarArchiveEntry = getTarArchiveEntry(archive, directoryName); + assertNotNull(tarArchiveEntry); + assertTrue(tarArchiveEntry.isDirectory()); + } + + private static void assertTarArchiveEntryIsNonEmptyFile(File archive, String fileName) throws IOException { + TarArchiveEntry tarArchiveEntry = getTarArchiveEntry(archive, fileName); + assertNotNull(tarArchiveEntry); + assertTrue(tarArchiveEntry.isFile()); + assertTrue(tarArchiveEntry.getSize()>0); + } + + private static void assertTarArchiveEntryIsExecutableFile(File archive, String fileName) throws IOException { + TarArchiveEntry tarArchiveEntry = getTarArchiveEntry(archive, fileName); + assertNotNull(tarArchiveEntry); + assertTrue(tarArchiveEntry.isFile()); + assertEquals("should be executable", (tarArchiveEntry.getMode() & 0755), 0755); + } + + private static void assertTarArchiveEntryIsSymlink(File archive, String fileName, String expectedTarget) throws IOException { + TarArchiveEntry tarArchiveEntry = getTarArchiveEntry(archive, fileName); + assertNotNull(tarArchiveEntry); + assertTrue("should be a symbolic link", tarArchiveEntry.isSymbolicLink()); + assertEquals(expectedTarget, tarArchiveEntry.getLinkName()); + } + + /** + * Creates the following directory structure with files in the specified + * destination folder + * + * destinationFolder + * |__folderA + * | |__fileA + * |__folderB + * |__fileB + * |__subFolderB + * |__subFileB + * + * + * @param destinationFolder where to create the folder/files. + * @return the list of created files. + * @throws IOException if an error occurs while creating the folders/files. + */ + private static List createFoldersAndSubFolderWithFiles(Path destinationFolder) throws IOException { + List createdFiles = new ArrayList(); + Path folderA = destinationFolder.resolve("folderA"); + createdFiles.add(Files.createDirectories(folderA).toFile()); + createdFiles.add(createFileWithContent(folderA.resolve("fileA"))); + + Path folderB = destinationFolder.resolve("folderB"); + createdFiles.add(Files.createDirectories(folderB).toFile()); + createdFiles.add(createFileWithContent(folderB.resolve("fileB"))); + + Path subFolderB = folderB.resolve("subFolderB"); + createdFiles.add(Files.createDirectories(subFolderB).toFile()); + createdFiles.add(createFileWithContent(folderA.resolve("subFileB"))); + return createdFiles; + } + + private static File createFileWithContent(Path fileToCreate) throws IOException { + try (InputStream in = new ByteArrayInputStream("some content".getBytes())) { + Files.copy(in, fileToCreate); + } + return fileToCreate.toFile(); + } + + private static TarArchiveEntry getTarArchiveEntry(File tarArchive, String filename) throws IOException { + try (TarArchiveInputStream tarArchiveInputStream = new TarArchiveInputStream( + new GZIPInputStream(new BufferedInputStream(new FileInputStream(tarArchive))))) { + TarArchiveEntry entry; + while ((entry = tarArchiveInputStream.getNextTarEntry()) != null) { + if (entry.getName().equals(filename) + || entry.getName().endsWith("/" + filename) + || entry.getName().equals(filename + "/") + || entry.getName().endsWith("/" + filename + "/")) { + return entry; + } + } + } + return null; + } + + private static int getNumberOfEntryInArchive(File tarArchive) throws IOException { + int numberOfEntries = 0; + try (TarArchiveInputStream tarArchiveInputStream = new TarArchiveInputStream( + new GZIPInputStream(new BufferedInputStream(new FileInputStream(tarArchive))))) { + while ((tarArchiveInputStream.getNextTarEntry()) != null) { + numberOfEntries++; + } + } + return numberOfEntries; } }