Skip to content

Commit 4a38a90

Browse files
committed
[6.3.0] Wire credential helper to repository fetching.
After this change, credential helpers will be used when available to obtain credentials for repository fetching, taking precedence over the `auth` parameter to rctx.download and rctx.download_and_extract. Tests that need a credential helper are skipped on Windows for now, as otherwise the credential helper would have to be reimplemented in Batch or Powershell. Also improve the documentation for credential helper related flags. Fixes bazelbuild#15013. Closes bazelbuild#18173. PiperOrigin-RevId: 532454935 Change-Id: Ia3be8c21e001a36160f3d1df858593f8fb846055
1 parent e023bd3 commit 4a38a90

File tree

13 files changed

+265
-36
lines changed

13 files changed

+265
-36
lines changed

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

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -148,11 +148,13 @@ 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"
153+
+ " service.\n\n"
154+
+ "Credentials supplied by a helper take precedence over credentials supplied by"
155+
+ " --google_default_credentials, --google_credentials, a .netrc file, or the auth"
156+
+ " parameter to repository_ctx.download and repository_ctx.download_and_extract.\n\n"
157+
+ "May be specified multiple times to set up multiple helpers.\n\n"
156158
+ "See https://github.com/bazelbuild/proposals/blob/main/designs/2022-06-07-bazel-credential-helpers.md"
157159
+ " for details.")
158160
public List<UnresolvedScopedCredentialHelper> credentialHelpers;
@@ -164,8 +166,8 @@ public class AuthAndTLSOptions extends OptionsBase {
164166
documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
165167
effectTags = {OptionEffectTag.UNKNOWN},
166168
help =
167-
"Configures the timeout for the Credential Helper.\n\n"
168-
+ "Credential Helpers failing to respond within this timeout will fail the"
169+
"Configures the timeout for a credential helper.\n\n"
170+
+ "Credential helpers failing to respond within this timeout will fail the"
169171
+ " invocation.")
170172
public Duration credentialHelperTimeout;
171173

@@ -176,13 +178,13 @@ public class AuthAndTLSOptions extends OptionsBase {
176178
documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
177179
effectTags = {OptionEffectTag.UNKNOWN},
178180
help =
179-
"Configures the duration for which credentials from Credential Helpers are cached.\n\n"
181+
"The duration for which credentials supplied by a credential helper are cached.\n\n"
180182
+ "Invoking with a different value will adjust the lifetime of preexisting entries;"
181183
+ " pass zero to clear the cache. A clean command always clears the cache, regardless"
182184
+ " of this flag.")
183185
public Duration credentialHelperCacheTimeout;
184186

185-
/** One of the values of the `--credential_helper` flag. */
187+
/** One of the values of the `--experimental_credential_helper` flag. */
186188
@AutoValue
187189
public abstract static class UnresolvedScopedCredentialHelper {
188190
/** Returns the scope of the credential helper (if any). */
@@ -192,15 +194,19 @@ public abstract static class UnresolvedScopedCredentialHelper {
192194
public abstract String getPath();
193195
}
194196

195-
/** A {@link Converter} for the `--credential_helper` flag. */
197+
/** A {@link Converter} for the `--experimental_credential_helper` flag. */
196198
public static final class UnresolvedScopedCredentialHelperConverter
197199
extends Converter.Contextless<UnresolvedScopedCredentialHelper> {
198200
public static final UnresolvedScopedCredentialHelperConverter INSTANCE =
199201
new UnresolvedScopedCredentialHelperConverter();
200202

201203
@Override
202204
public String getTypeDescription() {
203-
return "An (unresolved) path to a credential helper for a scope.";
205+
return "Path to a credential helper. It may be absolute, relative to the PATH environment"
206+
+ " variable, or %workspace%-relative. The path be optionally prefixed by a scope "
207+
+ " followed by an '='. The scope is a domain name, optionally with a single leading '*'"
208+
+ " wildcard component. A helper applies to URIs matching its scope, with more specific"
209+
+ " scopes preferred. If a helper has no scope, it applies to every URI.";
204210
}
205211

206212
@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
@@ -17,6 +17,7 @@ java_library(
1717
deps = [
1818
"//src/main/java/com/google/devtools/build/lib:runtime",
1919
"//src/main/java/com/google/devtools/build/lib/authandtls",
20+
"//src/main/java/com/google/devtools/common/options",
2021
"//third_party:caffeine",
2122
"//third_party:guava",
2223
],

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
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

@@ -37,6 +38,11 @@ public Cache<URI, ImmutableMap<String, ImmutableList<String>>> getCredentialCach
3738
return credentialCache;
3839
}
3940

41+
@Override
42+
public Iterable<Class<? extends OptionsBase>> getCommonCommandOptions() {
43+
return ImmutableList.of(AuthAndTLSOptions.class);
44+
}
45+
4046
@Override
4147
public void beforeCommand(CommandEnvironment env) {
4248
// 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
@@ -26,6 +26,9 @@ java_library(
2626
"//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster",
2727
"//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories",
2828
"//src/main/java/com/google/devtools/build/lib/analysis:config/build_configuration",
29+
"//src/main/java/com/google/devtools/build/lib/authandtls",
30+
"//src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper",
31+
"//src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper:credential_module",
2932
"//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:common",
3033
"//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:inspection",
3134
"//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:inspection_impl",
@@ -59,6 +62,7 @@ java_library(
5962
"//src/main/java/com/google/devtools/common/options",
6063
"//src/main/protobuf:failure_details_java_proto",
6164
"//third_party:guava",
65+
"//third_party:jsr305",
6266
],
6367
)
6468

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

Lines changed: 48 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,8 @@ public class BazelRepositoryModule extends BlazeModule {
146155
private List<String> allowedYankedVersions = ImmutableList.of();
147156
private SingleExtensionEvalFunction singleExtensionEvalFunction;
148157

158+
@Nullable private CredentialModule credentialModule;
159+
149160
public BazelRepositoryModule() {
150161
this.starlarkRepositoryFunction = new StarlarkRepositoryFunction(downloadManager);
151162
this.repositoryHandlers = repositoryRules();
@@ -263,6 +274,8 @@ SkyFunctions.BAZEL_LOCK_FILE, new BazelLockFileFunction(directories.getWorkspace
263274
.addSkyFunction(SkyFunctions.SINGLE_EXTENSION_EVAL, singleExtensionEvalFunction)
264275
.addSkyFunction(SkyFunctions.SINGLE_EXTENSION_USAGES, new SingleExtensionUsagesFunction());
265276
filesystem = runtime.getFileSystem();
277+
278+
credentialModule = Preconditions.checkNotNull(runtime.getBlazeModule(CredentialModule.class));
266279
}
267280

268281
@Override
@@ -373,6 +386,41 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
373386
Code.BAD_DOWNLOADER_CONFIG));
374387
}
375388

389+
try {
390+
AuthAndTLSOptions authAndTlsOptions = env.getOptions().getOptions(AuthAndTLSOptions.class);
391+
var credentialHelperEnvironment =
392+
CredentialHelperEnvironment.newBuilder()
393+
.setEventReporter(env.getReporter())
394+
.setWorkspacePath(env.getWorkspace())
395+
.setClientEnvironment(env.getClientEnv())
396+
.setHelperExecutionTimeout(authAndTlsOptions.credentialHelperTimeout)
397+
.build();
398+
CredentialHelperProvider credentialHelperProvider =
399+
GoogleAuthUtils.newCredentialHelperProvider(
400+
credentialHelperEnvironment,
401+
env.getCommandLinePathFactory(),
402+
authAndTlsOptions.credentialHelpers);
403+
404+
downloadManager.setCredentialFactory(
405+
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", Code.CREDENTIALS_INIT_FAILURE)));
421+
return;
422+
}
423+
376424
if (repoOptions.experimentalDistdir != null) {
377425
downloadManager.setDistdir(
378426
repoOptions.experimentalDistdir.stream()

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

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,12 @@ public class DownloadManager {
6161
private int retries = 0;
6262
private boolean urlsAsDefaultCanonicalId;
6363
@Nullable private Credentials netrcCreds;
64+
private CredentialFactory credentialFactory = StaticCredentials::new;
65+
66+
/** Creates {@code Credentials} from a map of per-{@code URI} authentication headers. */
67+
public interface CredentialFactory {
68+
Credentials create(Map<URI, Map<String, List<String>>> authHeaders);
69+
}
6470

6571
public DownloadManager(RepositoryCache repositoryCache, Downloader downloader) {
6672
this.repositoryCache = repositoryCache;
@@ -92,6 +98,10 @@ public void setNetrcCreds(Credentials netrcCreds) {
9298
this.netrcCreds = netrcCreds;
9399
}
94100

101+
public void setCredentialFactory(CredentialFactory credentialFactory) {
102+
this.credentialFactory = credentialFactory;
103+
}
104+
95105
/**
96106
* Downloads file to disk and returns path.
97107
*
@@ -257,7 +267,7 @@ public Path download(
257267
try {
258268
downloader.download(
259269
rewrittenUrls,
260-
new StaticCredentials(rewrittenAuthHeaders),
270+
credentialFactory.create(rewrittenAuthHeaders),
261271
checksum,
262272
canonicalId,
263273
destination,
@@ -338,7 +348,7 @@ public byte[] downloadAndReadOneUrl(
338348
for (int attempt = 0; attempt <= retries; ++attempt) {
339349
try {
340350
return httpDownloader.downloadAndReadOneUrl(
341-
rewrittenUrls.get(0), new StaticCredentials(authHeaders), eventHandler, clientEnv);
351+
rewrittenUrls.get(0), credentialFactory.create(authHeaders), eventHandler, clientEnv);
342352
} catch (ContentLengthMismatchException e) {
343353
if (attempt == retries) {
344354
throw e;

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: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ java_library(
5050
"//src/main/java/com/google/devtools/build/lib/analysis:top_level_artifact_context",
5151
"//src/main/java/com/google/devtools/build/lib/analysis:transitive_info_collection",
5252
"//src/main/java/com/google/devtools/build/lib/analysis:workspace_status_action",
53+
"//src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper:credential_module",
5354
"//src/main/java/com/google/devtools/build/lib/bazel:main",
5455
"//src/main/java/com/google/devtools/build/lib/bazel:repository_module",
5556
"//src/main/java/com/google/devtools/build/lib/bugreport",

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;
@@ -530,7 +531,8 @@ protected BlazeRuntime.Builder getRuntimeBuilder() throws Exception {
530531
.addBlazeModule(new BuildIntegrationTestCommandsModule())
531532
.addBlazeModule(new OutputFilteringModule())
532533
.addBlazeModule(connectivityModule)
533-
.addBlazeModule(getMockBazelRepositoryModule());
534+
.addBlazeModule(getMockBazelRepositoryModule())
535+
.addBlazeModule(new CredentialModule());
534536
getSpawnModules().forEach(builder::addBlazeModule);
535537
builder
536538
.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)