From 2baa5a4284467d70f1b31ea779b79d38ef07d164 Mon Sep 17 00:00:00 2001 From: Tiago Quelhas Date: Tue, 29 Nov 2022 04:21:40 -0800 Subject: [PATCH] Keep credentials cached across build commands. When using a credential helper, the lifetime of the credential cache is currently tied to an individual command, which causes the helper to be called for every command resulting in poor incremental build latency for builds using a non-trivial helper. Since the cache must be shared by RemoteModule and BazelBuildServiceModule, I've introduced a new CredentialModule whose sole purpose is to provide access to it. Closes #16822. PiperOrigin-RevId: 491598103 Change-Id: Ib668954b635a0e9498f0a7418707d6a2dfae0265 --- .../lib/authandtls/AuthAndTLSOptions.java | 6 +- .../devtools/build/lib/authandtls/BUILD | 1 + .../build/lib/authandtls/GoogleAuthUtils.java | 15 ++- .../lib/authandtls/credentialhelper/BUILD | 16 ++- .../credentialhelper/CredentialHelper.java | 13 ++- .../CredentialHelperCredentials.java | 106 +++++++++--------- .../credentialhelper/CredentialModule.java | 52 +++++++++ .../com/google/devtools/build/lib/bazel/BUILD | 1 + .../devtools/build/lib/bazel/Bazel.java | 3 + .../build/lib/buildeventservice/BUILD | 2 + .../BazelBuildEventServiceModule.java | 14 +++ .../google/devtools/build/lib/remote/BUILD | 1 + .../build/lib/remote/RemoteModule.java | 94 ++++++++-------- .../build/lib/buildeventservice/BUILD | 1 + .../BazelBuildEventServiceModuleTest.java | 2 + .../google/devtools/build/lib/buildtool/BUILD | 1 + .../buildtool/TargetCompleteEventTest.java | 2 + .../google/devtools/build/lib/remote/BUILD | 4 + .../BuildWithoutTheBytesIntegrationTest.java | 2 + .../lib/remote/DiskCacheIntegrationTest.java | 2 + .../build/lib/remote/RemoteModuleTest.java | 34 ++++-- .../bazel/remote/remote_execution_test.sh | 102 +++++++++++++---- 22 files changed, 328 insertions(+), 146 deletions(-) create mode 100644 src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialModule.java diff --git a/src/main/java/com/google/devtools/build/lib/authandtls/AuthAndTLSOptions.java b/src/main/java/com/google/devtools/build/lib/authandtls/AuthAndTLSOptions.java index 8f3aae8ed30980..7423b555a13e9c 100644 --- a/src/main/java/com/google/devtools/build/lib/authandtls/AuthAndTLSOptions.java +++ b/src/main/java/com/google/devtools/build/lib/authandtls/AuthAndTLSOptions.java @@ -175,7 +175,11 @@ public class AuthAndTLSOptions extends OptionsBase { converter = DurationConverter.class, documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, effectTags = {OptionEffectTag.UNKNOWN}, - help = "Configures the duration for which credentials from Credential Helpers are cached.") + help = + "Configures the duration for which credentials from Credential Helpers 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. */ diff --git a/src/main/java/com/google/devtools/build/lib/authandtls/BUILD b/src/main/java/com/google/devtools/build/lib/authandtls/BUILD index 252e6587c23c43..d220892d066b1b 100644 --- a/src/main/java/com/google/devtools/build/lib/authandtls/BUILD +++ b/src/main/java/com/google/devtools/build/lib/authandtls/BUILD @@ -22,6 +22,7 @@ java_library( "//src/main/java/com/google/devtools/common/options", "//third_party:auth", "//third_party:auto_value", + "//third_party:caffeine", "//third_party:guava", "//third_party:jsr305", "//third_party:netty", 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 fa4b81da46e7e3..e226e6b069db20 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 @@ -14,11 +14,14 @@ package com.google.devtools.build.lib.authandtls; +import com.github.benmanes.caffeine.cache.Cache; import com.google.auth.Credentials; import com.google.auth.oauth2.GoogleCredentials; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.base.Strings; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; 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; @@ -48,6 +51,7 @@ import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; +import java.net.URI; import java.util.List; import java.util.Map; import java.util.Optional; @@ -248,6 +252,7 @@ public static CallCredentialsProvider newCallCredentialsProvider(@Nullable Crede */ public static Credentials newCredentials( CredentialHelperEnvironment credentialHelperEnvironment, + Cache>> credentialCache, CommandLinePathFactory commandLinePathFactory, FileSystem fileSystem, AuthAndTLSOptions authAndTlsOptions) @@ -257,12 +262,12 @@ public static Credentials newCredentials( Preconditions.checkNotNull(fileSystem); Preconditions.checkNotNull(authAndTlsOptions); - Optional credentials = newGoogleCredentials(authAndTlsOptions); + Optional fallbackCredentials = newGoogleCredentials(authAndTlsOptions); - if (credentials.isEmpty()) { + if (fallbackCredentials.isEmpty()) { // Fallback to .netrc if it exists. try { - credentials = + fallbackCredentials = newCredentialsFromNetrc(credentialHelperEnvironment.getClientEnvironment(), fileSystem); } catch (IOException e) { // TODO(yannic): Make this fail the build. @@ -276,8 +281,8 @@ public static Credentials newCredentials( commandLinePathFactory, authAndTlsOptions.credentialHelpers), credentialHelperEnvironment, - credentials, - authAndTlsOptions.credentialHelperCacheTimeout); + credentialCache, + fallbackCredentials); } /** diff --git a/src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/BUILD b/src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/BUILD index b14a03cf3e437d..275c159b60087c 100644 --- a/src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/BUILD +++ b/src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/BUILD @@ -8,9 +8,23 @@ filegroup( visibility = ["//src:__subpackages__"], ) +java_library( + name = "credential_module", + srcs = ["CredentialModule.java"], + deps = [ + "//src/main/java/com/google/devtools/build/lib:runtime", + "//src/main/java/com/google/devtools/build/lib/authandtls", + "//third_party:caffeine", + "//third_party:guava", + ], +) + java_library( name = "credentialhelper", - srcs = glob(["*.java"]), + srcs = glob( + ["*.java"], + exclude = ["CredentialModule.java"], + ), deps = [ "//src/main/java/com/google/devtools/build/lib/events", "//src/main/java/com/google/devtools/build/lib/profiler", diff --git a/src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialHelper.java b/src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialHelper.java index 2219a595741fa4..c1f0a09f12bb7f 100644 --- a/src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialHelper.java +++ b/src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialHelper.java @@ -67,7 +67,7 @@ public Path getPath() { * @return The response from the subprocess. */ public GetCredentialsResponse getCredentials(CredentialHelperEnvironment environment, URI uri) - throws InterruptedException, IOException { + throws IOException { Preconditions.checkNotNull(environment); Preconditions.checkNotNull(uri); @@ -81,7 +81,16 @@ public GetCredentialsResponse getCredentials(CredentialHelperEnvironment environ GSON.toJson(GetCredentialsRequest.newBuilder().setUri(uri).build(), stdin); } - process.waitFor(); + try { + process.waitFor(); + } catch (InterruptedException e) { + throw new CredentialHelperException( + String.format( + Locale.US, + "Failed to get credentials for '%s' from helper '%s': process was interrupted", + uri, + path)); + } if (process.timedout()) { throw new CredentialHelperException( diff --git a/src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialHelperCredentials.java b/src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialHelperCredentials.java index ecc40e15707d8d..5de760857dd1bf 100644 --- a/src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialHelperCredentials.java +++ b/src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialHelperCredentials.java @@ -14,15 +14,13 @@ 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.github.benmanes.caffeine.cache.Cache; import com.google.auth.Credentials; import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableList; 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; @@ -33,29 +31,34 @@ * helper} as subprocess, falling back to another {@link Credentials} if no suitable helper exists. */ public class CredentialHelperCredentials extends Credentials { + private final CredentialHelperProvider credentialHelperProvider; + private final CredentialHelperEnvironment credentialHelperEnvironment; + private final Cache>> credentialCache; private final Optional fallbackCredentials; - private final LoadingCache credentialCache; + /** Wraps around an {@link IOException} so we can smuggle it through {@link Cache#get}. */ + public static final class WrappedIOException extends RuntimeException { + private final IOException wrapped; + + WrappedIOException(IOException e) { + super(e); + this.wrapped = e; + } + + IOException getWrapped() { + return wrapped; + } + } public CredentialHelperCredentials( CredentialHelperProvider credentialHelperProvider, CredentialHelperEnvironment credentialHelperEnvironment, - Optional fallbackCredentials, - Duration cacheTimeout) { - Preconditions.checkNotNull(credentialHelperProvider); - Preconditions.checkNotNull(credentialHelperEnvironment); + Cache>> credentialCache, + Optional fallbackCredentials) { + this.credentialHelperProvider = Preconditions.checkNotNull(credentialHelperProvider); + this.credentialHelperEnvironment = Preconditions.checkNotNull(credentialHelperEnvironment); + this.credentialCache = Preconditions.checkNotNull(credentialCache); this.fallbackCredentials = Preconditions.checkNotNull(fallbackCredentials); - Preconditions.checkNotNull(cacheTimeout); - Preconditions.checkArgument( - !cacheTimeout.isNegative() && !cacheTimeout.isZero(), - "Cache timeout must be greater than 0"); - - credentialCache = - Caffeine.newBuilder() - .expireAfterWrite(cacheTimeout) - .build( - new CredentialHelperCacheLoader( - credentialHelperProvider, credentialHelperEnvironment)); } @Override @@ -68,12 +71,18 @@ public String getAuthenticationType() { } @Override + @SuppressWarnings("unchecked") // Map> to Map> public Map> getRequestMetadata(URI uri) throws IOException { Preconditions.checkNotNull(uri); - Optional>> credentials = getRequestMetadataFromCredentialHelper(uri); - if (credentials.isPresent()) { - return credentials.get(); + ImmutableMap> credentials; + try { + credentials = credentialCache.get(uri, this::getCredentialsFromHelper); + } catch (WrappedIOException e) { + throw e.getWrapped(); + } + if (credentials != null) { + return (Map) credentials; } if (fallbackCredentials.isPresent()) { @@ -83,13 +92,28 @@ public Map> getRequestMetadata(URI uri) throws IOException return ImmutableMap.of(); } - @SuppressWarnings("unchecked") // Map> to Map> - private Optional>> getRequestMetadataFromCredentialHelper(URI uri) { + @Nullable + private ImmutableMap> getCredentialsFromHelper(URI uri) { Preconditions.checkNotNull(uri); - GetCredentialsResponse response = credentialCache.get(uri); + Optional maybeCredentialHelper = + credentialHelperProvider.findCredentialHelper(uri); + if (maybeCredentialHelper.isEmpty()) { + return null; + } + CredentialHelper credentialHelper = maybeCredentialHelper.get(); + + GetCredentialsResponse response; + try { + response = credentialHelper.getCredentials(credentialHelperEnvironment, uri); + } catch (IOException e) { + throw new WrappedIOException(e); + } + if (response == null) { + return null; + } - return Optional.ofNullable(response).map(value -> (Map) value.getHeaders()); + return response.getHeaders(); } @Override @@ -110,32 +134,4 @@ public void refresh() throws IOException { credentialCache.invalidateAll(); } - - private static final class CredentialHelperCacheLoader - implements CacheLoader { - private final CredentialHelperProvider credentialHelperProvider; - private final CredentialHelperEnvironment credentialHelperEnvironment; - - public CredentialHelperCacheLoader( - CredentialHelperProvider credentialHelperProvider, - CredentialHelperEnvironment credentialHelperEnvironment) { - this.credentialHelperProvider = Preconditions.checkNotNull(credentialHelperProvider); - this.credentialHelperEnvironment = Preconditions.checkNotNull(credentialHelperEnvironment); - } - - @Nullable - @Override - public GetCredentialsResponse load(URI uri) throws IOException, InterruptedException { - Preconditions.checkNotNull(uri); - - Optional maybeCredentialHelper = - credentialHelperProvider.findCredentialHelper(uri); - if (maybeCredentialHelper.isEmpty()) { - return null; - } - CredentialHelper credentialHelper = maybeCredentialHelper.get(); - - return credentialHelper.getCredentials(credentialHelperEnvironment, uri); - } - } } diff --git a/src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialModule.java b/src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialModule.java new file mode 100644 index 00000000000000..95af2af5fcced0 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialModule.java @@ -0,0 +1,52 @@ +// Copyright 2022 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.devtools.build.lib.authandtls.credentialhelper; + +import com.github.benmanes.caffeine.cache.Cache; +import com.github.benmanes.caffeine.cache.Caffeine; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.devtools.build.lib.authandtls.AuthAndTLSOptions; +import com.google.devtools.build.lib.runtime.BlazeModule; +import com.google.devtools.build.lib.runtime.CommandEnvironment; +import java.net.URI; +import java.time.Duration; + +/** A module whose sole purpose is to hold the credential cache which is shared by other modules. */ +public class CredentialModule extends BlazeModule { + private final Cache>> credentialCache = + Caffeine.newBuilder().expireAfterWrite(Duration.ZERO).build(); + + /** Returns the credential cache. */ + public Cache>> getCredentialCache() { + return credentialCache; + } + + @Override + public void beforeCommand(CommandEnvironment env) { + // Update the cache expiration policy according to the command options. + AuthAndTLSOptions authAndTlsOptions = env.getOptions().getOptions(AuthAndTLSOptions.class); + credentialCache + .policy() + .expireAfterWrite() + .get() + .setExpiresAfter(authAndTlsOptions.credentialHelperCacheTimeout); + + // Clear the cache on clean. + if (env.getCommand().name().equals("clean")) { + credentialCache.invalidateAll(); + } + } +} 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 499a038e030eb7..550425dbe3f274 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/BUILD +++ b/src/main/java/com/google/devtools/build/lib/bazel/BUILD @@ -136,6 +136,7 @@ java_library( ":spawn_log_module", "//src/main/java/com/google/devtools/build/lib:runtime", "//src/main/java/com/google/devtools/build/lib/analysis:blaze_version_info", + "//src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper:credential_module", "//src/main/java/com/google/devtools/build/lib/bazel/coverage", "//src/main/java/com/google/devtools/build/lib/bazel/debug:workspace-rule-module", "//src/main/java/com/google/devtools/build/lib/bazel/repository", diff --git a/src/main/java/com/google/devtools/build/lib/bazel/Bazel.java b/src/main/java/com/google/devtools/build/lib/bazel/Bazel.java index fb10eddbea35af..e35b11a8386f5d 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/Bazel.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/Bazel.java @@ -16,6 +16,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.analysis.BlazeVersionInfo; +import com.google.devtools.build.lib.authandtls.credentialhelper.CredentialModule; import com.google.devtools.build.lib.runtime.BlazeModule; import com.google.devtools.build.lib.runtime.BlazeRuntime; import java.io.IOException; @@ -42,6 +43,8 @@ public final class Bazel { // This module needs to be registered before any module providing a SpawnCache // implementation. com.google.devtools.build.lib.runtime.NoSpawnCacheModule.class, + // This module needs to be registered before any module that uses the credential cache. + CredentialModule.class, com.google.devtools.build.lib.runtime.CommandLogModule.class, com.google.devtools.build.lib.runtime.MemoryPressureModule.class, com.google.devtools.build.lib.platform.SleepPreventionModule.class, diff --git a/src/main/java/com/google/devtools/build/lib/buildeventservice/BUILD b/src/main/java/com/google/devtools/build/lib/buildeventservice/BUILD index bde58e545a8739..6c61f671a71326 100644 --- a/src/main/java/com/google/devtools/build/lib/buildeventservice/BUILD +++ b/src/main/java/com/google/devtools/build/lib/buildeventservice/BUILD @@ -38,9 +38,11 @@ java_library( ":buildeventservice-options", "//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/analysis:blaze_directories", "//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/authandtls/credentialhelper:credential_module", "//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", diff --git a/src/main/java/com/google/devtools/build/lib/buildeventservice/BazelBuildEventServiceModule.java b/src/main/java/com/google/devtools/build/lib/buildeventservice/BazelBuildEventServiceModule.java index ac876b0399819f..3889ce01e673ba 100644 --- a/src/main/java/com/google/devtools/build/lib/buildeventservice/BazelBuildEventServiceModule.java +++ b/src/main/java/com/google/devtools/build/lib/buildeventservice/BazelBuildEventServiceModule.java @@ -22,12 +22,16 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; +import com.google.devtools.build.lib.analysis.BlazeDirectories; 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.authandtls.credentialhelper.CredentialModule; 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.BlazeRuntime; import com.google.devtools.build.lib.runtime.CommandEnvironment; +import com.google.devtools.build.lib.runtime.WorkspaceBuilder; import io.grpc.ClientInterceptor; import io.grpc.ManagedChannel; import io.grpc.Metadata; @@ -68,6 +72,15 @@ static BackendConfig create( private BuildEventServiceClient client; private BackendConfig config; + private CredentialModule credentialModule; + + @Override + public void workspaceInit( + BlazeRuntime runtime, BlazeDirectories directories, WorkspaceBuilder builder) { + Preconditions.checkState(credentialModule == null, "credentialModule must be null"); + credentialModule = Preconditions.checkNotNull(runtime.getBlazeModule(CredentialModule.class)); + } + @Override protected Class optionsClass() { return BuildEventServiceOptions.class; @@ -93,6 +106,7 @@ protected BuildEventServiceClient getBesClient( .setClientEnvironment(env.getClientEnv()) .setHelperExecutionTimeout(authAndTLSOptions.credentialHelperTimeout) .build(), + credentialModule.getCredentialCache(), env.getCommandLinePathFactory(), env.getRuntime().getFileSystem(), newConfig.authAndTLSOptions()); diff --git a/src/main/java/com/google/devtools/build/lib/remote/BUILD b/src/main/java/com/google/devtools/build/lib/remote/BUILD index 6a3292448d9b83..6a844d33dd3378 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/BUILD +++ b/src/main/java/com/google/devtools/build/lib/remote/BUILD @@ -63,6 +63,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/analysis/platform:platform_utils", "//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/repository/downloader", "//src/main/java/com/google/devtools/build/lib/buildeventstream", "//src/main/java/com/google/devtools/build/lib/clock", diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java index b57d116f99b25b..74013a19ba94a7 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java @@ -21,6 +21,7 @@ import build.bazel.remote.execution.v2.DigestFunction; import build.bazel.remote.execution.v2.ServerCapabilities; import build.bazel.remote.execution.v2.SymlinkAbsolutePathStrategy; +import com.github.benmanes.caffeine.cache.Cache; import com.google.auth.Credentials; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Ascii; @@ -28,6 +29,7 @@ import com.google.common.base.Strings; import com.google.common.base.Throwables; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.common.util.concurrent.ListeningScheduledExecutorService; import com.google.common.util.concurrent.MoreExecutors; import com.google.common.util.concurrent.ThreadFactoryBuilder; @@ -42,6 +44,7 @@ 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.authandtls.credentialhelper.CredentialModule; import com.google.devtools.build.lib.bazel.repository.downloader.Downloader; import com.google.devtools.build.lib.bazel.repository.downloader.HttpDownloader; import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploader; @@ -159,6 +162,8 @@ public ManagedChannel newChannel( private final MutableSupplier remoteDownloaderSupplier = new MutableSupplier<>(); + private CredentialModule credentialModule; + @Override public void serverInit(OptionsParsingResult startupOptions, ServerBuilder builder) { builder.addBuildEventArtifactUploaderFactory( @@ -211,33 +216,16 @@ private static ServerCapabilities getAndVerifyServerCapabilities( private void initHttpAndDiskCache( CommandEnvironment env, + Credentials credentials, AuthAndTLSOptions authAndTlsOptions, RemoteOptions remoteOptions, DigestUtil digestUtil) { - Credentials creds; - try { - creds = - newCredentials( - CredentialHelperEnvironment.newBuilder() - .setEventReporter(env.getReporter()) - .setWorkspacePath(env.getWorkspace()) - .setClientEnvironment(env.getClientEnv()) - .setHelperExecutionTimeout(authAndTlsOptions.credentialHelperTimeout) - .build(), - env.getCommandLinePathFactory(), - env.getRuntime().getFileSystem(), - authAndTlsOptions, - remoteOptions); - } catch (IOException e) { - handleInitFailure(env, e, Code.CREDENTIALS_INIT_FAILURE); - return; - } RemoteCacheClient cacheClient; try { cacheClient = RemoteCacheClientFactory.create( remoteOptions, - creds, + credentials, authAndTlsOptions, Preconditions.checkNotNull(env.getWorkingDirectory(), "workingDirectory"), digestUtil); @@ -256,8 +244,10 @@ private void initHttpAndDiskCache( public void workspaceInit( BlazeRuntime runtime, BlazeDirectories directories, WorkspaceBuilder builder) { Preconditions.checkState(blockWaitingModule == null, "blockWaitingModule must be null"); + Preconditions.checkState(credentialModule == null, "credentialModule must be null"); blockWaitingModule = Preconditions.checkNotNull(runtime.getBlazeModule(BlockWaitingModule.class)); + credentialModule = Preconditions.checkNotNull(runtime.getBlazeModule(CredentialModule.class)); } @Override @@ -358,8 +348,28 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException { executorService = Executors.newCachedThreadPool(threadFactory); } + Credentials credentials; + try { + credentials = + createCredentials( + CredentialHelperEnvironment.newBuilder() + .setEventReporter(env.getReporter()) + .setWorkspacePath(env.getWorkspace()) + .setClientEnvironment(env.getClientEnv()) + .setHelperExecutionTimeout(authAndTlsOptions.credentialHelperTimeout) + .build(), + credentialModule.getCredentialCache(), + env.getCommandLinePathFactory(), + env.getRuntime().getFileSystem(), + authAndTlsOptions, + remoteOptions); + } catch (IOException e) { + handleInitFailure(env, e, Code.CREDENTIALS_INIT_FAILURE); + return; + } + if ((enableHttpCache || enableDiskCache) && !enableGrpcCache) { - initHttpAndDiskCache(env, authAndTlsOptions, remoteOptions, digestUtil); + initHttpAndDiskCache(env, credentials, authAndTlsOptions, remoteOptions, digestUtil); return; } @@ -456,27 +466,9 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException { } } - CallCredentialsProvider callCredentialsProvider; - try { - callCredentialsProvider = - GoogleAuthUtils.newCallCredentialsProvider( - newCredentials( - CredentialHelperEnvironment.newBuilder() - .setEventReporter(env.getReporter()) - .setWorkspacePath(env.getWorkspace()) - .setClientEnvironment(env.getClientEnv()) - .setHelperExecutionTimeout(authAndTlsOptions.credentialHelperTimeout) - .build(), - env.getCommandLinePathFactory(), - env.getRuntime().getFileSystem(), - authAndTlsOptions, - remoteOptions)); - } catch (IOException e) { - handleInitFailure(env, e, Code.CREDENTIALS_INIT_FAILURE); - return; - } - - CallCredentials credentials = callCredentialsProvider.getCallCredentials(); + CallCredentialsProvider callCredentialsProvider = + GoogleAuthUtils.newCallCredentialsProvider(credentials); + CallCredentials callCredentials = callCredentialsProvider.getCallCredentials(); RemoteRetrier retrier = new RemoteRetrier( @@ -500,7 +492,7 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException { getAndVerifyServerCapabilities( remoteOptions, execChannel, - credentials, + callCredentials, retrier, env, digestUtil, @@ -509,7 +501,7 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException { getAndVerifyServerCapabilities( remoteOptions, cacheChannel, - credentials, + callCredentials, retrier, env, digestUtil, @@ -519,7 +511,7 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException { getAndVerifyServerCapabilities( remoteOptions, execChannel, - credentials, + callCredentials, retrier, env, digestUtil, @@ -530,7 +522,7 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException { getAndVerifyServerCapabilities( remoteOptions, cacheChannel, - credentials, + callCredentials, retrier, env, digestUtil, @@ -683,7 +675,7 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException { buildRequestId, invocationId, downloaderChannel.retain(), - Optional.ofNullable(credentials), + Optional.ofNullable(callCredentials), retrier, cacheClient, remoteOptions, @@ -1084,8 +1076,10 @@ RemoteActionContextProvider getActionContextProvider() { return actionContextProvider; } - static Credentials newCredentials( + @VisibleForTesting + static Credentials createCredentials( CredentialHelperEnvironment credentialHelperEnvironment, + Cache>> credentialCache, CommandLinePathFactory commandLinePathFactory, FileSystem fileSystem, AuthAndTLSOptions authAndTlsOptions, @@ -1093,7 +1087,11 @@ static Credentials newCredentials( throws IOException { Credentials credentials = GoogleAuthUtils.newCredentials( - credentialHelperEnvironment, commandLinePathFactory, fileSystem, authAndTlsOptions); + credentialHelperEnvironment, + credentialCache, + commandLinePathFactory, + fileSystem, + authAndTlsOptions); try { if (credentials != null diff --git a/src/test/java/com/google/devtools/build/lib/buildeventservice/BUILD b/src/test/java/com/google/devtools/build/lib/buildeventservice/BUILD index 793238eb0f0c56..4aa7f3221587fa 100644 --- a/src/test/java/com/google/devtools/build/lib/buildeventservice/BUILD +++ b/src/test/java/com/google/devtools/build/lib/buildeventservice/BUILD @@ -48,6 +48,7 @@ java_test( "//src/main/java/com/google/devtools/build/lib:runtime", "//src/main/java/com/google/devtools/build/lib/actions:action_lookup_data", "//src/main/java/com/google/devtools/build/lib/authandtls", + "//src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper:credential_module", "//src/main/java/com/google/devtools/build/lib/bugreport", "//src/main/java/com/google/devtools/build/lib/buildeventservice", "//src/main/java/com/google/devtools/build/lib/buildeventservice:buildeventservice-options", diff --git a/src/test/java/com/google/devtools/build/lib/buildeventservice/BazelBuildEventServiceModuleTest.java b/src/test/java/com/google/devtools/build/lib/buildeventservice/BazelBuildEventServiceModuleTest.java index 50b5433da0c635..4124c431c2446b 100644 --- a/src/test/java/com/google/devtools/build/lib/buildeventservice/BazelBuildEventServiceModuleTest.java +++ b/src/test/java/com/google/devtools/build/lib/buildeventservice/BazelBuildEventServiceModuleTest.java @@ -32,6 +32,7 @@ import com.google.devtools.build.lib.actions.ActionLookupData; import com.google.devtools.build.lib.analysis.util.AnalysisMock; import com.google.devtools.build.lib.authandtls.AuthAndTLSOptions; +import com.google.devtools.build.lib.authandtls.credentialhelper.CredentialModule; import com.google.devtools.build.lib.bugreport.BugReport; import com.google.devtools.build.lib.bugreport.Crash; import com.google.devtools.build.lib.bugreport.CrashContext; @@ -145,6 +146,7 @@ public void beforeCommand(CommandEnvironment env) { } }) .addBlazeModule(new NoSpawnCacheModule()) + .addBlazeModule(new CredentialModule()) .addBlazeModule( new BazelBuildEventServiceModule() { @Override diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/BUILD b/src/test/java/com/google/devtools/build/lib/buildtool/BUILD index 0733ee95aa31e1..c604a800700a67 100644 --- a/src/test/java/com/google/devtools/build/lib/buildtool/BUILD +++ b/src/test/java/com/google/devtools/build/lib/buildtool/BUILD @@ -530,6 +530,7 @@ java_test( "//src/main/java/com/google/devtools/build/lib/actions:file_metadata", "//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster", "//src/main/java/com/google/devtools/build/lib/analysis:configured_target", + "//src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper:credential_module", "//src/main/java/com/google/devtools/build/lib/buildeventservice", "//src/main/java/com/google/devtools/build/lib/buildeventstream/proto:build_event_stream_java_proto", "//src/main/java/com/google/devtools/build/lib/cmdline", diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/TargetCompleteEventTest.java b/src/test/java/com/google/devtools/build/lib/buildtool/TargetCompleteEventTest.java index ea810373900f2b..9ffd40e6a94498 100644 --- a/src/test/java/com/google/devtools/build/lib/buildtool/TargetCompleteEventTest.java +++ b/src/test/java/com/google/devtools/build/lib/buildtool/TargetCompleteEventTest.java @@ -28,6 +28,7 @@ import com.google.devtools.build.lib.analysis.TargetCompleteEvent; import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget; import com.google.devtools.build.lib.analysis.util.AnalysisMock; +import com.google.devtools.build.lib.authandtls.credentialhelper.CredentialModule; import com.google.devtools.build.lib.buildeventservice.BazelBuildEventServiceModule; import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos; import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.BuildEvent; @@ -71,6 +72,7 @@ public void stageEmbeddedTools() throws Exception { protected BlazeRuntime.Builder getRuntimeBuilder() throws Exception { return super.getRuntimeBuilder() .addBlazeModule(new NoSpawnCacheModule()) + .addBlazeModule(new CredentialModule()) .addBlazeModule(new BazelBuildEventServiceModule()); } diff --git a/src/test/java/com/google/devtools/build/lib/remote/BUILD b/src/test/java/com/google/devtools/build/lib/remote/BUILD index c0435551113347..7cef85b3320396 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/BUILD +++ b/src/test/java/com/google/devtools/build/lib/remote/BUILD @@ -59,6 +59,7 @@ java_test( "//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/authandtls/credentialhelper:credential_module", "//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", @@ -101,6 +102,7 @@ java_test( "//src/test/java/com/google/devtools/build/lib/testutil:TestUtils", "//third_party:api_client", "//third_party:auth", + "//third_party:caffeine", "//third_party:guava", "//third_party:junit4", "//third_party:mockito", @@ -154,6 +156,7 @@ java_test( "//src/main/java/com/google/devtools/build/lib:runtime", "//src/main/java/com/google/devtools/build/lib/actions", "//src/main/java/com/google/devtools/build/lib/actions:artifacts", + "//src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper:credential_module", "//src/main/java/com/google/devtools/build/lib/dynamic", "//src/main/java/com/google/devtools/build/lib/remote", "//src/main/java/com/google/devtools/build/lib/standalone", @@ -170,6 +173,7 @@ java_test( srcs = ["DiskCacheIntegrationTest.java"], deps = [ "//src/main/java/com/google/devtools/build/lib:runtime", + "//src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper:credential_module", "//src/main/java/com/google/devtools/build/lib/remote", "//src/main/java/com/google/devtools/build/lib/standalone", "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", diff --git a/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTest.java index 70e138cbc34ed3..8c49c36b9c7d6f 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTest.java @@ -21,6 +21,7 @@ import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.BuildFailedException; +import com.google.devtools.build.lib.authandtls.credentialhelper.CredentialModule; import com.google.devtools.build.lib.dynamic.DynamicExecutionModule; import com.google.devtools.build.lib.remote.util.IntegrationTestUtils.WorkerInstance; import com.google.devtools.build.lib.runtime.BlazeModule; @@ -77,6 +78,7 @@ protected ImmutableList getSpawnModules() { return ImmutableList.builder() .addAll(super.getSpawnModules()) .add(new StandaloneModule()) + .add(new CredentialModule()) .add(new RemoteModule()) .add(new DynamicExecutionModule()) .build(); diff --git a/src/test/java/com/google/devtools/build/lib/remote/DiskCacheIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/remote/DiskCacheIntegrationTest.java index 3cf339217551b4..ee64e175c495dd 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/DiskCacheIntegrationTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/DiskCacheIntegrationTest.java @@ -16,6 +16,7 @@ import static com.google.devtools.build.lib.testutil.TestUtils.tmpDirFile; import com.google.common.collect.ImmutableList; +import com.google.devtools.build.lib.authandtls.credentialhelper.CredentialModule; import com.google.devtools.build.lib.buildtool.util.BuildIntegrationTestCase; import com.google.devtools.build.lib.runtime.BlazeModule; import com.google.devtools.build.lib.runtime.BlazeRuntime; @@ -60,6 +61,7 @@ protected ImmutableList getSpawnModules() { @Override protected BlazeRuntime.Builder getRuntimeBuilder() throws Exception { return super.getRuntimeBuilder() + .addBlazeModule(new CredentialModule()) .addBlazeModule(new RemoteModule()) .addBlazeModule(new BuildSummaryStatsModule()) .addBlazeModule(new BlockWaitingModule()); diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteModuleTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteModuleTest.java index 2deeff89cae143..a56673388d1779 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteModuleTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteModuleTest.java @@ -26,6 +26,8 @@ import build.bazel.remote.execution.v2.GetCapabilitiesRequest; import build.bazel.remote.execution.v2.ServerCapabilities; import build.bazel.remote.execution.v2.SymlinkAbsolutePathStrategy; +import com.github.benmanes.caffeine.cache.Cache; +import com.github.benmanes.caffeine.cache.Caffeine; import com.google.auth.Credentials; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -35,6 +37,7 @@ import com.google.devtools.build.lib.analysis.config.CoreOptions; import com.google.devtools.build.lib.authandtls.AuthAndTLSOptions; import com.google.devtools.build.lib.authandtls.credentialhelper.CredentialHelperEnvironment; +import com.google.devtools.build.lib.authandtls.credentialhelper.CredentialModule; import com.google.devtools.build.lib.events.Reporter; import com.google.devtools.build.lib.exec.BinTools; import com.google.devtools.build.lib.exec.ExecutionOptions; @@ -103,7 +106,8 @@ public final class RemoteModuleTest { CacheCapabilities.newBuilder().addDigestFunctions(Value.SHA256).build()) .build(); - private static CommandEnvironment createTestCommandEnvironment(RemoteOptions remoteOptions) + private static CommandEnvironment createTestCommandEnvironment( + RemoteModule remoteModule, RemoteOptions remoteOptions) throws IOException, AbruptExitException { CoreOptions coreOptions = Options.getDefaults(CoreOptions.class); CommonCommandOptions commonCommandOptions = Options.getDefaults(CommonCommandOptions.class); @@ -134,6 +138,8 @@ private static CommandEnvironment createTestCommandEnvironment(RemoteOptions rem .setServerDirectories(serverDirectories) .setStartupOptionsProvider( OptionsParser.builder().optionsClasses(BlazeServerStartupOptions.class).build()) + .addBlazeModule(new CredentialModule()) + .addBlazeModule(remoteModule) .addBlazeModule(new BlockWaitingModule()) .build(); @@ -198,7 +204,7 @@ public void testVerifyCapabilities_executionAndCacheForSingleEndpoint() throws E RemoteOptions remoteOptions = Options.getDefaults(RemoteOptions.class); remoteOptions.remoteExecutor = executionServerName; - CommandEnvironment env = createTestCommandEnvironment(remoteOptions); + CommandEnvironment env = createTestCommandEnvironment(remoteModule, remoteOptions); remoteModule.beforeCommand(env); @@ -226,7 +232,7 @@ public void testVerifyCapabilities_cacheOnlyEndpoint() throws Exception { RemoteOptions remoteOptions = Options.getDefaults(RemoteOptions.class); remoteOptions.remoteCache = cacheServerName; - CommandEnvironment env = createTestCommandEnvironment(remoteOptions); + CommandEnvironment env = createTestCommandEnvironment(remoteModule, remoteOptions); remoteModule.beforeCommand(env); @@ -272,7 +278,7 @@ public void testVerifyCapabilities_executionAndCacheForDifferentEndpoints() thro remoteOptions.remoteExecutor = executionServerName; remoteOptions.remoteCache = cacheServerName; - CommandEnvironment env = createTestCommandEnvironment(remoteOptions); + CommandEnvironment env = createTestCommandEnvironment(remoteModule, remoteOptions); remoteModule.beforeCommand(env); @@ -327,7 +333,7 @@ public void testVerifyCapabilities_executionOnlyAndCacheOnlyEndpoints() throws E remoteOptions.remoteExecutor = executionServerName; remoteOptions.remoteCache = cacheServerName; - CommandEnvironment env = createTestCommandEnvironment(remoteOptions); + CommandEnvironment env = createTestCommandEnvironment(remoteModule, remoteOptions); remoteModule.beforeCommand(env); @@ -365,7 +371,7 @@ public void testLocalFallback_shouldErrorForRemoteCacheWithoutRequiredCapabiliti (target, proxy, options, interceptors) -> InProcessChannelBuilder.forName(target).directExecutor().build()); - CommandEnvironment env = createTestCommandEnvironment(remoteOptions); + CommandEnvironment env = createTestCommandEnvironment(remoteModule, remoteOptions); assertThrows(AbruptExitException.class, () -> remoteModule.beforeCommand(env)); } finally { @@ -398,7 +404,7 @@ public void getCapabilities( (target, proxy, options, interceptors) -> InProcessChannelBuilder.forName(target).directExecutor().build()); - CommandEnvironment env = createTestCommandEnvironment(remoteOptions); + CommandEnvironment env = createTestCommandEnvironment(remoteModule, remoteOptions); assertThrows(AbruptExitException.class, () -> remoteModule.beforeCommand(env)); } finally { @@ -430,7 +436,7 @@ public void getCapabilities( (target, proxy, options, interceptors) -> InProcessChannelBuilder.forName(target).directExecutor().build()); - CommandEnvironment env = createTestCommandEnvironment(remoteOptions); + CommandEnvironment env = createTestCommandEnvironment(remoteModule, remoteOptions); remoteModule.beforeCommand(env); @@ -468,7 +474,7 @@ public void getCapabilities( (target, proxy, options, interceptors) -> InProcessChannelBuilder.forName(target).directExecutor().build()); - CommandEnvironment env = createTestCommandEnvironment(remoteOptions); + CommandEnvironment env = createTestCommandEnvironment(remoteModule, remoteOptions); remoteModule.beforeCommand(env); @@ -492,14 +498,18 @@ public void testNetrc_netrcWithoutRemoteCache() throws Exception { AuthAndTLSOptions authAndTLSOptions = Options.getDefaults(AuthAndTLSOptions.class); RemoteOptions remoteOptions = Options.getDefaults(RemoteOptions.class); + Cache>> credentialCache = + Caffeine.newBuilder().build(); + Credentials credentials = - RemoteModule.newCredentials( + RemoteModule.createCredentials( CredentialHelperEnvironment.newBuilder() .setEventReporter(new Reporter(new EventBus())) .setWorkspacePath(fileSystem.getPath("/workspace")) .setClientEnvironment(ImmutableMap.of("NETRC", netrc)) .setHelperExecutionTimeout(Duration.ZERO) .build(), + credentialCache, new CommandLinePathFactory(fileSystem, ImmutableMap.of()), fileSystem, authAndTLSOptions, @@ -526,7 +536,7 @@ public void testCacheCapabilities_propagatedToRemoteCache() throws Exception { RemoteOptions remoteOptions = Options.getDefaults(RemoteOptions.class); remoteOptions.remoteCache = cacheServerName; - CommandEnvironment env = createTestCommandEnvironment(remoteOptions); + CommandEnvironment env = createTestCommandEnvironment(remoteModule, remoteOptions); remoteModule.beforeCommand(env); @@ -558,7 +568,7 @@ public void testCacheCapabilities_propagatedToRemoteExecutionCache() throws Exce RemoteOptions remoteOptions = Options.getDefaults(RemoteOptions.class); remoteOptions.remoteExecutor = executionServerName; - CommandEnvironment env = createTestCommandEnvironment(remoteOptions); + CommandEnvironment env = createTestCommandEnvironment(remoteModule, remoteOptions); remoteModule.beforeCommand(env); diff --git a/src/test/shell/bazel/remote/remote_execution_test.sh b/src/test/shell/bazel/remote/remote_execution_test.sh index c3d4b645ecd8e2..26531744461be8 100755 --- a/src/test/shell/bazel/remote/remote_execution_test.sh +++ b/src/test/shell/bazel/remote/remote_execution_test.sh @@ -76,56 +76,76 @@ function has_utf8_locale() { [[ "${charmap}" == "UTF-8" ]] } -function setup_credential_helper() { +function setup_credential_helper_test() { + # Each helper call atomically writes one byte to this file. + # We can later read the file to determine how many calls were made. + cat > "${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 secret_token"]}}""") EOF chmod +x "${TEST_TMPDIR}/credhelper" -} - -function test_credential_helper_remote_cache() { - setup_credential_helper mkdir -p a cat > a/BUILD <<'EOF' -genrule( - name = "a", - outs = ["a.txt"], +[genrule( + name = x, + outs = [x + ".txt"], cmd = "touch $(OUTS)", -) +) for x in ["a", "b"]] EOF stop_worker start_worker --expected_authorization_token=secret_token +} + +function expect_credential_helper_calls() { + local -r expected=$1 + local -r actual=$(wc -c "${TEST_TMPDIR}/credhelper_log" | awk '{print $1}') + if [[ "$expected" != "$actual" ]]; then + fail "expected $expected instead of $actual credential helper calls" + fi +} + +function test_credential_helper_remote_cache() { + setup_credential_helper_test bazel build \ --remote_cache=grpc://localhost:${worker_port} \ //a:a >& $TEST_log && fail "Build without credentials should have failed" expect_log "Failed to query remote execution capabilities" + # Helper shouldn't have been called yet. + expect_credential_helper_calls 0 + bazel build \ --remote_cache=grpc://localhost:${worker_port} \ --experimental_credential_helper="${TEST_TMPDIR}/credhelper" \ //a:a >& $TEST_log || fail "Build with credentials should have succeeded" -} -function test_credential_helper_remote_execution() { - setup_credential_helper + # First build should have called helper for 4 distinct URIs. + expect_credential_helper_calls 4 - mkdir -p a + bazel build \ + --remote_cache=grpc://localhost:${worker_port} \ + --experimental_credential_helper="${TEST_TMPDIR}/credhelper" \ + //a:b >& $TEST_log || fail "Build with credentials should have succeeded" - cat > a/BUILD <<'EOF' -genrule( - name = "a", - outs = ["a.txt"], - cmd = "touch $(OUTS)", -) -EOF + # Second build should have hit the credentials cache. + expect_credential_helper_calls 4 +} - stop_worker - start_worker --expected_authorization_token=secret_token +function test_credential_helper_remote_execution() { + setup_credential_helper_test bazel build \ --spawn_strategy=remote \ @@ -133,11 +153,49 @@ EOF //a:a >& $TEST_log && fail "Build without credentials should have failed" expect_log "Failed to query remote execution capabilities" + # Helper shouldn't have been called yet. + expect_credential_helper_calls 0 + + bazel build \ + --spawn_strategy=remote \ + --remote_executor=grpc://localhost:${worker_port} \ + --experimental_credential_helper="${TEST_TMPDIR}/credhelper" \ + //a:a >& $TEST_log || fail "Build with credentials should have succeeded" + + # First build should have called helper for 5 distinct URIs. + expect_credential_helper_calls 5 + + bazel build \ + --spawn_strategy=remote \ + --remote_executor=grpc://localhost:${worker_port} \ + --experimental_credential_helper="${TEST_TMPDIR}/credhelper" \ + //a:b >& $TEST_log || fail "Build with credentials should have succeeded" + + # Second build should have hit the credentials cache. + expect_credential_helper_calls 5 +} + +function test_credential_helper_clear_cache() { + setup_credential_helper_test + bazel build \ --spawn_strategy=remote \ --remote_executor=grpc://localhost:${worker_port} \ --experimental_credential_helper="${TEST_TMPDIR}/credhelper" \ //a:a >& $TEST_log || fail "Build with credentials should have succeeded" + + expect_credential_helper_calls 5 + + bazel clean + + bazel build \ + --spawn_strategy=remote \ + --remote_executor=grpc://localhost:${worker_port} \ + --experimental_credential_helper="${TEST_TMPDIR}/credhelper" \ + //a:b >& $TEST_log || fail "Build with credentials should have succeeded" + + # Build after clean should have called helper again. + expect_credential_helper_calls 10 } function test_remote_grpc_cache_with_protocol() {