diff --git a/src/main/java/com/google/devtools/build/lib/actions/EnvironmentalExecException.java b/src/main/java/com/google/devtools/build/lib/actions/EnvironmentalExecException.java index 66257fe73b305e..5dacc38ba4fd35 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/EnvironmentalExecException.java +++ b/src/main/java/com/google/devtools/build/lib/actions/EnvironmentalExecException.java @@ -36,7 +36,7 @@ public EnvironmentalExecException(IOException cause, FailureDetails.Execution.Co FailureDetail.newBuilder().setExecution(Execution.newBuilder().setCode(code)).build(); } - public EnvironmentalExecException(Exception cause, FailureDetail failureDetail) { + public EnvironmentalExecException(Throwable cause, FailureDetail failureDetail) { super(failureDetail.getMessage(), cause); this.failureDetail = failureDetail; } diff --git a/src/main/java/com/google/devtools/build/lib/remote/GoogleChannelConnectionFactory.java b/src/main/java/com/google/devtools/build/lib/remote/GoogleChannelConnectionFactory.java index 6481c5b7bb8091..a63f182e532b33 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/GoogleChannelConnectionFactory.java +++ b/src/main/java/com/google/devtools/build/lib/remote/GoogleChannelConnectionFactory.java @@ -30,10 +30,10 @@ import com.google.devtools.build.lib.events.Reporter; import com.google.devtools.build.lib.profiler.Profiler; import com.google.devtools.build.lib.remote.RemoteServerCapabilities.ServerCapabilitiesRequirement; +import com.google.devtools.build.lib.remote.common.RemoteExecutionCapabilitiesException; import com.google.devtools.build.lib.remote.grpc.ChannelConnectionFactory; import com.google.devtools.build.lib.remote.options.RemoteOptions; import com.google.devtools.build.lib.remote.util.RxFutures; -import com.google.devtools.build.lib.remote.util.Utils; import io.grpc.ClientInterceptor; import io.grpc.ManagedChannel; import io.reactivex.rxjava3.core.Single; @@ -153,18 +153,14 @@ public void onSuccess(ServerCapabilities result) { @Override public void onFailure(Throwable error) { - String message = - "Failed to query remote execution capabilities: " - + Utils.grpcAwareErrorMessage(error, verboseFailures); - reporter.handle(Event.error(message)); - - IOException exception; - if (error instanceof IOException ioException) { - exception = ioException; + Throwable cause; + if (!(error instanceof IOException) + && error.getCause() instanceof IOException ioException) { + cause = ioException; } else { - exception = new IOException(error); + cause = error; } - serverCapabilities.setException(exception); + serverCapabilities.setException(new RemoteExecutionCapabilitiesException(cause)); } }, directExecutor()); diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java index 456423a97c59da..eecaaa07d655d6 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java @@ -15,6 +15,8 @@ import static com.google.common.base.Strings.isNullOrEmpty; import static com.google.devtools.build.lib.profiler.ProfilerTask.REMOTE_DOWNLOAD; +import static com.google.devtools.build.lib.remote.util.Utils.createExecExceptionForCredentialHelperException; +import static com.google.devtools.build.lib.remote.util.Utils.createExecExceptionFromRemoteExecutionCapabilitiesException; import static com.google.devtools.build.lib.remote.util.Utils.createSpawnResult; import com.google.common.annotations.VisibleForTesting; @@ -27,6 +29,7 @@ import com.google.devtools.build.lib.actions.SpawnMetrics; import com.google.devtools.build.lib.actions.SpawnResult; import com.google.devtools.build.lib.actions.cache.VirtualActionInput; +import com.google.devtools.build.lib.authandtls.credentialhelper.CredentialHelperException; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.exec.SpawnCache; @@ -39,6 +42,7 @@ import com.google.devtools.build.lib.remote.common.BulkTransferException; import com.google.devtools.build.lib.remote.common.CacheNotFoundException; import com.google.devtools.build.lib.remote.common.RemoteCacheClient; +import com.google.devtools.build.lib.remote.common.RemoteExecutionCapabilitiesException; import com.google.devtools.build.lib.remote.options.RemoteOptions; import com.google.devtools.build.lib.remote.util.DigestUtil; import com.google.devtools.build.lib.remote.util.Utils; @@ -165,6 +169,10 @@ public CacheHandle lookup(Spawn spawn, SpawnExecutionContext context) } } catch (CacheNotFoundException e) { // Intentionally left blank + } catch (CredentialHelperException e) { + throw createExecExceptionForCredentialHelperException(e); + } catch (RemoteExecutionCapabilitiesException e) { + throw createExecExceptionFromRemoteExecutionCapabilitiesException(e); } catch (IOException e) { if (BulkTransferException.allCausedByCacheNotFoundException(e)) { // Intentionally left blank diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java index 286137c609bac1..fb4c8b86db1cb0 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java @@ -22,6 +22,8 @@ import static com.google.devtools.build.lib.profiler.ProfilerTask.REMOTE_QUEUE; import static com.google.devtools.build.lib.profiler.ProfilerTask.REMOTE_SETUP; import static com.google.devtools.build.lib.profiler.ProfilerTask.UPLOAD_TIME; +import static com.google.devtools.build.lib.remote.util.Utils.createExecExceptionForCredentialHelperException; +import static com.google.devtools.build.lib.remote.util.Utils.createExecExceptionFromRemoteExecutionCapabilitiesException; import static com.google.devtools.build.lib.remote.util.Utils.createSpawnResult; import static java.lang.Math.max; @@ -42,6 +44,7 @@ import com.google.devtools.build.lib.actions.SpawnResult; import com.google.devtools.build.lib.actions.SpawnResult.Status; import com.google.devtools.build.lib.actions.cache.VirtualActionInput; +import com.google.devtools.build.lib.authandtls.credentialhelper.CredentialHelperException; import com.google.devtools.build.lib.clock.BlazeClock; import com.google.devtools.build.lib.clock.BlazeClock.MillisSinceEpochToNanosConverter; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; @@ -62,6 +65,7 @@ import com.google.devtools.build.lib.remote.circuitbreaker.CircuitBreakerFactory; import com.google.devtools.build.lib.remote.common.BulkTransferException; import com.google.devtools.build.lib.remote.common.OperationObserver; +import com.google.devtools.build.lib.remote.common.RemoteExecutionCapabilitiesException; import com.google.devtools.build.lib.remote.options.RemoteOptions; import com.google.devtools.build.lib.remote.util.DigestUtil; import com.google.devtools.build.lib.remote.util.Utils; @@ -241,6 +245,8 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionContext context) } } } + } catch (CredentialHelperException e) { + throw createExecExceptionForCredentialHelperException(e); } catch (IOException e) { return execLocallyAndUploadOrFail(action, spawn, context, uploadLocalResults, e); } @@ -328,6 +334,8 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionContext context) throw e; } }); + } catch (CredentialHelperException e) { + throw createExecExceptionForCredentialHelperException(e); } catch (IOException e) { return execLocallyAndUploadOrFail(action, spawn, context, uploadLocalResults, e); } @@ -575,6 +583,9 @@ private SpawnResult execLocallyAndUploadOrFail( private SpawnResult handleError( RemoteAction action, IOException exception, SpawnExecutionContext context) throws ExecException, InterruptedException, IOException { + if (exception instanceof RemoteExecutionCapabilitiesException e) { + throw createExecExceptionFromRemoteExecutionCapabilitiesException(e); + } boolean remoteCacheFailed = BulkTransferException.allCausedByCacheNotFoundException(exception); if (exception.getCause() instanceof ExecutionStatusException e) { RemoteActionResult result = null; diff --git a/src/main/java/com/google/devtools/build/lib/remote/common/RemoteExecutionCapabilitiesException.java b/src/main/java/com/google/devtools/build/lib/remote/common/RemoteExecutionCapabilitiesException.java new file mode 100644 index 00000000000000..a708453ddc8dba --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/remote/common/RemoteExecutionCapabilitiesException.java @@ -0,0 +1,23 @@ +// Copyright 2024 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.remote.common; + +import java.io.IOException; + +/** An exception to indicate that Bazel failed to query remote execution capabilities. */ +public class RemoteExecutionCapabilitiesException extends IOException { + public RemoteExecutionCapabilitiesException(Throwable cause) { + super(cause); + } +} diff --git a/src/main/java/com/google/devtools/build/lib/remote/util/Utils.java b/src/main/java/com/google/devtools/build/lib/remote/util/Utils.java index 327d58ee658c75..c8d3c702279892 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/util/Utils.java +++ b/src/main/java/com/google/devtools/build/lib/remote/util/Utils.java @@ -34,6 +34,8 @@ import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; import com.google.devtools.build.lib.actions.ActionInput; +import com.google.devtools.build.lib.actions.EnvironmentalExecException; +import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.ExecutionRequirements; import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.actions.SpawnMetrics; @@ -46,9 +48,11 @@ import com.google.devtools.build.lib.remote.common.CacheNotFoundException; import com.google.devtools.build.lib.remote.common.OutputDigestMismatchException; import com.google.devtools.build.lib.remote.common.RemoteCacheClient.ActionKey; +import com.google.devtools.build.lib.remote.common.RemoteExecutionCapabilitiesException; import com.google.devtools.build.lib.remote.options.RemoteOptions; import com.google.devtools.build.lib.server.FailureDetails; import com.google.devtools.build.lib.server.FailureDetails.FailureDetail; +import com.google.devtools.build.lib.server.FailureDetails.RemoteExecution; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.protobuf.Any; import com.google.protobuf.ByteString; @@ -666,4 +670,30 @@ public static ListenableFuture mergeBulkTransfer( }, directExecutor()); } + + public static ExecException createExecExceptionForCredentialHelperException( + CredentialHelperException e) { + return new EnvironmentalExecException( + e, + FailureDetail.newBuilder() + .setRemoteOptions( + FailureDetails.RemoteOptions.newBuilder() + .setCode(FailureDetails.RemoteOptions.Code.CREDENTIALS_READ_FAILURE) + .build()) + .setMessage("Exec failed due to CredentialHelperException") + .build()); + } + + public static ExecException createExecExceptionFromRemoteExecutionCapabilitiesException( + RemoteExecutionCapabilitiesException e) { + return new EnvironmentalExecException( + e.getCause(), + FailureDetail.newBuilder() + .setRemoteExecution( + RemoteExecution.newBuilder() + .setCode(RemoteExecution.Code.CAPABILITIES_QUERY_FAILURE) + .build()) + .setMessage("Failed to query remote execution capabilities") + .build()); + } } diff --git a/src/test/shell/bazel/remote/remote_execution_test.sh b/src/test/shell/bazel/remote/remote_execution_test.sh index 04030f49668343..c9d73ea6bc75da 100755 --- a/src/test/shell/bazel/remote/remote_execution_test.sh +++ b/src/test/shell/bazel/remote/remote_execution_test.sh @@ -97,11 +97,9 @@ function test_credential_helper_remote_cache() { bazel build \ --remote_cache=grpc://localhost:${worker_port} \ - //a:a >& $TEST_log || fail "Build without credentials should have succeeded" + //a:a >& $TEST_log && fail "Build without credentials should have failed" expect_log "Failed to query remote execution capabilities" - bazel clean - # Helper shouldn't have been called yet. expect_credential_helper_calls 0 diff --git a/src/test/shell/bazel/remote/remote_execution_tls_test.sh b/src/test/shell/bazel/remote/remote_execution_tls_test.sh index 7c6fed2714feb5..fe24797324b68b 100755 --- a/src/test/shell/bazel/remote/remote_execution_tls_test.sh +++ b/src/test/shell/bazel/remote/remote_execution_tls_test.sh @@ -101,7 +101,7 @@ function test_mtls_fails_if_client_has_no_cert() { --remote_cache=grpcs://localhost:${worker_port} \ --tls_certificate="${cert_path}/ca.crt" \ //a:foo &> $TEST_log \ - || fail "Failed to build //a:foo" + && fail "Expected bazel to fail without a client cert" || true expect_log "Failed to query remote execution capabilities:" } @@ -125,8 +125,8 @@ function test_remote_cache_with_incompatible_tls_enabled_removed_grpc_scheme() { --remote_cache=grpc://localhost:${worker_port} \ --tls_certificate="${cert_path}/ca.crt" \ ${client_mtls_flags} \ - //a:foo >& $TEST_log \ - || fail "Failed to build //a:foo" || true + //a:foo &> $TEST_log \ + && fail "Expected test failure" || true expect_log "Failed to query remote execution capabilities:" }