From fdaaa6a7b0aaa304b3e119157fb41b26a69582de Mon Sep 17 00:00:00 2001 From: Googler Date: Tue, 21 May 2024 02:36:35 -0700 Subject: [PATCH] Basic strategy-policy implementation Basic foundation for limiting strategies used in bazel a la invocation policy. Implemented as an explicit policy for fine-grained control, though for now we just implement an allowlist (in the future it should be extensible enough for more creativity). Naming of the fields in the proto definition is biased towards more ergonomic textproto configuration. Alternatives considered include extending invocation policy to support key/value flags, but this seemed like too much power being added to the flag system for too narrow of a use case. Note that this policy applies universally, regardless of whether or not strategies come by flag or are hard-coded in bazel. This is intentional - it allows *full* control over where/how bazel is executing actions. --strategy_regexp is presently ignored, it requires larger changes to implement, will be done in the next CLs. PiperOrigin-RevId: 635733824 Change-Id: Ic43cf76c2b58b0fcce089119b8ce43a0116c8490 --- .../build/lib/buildtool/ExecutionTool.java | 3 +- .../com/google/devtools/build/lib/exec/BUILD | 11 ++ .../build/lib/exec/SpawnStrategyPolicy.java | 80 +++++++++++++ .../build/lib/exec/SpawnStrategyRegistry.java | 110 +++++++++++------- .../lib/runtime/BlazeCommandDispatcher.java | 1 + .../build/lib/runtime/BlazeWorkspace.java | 3 + .../build/lib/runtime/CommandEnvironment.java | 7 ++ .../runtime/commands/CanonicalizeCommand.java | 6 +- src/main/protobuf/BUILD | 19 ++- src/main/protobuf/invocation_policy.proto | 5 + src/main/protobuf/strategy_policy.proto | 67 +++++++++++ .../buildtool/util/BlazeRuntimeWrapper.java | 1 + .../com/google/devtools/build/lib/exec/BUILD | 2 + .../lib/exec/SpawnStrategyPolicyTest.java | 94 +++++++++++++++ .../lib/exec/SpawnStrategyRegistryTest.java | 54 +++++++++ .../google/devtools/build/lib/remote/BUILD | 1 + .../build/lib/remote/RemoteModuleTest.java | 2 + .../build/lib/runtime/BlazeRuntimeTest.java | 3 + 18 files changed, 425 insertions(+), 44 deletions(-) create mode 100644 src/main/java/com/google/devtools/build/lib/exec/SpawnStrategyPolicy.java create mode 100644 src/main/protobuf/strategy_policy.proto create mode 100644 src/test/java/com/google/devtools/build/lib/exec/SpawnStrategyPolicyTest.java diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java index 75d8c7f9fc4227..6e6abdf47847e4 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java @@ -162,7 +162,8 @@ public class ExecutionTool { ExecutorBuilder executorBuilder = new ExecutorBuilder(); ModuleActionContextRegistry.Builder actionContextRegistryBuilder = ModuleActionContextRegistry.builder(); - SpawnStrategyRegistry.Builder spawnStrategyRegistryBuilder = SpawnStrategyRegistry.builder(); + SpawnStrategyRegistry.Builder spawnStrategyRegistryBuilder = + SpawnStrategyRegistry.builder(env.getInvocationPolicy().getStrategyPolicy()); actionContextRegistryBuilder.register(SpawnStrategyResolver.class, new SpawnStrategyResolver()); actionContextRegistryBuilder.register( ImportantOutputHandler.class, ImportantOutputHandler.NO_OP); diff --git a/src/main/java/com/google/devtools/build/lib/exec/BUILD b/src/main/java/com/google/devtools/build/lib/exec/BUILD index 220c071bcfde5d..43aa11ad9485a6 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/BUILD +++ b/src/main/java/com/google/devtools/build/lib/exec/BUILD @@ -337,18 +337,29 @@ java_library( ], ) +java_library( + name = "spawn_strategy_policy", + srcs = ["SpawnStrategyPolicy.java"], + deps = [ + "//src/main/protobuf:strategy_policy_java_proto", + "//third_party:guava", + ], +) + java_library( name = "spawn_strategy_registry", srcs = ["SpawnStrategyRegistry.java"], deps = [ ":abstract_spawn_strategy", ":remote_local_fallback_registry", + ":spawn_strategy_policy", "//src/main/java/com/google/devtools/build/lib/actions", "//src/main/java/com/google/devtools/build/lib/events", "//src/main/java/com/google/devtools/build/lib/util", "//src/main/java/com/google/devtools/build/lib/util:abrupt_exit_exception", "//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code", "//src/main/protobuf:failure_details_java_proto", + "//src/main/protobuf:strategy_policy_java_proto", "//third_party:auto_value", "//third_party:flogger", "//third_party:guava", diff --git a/src/main/java/com/google/devtools/build/lib/exec/SpawnStrategyPolicy.java b/src/main/java/com/google/devtools/build/lib/exec/SpawnStrategyPolicy.java new file mode 100644 index 00000000000000..ab3669ad287a3e --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/exec/SpawnStrategyPolicy.java @@ -0,0 +1,80 @@ +// Copyright 2022 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.exec; + +import static com.google.common.collect.ImmutableList.toImmutableList; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import com.google.devtools.build.lib.runtime.proto.MnemonicPolicy; +import com.google.devtools.build.lib.runtime.proto.StrategiesForMnemonic; +import java.util.List; + +/** Policy for filtering spawn strategies. */ +public interface SpawnStrategyPolicy { + + /** Returns result of applying policy to per-mnemonic strategies. */ + ImmutableList apply(String mnemonic, List strategies); + + /** Creates new policy from proto descriptor. Empty proto policy implies everything allowed. */ + static SpawnStrategyPolicy create(MnemonicPolicy policy) { + if (MnemonicPolicy.getDefaultInstance().equals(policy)) { + return new AllowAllStrategiesPolicy(); + } + + ImmutableMap.Builder> perMnemonicAllowList = + ImmutableMap.builder(); + for (StrategiesForMnemonic strategiesForMnemonic : policy.getStrategyAllowlistList()) { + perMnemonicAllowList.put( + strategiesForMnemonic.getMnemonic(), + ImmutableSet.copyOf(strategiesForMnemonic.getStrategyList())); + } + return new SpawnStrategyPolicyImpl( + perMnemonicAllowList.buildKeepingLast(), + ImmutableSet.copyOf(policy.getDefaultAllowlistList())); + } + + /** Allows all strategies - effectively a no-op strategy. */ + class AllowAllStrategiesPolicy implements SpawnStrategyPolicy { + + private AllowAllStrategiesPolicy() {} + + @Override + public ImmutableList apply(String mnemonic, List strategies) { + return ImmutableList.copyOf(strategies); + } + } + + /** Enforces a real strategy policy based on provided config. */ + class SpawnStrategyPolicyImpl implements SpawnStrategyPolicy { + + private final ImmutableMap> perMnemonicAllowList; + private final ImmutableSet defaultAllowList; + + private SpawnStrategyPolicyImpl( + ImmutableMap> perMnemonicAllowList, + ImmutableSet defaultAllowList) { + this.perMnemonicAllowList = perMnemonicAllowList; + this.defaultAllowList = defaultAllowList; + } + + @Override + public ImmutableList apply(String mnemonic, List strategies) { + ImmutableSet allowList = + perMnemonicAllowList.getOrDefault(mnemonic, defaultAllowList); + return strategies.stream().filter(allowList::contains).collect(toImmutableList()); + } + } +} diff --git a/src/main/java/com/google/devtools/build/lib/exec/SpawnStrategyRegistry.java b/src/main/java/com/google/devtools/build/lib/exec/SpawnStrategyRegistry.java index 5226916f039f79..6a7c52a5e68474 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/SpawnStrategyRegistry.java +++ b/src/main/java/com/google/devtools/build/lib/exec/SpawnStrategyRegistry.java @@ -36,6 +36,8 @@ import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.EventHandler; import com.google.devtools.build.lib.events.Reporter; +import com.google.devtools.build.lib.runtime.proto.MnemonicPolicy; +import com.google.devtools.build.lib.runtime.proto.StrategyPolicy; import com.google.devtools.build.lib.server.FailureDetails; import com.google.devtools.build.lib.server.FailureDetails.ExecutionOptions.Code; import com.google.devtools.build.lib.server.FailureDetails.FailureDetail; @@ -61,6 +63,9 @@ public final class SpawnStrategyRegistry implements DynamicStrategyRegistry, ActionContext, RemoteLocalFallbackRegistry { private static final GoogleLogger logger = GoogleLogger.forEnclosingClass(); + private static final SpawnStrategyPolicy ALLOW_ALL_STRATEGIES = + SpawnStrategyPolicy.create(MnemonicPolicy.getDefaultInstance()); + private final ImmutableListMultimap mnemonicToStrategies; private final ImmutableListMultimap filterToStrategies; private final ImmutableList defaultStrategies; @@ -232,8 +237,20 @@ private static String toImplementationNames(Collection strategies) { } /** Returns a new {@link Builder} suitable for creating instances of SpawnStrategyRegistry. */ + @VisibleForTesting public static Builder builder() { - return new Builder(); + return new Builder( + /* strategyPolicy= */ ALLOW_ALL_STRATEGIES, + /* dynamicRemotePolicy= */ ALLOW_ALL_STRATEGIES, + /* dynamicLocalPolicy= */ ALLOW_ALL_STRATEGIES); + } + + /** Returns a new {@link Builder} suitable for creating instances of SpawnStrategyRegistry. */ + public static Builder builder(StrategyPolicy strategyPolicyProto) { + return new Builder( + SpawnStrategyPolicy.create(strategyPolicyProto.getMnemonicPolicy()), + SpawnStrategyPolicy.create(strategyPolicyProto.getDynamicRemotePolicy()), + SpawnStrategyPolicy.create(strategyPolicyProto.getDynamicLocalPolicy())); } /** @@ -257,6 +274,9 @@ public static final class Builder { private ImmutableList explicitDefaultStrategies = ImmutableList.of(); + private final SpawnStrategyPolicy strategyPolicy; + private final SpawnStrategyPolicy dynamicRemotePolicy; + private final SpawnStrategyPolicy dynamicLocalPolicy; // TODO(schmitt): Using a list and autovalue so as to be able to reverse order while legacy sort // is supported. Can be converted to same as mnemonics once legacy behavior is removed. private final List filterAndIdentifiers = new ArrayList<>(); @@ -269,6 +289,15 @@ public static final class Builder { @Nullable private String remoteLocalFallbackStrategyIdentifier; + private Builder( + SpawnStrategyPolicy strategyPolicy, + SpawnStrategyPolicy dynamicRemotePolicy, + SpawnStrategyPolicy dynamicLocalPolicy) { + this.strategyPolicy = strategyPolicy; + this.dynamicRemotePolicy = dynamicRemotePolicy; + this.dynamicLocalPolicy = dynamicLocalPolicy; + } + /** * Adds a filter limiting any spawn whose {@linkplain * com.google.devtools.build.lib.actions.ActionExecutionMetadata#getProgressMessage() owner's @@ -292,17 +321,45 @@ public Builder addDescriptionFilter(RegexFilter filter, List identifiers * command-line identifiers, in order. * *

If the same mnemonic is registered multiple times the last such call will take precedence. + * Or in other words, last one wins. * *

Note that if a spawn matches a {@linkplain #addDescriptionFilter registered description * filter} that filter will take precedence over any mnemonic-based filters. */ - // last one wins @CanIgnoreReturnValue public Builder addMnemonicFilter(String mnemonic, List identifiers) { mnemonicToIdentifiers.put(mnemonic, identifiers); return this; } + /** + * Sets the strategy names to use in the remote branch of dynamic execution for a set of action + * mnemonics. + * + *

During execution, each strategy is {@linkplain SpawnStrategy#canExec(Spawn, + * ActionContextRegistry) asked} whether it can execute a given Spawn. The first strategy in the + * list that says so will get the job. + */ + @CanIgnoreReturnValue + public Builder addDynamicRemoteStrategies(Map> strategies) { + mnemonicToRemoteDynamicIdentifiers.putAll(strategies); + return this; + } + + /** + * Sets the strategy names to use in the local branch of dynamic execution for a number of + * action mnemonics. + * + *

During execution, each strategy is {@linkplain SpawnStrategy#canExec(Spawn, + * ActionContextRegistry) asked} whether it can execute a given Spawn. The first strategy in the + * list that says so will get the job. + */ + @CanIgnoreReturnValue + public Builder addDynamicLocalStrategies(Map> strategies) { + mnemonicToLocalDynamicIdentifiers.putAll(strategies); + return this; + } + /** * Registers a strategy implementation with this collector, distinguishing it from other * strategies with the given command-line identifiers (of which at least one is required). @@ -348,34 +405,6 @@ public Builder resetDefaultStrategies() { return this; } - /** - * Sets the strategy names to use in the remote branch of dynamic execution for a set of action - * mnemonics. - * - *

During execution, each strategy is {@linkplain SpawnStrategy#canExec(Spawn, - * ActionContextRegistry) asked} whether it can execute a given Spawn. The first strategy in the - * list that says so will get the job. - */ - @CanIgnoreReturnValue - public Builder addDynamicRemoteStrategies(Map> strategies) { - mnemonicToRemoteDynamicIdentifiers.putAll(strategies); - return this; - } - - /** - * Sets the strategy names to use in the local branch of dynamic execution for a number of - * action mnemonics. - * - *

During execution, each strategy is {@linkplain SpawnStrategy#canExec(Spawn, - * ActionContextRegistry) asked} whether it can execute a given Spawn. The first strategy in the - * list that says so will get the job. - */ - @CanIgnoreReturnValue - public Builder addDynamicLocalStrategies(Map> strategies) { - mnemonicToLocalDynamicIdentifiers.putAll(strategies); - return this; - } - /** * Sets the commandline identifier of the strategy to be used when falling back from remote to * local execution. @@ -412,24 +441,31 @@ public SpawnStrategyRegistry build() throws AbruptExitException { ImmutableListMultimap.Builder mnemonicToStrategies = new ImmutableListMultimap.Builder<>(); for (Map.Entry> entry : mnemonicToIdentifiers.entrySet()) { + String mnemonic = entry.getKey(); + ImmutableList sanitizedStrategies = + strategyPolicy.apply(mnemonic, entry.getValue()); mnemonicToStrategies.putAll( - entry.getKey(), toStrategies(entry.getValue(), "mnemonic " + entry.getKey())); + mnemonic, toStrategies(sanitizedStrategies, "mnemonic " + mnemonic)); } ImmutableListMultimap.Builder mnemonicToLocalStrategies = new ImmutableListMultimap.Builder<>(); for (Map.Entry> entry : mnemonicToLocalDynamicIdentifiers.entrySet()) { + String mnemonic = entry.getKey(); + ImmutableList sanitizedStrategies = + dynamicLocalPolicy.apply(mnemonic, entry.getValue()); mnemonicToLocalStrategies.putAll( - entry.getKey(), - toSandboxedStrategies(entry.getValue(), "local mnemonic " + entry.getKey())); + mnemonic, toSandboxedStrategies(sanitizedStrategies, "local mnemonic " + mnemonic)); } ImmutableListMultimap.Builder mnemonicToRemoteStrategies = new ImmutableListMultimap.Builder<>(); for (Map.Entry> entry : mnemonicToRemoteDynamicIdentifiers.entrySet()) { + String mnemonic = entry.getKey(); + ImmutableList sanitizedStrategies = + dynamicRemotePolicy.apply(mnemonic, entry.getValue()); mnemonicToRemoteStrategies.putAll( - entry.getKey(), - toSandboxedStrategies(entry.getValue(), "remote mnemonic " + entry.getKey())); + mnemonic, toSandboxedStrategies(sanitizedStrategies, "remote mnemonic " + mnemonic)); } AbstractSpawnStrategy remoteLocalFallbackStrategy = null; @@ -515,10 +551,6 @@ private Iterable toSandboxedStrategies( } } - public ImmutableListMultimap getMnemonicToStrategies() { - return mnemonicToStrategies; - } - private static AbruptExitException createExitException(String message, Code detailedCode) { return new AbruptExitException( DetailedExitCode.of( diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java index c1ba7d73fc59d7..3be27fdf2e78a3 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java @@ -348,6 +348,7 @@ private BlazeCommandResult execExclusively( workspace.initCommand( commandAnnotation, options, + invocationPolicy, commandEnvWarnings, waitTimeInMs, firstContactTime, diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeWorkspace.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeWorkspace.java index 19d8766abbfbf7..c8233a9a1e8295 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeWorkspace.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeWorkspace.java @@ -31,6 +31,7 @@ import com.google.devtools.build.lib.profiler.AutoProfiler; import com.google.devtools.build.lib.profiler.ProfilerTask; import com.google.devtools.build.lib.profiler.memory.AllocationTracker; +import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.InvocationPolicy; import com.google.devtools.build.lib.skyframe.SkyframeExecutor; import com.google.devtools.build.lib.util.io.CommandExtensionReporter; import com.google.devtools.build.lib.vfs.FileSystemUtils; @@ -205,6 +206,7 @@ public Range getLastExecutionTimeRange() { public CommandEnvironment initCommand( Command command, OptionsParsingResult options, + InvocationPolicy invocationPolicy, List warnings, long waitTimeInMs, long commandStartTime, @@ -221,6 +223,7 @@ public CommandEnvironment initCommand( Thread.currentThread(), command, options, + invocationPolicy, syscallCache, quiescingExecutors, warnings, diff --git a/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java b/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java index 2db853500a2b35..e3e68acc6d3460 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java @@ -108,6 +108,7 @@ public class CommandEnvironment { private final Thread commandThread; private final Command command; private final OptionsParsingResult options; + private final InvocationPolicy invocationPolicy; private final PathPackageLocator packageLocator; private final Path workingDirectory; private final PathFragment relativeWorkingDirectory; @@ -193,6 +194,7 @@ public void exit(AbruptExitException exception) { Thread commandThread, Command command, OptionsParsingResult options, + InvocationPolicy invocationPolicy, SyscallCache syscallCache, QuiescingExecutors quiescingExecutors, List warnings, @@ -212,6 +214,7 @@ public void exit(AbruptExitException exception) { this.commandThread = commandThread; this.command = command; this.options = options; + this.invocationPolicy = invocationPolicy; this.shutdownReasonConsumer = shutdownReasonConsumer; this.syscallCache = syscallCache; this.quiescingExecutors = quiescingExecutors; @@ -448,6 +451,10 @@ public OptionsParsingResult getOptions() { return options; } + public InvocationPolicy getInvocationPolicy() { + return invocationPolicy; + } + public void setInvocationPolicyFlags(ImmutableList invocationPolicyFlags) { this.invocationPolicyFlags = invocationPolicyFlags; } diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/CanonicalizeCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/CanonicalizeCommand.java index d4c1504681ae3c..dcd6b9db200b75 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/commands/CanonicalizeCommand.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/CanonicalizeCommand.java @@ -62,9 +62,9 @@ mustRunInWorkspace = false, shortDescription = "Canonicalizes a list of %{product} options.", help = - "This command canonicalizes a list of %{product} options. Don't forget to prepend " - + " '--' to end option parsing before the flags to canonicalize.\n" - + "%{options}") + "This command canonicalizes a list of %{product} options. Don't forget to prepend '--' to" + + " end option parsing before the flags to canonicalize. This command doesn't support" + + " the strategy policies under --invocation_policy flag.\n%{options}") public final class CanonicalizeCommand implements BlazeCommand { public static class Options extends OptionsBase { diff --git a/src/main/protobuf/BUILD b/src/main/protobuf/BUILD index 3551f24db1a1c0..177d778bde7322 100644 --- a/src/main/protobuf/BUILD +++ b/src/main/protobuf/BUILD @@ -18,9 +18,9 @@ FILES = [ "desugar_deps", "execution_statistics", "extra_actions_base", - "invocation_policy", "java_compilation", "memory_pressure", + "strategy_policy", "test_status", "worker_protocol", "execution_graph", @@ -107,6 +107,22 @@ java_library_srcs( deps = [":failure_details_java_proto"], ) +proto_library( + name = "invocation_policy_proto", + srcs = ["invocation_policy.proto"], + deps = [":strategy_policy_proto"], +) + +java_proto_library( + name = "invocation_policy_java_proto", + deps = [":invocation_policy_proto"], +) + +java_library_srcs( + name = "invocation_policy_java_proto_srcs", + deps = [":invocation_policy_java_proto"], +) + proto_library( name = "option_filters_proto", srcs = ["option_filters.proto"], @@ -384,6 +400,7 @@ filegroup( ":command_server_java_grpc_srcs", ":command_server_java_proto_srcs", ":failure_details_java_proto_srcs", + ":invocation_policy_java_proto_srcs", ":option_filters_java_proto_srcs", ":profile_java_proto_srcs", ":remote_execution_log_java_proto_srcs", diff --git a/src/main/protobuf/invocation_policy.proto b/src/main/protobuf/invocation_policy.proto index f54a0f5fcba7c8..56e3cc0c2d58a2 100644 --- a/src/main/protobuf/invocation_policy.proto +++ b/src/main/protobuf/invocation_policy.proto @@ -13,8 +13,11 @@ // limitations under the License. syntax = "proto2"; + package blaze.invocation_policy; +import "src/main/protobuf/strategy_policy.proto"; + // option java_api_version = 2; option java_package = "com.google.devtools.build.lib.runtime.proto"; @@ -26,6 +29,8 @@ message InvocationPolicy { // requirements, only the final policy on a specific flag will be enforced // onto the user's command line. repeated FlagPolicy flag_policies = 1; + + optional blaze.strategy_policy.StrategyPolicy strategy_policy = 2; } // A policy for controlling the value of a flag. diff --git a/src/main/protobuf/strategy_policy.proto b/src/main/protobuf/strategy_policy.proto new file mode 100644 index 00000000000000..0f58c9b81f359d --- /dev/null +++ b/src/main/protobuf/strategy_policy.proto @@ -0,0 +1,67 @@ +// Copyright 2022 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 = "proto2"; + +package blaze.strategy_policy; + +option java_multiple_files = true; +// option java_api_version = 2; +option java_package = "com.google.devtools.build.lib.runtime.proto"; + +// Provides control over what strategies (local, remote, etc) may be used. +// +// An empty policies (e.g. unset) implies no enforcement, anything is allowed. +// +// Policies are enforced against both user-provided values (flags) and +// application-internal defaults. The latter is useful for guarding against +// unexpectedly hard-coded defaults. +// +// Sample usage to allow everything to execute remotely, while only allowing +// genrules to execute locally: +// +// strategy_policy { +// mnemonic_policy { +// default_allowlist: ["remote"] +// strategy_allowlist: [ +// { mnemonic: "Genrule" strategy: ["local"] } +// ] +// } +// } +message StrategyPolicy { + // Controls per-mnemonic policies for regular spawn/action execution. Relevant + // command-line flags this controls include --strategy and --genrule_strategy. + optional MnemonicPolicy mnemonic_policy = 1; + + // Controls per-mnemonic policies for the remote execution leg of dynamic + // execution. Relevant flag is --dynamic_remote_strategy. + optional MnemonicPolicy dynamic_remote_policy = 2; + + // Controls per-mnemonic policies for the local execution leg of dynamic + // execution. Relevant flag is --dynamic_local_strategy. + optional MnemonicPolicy dynamic_local_policy = 3; +} + +message MnemonicPolicy { + // Default allowed strategies for mnemonics not present in `strategy` list. + repeated string default_allowlist = 1; + + repeated StrategiesForMnemonic strategy_allowlist = 2; +} + +// Per-mnemonic allowlist settings. +message StrategiesForMnemonic { + optional string mnemonic = 1; + repeated string strategy = 2; +} diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/util/BlazeRuntimeWrapper.java b/src/test/java/com/google/devtools/build/lib/buildtool/util/BlazeRuntimeWrapper.java index 5739f4b3455272..65f75376fd1ce5 100644 --- a/src/test/java/com/google/devtools/build/lib/buildtool/util/BlazeRuntimeWrapper.java +++ b/src/test/java/com/google/devtools/build/lib/buildtool/util/BlazeRuntimeWrapper.java @@ -196,6 +196,7 @@ public final CommandEnvironment newCommandWithExtensions( .initCommand( commandAnnotation, optionsParser, + InvocationPolicy.getDefaultInstance(), workspaceSetupWarnings, /* waitTimeInMs= */ 0L, /* commandStartTime= */ 0L, diff --git a/src/test/java/com/google/devtools/build/lib/exec/BUILD b/src/test/java/com/google/devtools/build/lib/exec/BUILD index 0535027ae2ec62..4aef94b89e4516 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/BUILD +++ b/src/test/java/com/google/devtools/build/lib/exec/BUILD @@ -52,6 +52,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/exec:spawn_log_context", "//src/main/java/com/google/devtools/build/lib/exec:spawn_log_context_utils", "//src/main/java/com/google/devtools/build/lib/exec:spawn_runner", + "//src/main/java/com/google/devtools/build/lib/exec:spawn_strategy_policy", "//src/main/java/com/google/devtools/build/lib/exec:spawn_strategy_registry", "//src/main/java/com/google/devtools/build/lib/exec:standalone_test_result", "//src/main/java/com/google/devtools/build/lib/exec:standalone_test_strategy", @@ -77,6 +78,7 @@ java_library( "//src/main/java/com/google/devtools/common/options", "//src/main/protobuf:failure_details_java_proto", "//src/main/protobuf:spawn_java_proto", + "//src/main/protobuf:strategy_policy_java_proto", "//src/main/protobuf:test_status_java_proto", "//src/test/java/com/google/devtools/build/lib/actions/util", "//src/test/java/com/google/devtools/build/lib/analysis/util", diff --git a/src/test/java/com/google/devtools/build/lib/exec/SpawnStrategyPolicyTest.java b/src/test/java/com/google/devtools/build/lib/exec/SpawnStrategyPolicyTest.java new file mode 100644 index 00000000000000..dc2c117f11cf8b --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/exec/SpawnStrategyPolicyTest.java @@ -0,0 +1,94 @@ +// Copyright 2022 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.exec; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.common.collect.ImmutableList; +import com.google.devtools.build.lib.runtime.proto.MnemonicPolicy; +import com.google.devtools.build.lib.runtime.proto.StrategiesForMnemonic; +import java.util.List; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class SpawnStrategyPolicyTest { + + @Test + public void applyEmptyPolicyListAllowsEverything() { + SpawnStrategyPolicy underTest = SpawnStrategyPolicy.create(MnemonicPolicy.getDefaultInstance()); + + ImmutableList strategies = ImmutableList.of("foo", "bar"); + assertThat(underTest.apply("mnemonic1", strategies)) + .containsExactlyElementsIn(strategies) + .inOrder(); + } + + @Test + public void applyNonOverriddenMnemonicUsesDefaultAllowList() { + SpawnStrategyPolicy underTest = + SpawnStrategyPolicy.create( + mnemonicPolicy( + ImmutableList.of(strategiesForMnemonic("mnemonic1", "baz")), + ImmutableList.of("foo", "bar"))); + + assertThat(underTest.apply("not-mnemonic1", ImmutableList.of("foo", "bar", "baz"))) + .containsExactly("foo", "bar") + .inOrder(); + } + + @Test + public void applyPerStrategyAllowListUsedToFilterStrategies() { + SpawnStrategyPolicy underTest = + SpawnStrategyPolicy.create( + mnemonicPolicy( + ImmutableList.of(strategiesForMnemonic("mnemonic1", "baz")), + ImmutableList.of("foo", "bar"))); + + assertThat(underTest.apply("mnemonic1", ImmutableList.of("foo", "bar", "baz"))) + .containsExactly("baz"); + } + + @Test + public void applyPerStrategyAllowListLastListPerMnemonicWins() { + SpawnStrategyPolicy underTest = + SpawnStrategyPolicy.create( + mnemonicPolicy( + ImmutableList.of( + strategiesForMnemonic("mnemonic1", "bar"), + strategiesForMnemonic("mnemonic1", "foo", "bar")), + ImmutableList.of("boom"))); + + assertThat(underTest.apply("mnemonic1", ImmutableList.of("foo", "bar", "baz"))) + .containsExactly("foo", "bar") + .inOrder(); + } + + private static MnemonicPolicy mnemonicPolicy( + List strategyAllowList, List defaultAllowlist) { + return MnemonicPolicy.newBuilder() + .addAllStrategyAllowlist(strategyAllowList) + .addAllDefaultAllowlist(defaultAllowlist) + .build(); + } + + private static StrategiesForMnemonic strategiesForMnemonic( + String mnemonic, String... strategies) { + return StrategiesForMnemonic.newBuilder() + .setMnemonic(mnemonic) + .addAllStrategy(ImmutableList.copyOf(strategies)) + .build(); + } +} diff --git a/src/test/java/com/google/devtools/build/lib/exec/SpawnStrategyRegistryTest.java b/src/test/java/com/google/devtools/build/lib/exec/SpawnStrategyRegistryTest.java index 6b081a5d4475f2..c7fa9c94e32865 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/SpawnStrategyRegistryTest.java +++ b/src/test/java/com/google/devtools/build/lib/exec/SpawnStrategyRegistryTest.java @@ -32,6 +32,8 @@ import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.exec.util.FakeOwner; +import com.google.devtools.build.lib.runtime.proto.MnemonicPolicy; +import com.google.devtools.build.lib.runtime.proto.StrategyPolicy; import com.google.devtools.build.lib.util.AbruptExitException; import com.google.devtools.build.lib.util.RegexFilter; import javax.annotation.Nullable; @@ -84,6 +86,29 @@ public void testMnemonicFilter() throws Exception { .containsExactly(strategy2, strategy1); } + @Test + public void testStrategyPolicyAppliedToPerMnemonicStrategies() throws Exception { + NoopStrategy strategy1 = new NoopStrategy("1"); + NoopStrategy strategy2 = new NoopStrategy("2"); + StrategyPolicy strategyPolicyProto = + StrategyPolicy.newBuilder() + .setMnemonicPolicy(MnemonicPolicy.newBuilder().addDefaultAllowlist("foo")) + .build(); + + SpawnStrategyRegistry strategyRegistry = + SpawnStrategyRegistry.builder(strategyPolicyProto) + .registerStrategy(strategy1, "foo") + .registerStrategy(strategy2, "bar") + .addMnemonicFilter("some-mnemonic", ImmutableList.of("bar", "foo")) + .build(); + + assertThat( + strategyRegistry.getStrategies( + createSpawnWithMnemonicAndDescription("some-mnemonic", ""), + SpawnStrategyRegistryTest::noopEventHandler)) + .containsExactly(strategy1); + } + @Test public void testLaterStrategyOverridesEarlier() throws Exception { NoopStrategy strategy1 = new NoopStrategy("1"); @@ -417,6 +442,35 @@ public void testDynamicStrategyNotSandboxed() { assertThat(exception).hasMessageThat().containsMatch("sandboxed strategy"); } + @Test + public void testDynamicStrategiesHonorStrategyPolicy() throws Exception { + NoopStrategy remoteStrategy = new NoopSandboxedStrategy("remote"); + NoopStrategy localStrategy = new NoopSandboxedStrategy("local"); + SpawnStrategyRegistry strategyRegistry = + SpawnStrategyRegistry.builder( + StrategyPolicy.newBuilder() + .setDynamicRemotePolicy( + MnemonicPolicy.newBuilder().addDefaultAllowlist("remote")) + .setDynamicLocalPolicy(MnemonicPolicy.newBuilder().addDefaultAllowlist("local")) + .build()) + .registerStrategy(remoteStrategy, "remote") + .registerStrategy(localStrategy, "local") + // Pointlessly register both strategies in order to test that policy filters them. + .addDynamicLocalStrategies(ImmutableMap.of("mnem", ImmutableList.of("remote", "local"))) + .addDynamicRemoteStrategies( + ImmutableMap.of("mnem", ImmutableList.of("remote", "local"))) + .build(); + + assertThat( + strategyRegistry.getDynamicSpawnActionContexts( + createSpawnWithMnemonicAndDescription("mnem", ""), DynamicMode.REMOTE)) + .containsExactly(remoteStrategy); + assertThat( + strategyRegistry.getDynamicSpawnActionContexts( + createSpawnWithMnemonicAndDescription("mnem", ""), DynamicMode.LOCAL)) + .containsExactly(localStrategy); + } + @Test public void testRemoteLocalFallback() throws Exception { NoopAbstractStrategy strategy1 = new NoopAbstractStrategy("1"); 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 64298e9b82b6da..108943e795e84e 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/BUILD +++ b/src/test/java/com/google/devtools/build/lib/remote/BUILD @@ -129,6 +129,7 @@ java_library( "//src/main/java/com/google/devtools/common/options", "//src/main/protobuf:cache_salt_java_proto", "//src/main/protobuf:failure_details_java_proto", + "//src/main/protobuf:invocation_policy_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", diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteModuleTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteModuleTest.java index f3edf8c549b447..f35af974ec3208 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteModuleTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteModuleTest.java @@ -56,6 +56,7 @@ import com.google.devtools.build.lib.runtime.CommandLinePathFactory; import com.google.devtools.build.lib.runtime.CommonCommandOptions; import com.google.devtools.build.lib.runtime.commands.BuildCommand; +import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.InvocationPolicy; import com.google.devtools.build.lib.testutil.Scratch; import com.google.devtools.build.lib.util.AbruptExitException; import com.google.devtools.build.lib.vfs.DigestHashFunction; @@ -172,6 +173,7 @@ private static CommandEnvironment createTestCommandEnvironment( return workspace.initCommand( command, options, + InvocationPolicy.getDefaultInstance(), /* warnings= */ new ArrayList<>(), /* waitTimeInMs= */ 0, /* commandStartTime= */ 0, diff --git a/src/test/java/com/google/devtools/build/lib/runtime/BlazeRuntimeTest.java b/src/test/java/com/google/devtools/build/lib/runtime/BlazeRuntimeTest.java index 8ddd3429819d67..26fc16bdc7ef70 100644 --- a/src/test/java/com/google/devtools/build/lib/runtime/BlazeRuntimeTest.java +++ b/src/test/java/com/google/devtools/build/lib/runtime/BlazeRuntimeTest.java @@ -23,6 +23,7 @@ import com.google.devtools.build.lib.analysis.ServerDirectories; import com.google.devtools.build.lib.exec.BinTools; import com.google.devtools.build.lib.runtime.commands.VersionCommand; +import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.InvocationPolicy; import com.google.devtools.build.lib.server.FailureDetails.Crash; import com.google.devtools.build.lib.server.FailureDetails.Crash.Code; import com.google.devtools.build.lib.server.FailureDetails.FailureDetail; @@ -105,6 +106,7 @@ public void crashTest() throws Exception { commandThread, VersionCommand.class.getAnnotation(Command.class), options, + InvocationPolicy.getDefaultInstance(), SyscallCache.NO_CACHE, QuiescingExecutorsImpl.forTesting(), /* warnings= */ ImmutableList.of(), @@ -161,6 +163,7 @@ public void resultExtensions() throws Exception { Thread.currentThread(), VersionCommand.class.getAnnotation(Command.class), OptionsParser.builder().optionsClasses(COMMAND_ENV_REQUIRED_OPTIONS).build(), + InvocationPolicy.getDefaultInstance(), SyscallCache.NO_CACHE, QuiescingExecutorsImpl.forTesting(), /* warnings= */ ImmutableList.of(),