Skip to content

Commit

Permalink
Add option to copy symbolic links pointing outside of project directo…
Browse files Browse the repository at this point in the history 
…ry (#1199)

* Add option to copy symbolic links pointing outside of project directory

`digdag push` fails if the project includes a symbolic link that points
outside of the project directory. This is hard to avoid because the
archive package must be self-contained but such files pointed by the
symlinks won't be included in the archive.

This new option provides a solution to it. If the option is given, the
command copies files or directories instead of throwing an exception.

* Better naming for --copy-outgoing-symlinks

* Fixed symlink handling at Archiver

When archiver creates a symbolic link pointing to a directory, files in
it shouldn't be copied. Such files should be copied through the
pointed directory, not though the symbolic link.

* fixed test cases
  • Loading branch information
@frsyuki
frsyuki authored Jun 30, 2020
1 parent 0416072 commit 5a72fa0
Show file tree
Hide file tree
Showing 8 changed files with 271 additions and 73 deletions.
6 changes: 5 additions & 1 deletion digdag-cli/src/main/java/io/digdag/cli/client/Archive.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ public class Archive
@Parameter(names = {"-o", "--output"})
String output = "digdag.archive.tar.gz";

@Parameter(names = {"--copy-outgoing-symlinks"})
boolean copyOutgoingSymlinks = false;

@Override
public void main()
throws Exception
Expand All @@ -52,6 +55,7 @@ public SystemExitException usage(String error)
err.println(" Options:");
err.println(" --project DIR use this directory as the project directory (default: current directory)");
err.println(" -o, --output ARCHIVE.tar.gz output path (default: digdag.archive.tar.gz)");
err.println(" --copy-outgoing-symlinks transform symbolic links to regular files or directories");
Main.showCommonOptions(env, err);
return systemExit(error);
}
Expand Down Expand Up @@ -85,7 +89,7 @@ private void archive(Injector injector)
Path projectPath = (projectDirName == null) ?
Paths.get("").toAbsolutePath() :
Paths.get(projectDirName).normalize().toAbsolutePath();
injector.getInstance(Archiver.class).createArchive(projectPath, Paths.get(output));
injector.getInstance(Archiver.class).createArchive(projectPath, Paths.get(output), copyOutgoingSymlinks);

out.println("Created " + output + ".");
out.println("Use `" + programName + " upload <path.tar.gz> <project> <revision>` to upload it a server.");
Expand Down
161 changes: 96 additions & 65 deletions digdag-cli/src/main/java/io/digdag/cli/client/Archiver.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import java.io.PrintStream;
import java.nio.file.DirectoryStream;
import java.nio.file.Files;
import java.nio.file.LinkOption;
import java.nio.file.Path;
import java.nio.file.attribute.PosixFilePermission;
import java.util.List;
Expand All @@ -43,7 +44,7 @@ class Archiver
this.cf = cf;
}

List<String> createArchive(Path projectPath, Path output)
List<String> createArchive(Path projectPath, Path output, boolean copyOutgoingSymlinks)
throws IOException
{
out.println("Creating " + output + "...");
Expand All @@ -57,15 +58,10 @@ List<String> createArchive(Path projectPath, Path output)
tar.setLongFileMode(TarArchiveOutputStream.LONGFILE_POSIX);

project.listFiles((resourceName, absPath) -> {
if (!Files.isDirectory(absPath)) {
out.println(" Archiving " + resourceName);

TarArchiveEntry e = buildTarArchiveEntry(projectPath, absPath, resourceName);
TarArchiveEntry e = buildFileOrSymlinkEntryOrNull(projectPath, absPath, resourceName, copyOutgoingSymlinks);
if (e != null) {
tar.putArchiveEntry(e);
if (e.isSymbolicLink()) {
out.println(" symlink -> " + e.getLinkName());
}
else {
if (!e.isSymbolicLink()) {
try (InputStream in = Files.newInputStream(absPath)) {
ByteStreams.copy(in, tar);
}
Expand All @@ -75,79 +71,114 @@ List<String> createArchive(Path projectPath, Path output)
if (WorkflowResourceMatcher.defaultMatcher().matches(resourceName, absPath)) {
workflowResources.add(resourceName);
}

// If symbolic link entry is created, don't copy files recursively
return !e.isSymbolicLink();
}
return true;
});
}

return workflowResources.build();
}

private TarArchiveEntry buildTarArchiveEntry(Path projectPath, Path absPath, String name)
private TarArchiveEntry buildFileOrSymlinkEntryOrNull(Path projectPath, Path absPath, String resourceName,
boolean copyOutgoingSymlinks)
throws IOException
{
TarArchiveEntry e;
// If symbolic link, try to create symlink entry
if (Files.isSymbolicLink(absPath)) {
e = new TarArchiveEntry(name, TarConstants.LF_SYMLINK);
Path rawDest = Files.readSymbolicLink(absPath);
Path normalizedAbsDest = absPath.getParent().resolve(rawDest).normalize();

if (!normalizedAbsDest.startsWith(projectPath)) {
throw new IllegalArgumentException(String.format(ENGLISH,
"Invalid symbolic link: Given path '%s' is outside of project directory '%s'", normalizedAbsDest, projectPath));
TarArchiveEntry e = createSymlinkEntryOrNull(projectPath, absPath, resourceName,
copyOutgoingSymlinks);
if (e != null) {
out.println(" Archiving " + resourceName);
out.println(" symlink -> " + e.getLinkName());
return e;
}
// The path is a symbolic link and the file will be copied
}

// absolute path will be invalid on a server. convert it to a relative path
Path normalizedRelativeDest = absPath.getParent().relativize(normalizedAbsDest);

String linkName = normalizedRelativeDest.toString();

// TarArchiveEntry(File) does this normalization but setLinkName doesn't. So do it here:
linkName = linkName.replace(File.separatorChar, '/');
e.setLinkName(linkName);
// Skip directories directories because directories will be created automatically
// when server extracts the archive
if (Files.isDirectory(absPath)) {
return null;
}
else {
e = new TarArchiveEntry(absPath.toFile(), name);
try {
int mode = 0;
for (PosixFilePermission perm : Files.getPosixFilePermissions(absPath)) {
switch (perm) {
case OWNER_READ:
mode |= 0400;
break;
case OWNER_WRITE:
mode |= 0200;
break;
case OWNER_EXECUTE:
mode |= 0100;
break;
case GROUP_READ:
mode |= 0040;
break;
case GROUP_WRITE:
mode |= 0020;
break;
case GROUP_EXECUTE:
mode |= 0010;
break;
case OTHERS_READ:
mode |= 0004;
break;
case OTHERS_WRITE:
mode |= 0002;
break;
case OTHERS_EXECUTE:
mode |= 0001;
break;
default:
// ignore
}

// Create a regular file TarArchiveEntry (follow link if symlink)
out.println(" Archiving " + resourceName);
TarArchiveEntry e = new TarArchiveEntry(absPath.toFile(), resourceName);
try {
int mode = 0;
for (PosixFilePermission perm : Files.getPosixFilePermissions(absPath)) {
switch (perm) {
case OWNER_READ:
mode |= 0400;
break;
case OWNER_WRITE:
mode |= 0200;
break;
case OWNER_EXECUTE:
mode |= 0100;
break;
case GROUP_READ:
mode |= 0040;
break;
case GROUP_WRITE:
mode |= 0020;
break;
case GROUP_EXECUTE:
mode |= 0010;
break;
case OTHERS_READ:
mode |= 0004;
break;
case OTHERS_WRITE:
mode |= 0002;
break;
case OTHERS_EXECUTE:
mode |= 0001;
break;
default:
// ignore
}
e.setMode(mode);
}
catch (UnsupportedOperationException ex) {
// ignore custom mode
e.setMode(mode);
}
catch (UnsupportedOperationException ex) {
// ignore custom mode
}

return e;
}

private TarArchiveEntry createSymlinkEntryOrNull(Path projectPath, Path absPath, String resourceName,
boolean copyOutgoingSymlinks)
throws IOException
{
Path rawDest = Files.readSymbolicLink(absPath);
Path normalizedAbsDest = absPath.getParent().resolve(rawDest).normalize();

if (!normalizedAbsDest.startsWith(projectPath)) {
// outside of projectPath
if (copyOutgoingSymlinks) {
return null;
}
throw new IllegalArgumentException(String.format(ENGLISH,
"Invalid symbolic link: Given path '%s' is outside of project directory '%s'. Consider to add --copy-outgoing-symlinks option", normalizedAbsDest, projectPath));
}

// Create a TarArchiveEntry of symlink
TarArchiveEntry e = new TarArchiveEntry(resourceName, TarConstants.LF_SYMLINK);

// absolute path will be invalid on a server. convert it to a relative path
Path normalizedRelativeDest = absPath.getParent().relativize(normalizedAbsDest);

String linkName = normalizedRelativeDest.toString();

// TarArchiveEntry(File) does this normalization but setLinkName doesn't. So do it here:
linkName = linkName.replace(File.separatorChar, '/');
e.setLinkName(linkName);

return e;
}
}
6 changes: 5 additions & 1 deletion digdag-cli/src/main/java/io/digdag/cli/client/Push.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ public class Push
@Parameter(names = {"--schedule-from"})
String scheduleFromString = null;

@Parameter(names = {"--copy-outgoing-symlinks"})
boolean copyOutgoingSymlinks = false;

@Override
public void mainWithClientException()
throws Exception
Expand All @@ -61,6 +64,7 @@ public SystemExitException usage(String error)
err.println(" --project DIR use this directory as the project directory (default: current directory)");
err.println(" -r, --revision REVISION specific revision name instead of auto-generated UUID");
err.println(" --schedule-from \"yyyy-MM-dd HH:mm:ss Z\" start schedules from this time instead of current time");
err.println(" --copy-outgoing-symlinks transform symbolic links to regular files or directories");
showCommonOptions();
return systemExit(error);
}
Expand Down Expand Up @@ -107,7 +111,7 @@ private void push(String projName)
Path projectPath = (projectDirName == null) ?
Paths.get("").toAbsolutePath() :
Paths.get(projectDirName).normalize().toAbsolutePath();
List<String> workflows = injector.getInstance(Archiver.class).createArchive(projectPath, archivePath);
List<String> workflows = injector.getInstance(Archiver.class).createArchive(projectPath, archivePath, copyOutgoingSymlinks);
out.println("Workflows:");
if (workflows.isEmpty()) {
out.println(" WARNING: This project doesn't include workflows. Usually, this is a mistake.");
Expand Down
6 changes: 3 additions & 3 deletions digdag-core/src/main/java/io/digdag/core/archive/ProjectArchive.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public static String resourceNameToWorkflowName(String resourceName)

public interface PathConsumer
{
public void accept(String resourceName, Path absolutePath) throws IOException;
public boolean accept(String resourceName, Path absolutePath) throws IOException;
}

private final Path projectPath;
Expand Down Expand Up @@ -97,8 +97,8 @@ private static void listFilesRecursively(Path projectPath, Path targetDir, PathC
for (Path path : ds) {
String resourceName = realPathToResourceName(projectPath, path);
if (listed.add(resourceName)) {
consumer.accept(resourceName, path);
if (Files.isDirectory(path)) {
boolean cont = consumer.accept(resourceName, path);
if (cont && Files.isDirectory(path)) {
listFilesRecursively(projectPath, path, consumer, listed);
}
}
Expand Down
1 change: 1 addition & 0 deletions digdag-core/src/main/java/io/digdag/core/archive/ProjectArchiveLoader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ public ProjectArchive load(
errors.add(ex);
}
}
return true;
});

try {
Expand Down
10 changes: 8 additions & 2 deletions digdag-core/src/test/java/io/digdag/core/archive/ProjectArchiveTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,10 @@ public void listFilesLookupRecursively()
}

Map<String, Path> files = new HashMap<>();
projectArchive().listFiles(files::put);
projectArchive().listFiles((name, path) -> {
files.put(name, path);
return true;
});

// keys are normalized path names
ImmutableMap.Builder<String, Path> expected = ImmutableMap.builder();
Expand Down Expand Up @@ -112,7 +115,10 @@ public void listFilesExcludeDotFiles()
}

Map<String, Path> files = new HashMap<>();
projectArchive().listFiles(files::put);
projectArchive().listFiles((name, path) -> {
files.put(name, path);
return true;
});

ImmutableMap.Builder<String, Path> expected = ImmutableMap.builder();
expected.put("d1", d1);
Expand Down
4 changes: 4 additions & 0 deletions digdag-docs/src/command_reference.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -835,6 +835,10 @@ Creates a project archive and upload it to the server. This command uploads work

Example: ``--schedule-from "2017-07-29 00:00:00 +0200"``

:command:`--copy-outside-symlinks`
Transform symlinks to regular files or directories if the symlink points a file or directory outside of the target directory. Without this option, such case fails because the files or directories won't be included unless copying.

Example: ``--copy-outside-symlinks``

download
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Expand Down
Loading

0 comments on commit 5a72fa0

Please sign in to comment.