Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GrpcRemoteDownloader: optionally propagate credentials to remote server #23578

Closed
wants to merge 1 commit into from
Closed

GrpcRemoteDownloader: optionally propagate credentials to remote server #23578

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 37 additions & 3 deletions src/main/java/com/google/devtools/build/lib/remote/downloader/GrpcRemoteDownloader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import io.grpc.StatusRuntimeException;
import java.io.IOException;
import java.io.OutputStream;
import java.net.URISyntaxException;
import java.net.URL;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -83,6 +84,9 @@ public class GrpcRemoteDownloader implements AutoCloseable, Downloader {
// delimit the qualifier prefix which denotes an HTTP header qualifer from the
// header name itself.
private static final String QUALIFIER_HTTP_HEADER_PREFIX = "http_header:";
// Same as HTTP_HEADER_PREFIX, but only apply for a specific URL.
// The index starts from 0 and corresponds to the URL index in the request.
private static final String QUALIFIER_HTTP_HEADER_URL_PREFIX = "http_header_url:";

public GrpcRemoteDownloader(
String buildRequestId,
Expand Down Expand Up @@ -135,7 +139,14 @@ public void download(

final FetchBlobRequest request =
newFetchBlobRequest(
options.remoteInstanceName, urls, checksum, canonicalId, digestFunction, headers);
options.remoteInstanceName,
options.remoteDownloaderPropagateCredentials,
urls,
checksum,
canonicalId,
digestFunction,
headers,
credentials);
try {
FetchBlobResponse response =
retrier.execute(
Expand Down Expand Up @@ -180,17 +191,40 @@ public void download(
@VisibleForTesting
static FetchBlobRequest newFetchBlobRequest(
String instanceName,
boolean remoteDownloaderPropagateCredentials,
List<URL> urls,
Optional<Checksum> checksum,
String canonicalId,
DigestFunction.Value digestFunction,
Map<String, List<String>> headers) {
Map<String, List<String>> headers,
Credentials credentials)
throws IOException {
FetchBlobRequest.Builder requestBuilder =
FetchBlobRequest.newBuilder()
.setInstanceName(instanceName)
.setDigestFunction(digestFunction);
for (URL url : urls) {
for (int i = 0; i < urls.size(); i++) {
var url = urls.get(i);
requestBuilder.addUris(url.toString());

if (!remoteDownloaderPropagateCredentials) {
continue;
}

try {
var metadata = credentials.getRequestMetadata(url.toURI());
for (var entry : metadata.entrySet()) {
for (var value : entry.getValue()) {
requestBuilder.addQualifiers(
Qualifier.newBuilder()
.setName(QUALIFIER_HTTP_HEADER_URL_PREFIX + i + ":" + entry.getKey())
.setValue(value)
.build());
}
}
} catch (URISyntaxException e) {
throw new IOException(e);
}
}

if (checksum.isPresent()) {
Expand Down
12 changes: 12 additions & 0 deletions src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,18 @@ public final class RemoteOptions extends CommonRemoteOptions {
help = "Whether to fall back to the local downloader if remote downloader fails.")
public boolean remoteDownloaderLocalFallback;

@Option(
name = "experimental_remote_downloader_propagate_credentials",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.REMOTE,
effectTags = {OptionEffectTag.UNKNOWN},
help =
"Whether to propagate credentials from netrc and credential helper to the remote"
+ " downloader server. The server implementation needs to support the new"
+ " `http_header_url:<url-index>:<header-key>` qualifier where the `<url-index>` is a"
+ " 0-based position of the URL inside the FetchBlobRequest's `uris` field.")
public boolean remoteDownloaderPropagateCredentials;

@Option(
name = "remote_header",
converter = Converters.AssignmentConverter.class,
Expand Down
1 change: 1 addition & 0 deletions src/test/java/com/google/devtools/build/lib/remote/downloader/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ java_library(
"//src/test/java/com/google/devtools/build/lib/remote/util",
"//src/test/java/com/google/devtools/build/lib/testutil",
"//src/test/java/com/google/devtools/build/lib/testutil:TestUtils",
"//third_party:auth",
"//third_party:guava",
"//third_party:jsr305",
"//third_party:junit4",
Expand Down
90 changes: 89 additions & 1 deletion src/test/java/com/google/devtools/build/lib/remote/downloader/GrpcRemoteDownloaderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,12 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.devtools.build.lib.remote.util.Utils.getFromFuture;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.Collections.singletonList;
import static org.junit.Assert.assertThrows;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

import build.bazel.remote.asset.v1.FetchBlobRequest;
import build.bazel.remote.asset.v1.FetchBlobResponse;
Expand All @@ -29,6 +31,7 @@
import build.bazel.remote.execution.v2.Digest;
import build.bazel.remote.execution.v2.RequestMetadata;
import build.bazel.remote.execution.v2.ServerCapabilities;
import com.google.auth.Credentials;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.io.ByteStreams;
Expand Down Expand Up @@ -69,6 +72,7 @@
import java.io.IOException;
import java.io.InputStream;
import java.net.URL;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
Expand Down Expand Up @@ -348,6 +352,7 @@ public void testFetchBlobRequest() throws Exception {
FetchBlobRequest request =
GrpcRemoteDownloader.newFetchBlobRequest(
"instance name",
false,
ImmutableList.of(
new URL("http://example.com/a"),
new URL("http://example.com/b"),
Expand All @@ -359,7 +364,8 @@ public void testFetchBlobRequest() throws Exception {
DIGEST_UTIL.getDigestFunction(),
ImmutableMap.of(
"Authorization", ImmutableList.of("Basic Zm9vOmJhcg=="),
"X-Custom-Token", ImmutableList.of("foo", "bar")));
"X-Custom-Token", ImmutableList.of("foo", "bar")),
StaticCredentials.EMPTY);

assertThat(request)
.isEqualTo(
Expand All @@ -385,4 +391,86 @@ public void testFetchBlobRequest() throws Exception {
.setValue("foo,bar"))
.build());
}

@Test
public void testFetchBlobRequest_withCredentialsPropagation() throws Exception {
var shouldPropagateCredentials = true;
var url = new URL("http://example.com/a");

Credentials credentials = mock(Credentials.class);
when(credentials.hasRequestMetadata()).thenReturn(true);
Map<String, List<String>> headers = new HashMap<>();
headers.put("CredKey", singletonList("CredValue"));
when(credentials.getRequestMetadata(url.toURI())).thenReturn(headers);

FetchBlobRequest request =
GrpcRemoteDownloader.newFetchBlobRequest(
"instance name",
shouldPropagateCredentials,
ImmutableList.of(url),
Optional.<Checksum>of(
Checksum.fromSubresourceIntegrity(
"sha256-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=")),
"canonical ID",
DIGEST_UTIL.getDigestFunction(),
ImmutableMap.of(),
credentials);

assertThat(request)
.isEqualTo(
FetchBlobRequest.newBuilder()
.setInstanceName("instance name")
.setDigestFunction(DIGEST_UTIL.getDigestFunction())
.addUris("http://example.com/a")
.addQualifiers(
Qualifier.newBuilder()
.setName("http_header_url:0:CredKey")
.setValue("CredValue"))
.addQualifiers(
Qualifier.newBuilder()
.setName("checksum.sri")
.setValue("sha256-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA="))
.addQualifiers(
Qualifier.newBuilder().setName("bazel.canonical_id").setValue("canonical ID"))
.build());
}

@Test
public void testFetchBlobRequest_withoutCredentialsPropagation() throws Exception {
var shouldPropagateCredentials = false;
var url = new URL("http://example.com/a");

Credentials credentials = mock(Credentials.class);
when(credentials.hasRequestMetadata()).thenReturn(true);
Map<String, List<String>> headers = new HashMap<>();
headers.put("CredKey", singletonList("CredValue"));
when(credentials.getRequestMetadata(url.toURI())).thenReturn(headers);

FetchBlobRequest request =
GrpcRemoteDownloader.newFetchBlobRequest(
"instance name",
shouldPropagateCredentials,
ImmutableList.of(url),
Optional.<Checksum>of(
Checksum.fromSubresourceIntegrity(
"sha256-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=")),
"canonical ID",
DIGEST_UTIL.getDigestFunction(),
ImmutableMap.of(),
credentials);

assertThat(request)
.isEqualTo(
FetchBlobRequest.newBuilder()
.setInstanceName("instance name")
.setDigestFunction(DIGEST_UTIL.getDigestFunction())
.addUris("http://example.com/a")
.addQualifiers(
Qualifier.newBuilder()
.setName("checksum.sri")
.setValue("sha256-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA="))
.addQualifiers(
Qualifier.newBuilder().setName("bazel.canonical_id").setValue("canonical ID"))
.build());
}
}
Loading