[remote/downloader] Add incompatible flag for sending a list of headers as qualifier

src/main/java/com/google/devtools/build/lib/remote/downloader/GrpcRemoteDownloader.java

@@ -38,7 +38,9 @@
 import com.google.devtools.build.lib.remote.util.Utils;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.gson.Gson;
+import com.google.gson.JsonArray;
 import com.google.gson.JsonObject;
+import com.google.gson.JsonPrimitive;
 import io.grpc.CallCredentials;
 import io.grpc.Channel;
 import io.grpc.StatusRuntimeException;
@@ -129,7 +131,13 @@ public void download(
         RemoteActionExecutionContext.create(metadata);
 
     final FetchBlobRequest request =
-        newFetchBlobRequest(options.remoteInstanceName, urls, authHeaders, checksum, canonicalId);
+        newFetchBlobRequest(
+                options.remoteInstanceName,
+                urls,
+                authHeaders,
+                checksum,
+                canonicalId,
+                options.remoteDownloaderSendAllHeaders);
     try {
       FetchBlobResponse response =
           retrier.execute(
@@ -169,7 +177,8 @@ static FetchBlobRequest newFetchBlobRequest(
       List<URL> urls,
       Map<URI, Map<String, List<String>>> authHeaders,
       com.google.common.base.Optional<Checksum> checksum,
-      String canonicalId) {
+      String canonicalId,
+      boolean includeAllHeaders) {
     FetchBlobRequest.Builder requestBuilder =
         FetchBlobRequest.newBuilder().setInstanceName(instanceName);
     for (URL url : urls) {
@@ -190,7 +199,7 @@ static FetchBlobRequest newFetchBlobRequest(
       requestBuilder.addQualifiers(
           Qualifier.newBuilder()
               .setName(QUALIFIER_AUTH_HEADERS)
-              .setValue(authHeadersJson(authHeaders))
+              .setValue(authHeadersJson(authHeaders, includeAllHeaders))
               .build());
     }
 
@@ -216,16 +225,23 @@ private OutputStream newOutputStream(
     return out;
   }
 
-  private static String authHeadersJson(Map<URI, Map<String, List<String>>> authHeaders) {
+  private static String authHeadersJson(Map<URI, Map<String, List<String>>> authHeaders, boolean includeAllHeaders) {
     Map<String, JsonObject> subObjects = new TreeMap<>();
     for (Map.Entry<URI, Map<String, List<String>>> entry : authHeaders.entrySet()) {
       JsonObject subObject = new JsonObject();
       Map<String, List<String>> orderedHeaders = new TreeMap<>(entry.getValue());
       for (Map.Entry<String, List<String>> subEntry : orderedHeaders.entrySet()) {
-        // TODO(yannic): Introduce incompatible flag for including all headers, not just the first.
-        String value = Iterables.getFirst(subEntry.getValue(), null);
-        if (value != null) {
-          subObject.addProperty(subEntry.getKey(), value);
+        if (includeAllHeaders) {
+          JsonArray values = new JsonArray(subEntry.getValue().size());
+          for (String value : subEntry.getValue()) {
+            values.add(value);
+          }
+          subObject.add(subEntry.getKey(), values);
+        } else {
+          String value = Iterables.getFirst(subEntry.getValue(), null);
+          if (value != null) {
+            subObject.addProperty(subEntry.getKey(), value);
+          }
         }
       }
       subObjects.put(entry.getKey().toString(), subObject);

src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java

@@ -608,6 +608,15 @@ public RemoteOutputsStrategyConverter() {
               + "`all` to print always.")
   public ExecutionMessagePrintMode remotePrintExecutionMessages;
 
+  @Option(
+          name = "incompatible_remote_downloader_send_all_headers",
+          defaultValue = "false",
+          documentationCategory = OptionDocumentationCategory.REMOTE,
+          effectTags = {OptionEffectTag.UNKNOWN},
+          metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE},
+          help = "Whether to send all values of a multi-valued header to the remote downloader instead of just the first.")
+  public boolean remoteDownloaderSendAllHeaders;
+
   // The below options are not configurable by users, only tests.
   // This is part of the effort to reduce the overall number of flags.
 

src/test/java/com/google/devtools/build/lib/remote/downloader/GrpcRemoteDownloaderTest.java

@@ -354,13 +354,14 @@ public void testFetchBlobRequest() throws Exception {
                 new URI("http://example.com"),
                 ImmutableMap.of(
                     "Some-Header", ImmutableList.of("some header content"),
-                    "Another-Header", ImmutableList.of("another header content")),
+                    "Another-Header", ImmutableList.of("another header content", "even more header content")),
                 new URI("http://example.org"),
-                ImmutableMap.of("Org-Header", ImmutableList.of("org header content"))),
+                ImmutableMap.of("Org-Header", ImmutableList.of("org header content", "and a second one", "and a third one"))),
             com.google.common.base.Optional.<Checksum>of(
                 Checksum.fromSubresourceIntegrity(
                     "sha256-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=")),
-            "canonical ID");
+            "canonical ID",
+                /* includeAllHeaders= */ false);
 
     final String expectedAuthHeadersJson =
         "{"
@@ -392,4 +393,57 @@ public void testFetchBlobRequest() throws Exception {
                         .setValue(expectedAuthHeadersJson))
                 .build());
   }
+
+  @Test
+  public void testFetchBlobRequestWithAllHeaders() throws Exception {
+    FetchBlobRequest request =
+            GrpcRemoteDownloader.newFetchBlobRequest(
+                    "instance name",
+                    ImmutableList.of(
+                            new URL("http://example.com/a"),
+                            new URL("http://example.com/b"),
+                            new URL("file:/not/limited/to/http")),
+                    ImmutableMap.of(
+                            new URI("http://example.com"),
+                            ImmutableMap.of(
+                                    "Some-Header", ImmutableList.of("some header content"),
+                                    "Another-Header", ImmutableList.of("another header content", "even more header content")),
+                            new URI("http://example.org"),
+                            ImmutableMap.of("Org-Header", ImmutableList.of("org header content", "and a second one", "and a third one"))),
+                    com.google.common.base.Optional.<Checksum>of(
+                            Checksum.fromSubresourceIntegrity(
+                                    "sha256-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=")),
+                    "canonical ID",
+                    /* includeAllHeaders= */ true);
+
+    final String expectedAuthHeadersJson =
+            "{"
+                    + "\"http://example.com\":{"
+                    + "\"Another-Header\":[\"another header content\",\"even more header content\"],"
+                    + "\"Some-Header\":[\"some header content\"]"
+                    + "},"
+                    + "\"http://example.org\":{"
+                    + "\"Org-Header\":[\"org header content\",\"and a second one\",\"and a third one\"]"
+                    + "}"
+                    + "}";
+
+    assertThat(request)
+            .isEqualTo(
+                    FetchBlobRequest.newBuilder()
+                            .setInstanceName("instance name")
+                            .addUris("http://example.com/a")
+                            .addUris("http://example.com/b")
+                            .addUris("file:/not/limited/to/http")
+                            .addQualifiers(
+                                    Qualifier.newBuilder()
+                                            .setName("checksum.sri")
+                                            .setValue("sha256-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA="))
+                            .addQualifiers(
+                                    Qualifier.newBuilder().setName("bazel.canonical_id").setValue("canonical ID"))
+                            .addQualifiers(
+                                    Qualifier.newBuilder()
+                                            .setName("bazel.auth_headers")
+                                            .setValue(expectedAuthHeadersJson))
+                            .build());
+  }
 }