[remote/downloader] Add incompatible flag for sending a list of heade…

…rs as qualifier

Closes #16312.

PiperOrigin-RevId: 477770765
Change-Id: I31a2eac5b93c184dee7580a058b9b9fb10ddd9ff

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

@@ -38,6 +38,7 @@
 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 io.grpc.CallCredentials;
 import io.grpc.Channel;
@@ -129,7 +130,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 +176,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 +198,7 @@ static FetchBlobRequest newFetchBlobRequest(
       requestBuilder.addQualifiers(
           Qualifier.newBuilder()
               .setName(QUALIFIER_AUTH_HEADERS)
-              .setValue(authHeadersJson(authHeaders))
+              .setValue(authHeadersJson(authHeaders, includeAllHeaders))
               .build());
     }
 
@@ -216,16 +224,24 @@ 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

@@ -620,6 +620,17 @@ 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,17 @@ 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 +396,60 @@ 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());
+  }
 }