Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wire up credential helper to command-line flag(s) #15947

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,41 @@ public class AuthAndTLSOptions extends OptionsBase {
+ "pings are disabled, then this setting is ignored.")
public Duration grpcKeepaliveTimeout;

@Option(
name = "experimental_experimental_credential_helper",
Yannic marked this conversation as resolved.
Show resolved Hide resolved
defaultValue = "null",
allowMultiple = true,
converter = UnresolvedScopedCredentialHelperConverter.class,
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> or <code>.netrc</code>.\n\n"
Yannic marked this conversation as resolved.
Show resolved Hide resolved
+ "See https://github.com/bazelbuild/proposals/blob/main/designs/2022-06-07-bazel-credential-helpers.md for details.")
public List<UnresolvedScopedCredentialHelper> credentialHelpers;

@Option(
name = "credential_helper_timeout",
Yannic marked this conversation as resolved.
Show resolved Hide resolved
defaultValue = "5s",
converter = DurationConverter.class,
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 invocation.")
public Duration credentialHelperTimeout;

@Option(
name = "experimental_credential_helper_cache_duration",
defaultValue = "15m",
Yannic marked this conversation as resolved.
Show resolved Hide resolved
converter = DurationConverter.class,
documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
effectTags = {OptionEffectTag.UNKNOWN},
help = "Configures the duration for which credentials from Credential Helpers are cached.")
public Duration credentialHelperCacheTimeout;

/** One of the values of the `--credential_helper` flag. */
@AutoValue
public abstract static class UnresolvedScopedCredentialHelper {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.base.Strings;
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.events.Event;
Expand Down Expand Up @@ -222,36 +223,48 @@ public static CallCredentialsProvider newCallCredentialsProvider(@Nullable Crede
}

/**
* Create a new {@link Credentials} with following order:
* Create a new {@link Credentials} retrieving call credentials in the following order:
*
* <ol>
* <li>If authentication enabled by flags, use it to create credentials
* <li>Use .netrc to provide credentials if exists
* <li>Otherwise, return {@code null}
* <li>If a Credential Helper is configured for the scope, use the credentials provided by the helper.
* <li>If (Google) authentication is enabled by flags, use it to create credentials.
* <li>Use {@code .netrc} to provide credentials if exists.
* </ol>
*
* @throws IOException in case the credentials can't be constructed.
*/
@Nullable
public static Credentials newCredentials(
Yannic marked this conversation as resolved.
Show resolved Hide resolved
Reporter reporter,
Map<String, String> clientEnv,
CredentialHelperEnvironment credentialHelperEnvironment,
CommandLinePathFactory commandLinePathFactory,
FileSystem fileSystem,
AuthAndTLSOptions authAndTlsOptions)
throws IOException {
Preconditions.checkNotNull(credentialHelperEnvironment);
Preconditions.checkNotNull(commandLinePathFactory);
Preconditions.checkNotNull(fileSystem);
Preconditions.checkNotNull(authAndTlsOptions);

Optional<Credentials> credentials = newGoogleCredentials(authAndTlsOptions);

if (credentials.isEmpty()) {
// Fallback to .netrc if it exists.
try {
credentials = newCredentialsFromNetrc(clientEnv, fileSystem);
credentials = newCredentialsFromNetrc(
credentialHelperEnvironment.getClientEnvironment(), fileSystem);
} catch (IOException e) {
// TODO(yannic): Make this fail the build.
reporter.handle(Event.warn(e.getMessage()));
credentialHelperEnvironment.getEventReporter().handle(Event.warn(e.getMessage()));
}
}

return credentials.orElse(null);
return new CredentialHelperCredentials(
newCredentialHelperProvider(
credentialHelperEnvironment,
commandLinePathFactory,
authAndTlsOptions.credentialHelpers),
credentialHelperEnvironment,
credentials,
authAndTlsOptions.credentialHelperCacheTimeout);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/events",
"//src/main/java/com/google/devtools/build/lib/shell",
"//src/main/java/com/google/devtools/build/lib/vfs",
"//third_party:auth",
"//third_party:auto_value",
"//third_party:caffeine",
"//third_party:error_prone_annotations",
"//third_party:gson",
"//third_party:guava",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
package com.google.devtools.build.lib.authandtls.credentialhelper;

import com.github.benmanes.caffeine.cache.CacheLoader;
import com.github.benmanes.caffeine.cache.Caffeine;
import com.github.benmanes.caffeine.cache.LoadingCache;
import com.google.auth.Credentials;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableMap;
import java.io.IOException;
import java.net.URI;
import java.time.Duration;
import java.util.List;
import java.util.Map;
import java.util.Optional;

/**
* Implementation of {@link Credentials} which fetches credentials by invoking a {@code credential
* helper} as subprocess, falling back to another {@link Credentials} if no suitable helper exists.
*/
public class CredentialHelperCredentials extends Credentials {
private final Optional<Credentials> fallbackCredentials;

private final LoadingCache<URI, GetCredentialsResponse> credentialCache;

public CredentialHelperCredentials(
CredentialHelperProvider credentialHelperProvider,
CredentialHelperEnvironment credentialHelperEnvironment,
Optional<Credentials> fallbackCredentials,
Duration cacheTimeout) {
Preconditions.checkNotNull(credentialHelperProvider);
Preconditions.checkNotNull(credentialHelperEnvironment);
this.fallbackCredentials = Preconditions.checkNotNull(fallbackCredentials);
Preconditions.checkNotNull(cacheTimeout);
Preconditions.checkArgument(!cacheTimeout.isNegative() && !cacheTimeout.isZero(), "Cache timeout must be greater than 0");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to confirm, does the flag parsing logic already reject negative values? (i.e., is this checking for a programmer error or a user error?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


credentialCache =
Caffeine.newBuilder()
.expireAfterWrite(cacheTimeout)
.build(
new CredentialHelperCacheLoader(
credentialHelperProvider, credentialHelperEnvironment));
}

@Override
public String getAuthenticationType() {
if (fallbackCredentials.isPresent()) {
return "credential-helper-with-fallback-" + fallbackCredentials.get().getAuthenticationType();
}

return "credential-helper";
}

@Override
public Map<String, List<String>> getRequestMetadata(URI uri) throws IOException {
Preconditions.checkNotNull(uri);

Optional<Map<String, List<String>>> credentials =
getRequestMetadataFromCredentialHelper(uri);
if (credentials.isPresent()) {
return credentials.get();
}

if (fallbackCredentials.isPresent()) {
return fallbackCredentials.get().getRequestMetadata(uri);
}

return ImmutableMap.of();
}

private Optional<Map<String, List<String>>> getRequestMetadataFromCredentialHelper(URI uri) {
Preconditions.checkNotNull(uri);

GetCredentialsResponse response = credentialCache.get(uri);
if (response == null) {
return Optional.empty();
}

// The cast is needed to convert value type of map from `ImmutableList` to `List`.
return Optional.of((Map)response.getHeaders());
}

@Override
public boolean hasRequestMetadata() {
return true;
}

@Override
public boolean hasRequestMetadataOnly() {
return false;
}

@Override
public void refresh() throws IOException {
if (fallbackCredentials.isPresent()) {
fallbackCredentials.get().refresh();
}

credentialCache.invalidateAll();
}

private static final class CredentialHelperCacheLoader implements CacheLoader<URI, GetCredentialsResponse> {
private final CredentialHelperProvider credentialHelperProvider;
private final CredentialHelperEnvironment credentialHelperEnvironment;

public CredentialHelperCacheLoader(
CredentialHelperProvider credentialHelperProvider,
CredentialHelperEnvironment credentialHelperEnvironment) {
this.credentialHelperProvider = Preconditions.checkNotNull(credentialHelperProvider);
this.credentialHelperEnvironment = Preconditions.checkNotNull(credentialHelperEnvironment);
}

@Override
public GetCredentialsResponse load(URI uri) throws IOException, InterruptedException {
Preconditions.checkNotNull(uri);

Optional<CredentialHelper> maybeCredentialHelper = credentialHelperProvider.findCredentialHelper(uri);
if (!maybeCredentialHelper.isPresent()) {
return null;
}
CredentialHelper credentialHelper = maybeCredentialHelper.get();

return credentialHelper.getCredentials(credentialHelperEnvironment, uri);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib:runtime",
"//src/main/java/com/google/devtools/build/lib/analysis:test/test_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/bugreport",
"//src/main/java/com/google/devtools/build/lib/buildeventservice/client",
"//src/main/java/com/google/devtools/build/lib/buildeventstream",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.authandtls.AuthAndTLSOptions;
import com.google.devtools.build.lib.authandtls.GoogleAuthUtils;
import com.google.devtools.build.lib.authandtls.credentialhelper.CredentialHelperEnvironment;
import com.google.devtools.build.lib.buildeventservice.client.BuildEventServiceClient;
import com.google.devtools.build.lib.buildeventservice.client.BuildEventServiceGrpcClient;
import com.google.devtools.build.lib.runtime.CommandEnvironment;
Expand Down Expand Up @@ -86,8 +87,13 @@ protected BuildEventServiceClient getBesClient(

Credentials credentials =
GoogleAuthUtils.newCredentials(
env.getReporter(),
env.getClientEnv(),
CredentialHelperEnvironment.newBuilder()
.setEventReporter(env.getReporter())
.setWorkspacePath(env.getWorkspace())
.setClientEnvironment(env.getClientEnv())
.setHelperExecutionTimeout(authAndTLSOptions.credentialHelperTimeout)
.build(),
env.getCommandLinePathFactory(),
env.getRuntime().getFileSystem(),
newConfig.authAndTLSOptions());

Expand Down
1 change: 1 addition & 0 deletions src/main/java/com/google/devtools/build/lib/remote/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ java_library(
":abstract_action_input_prefetcher",
"//src/main/java/com/google/devtools/build/lib:build-request-options",
"//src/main/java/com/google/devtools/build/lib:runtime",
"//src/main/java/com/google/devtools/build/lib:runtime/command_line_path_factory",
"//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/lib/actions:action_input_helper",
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import com.google.devtools.build.lib.authandtls.AuthAndTLSOptions;
import com.google.devtools.build.lib.authandtls.CallCredentialsProvider;
import com.google.devtools.build.lib.authandtls.GoogleAuthUtils;
import com.google.devtools.build.lib.authandtls.credentialhelper.CredentialHelperEnvironment;
import com.google.devtools.build.lib.bazel.repository.downloader.Downloader;
import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploader;
import com.google.devtools.build.lib.buildeventstream.LocalFilesArtifactUploader;
Expand Down Expand Up @@ -75,6 +76,7 @@
import com.google.devtools.build.lib.runtime.BuildEventArtifactUploaderFactory;
import com.google.devtools.build.lib.runtime.Command;
import com.google.devtools.build.lib.runtime.CommandEnvironment;
import com.google.devtools.build.lib.runtime.CommandLinePathFactory;
import com.google.devtools.build.lib.runtime.RepositoryRemoteExecutor;
import com.google.devtools.build.lib.runtime.RepositoryRemoteExecutorFactory;
import com.google.devtools.build.lib.runtime.ServerBuilder;
Expand Down Expand Up @@ -214,9 +216,14 @@ private void initHttpAndDiskCache(
try {
creds =
newCredentials(
env.getClientEnv(),
CredentialHelperEnvironment.newBuilder()
.setEventReporter(env.getReporter())
.setWorkspacePath(env.getWorkspace())
.setClientEnvironment(env.getClientEnv())
.setHelperExecutionTimeout(authAndTlsOptions.credentialHelperTimeout)
.build(),
env.getCommandLinePathFactory(),
env.getRuntime().getFileSystem(),
env.getReporter(),
authAndTlsOptions,
remoteOptions);
} catch (IOException e) {
Expand Down Expand Up @@ -429,9 +436,14 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
callCredentialsProvider =
GoogleAuthUtils.newCallCredentialsProvider(
newCredentials(
env.getClientEnv(),
CredentialHelperEnvironment.newBuilder()
.setEventReporter(env.getReporter())
.setWorkspacePath(env.getWorkspace())
.setClientEnvironment(env.getClientEnv())
.setHelperExecutionTimeout(authAndTlsOptions.credentialHelperTimeout)
.build(),
env.getCommandLinePathFactory(),
env.getRuntime().getFileSystem(),
env.getReporter(),
authAndTlsOptions,
remoteOptions));
} catch (IOException e) {
Expand Down Expand Up @@ -1040,26 +1052,32 @@ RemoteActionContextProvider getActionContextProvider() {
}

static Credentials newCredentials(
Map<String, String> clientEnv,
CredentialHelperEnvironment credentialHelperEnvironment,
CommandLinePathFactory commandLinePathFactory,
FileSystem fileSystem,
Reporter reporter,
AuthAndTLSOptions authAndTlsOptions,
RemoteOptions remoteOptions)
throws IOException {
Credentials credentials =
GoogleAuthUtils.newCredentials(reporter, clientEnv, fileSystem, authAndTlsOptions);
GoogleAuthUtils.newCredentials(
credentialHelperEnvironment,
commandLinePathFactory,
fileSystem,
authAndTlsOptions);

try {
if (credentials != null
&& remoteOptions.remoteCache != null
&& Ascii.toLowerCase(remoteOptions.remoteCache).startsWith("http://")
&& !credentials.getRequestMetadata(new URI(remoteOptions.remoteCache)).isEmpty()) {
// TODO(yannic): Make this a error aborting the build.
reporter.handle(
Event.warn(
"Credentials are transmitted in plaintext to "
+ remoteOptions.remoteCache
+ ". Please consider using an HTTPS endpoint."));
credentialHelperEnvironment
.getEventReporter()
.handle(
Event.warn(
"Credentials are transmitted in plaintext to "
+ remoteOptions.remoteCache
+ ". Please consider using an HTTPS endpoint."));
}
} catch (URISyntaxException e) {
throw new IOException(e.getMessage(), e);
Expand Down
2 changes: 2 additions & 0 deletions src/test/java/com/google/devtools/build/lib/remote/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ java_test(
test_class = "com.google.devtools.build.lib.AllTests",
deps = [
"//src/main/java/com/google/devtools/build/lib:runtime",
"//src/main/java/com/google/devtools/build/lib:runtime/command_line_path_factory",
"//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/lib/actions:action_input_helper",
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
Expand All @@ -53,6 +54,7 @@ java_test(
"//src/main/java/com/google/devtools/build/lib/analysis:config/core_options",
"//src/main/java/com/google/devtools/build/lib/analysis:server_directories",
"//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/buildeventstream",
"//src/main/java/com/google/devtools/build/lib/clock",
"//src/main/java/com/google/devtools/build/lib/collect/nestedset",
Expand Down
Loading