From e0c3e380e6ab4f3bf148cd7d89576c80849762fe Mon Sep 17 00:00:00 2001 From: Yannic Bonenberger Date: Mon, 7 Nov 2022 19:33:14 +0100 Subject: [PATCH] [remote/downloader] Wire credential helper to repository downloads Adapted from https://github.com/bazelbuild/bazel/pull/16682 Progress on https://github.com/bazelbuild/bazel/pull/16595 --- .../build/lib/authandtls/GoogleAuthUtils.java | 1 - .../com/google/devtools/build/lib/bazel/BUILD | 4 ++ .../lib/bazel/BazelRepositoryModule.java | 49 ++++++++++++++++ .../downloader/DownloadManager.java | 25 ++++++++- src/main/protobuf/failure_details.proto | 1 + src/test/shell/bazel/remote_helpers.sh | 3 +- .../shell/bazel/starlark_repository_test.sh | 56 +++++++++++++++++++ src/test/shell/bazel/testing_server.py | 2 +- 8 files changed, 135 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/authandtls/GoogleAuthUtils.java b/src/main/java/com/google/devtools/build/lib/authandtls/GoogleAuthUtils.java index e226e6b069db20..409263929c7cdc 100644 --- a/src/main/java/com/google/devtools/build/lib/authandtls/GoogleAuthUtils.java +++ b/src/main/java/com/google/devtools/build/lib/authandtls/GoogleAuthUtils.java @@ -372,7 +372,6 @@ static Optional newCredentialsFromNetrc( } } - @VisibleForTesting public static CredentialHelperProvider newCredentialHelperProvider( CredentialHelperEnvironment environment, CommandLinePathFactory pathFactory, diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BUILD b/src/main/java/com/google/devtools/build/lib/bazel/BUILD index ba22a5e4b5458d..9bc55920446063 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/BUILD +++ b/src/main/java/com/google/devtools/build/lib/bazel/BUILD @@ -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", @@ -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", diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java b/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java index 05e3b66b115d74..e17917667072c2 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java @@ -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; @@ -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; @@ -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 { @@ -146,6 +155,9 @@ public class BazelRepositoryModule extends BlazeModule { private List allowedYankedVersions = ImmutableList.of(); private SingleExtensionEvalFunction singleExtensionEvalFunction; + @Nullable + private CredentialModule credentialModule; + public BazelRepositoryModule() { this.starlarkRepositoryFunction = new StarlarkRepositoryFunction(downloadManager); this.repositoryHandlers = repositoryRules(); @@ -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 @@ -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() diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/DownloadManager.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/DownloadManager.java index 05250dc08608a1..4b20924164f532 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/DownloadManager.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/DownloadManager.java @@ -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; @@ -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; @@ -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. * @@ -257,7 +265,7 @@ public Path download( try { downloader.download( rewrittenUrls, - new StaticCredentials(rewrittenAuthHeaders), + credentialFactory.create(rewrittenAuthHeaders), checksum, canonicalId, destination, @@ -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; @@ -427,4 +435,17 @@ public boolean isFinished() { return isFinished; } } + + public interface CredentialFactory { + Credentials create(Map>> authHeaders); + } + + private static final class DefaultCredentialFactory implements CredentialFactory { + @Override + public Credentials create(Map>> authHeaders) { + Preconditions.checkNotNull(authHeaders); + + return new StaticCredentials(authHeaders); + } + } } diff --git a/src/main/protobuf/failure_details.proto b/src/main/protobuf/failure_details.proto index ec3c67ea41e940..8646a14e54b7da 100644 --- a/src/main/protobuf/failure_details.proto +++ b/src/main/protobuf/failure_details.proto @@ -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. diff --git a/src/test/shell/bazel/remote_helpers.sh b/src/test/shell/bazel/remote_helpers.sh index 58acd7b2b1b334..3f9af8c5217184 100755 --- a/src/test/shell/bazel/remote_helpers.sh +++ b/src/test/shell/bazel/remote_helpers.sh @@ -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. diff --git a/src/test/shell/bazel/starlark_repository_test.sh b/src/test/shell/bazel/starlark_repository_test.sh index f18607b449be0a..775b3cb6b7ea98 100755 --- a/src/test/shell/bazel/starlark_repository_test.sh +++ b/src/test/shell/bazel/starlark_repository_test.sh @@ -1757,6 +1757,62 @@ EOF || fail "Expected success despite needing a file behind basic auth" } +function test_credential_helper() { + # Each helper call atomically writes one byte to this file. + # We can later read the file to determine how many calls were made. + touch "${TEST_TMPDIR}/credhelper_log" + + cat > "${TEST_TMPDIR}/credhelper" <<'EOF' +#!/usr/bin/env python3 +import os + +path = os.path.join(os.environ["TEST_TMPDIR"], "credhelper_log") +fd = os.open(path, os.O_WRONLY|os.O_CREAT|os.O_APPEND) +os.write(fd, b"1") +os.close(fd) + +print("""{"headers":{"Authorization":["Bearer TOKEN"]}}""") +EOF + chmod +x "${TEST_TMPDIR}/credhelper" + + 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) < auth.bzl <<'EOF' +def _impl(ctx): + ctx.download_and_extract( + url = ctx.attr.url, + sha256 = ctx.attr.sha256, + ) + +with_auth = repository_rule( + implementation = _impl, + attrs = { "url" : attr.string(), "sha256" : attr.string() } +) +EOF + cat > BUILD <<'EOF' +genrule( + name = "it", + srcs = ["@ext//x:file.txt"], + outs = ["it.txt"], + cmd = "cp $< $@", +) +EOF + bazel build --experimental_credential_helper="${TEST_TMPDIR}/credhelper" \ + //:it || fail "Expected success despite needing a file behind basic auth" +} + function test_netrc_reading() { # Write a badly formatted, but correct, .netrc file cat > .netrc <<'EOF' diff --git a/src/test/shell/bazel/testing_server.py b/src/test/shell/bazel/testing_server.py index 9d6cd23a57ee88..d311ef1ff729df 100644 --- a/src/test/shell/bazel/testing_server.py +++ b/src/test/shell/bazel/testing_server.py @@ -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:]