Skip to content

Commit 2baa5a4

Browse files
tjgqcopybara-github
tjgq
authored and
copybara-github
committed
Keep credentials cached across build commands.
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 31e4bf4 commit 2baa5a4

File tree

22 files changed

+328
-146
lines changed

22 files changed

+328
-146
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
@@ -175,7 +175,11 @@ public class AuthAndTLSOptions extends OptionsBase {
175175
converter = DurationConverter.class,
176176
documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
177177
effectTags = {OptionEffectTag.UNKNOWN},
178-
help = "Configures the duration for which credentials from Credential Helpers are cached.")
178+
help =
179+
"Configures the duration for which credentials from Credential Helpers are cached.\n\n"
180+
+ "Invoking with a different value will adjust the lifetime of preexisting entries;"
181+
+ " pass zero to clear the cache. A clean command always clears the cache, regardless"
182+
+ " of this flag.")
179183
public Duration credentialHelperCacheTimeout;
180184

181185
/** 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;
@@ -48,6 +51,7 @@
4851
import java.io.FileNotFoundException;
4952
import java.io.IOException;
5053
import java.io.InputStream;
54+
import java.net.URI;
5155
import java.util.List;
5256
import java.util.Map;
5357
import java.util.Optional;
@@ -248,6 +252,7 @@ public static CallCredentialsProvider newCallCredentialsProvider(@Nullable Crede
248252
*/
249253
public static Credentials newCredentials(
250254
CredentialHelperEnvironment credentialHelperEnvironment,
255+
Cache<URI, ImmutableMap<String, ImmutableList<String>>> credentialCache,
251256
CommandLinePathFactory commandLinePathFactory,
252257
FileSystem fileSystem,
253258
AuthAndTLSOptions authAndTlsOptions)
@@ -257,12 +262,12 @@ public static Credentials newCredentials(
257262
Preconditions.checkNotNull(fileSystem);
258263
Preconditions.checkNotNull(authAndTlsOptions);
259264

260-
Optional<Credentials> credentials = newGoogleCredentials(authAndTlsOptions);
265+
Optional<Credentials> fallbackCredentials = newGoogleCredentials(authAndTlsOptions);
261266

262-
if (credentials.isEmpty()) {
267+
if (fallbackCredentials.isEmpty()) {
263268
// Fallback to .netrc if it exists.
264269
try {
265-
credentials =
270+
fallbackCredentials =
266271
newCredentialsFromNetrc(credentialHelperEnvironment.getClientEnvironment(), fileSystem);
267272
} catch (IOException e) {
268273
// TODO(yannic): Make this fail the build.
@@ -276,8 +281,8 @@ public static Credentials newCredentials(
276281
commandLinePathFactory,
277282
authAndTlsOptions.credentialHelpers),
278283
credentialHelperEnvironment,
279-
credentials,
280-
authAndTlsOptions.credentialHelperCacheTimeout);
284+
credentialCache,
285+
fallbackCredentials);
281286
}
282287

283288
/**

‎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
@@ -8,9 +8,23 @@ filegroup(
88
visibility = ["//src:__subpackages__"],
99
)
1010

11+
java_library(
12+
name = "credential_module",
13+
srcs = ["CredentialModule.java"],
14+
deps = [
15+
"//src/main/java/com/google/devtools/build/lib:runtime",
16+
"//src/main/java/com/google/devtools/build/lib/authandtls",
17+
"//third_party:caffeine",
18+
"//third_party:guava",
19+
],
20+
)
21+
1122
java_library(
1223
name = "credentialhelper",
13-
srcs = glob(["*.java"]),
24+
srcs = glob(
25+
["*.java"],
26+
exclude = ["CredentialModule.java"],
27+
),
1428
deps = [
1529
"//src/main/java/com/google/devtools/build/lib/events",
1630
"//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
@@ -136,6 +136,7 @@ java_library(
136136
":spawn_log_module",
137137
"//src/main/java/com/google/devtools/build/lib:runtime",
138138
"//src/main/java/com/google/devtools/build/lib/analysis:blaze_version_info",
139+
"//src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper:credential_module",
139140
"//src/main/java/com/google/devtools/build/lib/bazel/coverage",
140141
"//src/main/java/com/google/devtools/build/lib/bazel/debug:workspace-rule-module",
141142
"//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
@@ -16,6 +16,7 @@
1616
import com.google.common.collect.ImmutableList;
1717
import com.google.common.collect.ImmutableMap;
1818
import com.google.devtools.build.lib.analysis.BlazeVersionInfo;
19+
import com.google.devtools.build.lib.authandtls.credentialhelper.CredentialModule;
1920
import com.google.devtools.build.lib.runtime.BlazeModule;
2021
import com.google.devtools.build.lib.runtime.BlazeRuntime;
2122
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.runtime.MemoryPressureModule.class,
4750
com.google.devtools.build.lib.platform.SleepPreventionModule.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
@@ -38,9 +38,11 @@ java_library(
3838
":buildeventservice-options",
3939
"//src/main/java/com/google/devtools/build/lib:build-request-options",
4040
"//src/main/java/com/google/devtools/build/lib:runtime",
41+
"//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories",
4142
"//src/main/java/com/google/devtools/build/lib/analysis:test/test_configuration",
4243
"//src/main/java/com/google/devtools/build/lib/authandtls",
4344
"//src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper",
45+
"//src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper:credential_module",
4446
"//src/main/java/com/google/devtools/build/lib/bugreport",
4547
"//src/main/java/com/google/devtools/build/lib/buildeventservice/client",
4648
"//src/main/java/com/google/devtools/build/lib/buildeventstream",

0 commit comments

Comments
 (0)