Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[7.2.0] Preserve paths under hermetic /tmp in the sandbox #22407

Merged
merged 1 commit into from
May 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -170,19 +170,11 @@ protected boolean couldBeModifiedByMetadata(FileArtifactValue lastKnown) {
/**
* Optional materialization path.
*
* <p>If present, this artifact is a copy of another artifact whose contents live at this path.
* This can happen when it is declared as a file and not as an unresolved symlink but the action
* that creates it materializes it in the filesystem as a symlink to another output artifact. This
* information is useful in two situations:
*
* <ol>
* <li>When the symlink target is a remotely stored artifact, we can avoid downloading it
* multiple times when building without the bytes (see AbstractActionInputPrefetcher).
* <li>When the symlink target is inaccessible from the sandboxed environment an action runs
* under, we can rewrite it accordingly (see SandboxHelpers).
* </ol>
*
* @see com.google.devtools.build.lib.skyframe.TreeArtifactValue#getMaterializationExecPath().
* <p>If present, this artifact is a copy of another artifact. It is still tracked as a
* non-symlink by Bazel, but materialized in the local filesystem as a symlink to the original
* artifact, whose contents live at this location. This is used by {@link
* com.google.devtools.build.lib.remote.AbstractActionInputPrefetcher} to implement zero-cost
* copies of remotely stored artifacts.
*/
public Optional<PathFragment> getMaterializationExecPath() {
return Optional.empty();
Expand Down Expand Up @@ -223,12 +215,6 @@ public static FileArtifactValue createForSourceArtifact(
xattrProvider);
}

public static FileArtifactValue createForResolvedSymlink(
PathFragment realPath, FileArtifactValue metadata, @Nullable byte[] digest) {
return new ResolvedSymlinkFileArtifactValue(
realPath, digest, metadata.getContentsProxy(), metadata.getSize());
}

public static FileArtifactValue createFromInjectedDigest(
FileArtifactValue metadata, @Nullable byte[] digest) {
return createForNormalFile(digest, metadata.getContentsProxy(), metadata.getSize());
Expand Down Expand Up @@ -459,25 +445,7 @@ public String toString() {
}
}

private static final class ResolvedSymlinkFileArtifactValue extends RegularFileArtifactValue {
private final PathFragment realPath;

private ResolvedSymlinkFileArtifactValue(
PathFragment realPath,
@Nullable byte[] digest,
@Nullable FileContentsProxy proxy,
long size) {
super(digest, proxy, size);
this.realPath = realPath;
}

@Override
public Optional<PathFragment> getMaterializationExecPath() {
return Optional.of(realPath);
}
}

private static class RegularFileArtifactValue extends FileArtifactValue {
private static final class RegularFileArtifactValue extends FileArtifactValue {
private final byte[] digest;
@Nullable private final FileContentsProxy proxy;
private final long size;
Expand All @@ -500,8 +468,7 @@ public boolean equals(Object o) {
RegularFileArtifactValue that = (RegularFileArtifactValue) o;
return Arrays.equals(digest, that.digest)
&& Objects.equals(proxy, that.proxy)
&& size == that.size
&& Objects.equals(getMaterializationExecPath(), that.getMaterializationExecPath());
&& size == that.size;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.RootedPath;
import java.io.IOException;
import java.util.LinkedHashSet;
import java.util.Set;
Expand Down Expand Up @@ -163,9 +162,9 @@ void createInputs(Iterable<PathFragment> inputsToCreate, SandboxInputs inputs)
}
Path key = sandboxExecRoot.getRelative(fragment);
if (inputs.getFiles().containsKey(fragment)) {
RootedPath fileDest = inputs.getFiles().get(fragment);
Path fileDest = inputs.getFiles().get(fragment);
if (fileDest != null) {
copyFile(fileDest.asPath(), key);
copyFile(fileDest, key);
} else {
FileSystemUtils.createEmptyFile(key);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -360,21 +360,19 @@ private boolean wasTimeout(Duration timeout, Duration wallTime) {
/**
* Gets the list of directories that the spawn will assume to be writable.
*
* @param sandboxExecRoot the exec root of the sandbox from the point of view of the Bazel process
* @param withinSandboxExecRoot the exec root from the point of view of the sandboxed processes
* @param sandboxExecRoot the exec root of the sandbox
* @param env the environment of the sandboxed processes
* @throws IOException because we might resolve symlinks, which throws {@link IOException}.
*/
protected ImmutableSet<Path> getWritableDirs(
Path sandboxExecRoot, Path withinSandboxExecRoot, Map<String, String> env)
protected ImmutableSet<Path> getWritableDirs(Path sandboxExecRoot, Map<String, String> env)
throws IOException {
// We have to make the TEST_TMPDIR directory writable if it is specified.
ImmutableSet.Builder<Path> writablePaths = ImmutableSet.builder();

// On Windows, sandboxExecRoot is actually the main execroot. We will specify
// exactly which output path is writable.
if (OS.getCurrent() != OS.WINDOWS) {
writablePaths.add(withinSandboxExecRoot);
writablePaths.add(execRoot);
}

String testTmpdir = env.get("TEST_TMPDIR");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
import com.google.devtools.build.lib.vfs.FileSystem;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.Root;
import java.io.BufferedWriter;
import java.io.File;
import java.io.IOException;
Expand Down Expand Up @@ -103,7 +102,6 @@ private static boolean computeIsSupported() throws InterruptedException {

private final SandboxHelpers helpers;
private final Path execRoot;
private final ImmutableList<Root> packageRoots;
private final boolean allowNetwork;
private final ProcessWrapper processWrapper;
private final Path sandboxBase;
Expand Down Expand Up @@ -134,7 +132,6 @@ private static boolean computeIsSupported() throws InterruptedException {
super(cmdEnv);
this.helpers = helpers;
this.execRoot = cmdEnv.getExecRoot();
this.packageRoots = cmdEnv.getPackageLocator().getPathEntries();
this.allowNetwork = helpers.shouldAllowNetwork(cmdEnv.getOptions());
this.alwaysWritableDirs = getAlwaysWritableDirs(cmdEnv.getRuntime().getFileSystem());
this.processWrapper = ProcessWrapper.fromCommandEnvironment(cmdEnv);
Expand Down Expand Up @@ -221,18 +218,13 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context
localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), binTools, "/tmp");

final HashSet<Path> writableDirs = new HashSet<>(alwaysWritableDirs);
ImmutableSet<Path> extraWritableDirs =
getWritableDirs(sandboxExecRoot, sandboxExecRoot, environment);
ImmutableSet<Path> extraWritableDirs = getWritableDirs(sandboxExecRoot, environment);
writableDirs.addAll(extraWritableDirs);

SandboxInputs inputs =
helpers.processInputFiles(
context.getInputMapping(PathFragment.EMPTY_FRAGMENT, /* willAccessRepeatedly= */ true),
context.getInputMetadataProvider(),
execRoot,
execRoot,
packageRoots,
null);
execRoot);
SandboxOutputs outputs = helpers.getOutputs(spawn);

final Path sandboxConfigPath = sandboxPath.getRelative("sandbox.sb");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
import com.google.devtools.build.lib.util.ProcessUtils;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.Root;
import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
Expand Down Expand Up @@ -143,7 +142,6 @@ public static boolean isSupported(CommandEnvironment cmdEnv, Path dockerClient)

private final SandboxHelpers helpers;
private final Path execRoot;
private final ImmutableList<Root> packageRoots;
private final boolean allowNetwork;
private final Path dockerClient;
private final ProcessWrapper processWrapper;
Expand Down Expand Up @@ -181,7 +179,6 @@ public static boolean isSupported(CommandEnvironment cmdEnv, Path dockerClient)
super(cmdEnv);
this.helpers = helpers;
this.execRoot = cmdEnv.getExecRoot();
this.packageRoots = cmdEnv.getPackageLocator().getPathEntries();
this.allowNetwork = helpers.shouldAllowNetwork(cmdEnv.getOptions());
this.dockerClient = dockerClient;
this.processWrapper = ProcessWrapper.fromCommandEnvironment(cmdEnv);
Expand Down Expand Up @@ -226,11 +223,7 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context
SandboxInputs inputs =
helpers.processInputFiles(
context.getInputMapping(PathFragment.EMPTY_FRAGMENT, /* willAccessRepeatedly= */ true),
context.getInputMetadataProvider(),
execRoot,
execRoot,
packageRoots,
null);
execRoot);
SandboxOutputs outputs = helpers.getOutputs(spawn);

Duration timeout = context.getTimeout();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@
import static com.google.devtools.build.lib.sandbox.LinuxSandboxCommandLineBuilder.NetworkNamespace.NETNS;
import static com.google.devtools.build.lib.sandbox.LinuxSandboxCommandLineBuilder.NetworkNamespace.NETNS_WITH_LOOPBACK;

import com.google.auto.value.AutoValue;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.ExecutionRequirements;
import com.google.devtools.build.lib.vfs.Path;
Expand All @@ -36,20 +36,6 @@
* linux-sandbox} tool.
*/
public class LinuxSandboxCommandLineBuilder {
/** A bind mount that needs to be present when the sandboxed command runs. */
@AutoValue
public abstract static class BindMount {
public static BindMount of(Path mountPoint, Path source) {
return new AutoValue_LinuxSandboxCommandLineBuilder_BindMount(mountPoint, source);
}

/** "target" in mount(2) */
public abstract Path getMountPoint();

/** "source" in mount(2) */
public abstract Path getContent();
}

private final Path linuxSandboxPath;
private Path hermeticSandboxPath;
private Path workingDirectory;
Expand All @@ -60,7 +46,7 @@ public static BindMount of(Path mountPoint, Path source) {
private Path stderrPath;
private Set<Path> writableFilesAndDirectories = ImmutableSet.of();
private ImmutableSet<PathFragment> tmpfsDirectories = ImmutableSet.of();
private List<BindMount> bindMounts = ImmutableList.of();
private Map<Path, Path> bindMounts = ImmutableMap.of();
private Path statisticsPath;
private boolean useFakeHostname = false;
private NetworkNamespace createNetworkNamespace = NetworkNamespace.NO_NETNS;
Expand Down Expand Up @@ -164,7 +150,7 @@ public LinuxSandboxCommandLineBuilder setTmpfsDirectories(
* if any.
*/
@CanIgnoreReturnValue
public LinuxSandboxCommandLineBuilder setBindMounts(List<BindMount> bindMounts) {
public LinuxSandboxCommandLineBuilder setBindMounts(Map<Path, Path> bindMounts) {
this.bindMounts = bindMounts;
return this;
}
Expand Down Expand Up @@ -273,11 +259,12 @@ public ImmutableList<String> buildForCommand(List<String> commandArguments) {
for (PathFragment tmpfsPath : tmpfsDirectories) {
commandLineBuilder.add("-e", tmpfsPath.getPathString());
}
for (BindMount bindMount : bindMounts) {
commandLineBuilder.add("-M", bindMount.getContent().getPathString());
for (Path bindMountTarget : bindMounts.keySet()) {
Path bindMountSource = bindMounts.get(bindMountTarget);
commandLineBuilder.add("-M", bindMountSource.getPathString());
// The file is mounted in a custom location inside the sandbox.
if (!bindMount.getContent().equals(bindMount.getMountPoint())) {
commandLineBuilder.add("-m", bindMount.getMountPoint().getPathString());
if (!bindMountSource.equals(bindMountTarget)) {
commandLineBuilder.add("-m", bindMountTarget.getPathString());
}
}
if (statisticsPath != null) {
Expand Down
Loading
Loading