Skip to content

Commit 6d2d68d

Browse files
tjgqtjgq
authored
[5.4.0] Keep credentials cached across build commands. (bazelbuild#16884)
When using a credential helper, the lifetime of the credential cache is currently tied to an individual command, which causes the helper to be called for every command resulting in poor incremental build latency for builds using a non-trivial helper. Since the cache must be shared by RemoteModule and BazelBuildServiceModule, I've introduced a new CredentialModule whose sole purpose is to provide access to it. Closes bazelbuild#16822. PiperOrigin-RevId: 491598103 Change-Id: Ib668954b635a0e9498f0a7418707d6a2dfae0265
1 parent 2f0f3e1 commit 6d2d68d

File tree

19 files changed

+339
-147
lines changed

19 files changed

+339
-147
lines changed

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,11 @@ public class AuthAndTLSOptions extends OptionsBase {
174174
converter = DurationConverter.class,
175175
documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
176176
effectTags = {OptionEffectTag.UNKNOWN},
177-
help = "Configures the duration for which credentials from Credential Helpers are cached.")
177+
help =
178+
"Configures the duration for which credentials from Credential Helpers are cached.\n\n"
179+
+ "Invoking with a different value will adjust the lifetime of preexisting entries;"
180+
+ " pass zero to clear the cache. A clean command always clears the cache, regardless"
181+
+ " of this flag.")
178182
public Duration credentialHelperCacheTimeout;
179183

180184
/** One of the values of the `--credential_helper` flag. */

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ java_library(
2222
"//src/main/java/com/google/devtools/common/options",
2323
"//third_party:auth",
2424
"//third_party:auto_value",
25+
"//third_party:caffeine",
2526
"//third_party:guava",
2627
"//third_party:jsr305",
2728
"//third_party:netty",

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

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

1515
package com.google.devtools.build.lib.authandtls;
1616

17+
import com.github.benmanes.caffeine.cache.Cache;
1718
import com.google.auth.Credentials;
1819
import com.google.auth.oauth2.GoogleCredentials;
1920
import com.google.common.annotations.VisibleForTesting;
2021
import com.google.common.base.Preconditions;
2122
import com.google.common.base.Strings;
23+
import com.google.common.collect.ImmutableList;
24+
import com.google.common.collect.ImmutableMap;
2225
import com.google.devtools.build.lib.authandtls.credentialhelper.CredentialHelperCredentials;
2326
import com.google.devtools.build.lib.authandtls.credentialhelper.CredentialHelperEnvironment;
2427
import com.google.devtools.build.lib.authandtls.credentialhelper.CredentialHelperProvider;
@@ -47,6 +50,7 @@
4750
import java.io.FileNotFoundException;
4851
import java.io.IOException;
4952
import java.io.InputStream;
53+
import java.net.URI;
5054
import java.util.List;
5155
import java.util.Map;
5256
import java.util.Optional;
@@ -235,6 +239,7 @@ public static CallCredentialsProvider newCallCredentialsProvider(@Nullable Crede
235239
*/
236240
public static Credentials newCredentials(
237241
CredentialHelperEnvironment credentialHelperEnvironment,
242+
Cache<URI, ImmutableMap<String, ImmutableList<String>>> credentialCache,
238243
CommandLinePathFactory commandLinePathFactory,
239244
FileSystem fileSystem,
240245
AuthAndTLSOptions authAndTlsOptions)
@@ -244,12 +249,12 @@ public static Credentials newCredentials(
244249
Preconditions.checkNotNull(fileSystem);
245250
Preconditions.checkNotNull(authAndTlsOptions);
246251

247-
Optional<Credentials> credentials = newGoogleCredentials(authAndTlsOptions);
252+
Optional<Credentials> fallbackCredentials = newGoogleCredentials(authAndTlsOptions);
248253

249-
if (credentials.isEmpty()) {
254+
if (fallbackCredentials.isEmpty()) {
250255
// Fallback to .netrc if it exists.
251256
try {
252-
credentials =
257+
fallbackCredentials =
253258
newCredentialsFromNetrc(credentialHelperEnvironment.getClientEnvironment(), fileSystem);
254259
} catch (IOException e) {
255260
// TODO(yannic): Make this fail the build.
@@ -263,8 +268,8 @@ public static Credentials newCredentials(
263268
commandLinePathFactory,
264269
authAndTlsOptions.credentialHelpers),
265270
credentialHelperEnvironment,
266-
credentials,
267-
authAndTlsOptions.credentialHelperCacheTimeout);
271+
credentialCache,
272+
fallbackCredentials);
268273
}
269274

270275
/**

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

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,23 @@ filegroup(
1010
visibility = ["//src:__subpackages__"],
1111
)
1212

13+
java_library(
14+
name = "credential_module",
15+
srcs = ["CredentialModule.java"],
16+
deps = [
17+
"//src/main/java/com/google/devtools/build/lib:runtime",
18+
"//src/main/java/com/google/devtools/build/lib/authandtls",
19+
"//third_party:caffeine",
20+
"//third_party:guava",
21+
],
22+
)
23+
1324
java_library(
1425
name = "credentialhelper",
15-
srcs = glob(["*.java"]),
26+
srcs = glob(
27+
["*.java"],
28+
exclude = ["CredentialModule.java"],
29+
),
1630
deps = [
1731
"//src/main/java/com/google/devtools/build/lib/events",
1832
"//src/main/java/com/google/devtools/build/lib/profiler",

‎src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialHelper.java‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ public Path getPath() {
6767
* @return The response from the subprocess.
6868
*/
6969
public GetCredentialsResponse getCredentials(CredentialHelperEnvironment environment, URI uri)
70-
throws InterruptedException, IOException {
70+
throws IOException {
7171
Preconditions.checkNotNull(environment);
7272
Preconditions.checkNotNull(uri);
7373

@@ -81,7 +81,16 @@ public GetCredentialsResponse getCredentials(CredentialHelperEnvironment environ
8181
GSON.toJson(GetCredentialsRequest.newBuilder().setUri(uri).build(), stdin);
8282
}
8383

84-
process.waitFor();
84+
try {
85+
process.waitFor();
86+
} catch (InterruptedException e) {
87+
throw new CredentialHelperException(
88+
String.format(
89+
Locale.US,
90+
"Failed to get credentials for '%s' from helper '%s': process was interrupted",
91+
uri,
92+
path));
93+
}
8594

8695
if (process.timedout()) {
8796
throw new CredentialHelperException(

‎src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialHelperCredentials.java‎

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

1515
package com.google.devtools.build.lib.authandtls.credentialhelper;
1616

17-
import com.github.benmanes.caffeine.cache.CacheLoader;
18-
import com.github.benmanes.caffeine.cache.Caffeine;
19-
import com.github.benmanes.caffeine.cache.LoadingCache;
17+
import com.github.benmanes.caffeine.cache.Cache;
2018
import com.google.auth.Credentials;
2119
import com.google.common.base.Preconditions;
20+
import com.google.common.collect.ImmutableList;
2221
import com.google.common.collect.ImmutableMap;
2322
import java.io.IOException;
2423
import java.net.URI;
25-
import java.time.Duration;
2624
import java.util.List;
2725
import java.util.Map;
2826
import java.util.Optional;
@@ -33,29 +31,34 @@
3331
* helper} as subprocess, falling back to another {@link Credentials} if no suitable helper exists.
3432
*/
3533
public class CredentialHelperCredentials extends Credentials {
34+
private final CredentialHelperProvider credentialHelperProvider;
35+
private final CredentialHelperEnvironment credentialHelperEnvironment;
36+
private final Cache<URI, ImmutableMap<String, ImmutableList<String>>> credentialCache;
3637
private final Optional<Credentials> fallbackCredentials;
3738

38-
private final LoadingCache<URI, GetCredentialsResponse> credentialCache;
39+
/** Wraps around an {@link IOException} so we can smuggle it through {@link Cache#get}. */
40+
public static final class WrappedIOException extends RuntimeException {
41+
private final IOException wrapped;
42+
43+
WrappedIOException(IOException e) {
44+
super(e);
45+
this.wrapped = e;
46+
}
47+
48+
IOException getWrapped() {
49+
return wrapped;
50+
}
51+
}
3952

4053
public CredentialHelperCredentials(
4154
CredentialHelperProvider credentialHelperProvider,
4255
CredentialHelperEnvironment credentialHelperEnvironment,
43-
Optional<Credentials> fallbackCredentials,
44-
Duration cacheTimeout) {
45-
Preconditions.checkNotNull(credentialHelperProvider);
46-
Preconditions.checkNotNull(credentialHelperEnvironment);
56+
Cache<URI, ImmutableMap<String, ImmutableList<String>>> credentialCache,
57+
Optional<Credentials> fallbackCredentials) {
58+
this.credentialHelperProvider = Preconditions.checkNotNull(credentialHelperProvider);
59+
this.credentialHelperEnvironment = Preconditions.checkNotNull(credentialHelperEnvironment);
60+
this.credentialCache = Preconditions.checkNotNull(credentialCache);
4761
this.fallbackCredentials = Preconditions.checkNotNull(fallbackCredentials);
48-
Preconditions.checkNotNull(cacheTimeout);
49-
Preconditions.checkArgument(
50-
!cacheTimeout.isNegative() && !cacheTimeout.isZero(),
51-
"Cache timeout must be greater than 0");
52-
53-
credentialCache =
54-
Caffeine.newBuilder()
55-
.expireAfterWrite(cacheTimeout)
56-
.build(
57-
new CredentialHelperCacheLoader(
58-
credentialHelperProvider, credentialHelperEnvironment));
5962
}
6063

6164
@Override
@@ -68,12 +71,18 @@ public String getAuthenticationType() {
6871
}
6972

7073
@Override
74+
@SuppressWarnings("unchecked") // Map<String, ImmutableList<String>> to Map<String<List<String>>
7175
public Map<String, List<String>> getRequestMetadata(URI uri) throws IOException {
7276
Preconditions.checkNotNull(uri);
7377

74-
Optional<Map<String, List<String>>> credentials = getRequestMetadataFromCredentialHelper(uri);
75-
if (credentials.isPresent()) {
76-
return credentials.get();
78+
ImmutableMap<String, ImmutableList<String>> credentials;
79+
try {
80+
credentials = credentialCache.get(uri, this::getCredentialsFromHelper);
81+
} catch (WrappedIOException e) {
82+
throw e.getWrapped();
83+
}
84+
if (credentials != null) {
85+
return (Map) credentials;
7786
}
7887

7988
if (fallbackCredentials.isPresent()) {
@@ -83,13 +92,28 @@ public Map<String, List<String>> getRequestMetadata(URI uri) throws IOException
8392
return ImmutableMap.of();
8493
}
8594

86-
@SuppressWarnings("unchecked") // Map<String, ImmutableList<String>> to Map<String<List<String>>
87-
private Optional<Map<String, List<String>>> getRequestMetadataFromCredentialHelper(URI uri) {
95+
@Nullable
96+
private ImmutableMap<String, ImmutableList<String>> getCredentialsFromHelper(URI uri) {
8897
Preconditions.checkNotNull(uri);
8998

90-
GetCredentialsResponse response = credentialCache.get(uri);
99+
Optional<CredentialHelper> maybeCredentialHelper =
100+
credentialHelperProvider.findCredentialHelper(uri);
101+
if (maybeCredentialHelper.isEmpty()) {
102+
return null;
103+
}
104+
CredentialHelper credentialHelper = maybeCredentialHelper.get();
105+
106+
GetCredentialsResponse response;
107+
try {
108+
response = credentialHelper.getCredentials(credentialHelperEnvironment, uri);
109+
} catch (IOException e) {
110+
throw new WrappedIOException(e);
111+
}
112+
if (response == null) {
113+
return null;
114+
}
91115

92-
return Optional.ofNullable(response).map(value -> (Map) value.getHeaders());
116+
return response.getHeaders();
93117
}
94118

95119
@Override
@@ -110,32 +134,4 @@ public void refresh() throws IOException {
110134

111135
credentialCache.invalidateAll();
112136
}
113-
114-
private static final class CredentialHelperCacheLoader
115-
implements CacheLoader<URI, GetCredentialsResponse> {
116-
private final CredentialHelperProvider credentialHelperProvider;
117-
private final CredentialHelperEnvironment credentialHelperEnvironment;
118-
119-
public CredentialHelperCacheLoader(
120-
CredentialHelperProvider credentialHelperProvider,
121-
CredentialHelperEnvironment credentialHelperEnvironment) {
122-
this.credentialHelperProvider = Preconditions.checkNotNull(credentialHelperProvider);
123-
this.credentialHelperEnvironment = Preconditions.checkNotNull(credentialHelperEnvironment);
124-
}
125-
126-
@Nullable
127-
@Override
128-
public GetCredentialsResponse load(URI uri) throws IOException, InterruptedException {
129-
Preconditions.checkNotNull(uri);
130-
131-
Optional<CredentialHelper> maybeCredentialHelper =
132-
credentialHelperProvider.findCredentialHelper(uri);
133-
if (maybeCredentialHelper.isEmpty()) {
134-
return null;
135-
}
136-
CredentialHelper credentialHelper = maybeCredentialHelper.get();
137-
138-
return credentialHelper.getCredentials(credentialHelperEnvironment, uri);
139-
}
140-
}
141137
}

‎src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialModule.java‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
// Copyright 2022 The Bazel Authors. All rights reserved.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package com.google.devtools.build.lib.authandtls.credentialhelper;
16+
17+
import com.github.benmanes.caffeine.cache.Cache;
18+
import com.github.benmanes.caffeine.cache.Caffeine;
19+
import com.google.common.collect.ImmutableList;
20+
import com.google.common.collect.ImmutableMap;
21+
import com.google.devtools.build.lib.authandtls.AuthAndTLSOptions;
22+
import com.google.devtools.build.lib.runtime.BlazeModule;
23+
import com.google.devtools.build.lib.runtime.CommandEnvironment;
24+
import java.net.URI;
25+
import java.time.Duration;
26+
27+
/** A module whose sole purpose is to hold the credential cache which is shared by other modules. */
28+
public class CredentialModule extends BlazeModule {
29+
private final Cache<URI, ImmutableMap<String, ImmutableList<String>>> credentialCache =
30+
Caffeine.newBuilder().expireAfterWrite(Duration.ZERO).build();
31+
32+
/** Returns the credential cache. */
33+
public Cache<URI, ImmutableMap<String, ImmutableList<String>>> getCredentialCache() {
34+
return credentialCache;
35+
}
36+
37+
@Override
38+
public void beforeCommand(CommandEnvironment env) {
39+
// Update the cache expiration policy according to the command options.
40+
AuthAndTLSOptions authAndTlsOptions = env.getOptions().getOptions(AuthAndTLSOptions.class);
41+
credentialCache
42+
.policy()
43+
.expireAfterWrite()
44+
.get()
45+
.setExpiresAfter(authAndTlsOptions.credentialHelperCacheTimeout);
46+
47+
// Clear the cache on clean.
48+
if (env.getCommand().name().equals("clean")) {
49+
credentialCache.invalidateAll();
50+
}
51+
}
52+
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,7 @@ java_library(
131131
":spawn_log_module",
132132
"//src/main/java/com/google/devtools/build/lib:runtime",
133133
"//src/main/java/com/google/devtools/build/lib/analysis:blaze_version_info",
134+
"//src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper:credential_module",
134135
"//src/main/java/com/google/devtools/build/lib/bazel/coverage",
135136
"//src/main/java/com/google/devtools/build/lib/bazel/debug:workspace-rule-module",
136137
"//src/main/java/com/google/devtools/build/lib/bazel/repository",

‎src/main/java/com/google/devtools/build/lib/bazel/Bazel.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import com.google.common.collect.ImmutableMap;
1818
import com.google.devtools.build.lib.analysis.BlazeVersionInfo;
1919
import com.google.devtools.build.lib.bazel.repository.starlark.StarlarkRepositoryDebugModule;
20+
import com.google.devtools.build.lib.authandtls.credentialhelper.CredentialModule;
2021
import com.google.devtools.build.lib.runtime.BlazeModule;
2122
import com.google.devtools.build.lib.runtime.BlazeRuntime;
2223
import java.io.IOException;
@@ -42,6 +43,8 @@ public final class Bazel {
4243
// This module needs to be registered before any module providing a SpawnCache
4344
// implementation.
4445
com.google.devtools.build.lib.runtime.NoSpawnCacheModule.class,
46+
// This module needs to be registered before any module that uses the credential cache.
47+
CredentialModule.class,
4548
com.google.devtools.build.lib.runtime.CommandLogModule.class,
4649
com.google.devtools.build.lib.platform.SleepPreventionModule.class,
4750
com.google.devtools.build.lib.platform.SystemSuspensionModule.class,

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,11 @@ java_library(
3737
deps = [
3838
":buildeventservice-options",
3939
"//src/main/java/com/google/devtools/build/lib:runtime",
40+
"//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories",
4041
"//src/main/java/com/google/devtools/build/lib/analysis:test/test_configuration",
4142
"//src/main/java/com/google/devtools/build/lib/authandtls",
4243
"//src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper",
44+
"//src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper:credential_module",
4345
"//src/main/java/com/google/devtools/build/lib/bugreport",
4446
"//src/main/java/com/google/devtools/build/lib/buildeventservice/client",
4547
"//src/main/java/com/google/devtools/build/lib/buildeventstream",

0 commit comments

Comments
 (0)