Check whether the remote cache accepts absolute symlinks before uploading them.
Closes #16354.
PiperOrigin-RevId: 477812051
Change-Id: I12419094b2e9fab1d4d66c5f94f331ebaaf20695
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java
index 1fc369b..60ebc08 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java
@@ -20,6 +20,7 @@
import static com.google.devtools.build.lib.remote.util.Utils.getFromFuture;
import build.bazel.remote.execution.v2.ActionResult;
+import build.bazel.remote.execution.v2.CacheCapabilities;
import build.bazel.remote.execution.v2.Digest;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableSet;
@@ -83,17 +84,26 @@
private final CountDownLatch closeCountDownLatch = new CountDownLatch(1);
protected final AsyncTaskCache.NoResult<Digest> casUploadCache = AsyncTaskCache.NoResult.create();
+ protected final CacheCapabilities cacheCapabilities;
protected final RemoteCacheClient cacheProtocol;
protected final RemoteOptions options;
protected final DigestUtil digestUtil;
public RemoteCache(
- RemoteCacheClient cacheProtocol, RemoteOptions options, DigestUtil digestUtil) {
+ CacheCapabilities cacheCapabilities,
+ RemoteCacheClient cacheProtocol,
+ RemoteOptions options,
+ DigestUtil digestUtil) {
+ this.cacheCapabilities = cacheCapabilities;
this.cacheProtocol = cacheProtocol;
this.options = options;
this.digestUtil = digestUtil;
}
+ public CacheCapabilities getCacheCapabilities() {
+ return cacheCapabilities;
+ }
+
public CachedActionResult downloadActionResult(
RemoteActionExecutionContext context, ActionKey actionKey, boolean inlineOutErr)
throws IOException, InterruptedException {
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionCache.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionCache.java
index 2414c9f..421313e 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionCache.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionCache.java
@@ -23,6 +23,7 @@
import static com.google.devtools.build.lib.remote.util.RxUtils.toTransferResult;
import static java.lang.String.format;
+import build.bazel.remote.execution.v2.CacheCapabilities;
import build.bazel.remote.execution.v2.Digest;
import build.bazel.remote.execution.v2.Directory;
import com.google.common.base.Throwables;
@@ -59,10 +60,11 @@
public class RemoteExecutionCache extends RemoteCache {
public RemoteExecutionCache(
+ CacheCapabilities cacheCapabilities,
RemoteCacheClient protocolImpl,
RemoteOptions options,
DigestUtil digestUtil) {
- super(protocolImpl, options, digestUtil);
+ super(cacheCapabilities, protocolImpl, options, digestUtil);
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java
index 154d139..ef33003 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java
@@ -1270,6 +1270,7 @@
return UploadManifest.create(
remoteOptions,
+ remoteCache.getCacheCapabilities(),
digestUtil,
remotePathResolver,
action.getActionKey(),
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java
index 3d792d0..0c96be6 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java
@@ -16,8 +16,10 @@
import static java.util.concurrent.TimeUnit.SECONDS;
+import build.bazel.remote.execution.v2.CacheCapabilities;
import build.bazel.remote.execution.v2.DigestFunction;
import build.bazel.remote.execution.v2.ServerCapabilities;
+import build.bazel.remote.execution.v2.SymlinkAbsolutePathStrategy;
import com.google.auth.Credentials;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Ascii;
@@ -125,6 +127,11 @@
private static final GoogleLogger logger = GoogleLogger.forEnclosingClass();
+ private static final CacheCapabilities DISK_CACHE_CAPABILITIES =
+ CacheCapabilities.newBuilder()
+ .setSymlinkAbsolutePathStrategy(SymlinkAbsolutePathStrategy.Value.ALLOWED)
+ .build();
+
private final ListeningScheduledExecutorService retryScheduler =
MoreExecutors.listeningDecorator(Executors.newScheduledThreadPool(1));
@@ -181,7 +188,7 @@
return !Strings.isNullOrEmpty(options.remoteDownloader);
}
- private static void verifyServerCapabilities(
+ private static ServerCapabilities getAndVerifyServerCapabilities(
RemoteOptions remoteOptions,
ReferenceCountedChannel channel,
CallCredentials credentials,
@@ -202,7 +209,7 @@
capabilities = rsc.get(env.getBuildRequestId(), env.getCommandId().toString());
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
- return;
+ return null;
}
checkClientServerCompatibility(
capabilities,
@@ -210,6 +217,7 @@
digestUtil.getDigestFunction(),
env.getReporter(),
requirement);
+ return capabilities;
}
private void initHttpAndDiskCache(
@@ -248,7 +256,8 @@
handleInitFailure(env, e, Code.CACHE_INIT_FAILURE);
return;
}
- RemoteCache remoteCache = new RemoteCache(cacheClient, remoteOptions, digestUtil);
+ RemoteCache remoteCache =
+ new RemoteCache(DISK_CACHE_CAPABILITIES, cacheClient, remoteOptions, digestUtil);
actionContextProvider =
RemoteActionContextProvider.createForRemoteCaching(
executorService, env, remoteCache, /* retryScheduler= */ null, digestUtil);
@@ -483,44 +492,51 @@
//
// If they point to different endpoints, we check the endpoint with execution or cache
// capabilities respectively.
+ ServerCapabilities execCapabilities = null;
+ ServerCapabilities cacheCapabilities = null;
try {
if (execChannel != null) {
if (cacheChannel != execChannel) {
- verifyServerCapabilities(
- remoteOptions,
- execChannel,
- credentials,
- retrier,
- env,
- digestUtil,
- ServerCapabilitiesRequirement.EXECUTION);
- verifyServerCapabilities(
- remoteOptions,
- cacheChannel,
- credentials,
- retrier,
- env,
- digestUtil,
- ServerCapabilitiesRequirement.CACHE);
+ execCapabilities =
+ getAndVerifyServerCapabilities(
+ remoteOptions,
+ execChannel,
+ credentials,
+ retrier,
+ env,
+ digestUtil,
+ ServerCapabilitiesRequirement.EXECUTION);
+ cacheCapabilities =
+ getAndVerifyServerCapabilities(
+ remoteOptions,
+ cacheChannel,
+ credentials,
+ retrier,
+ env,
+ digestUtil,
+ ServerCapabilitiesRequirement.CACHE);
} else {
- verifyServerCapabilities(
- remoteOptions,
- execChannel,
- credentials,
- retrier,
- env,
- digestUtil,
- ServerCapabilitiesRequirement.EXECUTION_AND_CACHE);
+ execCapabilities =
+ cacheCapabilities =
+ getAndVerifyServerCapabilities(
+ remoteOptions,
+ execChannel,
+ credentials,
+ retrier,
+ env,
+ digestUtil,
+ ServerCapabilitiesRequirement.EXECUTION_AND_CACHE);
}
} else {
- verifyServerCapabilities(
- remoteOptions,
- cacheChannel,
- credentials,
- retrier,
- env,
- digestUtil,
- ServerCapabilitiesRequirement.CACHE);
+ cacheCapabilities =
+ getAndVerifyServerCapabilities(
+ remoteOptions,
+ cacheChannel,
+ credentials,
+ retrier,
+ env,
+ digestUtil,
+ ServerCapabilitiesRequirement.CACHE);
}
} catch (IOException e) {
String errorMessage =
@@ -602,7 +618,8 @@
}
execChannel.release();
RemoteExecutionCache remoteCache =
- new RemoteExecutionCache(cacheClient, remoteOptions, digestUtil);
+ new RemoteExecutionCache(
+ cacheCapabilities.getCacheCapabilities(), cacheClient, remoteOptions, digestUtil);
actionContextProvider =
RemoteActionContextProvider.createForRemoteExecution(
executorService,
@@ -637,7 +654,9 @@
}
}
- RemoteCache remoteCache = new RemoteCache(cacheClient, remoteOptions, digestUtil);
+ RemoteCache remoteCache =
+ new RemoteCache(
+ cacheCapabilities.getCacheCapabilities(), cacheClient, remoteOptions, digestUtil);
actionContextProvider =
RemoteActionContextProvider.createForRemoteCaching(
executorService, env, remoteCache, retryScheduler, digestUtil);
diff --git a/src/main/java/com/google/devtools/build/lib/remote/UploadManifest.java b/src/main/java/com/google/devtools/build/lib/remote/UploadManifest.java
index a7fc6ed..b75c897 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/UploadManifest.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/UploadManifest.java
@@ -22,9 +22,11 @@
import build.bazel.remote.execution.v2.Action;
import build.bazel.remote.execution.v2.ActionResult;
+import build.bazel.remote.execution.v2.CacheCapabilities;
import build.bazel.remote.execution.v2.Command;
import build.bazel.remote.execution.v2.Digest;
import build.bazel.remote.execution.v2.Directory;
+import build.bazel.remote.execution.v2.SymlinkAbsolutePathStrategy;
import build.bazel.remote.execution.v2.Tree;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
@@ -77,6 +79,7 @@
private final ActionResult.Builder result;
private final boolean followSymlinks;
private final boolean allowDanglingSymlinks;
+ private final boolean allowAbsoluteSymlinks;
private final Map<Digest, Path> digestToFile = new HashMap<>();
private final Map<Digest, ByteString> digestToBlobs = new HashMap<>();
@Nullable private ActionKey actionKey;
@@ -85,6 +88,7 @@
public static UploadManifest create(
RemoteOptions remoteOptions,
+ CacheCapabilities cacheCapabilities,
DigestUtil digestUtil,
RemotePathResolver remotePathResolver,
ActionKey actionKey,
@@ -105,7 +109,10 @@
remotePathResolver,
result,
/* followSymlinks= */ !remoteOptions.incompatibleRemoteSymlinks,
- /* allowDanglingSymlinks= */ remoteOptions.incompatibleRemoteDanglingSymlinks);
+ /* allowDanglingSymlinks= */ remoteOptions.incompatibleRemoteDanglingSymlinks,
+ /* allowAbsoluteSymlinks= */ cacheCapabilities
+ .getSymlinkAbsolutePathStrategy()
+ .equals(SymlinkAbsolutePathStrategy.Value.ALLOWED));
manifest.addFiles(outputFiles);
manifest.setStdoutStderr(outErr);
manifest.addAction(actionKey, action, command);
@@ -143,12 +150,14 @@
RemotePathResolver remotePathResolver,
ActionResult.Builder result,
boolean followSymlinks,
- boolean allowDanglingSymlinks) {
+ boolean allowDanglingSymlinks,
+ boolean allowAbsoluteSymlinks) {
this.digestUtil = digestUtil;
this.remotePathResolver = remotePathResolver;
this.result = result;
this.followSymlinks = followSymlinks;
this.allowDanglingSymlinks = allowDanglingSymlinks;
+ this.allowAbsoluteSymlinks = allowAbsoluteSymlinks;
}
private void setStdoutStderr(FileOutErr outErr) throws IOException {
@@ -191,13 +200,19 @@
FileStatus statFollow = file.statIfFound(Symlinks.FOLLOW);
if (statFollow == null) {
if (allowDanglingSymlinks) {
+ if (target.isAbsolute() && !allowAbsoluteSymlinks) {
+ throw new IOException(
+ String.format(
+ "Action output %s is an absolute symbolic link to %s, which is not allowed by"
+ + " the remote cache",
+ file, target));
+ }
// Report symlink to a file since we don't know any better.
- // TODO(tjgq): Check for the SymlinkAbsolutePathStrategy.ALLOW server capability before
- // uploading an absolute symlink.
addFileSymbolicLink(file, target);
} else {
throw new IOException(
- String.format("Action output %s is a dangling symbolic link to %s ", file, target));
+ String.format(
+ "Action output %s is a dangling symbolic link to %s. ", file, target));
}
} else if (statFollow.isSpecialFile()) {
illegalOutput(file);
