From 24de276977b44c6135813ced5e113c36da7e9953 Mon Sep 17 00:00:00 2001 From: Tiago Quelhas Date: Mon, 9 Oct 2023 08:38:41 -0700 Subject: [PATCH] Introduce options for remote cache key scrubbing. Introduce an experimental flag for remote cache key scrubbing. This PR introduces the --experimental_remote_scrubbing_config flag, which can be used to scrub platform-dependent data from the key used to retrieve and store action results from a remote or disk cache This way, actions executing on different platforms but targeting the same platform may be able to share cache entries. To avoid flag proliferation and make for a nicer API, the scrubbing parameters are supplied via an external file in text proto format. This is a simplified implementation of one of the ideas described in [1], highly influenced by Olivier Notteghem's earlier proposal [2], intended to provide a simple yet flexible API to enable further experimentation. It must be used with care, as incorrect settings can compromise build correctness. [1] https://docs.google.com/document/d/1uMPj2s0TlHSIKSngqOkWJoeqOtKzaxQLtBrRfYif3Lo/edit?usp=sharing [2] https://github.com/bazelbuild/bazel/pull/18669 Closes #19523. PiperOrigin-RevId: 571947013 Change-Id: Ic8da59946ea6d6811b840aee01548746aab2eba0 --- .../google/devtools/build/lib/remote/BUILD | 15 + .../lib/remote/RemoteExecutionService.java | 74 ++- .../build/lib/remote/RemoteModule.java | 8 + .../devtools/build/lib/remote/Scrubber.java | 178 +++++++ .../build/lib/remote/merkletree/BUILD | 1 + .../merkletree/DirectoryTreeBuilder.java | 20 + .../lib/remote/merkletree/MerkleTree.java | 11 +- .../devtools/build/lib/remote/options/BUILD | 1 + .../lib/remote/options/RemoteOptions.java | 44 ++ src/main/protobuf/BUILD | 18 +- src/main/protobuf/failure_details.proto | 1 + src/main/protobuf/remote_scrubbing.proto | 82 ++++ src/main/protobuf/spawn.proto | 9 + .../google/devtools/build/lib/remote/BUILD | 2 + .../build/lib/remote/GrpcCacheClientTest.java | 1 + .../remote/RemoteExecutionServiceTest.java | 67 ++- .../build/lib/remote/ScrubberTest.java | 433 ++++++++++++++++++ .../ActionInputDirectoryTreeTest.java | 4 + .../lib/remote/merkletree/MerkleTreeTest.java | 6 + .../bazel/remote/remote_execution_test.sh | 56 +++ 20 files changed, 1004 insertions(+), 27 deletions(-) create mode 100644 src/main/java/com/google/devtools/build/lib/remote/Scrubber.java create mode 100644 src/main/protobuf/remote_scrubbing.proto create mode 100644 src/test/java/com/google/devtools/build/lib/remote/ScrubberTest.java diff --git a/src/main/java/com/google/devtools/build/lib/remote/BUILD b/src/main/java/com/google/devtools/build/lib/remote/BUILD index f63d7895d38ccb..39d0ce2cfabff1 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/BUILD +++ b/src/main/java/com/google/devtools/build/lib/remote/BUILD @@ -37,6 +37,7 @@ java_library( "RemoteOutputChecker.java", "AbstractActionInputPrefetcher.java", "LeaseService.java", + "Scrubber.java", "Store.java", ], ), @@ -90,6 +91,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/exec/local", "//src/main/java/com/google/devtools/build/lib/packages/semantics", "//src/main/java/com/google/devtools/build/lib/profiler", + "//src/main/java/com/google/devtools/build/lib/remote:scrubber", "//src/main/java/com/google/devtools/build/lib/remote/circuitbreaker", "//src/main/java/com/google/devtools/build/lib/remote/common", "//src/main/java/com/google/devtools/build/lib/remote/common:bulk_transfer_exception", @@ -252,6 +254,19 @@ java_library( ], ) +java_library( + name = "scrubber", + srcs = ["Scrubber.java"], + deps = [ + "//src/main/java/com/google/devtools/build/lib/actions", + "//src/main/java/com/google/devtools/build/lib/actions:artifacts", + "//src/main/protobuf:remote_scrubbing_java_proto", + "//third_party:guava", + "//third_party:jsr305", + "//third_party/protobuf:protobuf_java", + ], +) + java_library( name = "store", srcs = ["Store.java"], 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 ab46af65adc42b..6cc3dbb9cfead2 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 @@ -87,6 +87,7 @@ import com.google.devtools.build.lib.remote.RemoteExecutionService.ActionResultMetadata.DirectoryMetadata; import com.google.devtools.build.lib.remote.RemoteExecutionService.ActionResultMetadata.FileMetadata; import com.google.devtools.build.lib.remote.RemoteExecutionService.ActionResultMetadata.SymlinkMetadata; +import com.google.devtools.build.lib.remote.Scrubber.SpawnScrubber; import com.google.devtools.build.lib.remote.common.BulkTransferException; import com.google.devtools.build.lib.remote.common.OperationObserver; import com.google.devtools.build.lib.remote.common.OutputDigestMismatchException; @@ -179,6 +180,8 @@ public class RemoteExecutionService { @Nullable private final RemoteOutputChecker remoteOutputChecker; + @Nullable private final Scrubber scrubber; + public RemoteExecutionService( Executor executor, Reporter reporter, @@ -210,6 +213,7 @@ public RemoteExecutionService( if (remoteOptions.remoteMerkleTreeCacheSize != 0) { merkleTreeCacheBuilder.maximumSize(remoteOptions.remoteMerkleTreeCacheSize); } + this.scrubber = remoteOptions.scrubber; this.merkleTreeCache = merkleTreeCacheBuilder.build(); this.tempPathGenerator = tempPathGenerator; @@ -219,12 +223,13 @@ public RemoteExecutionService( this.remoteOutputChecker = remoteOutputChecker; } - static Command buildCommand( + private Command buildCommand( Collection outputs, List arguments, ImmutableMap env, @Nullable Platform platform, - RemotePathResolver remotePathResolver) { + RemotePathResolver remotePathResolver, + @Nullable SpawnScrubber spawnScrubber) { Command.Builder command = Command.newBuilder(); ArrayList outputFiles = new ArrayList<>(); ArrayList outputDirectories = new ArrayList<>(); @@ -249,6 +254,9 @@ static Command buildCommand( command.setPlatform(platform); } for (String arg : arguments) { + if (spawnScrubber != null) { + arg = spawnScrubber.transformArgument(arg); + } command.addArguments(decodeBytestringUtf8(arg)); } // Sorting the environment pairs by variable name. @@ -349,15 +357,18 @@ private SortedMap buildOutputDirMap(Spawn spawn) { } private MerkleTree buildInputMerkleTree( - Spawn spawn, SpawnExecutionContext context, ToolSignature toolSignature) + Spawn spawn, + SpawnExecutionContext context, + ToolSignature toolSignature, + @Nullable SpawnScrubber spawnScrubber) throws IOException, ForbiddenActionInputException { // Add output directories to inputs so that they are created as empty directories by the // executor. The spec only requires the executor to create the parent directory of an output // directory, which differs from the behavior of both local and sandboxed execution. SortedMap outputDirMap = buildOutputDirMap(spawn); boolean useMerkleTreeCache = remoteOptions.remoteMerkleTreeCache; - if (toolSignature != null) { - // Marking tool files is not yet supported in conjunction with the merkle tree cache. + if (toolSignature != null || spawnScrubber != null) { + // The Merkle tree cache is not yet compatible with scrubbing or marking tool files. useMerkleTreeCache = false; } if (useMerkleTreeCache) { @@ -366,11 +377,14 @@ private MerkleTree buildInputMerkleTree( remotePathResolver.walkInputs( spawn, context, - (Object nodeKey, InputWalker walker) -> { - subMerkleTrees.add( - buildMerkleTreeVisitor( - nodeKey, walker, inputMetadataProvider, context.getPathResolver())); - }); + (Object nodeKey, InputWalker walker) -> + subMerkleTrees.add( + buildMerkleTreeVisitor( + nodeKey, + walker, + inputMetadataProvider, + context.getPathResolver(), + spawnScrubber))); if (!outputDirMap.isEmpty()) { subMerkleTrees.add( MerkleTree.build( @@ -378,6 +392,7 @@ private MerkleTree buildInputMerkleTree( inputMetadataProvider, execRoot, context.getPathResolver(), + /* spawnScrubber= */ null, digestUtil)); } return MerkleTree.merge(subMerkleTrees, digestUtil); @@ -399,6 +414,7 @@ private MerkleTree buildInputMerkleTree( context.getInputMetadataProvider(), execRoot, context.getPathResolver(), + spawnScrubber, digestUtil); } } @@ -407,7 +423,8 @@ private MerkleTree buildMerkleTreeVisitor( Object nodeKey, InputWalker walker, InputMetadataProvider inputMetadataProvider, - ArtifactPathResolver artifactPathResolver) + ArtifactPathResolver artifactPathResolver, + @Nullable SpawnScrubber spawnScrubber) throws IOException, ForbiddenActionInputException { // Deduplicate concurrent computations for the same node. It's not possible to use // MerkleTreeCache#get(key, loader) because the loading computation may cause other nodes to be @@ -419,7 +436,8 @@ private MerkleTree buildMerkleTreeVisitor( // No preexisting cache entry, so we must do the computation ourselves. try { freshFuture.complete( - uncachedBuildMerkleTreeVisitor(walker, inputMetadataProvider, artifactPathResolver)); + uncachedBuildMerkleTreeVisitor( + walker, inputMetadataProvider, artifactPathResolver, spawnScrubber)); } catch (Exception e) { freshFuture.completeExceptionally(e); } @@ -443,7 +461,8 @@ private MerkleTree buildMerkleTreeVisitor( public MerkleTree uncachedBuildMerkleTreeVisitor( InputWalker walker, InputMetadataProvider inputMetadataProvider, - ArtifactPathResolver artifactPathResolver) + ArtifactPathResolver artifactPathResolver, + @Nullable SpawnScrubber scrubber) throws IOException, ForbiddenActionInputException { ConcurrentLinkedQueue subMerkleTrees = new ConcurrentLinkedQueue<>(); subMerkleTrees.add( @@ -452,18 +471,18 @@ public MerkleTree uncachedBuildMerkleTreeVisitor( inputMetadataProvider, execRoot, artifactPathResolver, + scrubber, digestUtil)); walker.visitNonLeaves( - (Object subNodeKey, InputWalker subWalker) -> { - subMerkleTrees.add( - buildMerkleTreeVisitor( - subNodeKey, subWalker, inputMetadataProvider, artifactPathResolver)); - }); + (Object subNodeKey, InputWalker subWalker) -> + subMerkleTrees.add( + buildMerkleTreeVisitor( + subNodeKey, subWalker, inputMetadataProvider, artifactPathResolver, scrubber))); return MerkleTree.merge(subMerkleTrees, digestUtil); } @Nullable - private static ByteString buildSalt(Spawn spawn) { + private static ByteString buildSalt(Spawn spawn, @Nullable SpawnScrubber spawnScrubber) { CacheSalt.Builder saltBuilder = CacheSalt.newBuilder().setMayBeExecutedRemotely(Spawns.mayBeExecutedRemotely(spawn)); @@ -473,6 +492,11 @@ private static ByteString buildSalt(Spawn spawn) { saltBuilder.setWorkspace(workspace); } + if (spawnScrubber != null) { + saltBuilder.setScrubSalt( + CacheSalt.ScrubSalt.newBuilder().setSalt(spawnScrubber.getSalt()).build()); + } + return saltBuilder.build().toByteString(); } @@ -508,7 +532,9 @@ public RemoteAction buildRemoteAction(Spawn spawn, SpawnExecutionContext context remoteActionBuildingSemaphore.acquire(); try { ToolSignature toolSignature = getToolSignature(spawn, context); - final MerkleTree merkleTree = buildInputMerkleTree(spawn, context, toolSignature); + SpawnScrubber spawnScrubber = scrubber != null ? scrubber.forSpawn(spawn) : null; + final MerkleTree merkleTree = + buildInputMerkleTree(spawn, context, toolSignature, spawnScrubber); // Get the remote platform properties. Platform platform; @@ -526,7 +552,8 @@ public RemoteAction buildRemoteAction(Spawn spawn, SpawnExecutionContext context spawn.getArguments(), spawn.getEnvironment(), platform, - remotePathResolver); + remotePathResolver, + spawnScrubber); Digest commandHash = digestUtil.compute(command); Action action = Utils.buildAction( @@ -535,7 +562,7 @@ public RemoteAction buildRemoteAction(Spawn spawn, SpawnExecutionContext context platform, context.getTimeout(), Spawns.mayBeCachedRemotely(spawn), - buildSalt(spawn)); + buildSalt(spawn, spawnScrubber)); ActionKey actionKey = digestUtil.computeActionKey(action); @@ -1414,7 +1441,8 @@ public void uploadInputsIfNotPresent(RemoteAction action, boolean force) Spawn spawn = action.getSpawn(); SpawnExecutionContext context = action.getSpawnExecutionContext(); ToolSignature toolSignature = getToolSignature(spawn, context); - merkleTree = buildInputMerkleTree(spawn, context, toolSignature); + SpawnScrubber spawnScrubber = scrubber != null ? scrubber.forSpawn(spawn) : null; + merkleTree = buildInputMerkleTree(spawn, context, toolSignature, spawnScrubber); } remoteExecutionCache.ensureInputsPresent( 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 4c48b035b96bea..db9adc0e03c0b3 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 @@ -311,6 +311,14 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException { FailureDetails.RemoteOptions.Code.EXECUTION_WITH_INVALID_CACHE); } + boolean enableScrubbing = remoteOptions.scrubber != null; + if (enableScrubbing && enableRemoteExecution) { + + throw createOptionsExitException( + "Cannot combine remote cache key scrubbing with remote execution", + FailureDetails.RemoteOptions.Code.EXECUTION_WITH_SCRUBBING); + } + // TODO(bazel-team): Consider adding a warning or more validation if the remoteDownloadRegex is // used without Build without the Bytes. ImmutableList.Builder patternsToDownloadBuilder = ImmutableList.builder(); diff --git a/src/main/java/com/google/devtools/build/lib/remote/Scrubber.java b/src/main/java/com/google/devtools/build/lib/remote/Scrubber.java new file mode 100644 index 00000000000000..e0ff40881a597b --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/remote/Scrubber.java @@ -0,0 +1,178 @@ +// Copyright 2023 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.remote; + +import static com.google.common.collect.ImmutableList.toImmutableList; +import static com.google.common.collect.ImmutableMap.toImmutableMap; + +import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.devtools.build.lib.actions.ActionInput; +import com.google.devtools.build.lib.actions.Spawn; +import com.google.devtools.build.lib.actions.cache.VirtualActionInput; +import com.google.devtools.build.lib.remote.RemoteScrubbing.Config; +import com.google.protobuf.TextFormat; +import java.io.BufferedReader; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Map; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import java.util.regex.PatternSyntaxException; +import javax.annotation.Nullable; + +/** + * The {@link Scrubber} implements scrubbing of remote cache keys. + * + *

See the documentation for the {@code --experimental_remote_scrub_config} flag for more + * information. + */ +public class Scrubber { + + /** An error that occurred while parsing the scrubbing configuration. */ + public static class ConfigParseException extends Exception { + + private ConfigParseException(String message, Throwable cause) { + super(message, cause); + } + } + + private final ImmutableList spawnScrubbers; + + @VisibleForTesting + Scrubber(Config configProto) { + ArrayList spawnScrubbers = new ArrayList<>(); + for (Config.Rule ruleProto : configProto.getRulesList()) { + spawnScrubbers.add(new SpawnScrubber(ruleProto)); + } + // Reverse the order so that later rules supersede earlier ones. + Collections.reverse(spawnScrubbers); + this.spawnScrubbers = ImmutableList.copyOf(spawnScrubbers); + } + + /** + * Constructs a {@link Scrubber} from the given configuration file, which must contain a {@link + * Config} protocol buffer in text format. + */ + public static Scrubber parse(String configPath) throws ConfigParseException { + try (BufferedReader reader = Files.newBufferedReader(Paths.get(configPath))) { + var builder = Config.newBuilder(); + TextFormat.getParser().merge(reader, builder); + return new Scrubber(builder.build()); + } catch (IOException e) { + throw new ConfigParseException(e.getMessage(), e); + } catch (PatternSyntaxException e) { + throw new ConfigParseException( + String.format("in regex '%s': %s", e.getPattern(), e.getMessage()), e); + } + } + + /** + * Returns a {@link SpawnScrubber} suitable for a {@link Spawn}, or {@code null} if the spawn does + * not need to be scrubbed. + */ + @Nullable + public SpawnScrubber forSpawn(Spawn spawn) { + for (SpawnScrubber spawnScrubber : spawnScrubbers) { + if (spawnScrubber.matches(spawn)) { + return spawnScrubber; + } + } + return null; + } + + /** + * Encapsulates a set of transformations required to scrub the remote cache key for a set of + * spawns. + */ + public static class SpawnScrubber { + + private final Pattern mnemonicPattern; + private final Pattern labelPattern; + private final boolean matchTools; + + private final ImmutableList omittedInputPatterns; + private final ImmutableMap argReplacements; + private final String salt; + + private SpawnScrubber(Config.Rule ruleProto) { + Config.Matcher matcherProto = ruleProto.getMatcher(); + this.mnemonicPattern = Pattern.compile(emptyToAll(matcherProto.getMnemonic())); + this.labelPattern = Pattern.compile(emptyToAll(matcherProto.getLabel())); + this.matchTools = matcherProto.getMatchTools(); + + Config.Transform transformProto = ruleProto.getTransform(); + this.omittedInputPatterns = + transformProto.getOmittedInputsList().stream() + .map(Pattern::compile) + .collect(toImmutableList()); + this.argReplacements = + transformProto.getArgReplacementsList().stream() + .collect(toImmutableMap(r -> Pattern.compile(r.getSource()), r -> r.getTarget())); + this.salt = ruleProto.getTransform().getSalt(); + } + + private String emptyToAll(String s) { + return s.isEmpty() ? ".*" : s; + } + + /** Whether this scrubber applies to the given {@link Spawn}. */ + private boolean matches(Spawn spawn) { + String mnemonic = spawn.getMnemonic(); + String label = spawn.getResourceOwner().getOwner().getLabel().getCanonicalForm(); + boolean isForTool = spawn.getResourceOwner().getOwner().isBuildConfigurationForTool(); + + return (!isForTool || matchTools) + && mnemonicPattern.matcher(mnemonic).matches() + && labelPattern.matcher(label).matches(); + } + + /** Whether the given input should be omitted from the cache key. */ + public boolean shouldOmitInput(ActionInput input) { + if (input.equals(VirtualActionInput.EMPTY_MARKER)) { + return false; + } + String execPath = input.getExecPathString(); + for (Pattern pattern : omittedInputPatterns) { + if (pattern.matcher(execPath).matches()) { + return true; + } + } + return false; + } + + /** Transforms a command line argument. */ + public String transformArgument(String arg) { + for (Map.Entry entry : argReplacements.entrySet()) { + Pattern pattern = entry.getKey(); + String replacement = entry.getValue(); + // Don't use Pattern#replaceFirst because it allows references to capture groups. + Matcher m = pattern.matcher(arg); + if (m.find()) { + arg = arg.substring(0, m.start()) + replacement + arg.substring(m.end()); + } + } + return arg; + } + + /** Returns the scrubbing salt. */ + public String getSalt() { + return salt; + } + } +} diff --git a/src/main/java/com/google/devtools/build/lib/remote/merkletree/BUILD b/src/main/java/com/google/devtools/build/lib/remote/merkletree/BUILD index f19db0ddf06637..47f7e9e7325eb2 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/merkletree/BUILD +++ b/src/main/java/com/google/devtools/build/lib/remote/merkletree/BUILD @@ -20,6 +20,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/actions:artifacts", "//src/main/java/com/google/devtools/build/lib/actions:file_metadata", "//src/main/java/com/google/devtools/build/lib/profiler", + "//src/main/java/com/google/devtools/build/lib/remote:scrubber", "//src/main/java/com/google/devtools/build/lib/remote/util", "//src/main/java/com/google/devtools/build/lib/util:string", "//src/main/java/com/google/devtools/build/lib/vfs", diff --git a/src/main/java/com/google/devtools/build/lib/remote/merkletree/DirectoryTreeBuilder.java b/src/main/java/com/google/devtools/build/lib/remote/merkletree/DirectoryTreeBuilder.java index 268bde9d2b33c9..baaf104d7c6a34 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/merkletree/DirectoryTreeBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/remote/merkletree/DirectoryTreeBuilder.java @@ -24,6 +24,7 @@ import com.google.devtools.build.lib.actions.FileArtifactValue; import com.google.devtools.build.lib.actions.InputMetadataProvider; import com.google.devtools.build.lib.actions.cache.VirtualActionInput; +import com.google.devtools.build.lib.remote.Scrubber.SpawnScrubber; import com.google.devtools.build.lib.remote.merkletree.DirectoryTree.DirectoryNode; import com.google.devtools.build.lib.remote.merkletree.DirectoryTree.FileNode; import com.google.devtools.build.lib.remote.merkletree.DirectoryTree.SymlinkNode; @@ -39,6 +40,7 @@ import java.util.Set; import java.util.SortedMap; import java.util.TreeMap; +import javax.annotation.Nullable; /** Builder for directory trees. */ class DirectoryTreeBuilder { @@ -63,6 +65,7 @@ static DirectoryTree fromActionInputs( InputMetadataProvider inputMetadataProvider, Path execRoot, ArtifactPathResolver artifactPathResolver, + @Nullable SpawnScrubber spawnScrubber, DigestUtil digestUtil) throws IOException { return fromActionInputs( @@ -71,6 +74,7 @@ static DirectoryTree fromActionInputs( inputMetadataProvider, execRoot, artifactPathResolver, + spawnScrubber, digestUtil); } @@ -80,6 +84,7 @@ static DirectoryTree fromActionInputs( InputMetadataProvider inputMetadataProvider, Path execRoot, ArtifactPathResolver artifactPathResolver, + @Nullable SpawnScrubber spawnScrubber, DigestUtil digestUtil) throws IOException { Map tree = new HashMap<>(); @@ -90,6 +95,7 @@ static DirectoryTree fromActionInputs( inputMetadataProvider, execRoot, artifactPathResolver, + spawnScrubber, digestUtil, tree); return new DirectoryTree(tree, numFiles); @@ -114,6 +120,9 @@ static DirectoryTree fromPaths(SortedMap inputFiles, DigestU /** * Adds the files in {@code inputs} as nodes to {@code tree}. * + *

Prefer {@link #buildFromActionInputs} if this Merkle tree is for an action spawn (as opposed + * to repository fetching). + * *

This method mutates {@code tree}. * * @param inputs map of paths to files. The key determines the path at which the file should be @@ -128,6 +137,7 @@ private static int buildFromPaths( return build( inputs, tree, + /* scrubber= */ null, (input, path, currDir) -> { if (!input.isFile(Symlinks.NOFOLLOW)) { throw new IOException(String.format("Input '%s' is not a file.", input)); @@ -152,12 +162,14 @@ private static int buildFromActionInputs( InputMetadataProvider inputMetadataProvider, Path execRoot, ArtifactPathResolver artifactPathResolver, + @Nullable SpawnScrubber spawnScrubber, DigestUtil digestUtil, Map tree) throws IOException { return build( inputs, tree, + spawnScrubber, (input, path, currDir) -> { if (input instanceof VirtualActionInput) { VirtualActionInput virtualActionInput = (VirtualActionInput) input; @@ -198,6 +210,7 @@ private static int buildFromActionInputs( inputMetadataProvider, execRoot, artifactPathResolver, + spawnScrubber, digestUtil, tree); @@ -234,6 +247,7 @@ private static int buildFromActionInputs( private static int build( SortedMap inputs, Map tree, + @Nullable SpawnScrubber scrubber, FileNodeVisitor fileNodeVisitor) throws IOException { if (inputs.isEmpty()) { @@ -248,6 +262,12 @@ private static int build( PathFragment path = e.getKey(); T input = e.getValue(); + if (scrubber != null + && input instanceof ActionInput + && scrubber.shouldOmitInput((ActionInput) input)) { + continue; + } + if (input instanceof DerivedArtifact && ((DerivedArtifact) input).isTreeArtifact()) { // SpawnInputExpander has already expanded non-empty tree artifacts into a collection of // TreeFileArtifacts. Thus, at this point, tree artifacts represent empty directories, which diff --git a/src/main/java/com/google/devtools/build/lib/remote/merkletree/MerkleTree.java b/src/main/java/com/google/devtools/build/lib/remote/merkletree/MerkleTree.java index 9089434bcf78f8..2492924eaef329 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/merkletree/MerkleTree.java +++ b/src/main/java/com/google/devtools/build/lib/remote/merkletree/MerkleTree.java @@ -33,6 +33,7 @@ import com.google.devtools.build.lib.actions.InputMetadataProvider; import com.google.devtools.build.lib.profiler.Profiler; import com.google.devtools.build.lib.profiler.SilentCloseable; +import com.google.devtools.build.lib.remote.Scrubber.SpawnScrubber; import com.google.devtools.build.lib.remote.util.DigestUtil; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; @@ -223,12 +224,18 @@ public static MerkleTree build( InputMetadataProvider inputMetadataProvider, Path execRoot, ArtifactPathResolver artifactPathResolver, + @Nullable SpawnScrubber spawnScrubber, DigestUtil digestUtil) throws IOException { try (SilentCloseable c = Profiler.instance().profile("MerkleTree.build(ActionInput)")) { DirectoryTree tree = DirectoryTreeBuilder.fromActionInputs( - inputs, inputMetadataProvider, execRoot, artifactPathResolver, digestUtil); + inputs, + inputMetadataProvider, + execRoot, + artifactPathResolver, + spawnScrubber, + digestUtil); return build(tree, digestUtil); } } @@ -250,6 +257,7 @@ public static MerkleTree build( InputMetadataProvider inputMetadataProvider, Path execRoot, ArtifactPathResolver artifactPathResolver, + @Nullable SpawnScrubber spawnScrubber, DigestUtil digestUtil) throws IOException { try (SilentCloseable c = Profiler.instance().profile("MerkleTree.build(ActionInput)")) { @@ -260,6 +268,7 @@ public static MerkleTree build( inputMetadataProvider, execRoot, artifactPathResolver, + spawnScrubber, digestUtil); return build(tree, digestUtil); } diff --git a/src/main/java/com/google/devtools/build/lib/remote/options/BUILD b/src/main/java/com/google/devtools/build/lib/remote/options/BUILD index 7b37c977f9fd7a..806d6abdbb7d6c 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/options/BUILD +++ b/src/main/java/com/google/devtools/build/lib/remote/options/BUILD @@ -16,6 +16,7 @@ java_library( srcs = glob(["*.java"]), deps = [ "//src/main/java/com/google/devtools/build/lib/actions", + "//src/main/java/com/google/devtools/build/lib/remote:scrubber", "//src/main/java/com/google/devtools/build/lib/util", "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", "//src/main/java/com/google/devtools/common/options", diff --git a/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java b/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java index b74563d3c08c80..ecfd247085152d 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java +++ b/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java @@ -19,11 +19,13 @@ import com.google.common.base.Strings; import com.google.common.collect.ImmutableSortedMap; import com.google.devtools.build.lib.actions.UserExecException; +import com.google.devtools.build.lib.remote.Scrubber; import com.google.devtools.build.lib.server.FailureDetails.FailureDetail; import com.google.devtools.build.lib.server.FailureDetails.RemoteExecution; import com.google.devtools.build.lib.server.FailureDetails.RemoteExecution.Code; import com.google.devtools.build.lib.util.OptionsUtils; import com.google.devtools.build.lib.vfs.PathFragment; +import com.google.devtools.common.options.Converter; import com.google.devtools.common.options.Converters; import com.google.devtools.common.options.Converters.AssignmentConverter; import com.google.devtools.common.options.EnumConverter; @@ -31,6 +33,7 @@ import com.google.devtools.common.options.OptionDocumentationCategory; import com.google.devtools.common.options.OptionEffectTag; import com.google.devtools.common.options.OptionMetadataTag; +import com.google.devtools.common.options.OptionsParsingException; import com.google.protobuf.TextFormat; import com.google.protobuf.TextFormat.ParseException; import java.time.Duration; @@ -709,6 +712,47 @@ public RemoteOutputsStrategyConverter() { + " frequency is based on the value of `--experimental_remote_cache_ttl`.") public boolean remoteCacheLeaseExtension; + @Option( + name = "experimental_remote_scrubbing_config", + converter = ScrubberConverter.class, + defaultValue = "null", + documentationCategory = OptionDocumentationCategory.REMOTE, + effectTags = {OptionEffectTag.UNKNOWN}, + help = + "Enables remote cache key scrubbing with the supplied configuration file, which must be a" + + " ScrubbingConfig protocol buffer in text format.\n\n" + + "This feature is intended to facilitate sharing a remote/disk cache between actions" + + " executing on different platforms but targeting the same platform. It should be" + + " used with extreme care, as improper settings may cause accidental sharing of" + + " cache entries and result in incorrect builds.\n\n" + + "Scrubbing does not affect how an action is executed, only how its remote/disk" + + " cache key is computed for the purpose of retrieving or storing an action result." + + " It cannot be used in conjunction with remote execution. Modifying the scrubbing" + + " configuration does not invalidate outputs present in the local filesystem or" + + " internal caches; a clean build is required to reexecute affected actions.\n\n" + + "In order to successfully use this feature, you likely want to set a custom" + + " --host_platform together with --experimental_platform_in_output_dir (to normalize" + + " output prefixes) and --incompatible_strict_action_env (to normalize environment" + + " variables).") + public Scrubber scrubber; + + private static final class ScrubberConverter extends Converter.Contextless { + + @Override + public Scrubber convert(String path) throws OptionsParsingException { + try { + return Scrubber.parse(path); + } catch (Scrubber.ConfigParseException e) { + throw new OptionsParsingException("Failed to parse ScrubbingConfig: " + e.getMessage(), e); + } + } + + @Override + public String getTypeDescription() { + return "Converts to a Scrubber"; + } + } + // The below options are not configurable by users, only tests. // This is part of the effort to reduce the overall number of flags. diff --git a/src/main/protobuf/BUILD b/src/main/protobuf/BUILD index ca6ffb099a7f89..1440c3eabe98f4 100644 --- a/src/main/protobuf/BUILD +++ b/src/main/protobuf/BUILD @@ -1,5 +1,5 @@ -load("@rules_python//python:proto.bzl", "py_proto_library") load("@rules_proto//proto:defs.bzl", "proto_library") +load("@rules_python//python:proto.bzl", "py_proto_library") load("//third_party/grpc:build_defs.bzl", "java_grpc_library") load("//third_party/grpc/bazel:cc_grpc_library.bzl", "cc_grpc_library") load("//tools/build_rules:utilities.bzl", "java_library_srcs") @@ -221,6 +221,21 @@ java_library_srcs( deps = [":remote_execution_log_java_proto"], ) +proto_library( + name = "remote_scrubbing_proto", + srcs = ["remote_scrubbing.proto"], +) + +java_proto_library( + name = "remote_scrubbing_java_proto", + deps = [":remote_scrubbing_proto"], +) + +java_library_srcs( + name = "remote_scrubbing_java_proto_srcs", + deps = [":remote_scrubbing_java_proto"], +) + proto_library( name = "spawn_proto", srcs = ["spawn.proto"], @@ -276,6 +291,7 @@ filegroup( ":option_filters_java_proto_srcs", ":profile_java_proto_srcs", ":remote_execution_log_java_proto_srcs", + ":remote_scrubbing_java_proto_srcs", ":spawn_java_proto_srcs", ":xcode_java_proto_srcs", ], diff --git a/src/main/protobuf/failure_details.proto b/src/main/protobuf/failure_details.proto index 3cc189c0a1ff0c..e2f6ed867f7c07 100644 --- a/src/main/protobuf/failure_details.proto +++ b/src/main/protobuf/failure_details.proto @@ -295,6 +295,7 @@ message RemoteOptions { CREDENTIALS_WRITE_FAILURE = 3 [(metadata) = { exit_code: 36 }]; DOWNLOADER_WITHOUT_GRPC_CACHE = 4 [(metadata) = { exit_code: 2 }]; EXECUTION_WITH_INVALID_CACHE = 5 [(metadata) = { exit_code: 2 }]; + EXECUTION_WITH_SCRUBBING = 6 [(metadata) = { exit_code: 2 }]; } Code code = 1; diff --git a/src/main/protobuf/remote_scrubbing.proto b/src/main/protobuf/remote_scrubbing.proto new file mode 100644 index 00000000000000..9cbffd93448fa4 --- /dev/null +++ b/src/main/protobuf/remote_scrubbing.proto @@ -0,0 +1,82 @@ +// Copyright 2023 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. + +syntax = "proto3"; + +package remote_scrubbing; + +option java_package = "com.google.devtools.build.lib.remote"; + +// Describes when and how to scrub remote cache keys. +// See the documentation for the --experimental_remote_scrubbing_config flag. +message Config { + // Describes a set of actions. An action must pass all criteria to match. + message Matcher { + // A regex matching the canonical label of the action owner. + // Use .* if a partial match is desired. + // If empty, matches every label. + string label = 1; + // A regex matching the action mnemonic. + // Use .* if a partial match is desired. + // If empty, matches every mnemonic. + string mnemonic = 2; + // Whether to match actions built for a tool configuration. + // The default is to match only actions built for non-tool configurations. + bool match_tools = 3; + } + + // Describes a string replacement. + message Replacement { + // A regex matching a substring to be replaced. + string source = 1; + // The string to replace the first matching substring with. + string target = 2; + } + + // Describes a transformation to be applied to the cache key. + message Transform { + // A list of regexes matching an input path. + // An input whose path (relative to the execution root) exactly matches at + // least one of the regexes will be omitted from the cache key. + // Use .* if a partial match is desired. + repeated string omitted_inputs = 1; + + // A list of replacements to be applied to each command line argument. + // Each replacement is successively applied to the command line argument, + // operating on the result of the previous replacement. + repeated Replacement arg_replacements = 2; + + // An arbitrary value to be included in the remote cache key. + // It may be used to forcefully invalidate cache entries. + string salt = 3; + } + + // Describes a set of transformations to be applied against a set of actions. + message Rule { + // A matcher for actions this rule applies to. + Matcher matcher = 1; + // The transformation applied by this rule. + Transform transform = 2; + } + + // A list of configuration rules. + // For every action, the last matching rule is applied, and the remainder are + // ignored. Therefore, more specific rules should be specified last. + // + // Beware that every matcher is potentially applied to every action, and + // every transform is potentially applied to every input of a matching action, + // so a large number of rules containing costly regexes may have a noticeable + // impact on build performance. + repeated Rule rules = 1; +} diff --git a/src/main/protobuf/spawn.proto b/src/main/protobuf/spawn.proto index 5863f973424431..620cf1c2ace328 100644 --- a/src/main/protobuf/spawn.proto +++ b/src/main/protobuf/spawn.proto @@ -202,4 +202,13 @@ message CacheSalt { // Requires the execution service do NOT share caches across different // workspace. string workspace = 2; + + message ScrubSalt { + // A unique value used to bust the cache for scrubbed actions. + string salt = 1; + } + + // Ensures that a scrubbed action can never collide with a non-scrubbed one. + // See the documentation for the --experimental_remote_scrub_config flag. + ScrubSalt scrub_salt = 3; } diff --git a/src/test/java/com/google/devtools/build/lib/remote/BUILD b/src/test/java/com/google/devtools/build/lib/remote/BUILD index 01ddf17ee7ff7f..d0191278b30d06 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/BUILD +++ b/src/test/java/com/google/devtools/build/lib/remote/BUILD @@ -95,6 +95,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/remote", "//src/main/java/com/google/devtools/build/lib/remote:abstract_action_input_prefetcher", "//src/main/java/com/google/devtools/build/lib/remote:remote_output_checker", + "//src/main/java/com/google/devtools/build/lib/remote:scrubber", "//src/main/java/com/google/devtools/build/lib/remote:store", "//src/main/java/com/google/devtools/build/lib/remote/circuitbreaker", "//src/main/java/com/google/devtools/build/lib/remote/common", @@ -117,6 +118,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs", "//src/main/java/com/google/devtools/common/options", "//src/main/protobuf:failure_details_java_proto", + "//src/main/protobuf:remote_scrubbing_java_proto", "//src/main/protobuf:spawn_java_proto", "//src/test/java/com/google/devtools/build/lib/actions/util", "//src/test/java/com/google/devtools/build/lib/exec/util", diff --git a/src/test/java/com/google/devtools/build/lib/remote/GrpcCacheClientTest.java b/src/test/java/com/google/devtools/build/lib/remote/GrpcCacheClientTest.java index 1f089b4dd32ea0..96c6ed2b56e911 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/GrpcCacheClientTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/GrpcCacheClientTest.java @@ -286,6 +286,7 @@ public void testVirtualActionInputSupport() throws Exception { fakeFileCache, execRoot, ArtifactPathResolver.forExecRoot(execRoot), + /* spawnScrubber= */ null, DIGEST_UTIL); Digest digest = DIGEST_UTIL.compute(virtualActionInput.getBytes().toByteArray()); diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java index d8fb493413f87e..27ff9089e11438 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java @@ -87,6 +87,7 @@ import com.google.devtools.build.lib.exec.util.FakeOwner; import com.google.devtools.build.lib.exec.util.SpawnBuilder; import com.google.devtools.build.lib.remote.RemoteExecutionService.RemoteActionResult; +import com.google.devtools.build.lib.remote.RemoteScrubbing.Config; import com.google.devtools.build.lib.remote.common.BulkTransferException; import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; import com.google.devtools.build.lib.remote.common.RemoteCacheClient.CachedActionResult; @@ -94,6 +95,7 @@ import com.google.devtools.build.lib.remote.common.RemotePathResolver; import com.google.devtools.build.lib.remote.common.RemotePathResolver.DefaultRemotePathResolver; import com.google.devtools.build.lib.remote.common.RemotePathResolver.SiblingRepositoryLayoutResolver; +import com.google.devtools.build.lib.remote.merkletree.MerkleTree; import com.google.devtools.build.lib.remote.options.RemoteOptions; import com.google.devtools.build.lib.remote.util.DigestUtil; import com.google.devtools.build.lib.remote.util.FakeSpawnExecutionContext; @@ -2079,7 +2081,7 @@ public void buildMerkleTree_withMemoization_works() throws Exception { service.buildRemoteAction(spawn1, context1); // assert first time - verify(service, times(6)).uncachedBuildMerkleTreeVisitor(any(), any(), any()); + verify(service, times(6)).uncachedBuildMerkleTreeVisitor(any(), any(), any(), any()); assertThat(service.getMerkleTreeCache().asMap().keySet()) .containsExactly( ImmutableList.of( @@ -2102,7 +2104,7 @@ public void buildMerkleTree_withMemoization_works() throws Exception { service.buildRemoteAction(spawn2, context2); // assert second time - verify(service, times(6 + 2)).uncachedBuildMerkleTreeVisitor(any(), any(), any()); + verify(service, times(6 + 2)).uncachedBuildMerkleTreeVisitor(any(), any(), any(), any()); assertThat(service.getMerkleTreeCache().asMap().keySet()) .containsExactly( ImmutableList.of( @@ -2203,6 +2205,67 @@ public void buildRemoteActionForRemotePersistentWorkers() throws Exception { .build()); } + @Test + public void buildRemoteActionWithScrubbing() throws Exception { + var keptInput = ActionsTestUtil.createArtifact(artifactRoot, "kept_input"); + fakeFileCache.createScratchInput(keptInput, "kept"); + var scrubbedInput = ActionsTestUtil.createArtifact(artifactRoot, "scrubbed_input"); + fakeFileCache.createScratchInput(scrubbedInput, "scrubbed"); + + Spawn spawn = + new SpawnBuilder("some/path/cmd") + .withInputs(keptInput, scrubbedInput) + .withExecutionInfo(ExecutionRequirements.NO_REMOTE_EXEC, "") + .build(); + + FakeSpawnExecutionContext context = newSpawnExecutionContext(spawn); + remoteOptions.scrubber = + new Scrubber( + Config.newBuilder() + .addRules( + Config.Rule.newBuilder() + .setTransform( + Config.Transform.newBuilder() + .setSalt("NaCl") + .addOmittedInputs(".*scrubbed.*") + .addArgReplacements( + Config.Replacement.newBuilder() + .setSource("some/path") + .setTarget("another/dir")))) + .build()); + RemoteExecutionService service = newRemoteExecutionService(remoteOptions); + + RemoteAction remoteAction = service.buildRemoteAction(spawn, context); + + MerkleTree merkleTree = remoteAction.getMerkleTree(); + Directory actualRootDir = + merkleTree.getDirectoryByDigest(merkleTree.getRootProto().getDirectories(0).getDigest()); + + Directory expectedRootDir = + Directory.newBuilder() + .addFiles( + FileNode.newBuilder() + .setName("kept_input") + .setDigest( + Digest.newBuilder() + .setHash( + "79f076abdd19a752db7267bfff2f9022161d120dea919fdaca2ffdfc24ca8c96") + .setSizeBytes(4)) + .setIsExecutable(true)) + .build(); + + assertThat(actualRootDir).isEqualTo(expectedRootDir); + + assertThat(remoteAction.getCommand().getArgumentsList()).containsExactly("another/dir/cmd"); + + assertThat(remoteAction.getAction().getSalt()) + .isEqualTo( + CacheSalt.newBuilder() + .setScrubSalt(CacheSalt.ScrubSalt.newBuilder().setSalt("NaCl")) + .build() + .toByteString()); + } + private Spawn newSpawnFromResult(RemoteActionResult result) { return newSpawnFromResult(ImmutableMap.of(), result); } diff --git a/src/test/java/com/google/devtools/build/lib/remote/ScrubberTest.java b/src/test/java/com/google/devtools/build/lib/remote/ScrubberTest.java new file mode 100644 index 00000000000000..2eb2c0cc050b8b --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/remote/ScrubberTest.java @@ -0,0 +1,433 @@ +// Copyright 2023 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.remote; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.devtools.build.lib.actions.ActionInputHelper; +import com.google.devtools.build.lib.actions.Spawn; +import com.google.devtools.build.lib.actions.cache.VirtualActionInput; +import com.google.devtools.build.lib.exec.util.SpawnBuilder; +import com.google.devtools.build.lib.remote.RemoteScrubbing.Config; +import com.google.devtools.build.lib.remote.Scrubber.SpawnScrubber; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Tests for {@link Scrubber}. */ +@RunWith(JUnit4.class) +public class ScrubberTest { + + @Test + public void noScrubbing() { + var scrubber = new Scrubber(Config.getDefaultInstance()); + + assertThat(scrubber.forSpawn(createSpawn("//foo:bar", "Foo"))).isNull(); + } + + @Test + public void matchExactMnemonic() { + var scrubber = + new Scrubber( + Config.newBuilder() + .addRules( + Config.Rule.newBuilder() + .setMatcher(Config.Matcher.newBuilder().setMnemonic("Foo"))) + .build()); + + assertThat(scrubber.forSpawn(createSpawn("//foo:bar", "Foo"))).isNotNull(); + assertThat(scrubber.forSpawn(createSpawn("//foo:bar", "Foobar"))).isNull(); + } + + @Test + public void matchUnionMnemonic() { + var scrubber = + new Scrubber( + Config.newBuilder() + .addRules( + Config.Rule.newBuilder() + .setMatcher(Config.Matcher.newBuilder().setMnemonic("Foo|Bar"))) + .build()); + + assertThat(scrubber.forSpawn(createSpawn("//foo:bar", "Foo"))).isNotNull(); + assertThat(scrubber.forSpawn(createSpawn("//foo:bar", "Bar"))).isNotNull(); + assertThat(scrubber.forSpawn(createSpawn("//foo:bar", "Baz"))).isNull(); + } + + @Test + public void matchWildcardMnemonic() { + var scrubber = + new Scrubber( + Config.newBuilder() + .addRules( + Config.Rule.newBuilder() + .setMatcher(Config.Matcher.newBuilder().setMnemonic("Foo.*"))) + .build()); + + assertThat(scrubber.forSpawn(createSpawn("//foo:bar", "Foo"))).isNotNull(); + assertThat(scrubber.forSpawn(createSpawn("//foo:bar", "Foobar"))).isNotNull(); + assertThat(scrubber.forSpawn(createSpawn("//foo:bar", "Bar"))).isNull(); + } + + @Test + public void matchExactLabel() { + var scrubber = + new Scrubber( + Config.newBuilder() + .addRules( + Config.Rule.newBuilder() + .setMatcher(Config.Matcher.newBuilder().setLabel("//foo:bar"))) + .build()); + + assertThat(scrubber.forSpawn(createSpawn("//foo:bar", "Foo"))).isNotNull(); + assertThat(scrubber.forSpawn(createSpawn("//foo:barbaz", "Foo"))).isNull(); + } + + @Test + public void matchUnionLabel() { + var scrubber = + new Scrubber( + Config.newBuilder() + .addRules( + Config.Rule.newBuilder() + .setMatcher(Config.Matcher.newBuilder().setLabel("//foo:bar|//spam:eggs"))) + .build()); + + assertThat(scrubber.forSpawn(createSpawn("//foo:bar", "Foo"))).isNotNull(); + assertThat(scrubber.forSpawn(createSpawn("//spam:eggs", "Foo"))).isNotNull(); + assertThat(scrubber.forSpawn(createSpawn("//quux:xyzzy", "Foo"))).isNull(); + } + + @Test + public void matchWildcardLabel() { + var scrubber = + new Scrubber( + Config.newBuilder() + .addRules( + Config.Rule.newBuilder() + .setMatcher(Config.Matcher.newBuilder().setLabel("//foo:.*"))) + .build()); + + assertThat(scrubber.forSpawn(createSpawn("//foo:bar", "Foo"))).isNotNull(); + assertThat(scrubber.forSpawn(createSpawn("//foo:baz", "Foo"))).isNotNull(); + assertThat(scrubber.forSpawn(createSpawn("//spam:eggs", "Foo"))).isNull(); + } + + @Test + public void rejectToolAction() { + var scrubber = + new Scrubber( + Config.newBuilder() + .addRules( + Config.Rule.newBuilder() + .setMatcher( + Config.Matcher.newBuilder().setLabel("//foo:bar").setMnemonic("Foo"))) + .build()); + + assertThat(scrubber.forSpawn(createSpawn("//foo:bar", "Foo"))).isNotNull(); + assertThat(scrubber.forSpawn(createSpawn("//foo:bar", "Foo", /* forTool= */ true))).isNull(); + } + + @Test + public void acceptToolAction() { + var scrubber = + new Scrubber( + Config.newBuilder() + .addRules( + Config.Rule.newBuilder() + .setMatcher( + Config.Matcher.newBuilder() + .setLabel("//foo:bar") + .setMnemonic("Foo") + .setMatchTools(true))) + .build()); + + assertThat(scrubber.forSpawn(createSpawn("//foo:bar", "Foo"))).isNotNull(); + assertThat(scrubber.forSpawn(createSpawn("//foo:bar", "Foo", /* forTool= */ true))).isNotNull(); + } + + @Test + public void noOmittedInputs() { + var spawnScrubber = + new Scrubber(Config.newBuilder().addRules(Config.Rule.getDefaultInstance()).build()) + .forSpawn(createSpawn()); + + assertThat(spawnScrubber.shouldOmitInput(ActionInputHelper.fromPath("foo/bar"))).isFalse(); + } + + @Test + public void exactOmittedInput() { + var spawnScrubber = + new Scrubber( + Config.newBuilder() + .addRules( + Config.Rule.newBuilder() + .setTransform( + Config.Transform.newBuilder().addOmittedInputs("foo/bar"))) + .build()) + .forSpawn(createSpawn()); + + assertThat(spawnScrubber.shouldOmitInput(ActionInputHelper.fromPath("foo/bar"))).isTrue(); + assertThat(spawnScrubber.shouldOmitInput(ActionInputHelper.fromPath("foo/bar/baz"))).isFalse(); + assertThat(spawnScrubber.shouldOmitInput(ActionInputHelper.fromPath("bazel-out/foo/bar"))) + .isFalse(); + } + + @Test + public void wildcardOmittedInput() { + var spawnScrubber = + new Scrubber( + Config.newBuilder() + .addRules( + Config.Rule.newBuilder() + .setTransform( + Config.Transform.newBuilder().addOmittedInputs("foo/bar.*"))) + .build()) + .forSpawn(createSpawn()); + + assertThat(spawnScrubber.shouldOmitInput(ActionInputHelper.fromPath("foo/bar"))).isTrue(); + assertThat(spawnScrubber.shouldOmitInput(ActionInputHelper.fromPath("foo/bar/baz"))).isTrue(); + assertThat(spawnScrubber.shouldOmitInput(ActionInputHelper.fromPath("bazel-out/foo/bar"))) + .isFalse(); + } + + @Test + public void multipleOmittedInputs() { + var spawnScrubber = + new Scrubber( + Config.newBuilder() + .addRules( + Config.Rule.newBuilder() + .setTransform( + Config.Transform.newBuilder() + .addOmittedInputs("foo/bar") + .addOmittedInputs("spam/eggs"))) + .build()) + .forSpawn(createSpawn()); + + assertThat(spawnScrubber.shouldOmitInput(ActionInputHelper.fromPath("foo/bar"))).isTrue(); + assertThat(spawnScrubber.shouldOmitInput(ActionInputHelper.fromPath("spam/eggs"))).isTrue(); + assertThat(spawnScrubber.shouldOmitInput(ActionInputHelper.fromPath("foo/bar/baz"))).isFalse(); + assertThat(spawnScrubber.shouldOmitInput(ActionInputHelper.fromPath("bazel-out/foo/bar"))) + .isFalse(); + assertThat(spawnScrubber.shouldOmitInput(ActionInputHelper.fromPath("spam/eggs/bacon"))) + .isFalse(); + assertThat(spawnScrubber.shouldOmitInput(ActionInputHelper.fromPath("bazel-out/spam/eggs"))) + .isFalse(); + } + + @Test + public void doNotScrubEmptyMarker() { + var spawnScrubber = + new Scrubber( + Config.newBuilder() + .addRules( + Config.Rule.newBuilder() + .setTransform(Config.Transform.newBuilder().addOmittedInputs(".*"))) + .build()) + .forSpawn(createSpawn()); + + assertThat(spawnScrubber.shouldOmitInput(VirtualActionInput.EMPTY_MARKER)).isFalse(); + } + + @Test + public void simpleArgReplacement() { + var spawnScrubber = + new Scrubber( + Config.newBuilder() + .addRules( + Config.Rule.newBuilder() + .setTransform( + Config.Transform.newBuilder() + .addArgReplacements( + Config.Replacement.newBuilder() + .setSource("foo") + .setTarget("bar")))) + .build()) + .forSpawn(createSpawn()); + + assertThat(spawnScrubber.transformArgument("foo")).isEqualTo("bar"); + assertThat(spawnScrubber.transformArgument("abcfooxyz")).isEqualTo("abcbarxyz"); + assertThat(spawnScrubber.transformArgument("bar")).isEqualTo("bar"); + assertThat(spawnScrubber.transformArgument("foofoo")).isEqualTo("barfoo"); + } + + @Test + public void anchoredArgReplacement() { + var spawnScrubber = + new Scrubber( + Config.newBuilder() + .addRules( + Config.Rule.newBuilder() + .setTransform( + Config.Transform.newBuilder() + .addArgReplacements( + Config.Replacement.newBuilder() + .setSource("^foo$") + .setTarget("bar")))) + .build()) + .forSpawn(createSpawn()); + + assertThat(spawnScrubber.transformArgument("foo")).isEqualTo("bar"); + assertThat(spawnScrubber.transformArgument("abcfooxyz")).isEqualTo("abcfooxyz"); + } + + @Test + public void wildcardArgReplacement() { + var spawnScrubber = + new Scrubber( + Config.newBuilder() + .addRules( + Config.Rule.newBuilder() + .setTransform( + Config.Transform.newBuilder() + .addArgReplacements( + Config.Replacement.newBuilder() + .setSource("foo[12]") + .setTarget("bar")))) + .build()) + .forSpawn(createSpawn()); + + assertThat(spawnScrubber.transformArgument("foo1")).isEqualTo("bar"); + assertThat(spawnScrubber.transformArgument("foo2")).isEqualTo("bar"); + assertThat(spawnScrubber.transformArgument("foo3")).isEqualTo("foo3"); + } + + @Test + public void multipleArgReplacements() { + var spawnScrubber = + new Scrubber( + Config.newBuilder() + .addRules( + Config.Rule.newBuilder() + .setTransform( + Config.Transform.newBuilder() + .addArgReplacements( + Config.Replacement.newBuilder() + .setSource("foo") + .setTarget("bar")) + .addArgReplacements( + Config.Replacement.newBuilder() + .setSource("spam") + .setTarget("eggs")))) + .build()) + .forSpawn(createSpawn()); + + assertThat(spawnScrubber.transformArgument("abcfoo123spamxyz")).isEqualTo("abcbar123eggsxyz"); + assertThat(spawnScrubber.transformArgument("abcfoo")).isEqualTo("abcbar"); + assertThat(spawnScrubber.transformArgument("abcspam")).isEqualTo("abceggs"); + assertThat(spawnScrubber.transformArgument("bareggs")).isEqualTo("bareggs"); + } + + @Test + public void withoutSalt() { + var spawnScrubber = + new Scrubber(Config.newBuilder().addRules(Config.Rule.getDefaultInstance()).build()) + .forSpawn(createSpawn()); + + assertThat(spawnScrubber.getSalt()).isEmpty(); + } + + @Test + public void withSalt() { + var spawnScrubber = + new Scrubber( + Config.newBuilder() + .addRules( + Config.Rule.newBuilder() + .setTransform(Config.Transform.newBuilder().setSalt("NaCl"))) + .build()) + .forSpawn(createSpawn()); + + assertThat(spawnScrubber.getSalt()).isEqualTo("NaCl"); + } + + @Test + public void orthogonalRules() { + var scrubber = + new Scrubber( + Config.newBuilder() + .addRules( + Config.Rule.newBuilder() + .setMatcher(Config.Matcher.newBuilder().setLabel("//foo:bar")) + .setTransform( + Config.Transform.newBuilder() + .addArgReplacements( + Config.Replacement.newBuilder() + .setSource("foo") + .setTarget("bar")))) + .addRules( + Config.Rule.newBuilder() + .setMatcher(Config.Matcher.newBuilder().setLabel("//spam:eggs")) + .setTransform( + Config.Transform.newBuilder() + .addArgReplacements( + Config.Replacement.newBuilder() + .setSource("spam") + .setTarget("eggs")))) + .build()); + + SpawnScrubber spawnScrubberForFooBar = scrubber.forSpawn(createSpawn("//foo:bar", "Foo")); + assertThat(spawnScrubberForFooBar).isNotNull(); + assertThat(spawnScrubberForFooBar.transformArgument("foospam")).isEqualTo("barspam"); + + SpawnScrubber spawnScrubberForSpamEggs = scrubber.forSpawn(createSpawn("//spam:eggs", "Spam")); + assertThat(spawnScrubberForSpamEggs).isNotNull(); + assertThat(spawnScrubberForSpamEggs.transformArgument("foospam")).isEqualTo("fooeggs"); + } + + @Test + public void lastRuleWins() { + var spawnScrubber = + new Scrubber( + Config.newBuilder() + .addRules( + Config.Rule.newBuilder() + .setTransform( + Config.Transform.newBuilder() + .addArgReplacements( + Config.Replacement.newBuilder() + .setSource("foo") + .setTarget("bar")))) + .addRules( + Config.Rule.newBuilder() + .setTransform( + Config.Transform.newBuilder() + .addArgReplacements( + Config.Replacement.newBuilder() + .setSource("spam") + .setTarget("eggs")))) + .build()) + .forSpawn(createSpawn()); + + assertThat(spawnScrubber.transformArgument("foospam")).isEqualTo("fooeggs"); + } + + private static Spawn createSpawn() { + return createSpawn("//foo:bar", "Foo"); + } + + private static Spawn createSpawn(String label, String mnemonic) { + return createSpawn(label, mnemonic, /* forTool= */ false); + } + + private static Spawn createSpawn(String label, String mnemonic, boolean forTool) { + return new SpawnBuilder("cmd") + .withOwnerLabel(label) + .withMnemonic(mnemonic) + .setBuiltForToolConfiguration(forTool) + .build(); + } +} diff --git a/src/test/java/com/google/devtools/build/lib/remote/merkletree/ActionInputDirectoryTreeTest.java b/src/test/java/com/google/devtools/build/lib/remote/merkletree/ActionInputDirectoryTreeTest.java index 87eef72bf77077..0b6ef3c1200f4b 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/merkletree/ActionInputDirectoryTreeTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/merkletree/ActionInputDirectoryTreeTest.java @@ -55,6 +55,7 @@ protected DirectoryTree build(Path... paths) throws IOException { new StaticInputMetadataProvider(metadata), execRoot, ArtifactPathResolver.forExecRoot(execRoot), + /* spawnScrubber= */ null, digestUtil); } @@ -72,6 +73,7 @@ public void virtualActionInputShouldWork() throws Exception { new StaticInputMetadataProvider(metadata), execRoot, ArtifactPathResolver.forExecRoot(execRoot), + /* spawnScrubber= */ null, digestUtil); assertLexicographicalOrder(tree); @@ -121,6 +123,7 @@ public void directoryInputShouldBeExpanded() throws Exception { new StaticInputMetadataProvider(metadata), execRoot, ArtifactPathResolver.forExecRoot(execRoot), + /* spawnScrubber= */ null, digestUtil); assertLexicographicalOrder(tree); @@ -169,6 +172,7 @@ public void filesShouldBeDeduplicated() throws Exception { new StaticInputMetadataProvider(metadata), execRoot, ArtifactPathResolver.forExecRoot(execRoot), + /* spawnScrubber= */ null, digestUtil); assertLexicographicalOrder(tree); diff --git a/src/test/java/com/google/devtools/build/lib/remote/merkletree/MerkleTreeTest.java b/src/test/java/com/google/devtools/build/lib/remote/merkletree/MerkleTreeTest.java index 86be3e066731a8..6c91c85bde8517 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/merkletree/MerkleTreeTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/merkletree/MerkleTreeTest.java @@ -74,6 +74,7 @@ public void emptyMerkleTree() throws IOException { new StaticInputMetadataProvider(Collections.emptyMap()), execRoot, ArtifactPathResolver.forExecRoot(execRoot), + /* spawnScrubber= */ null, digestUtil); Digest emptyDigest = digestUtil.compute(new byte[0]); assertThat(tree.getRootDigest()).isEqualTo(emptyDigest); @@ -115,6 +116,7 @@ public void buildMerkleTree() throws IOException { new StaticInputMetadataProvider(metadata), execRoot, ArtifactPathResolver.forExecRoot(execRoot), + /* spawnScrubber= */ null, digestUtil); // assert @@ -168,6 +170,7 @@ public void mergeMerkleTrees() throws IOException { new StaticInputMetadataProvider(metadata), execRoot, ArtifactPathResolver.forExecRoot(execRoot), + /* spawnScrubber= */ null, digestUtil); MerkleTree tree1 = MerkleTree.build( @@ -175,6 +178,7 @@ public void mergeMerkleTrees() throws IOException { new StaticInputMetadataProvider(metadata), execRoot, ArtifactPathResolver.forExecRoot(execRoot), + /* spawnScrubber= */ null, digestUtil); MerkleTree tree2 = MerkleTree.build( @@ -182,6 +186,7 @@ public void mergeMerkleTrees() throws IOException { new StaticInputMetadataProvider(metadata), execRoot, ArtifactPathResolver.forExecRoot(execRoot), + /* spawnScrubber= */ null, digestUtil); MerkleTree treeAll = MerkleTree.build( @@ -189,6 +194,7 @@ public void mergeMerkleTrees() throws IOException { new StaticInputMetadataProvider(metadata), execRoot, ArtifactPathResolver.forExecRoot(execRoot), + /* spawnScrubber= */ null, digestUtil); // act diff --git a/src/test/shell/bazel/remote/remote_execution_test.sh b/src/test/shell/bazel/remote/remote_execution_test.sh index 2818dcc6ba216a..5c0573752adb96 100755 --- a/src/test/shell/bazel/remote/remote_execution_test.sh +++ b/src/test/shell/bazel/remote/remote_execution_test.sh @@ -3281,4 +3281,60 @@ EOF //:x &> $TEST_log || fail "expected success" } +function test_cache_key_scrubbing() { + echo foo > foo + echo bar > bar + cat<<'EOF' > BUILD +exports_files(["foo", "bar"]) + +label_flag( + name = "src", + build_setting_default = "//missing:target", +) + +genrule( + name = "gen", + srcs = [":src"], + outs = ["out"], + cmd = "echo built > $@", +) +EOF + cat<<'EOF' > scrubbing.cfg +rules { + transform { + omitted_inputs: "^(foo|bar)$" + } +} +EOF + + # First build without a cache. Even though the remote cache keys can be + # scrubbed, Bazel still considers the actions distinct. + + bazel build --experimental_remote_scrubbing_config=scrubbing.cfg \ + --//:src=//:foo :gen &> $TEST_log \ + || fail "failed to build with input foo and no cache" + expect_log "2 processes: 1 internal, 1 .*-sandbox" + + bazel build --experimental_remote_scrubbing_config=scrubbing.cfg \ + --//:src=//:bar :gen &> $TEST_log \ + || fail "failed to build with input bar and no cache" + expect_log "2 processes: 1 internal, 1 .*-sandbox" + + # Now build with a cache. Even though Bazel considers the actions distinct, + # they will be looked up in the remote cache using the scrubbed key, so one + # can serve as a cache hit for the other. + + CACHEDIR=$(mktemp -d) + + bazel build --experimental_remote_scrubbing_config=scrubbing.cfg \ + --disk_cache=$CACHEDIR --//:src=//:foo :gen &> $TEST_log \ + || fail "failed to build with input foo and a cache" + expect_log "2 processes: 1 internal, 1 .*-sandbox" + + bazel build --experimental_remote_scrubbing_config=scrubbing.cfg \ + --disk_cache=$CACHEDIR --//:src=//:bar :gen &> $TEST_log \ + || fail "failed to build with input bar and a cache" + expect_log "2 processes: 1 disk cache hit, 1 internal" +} + run_suite "Remote execution and remote cache tests"