Check whether the remote cache accepts absolute symlinks before uploa…

…ding them.

Closes #16354.

PiperOrigin-RevId: 477812051
Change-Id: I12419094b2e9fab1d4d66c5f94f331ebaaf20695

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 @@ public class RemoteCache extends AbstractReferenceCounted {
   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 {

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);
   }
 
   /**

src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java

@@ -1270,6 +1270,7 @@ private Single<UploadManifest> buildUploadManifestAsync(
 
           return UploadManifest.create(
               remoteOptions,
+              remoteCache.getCacheCapabilities(),
               digestUtil,
               remotePathResolver,
               action.getActionKey(),

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 @@ public final class RemoteModule extends BlazeModule {
 
   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 @@ private static boolean shouldEnableRemoteDownloader(RemoteOptions options) {
     return !Strings.isNullOrEmpty(options.remoteDownloader);
   }
 
-  private static void verifyServerCapabilities(
+  private static ServerCapabilities getAndVerifyServerCapabilities(
       RemoteOptions remoteOptions,
       ReferenceCountedChannel channel,
       CallCredentials credentials,
@@ -202,14 +209,15 @@ private static void verifyServerCapabilities(
       capabilities = rsc.get(env.getBuildRequestId(), env.getCommandId().toString());
     } catch (InterruptedException e) {
       Thread.currentThread().interrupt();
-      return;
+      return null;
     }
     checkClientServerCompatibility(
         capabilities,
         remoteOptions,
         digestUtil.getDigestFunction(),
         env.getReporter(),
         requirement);
+    return capabilities;
   }
 
   private void initHttpAndDiskCache(
@@ -248,7 +256,8 @@ private void initHttpAndDiskCache(
       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 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
     //
     // 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 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
       }
       execChannel.release();
       RemoteExecutionCache remoteCache =
-          new RemoteExecutionCache(cacheClient, remoteOptions, digestUtil);
+          new RemoteExecutionCache(
+              cacheCapabilities.getCacheCapabilities(), cacheClient, remoteOptions, digestUtil);
       actionContextProvider =
           RemoteActionContextProvider.createForRemoteExecution(
               executorService,
@@ -637,7 +654,9 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
         }
       }
 
-      RemoteCache remoteCache = new RemoteCache(cacheClient, remoteOptions, digestUtil);
+      RemoteCache remoteCache =
+          new RemoteCache(
+              cacheCapabilities.getCacheCapabilities(), cacheClient, remoteOptions, digestUtil);
       actionContextProvider =
           RemoteActionContextProvider.createForRemoteCaching(
               executorService, env, remoteCache, retryScheduler, digestUtil);

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 @@ public class UploadManifest {
   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 class UploadManifest {
 
   public static UploadManifest create(
       RemoteOptions remoteOptions,
+      CacheCapabilities cacheCapabilities,
       DigestUtil digestUtil,
       RemotePathResolver remotePathResolver,
       ActionKey actionKey,
@@ -105,7 +109,10 @@ public static UploadManifest create(
             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 @@ public UploadManifest(
       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 @@ void addFiles(Collection<Path> files) throws ExecException, IOException {
         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);

src/test/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderTest.java

@@ -22,6 +22,7 @@
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 
+import build.bazel.remote.execution.v2.CacheCapabilities;
 import build.bazel.remote.execution.v2.Digest;
 import com.google.bytestream.ByteStreamProto.WriteRequest;
 import com.google.bytestream.ByteStreamProto.WriteResponse;
@@ -469,7 +470,8 @@ private RemoteCache newRemoteCache(
         .when(cacheClient)
         .findMissingDigests(any(), any());
 
-    return new RemoteCache(cacheClient, remoteOptions, DIGEST_UTIL);
+    return new RemoteCache(
+        CacheCapabilities.getDefaultInstance(), cacheClient, remoteOptions, DIGEST_UTIL);
   }
 
   private ByteStreamBuildEventArtifactUploader newArtifactUploader(RemoteCache remoteCache) {

src/test/java/com/google/devtools/build/lib/remote/FakeActionInputFileCache.java

@@ -27,6 +27,7 @@
 import com.google.devtools.build.lib.vfs.FileStatus;
 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.Symlinks;
 import com.google.devtools.build.lib.vfs.SyscallCache;
 import java.io.IOException;
@@ -77,4 +78,13 @@ public Digest createScratchInputDirectory(ActionInput input, Tree content) throw
     setDigest(input, digest.getHash());
     return digest;
   }
+
+  public Digest createScratchInputSymlink(ActionInput input, String target) throws IOException {
+    Path inputFile = execRoot.getRelative(input.getExecPath());
+    inputFile.getParentDirectory().createDirectoryAndParents();
+    inputFile.createSymbolicLink(PathFragment.create(target));
+    Digest digest = digestUtil.compute(inputFile);
+    setDigest(input, digest.getHash());
+    return digest;
+  }
 }