Skip to content

Commit 2756319

Browse files
YannicYannic
committed
[remote/downloader] Migrate Downloader to take Credentials
Progress on #15856
1 parent 6c1e4b7 commit 2756319

File tree

16 files changed

+121
-38
lines changed

16 files changed

+121
-38
lines changed

‎src/main/java/com/google/devtools/build/lib/authandtls/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ filegroup(
66
name = "srcs",
77
srcs = glob(["**"]) + [
88
"//src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper:srcs",
9+
"//src/main/java/com/google/devtools/build/lib/authandtls/staticcredentials:srcs",
910
],
1011
visibility = ["//src:__subpackages__"],
1112
)

‎src/main/java/com/google/devtools/build/lib/authandtls/staticcredentials/BUILD

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
load("@rules_java//java:defs.bzl", "java_library")
2+
3+
package(default_visibility = ["//src:__subpackages__"])
4+
5+
filegroup(
6+
name = "srcs",
7+
srcs = glob(["**"]),
8+
visibility = ["//src:__subpackages__"],
9+
)
10+
11+
java_library(
12+
name = "staticcredentials",
13+
srcs = glob(["*.java"]),
14+
deps = [
15+
"//third_party:auth",
16+
"//third_party:guava",
17+
],
18+
)

‎src/main/java/com/google/devtools/build/lib/authandtls/staticcredentials/StaticCredentials.java

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
package com.google.devtools.build.lib.authandtls.staticcredentials;
2+
3+
import com.google.auth.Credentials;
4+
import com.google.common.base.Preconditions;
5+
import com.google.common.collect.ImmutableMap;
6+
import java.io.IOException;
7+
import java.net.URI;
8+
import java.util.List;
9+
import java.util.Map;
10+
11+
/** Implementation of {@link Credentials} which provides a static set of credentials. */
12+
public final class StaticCredentials extends Credentials {
13+
public static final StaticCredentials EMPTY = new StaticCredentials(ImmutableMap.of());
14+
15+
private final ImmutableMap<URI, Map<String, List<String>>> credentials;
16+
17+
public StaticCredentials(Map<URI, Map<String, List<String>>> credentials) {
18+
Preconditions.checkNotNull(credentials);
19+
20+
this.credentials = ImmutableMap.copyOf(credentials);
21+
}
22+
23+
@Override
24+
public String getAuthenticationType() {
25+
return "static";
26+
}
27+
28+
@Override
29+
public Map<String, List<String>> getRequestMetadata(URI uri) throws IOException {
30+
Preconditions.checkNotNull(uri);
31+
32+
return credentials.getOrDefault(uri, ImmutableMap.of());
33+
}
34+
35+
@Override
36+
public boolean hasRequestMetadata() {
37+
return true;
38+
}
39+
40+
@Override
41+
public boolean hasRequestMetadataOnly() {
42+
return true;
43+
}
44+
45+
@Override
46+
public void refresh() {
47+
// Can't refresh static credentials.
48+
}
49+
}

‎src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ java_library(
1414
deps = [
1515
"//src/main/java/com/google/devtools/build/lib/analysis:blaze_version_info",
1616
"//src/main/java/com/google/devtools/build/lib/authandtls",
17+
"//src/main/java/com/google/devtools/build/lib/authandtls/staticcredentials",
1718
"//src/main/java/com/google/devtools/build/lib/bazel/repository/cache",
1819
"//src/main/java/com/google/devtools/build/lib/bazel/repository/cache:events",
1920
"//src/main/java/com/google/devtools/build/lib/buildeventstream",

‎src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/DelegatingDownloader.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
package com.google.devtools.build.lib.bazel.repository.downloader;
1616

17+
import com.google.auth.Credentials;
1718
import com.google.common.base.Optional;
1819
import com.google.devtools.build.lib.events.ExtendedEventHandler;
1920
import com.google.devtools.build.lib.vfs.Path;
@@ -47,7 +48,7 @@ public void setDelegate(@Nullable Downloader delegate) {
4748
@Override
4849
public void download(
4950
List<URL> urls,
50-
Map<URI, Map<String, List<String>>> authHeaders,
51+
Credentials credentials,
5152
Optional<Checksum> checksum,
5253
String canonicalId,
5354
Path destination,
@@ -60,6 +61,6 @@ public void download(
6061
downloader = delegate;
6162
}
6263
downloader.download(
63-
urls, authHeaders, checksum, canonicalId, destination, eventHandler, clientEnv, type);
64+
urls, credentials, checksum, canonicalId, destination, eventHandler, clientEnv, type);
6465
}
6566
}

‎src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/DownloadManager.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import com.google.common.collect.ImmutableList;
2525
import com.google.common.collect.ImmutableMap;
2626
import com.google.common.collect.ImmutableSet;
27+
import com.google.devtools.build.lib.authandtls.staticcredentials.StaticCredentials;
2728
import com.google.devtools.build.lib.bazel.repository.cache.RepositoryCache;
2829
import com.google.devtools.build.lib.bazel.repository.cache.RepositoryCache.KeyType;
2930
import com.google.devtools.build.lib.bazel.repository.cache.RepositoryCacheHitEvent;
@@ -256,7 +257,7 @@ public Path download(
256257
try {
257258
downloader.download(
258259
rewrittenUrls,
259-
rewrittenAuthHeaders,
260+
new StaticCredentials(rewrittenAuthHeaders),
260261
checksum,
261262
canonicalId,
262263
destination,
@@ -337,7 +338,7 @@ public byte[] downloadAndReadOneUrl(
337338
for (int attempt = 0; attempt <= retries; ++attempt) {
338339
try {
339340
return httpDownloader.downloadAndReadOneUrl(
340-
rewrittenUrls.get(0), authHeaders, eventHandler, clientEnv);
341+
rewrittenUrls.get(0), new StaticCredentials(authHeaders), eventHandler, clientEnv);
341342
} catch (ContentLengthMismatchException e) {
342343
if (attempt == retries) {
343344
throw e;

‎src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/Downloader.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
package com.google.devtools.build.lib.bazel.repository.downloader;
1616

17+
import com.google.auth.Credentials;
1718
import com.google.common.base.Optional;
1819
import com.google.devtools.build.lib.events.ExtendedEventHandler;
1920
import com.google.devtools.build.lib.vfs.Path;
@@ -33,7 +34,7 @@ public interface Downloader {
3334
* caller is responsible for cleaning up outputs of failed downloads.
3435
*
3536
* @param urls list of mirror URLs with identical content
36-
* @param authHeaders map of authentication headers per URL
37+
* @param credentials credentials to use when connecting to URLs
3738
* @param checksum valid checksum which is checked, or absent to disable
3839
* @param output path to the destination file to write
3940
* @param type extension, e.g. "tar.gz" to force on downloaded filename, or empty to not do this
@@ -42,7 +43,7 @@ public interface Downloader {
4243
*/
4344
void download(
4445
List<URL> urls,
45-
Map<URI, Map<String, List<String>>> authHeaders,
46+
Credentials credentials,
4647
Optional<Checksum> checksum,
4748
String canonicalId,
4849
Path output,

‎src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpConnectorMultiplexer.java

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,15 @@
1414

1515
package com.google.devtools.build.lib.bazel.repository.downloader;
1616

17+
import com.google.auth.Credentials;
1718
import com.google.common.annotations.VisibleForTesting;
1819
import com.google.common.base.Function;
1920
import com.google.common.base.Optional;
2021
import com.google.common.base.Preconditions;
2122
import com.google.common.collect.ImmutableList;
2223
import com.google.common.collect.ImmutableMap;
2324
import com.google.devtools.build.lib.analysis.BlazeVersionInfo;
25+
import com.google.devtools.build.lib.authandtls.staticcredentials.StaticCredentials;
2426
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
2527
import com.google.devtools.build.lib.events.Event;
2628
import com.google.devtools.build.lib.events.EventHandler;
@@ -74,7 +76,7 @@ final class HttpConnectorMultiplexer {
7476
}
7577

7678
public HttpStream connect(URL url, Optional<Checksum> checksum) throws IOException {
77-
return connect(url, checksum, ImmutableMap.of(), Optional.absent());
79+
return connect(url, checksum, StaticCredentials.EMPTY, Optional.absent());
7880
}
7981

8082
/**
@@ -87,7 +89,7 @@ public HttpStream connect(URL url, Optional<Checksum> checksum) throws IOExcepti
8789
*
8890
* @param url the URL to conenct to. can be: file, http, or https
8991
* @param checksum checksum lazily checked on entire payload, or empty to disable
90-
* @param authHeaders the authentication headers
92+
* @param credentials the credentials
9193
* @param type extension, e.g. "tar.gz" to force on downloaded filename, or empty to not do this
9294
* @return an {@link InputStream} of response payload
9395
* @throws IOException if all mirrors are down and contains suppressed exception of each attempt
@@ -97,15 +99,15 @@ public HttpStream connect(URL url, Optional<Checksum> checksum) throws IOExcepti
9799
public HttpStream connect(
98100
URL url,
99101
Optional<Checksum> checksum,
100-
Map<URI, Map<String, List<String>>> authHeaders,
102+
Credentials credentials,
101103
Optional<String> type)
102104
throws IOException {
103105
Preconditions.checkArgument(HttpUtils.isUrlSupportedByDownloader(url));
104106
if (Thread.interrupted()) {
105107
throw new InterruptedIOException();
106108
}
107109
Function<URL, ImmutableMap<String, List<String>>> headerFunction =
108-
getHeaderFunction(REQUEST_HEADERS, authHeaders);
110+
getHeaderFunction(REQUEST_HEADERS, credentials);
109111
URLConnection connection = connector.connect(url, headerFunction);
110112
return httpStreamFactory.create(
111113
connection,
@@ -128,20 +130,20 @@ public HttpStream connect(
128130
@VisibleForTesting
129131
static Function<URL, ImmutableMap<String, List<String>>> getHeaderFunction(
130132
Map<String, List<String>> baseHeaders,
131-
Map<URI, Map<String, List<String>>> additionalHeaders) {
133+
Credentials credentials) {
134+
Preconditions.checkNotNull(baseHeaders);
135+
Preconditions.checkNotNull(credentials);
136+
132137
return url -> {
133-
ImmutableMap<String, List<String>> headers = ImmutableMap.copyOf(baseHeaders);
138+
Map<String, List<String>> headers = new HashMap<>(baseHeaders);
134139
try {
135-
if (additionalHeaders.containsKey(url.toURI())) {
136-
Map<String, List<String>> newHeaders = new HashMap<>(headers);
137-
newHeaders.putAll(additionalHeaders.get(url.toURI()));
138-
headers = ImmutableMap.copyOf(newHeaders);
139-
}
140-
} catch (URISyntaxException e) {
141-
// If we can't convert the URL to a URI (because it is syntactically malformed), still try
142-
// to do the connection, not adding authentication information as we cannot look it up.
140+
headers.putAll(credentials.getRequestMetadata(url.toURI()));
141+
} catch (URISyntaxException | IOException e) {
142+
// If we can't convert the URL to a URI (because it is syntactically malformed), or fetching
143+
// credentials fails for any other reason, still try to do the connection, not adding
144+
// authentication information as we cannot look it up.
143145
}
144-
return headers;
146+
return ImmutableMap.copyOf(headers);
145147
};
146148
}
147149
}

‎src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpDownloader.java

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
package com.google.devtools.build.lib.bazel.repository.downloader;
1616

17+
import com.google.auth.Credentials;
1718
import com.google.common.base.Optional;
1819
import com.google.common.collect.ImmutableList;
1920
import com.google.common.collect.Iterables;
@@ -63,7 +64,7 @@ public void setTimeoutScaling(float timeoutScaling) {
6364
@Override
6465
public void download(
6566
List<URL> urls,
66-
Map<URI, Map<String, List<String>>> authHeaders,
67+
Credentials credentials,
6768
Optional<Checksum> checksum,
6869
String canonicalId,
6970
Path destination,
@@ -82,8 +83,8 @@ public void download(
8283
for (URL url : urls) {
8384
SEMAPHORE.acquire();
8485

85-
try (HttpStream payload = multiplexer.connect(url, checksum, authHeaders, type);
86-
OutputStream out = destination.getOutputStream()) {
86+
try (HttpStream payload = multiplexer.connect(url, checksum, credentials, type);
87+
OutputStream out = destination.getOutputStream()) {
8788
try {
8889
ByteStreams.copy(payload, out);
8990
} catch (SocketTimeoutException e) {
@@ -132,7 +133,7 @@ public void download(
132133
/** Downloads the contents of one URL and reads it into a byte array. */
133134
public byte[] downloadAndReadOneUrl(
134135
URL url,
135-
Map<URI, Map<String, List<String>>> authHeaders,
136+
Credentials credentials,
136137
ExtendedEventHandler eventHandler,
137138
Map<String, String> clientEnv)
138139
throws IOException, InterruptedException {
@@ -141,7 +142,7 @@ public byte[] downloadAndReadOneUrl(
141142
ByteArrayOutputStream out = new ByteArrayOutputStream();
142143
SEMAPHORE.acquire();
143144
try (HttpStream payload =
144-
multiplexer.connect(url, Optional.absent(), authHeaders, Optional.absent())) {
145+
multiplexer.connect(url, Optional.absent(), credentials, Optional.absent())) {
145146
ByteStreams.copy(payload, out);
146147
} catch (SocketTimeoutException e) {
147148
// SocketTimeoutExceptions are InterruptedIOExceptions; however they do not signify

‎src/main/java/com/google/devtools/build/lib/remote/downloader/BUILD

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ java_library(
1414
name = "downloader",
1515
srcs = glob(["*.java"]),
1616
deps = [
17+
"//src/main/java/com/google/devtools/build/lib/authandtls/staticcredentials",
1718
"//src/main/java/com/google/devtools/build/lib/bazel/repository/downloader",
1819
"//src/main/java/com/google/devtools/build/lib/events",
1920
"//src/main/java/com/google/devtools/build/lib/remote:ReferenceCountedChannel",
@@ -22,6 +23,7 @@ java_library(
2223
"//src/main/java/com/google/devtools/build/lib/remote/options",
2324
"//src/main/java/com/google/devtools/build/lib/remote/util",
2425
"//src/main/java/com/google/devtools/build/lib/vfs",
26+
"//third_party:auth",
2527
"//third_party:guava",
2628
"//third_party:jsr305",
2729
"//third_party/grpc-java:grpc-jar",

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import build.bazel.remote.asset.v1.Qualifier;
2222
import build.bazel.remote.execution.v2.Digest;
2323
import build.bazel.remote.execution.v2.RequestMetadata;
24+
import com.google.auth.Credentials;
2425
import com.google.common.annotations.VisibleForTesting;
2526
import com.google.common.base.Strings;
2627
import com.google.devtools.build.lib.bazel.repository.downloader.Checksum;
@@ -41,7 +42,6 @@
4142
import io.grpc.StatusRuntimeException;
4243
import java.io.IOException;
4344
import java.io.OutputStream;
44-
import java.net.URI;
4545
import java.net.URL;
4646
import java.util.List;
4747
import java.util.Map;
@@ -110,7 +110,7 @@ public void close() {
110110
@Override
111111
public void download(
112112
List<URL> urls,
113-
Map<URI, Map<String, List<String>>> authHeaders,
113+
Credentials credentials,
114114
com.google.common.base.Optional<Checksum> checksum,
115115
String canonicalId,
116116
Path destination,
@@ -154,7 +154,7 @@ public void download(
154154
eventHandler.handle(
155155
Event.warn("Remote Cache: " + Utils.grpcAwareErrorMessage(e, verboseFailures)));
156156
fallbackDownloader.download(
157-
urls, authHeaders, checksum, canonicalId, destination, eventHandler, clientEnv, type);
157+
urls, credentials, checksum, canonicalId, destination, eventHandler, clientEnv, type);
158158
}
159159
}
160160

‎src/test/java/com/google/devtools/build/lib/bazel/repository/downloader/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ java_library(
1717
srcs = glob(["*.java"]),
1818
deps = [
1919
"//src/main/java/com/google/devtools/build/lib/authandtls",
20+
"//src/main/java/com/google/devtools/build/lib/authandtls/staticcredentials",
2021
"//src/main/java/com/google/devtools/build/lib/bazel/repository/cache",
2122
"//src/main/java/com/google/devtools/build/lib/bazel/repository/downloader",
2223
"//src/main/java/com/google/devtools/build/lib/events",

‎src/test/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpConnectorMultiplexerTest.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import com.google.common.base.Optional;
3333
import com.google.common.collect.ImmutableList;
3434
import com.google.common.collect.ImmutableMap;
35+
import com.google.devtools.build.lib.authandtls.staticcredentials.StaticCredentials;
3536
import com.google.devtools.build.lib.bazel.repository.cache.RepositoryCache.KeyType;
3637
import com.google.devtools.build.lib.bazel.repository.downloader.RetryingInputStream.Reconnector;
3738
import com.google.devtools.build.lib.events.EventHandler;
@@ -163,7 +164,8 @@ public void testHeaderComputationFunction() throws Exception {
163164
ImmutableMap.of("Authentication", ImmutableList.of("Zm9vOmZvb3NlY3JldA==")));
164165

165166
Function<URL, ImmutableMap<String, List<String>>> headerFunction =
166-
HttpConnectorMultiplexer.getHeaderFunction(baseHeaders, additionalHeaders);
167+
HttpConnectorMultiplexer.getHeaderFunction(
168+
baseHeaders, new StaticCredentials(additionalHeaders));
167169

168170
// Unrelated URL
169171
assertThat(headerFunction.apply(new URL("http://example.org/some/path/file.txt")))
@@ -215,7 +217,8 @@ public void testHeaderComputationFunction() throws Exception {
215217
ImmutableMap<String, List<String>> annonAuth =
216218
ImmutableMap.of("Authentication", ImmutableList.of("YW5vbnltb3VzOmZvb0BleGFtcGxlLm9yZw=="));
217219
Function<URL, ImmutableMap<String, List<String>>> combinedHeaders =
218-
HttpConnectorMultiplexer.getHeaderFunction(annonAuth, additionalHeaders);
220+
HttpConnectorMultiplexer.getHeaderFunction(
221+
annonAuth, new StaticCredentials(additionalHeaders));
219222
assertThat(combinedHeaders.apply(new URL("http://hosting.example.com/user/foo/file.txt")))
220223
.containsExactly("Authentication", ImmutableList.of("Zm9vOmZvb3NlY3JldA=="));
221224
assertThat(combinedHeaders.apply(new URL("http://unreleated.example.org/user/foo/file.txt")))

0 commit comments

Comments
 (0)