Skip to content

Commit

Permalink
Wire credential helper to repository fetching.
Browse files Browse the repository at this point in the history
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 #15013.
  • Loading branch information
Yannic authored and tjgq committed May 15, 2023
1 parent 4330694 commit ea84c9c
Show file tree
Hide file tree
Showing 9 changed files with 217 additions and 39 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -148,11 +148,12 @@ public class AuthAndTLSOptions extends OptionsBase {
documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
effectTags = {OptionEffectTag.UNKNOWN},
help =
"Configures Credential Helpers to use for retrieving credentials for the provided scope"
+ " (domain).\n\n"
+ "Credentials from Credential Helpers take precedence over credentials from"
+ " <code>--google_default_credentials</code>, `--google_credentials`, or"
+ " <code>.netrc</code>.\n\n"
"Configures a credential helper to use for retrieving authorization credentials for "
+ " repository fetching, remote caching and execution, and the build event service.\n\n"
+ "Credentials supplied by a helper take precedence over credentials supplied by"
+ " --google_default_credentials, --google_credentials, a .netrc file, or the auth"
+ " parameter to repository_ctx.download and repository_ctx.download_and_extract.\n\n"
+ "May be specified multiple times to set up multiple helpers.\n\n"
+ "See https://github.com/bazelbuild/proposals/blob/main/designs/2022-06-07-bazel-credential-helpers.md"
+ " for details.")
public List<UnresolvedScopedCredentialHelper> credentialHelpers;
Expand All @@ -164,8 +165,8 @@ public class AuthAndTLSOptions extends OptionsBase {
documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
effectTags = {OptionEffectTag.UNKNOWN},
help =
"Configures the timeout for the Credential Helper.\n\n"
+ "Credential Helpers failing to respond within this timeout will fail the"
"Configures the timeout for a credential helper.\n\n"
+ "Credential helpers failing to respond within this timeout will fail the"
+ " invocation.")
public Duration credentialHelperTimeout;

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

/** One of the values of the `--credential_helper` flag. */
/**
* One of the values of the `--experimental_credential_helper` flag.
*/
@AutoValue
public abstract static class UnresolvedScopedCredentialHelper {
/** Returns the scope of the credential helper (if any). */

/**
* Returns the scope of the credential helper (if any).
*/
public abstract Optional<String> getScope();

/** Returns the (unparsed) path of the credential helper. */
/**
* Returns the (unparsed) path of the credential helper.
*/
public abstract String getPath();
}

/** A {@link Converter} for the `--credential_helper` flag. */
/**
* A {@link Converter} for the `--experimental_credential_helper` flag.
*/
public static final class UnresolvedScopedCredentialHelperConverter
extends Converter.Contextless<UnresolvedScopedCredentialHelper> {

public static final UnresolvedScopedCredentialHelperConverter INSTANCE =
new UnresolvedScopedCredentialHelperConverter();

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

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,6 @@ static Optional<Credentials> newCredentialsFromNetrc(
}
}

@VisibleForTesting
public static CredentialHelperProvider newCredentialHelperProvider(
CredentialHelperEnvironment environment,
CommandLinePathFactory pathFactory,
Expand Down
4 changes: 4 additions & 0 deletions src/main/java/com/google/devtools/build/lib/bazel/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster",
"//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories",
"//src/main/java/com/google/devtools/build/lib/analysis:config/build_configuration",
"//src/main/java/com/google/devtools/build/lib/authandtls",
"//src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper",
"//src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper:credential_module",
"//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:common",
"//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:inspection",
"//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:inspection_impl",
Expand Down Expand Up @@ -55,6 +58,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/starlarkbuildapi/repository",
"//src/main/java/com/google/devtools/build/lib/util:abrupt_exit_exception",
"//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code",
"//src/main/java/com/google/devtools/build/lib/util:exit_code",
"//src/main/java/com/google/devtools/build/lib/vfs",
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
"//src/main/java/com/google/devtools/common/options",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

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

import com.google.common.base.Preconditions;
import com.google.common.base.Strings;
import com.google.common.base.Supplier;
import com.google.common.collect.ImmutableList;
Expand All @@ -25,6 +26,13 @@
import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
import com.google.devtools.build.lib.analysis.RuleDefinition;
import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue;
import com.google.devtools.build.lib.authandtls.AuthAndTLSOptions;
import com.google.devtools.build.lib.authandtls.GoogleAuthUtils;
import com.google.devtools.build.lib.authandtls.StaticCredentials;
import com.google.devtools.build.lib.authandtls.credentialhelper.CredentialHelperCredentials;
import com.google.devtools.build.lib.authandtls.credentialhelper.CredentialHelperEnvironment;
import com.google.devtools.build.lib.authandtls.credentialhelper.CredentialHelperProvider;
import com.google.devtools.build.lib.authandtls.credentialhelper.CredentialModule;
import com.google.devtools.build.lib.bazel.bzlmod.AttributeValues;
import com.google.devtools.build.lib.bazel.bzlmod.BazelDepGraphFunction;
import com.google.devtools.build.lib.bazel.bzlmod.BazelLockFileFunction;
Expand Down Expand Up @@ -110,6 +118,7 @@
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.stream.Collectors;
import javax.annotation.Nullable;

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

@Nullable
private CredentialModule credentialModule;

public BazelRepositoryModule() {
this.starlarkRepositoryFunction = new StarlarkRepositoryFunction(downloadManager);
this.repositoryHandlers = repositoryRules();
Expand Down Expand Up @@ -263,6 +275,8 @@ SkyFunctions.BAZEL_LOCK_FILE, new BazelLockFileFunction(directories.getWorkspace
.addSkyFunction(SkyFunctions.SINGLE_EXTENSION_EVAL, singleExtensionEvalFunction)
.addSkyFunction(SkyFunctions.SINGLE_EXTENSION_USAGES, new SingleExtensionUsagesFunction());
filesystem = runtime.getFileSystem();

credentialModule = Preconditions.checkNotNull(runtime.getBlazeModule(CredentialModule.class));
}

@Override
Expand Down Expand Up @@ -373,6 +387,41 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
Code.BAD_DOWNLOADER_CONFIG));
}

try {
AuthAndTLSOptions authAndTlsOptions = env.getOptions().getOptions(AuthAndTLSOptions.class);
var credentialHelperEnvironment =
CredentialHelperEnvironment.newBuilder()
.setEventReporter(env.getReporter())
.setWorkspacePath(env.getWorkspace())
.setClientEnvironment(env.getClientEnv())
.setHelperExecutionTimeout(authAndTlsOptions.credentialHelperTimeout)
.build();
CredentialHelperProvider credentialHelperProvider =
GoogleAuthUtils.newCredentialHelperProvider(
credentialHelperEnvironment,
env.getCommandLinePathFactory(),
authAndTlsOptions.credentialHelpers);

downloadManager.setCredentialFactory(headers -> {
Preconditions.checkNotNull(headers);

return new CredentialHelperCredentials(
credentialHelperProvider,
credentialHelperEnvironment,
credentialModule.getCredentialCache(),
Optional.of(new StaticCredentials(headers)));
});
} catch (IOException e) {
env.getReporter().handle(Event.error(e.getMessage()));
env.getBlazeModuleEnvironment()
.exit(
new AbruptExitException(
detailedExitCode(
"Error initializing credential helper",
Code.CREDENTIALS_INIT_FAILURE)));
return;
}

if (repoOptions.experimentalDistdir != null) {
downloadManager.setDistdir(
repoOptions.experimentalDistdir.stream()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import com.google.auth.Credentials;
import com.google.common.base.MoreObjects;
import com.google.common.base.Optional;
import com.google.common.base.Preconditions;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
Expand Down Expand Up @@ -61,6 +62,7 @@ public class DownloadManager {
private int retries = 0;
private boolean urlsAsDefaultCanonicalId;
@Nullable private Credentials netrcCreds;
private CredentialFactory credentialFactory = new DefaultCredentialFactory();

public DownloadManager(RepositoryCache repositoryCache, Downloader downloader) {
this.repositoryCache = repositoryCache;
Expand Down Expand Up @@ -92,6 +94,12 @@ public void setNetrcCreds(Credentials netrcCreds) {
this.netrcCreds = netrcCreds;
}

public void setCredentialFactory(CredentialFactory credentialFactory) {
Preconditions.checkNotNull(credentialFactory);

this.credentialFactory = credentialFactory;
}

/**
* Downloads file to disk and returns path.
*
Expand Down Expand Up @@ -257,7 +265,7 @@ public Path download(
try {
downloader.download(
rewrittenUrls,
new StaticCredentials(rewrittenAuthHeaders),
credentialFactory.create(rewrittenAuthHeaders),
checksum,
canonicalId,
destination,
Expand Down Expand Up @@ -338,7 +346,7 @@ public byte[] downloadAndReadOneUrl(
for (int attempt = 0; attempt <= retries; ++attempt) {
try {
return httpDownloader.downloadAndReadOneUrl(
rewrittenUrls.get(0), new StaticCredentials(authHeaders), eventHandler, clientEnv);
rewrittenUrls.get(0), credentialFactory.create(authHeaders), eventHandler, clientEnv);
} catch (ContentLengthMismatchException e) {
if (attempt == retries) {
throw e;
Expand Down Expand Up @@ -427,4 +435,17 @@ public boolean isFinished() {
return isFinished;
}
}

public interface CredentialFactory {
Credentials create(Map<URI, Map<String, List<String>>> authHeaders);
}

private static final class DefaultCredentialFactory implements CredentialFactory {
@Override
public Credentials create(Map<URI, Map<String, List<String>>> authHeaders) {
Preconditions.checkNotNull(authHeaders);

return new StaticCredentials(authHeaders);
}
}
}
1 change: 1 addition & 0 deletions src/main/protobuf/failure_details.proto
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,7 @@ message ExternalRepository {
OVERRIDE_DISALLOWED_MANAGED_DIRECTORIES = 1 [(metadata) = { exit_code: 2 }];
BAD_DOWNLOADER_CONFIG = 2 [(metadata) = { exit_code: 2 }];
REPOSITORY_MAPPING_RESOLUTION_FAILED = 3 [(metadata) = { exit_code: 37 }];
CREDENTIALS_INIT_FAILURE = 4 [(metadata) = { exit_code: 2 }];
}
Code code = 1;
// Additional data could include external repository names.
Expand Down
4 changes: 1 addition & 3 deletions src/test/shell/bazel/remote_helpers.sh
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,7 @@ function serve_file() {
cd -
}

# Serves $1 as a file on localhost:$nc_port insisting on authentication (but
# accepting any credentials.
# Serves $1 as a file on localhost:$nc_port expecting authentication.
# * nc_port - the port nc is listening on.
# * nc_log - the path to nc's log.
# * nc_pid - the PID of nc.
Expand Down Expand Up @@ -280,7 +279,6 @@ fd = os.open(path, os.O_WRONLY|os.O_CREAT|os.O_APPEND)
os.write(fd, b"1")
os.close(fd)
# Must match //src/test/shell/bazel/testing_server.py.
print("""{"headers":{"Authorization":["Bearer TOKEN"]}}""")
EOF
chmod +x "${TEST_TMPDIR}/credhelper"
Expand Down
Loading

0 comments on commit ea84c9c

Please sign in to comment.