Skip to content

Commit

Permalink
[remote/downloader] Wire credential helper to repository downloads
Browse files Browse the repository at this point in the history
Adapted from #16682

Progress on #16595
  • Loading branch information
Yannic authored and tjgq committed Apr 28, 2023
1 parent 1b66046 commit abe83bd
Show file tree
Hide file tree
Showing 8 changed files with 129 additions and 22 deletions.
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
3 changes: 1 addition & 2 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
66 changes: 50 additions & 16 deletions src/test/shell/bazel/starlark_repository_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -1713,38 +1713,56 @@ EOF
expect_log "//:b.bzl"
}

function test_auth_provided() {
# Common setup logic for test_auth_*.
# Argument determines whether to pass `auth` to repository rule.
function setup_auth() {
local -r use_auth=$1

mkdir x
echo 'exports_files(["file.txt"])' > x/BUILD
echo 'Hello World' > x/file.txt
tar cvf x.tar x
sha256="$(sha256sum x.tar | head -c 64)"
serve_file_auth x.tar
cat >> $(create_workspace_with_default_repos WORKSPACE) <<EOF
load("//:auth.bzl", "with_auth")
with_auth(
name="ext",
url = "http://127.0.0.1:$nc_port/x.tar",
sha256 = "$sha256",
)
EOF

cat > auth.bzl <<'EOF'
def _impl(ctx):
# Use the username/password pair hard-coded
# in the testing server.
auth = {
ctx.attr.url: {
"type": "basic",
"login" : "foo",
"password" : "bar",
},
}
ctx.download_and_extract(
url = ctx.attr.url,
sha256 = ctx.attr.sha256,
# Use the username/password pair hard-coded
# in the testing server.
auth = {ctx.attr.url : { "type": "basic",
"login" : "foo",
"password" : "bar"}}
auth = auth if ctx.attr.use_auth else {},
)
with_auth = repository_rule(
implementation = _impl,
attrs = { "url" : attr.string(), "sha256" : attr.string() }
attrs = {
"url" : attr.string(),
"sha256" : attr.string(),
"use_auth": attr.bool(),
}
)
EOF

cat >> $(create_workspace_with_default_repos WORKSPACE) <<EOF
load("//:auth.bzl", "with_auth")
with_auth(
name="ext",
url = "http://127.0.0.1:$nc_port/x.tar",
sha256 = "$sha256",
use_auth = $use_auth,
)
EOF

cat > BUILD <<'EOF'
genrule(
name = "it",
Expand All @@ -1753,8 +1771,24 @@ genrule(
cmd = "cp $< $@",
)
EOF
}

function test_auth_via_starlark() {
setup_auth True

bazel build //:it \
|| fail "Expected success despite needing a file behind basic auth"
|| fail "Expected success when downloading repo with basic auth"
}

function test_auth_via_credential_helper() {
setup_credential_helper

setup_auth False

bazel build --experimental_credential_helper="${TEST_TMPDIR}/credhelper" \
//:it || fail "Expected success when downloading repo with credential helper"

expect_credential_helper_calls 1
}

function test_netrc_reading() {
Expand Down
2 changes: 1 addition & 1 deletion src/test/shell/bazel/testing_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ def do_GET(self): # pylint: disable=invalid-name
self.serve_file()
else:
self.do_AUTHHEAD()
self.wfile.write(b'Login required.' + str(auth_header))
self.wfile.write("Bad authorization header: {}".format(auth_header).encode("ascii"))

def serve_file(self):
path_to_serve = self.path[1:]
Expand Down

0 comments on commit abe83bd

Please sign in to comment.