Skip to content

Commit 996abbf

Browse files
Yannictjgq
authored andcommitted
Wire credential helper to repository fetching.
After this change, credential helpers will be used to fetch credentials for repository fetching (rctx.download and rctx.download_and_extract), which take precedence over the `auth` parameter. Also improve the documentation for credential helper related flags. Fixes bazelbuild#15013.
1 parent f5bb27d commit 996abbf

File tree

14 files changed

+277
-40
lines changed

14 files changed

+277
-40
lines changed

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

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -148,11 +148,12 @@ public class AuthAndTLSOptions extends OptionsBase {
148148
documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
149149
effectTags = {OptionEffectTag.UNKNOWN},
150150
help =
151-
"Configures Credential Helpers to use for retrieving credentials for the provided scope"
152-
+ " (domain).\n\n"
153-
+ "Credentials from Credential Helpers take precedence over credentials from"
154-
+ " <code>--google_default_credentials</code>, `--google_credentials`, or"
155-
+ " <code>.netrc</code>.\n\n"
151+
"Configures a credential helper to use for retrieving authorization credentials for "
152+
+ " repository fetching, remote caching and execution, and the build event service.\n\n"
153+
+ "Credentials supplied by a helper take precedence over credentials supplied by"
154+
+ " --google_default_credentials, --google_credentials, a .netrc file, or the auth"
155+
+ " parameter to repository_ctx.download and repository_ctx.download_and_extract.\n\n"
156+
+ "May be specified multiple times to set up multiple helpers.\n\n"
156157
+ "See https://github.com/bazelbuild/proposals/blob/main/designs/2022-06-07-bazel-credential-helpers.md"
157158
+ " for details.")
158159
public List<UnresolvedScopedCredentialHelper> credentialHelpers;
@@ -164,8 +165,8 @@ public class AuthAndTLSOptions extends OptionsBase {
164165
documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
165166
effectTags = {OptionEffectTag.UNKNOWN},
166167
help =
167-
"Configures the timeout for the Credential Helper.\n\n"
168-
+ "Credential Helpers failing to respond within this timeout will fail the"
168+
"Configures the timeout for a credential helper.\n\n"
169+
+ "Credential helpers failing to respond within this timeout will fail the"
169170
+ " invocation.")
170171
public Duration credentialHelperTimeout;
171172

@@ -176,31 +177,43 @@ public class AuthAndTLSOptions extends OptionsBase {
176177
documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
177178
effectTags = {OptionEffectTag.UNKNOWN},
178179
help =
179-
"Configures the duration for which credentials from Credential Helpers are cached.\n\n"
180+
"The duration for which credentials supplied by a credential helper are cached.\n\n"
180181
+ "Invoking with a different value will adjust the lifetime of preexisting entries;"
181182
+ " pass zero to clear the cache. A clean command always clears the cache, regardless"
182183
+ " of this flag.")
183184
public Duration credentialHelperCacheTimeout;
184185

185-
/** One of the values of the `--credential_helper` flag. */
186+
/**
187+
* One of the values of the `--experimental_credential_helper` flag.
188+
*/
186189
@AutoValue
187190
public abstract static class UnresolvedScopedCredentialHelper {
188-
/** Returns the scope of the credential helper (if any). */
191+
192+
/**
193+
* Returns the scope of the credential helper (if any).
194+
*/
189195
public abstract Optional<String> getScope();
190196

191-
/** Returns the (unparsed) path of the credential helper. */
197+
/**
198+
* Returns the (unparsed) path of the credential helper.
199+
*/
192200
public abstract String getPath();
193201
}
194202

195-
/** A {@link Converter} for the `--credential_helper` flag. */
203+
/**
204+
* A {@link Converter} for the `--experimental_credential_helper` flag.
205+
*/
196206
public static final class UnresolvedScopedCredentialHelperConverter
197207
extends Converter.Contextless<UnresolvedScopedCredentialHelper> {
208+
198209
public static final UnresolvedScopedCredentialHelperConverter INSTANCE =
199210
new UnresolvedScopedCredentialHelperConverter();
200211

201212
@Override
202213
public String getTypeDescription() {
203-
return "An (unresolved) path to a credential helper for a scope.";
214+
return "Path to a credential helper, optionally prefixed by a scope (domain name) followed"
215+
+ " an '='. The path may be absolute or relative. If no scope is provided, the helper"
216+
+ " has universal scope.";
204217
}
205218

206219
@Override

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -372,7 +372,6 @@ static Optional<Credentials> newCredentialsFromNetrc(
372372
}
373373
}
374374

375-
@VisibleForTesting
376375
public static CredentialHelperProvider newCredentialHelperProvider(
377376
CredentialHelperEnvironment environment,
378377
CommandLinePathFactory pathFactory,

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ java_library(
2020
deps = [
2121
"//src/main/java/com/google/devtools/build/lib:runtime",
2222
"//src/main/java/com/google/devtools/build/lib/authandtls",
23+
"//src/main/java/com/google/devtools/common/options:options_internal",
2324
"//third_party:caffeine",
2425
"//third_party:guava",
2526
],

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

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,22 +21,33 @@
2121
import com.google.devtools.build.lib.authandtls.AuthAndTLSOptions;
2222
import com.google.devtools.build.lib.runtime.BlazeModule;
2323
import com.google.devtools.build.lib.runtime.CommandEnvironment;
24+
import com.google.devtools.common.options.OptionsBase;
2425
import java.net.URI;
2526
import java.time.Duration;
2627

27-
/** A module whose sole purpose is to hold the credential cache which is shared by other modules. */
28+
/**
29+
* A module whose sole purpose is to hold the credential cache which is shared by other modules.
30+
*/
2831
public class CredentialModule extends BlazeModule {
32+
2933
private final Cache<URI, ImmutableMap<String, ImmutableList<String>>> credentialCache =
3034
Caffeine.newBuilder()
3135
.expireAfterWrite(Duration.ZERO)
3236
.ticker(SystemMillisTicker.INSTANCE)
3337
.build();
3438

35-
/** Returns the credential cache. */
39+
/**
40+
* Returns the credential cache.
41+
*/
3642
public Cache<URI, ImmutableMap<String, ImmutableList<String>>> getCredentialCache() {
3743
return credentialCache;
3844
}
3945

46+
@Override
47+
public Iterable<Class<? extends OptionsBase>> getCommonCommandOptions() {
48+
return ImmutableList.of(AuthAndTLSOptions.class);
49+
}
50+
4051
@Override
4152
public void beforeCommand(CommandEnvironment env) {
4253
// Update the cache expiration policy according to the command options.

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ java_library(
2727
"//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster",
2828
"//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories",
2929
"//src/main/java/com/google/devtools/build/lib/analysis:config/build_configuration",
30+
"//src/main/java/com/google/devtools/build/lib/authandtls",
31+
"//src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper",
32+
"//src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper:credential_module",
3033
"//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:common",
3134
"//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:inspection",
3235
"//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:inspection_impl",
@@ -55,6 +58,7 @@ java_library(
5558
"//src/main/java/com/google/devtools/build/lib/starlarkbuildapi/repository",
5659
"//src/main/java/com/google/devtools/build/lib/util:abrupt_exit_exception",
5760
"//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code",
61+
"//src/main/java/com/google/devtools/build/lib/util:exit_code",
5862
"//src/main/java/com/google/devtools/build/lib/vfs",
5963
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
6064
"//src/main/java/com/google/devtools/common/options",

src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
package com.google.devtools.build.lib.bazel;
1717

18+
import com.google.common.base.Preconditions;
1819
import com.google.common.base.Strings;
1920
import com.google.common.base.Supplier;
2021
import com.google.common.collect.ImmutableList;
@@ -25,6 +26,13 @@
2526
import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
2627
import com.google.devtools.build.lib.analysis.RuleDefinition;
2728
import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue;
29+
import com.google.devtools.build.lib.authandtls.AuthAndTLSOptions;
30+
import com.google.devtools.build.lib.authandtls.GoogleAuthUtils;
31+
import com.google.devtools.build.lib.authandtls.StaticCredentials;
32+
import com.google.devtools.build.lib.authandtls.credentialhelper.CredentialHelperCredentials;
33+
import com.google.devtools.build.lib.authandtls.credentialhelper.CredentialHelperEnvironment;
34+
import com.google.devtools.build.lib.authandtls.credentialhelper.CredentialHelperProvider;
35+
import com.google.devtools.build.lib.authandtls.credentialhelper.CredentialModule;
2836
import com.google.devtools.build.lib.bazel.bzlmod.AttributeValues;
2937
import com.google.devtools.build.lib.bazel.bzlmod.BazelDepGraphFunction;
3038
import com.google.devtools.build.lib.bazel.bzlmod.BazelLockFileFunction;
@@ -110,6 +118,7 @@
110118
import java.util.Set;
111119
import java.util.concurrent.atomic.AtomicBoolean;
112120
import java.util.stream.Collectors;
121+
import javax.annotation.Nullable;
113122

114123
/** Adds support for fetching external code. */
115124
public class BazelRepositoryModule extends BlazeModule {
@@ -146,6 +155,9 @@ public class BazelRepositoryModule extends BlazeModule {
146155
private List<String> allowedYankedVersions = ImmutableList.of();
147156
private SingleExtensionEvalFunction singleExtensionEvalFunction;
148157

158+
@Nullable
159+
private CredentialModule credentialModule;
160+
149161
public BazelRepositoryModule() {
150162
this.starlarkRepositoryFunction = new StarlarkRepositoryFunction(downloadManager);
151163
this.repositoryHandlers = repositoryRules();
@@ -263,6 +275,8 @@ SkyFunctions.BAZEL_LOCK_FILE, new BazelLockFileFunction(directories.getWorkspace
263275
.addSkyFunction(SkyFunctions.SINGLE_EXTENSION_EVAL, singleExtensionEvalFunction)
264276
.addSkyFunction(SkyFunctions.SINGLE_EXTENSION_USAGES, new SingleExtensionUsagesFunction());
265277
filesystem = runtime.getFileSystem();
278+
279+
credentialModule = Preconditions.checkNotNull(runtime.getBlazeModule(CredentialModule.class));
266280
}
267281

268282
@Override
@@ -373,6 +387,41 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
373387
Code.BAD_DOWNLOADER_CONFIG));
374388
}
375389

390+
try {
391+
AuthAndTLSOptions authAndTlsOptions = env.getOptions().getOptions(AuthAndTLSOptions.class);
392+
var credentialHelperEnvironment =
393+
CredentialHelperEnvironment.newBuilder()
394+
.setEventReporter(env.getReporter())
395+
.setWorkspacePath(env.getWorkspace())
396+
.setClientEnvironment(env.getClientEnv())
397+
.setHelperExecutionTimeout(authAndTlsOptions.credentialHelperTimeout)
398+
.build();
399+
CredentialHelperProvider credentialHelperProvider =
400+
GoogleAuthUtils.newCredentialHelperProvider(
401+
credentialHelperEnvironment,
402+
env.getCommandLinePathFactory(),
403+
authAndTlsOptions.credentialHelpers);
404+
405+
downloadManager.setCredentialFactory(headers -> {
406+
Preconditions.checkNotNull(headers);
407+
408+
return new CredentialHelperCredentials(
409+
credentialHelperProvider,
410+
credentialHelperEnvironment,
411+
credentialModule.getCredentialCache(),
412+
Optional.of(new StaticCredentials(headers)));
413+
});
414+
} catch (IOException e) {
415+
env.getReporter().handle(Event.error(e.getMessage()));
416+
env.getBlazeModuleEnvironment()
417+
.exit(
418+
new AbruptExitException(
419+
detailedExitCode(
420+
"Error initializing credential helper",
421+
Code.CREDENTIALS_INIT_FAILURE)));
422+
return;
423+
}
424+
376425
if (repoOptions.experimentalDistdir != null) {
377426
downloadManager.setDistdir(
378427
repoOptions.experimentalDistdir.stream()

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

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import com.google.auth.Credentials;
2121
import com.google.common.base.MoreObjects;
2222
import com.google.common.base.Optional;
23+
import com.google.common.base.Preconditions;
2324
import com.google.common.base.Strings;
2425
import com.google.common.collect.ImmutableList;
2526
import com.google.common.collect.ImmutableMap;
@@ -61,6 +62,7 @@ public class DownloadManager {
6162
private int retries = 0;
6263
private boolean urlsAsDefaultCanonicalId;
6364
@Nullable private Credentials netrcCreds;
65+
private CredentialFactory credentialFactory = new DefaultCredentialFactory();
6466

6567
public DownloadManager(RepositoryCache repositoryCache, Downloader downloader) {
6668
this.repositoryCache = repositoryCache;
@@ -92,6 +94,10 @@ public void setNetrcCreds(Credentials netrcCreds) {
9294
this.netrcCreds = netrcCreds;
9395
}
9496

97+
public void setCredentialFactory(CredentialFactory credentialFactory) {
98+
this.credentialFactory = credentialFactory;
99+
}
100+
95101
/**
96102
* Downloads file to disk and returns path.
97103
*
@@ -257,7 +263,7 @@ public Path download(
257263
try {
258264
downloader.download(
259265
rewrittenUrls,
260-
new StaticCredentials(rewrittenAuthHeaders),
266+
credentialFactory.create(rewrittenAuthHeaders),
261267
checksum,
262268
canonicalId,
263269
destination,
@@ -338,7 +344,7 @@ public byte[] downloadAndReadOneUrl(
338344
for (int attempt = 0; attempt <= retries; ++attempt) {
339345
try {
340346
return httpDownloader.downloadAndReadOneUrl(
341-
rewrittenUrls.get(0), new StaticCredentials(authHeaders), eventHandler, clientEnv);
347+
rewrittenUrls.get(0), credentialFactory.create(authHeaders), eventHandler, clientEnv);
342348
} catch (ContentLengthMismatchException e) {
343349
if (attempt == retries) {
344350
throw e;
@@ -427,4 +433,17 @@ public boolean isFinished() {
427433
return isFinished;
428434
}
429435
}
436+
437+
public interface CredentialFactory {
438+
Credentials create(Map<URI, Map<String, List<String>>> authHeaders);
439+
}
440+
441+
private static final class DefaultCredentialFactory implements CredentialFactory {
442+
@Override
443+
public Credentials create(Map<URI, Map<String, List<String>>> authHeaders) {
444+
Preconditions.checkNotNull(authHeaders);
445+
446+
return new StaticCredentials(authHeaders);
447+
}
448+
}
430449
}

src/main/protobuf/failure_details.proto

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,7 @@ message ExternalRepository {
249249
OVERRIDE_DISALLOWED_MANAGED_DIRECTORIES = 1 [(metadata) = { exit_code: 2 }];
250250
BAD_DOWNLOADER_CONFIG = 2 [(metadata) = { exit_code: 2 }];
251251
REPOSITORY_MAPPING_RESOLUTION_FAILED = 3 [(metadata) = { exit_code: 37 }];
252+
CREDENTIALS_INIT_FAILURE = 4 [(metadata) = { exit_code: 2 }];
252253
}
253254
Code code = 1;
254255
// Additional data could include external repository names.

src/test/java/com/google/devtools/build/lib/buildtool/util/BUILD

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,8 @@ java_library(
5454
"//src/main/java/com/google/devtools/build/lib/analysis:top_level_artifact_context",
5555
"//src/main/java/com/google/devtools/build/lib/analysis:transitive_info_collection",
5656
"//src/main/java/com/google/devtools/build/lib/analysis:workspace_status_action",
57+
"//src/main/java/com/google/devtools/build/lib/authandtls",
58+
"//src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper:credential_module",
5759
"//src/main/java/com/google/devtools/build/lib/bazel:main",
5860
"//src/main/java/com/google/devtools/build/lib/bazel:repository_module",
5961
"//src/main/java/com/google/devtools/build/lib/bugreport",

src/test/java/com/google/devtools/build/lib/buildtool/util/BlazeRuntimeWrapper.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import com.google.devtools.build.lib.analysis.config.BuildOptions;
3333
import com.google.devtools.build.lib.analysis.config.BuildOptionsView;
3434
import com.google.devtools.build.lib.analysis.config.ExecutionTransitionFactory;
35+
import com.google.devtools.build.lib.authandtls.AuthAndTLSOptions;
3536
import com.google.devtools.build.lib.bugreport.Crash;
3637
import com.google.devtools.build.lib.bugreport.CrashContext;
3738
import com.google.devtools.build.lib.buildeventstream.BuildEventProtocolOptions;
@@ -285,6 +286,7 @@ private OptionsParser createOptionsParser(Command commandAnnotation) {
285286
Set<Class<? extends OptionsBase>> options =
286287
new HashSet<>(
287288
ImmutableList.of(
289+
AuthAndTLSOptions.class,
288290
BuildRequestOptions.class,
289291
BuildEventProtocolOptions.class,
290292
ExecutionOptions.class,

src/test/java/com/google/devtools/build/lib/buildtool/util/BuildIntegrationTestCase.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@
5353
import com.google.devtools.build.lib.analysis.util.AnalysisMock;
5454
import com.google.devtools.build.lib.analysis.util.AnalysisTestUtil;
5555
import com.google.devtools.build.lib.analysis.util.AnalysisTestUtil.DummyWorkspaceStatusActionContext;
56+
import com.google.devtools.build.lib.authandtls.credentialhelper.CredentialModule;
5657
import com.google.devtools.build.lib.bazel.BazelRepositoryModule;
5758
import com.google.devtools.build.lib.bugreport.BugReport;
5859
import com.google.devtools.build.lib.bugreport.BugReporter;
@@ -571,7 +572,8 @@ protected BlazeRuntime.Builder getRuntimeBuilder() throws Exception {
571572
.addBlazeModule(new OutputFilteringModule())
572573
.addBlazeModule(connectivityModule)
573574
.addBlazeModule(new SkymeldModule())
574-
.addBlazeModule(getMockBazelRepositoryModule());
575+
.addBlazeModule(getMockBazelRepositoryModule())
576+
.addBlazeModule(new CredentialModule());
575577
getSpawnModules().forEach(builder::addBlazeModule);
576578
builder
577579
.addBlazeModule(getBuildInfoModule())

src/test/shell/bazel/remote_helpers.sh

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,7 @@ function serve_file() {
3838
cd -
3939
}
4040

41-
# Serves $1 as a file on localhost:$nc_port insisting on authentication (but
42-
# accepting any credentials.
41+
# Serves $1 as a file on localhost:$nc_port expecting authentication.
4342
# * nc_port - the port nc is listening on.
4443
# * nc_log - the path to nc's log.
4544
# * nc_pid - the PID of nc.

0 commit comments

Comments
 (0)