Put Action messages into the CAS.

Experience has shown that having access to a copy of the Action message
available makes debugging much easier, as not doing so risks the client
simply throwing it away. It also results in duplication, with the same
or similar Actions being sent repeatedly, wasting bandwidth.

Putting the Action messages into the CAS ensure that they will remain
accessible, and additionally allows servers to perform access control on
UpdateActionResult based on the contents of an Action (for instance, to
prevent an individual user from poisoning actions that might be executed
by a continuous integration system).

1 files changed, 25 insertions, 17 deletions

diff --git a/build/bazel/remote/execution/v2/remote_execution.proto b/build/bazel/remote/execution/v2/remote_execution.proto
index 5d75113..3aa6d1d 100644
--- a/build/bazel/remote/execution/v2/remote_execution.proto
+++ b/build/bazel/remote/execution/v2/remote_execution.proto
@@ -42,12 +42,12 @@ service Execution {
   // Execute an action remotely.
   //
   // In order to execute an action, the client must first upload all of the
-  // inputs, as well as the
-  // [Command][build.bazel.remote.execution.v2.Command] to run, into the
+  // inputs, the
+  // [Command][build.bazel.remote.execution.v2.Command] to run, and the
+  // [Action][build.bazel.remote.execution.v2.Action] into the
   // [ContentAddressableStorage][build.bazel.remote.execution.v2.ContentAddressableStorage].
-  // It then calls `Execute` with an
-  // [Action][build.bazel.remote.execution.v2.Action] referring to them.
-  // The server will run the action and eventually return the result.
+  // It then calls `Execute` with an `action_digest` referring to them. The
+  // server will run the action and eventually return the result.
   //
   // The input `Action`'s fields MUST meet the various canonicalization
   // requirements specified in the documentation for their types so that it has
@@ -148,6 +148,13 @@ service ActionCache {
   // [Execution][build.bazel.remote.execution.v2.Execution] API. As a
   // result, it is OPTIONAL for servers to implement.
   //
+  // In order to allow the server to perform access control based on the type of
+  // action, and to assist with client debugging, the client MUST first upload
+  // the [Action][build.bazel.remote.execution.v2.Execution] that produced the
+  // result, along with its
+  // [Command][build.bazel.remote.execution.v2.Command], into the
+  // `ContentAddressableStorage`.
+  //
   // Errors:
   // * `NOT_IMPLEMENTED`: This method is not supported by the server.
   // * `RESOURCE_EXHAUSTED`: There is insufficient storage space to add the
@@ -819,12 +826,12 @@ message OutputDirectory {
 message ExecutionPolicy {
   // The priority (relative importance) of this action. Generally, a lower value
   // means that the action should be run sooner than actions having a greater
-  // priority value, but the interpretation of a given value is server-dependent.
-  // A priority of 0 means the *default* priority. Priorities may be positive or
-  // negative, and such actions should run later or sooner than actions having
-  // the default priority, respectively. The particular semantics of this field
-  // is up to the server. In particular, every server will have their own
-  // supported range of priorities, and will decide how these map into
+  // priority value, but the interpretation of a given value is server-
+  // dependent. A priority of 0 means the *default* priority. Priorities may be
+  // positive or negative, and such actions should run later or sooner than
+  // actions having the default priority, respectively. The particular semantics
+  // of this field is up to the server. In particular, every server will have
+  // their own supported range of priorities, and will decide how these map into
   // scheduling policy.
   int32 priority = 1;
 }
@@ -853,24 +860,25 @@ message ExecuteRequest {
   // omitted.
   string instance_name = 1;
 
-  // The action to be performed.
-  Action action = 2;
-
   // If true, the action will be executed anew even if its result was already
   // present in the cache. If false, the result may be served from the
   // [ActionCache][build.bazel.remote.execution.v2.ActionCache].
   bool skip_cache_lookup = 3;
 
-  reserved 4, 5; // Used for removed fields in an earlier version of the API.
+  reserved 2, 4, 5; // Used for removed fields in an earlier version of the API.
+
+  // The digest of the [Action][build.bazel.remote.execution.v2.Action] to
+  // execute.
+  Digest action_digest = 6;
 
   // An optional policy for execution of the action.
   // The server will have a default policy if this is not provided.
-  ExecutionPolicy execution_policy = 6;
+  ExecutionPolicy execution_policy = 7;
 
   // An optional policy for the results of this execution in the remote cache.
   // The server will have a default policy if this is not provided.
   // This may be applied to both the ActionResult and the associated blobs.
-  ResultsCachePolicy results_cache_policy = 7;
+  ResultsCachePolicy results_cache_policy = 8;
 }
 
 // A `LogFile` is a log stored in the CAS.