Change in bazel[master]: Add customized path mounting.
387 views
Skip to first unread message
Xin Gao (Gerrit)
Nov 2, 2016, 4:46:23 PM11/2/16
to bazel-de...@googlegroups.com
Xin Gao uploaded Add customized path mounting. for review.
Add customized path mounting. Change-Id: Ifbacfc0e16bbaedcf5b6d3937799710f2cfa3d58 --- M src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxRunner.java M src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java M src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java M src/main/tools/linux-sandbox-options.cc M src/main/tools/linux-sandbox-options.h M src/main/tools/linux-sandbox-pid1.cc A src/test/java/com/google/devtools/build/lib/sandbox/SandboxOptionsTest.java M src/test/shell/bazel/linux-sandbox_test.sh 8 files changed, 267 insertions(+), 31 deletions(-)
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxRunner.java
index da50cda..a030543 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxRunner.java
@@ -45,7 +45,7 @@
private final Set<Path> writableDirs;
private final Set<Path> inaccessiblePaths;
private final Set<Path> tmpfsPaths;
- private final Set<Path> bindMounts;
+ private final Map<Path, Path> bindMounts;
private final boolean sandboxDebug;
LinuxSandboxRunner(
@@ -56,7 +56,7 @@
Set<Path> writableDirs,
Set<Path> inaccessiblePaths,
Set<Path> tmpfsPaths,
- Set<Path> bindMounts,
+ Map<Path, Path> bindMounts,
boolean verboseFailures,
boolean sandboxDebug) {
super(sandboxExecRoot, verboseFailures);
@@ -155,9 +155,15 @@
fileArgs.add(tmpfsPath.getPathString());
}
- for (Path bindMount : bindMounts) {
- fileArgs.add("-b");
- fileArgs.add(bindMount.getPathString());
+ for (ImmutableMap.Entry<Path, Path> bindMount : bindMounts.entrySet()) {
+ fileArgs.add("-M");
+ fileArgs.add(bindMount.getKey().getPathString());
+
+ // The file is mounted in a custom location inside the sandbox.
+ if (!bindMount.getValue().equals(bindMount.getKey())) {
+ fileArgs.add("-m");
+ fileArgs.add(bindMount.getValue().getPathString());
+ }
}
if (!allowNetwork) {
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java
index b4ed542..45146e4 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java
@@ -14,6 +14,7 @@
package com.google.devtools.build.lib.sandbox;
+import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
import com.google.devtools.build.lib.actions.ExecException;
@@ -159,7 +160,7 @@
getWritableDirs(sandboxExecRoot, spawn.getEnvironment()),
getInaccessiblePaths(),
getTmpfsPaths(),
- getBindMounts(blazeDirs),
+ getBindMounts(blazeDirs, sandboxExecRoot),
verboseFailures,
sandboxOptions.sandboxDebug);
} else {
@@ -175,15 +176,28 @@
return tmpfsPaths.build();
}
- private ImmutableSet<Path> getBindMounts(BlazeDirectories blazeDirs) {
+ private ImmutableMap<Path, Path> getBindMounts(BlazeDirectories blazeDirs, Path sandboxExecRoot) {
Path tmpPath = blazeDirs.getFileSystem().getPath("/tmp");
- ImmutableSet.Builder<Path> bindMounts = ImmutableSet.builder();
+ ImmutableMap.Builder<Path, Path> bindMounts = ImmutableMap.builder();
if (blazeDirs.getWorkspace().startsWith(tmpPath)) {
- bindMounts.add(blazeDirs.getWorkspace());
+ bindMounts.put(blazeDirs.getWorkspace(), blazeDirs.getWorkspace());
}
if (blazeDirs.getOutputBase().startsWith(tmpPath)) {
- bindMounts.add(blazeDirs.getOutputBase());
+ bindMounts.put(blazeDirs.getOutputBase(), blazeDirs.getOutputBase());
}
+
+ for (ImmutableMap.Entry<String, String> additionalMountPath :
+ sandboxOptions.sandboxAdditionalMounts) {
+
+ // If source path is relative, treat it as a relative path inside an external repository
+ Path mountSource =
+ additionalMountPath.getKey().startsWith("/")
+ ? blazeDirs.getFileSystem().getPath(additionalMountPath.getKey())
+ : sandboxExecRoot.getRelative("external").getRelative(additionalMountPath.getKey());
+ bindMounts.put(
+ mountSource, blazeDirs.getFileSystem().getPath(additionalMountPath.getValue()));
+ }
+
return bindMounts.build();
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java
index 909e07c..4d660b1 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java
@@ -14,14 +14,50 @@
package com.google.devtools.build.lib.sandbox;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
+import com.google.devtools.common.options.Converter;
import com.google.devtools.common.options.Option;
import com.google.devtools.common.options.OptionsBase;
+import com.google.devtools.common.options.OptionsParsingException;
import java.util.List;
-/**
- * Options for sandboxed execution.
- */
+/** Options for sandboxed execution. */
public class SandboxOptions extends OptionsBase {
+
+ /**
+ * A converter for customized path mounting pair from the parameter list of a bazel command
+ * invocation. Pairs are expected to have the form "sourece:target.
+ */
+ public static class MountPairConverter implements Converter<ImmutableMap.Entry<String, String>> {
+
+ @Override
+ public ImmutableMap.Entry<String, String> convert(String input) throws OptionsParsingException {
+
+ List<String> paths = Lists.newArrayList();
+ for (String path : input.split("(?<!\\\\):")) { // Split on ':' but not on '\:'
+ if (path != null && !path.trim().isEmpty()) {
+ paths.add(path.replace("\\:", ":"));
+ }
+ }
+
+ if (paths.size() < 1 || paths.size() > 2) {
+ throw new OptionsParsingException(
+ "Input must be a single path to mount inside the sandbox or "
+ + "a mounting pair in the form of 'source:target'");
+ }
+
+ return paths.size() == 1
+ ? Maps.immutableEntry(paths.get(0), paths.get(0))
+ : Maps.immutableEntry(paths.get(0), paths.get(1));
+ }
+
+ @Override
+ public String getTypeDescription() {
+ return "a singe path or a 'source:target' pair";
+ }
+ }
@Option(
name = "ignore_unsupported_sandboxing",
@@ -51,12 +87,23 @@
public List<String> sandboxBlockPath;
@Option(
- name = "sandbox_tmpfs_path",
- allowMultiple = true,
- defaultValue = "",
- category = "config",
- help = "For sandboxed actions, mount an empty, writable directory at this path"
- + " (if supported by the sandboxing implementation, ignored otherwise)."
+ name = "sandbox_tmpfs_path",
+ allowMultiple = true,
+ defaultValue = "",
+ category = "config",
+ help =
+ "For sandboxed actions, mount an empty, writable directory at this path"
+ + " (if supported by the sandboxing implementation, ignored otherwise)."
)
public List<String> sandboxTmpfsPath;
+
+ @Option(
+ name = "sandbox_add_mount_pair",
+ allowMultiple = true,
+ converter = MountPairConverter.class,
+ defaultValue = "",
+ category = "config",
+ help = "Add additional path pair to mount in sandbox."
+ )
+ public List<ImmutableMap.Entry<String, String>> sandboxAdditionalMounts;
}
diff --git a/src/main/tools/linux-sandbox-options.cc b/src/main/tools/linux-sandbox-options.cc
index 49e6eaf..d6200f7 100644
--- a/src/main/tools/linux-sandbox-options.cc
+++ b/src/main/tools/linux-sandbox-options.cc
@@ -69,7 +69,11 @@
" -i <file> make a file or directory inaccessible for the "
"sandboxed process\n"
" -e <dir> mount an empty tmpfs on a directory\n"
- " -b <dir> bind mount a file or directory inside the sandbox\n"
+ " -M/-m <source/target> directory to mount inside the sandbox\n"
+ " Multiple directories can be specified and each of them will be "
+ "mounted readonly.\n"
+ " The -M option specifies which directory to mount, the -m option "
+ "specifies where to\n"
" -N if set, a new network namespace will be created\n"
" -R if set, make the uid/gid be root, otherwise use nobody\n"
" -D if set, debug info will be printed\n"
@@ -106,6 +110,14 @@
return EXIT_SUCCESS;
}
+static void ValidatePreviousMountTarget() {
+ // The previous -M flag wasn't followed by an -m flag, so assume that the
+ // source should be mounted in the sandbox in the same path as outside.
+ if (opt.bind_mount_sources.size() > opt.bind_mount_targets.size()) {
+ opt.bind_mount_targets.push_back(strdup(opt.bind_mount_sources.back()));
+ }
+}
+
// Parses command line flags from an argv array and puts the results into an
// Options structure passed in as an argument.
static void ParseCommandLine(unique_ptr<vector<char *>> args) {
@@ -114,7 +126,7 @@
int c;
while ((c = getopt(args->size(), args->data(),
- ":CS:W:T:t:l:L:w:i:e:b:NRD")) != -1) {
+ ":CS:W:T:t:l:L:w:i:e:M:m:NRD")) != -1) {
switch (c) {
case 'C':
// Shortcut for the "does this system support sandboxing" check.
@@ -193,12 +205,25 @@
}
opt.tmpfs_dirs.push_back(strdup(optarg));
break;
- case 'b':
+ case 'M':
if (optarg[0] != '/') {
Usage(args->front(),
- "The -b option must be used with absolute paths only.");
+ "The -M option must be used with absolute paths only.");
}
- opt.bind_mounts.push_back(strdup(optarg));
+ ValidatePreviousMountTarget();
+ // Add the current source path
+ opt.bind_mount_sources.push_back(strdup(optarg));
+ break;
+ case 'm':
+ if (optarg[0] != '/') {
+ Usage(args->front(),
+ "The -m option must be used with absolute paths only.");
+ }
+ if (opt.bind_mount_sources.size() <= opt.bind_mount_targets.size()) {
+ Usage(args->front(),
+ "The -m option must be preceded by an -M option.");
+ }
+ opt.bind_mount_targets.push_back(strdup(optarg));
break;
case 'N':
opt.create_netns = true;
@@ -218,6 +243,9 @@
}
}
+ // Check if the last -M flag is followed by -m flag
+ ValidatePreviousMountTarget();
+
if (optind < static_cast<int>(args->size())) {
if (opt.args.empty()) {
opt.args.assign(args->begin() + optind, args->end());
diff --git a/src/main/tools/linux-sandbox-options.h b/src/main/tools/linux-sandbox-options.h
index 5554f9e..5f33a06 100644
--- a/src/main/tools/linux-sandbox-options.h
+++ b/src/main/tools/linux-sandbox-options.h
@@ -40,8 +40,10 @@
std::vector<const char *> inaccessible_files;
// Directories where to mount an empty tmpfs (-e)
std::vector<const char *> tmpfs_dirs;
- // Files or directories to explicitly bind mount into the sandbox (-b)
- std::vector<const char *> bind_mounts;
+ // Source of files or directories to explicitly bind mount in the sandbox (-M)
+ std::vector<const char *> bind_mount_sources;
+ // Target of files or directories to explicitly bind mount in the sandbox (-m)
+ std::vector<const char *> bind_mount_targets;
// Create a new network namespace (-N)
bool create_netns;
// Pretend to be root inside the namespace (-R)
diff --git a/src/main/tools/linux-sandbox-pid1.cc b/src/main/tools/linux-sandbox-pid1.cc
index 5f0482e..ea63e2a 100644
--- a/src/main/tools/linux-sandbox-pid1.cc
+++ b/src/main/tools/linux-sandbox-pid1.cc
@@ -249,18 +249,32 @@
// Make sure that our working directory is a mount point. The easiest way to
// do this is by bind-mounting it upon itself.
PRINT_DEBUG("working dir: %s", opt.working_dir);
+ PRINT_DEBUG("sandbox root: %s", opt.sandbox_root_dir);
+
CreateTarget(opt.working_dir + 1, true);
if (mount(opt.working_dir, opt.working_dir + 1, NULL, MS_BIND, NULL) < 0) {
DIE("mount(%s, %s, NULL, MS_BIND, NULL)", opt.working_dir,
opt.working_dir + 1);
}
- for (const char *bind_mount : opt.bind_mounts) {
- PRINT_DEBUG("bind mount: %s", bind_mount);
- CreateTarget(bind_mount + 1, IsDirectory(bind_mount));
- if (mount(bind_mount, bind_mount + 1, NULL, MS_BIND, NULL) < 0) {
- DIE("mount(%s, %s, NULL, MS_BIND, NULL)", bind_mount, bind_mount + 1);
+ int num_bind_mounts = 0;
+ while (num_bind_mounts < (int)opt.bind_mount_sources.size()) {
+ // The source is a path in the host system outside the sandbox
+ const char *source = opt.bind_mount_sources.at(num_bind_mounts);
+ // The target is inside the sandbox
+ char *target =
+ (char *)malloc(strlen(opt.sandbox_root_dir) +
+ strlen(opt.bind_mount_targets.at(num_bind_mounts)) + 1);
+ strcpy(target, opt.sandbox_root_dir);
+ strcat(target, opt.bind_mount_targets.at(num_bind_mounts));
+ PRINT_DEBUG("bind mount: %s -> %s", source, target);
+ // Create target file or directory according to the stats of source
+ CreateTarget(target, IsDirectory(source));
+ if (mount(source, target, NULL, MS_BIND, NULL) < 0) {
+ DIE("mount(%s, %s, NULL, MS_BIND, NULL)", source, target);
}
+ num_bind_mounts++;
+ free(target);
}
for (const char *writable_file : opt.writable_files) {
diff --git a/src/test/java/com/google/devtools/build/lib/sandbox/SandboxOptionsTest.java b/src/test/java/com/google/devtools/build/lib/sandbox/SandboxOptionsTest.java
new file mode 100644
index 0000000..9f18686
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/sandbox/SandboxOptionsTest.java
@@ -0,0 +1,111 @@
+// Copyright 2016 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.sandbox;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.devtools.common.options.OptionsParsingException;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/** Tests for {@code SandboxOptions}. */
+@RunWith(JUnit4.class)
+public class SandboxOptionsTest extends SandboxTestCase {
+
+ private ImmutableMap.Entry<String, String> pathPair;
+
+ @Test
+ public void testParsingAdditionalMounts_SinglePathWithoutColonSucess() throws Exception {
+
+ String source = "/a/bc/def/gh";
+ String target = source;
+ String input = source;
+ pathPair = new SandboxOptions.MountPairConverter().convert(input);
+
+ assertMountPair(pathPair, source, target);
+ }
+
+ @Test
+ public void testParsingAdditionalMounts_SinglePathWithColonSucess() throws Exception {
+
+ String source = "/a/b:c/def/gh";
+ String target = source;
+ String input = "/a/b\\:c/def/gh";
+ pathPair = new SandboxOptions.MountPairConverter().convert(input);
+
+ assertMountPair(pathPair, source, target);
+ }
+
+ @Test
+ public void testParsingAdditionalMounts_PathPairWithoutColonSucess() throws Exception {
+
+ String source = "/a/bc/def/gh";
+ String target = "/1/2/3/4/5";
+ String input = source + ":" + target;
+ pathPair = new SandboxOptions.MountPairConverter().convert(input);
+
+ assertMountPair(pathPair, source, target);
+ }
+
+ @Test
+ public void testParsingAdditionalMounts_PathPairWithColonSucess() throws Exception {
+
+ String source = "/a:/bc:/d:ef/gh";
+ String target = ":/1/2/3/4/5";
+ String input = "/a\\:/bc\\:/d\\:ef/gh:\\:/1/2/3/4/5";
+ pathPair = new SandboxOptions.MountPairConverter().convert(input);
+
+ assertMountPair(pathPair, source, target);
+ }
+
+ @Test
+ public void testParsingAdditionalMounts_TooManyPaths() throws Exception {
+
+ String input = "a/bc/def/gh:/1/2/3:x/y/z";
+ try {
+ pathPair = new SandboxOptions.MountPairConverter().convert(input);
+ fail();
+ } catch (OptionsParsingException e) {
+ assertEquals(
+ e.getMessage(),
+ "Input must be a single path to mount inside the sandbox or "
+ + "a mounting pair in the form of 'source:target'");
+ }
+ }
+
+ @Test
+ public void testParsingAdditionalMounts_EmptyInput() throws Exception {
+
+ String input = "";
+ try {
+ pathPair = new SandboxOptions.MountPairConverter().convert(input);
+ fail();
+ } catch (OptionsParsingException e) {
+ assertEquals(
+ e.getMessage(),
+ "Input must be a single path to mount inside the sandbox or "
+ + "a mounting pair in the form of 'source:target'");
+ }
+ }
+
+ private static void assertMountPair(
+ ImmutableMap.Entry<String, String> pathPair, String source, String target) {
+ assertEquals(pathPair.getKey(), source);
+ assertEquals(pathPair.getValue(), target);
+ }
+}
diff --git a/src/test/shell/bazel/linux-sandbox_test.sh b/src/test/shell/bazel/linux-sandbox_test.sh
index 549f84f..d149f93 100755
--- a/src/test/shell/bazel/linux-sandbox_test.sh
+++ b/src/test/shell/bazel/linux-sandbox_test.sh
@@ -28,12 +28,14 @@
readonly OUT="${OUT_DIR}/outfile"
readonly ERR="${OUT_DIR}/errfile"
readonly SANDBOX_DIR="${OUT_DIR}/sandbox"
+readonly SANDBOX_ROOT="${TEST_TMPDIR}/sandbox.root"
SANDBOX_DEFAULT_OPTS="-W $SANDBOX_DIR"
function set_up {
rm -rf $OUT_DIR
mkdir -p $SANDBOX_DIR
+ mkdir -p $SANDBOX_ROOT
}
function test_basic_functionality() {
@@ -111,6 +113,18 @@
expect_log "child exited normally with exitcode 0"
}
+function test_mount_additional_paths() {
+ mkdir -p ${TEST_TMPDIR}/foo
+ touch ${TEST_TMPDIR}/testfile
+ $linux_sandbox $SANDBOX_DEFAULT_OPTS -D -S ${SANDBOX_ROOT} \
+ -M ${TEST_TMPDIR}/foo -m /tmp/foo \
+ -M ${TEST_TMPDIR}/testfile -m /tmp/sandboxed_testfile \
+ -- /bin/true &> $TEST_log || code=$?
+ expect_log "bind mount: ${TEST_TMPDIR}/foo -> ${SANDBOX_ROOT}/tmp/foo\$"
+ expect_log "bind mount: ${TEST_TMPDIR}/testfile -> ${SANDBOX_ROOT}/tmp/sandboxed_testfile\$"
+ expect_log "child exited normally with exitcode 0"
+}
+
function test_redirect_output() {
$linux_sandbox $SANDBOX_DEFAULT_OPTS -l $OUT -L $ERR -- /bin/bash -c "echo out; echo err >&2" &> $TEST_log || code=$?
assert_equals "out" "$(cat $OUT)"
To view, visit this change. To unsubscribe, visit settings.
Nicolas Lopez (Gerrit)
Nov 3, 2016, 1:17:51 PM11/3/16
to Xin Gao
Nicolas Lopez posted comments on Add customized path mounting..
Patch Set 1: (10 comments)
File src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java:
Patch Set #1, Line 192:
an external repositoryI'm not sure about hardcoding the 'external' here. I think its better to assume paths are relative to the execroot (i.e., if you want to mount a path to an external repo you need to include 'external/reponame' in your path.
File src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java:
Patch Set #1, Line 31:
sourecesource
Patch Set #1, Line 33:
publicfinal
probably we want an error for the else case (or what is the expected behavior in that case)?
help = "For sandboxed actions, mount an empty, writable directory at this path" + " (if supported by the sandboxing implementation, ignored otherwise)."Undo changes in these lines if not related to your cl
File src/main/tools/linux-sandbox-options.cc:
We need to have some confirmation that it will be fine to replace this flag for the -M, otherwise leave it for backward compatibility.
Patch Set #1, Line 76:
specifies where toConsider the case in which the "where to" is repeated several times and deal with it appropriately (error out)?
// Check if the last -M flag is followed by -m flag ValidatePreviousMountTarget();
Just assume there will not be a -m after every -M (add the mount source as a mount target and then replace the last mount target if you do find a -m). Also, lets try to change this to verify that "-M path <some other flag> -m path2" is not valid.
File src/test/java/com/google/devtools/build/lib/sandbox/SandboxOptionsTest.java:
Patch Set #1, Line 28:
publicfinal
lower case after _ here and all methods below.
To view, visit this change. To unsubscribe, visit settings.
Xin Gao (Gerrit)
Nov 4, 2016, 5:21:25 PM11/4/16
to Nicolas Lopez
Xin Gao uploaded patch set #2 to Add customized path mounting..
Add customized path mounting. Change-Id: Ifbacfc0e16bbaedcf5b6d3937799710f2cfa3d58 --- M src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxRunner.java M src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java M src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java M src/main/tools/linux-sandbox-options.cc M src/main/tools/linux-sandbox-options.h M src/main/tools/linux-sandbox-pid1.cc A src/test/java/com/google/devtools/build/lib/sandbox/SandboxOptionsTest.java M src/test/shell/bazel/linux-sandbox_test.sh 8 files changed, 349 insertions(+), 28 deletions(-)
To view, visit this change. To unsubscribe, visit settings.
Xin Gao (Gerrit)
Nov 4, 2016, 5:21:32 PM11/4/16
to Nicolas Lopez
Xin Gao posted comments on Add customized path mounting..
Patch Set 1: (10 comments)
Patch Set #1, Line 192:
an external repositoryI'm not sure about hardcoding the 'external' here. I think its better to as
Patch Set #1, Line 31:
sourecesource
Done
Patch Set #1, Line 33:
publicfinal
Done
probably we want an error for the else case (or what is the expected behavi
Done
help = "For sandboxed actions, mount an empty, writable directory at this path" + " (if supported by the sandboxing implementation, ignored otherwise)."
Undo changes in these lines if not related to your cl
We need to have some confirmation that it will be fine to replace this flag
Will confirm with Bazel team when set out to them for review.
Patch Set #1, Line 76:
specifies where toConsider the case in which the "where to" is repeated several times and dea
The later source path will take effect. Added debug message.
// Check if the last -M flag is followed by -m flag ValidatePreviousMountTarget();
Just assume there will not be a -m after every -M (add the mount source as
Patch Set #1, Line 28:
publicfinal
Done
lower case after _ here and all methods below.
Not to be changed because of previous commit having upper case convention.
To view, visit this change. To unsubscribe, visit settings.
Nicolas Lopez (Gerrit)
Nov 7, 2016, 11:14:48 AM11/7/16
to Xin Gao
Nicolas Lopez posted comments on Add customized path mounting..
Patch Set 2: (7 comments)
File src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxRunner.java:
from paths ...
File src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java:
Patch Set #2, Line 195:
inside an externinside the execution root
Patch Set #2, Line 200:
pathspath
Patch Set #2, Line 200:
laterlatter
Patch Set #2, Line 200:
effecteffect, and a warning will be produced.
File src/main/tools/linux-sandbox-options.cc:
if (optarg[0] != '/') { Usage(args->front(), "The -M option must be used with absolute paths only."); }extract this check to a function and replace it everywhere (function receives the 'name' of the option to generate the message)
File src/main/tools/linux-sandbox-pid1.cc:
Patch Set #2, Line 273:
laterlatter
To view, visit this change. To unsubscribe, visit settings.
Xin Gao (Gerrit)
Nov 7, 2016, 1:18:36 PM11/7/16
to Nicolas Lopez
Xin Gao posted comments on Add customized path mounting..
Patch Set 1: (7 comments)
File src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxRunner.java:
from paths ...
Changed to 'of'
File src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java:
Patch Set #2, Line 195:
Path.getKey())inside the execution root
Done
latter
Done
Done
effect, and a warning will be produced.
Done
File src/main/tools/linux-sandbox-options.cc:
Usage(args->front(), "The -e option must be used with absolute paths only."); } o
extract this check to a function and replace it everywhere (function receiv
Done
File src/main/tools/linux-sandbox-pid1.cc:
Patch Set #2, Line 273:
nt(solatter
Done
To view, visit this change. To unsubscribe, visit settings.
Xin Gao (Gerrit)
Nov 7, 2016, 1:19:27 PM11/7/16
to Nicolas Lopez
Xin Gao uploaded patch set #3 to Add customized path mounting..
Add customized path mounting. Change-Id: Ifbacfc0e16bbaedcf5b6d3937799710f2cfa3d58 --- M src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxRunner.java M src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java M src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java M src/main/tools/linux-sandbox-options.cc M src/main/tools/linux-sandbox-options.h M src/main/tools/linux-sandbox-pid1.cc A src/test/java/com/google/devtools/build/lib/sandbox/SandboxOptionsTest.java M src/test/shell/bazel/linux-sandbox_test.sh 8 files changed, 367 insertions(+), 50 deletions(-)
To view, visit this change. To unsubscribe, visit settings.
Nicolas Lopez (Gerrit)
Nov 7, 2016, 1:56:24 PM11/7/16
to Xin Gao
Nicolas Lopez posted comments on Add customized path mounting..
Patch Set 3: Code-Review+1 (1 comment)
File src/test/java/com/google/devtools/build/lib/sandbox/SandboxOptionsTest.java:
Patch Set #3, Line 101:
"Input " + input + " contains one or more empty paths. Input must be a single path to mounbreak line
To view, visit this change. To unsubscribe, visit settings.
Xin Gao (Gerrit)
Nov 7, 2016, 2:02:00 PM11/7/16
to Nicolas Lopez
Xin Gao posted comments on Add customized path mounting..
Patch Set 3: (1 comment)
File src/test/java/com/google/devtools/build/lib/sandbox/SandboxOptionsTest.java:
Patch Set #3, Line 101:
"Input " + input + " contains one or more empty paths. Input must be a single path to mounbreak line
Done
To view, visit this change. To unsubscribe, visit settings.
Xin Gao (Gerrit)
Nov 7, 2016, 2:02:14 PM11/7/16
to Nicolas Lopez
Xin Gao uploaded patch set #4 to Add customized path mounting..
Add customized path mounting. Change-Id: Ifbacfc0e16bbaedcf5b6d3937799710f2cfa3d58 --- M src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxRunner.java M src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java M src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java M src/main/tools/linux-sandbox-options.cc M src/main/tools/linux-sandbox-options.h M src/main/tools/linux-sandbox-pid1.cc A src/test/java/com/google/devtools/build/lib/sandbox/SandboxOptionsTest.java M src/test/shell/bazel/linux-sandbox_test.sh 8 files changed, 370 insertions(+), 50 deletions(-)
To view, visit this change. To unsubscribe, visit settings.
Damien Martin-guillerez (Gerrit)
Nov 8, 2016, 4:09:14 AM11/8/16
to Xin Gao, Philipp Wollermann, Nicolas Lopez
Damien Martin-guillerez posted comments on Add customized path mounting..
Patch Set 4: I let Philip review this one :)
To view, visit this change. To unsubscribe, visit settings.
Xin Gao (Gerrit)
Nov 9, 2016, 5:34:13 PM11/9/16
to Nicolas Lopez, Philipp Wollermann, Damien Martin-guillerez
Xin Gao uploaded patch set #5 to Add customized path mounting..
Add customized path mounting. Change-Id: Ifbacfc0e16bbaedcf5b6d3937799710f2cfa3d58 --- M src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxRunner.java M src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java M src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java M src/main/tools/linux-sandbox-options.cc M src/main/tools/linux-sandbox-options.h M src/main/tools/linux-sandbox-pid1.cc A src/test/java/com/google/devtools/build/lib/sandbox/SandboxOptionsTest.java M src/test/shell/bazel/linux-sandbox_test.sh 8 files changed, 377 insertions(+), 50 deletions(-)
To view, visit this change. To unsubscribe, visit settings.
Xin Gao (Gerrit)
Nov 14, 2016, 5:55:43 PM11/14/16
to Damien Martin-guillerez, Philipp Wollermann, Nicolas Lopez
Xin Gao posted comments on Add customized path mounting..
Patch Set 5:
Friendly ping~
To view, visit this change. To unsubscribe, visit settings.
Xin Gao (Gerrit)
Nov 17, 2016, 3:46:17 PM11/17/16
to Nicolas Lopez, Philipp Wollermann, Damien Martin-guillerez
Xin Gao uploaded patch set #6 to Add customized path mounting in Bazel sandbox..
Add customized path mounting in Bazel sandbox. Change-Id: Ifbacfc0e16bbaedcf5b6d3937799710f2cfa3d58 --- M src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxRunner.java M src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java M src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java M src/main/tools/linux-sandbox-options.cc M src/main/tools/linux-sandbox-options.h M src/main/tools/linux-sandbox-pid1.cc A src/test/java/com/google/devtools/build/lib/sandbox/SandboxOptionsTest.java M src/test/shell/bazel/bazel_sandboxing_test.sh M src/test/shell/bazel/linux-sandbox_test.sh 9 files changed, 606 insertions(+), 50 deletions(-)
To view, visit this change. To unsubscribe, visit settings.
Philipp Wollermann (Gerrit)
Nov 22, 2016, 5:23:50 AM11/22/16
to Xin Gao, Damien Martin-guillerez, Nicolas Lopez
Philipp Wollermann posted comments on Add customized path mounting in Bazel sandbox..
Patch Set 6:
(6 comments)
As discussed in our last meeting, I'm keeping this CL on hold for a short while in order to try something related to overlayfs + this change. This might solve all known issues. If it doesn't work out, I'll merge this in. :)
File src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxRunner.java:
Patch Set #5, Line 161:
getValueConsidering that a) we never want to mount ontop of the same path twice and b) keys in maps are unique by definition, I'd prefer to swap getValue and getKey in the fileArgs.add() calls here and below. This makes it (IMHO) easier to understand the intention here.
File src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java:
Patch Set #5, Line 183:
ImmutableMapSortedMap
Patch Set #5, Line 183:
getBindMountsrefactor: rename method to getReadOnlyBindMounts, because that's what it is.
Patch Set #5, Line 187:
Maps.newHashMap()I think this should be a sorted map, because otherwise you run into issues when people specify sub dirs before base dirs, e.g. things like this: mount /something/datafiles to /build/data mount /something/buildenv to /build would mysteriously fail, because the latter mount hides the earlier /build/data mount, but the opposite order would work: mount /something/buildenv to /build mount /something/datafiles to /build/data Proposal: SortedMap<Path, Path> bindMounts = new TreeMap<>();
// If a target has more than one source path, the latter one will take effect, // and a warning will be produced. if (bindMounts.containsKey(mountTarget)) { executor .getEventHandler() .handle( Event.warn( String.format( "Target path %s has more than one source path specified. " + "Old source path %s will be overwritten.", mountTarget, bindMounts.get(mountTarget)))); }I think it's normal behavior that a later flag overrides an earlier one - e.g. we don't print a warning if you specify "--spawn_strategy=sandboxed" and then "--spawn_strategy=standalone" in the same command line either. If it's intentional, e.g. to override some site-specific config from your personal ~/.bazelrc, this warning would become annoying pretty quickly with no mechanism for the user to silence it. Personally I'd just leave out the warning here.
File src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java:
Patch Set #5, Line 66:
singetypo: single
To view, visit this change. To unsubscribe, visit settings.
Nicolas Lopez (Gerrit)
Nov 22, 2016, 1:50:18 PM11/22/16
to Xin Gao, Philipp Wollermann, Damien Martin-guillerez
Nicolas Lopez posted comments on Add customized path mounting in Bazel sandbox..
Patch Set 6:
(7 comments)
a few nits
File src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java:
Patch Set #6, Line 221:
ImmutableMap.Builder<Path, Path> immutableBindMounts = ImmutableMap.builder();declare this inline below
File src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java:
add corresponding closing "
File src/test/shell/bazel/bazel_sandboxing_test.sh:
r/ are
Patch Set #6, Line 479:
run timeruntime
However, we are using the customized path mounting feature in Bazel sandbox to mount this tool from inside the toolchain # to let it run successfully.
check line breaks
major_version: "local" minor_version: "" default_target_cpu: "k8" default_toolchain { cpu: "k8" toolchain_identifier: "linux_gnu_x86" } toolchain { abi_version: "gcc" abi_libc_version: "glibc_2.21" builtin_sysroot: "external/x86_64_unknown_linux_gnu/x86_64-unknown-linux-gnu/sysroot" compiler: "gcc" host_system_name: "i686-unknown-linux-gnu" needsPic: true supports_gold_linker: false supports_incremental_linker: false supports_fission: false supports_interface_shared_objects: false supports_normalizing_ar: true supports_start_end_lib: false supports_thin_archives: true target_libc: "glibc_2.19" target_cpu: "k8" target_system_name: "x86_64-unknown-linux-gnu" toolchain_identifier: "linux_gnu_x86" tool_path { name: "ar" path: "bin/x86_64-unknown-linux-gnu-ar" } tool_path { name: "compat-ld" path: "bin/x86_64-unknown-linux-gnu-ld" } tool_path { name: "cpp" path: "bin/x86_64-unknown-linux-gnu-gcc" } tool_path { name: "dwp" path: "bin/x86_64-unknown-linux-gnu-dwp" } tool_path { name: "gcc" path: "bin/x86_64-unknown-linux-gnu-gcc" } tool_path { name: "gcov" path: "bin/x86_64-unknown-linux-gnu-gcov" } tool_path { name: "ld" path: "bin/x86_64-unknown-linux-gnu-ld" } tool_path { name: "nm" path: "bin/x86_64-unknown-linux-gnu-nm" } tool_path { name: "objcopy" path: "bin/x86_64-unknown-linux-gnu-objcopy" } tool_path { name: "objdump" path: "bin/x86_64-unknown-linux-gnu-objdump" } tool_path { name: "strip" path: "bin/x86_64-unknown-linux-gnu-strip" } objcopy_embed_flag: "-I" objcopy_embed_flag: "binary" compiler_flag: "-U_FORTIFY_SOURCE" compiler_flag: "-D_FORTIFY_SOURCE=2" compiler_flag: "-fstack-protector" compiler_flag: "-Wall" compiler_flag: "-Wl,-z,-relro,-z,now" # -B flag indicates path to binutils and other binaries. compiler_flag: "-Bexternal/x86_64_unknown_linux_gnu/x86_64-unknown-linux-gnu/bin" compiler_flag: "-Wunused-but-set-parameter" compiler_flag: "-Wno-free-nonheap-object" compiler_flag: "-fno-omit-frame-pointer" compiler_flag: "-isystem" compiler_flag: "external/x86_64_unknown_linux_gnu/lib/gcc/x86_64-unknown-linux-gnu/4.8.5/include" compiler_flag: "-isystem" compiler_flag: "external/x86_64_unknown_linux_gnu/x86_64-unknown-linux-gnu/sysroot/usr/include" compiler_flag: "-isystem" compiler_flag: "external/x86_64_unknown_linux_gnu/lib/gcc/x86_64-unknown-linux-gnu/4.8.5/include-fixed" compiler_flag: "-isystem" compiler_flag: "external/x86_64_unknown_linux_gnu/x86_64-unknown-linux-gnu/sysroot/usr/include/python2.7" cxx_flag: "-isystem" cxx_flag: "external/x86_64_unknown_linux_gnu/x86_64-unknown-linux-gnu/include/c++/4.8.5/x86_64-unknown-linux-gnu" cxx_flag: "-isystem" cxx_flag: "external/x86_64_unknown_linux_gnu/x86_64-unknown-linux-gnu/include/c++/4.8.5" cxx_flag: "-std=gnu++11" cxx_builtin_include_directory: "%package(@x86_64_unknown_linux_gnu//include)%" cxx_builtin_include_directory: "%package(@x86_64_unknown_linux_gnu//x86_64-unknown-linux-gnu/sysroot/usr/include)%" cxx_builtin_include_directory: "%package(@x86_64_unknown_linux_gnu//x86_64-unknown-linux-gnu/include)%/c++/4.8.5" cxx_builtin_include_directory: "%package(@x86_64_unknown_linux_gnu//lib/gcc/x86_64-unknown-linux-gnu/4.8.5/include)%" cxx_builtin_include_directory: "%package(@x86_64_unknown_linux_gnu//lib/gcc/x86_64-unknown-linux-gnu/4.8.5/include-fixed)%" linker_flag: "-lstdc++" linker_flag: "-latomic" linker_flag: "-lm" linker_flag: "-lpthread" linker_flag: "-Lexternal/x86_64_unknown_linux_gnu/x86_64-unknown-linux-gnu/sysroot/lib" linker_flag: "-Lexternal/x86_64_unknown_linux_gnu/x86_64-unknown-linux-gnu/sysroot/usr/lib" linker_flag: "-Lexternal/x86_64_unknown_linux_gnu/x86_64-unknown-linux-gnu/lib64" linker_flag: "-Bexternal/x86_64_unknown_linux_gnu/x86_64-unknown-linux-gnu/bin" # Hardcode an absolute path to the ldd and libraries folders in all generated dynamically linked binaries and libraries. Worker / host needs to have the toolchain installed in this hardcoded path. linker_flag: "-Wl,--dynamic-linker=/tmp/x86_64-unknown-linux-gnu/sysroot/lib64/ld-2.19.so" linker_flag: "-Wl,-rpath=/tmp/x86_64-unknown-linux-gnu/sysroot/lib64/" # Anticipated future default. # This makes GCC and Clang do what we want when called through symlinks. unfiltered_cxx_flag: "-no-canonical-prefixes" linker_flag: "-no-canonical-prefixes" # Make C++ compilation deterministic. Use linkstamping instead of these # compiler symbols. unfiltered_cxx_flag: "-Wno-builtin-macro-redefined" unfiltered_cxx_flag: "-D__DATE__=\"redacted\"" unfiltered_cxx_flag: "-D__TIMESTAMP__=\"redacted\"" unfiltered_cxx_flag: "-D__TIME__=\"redacted\"" # Security hardening on by default. # Conservative choice; -D_FORTIFY_SOURCE=2 may be unsafe in some cases. # We need to undef it before redefining it as some distributions now have # it enabled by default. compiler_flag: "-U_FORTIFY_SOURCE" compiler_flag: "-fstack-protector" # fPIE is problematic for static linking, which we want to do often now. # compiler_flag: "-fPIE" # linker_flag: "-pie" linker_flag: "-Wl,-z,relro,-z,now" # Enable coloring even if there's no attached terminal. Bazel removes the # escape sequences if --nocolor is specified. # Not compatible with gcc 4.8.5 #compiler_flag: "-fdiagnostics-color=always" # All warnings are enabled. Maybe enable -Werror as well? compiler_flag: "-Wall" # Enable a few more warnings that aren't part of -Wall. compiler_flag: "-Wunused-but-set-parameter" # But disable some that are problematic. compiler_flag: "-Wno-free-nonheap-object" # has false positives # Keep stack frames for debugging, even in opt mode. compiler_flag: "-fno-omit-frame-pointer" # Stamp the binary with a unique identifier. linker_flag: "-Wl,--build-id=md5" linker_flag: "-Wl,--hash-style=gnu" compilation_mode_flags { mode: DBG # Enable debug symbols. compiler_flag: "-g" } compilation_mode_flags { mode: OPT # No debug symbols. # Maybe we should enable https://gcc.gnu.org/wiki/DebugFission for opt or # even generally? However, that can't happen here, as it requires special # handling in Bazel. compiler_flag: "-g0" # Conservative choice for -O # -O3 can increase binary size and even slow down the resulting binaries. # Profile first and / or use FDO if you need better performance than this. compiler_flag: "-O2" # Disable assertions compiler_flag: "-DNDEBUG" # Removal of unused code and data at link time (can this increase binary size in some cases?). compiler_flag: "-ffunction-sections" compiler_flag: "-fdata-sections" linker_flag: "-Wl,--gc-sections" } }Concatenate the lines that you need to the toolchain you download
Patch Set #6, Line 646:
https://asci-gce-backend.appspot.com.storage.googleapis.com/sandbox_testing/bazel_sandboxing_test_test_sandbox_mountLets create a new cloud project for alphasource-toolchain create a new bucket toolchain-testing
To view, visit this change. To unsubscribe, visit settings.
Xin Gao (Gerrit)
Nov 22, 2016, 5:36:30 PM11/22/16
to Nicolas Lopez, Philipp Wollermann, Damien Martin-guillerez
Xin Gao posted comments on Add customized path mounting in Bazel sandbox..
Patch Set 6:
(13 comments)
Patch Set #5, Line 161:
getValueConsidering that a) we never want to mount ontop of the same path twice and
Sorry, I'm not sure if I understand it. The bindMount map here has the target as key and the source as the value. So if we want to use -M to specify the source and -m to specify the target, then we should append the value here. Or do you mean to use -M for the target and -m for the source, and modify the code in sandbox binary accordingly? But they the target and source map will be constructed as two parallel lists in the sandbox binary code. So it doesn't make too much difference I guess?
File src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java:
Patch Set #5, Line 183:
getBindMounts
refactor: rename method to getReadOnlyBindMounts, because that's what it is
Done
Patch Set #5, Line 183:
ImmutableMapSortedMap
Done
Patch Set #5, Line 187:
Maps.newHashMap()I think this should be a sorted map, because otherwise you run into issues
Done
// If a target has more than one source path, the latter one will take effect, // and a warning will be produced. if (bindMounts.containsKey(mountTarget)) { executor .getEventHandler() .handle( Event.warn( String.format( "Target path %s has more than one source path specified. " + "Old source path %s will be overwritten.", mountTarget, bindMounts.get(mountTarget)))); }I think it's normal behavior that a later flag overrides an earlier one - e
Make sense. Removed.
File src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java:
Patch Set #6, Line 221:
ImmutableMap.Builder<Path, Path> immutableBindMounts = ImmutableMap.builder();declare this inline below
This line is removed due to the change to use a sorted map.
File src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java:
Patch Set #5, Line 66:
singetypo: single
add corresponding closing "
Changed to a ' ' pair to be consistent with the error message below.
File src/test/shell/bazel/bazel_sandboxing_test.sh:
r/ are
Done
Patch Set #6, Line 479:
run timeruntime
Done
However, we are using the customized path mounting feature in Bazel sandbox to mount this tool from inside the toolchain # to let it run successfully.
check line breaks
Done
I tried to modified the file and add the two lines to it. But didn't feel right about it. Will discuss with you offline.
Patch Set #6, Line 646:
https://asci-gce-backend.appspot.com.storage.googleapis.com/sandbox_testing/bazel_sandboxing_test_test_sandbox_mountLets create a new cloud project for alphasource-toolchain create a new buck
Done
To view, visit this change. To unsubscribe, visit settings.
Xin Gao (Gerrit)
Nov 22, 2016, 5:39:33 PM11/22/16
to Nicolas Lopez, Philipp Wollermann, Damien Martin-guillerez
Xin Gao uploaded patch set #7 to Add customized path mounting in Bazel sandbox..
Add customized path mounting in Bazel sandbox. Change-Id: Ifbacfc0e16bbaedcf5b6d3937799710f2cfa3d58 --- M src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxRunner.java M src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java M src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java M src/main/tools/linux-sandbox-options.cc M src/main/tools/linux-sandbox-options.h M src/main/tools/linux-sandbox-pid1.cc A src/test/java/com/google/devtools/build/lib/sandbox/SandboxOptionsTest.java M src/test/shell/bazel/bazel_sandboxing_test.sh M src/test/shell/bazel/linux-sandbox_test.sh 9 files changed, 591 insertions(+), 49 deletions(-)
To view, visit this change. To unsubscribe, visit settings.
Xin Gao (Gerrit)
Nov 23, 2016, 12:50:33 PM11/23/16
to Nicolas Lopez, Philipp Wollermann, Damien Martin-guillerez
Xin Gao uploaded patch set #8 to this change.
Add customized path mounting in Bazel sandbox. Change-Id: Ifbacfc0e16bbaedcf5b6d3937799710f2cfa3d58 --- M src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxRunner.java M src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java M src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java M src/main/tools/linux-sandbox-options.cc M src/main/tools/linux-sandbox-options.h M src/main/tools/linux-sandbox-pid1.cc A src/test/java/com/google/devtools/build/lib/sandbox/SandboxOptionsTest.java M src/test/shell/bazel/BUILD M src/test/shell/bazel/bazel_sandboxing_test.sh M src/test/shell/bazel/linux-sandbox_test.sh M src/test/shell/bazel/testdata/BUILD A src/test/shell/bazel/testdata/bazel_sandboxing_test_data/test_sandbox_mount_customized_path_CROSSTOOL 12 files changed, 602 insertions(+), 50 deletions(-)
To view, visit this change. To unsubscribe, visit settings.
Xin Gao (Gerrit)
Nov 30, 2016, 7:00:56 PM11/30/16
to Nicolas Lopez, Philipp Wollermann, Damien Martin-guillerez
Xin Gao uploaded patch set #9 to this change.
Add customized path mounting in Bazel sandbox. Change-Id: Ifbacfc0e16bbaedcf5b6d3937799710f2cfa3d58 --- M src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxRunner.java M src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java M src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextProvider.java M src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java M src/main/tools/linux-sandbox-options.cc M src/main/tools/linux-sandbox-options.h M src/main/tools/linux-sandbox-pid1.cc A src/test/java/com/google/devtools/build/lib/sandbox/SandboxOptionsTest.java M src/test/shell/bazel/BUILD M src/test/shell/bazel/bazel_sandboxing_test.sh M src/test/shell/bazel/linux-sandbox_test.sh M src/test/shell/bazel/testdata/BUILD A src/test/shell/bazel/testdata/bazel_sandboxing_test_data/test_sandbox_mount_customized_path_CROSSTOOL 13 files changed, 728 insertions(+), 54 deletions(-)
To view, visit this change. To unsubscribe, visit settings.
Nicolas Lopez (Gerrit)
Dec 2, 2016, 10:13:09 AM12/2/16
to Xin Gao, Philipp Wollermann, Damien Martin-guillerez
Nicolas Lopez posted comments on this change.
Patch Set 9:
(12 comments)
This is coming along petty well, I think we should be close to done.
File src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java:
Patch Set #9, Line 212:
private void validateBindMountsThese methods are getting a bit more complex now, add javadoc explaining what they do (i.e., what does this one validate)
Patch Set #9, Line 220:
, something is wrongIf target exists, but is not of the same type as the source then we cant mount it.
if (target.exists()) { if (source.isDirectory() && target.isDirectory()) { // We are fine. Nothing to be done. } else if ((source.isFile() || source.isSymbolicLink()) && (target.isFile() || target.isSymbolicLink())) { // We are fine. Nothing to be done. } else {Lets try to write a single if with the actual failure cases, delete both we are fine cases.
Patch Set #9, Line 234:
// Create the mount target according to the type of the source path if do not exist.this comment is for the else condition below? put it there. And rewrite: // Target does not exist; create it according to the type of the source path.
Patch Set #9, Line 241:
// Create an empty file.put comment inside the else if block below
File src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextProvider.java:
Patch Set #9, Line 69:
// Maintain a set of the oldest nonexistent ancestor path of the target patIf paths need to be mounted, maintain a set of the highest level nonexistent ancestor path of the target paths so that we can delete it later.
Patch Set #9, Line 120:
for (Path createdPath : this.createdPaths) {add comment before loop: // Delete any directories we created in order to satisfy mount paths.
File src/test/java/com/google/devtools/build/lib/sandbox/SandboxOptionsTest.java:
Sremove empty first line in all these methods
File src/test/shell/bazel/bazel_sandboxing_test.sh:
Patch Set #9, Line 487:
bazel_sandboxing_test_test_sandbox_mount_customized_path_toolchain_package.this is too long a name, and has test word twice and sandbox word twice (and no need to have toolchain_package in the name)
# Replace the CROSSTOOL file in the toolchain package with the customized one cp ${testdata_path}/bazel_sandboxing_test_data/test_sandbox_mount_customized_path_CROSSTOOL downloaded_toolchain/CROSSTOOLWhy do you need to replace the CROSSTOOL file now that below you are changing the single target_root_placeholder line? why not just have the toolchain package include that CROSSTOOL file?
Patch Set #9, Line 530:
rm -rf ${target_root}/x86_64-unknown-linux-gnuwhy do you have to delete the target parent directory at the end? Shouldn't you be checking that it does not exist (i.e., that bazel did clean it up)?
File src/test/shell/bazel/testdata/BUILD:
Patch Set #9, Line 30:
additional_fileis this just one additional file? Seems like the srcs is an entire directory though?
To view, visit this change. To unsubscribe, visit settings.
Xin Gao (Gerrit)
Dec 2, 2016, 11:38:03 AM12/2/16
to Nicolas Lopez, Philipp Wollermann, Damien Martin-guillerez
Xin Gao uploaded patch set #10 to this change.
Add customized path mounting in Bazel sandbox. Change-Id: Ifbacfc0e16bbaedcf5b6d3937799710f2cfa3d58 --- M src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxRunner.java M src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java M src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextProvider.java M src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java M src/main/tools/linux-sandbox-options.cc M src/main/tools/linux-sandbox-options.h M src/main/tools/linux-sandbox-pid1.cc A src/test/java/com/google/devtools/build/lib/sandbox/SandboxOptionsTest.java M src/test/shell/bazel/bazel_sandboxing_test.sh M src/test/shell/bazel/linux-sandbox_test.sh 10 files changed, 545 insertions(+), 53 deletions(-)
To view, visit this change. To unsubscribe, visit settings.
Xin Gao (Gerrit)
Dec 2, 2016, 11:38:20 AM12/2/16
to Nicolas Lopez, Philipp Wollermann, Damien Martin-guillerez
Xin Gao posted comments on this change.
Patch Set 10:
(12 comments)
These methods are getting a bit more complex now, add javadoc explaining wh
Done
If target exists, but is not of the same type as the source then we cant mo
Done
private void validateBindMounts(SortedMap<Path, Path> bindMounts) throws UserExecException { for (SortedMap.Entry<Path, Path> bindMount : bindMounts.entrySet()) { final Path source = bindMount.getValue(); final Path target = bindMount.getKey(); // Mount source should exist in the file system if (!sourc
Lets try to write a single if with the actual failure cases, delete both we
Done
Patch Set #9, Line 234:
boolean isTargetFile = target.isFile() || target.isSymbolicLink();
this comment is for the else condition below? put it there.
Done
Patch Set #9, Line 241:
target, source));
put comment inside the else if block below
Done
File src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextProvider.java:
Patch Set #9, Line 69:
// If paths need to be mounted, maintain a set of the highest level nonexis
If paths need to be mounted, maintain a set of the highest level nonexisten
Done
Patch Set #9, Line 120:
// Delete any directories we created in ordeadd comment before loop:
Done
File src/test/java/com/google/devtools/build/lib/sandbox/SandboxOptionsTest.java:
String source = "/a/bc/def/gh"; S
remove empty first line in all these methods
Done
File src/test/shell/bazel/bazel_sandboxing_test.sh:
Patch Set #9, Line 487:
mount_path_toolchain.tar.gz
this is too long a name, and has test word twice and sandbox word twice (an
Done
chmod -R 0755 downloaded_toolchain
Why do you need to replace the CROSSTOOL file now that below you are changi
Included the CROSSTOOL file in the toolchain and modify that one directly.
why do you have to delete the target parent directory at the end? Shouldn't
Bazel does the clean up but the sandbox binary does not do the clean up. Added more comments.
File src/test/shell/bazel/testdata/BUILD:
is this just one additional file? Seems like the srcs is an entire director
No longer need to have this filegroup.
To view, visit this change. To unsubscribe, visit settings.
Nicolas Lopez (Gerrit)
Dec 2, 2016, 11:45:03 AM12/2/16
to Xin Gao, Philipp Wollermann, Damien Martin-guillerez
Nicolas Lopez posted comments on this change.
Patch Set 10: Code-Review+1
(2 comments)
lgtm, couple final nits
File src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java:
remove trailing space
Patch Set #10, Line 222:
validatevalidateAndPrepare
To view, visit this change. To unsubscribe, visit settings.
Xin Gao (Gerrit)
Dec 2, 2016, 11:57:33 AM12/2/16
to Nicolas Lopez, Philipp Wollermann, Damien Martin-guillerez
Xin Gao uploaded patch set #11 to this change.
Add customized path mounting in Bazel sandbox. Change-Id: Ifbacfc0e16bbaedcf5b6d3937799710f2cfa3d58 --- M src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxRunner.java M src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java M src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextProvider.java M src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java M src/main/tools/linux-sandbox-options.cc M src/main/tools/linux-sandbox-options.h M src/main/tools/linux-sandbox-pid1.cc A src/test/java/com/google/devtools/build/lib/sandbox/SandboxOptionsTest.java M src/test/shell/bazel/bazel_sandboxing_test.sh M src/test/shell/bazel/linux-sandbox_test.sh 10 files changed, 545 insertions(+), 53 deletions(-)
To view, visit this change. To unsubscribe, visit settings.
Xin Gao (Gerrit)
Dec 2, 2016, 11:57:49 AM12/2/16
to Nicolas Lopez, Philipp Wollermann, Damien Martin-guillerez
Xin Gao posted comments on this change.
Patch Set 11:
(2 comments)
remove trailing space
Done
Patch Set #10, Line 222:
validatevalidateAndPrepare
Done
To view, visit this change. To unsubscribe, visit settings.
Philipp Wollermann (Gerrit)
Dec 6, 2016, 11:19:53 AM12/6/16
to Xin Gao, Nicolas Lopez, Damien Martin-guillerez
Philipp Wollermann posted comments on this change.
Patch Set 11:
(5 comments)
Generally your code looks good, very clean, thanks for that!
I'm still heavily working on the sandbox myself to fix a performance bug that prevents certain users including TensorFlow from using the sandbox. I can't merge a new feature until this is fixed and stabilized, which will take me a few more days.
In the meantime, could you please check whether my understanding of what happens when you pass --sandbox_add_mount_pair=/something:/usr/local is correct? I think it might actually wipe your entire /usr/local directory, or maybe I'm misread something in the code.
Generally, Bazel should not modify the filesystem outside of its output_base and well-known shared temporary directories (e.g. /tmp and /dev/shm). It is not safe to create and then delete directories in other places in the filesystem, because multiple instances of Bazel might be running at the same time, interfering with each other. Or Bazel might crash and the cleanup might never happen.
Would it still be OK for your use case, if you can only bind-mount on top of existing files / directories? I was hoping that we could use overlayfs to create these needed files / directories in the sandbox without modifying the actual filesystem on disk, but this is not an option, as unfortunately overlayfs requires root (even in a user/mount namespace :().
File src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextProvider.java:
Patch Set #11, Line 73:
curPathcurPath is now "/usr/local"
Patch Set #11, Line 75:
while (!curPath.getParentDirectory().exists()) {This loop does nothing, because "/usr/local".getParentDirectory() == "/usr" already exists.
Patch Set #11, Line 78:
createdPaths.add(curPath);We add "/usr/local" to the list of created paths.
Patch Set #11, Line 123:
FileSystemUtils.deleteTree(createdPath);We delete "/usr/local" and all of its contents.
File src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java:
Patch Set #11, Line 115:
public List<ImmutableMap.Entry<String, String>> sandboxAdditionalMounts;If I specify --sandbox_add_mount_pair=/something:/usr/local, I will get an Entry with key = "/something" and value = "/usr/local" here.
To view, visit this change. To unsubscribe, visit settings.
Xin Gao (Gerrit)
Dec 6, 2016, 8:07:47 PM12/6/16
to Nicolas Lopez, Philipp Wollermann, Damien Martin-guillerez
Xin Gao uploaded patch set #12 to this change.
Add customized path mounting in Bazel sandbox. Change-Id: Ifbacfc0e16bbaedcf5b6d3937799710f2cfa3d58 --- M src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxRunner.java M src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java M src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java M src/main/tools/linux-sandbox-options.cc M src/main/tools/linux-sandbox-options.h M src/main/tools/linux-sandbox-pid1.cc A src/test/java/com/google/devtools/build/lib/sandbox/SandboxOptionsTest.java M src/test/shell/bazel/bazel_sandboxing_test.sh M src/test/shell/bazel/linux-sandbox_test.sh 9 files changed, 493 insertions(+), 49 deletions(-)
To view, visit this change. To unsubscribe, visit settings.
Philipp Wollermann (Gerrit)
Dec 16, 2016, 11:33:51 AM12/16/16
to Xin Gao, Nicolas Lopez, Damien Martin-guillerez
Philipp Wollermann posted comments on this change.
Patch Set 12:
(3 comments)
File src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java:
final Path mountSource = additionalMountPath.getKey().startsWith("/") ? blazeDirs.getFileSystem().getPath(additionalMountPath.getKey()) : sandboxExecRoot.getRelative(additionalMountPath.getKey());Interestingly, getRelative() correctly handles absolute paths as well, so you can just replace this block with:
final Path mountSource = sandboxExecRoot.getRelative(additionalMountPath.getKey());
It will do the same thing. :)
File src/main/tools/linux-sandbox-options.h:
Patch Set #12, Line 22:
#include <map>The "map" type isn't used in this file, so we can remove the include here.
File src/main/tools/linux-sandbox-pid1.cc:
int num_bind_mounts = 0; std::map<std::string, std::string> mount_map; while (num_bind_mounts < (int)opt.bind_mount_sources.size()) { // The source is a path in the host system outside the sandbox char *source = strdup(opt.bind_mount_sources.at(num_bind_mounts)); // The target is inside the sandbox char *target = (char *)malloc(strlen(opt.sandbox_root_dir) + strlen(opt.bind_mount_targets.at(num_bind_mounts)) + 1); strcpy(target, opt.sandbox_root_dir); strcat(target, opt.bind_mount_targets.at(num_bind_mounts)); // Multiple sources being mounted to a single target is allowed. // The latter one will take effect. if (mount_map.find(std::string(target)) != mount_map.end()) { PRINT_DEBUG( "Target '%s' has been mounted by '%s' previously, now overwritten by " "'%s'", target, mount_map[std::string(target)].c_str(), source); } else { mount_map[std::string(target)] = std::string(source); } PRINT_DEBUG("bind mount: %s -> %s", source, target); if (mount(source, target, NULL, MS_BIND, NULL) < 0) { DIE("mount(%s, %s, NULL, MS_BIND, NULL)", source, target); } num_bind_mounts++; free(target); }We can simplify this a lot if we just say that it's fine to not print a warning message when there are two mounts to the same target - which I think is fine, because Bazel's Java code can handle that much better and IMHO it's not that important anyway, as you can just look at the "bind mount: a -> b" lines.
Can you try this (untested, but I think it should work)?
for (int i=0; i<opt.bind_mount_sources.size(); i++) { char *source = opt.bind_mount_sources.at(i); char *target = opt.bind_mount_targets.at(i); PRINT_DEBUG("bind mount: %s -> %s", source, target); if (mount(source, target+1, NULL, MS_BIND, NULL) < 0) { DIE("mount(%s, %s, NULL, MS_BIND, NULL)", source, target); } }The reason why we don't need the malloc/strcpy/strcat part is that we're already in the sandbox root directory at this point. By using "target+1" in the mount syscall, we strip the first slash, turning the absolute path into a relative one that will work just fine from our current directory.
To view, visit this change. To unsubscribe, visit settings.
Xin Gao (Gerrit)
Dec 16, 2016, 12:18:21 PM12/16/16
to Nicolas Lopez, Philipp Wollermann, Damien Martin-guillerez
Xin Gao uploaded patch set #13 to this change.
Add customized path mounting in Bazel sandbox. Change-Id: Ifbacfc0e16bbaedcf5b6d3937799710f2cfa3d58 --- M src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxRunner.java M src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java M src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java M src/main/tools/linux-sandbox-options.cc M src/main/tools/linux-sandbox-options.h M src/main/tools/linux-sandbox-pid1.cc A src/test/java/com/google/devtools/build/lib/sandbox/SandboxOptionsTest.java M src/test/shell/bazel/bazel_sandboxing_test.sh M src/test/shell/bazel/linux-sandbox_test.sh 9 files changed, 466 insertions(+), 49 deletions(-)
To view, visit this change. To unsubscribe, visit settings.
Xin Gao (Gerrit)
Dec 16, 2016, 12:18:36 PM12/16/16
to Philipp Wollermann, Nicolas Lopez, Damien Martin-guillerez
Xin Gao posted comments on this change.
Patch Set 13:
(3 comments)
final Path mountSource = sandboxExecRoot.getRelative(additionalMountPath.getKey()); // If a target has more than one source path, the latter one will take effect. bindMounts.put(mountTarget, mountSource); } catch (IllegalArgumentException e) {
Interestingly, getRelative() correctly handles absolute paths as well, so y
The "map" type isn't used in this file, so we can remove the include here.
Done
File src/main/tools/linux-sandbox-pid1.cc:
for (int i = 0; i < (int)opt.bind_mount_sources.size(); i++) { const char *source = opt.bind_mount_sources.at(i); const char *target = opt.bind_mount_targets.at(i); PRINT_DEBUG("bind mount: %s -> %s", source, target); if (mount(source, target + 1, NULL, MS_BIND, NULL) < 0) { DIE("mount(%s, %s, NULL, MS_BIND, NULL)", source, target + 1); } } for (const char *writable_file : opt.writable_files) { PRINT_DEBUG("writable: %s", writable_file); if (mount(writable_file, writable_file + 1, NULL, MS_BIND, NULL) < 0) { DIE("mount(%s, %s, NULL, MS_BIND, NULL)", writable_file, writable_file + 1); } } SetupHelperFiles(); for (const char *inaccessible_file : opt.inaccessible_files) { struct stat sb; if (stat(inaccessible_file, &sb) < 0) { DIE("stat(%s)", inaccessible_file); } if (S_ISDIR(sb.st_mode)) { PRINT_DEBUG("inaccessible dir: %s", inaccessible_file); if (mount(global_inaccessible_directory, inaccessible_file + 1, NULL, MS_BIND, NULL) < 0) {
We can simplify this a lot if we just say that it's fine to not print a war
Agree. Done.
To view, visit this change. To unsubscribe, visit settings.
Philipp Wollermann (Gerrit)
Dec 19, 2016, 3:47:45 AM12/19/16
to Xin Gao, Nicolas Lopez, Damien Martin-guillerez
Philipp Wollermann posted comments on this change.
Patch Set 13: Verified+1 Code-Review+2
Thanks so much for the effort. This is now possibly the most polished CL for Bazel in a long time. :) Merging!
Klaus Aehlig (Gerrit)
Dec 20, 2016, 8:35:30 AM12/20/16
to Xin Gao, Nicolas Lopez, Philipp Wollermann, Bazel CI, Damien Martin-guillerez
Klaus Aehlig uploaded patch set #14 to this change.
Add customized path mounting in Bazel sandbox. RELNOTES: New flag --sandbox_add_mount_pair to specify customized source:target path pairs to bind mount inside the sandbox. -- Change-Id: Ifbacfc0e16bbaedcf5b6d3937799710f2cfa3d58 Reviewed-on: https://cr.bazel.build/7150 PiperOrigin-RevId: 142542381 MOS_MIGRATED_REVID=142542381 --- M src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxRunner.java M src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java M src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java M src/main/tools/linux-sandbox-options.cc M src/main/tools/linux-sandbox-options.h M src/main/tools/linux-sandbox-pid1.cc A src/test/java/com/google/devtools/build/lib/sandbox/SandboxOptionsTest.java M src/test/shell/bazel/bazel_sandboxing_test.sh M src/test/shell/bazel/linux-sandbox_test.sh 9 files changed, 465 insertions(+), 49 deletions(-)
To view, visit this change. To unsubscribe, visit settings.
Klaus Aehlig (Gerrit)
Dec 20, 2016, 8:35:31 AM12/20/16
to Xin Gao, Bazel CI, Philipp Wollermann, Nicolas Lopez, Damien Martin-guillerez
Klaus Aehlig merged this change.
Add customized path mounting in Bazel sandbox. RELNOTES: New flag --sandbox_add_mount_pair to specify customized source:target path pairs to bind mount inside the sandbox. -- Change-Id: Ifbacfc0e16bbaedcf5b6d3937799710f2cfa3d58 Reviewed-on: https://cr.bazel.build/7150 PiperOrigin-RevId: 142542381 MOS_MIGRATED_REVID=142542381 --- M src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxRunner.java M src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java M src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java M src/main/tools/linux-sandbox-options.cc M src/main/tools/linux-sandbox-options.h M src/main/tools/linux-sandbox-pid1.cc A src/test/java/com/google/devtools/build/lib/sandbox/SandboxOptionsTest.java M src/test/shell/bazel/bazel_sandboxing_test.sh M src/test/shell/bazel/linux-sandbox_test.sh 9 files changed, 465 insertions(+), 49 deletions(-)
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxRunner.java
index da50cda..97151e2 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxRunner.java
@@ -45,7 +45,8 @@
private final Set<Path> writableDirs;
private final Set<Path> inaccessiblePaths;
private final Set<Path> tmpfsPaths;
- private final Set<Path> bindMounts;
+ // a <target, source> mapping of paths to bind mount
+ private final Map<Path, Path> bindMounts;
private final boolean sandboxDebug;
LinuxSandboxRunner(
@@ -56,7 +57,7 @@
Set<Path> writableDirs,
Set<Path> inaccessiblePaths,
Set<Path> tmpfsPaths,
- Set<Path> bindMounts,
+ Map<Path, Path> bindMounts,
boolean verboseFailures,
boolean sandboxDebug) {
super(sandboxExecRoot, verboseFailures);
@@ -155,9 +156,15 @@
fileArgs.add(tmpfsPath.getPathString());
}
- for (Path bindMount : bindMounts) {
- fileArgs.add("-b");
- fileArgs.add(bindMount.getPathString());
+ for (ImmutableMap.Entry<Path, Path> bindMount : bindMounts.entrySet()) {
+ fileArgs.add("-M");
+ fileArgs.add(bindMount.getValue().getPathString());
+
+ // The file is mounted in a custom location inside the sandbox.
+ if (!bindMount.getKey().equals(bindMount.getValue())) {
+ fileArgs.add("-m");
+ fileArgs.add(bindMount.getKey().getPathString());
+ }
}
if (!allowNetwork) {
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java
index b4ed542..dc4578e 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java
@@ -14,7 +14,9 @@
package com.google.devtools.build.lib.sandbox;
+import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Maps;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
import com.google.devtools.build.lib.actions.ExecException;
import com.google.devtools.build.lib.actions.ExecutionStrategy;
@@ -31,6 +33,7 @@
import com.google.devtools.build.lib.vfs.PathFragment;
import java.io.IOException;
import java.util.Set;
+import java.util.SortedMap;
import java.util.UUID;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicReference;
@@ -149,7 +152,8 @@
}
private SandboxRunner getSandboxRunner(
- Spawn spawn, Path sandboxPath, Path sandboxExecRoot, Path sandboxTempDir) {
+ Spawn spawn, Path sandboxPath, Path sandboxExecRoot, Path sandboxTempDir)
+ throws UserExecException {
if (fullySupported) {
return new LinuxSandboxRunner(
execRoot,
@@ -159,7 +163,7 @@
getWritableDirs(sandboxExecRoot, spawn.getEnvironment()),
getInaccessiblePaths(),
getTmpfsPaths(),
- getBindMounts(blazeDirs),
+ getReadOnlyBindMounts(blazeDirs, sandboxExecRoot),
verboseFailures,
sandboxOptions.sandboxDebug);
} else {
@@ -175,15 +179,71 @@
return tmpfsPaths.build();
}
- private ImmutableSet<Path> getBindMounts(BlazeDirectories blazeDirs) {
+ private SortedMap<Path, Path> getReadOnlyBindMounts(
+ BlazeDirectories blazeDirs, Path sandboxExecRoot) throws UserExecException {
Path tmpPath = blazeDirs.getFileSystem().getPath("/tmp");
- ImmutableSet.Builder<Path> bindMounts = ImmutableSet.builder();
+ final SortedMap<Path, Path> bindMounts = Maps.newTreeMap();
if (blazeDirs.getWorkspace().startsWith(tmpPath)) {
- bindMounts.add(blazeDirs.getWorkspace());
+ bindMounts.put(blazeDirs.getWorkspace(), blazeDirs.getWorkspace());
}
if (blazeDirs.getOutputBase().startsWith(tmpPath)) {
- bindMounts.add(blazeDirs.getOutputBase());
+ bindMounts.put(blazeDirs.getOutputBase(), blazeDirs.getOutputBase());
}
- return bindMounts.build();
+ for (ImmutableMap.Entry<String, String> additionalMountPath :
+ sandboxOptions.sandboxAdditionalMounts) {
+ try {
+ final Path mountTarget = blazeDirs.getFileSystem().getPath(additionalMountPath.getValue());
+ // If source path is relative, treat it as a relative path inside the execution root
+ final Path mountSource = sandboxExecRoot.getRelative(additionalMountPath.getKey());
+ // If a target has more than one source path, the latter one will take effect.
+ bindMounts.put(mountTarget, mountSource);
+ } catch (IllegalArgumentException e) {
+ throw new UserExecException(
+ String.format("Error occurred when analyzing bind mount pairs. %s", e.getMessage()));
+ }
+ }
+ validateBindMounts(bindMounts);
+ return bindMounts;
+ }
+
+ /**
+ * This method does the following things: - If mount source does not exist on the host system,
+ * throw an error message - If mount target exists, check whether the source and target are of the
+ * same type - If mount target does not exist on the host system, throw an error message
+ *
+ * @param bindMounts the bind mounts map with target as key and source as value
+ * @throws UserExecException
+ */
+ private void validateBindMounts(SortedMap<Path, Path> bindMounts) throws UserExecException {
+ for (SortedMap.Entry<Path, Path> bindMount : bindMounts.entrySet()) {
+ final Path source = bindMount.getValue();
+ final Path target = bindMount.getKey();
+ // Mount source should exist in the file system
+ if (!source.exists()) {
+ throw new UserExecException(String.format("Mount source '%s' does not exist.", source));
+ }
+ // If target exists, but is not of the same type as the source, then we cannot mount it.
+ if (target.exists()) {
+ boolean areBothDirectories = source.isDirectory() && target.isDirectory();
+ boolean isSourceFile = source.isFile() || source.isSymbolicLink();
+ boolean isTargetFile = target.isFile() || target.isSymbolicLink();
+ boolean areBothFiles = isSourceFile && isTargetFile;
+ if (!(areBothDirectories || areBothFiles)) {
+ // Source and target are not of the same type; we cannot mount it.
+ throw new UserExecException(
+ String.format(
+ "Mount target '%s' is not of the same type as mount source '%s'.",
+ target, source));
+ }
+ } else {
+ // Mount target should exist in the file system
+ throw new UserExecException(
+ String.format(
+ "Mount target '%s' does not exist. Bazel only supports bind mounting on top of "
+ + "existing files/directories. Please create an empty file or directory at "
+ + "the mount target path according to the type of mount source.",
+ target));
+ }
+ }
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java
index 909e07c..b8ceb80 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java
@@ -14,14 +14,58 @@
package com.google.devtools.build.lib.sandbox;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
+import com.google.devtools.common.options.Converter;
import com.google.devtools.common.options.Option;
import com.google.devtools.common.options.OptionsBase;
+import com.google.devtools.common.options.OptionsParsingException;
import java.util.List;
-/**
- * Options for sandboxed execution.
- */
+/** Options for sandboxed execution. */
public class SandboxOptions extends OptionsBase {
+
+ /**
+ * A converter for customized path mounting pair from the parameter list of a bazel command
+ * invocation. Pairs are expected to have the form 'source:target'.
+ */
+ public static final class MountPairConverter
+ implements Converter<ImmutableMap.Entry<String, String>> {
+
+ @Override
+ public ImmutableMap.Entry<String, String> convert(String input) throws OptionsParsingException {
+
+ List<String> paths = Lists.newArrayList();
+ for (String path : input.split("(?<!\\\\):")) { // Split on ':' but not on '\:'
+ if (path != null && !path.trim().isEmpty()) {
+ paths.add(path.replace("\\:", ":"));
+ } else {
+ throw new OptionsParsingException(
+ "Input "
+ + input
+ + " contains one or more empty paths. "
+ + "Input must be a single path to mount inside the sandbox or "
+ + "a mounting pair in the form of 'source:target'");
+ }
+ }
+
+ if (paths.size() < 1 || paths.size() > 2) {
+ throw new OptionsParsingException(
+ "Input must be a single path to mount inside the sandbox or "
+ + "a mounting pair in the form of 'source:target'");
+ }
+
+ return paths.size() == 1
+ ? Maps.immutableEntry(paths.get(0), paths.get(0))
+ : Maps.immutableEntry(paths.get(0), paths.get(1));
+ }
+
+ @Override
+ public String getTypeDescription() {
+ return "a single path or a 'source:target' pair";
+ }
+ }
@Option(
name = "ignore_unsupported_sandboxing",
@@ -59,4 +103,14 @@
+ " (if supported by the sandboxing implementation, ignored otherwise)."
)
public List<String> sandboxTmpfsPath;
+
+ @Option(
+ name = "sandbox_add_mount_pair",
+ allowMultiple = true,
+ converter = MountPairConverter.class,
+ defaultValue = "",
+ category = "config",
+ help = "Add additional path pair to mount in sandbox."
+ )
+ public List<ImmutableMap.Entry<String, String>> sandboxAdditionalMounts;
}
diff --git a/src/main/tools/linux-sandbox-options.cc b/src/main/tools/linux-sandbox-options.cc
index 49e6eaf..8f8c15d 100644
--- a/src/main/tools/linux-sandbox-options.cc
+++ b/src/main/tools/linux-sandbox-options.cc
@@ -69,7 +69,11 @@
" -i <file> make a file or directory inaccessible for the "
"sandboxed process\n"
" -e <dir> mount an empty tmpfs on a directory\n"
- " -b <dir> bind mount a file or directory inside the sandbox\n"
+ " -M/-m <source/target> directory to mount inside the sandbox\n"
+ " Multiple directories can be specified and each of them will be "
+ "mounted readonly.\n"
+ " The -M option specifies which directory to mount, the -m option "
+ "specifies where to\n"
" -N if set, a new network namespace will be created\n"
" -R if set, make the uid/gid be root, otherwise use nobody\n"
" -D if set, debug info will be printed\n"
@@ -106,15 +110,24 @@
return EXIT_SUCCESS;
}
+static void ValidateIsAbsolutePath(char *path, char *program_name, char flag) {
+ if (path[0] != '/') {
+ Usage(program_name, "The -%c option must be used with absolute paths only.",
+ flag);
+ }
+}
+
// Parses command line flags from an argv array and puts the results into an
// Options structure passed in as an argument.
static void ParseCommandLine(unique_ptr<vector<char *>> args) {
extern char *optarg;
extern int optind, optopt;
int c;
+ bool source_specified;
while ((c = getopt(args->size(), args->data(),
- ":CS:W:T:t:l:L:w:i:e:b:NRD")) != -1) {
+ ":CS:W:T:t:l:L:w:i:e:M:m:NRD")) != -1) {
+ if (c != 'M' && c != 'm') source_specified = false;
switch (c) {
case 'C':
// Shortcut for the "does this system support sandboxing" check.
@@ -122,22 +135,16 @@
break;
case 'S':
if (opt.sandbox_root_dir == NULL) {
- if (optarg[0] != '/') {
- Usage(args->front(),
- "The -r option must be used with absolute paths only.");
- }
+ ValidateIsAbsolutePath(optarg, args->front(), static_cast<char>(c));
opt.sandbox_root_dir = strdup(optarg);
} else {
Usage(args->front(),
- "Multiple root directories (-r) specified, expected one.");
+ "Multiple root directories (-S) specified, expected one.");
}
break;
case 'W':
if (opt.working_dir == NULL) {
- if (optarg[0] != '/') {
- Usage(args->front(),
- "The -W option must be used with absolute paths only.");
- }
+ ValidateIsAbsolutePath(optarg, args->front(), static_cast<char>(c));
opt.working_dir = strdup(optarg);
} else {
Usage(args->front(),
@@ -173,32 +180,33 @@
}
break;
case 'w':
- if (optarg[0] != '/') {
- Usage(args->front(),
- "The -w option must be used with absolute paths only.");
- }
+ ValidateIsAbsolutePath(optarg, args->front(), static_cast<char>(c));
opt.writable_files.push_back(strdup(optarg));
break;
case 'i':
- if (optarg[0] != '/') {
- Usage(args->front(),
- "The -i option must be used with absolute paths only.");
- }
+ ValidateIsAbsolutePath(optarg, args->front(), static_cast<char>(c));
opt.inaccessible_files.push_back(strdup(optarg));
break;
case 'e':
- if (optarg[0] != '/') {
- Usage(args->front(),
- "The -e option must be used with absolute paths only.");
- }
+ ValidateIsAbsolutePath(optarg, args->front(), static_cast<char>(c));
opt.tmpfs_dirs.push_back(strdup(optarg));
break;
- case 'b':
- if (optarg[0] != '/') {
+ case 'M':
+ ValidateIsAbsolutePath(optarg, args->front(), static_cast<char>(c));
+ // Add the current source path to both source and target lists
+ opt.bind_mount_sources.push_back(strdup(optarg));
+ opt.bind_mount_targets.push_back(strdup(optarg));
+ source_specified = true;
+ break;
+ case 'm':
+ ValidateIsAbsolutePath(optarg, args->front(), static_cast<char>(c));
+ if (!source_specified) {
Usage(args->front(),
- "The -b option must be used with absolute paths only.");
+ "The -m option must be strictly preceded by an -M option.");
}
- opt.bind_mounts.push_back(strdup(optarg));
+ opt.bind_mount_targets.pop_back();
+ opt.bind_mount_targets.push_back(strdup(optarg));
+ source_specified = false;
break;
case 'N':
opt.create_netns = true;
diff --git a/src/main/tools/linux-sandbox-options.h b/src/main/tools/linux-sandbox-options.h
index 5554f9e..5f33a06 100644
--- a/src/main/tools/linux-sandbox-options.h
+++ b/src/main/tools/linux-sandbox-options.h
@@ -40,8 +40,10 @@
std::vector<const char *> inaccessible_files;
// Directories where to mount an empty tmpfs (-e)
std::vector<const char *> tmpfs_dirs;
- // Files or directories to explicitly bind mount into the sandbox (-b)
- std::vector<const char *> bind_mounts;
+ // Source of files or directories to explicitly bind mount in the sandbox (-M)
+ std::vector<const char *> bind_mount_sources;
+ // Target of files or directories to explicitly bind mount in the sandbox (-m)
+ std::vector<const char *> bind_mount_targets;
// Create a new network namespace (-N)
bool create_netns;
// Pretend to be root inside the namespace (-R)
diff --git a/src/main/tools/linux-sandbox-pid1.cc b/src/main/tools/linux-sandbox-pid1.cc
index 4feedd9..5e0dff1 100644
--- a/src/main/tools/linux-sandbox-pid1.cc
+++ b/src/main/tools/linux-sandbox-pid1.cc
@@ -249,17 +249,20 @@
// Make sure that our working directory is a mount point. The easiest way to
// do this is by bind-mounting it upon itself.
PRINT_DEBUG("working dir: %s", opt.working_dir);
+ PRINT_DEBUG("sandbox root: %s", opt.sandbox_root_dir);
+
CreateTarget(opt.working_dir + 1, true);
if (mount(opt.working_dir, opt.working_dir + 1, NULL, MS_BIND, NULL) < 0) {
DIE("mount(%s, %s, NULL, MS_BIND, NULL)", opt.working_dir,
opt.working_dir + 1);
}
- for (const char *bind_mount : opt.bind_mounts) {
- PRINT_DEBUG("bind mount: %s", bind_mount);
- CreateTarget(bind_mount + 1, IsDirectory(bind_mount));
- if (mount(bind_mount, bind_mount + 1, NULL, MS_BIND, NULL) < 0) {
- DIE("mount(%s, %s, NULL, MS_BIND, NULL)", bind_mount, bind_mount + 1);
+ for (size_t i = 0; i < opt.bind_mount_sources.size(); i++) {
+ const char *source = opt.bind_mount_sources.at(i);
+ const char *target = opt.bind_mount_targets.at(i);
+ PRINT_DEBUG("bind mount: %s -> %s", source, target);
+ if (mount(source, target + 1, NULL, MS_BIND, NULL) < 0) {
+ DIE("mount(%s, %s, NULL, MS_BIND, NULL)", source, target + 1);
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/sandbox/SandboxOptionsTest.java b/src/test/java/com/google/devtools/build/lib/sandbox/SandboxOptionsTest.java
new file mode 100644
index 0000000..0441f78
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/sandbox/SandboxOptionsTest.java
@@ -0,0 +1,104 @@
+// Copyright 2016 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.sandbox;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.devtools.common.options.OptionsParsingException;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/** Tests for {@code SandboxOptions}. */
+@RunWith(JUnit4.class)
+public final class SandboxOptionsTest extends SandboxTestCase {
+
+ private ImmutableMap.Entry<String, String> pathPair;
+
+ @Test
+ public void testParsingAdditionalMounts_SinglePathWithoutColonSucess() throws Exception {
+ String source = "/a/bc/def/gh";
+ String target = source;
+ String input = source;
+ pathPair = new SandboxOptions.MountPairConverter().convert(input);
+ assertMountPair(pathPair, source, target);
+ }
+
+ @Test
+ public void testParsingAdditionalMounts_SinglePathWithColonSucess() throws Exception {
+ String source = "/a/b:c/def/gh";
+ String target = source;
+ String input = "/a/b\\:c/def/gh";
+ pathPair = new SandboxOptions.MountPairConverter().convert(input);
+ assertMountPair(pathPair, source, target);
+ }
+
+ @Test
+ public void testParsingAdditionalMounts_PathPairWithoutColonSucess() throws Exception {
+ String source = "/a/bc/def/gh";
+ String target = "/1/2/3/4/5";
+ String input = source + ":" + target;
+ pathPair = new SandboxOptions.MountPairConverter().convert(input);
+ assertMountPair(pathPair, source, target);
+ }
+
+ @Test
+ public void testParsingAdditionalMounts_PathPairWithColonSucess() throws Exception {
+ String source = "/a:/bc:/d:ef/gh";
+ String target = ":/1/2/3/4/5";
+ String input = "/a\\:/bc\\:/d\\:ef/gh:\\:/1/2/3/4/5";
+ pathPair = new SandboxOptions.MountPairConverter().convert(input);
+ assertMountPair(pathPair, source, target);
+ }
+
+ @Test
+ public void testParsingAdditionalMounts_TooManyPaths() throws Exception {
+ String input = "a/bc/def/gh:/1/2/3:x/y/z";
+ try {
+ pathPair = new SandboxOptions.MountPairConverter().convert(input);
+ fail();
+ } catch (OptionsParsingException e) {
+ assertEquals(
+ e.getMessage(),
+ "Input must be a single path to mount inside the sandbox or "
+ + "a mounting pair in the form of 'source:target'");
+ }
+ }
+
+ @Test
+ public void testParsingAdditionalMounts_EmptyInput() throws Exception {
+ String input = "";
+ try {
+ pathPair = new SandboxOptions.MountPairConverter().convert(input);
+ fail();
+ } catch (OptionsParsingException e) {
+ assertEquals(
+ e.getMessage(),
+ "Input "
+ + input
+ + " contains one or more empty paths. "
+ + "Input must be a single path to mount inside the sandbox or "
+ + "a mounting pair in the form of 'source:target'");
+ }
+ }
+
+ private static void assertMountPair(
+ ImmutableMap.Entry<String, String> pathPair, String source, String target) {
+ assertEquals(pathPair.getKey(), source);
+ assertEquals(pathPair.getValue(), target);
+ }
+}
diff --git a/src/test/shell/bazel/bazel_sandboxing_test.sh b/src/test/shell/bazel/bazel_sandboxing_test.sh
index 5d2199b..df015bb 100755
--- a/src/test/shell/bazel/bazel_sandboxing_test.sh
+++ b/src/test/shell/bazel/bazel_sandboxing_test.sh
@@ -445,6 +445,97 @@
expect_log "Executing genrule //:test failed: linux-sandbox failed: error executing command"
}
+function test_sandbox_mount_customized_path () {
+ # Create BUILD file
+ cat > BUILD <<'EOF'
+package(default_visibility = ["//visibility:public"])
+
+cc_binary(
+ name = "hello-world",
+ srcs = ["hello-world.cc"],
+)
+EOF
+
+ # Create cc file
+ cat > hello-world.cc << 'EOF'
+#include <iostream>
+int main(int argc, char** argv) {
+ std::cout << "Hello, world!" << std::endl;
+ return 0;
+}
+EOF
+
+ # Create WORKSPACE file
+ cat > WORKSPACE <<'EOF'
+local_repository(
+ name = 'x86_64_unknown_linux_gnu',
+ path = './downloaded_toolchain',
+)
+EOF
+
+ # Prepare the sandbox root
+ SANDBOX_ROOT="${TEST_TMPDIR}/sandbox.root"
+ mkdir -p ${SANDBOX_ROOT}
+
+ # Define the mount source and target.
+ source="${TEST_TMPDIR}/workspace/downloaded_toolchain/x86_64-unknown-linux-gnu/sysroot/lib64/ld-2.19.so"
+ target_root="${TEST_SRCDIR}/mount_targets"
+ target_folder="${target_root}/x86_64-unknown-linux-gnu/sysroot/lib64"
+ target="${target_folder}/ld-2.19.so"
+
+ # Download the toolchain package and unpack it.
+ wget -q https://asci-toolchain.appspot.com.storage.googleapis.com/toolchain-testing/mount_path_toolchain.tar.gz
+ mkdir downloaded_toolchain
+ tar -xf mount_path_toolchain.tar.gz -C ./downloaded_toolchain
+ chmod -R 0755 downloaded_toolchain
+
+ # Replace the target_root_placeholder with the actual target_root
+ sed -i "s|target_root_placeholder|$target_root|g" downloaded_toolchain/CROSSTOOL
+
+ # Prepare the bazel command flags
+ flags="--crosstool_top=@x86_64_unknown_linux_gnu//:toolchain --verbose_failures --spawn_strategy=sandboxed"
+ flags="${flags} --sandbox_add_mount_pair=${source}:${target}"
+
+ # Execute the bazel build command without creating the target. Should fail.
+ bazel clean --expunge &> $TEST_log
+ bazel build $flags //:hello-world &> $TEST_log && fail "Should fail"
+ expect_log "Bazel only supports bind mounting on top of existing files/directories."
+
+ # Create the mount target manually as Bazel does not create target paths
+ mkdir -p ${target_folder}
+ touch ${target}
+
+ # Execute bazel build command again. Should build.
+ bazel clean --expunge &> $TEST_log
+ bazel build $flags //:hello-world &> $TEST_log || fail "Should build"
+
+ # Remove the mount target folder as Bazel does not do the cleanup
+ rm -rf ${target_root}/x86_64-unknown-linux-gnu
+
+ # Assert that output binary exists
+ test -f bazel-bin/hello-world || fail "output not found"
+
+ # Use linux_sandbox binary to run bazel-bin/hello-world binary in the sandbox environment
+ # First, no path mounting. The execution should fail.
+ echo "Run the binary bazel-bin/hello-world without mounting the path"
+ $linux_sandbox -D -S ${SANDBOX_ROOT} -- bazel-bin/hello-world &> $TEST_log || code=$?
+ expect_log "child exited normally with exitcode 1"
+
+ # Second, with path mounting. The execution should succeed.
+ echo "Run the binary bazel-bin/hello-world with mounting the path"
+ # Create the mount target manually as sandbox binary does not create target paths
+ mkdir -p ${target_folder}
+ touch ${target}
+ $linux_sandbox -D -S ${SANDBOX_ROOT} \
+ -M ${source} \
+ -m ${target} \
+ -- bazel-bin/hello-world &> $TEST_log || code=$?
+ expect_log "Hello, world!"
+ expect_log "child exited normally with exitcode 0"
+ # Remove the mount target folder as sandbox binary does not do the cleanup
+ rm -rf ${target_root}/x86_64-unknown-linux-gnu
+}
+
# The test shouldn't fail if the environment doesn't support running it.
check_supported_platform || exit 0
check_sandbox_allowed || exit 0
diff --git a/src/test/shell/bazel/linux-sandbox_test.sh b/src/test/shell/bazel/linux-sandbox_test.sh
index 549f84f..c97f190 100755
--- a/src/test/shell/bazel/linux-sandbox_test.sh
+++ b/src/test/shell/bazel/linux-sandbox_test.sh
@@ -28,12 +28,15 @@
readonly OUT="${OUT_DIR}/outfile"
readonly ERR="${OUT_DIR}/errfile"
readonly SANDBOX_DIR="${OUT_DIR}/sandbox"
+readonly SANDBOX_ROOT="${TEST_TMPDIR}/sandbox.root"
+readonly MOUNT_TARGET_ROOT="${TEST_SRCDIR}/targets"
SANDBOX_DEFAULT_OPTS="-W $SANDBOX_DIR"
function set_up {
rm -rf $OUT_DIR
mkdir -p $SANDBOX_DIR
+ mkdir -p $SANDBOX_ROOT
}
function test_basic_functionality() {
@@ -111,6 +114,90 @@
expect_log "child exited normally with exitcode 0"
}
+function test_mount_additional_paths_success() {
+ mkdir -p ${TEST_TMPDIR}/foo
+ mkdir -p ${TEST_TMPDIR}/bar
+ touch ${TEST_TMPDIR}/testfile
+ mkdir -p ${MOUNT_TARGET_ROOT}/foo
+ touch ${MOUNT_TARGET_ROOT}/sandboxed_testfile
+
+ touch /tmp/sandboxed_testfile
+ $linux_sandbox $SANDBOX_DEFAULT_OPTS -D -S ${SANDBOX_ROOT} \
+ -M ${TEST_TMPDIR}/foo -m ${MOUNT_TARGET_ROOT}/foo \
+ -M ${TEST_TMPDIR}/bar \
+ -M ${TEST_TMPDIR}/testfile -m ${MOUNT_TARGET_ROOT}/sandboxed_testfile \
+ -- /bin/true &> $TEST_log || code=$?
+ # mount a directory to a customized path inside the sandbox
+ expect_log "bind mount: ${TEST_TMPDIR}/foo -> ${MOUNT_TARGET_ROOT}/foo\$"
+ # mount a directory to the same path inside the sanxbox
+ expect_log "bind mount: ${TEST_TMPDIR}/bar -> ${TEST_TMPDIR}/bar\$"
+ # mount a file to a customized path inside the sandbox
+ expect_log "bind mount: ${TEST_TMPDIR}/testfile -> ${MOUNT_TARGET_ROOT}/sandboxed_testfile\$"
+ expect_log "child exited normally with exitcode 0"
+ rm -rf ${MOUNT_TARGET_ROOT}/foo
+ rm -rf ${MOUNT_TARGET_ROOT}/sandboxed_testfile
+}
+
+function test_mount_additional_paths_relative_path() {
+ touch ${TEST_TMPDIR}/testfile
+ $linux_sandbox $SANDBOX_DEFAULT_OPTS -D -S ${SANDBOX_ROOT} \
+ -M ${TEST_TMPDIR}/testfile -m tmp/sandboxed_testfile \
+ -- /bin/true &> $TEST_log || code=$?
+ # mount a directory to a customized path inside the sandbox
+ expect_log "The -m option must be used with absolute paths only.\$"
+}
+
+function test_mount_additional_paths_leading_m() {
+ mkdir -p ${TEST_TMPDIR}/foo
+ touch ${TEST_TMPDIR}/testfile
+ $linux_sandbox $SANDBOX_DEFAULT_OPTS -D -S ${SANDBOX_ROOT} \
+ -m /tmp/foo \
+ -M ${TEST_TMPDIR}/testfile -m /tmp/sandboxed_testfile \
+ -- /bin/true &> $TEST_log || code=$?
+ # mount a directory to a customized path inside the sandbox
+ expect_log "The -m option must be strictly preceded by an -M option.\$"
+}
+
+function test_mount_additional_paths_m_not_preceeded_by_M() {
+ mkdir -p ${TEST_TMPDIR}/foo
+ mkdir -p ${TEST_TMPDIR}/bar
+ touch ${TEST_TMPDIR}/testfile
+ $linux_sandbox $SANDBOX_DEFAULT_OPTS -D -S ${SANDBOX_ROOT} \
+ -M ${TEST_TMPDIR}/testfile -m /tmp/sandboxed_testfile \
+ -m /tmp/foo \
+ -M ${TEST_TMPDIR}/bar \
+ -- /bin/true &> $TEST_log || code=$?
+ # mount a directory to a customized path inside the sandbox
+ expect_log "The -m option must be strictly preceded by an -M option.\$"
+}
+
+function test_mount_additional_paths_other_flag_between_M_m_pair() {
+ mkdir -p ${TEST_TMPDIR}/bar
+ touch ${TEST_TMPDIR}/testfile
+ $linux_sandbox $SANDBOX_DEFAULT_OPTS -S ${SANDBOX_ROOT} \
+ -M ${TEST_TMPDIR}/testfile -D -m /tmp/sandboxed_testfile \
+ -M ${TEST_TMPDIR}/bar \
+ -- /bin/true &> $TEST_log || code=$?
+ # mount a directory to a customized path inside the sandbox
+ expect_log "The -m option must be strictly preceded by an -M option.\$"
+}
+
+function test_mount_additional_paths_multiple_sources_mount_to_one_target() {
+ mkdir -p ${TEST_TMPDIR}/foo
+ mkdir -p ${TEST_TMPDIR}/bar
+ mkdir -p ${MOUNT_TARGET_ROOT}/foo
+ $linux_sandbox $SANDBOX_DEFAULT_OPTS -D -S ${SANDBOX_ROOT} \
+ -M ${TEST_TMPDIR}/foo -m ${MOUNT_TARGET_ROOT}/foo \
+ -M ${TEST_TMPDIR}/bar -m ${MOUNT_TARGET_ROOT}/foo \
+ -- /bin/true &> $TEST_log || code=$?
+ # mount a directory to a customized path inside the sandbox
+ expect_log "bind mount: ${TEST_TMPDIR}/foo -> ${MOUNT_TARGET_ROOT}/foo\$"
+ # mount a new source directory to the same target, which will overwrite the previous source path
+ expect_log "bind mount: ${TEST_TMPDIR}/bar -> ${MOUNT_TARGET_ROOT}/foo\$"
+ expect_log "child exited normally with exitcode 0"
+ rm -rf ${MOUNT_TARGET_ROOT}/foo
+}
+
function test_redirect_output() {
$linux_sandbox $SANDBOX_DEFAULT_OPTS -l $OUT -L $ERR -- /bin/bash -c "echo out; echo err >&2" &> $TEST_log || code=$?
assert_equals "out" "$(cat $OUT)"
To view, visit this change. To unsubscribe, visit settings.
Reply all
Reply to author
Forward
0 new messages