Skip to content

Commit 8226742

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 4330694 commit 8226742

File tree

9 files changed

+186
-37
lines changed

9 files changed

+186
-37
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/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: 23 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,12 @@ public void setNetrcCreds(Credentials netrcCreds) {
9294
this.netrcCreds = netrcCreds;
9395
}
9496

97+
public void setCredentialFactory(CredentialFactory credentialFactory) {
98+
Preconditions.checkNotNull(credentialFactory);
99+
100+
this.credentialFactory = credentialFactory;
101+
}
102+
95103
/**
96104
* Downloads file to disk and returns path.
97105
*
@@ -257,7 +265,7 @@ public Path download(
257265
try {
258266
downloader.download(
259267
rewrittenUrls,
260-
new StaticCredentials(rewrittenAuthHeaders),
268+
credentialFactory.create(rewrittenAuthHeaders),
261269
checksum,
262270
canonicalId,
263271
destination,
@@ -338,7 +346,7 @@ public byte[] downloadAndReadOneUrl(
338346
for (int attempt = 0; attempt <= retries; ++attempt) {
339347
try {
340348
return httpDownloader.downloadAndReadOneUrl(
341-
rewrittenUrls.get(0), new StaticCredentials(authHeaders), eventHandler, clientEnv);
349+
rewrittenUrls.get(0), credentialFactory.create(authHeaders), eventHandler, clientEnv);
342350
} catch (ContentLengthMismatchException e) {
343351
if (attempt == retries) {
344352
throw e;
@@ -427,4 +435,17 @@ public boolean isFinished() {
427435
return isFinished;
428436
}
429437
}
438+
439+
public interface CredentialFactory {
440+
Credentials create(Map<URI, Map<String, List<String>>> authHeaders);
441+
}
442+
443+
private static final class DefaultCredentialFactory implements CredentialFactory {
444+
@Override
445+
public Credentials create(Map<URI, Map<String, List<String>>> authHeaders) {
446+
Preconditions.checkNotNull(authHeaders);
447+
448+
return new StaticCredentials(authHeaders);
449+
}
450+
}
430451
}

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/shell/bazel/remote_helpers.sh

Lines changed: 1 addition & 3 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.
@@ -280,7 +279,6 @@ fd = os.open(path, os.O_WRONLY|os.O_CREAT|os.O_APPEND)
280279
os.write(fd, b"1")
281280
os.close(fd)
282281
283-
# Must match //src/test/shell/bazel/testing_server.py.
284282
print("""{"headers":{"Authorization":["Bearer TOKEN"]}}""")
285283
EOF
286284
chmod +x "${TEST_TMPDIR}/credhelper"

0 commit comments

Comments
 (0)