Skip to content

Commit

Permalink
Correctly propagate and abort the build for CredentialHelperException.
Browse files Browse the repository at this point in the history
  • Loading branch information
@coeuvre
coeuvre committed Sep 18, 2024
1 parent 94b72e4 commit 852f2e8
Show file tree
Hide file tree
Showing 5 changed files with 95 additions and 13 deletions.
30 changes: 20 additions & 10 deletions src/main/java/com/google/devtools/build/lib/remote/GoogleChannelConnectionFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,19 @@
import com.google.common.util.concurrent.Futures;
import com.google.common.util.concurrent.ListenableFuture;
import com.google.common.util.concurrent.SettableFuture;
import com.google.devtools.build.lib.actions.EnvironmentalExecException;
import com.google.devtools.build.lib.authandtls.AuthAndTLSOptions;
import com.google.devtools.build.lib.events.Event;
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.ExecIOException;
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 com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
import com.google.devtools.build.lib.server.FailureDetails.RemoteExecution;
import com.google.devtools.build.lib.server.FailureDetails.RemoteExecution.Code;
import io.grpc.ClientInterceptor;
import io.grpc.ManagedChannel;
import io.reactivex.rxjava3.core.Single;
Expand Down Expand Up @@ -153,18 +157,24 @@ 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;
IOException cause;
if (error instanceof IOException ioException) {
exception = ioException;
cause = ioException;
} else if (error.getCause() instanceof IOException ioException) {
cause = ioException;
} else {
exception = new IOException(error);
cause = new IOException("Failed to query remote execution capabilities", error);
}
serverCapabilities.setException(exception);
serverCapabilities.setException(
new ExecIOException(
new EnvironmentalExecException(
cause,
FailureDetail.newBuilder()
.setRemoteExecution(
RemoteExecution.newBuilder()
.setCode(Code.CAPABILITIES_QUERY_FAILURE)
.build())
.build())));
}
},
directExecutor());
Expand Down
17 changes: 17 additions & 0 deletions src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,15 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Stopwatch;
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.FileArtifactValue;
import com.google.devtools.build.lib.actions.ForbiddenActionInputException;
import com.google.devtools.build.lib.actions.Spawn;
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;
Expand All @@ -38,11 +40,15 @@
import com.google.devtools.build.lib.remote.RemoteExecutionService.RemoteActionResult;
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.ExecIOException;
import com.google.devtools.build.lib.remote.common.RemoteCacheClient;
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;
import com.google.devtools.build.lib.remote.util.Utils.InMemoryOutput;
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.RemoteOptions.Code;
import com.google.devtools.build.lib.vfs.Path;
import java.io.IOException;
import java.util.NoSuchElementException;
Expand Down Expand Up @@ -165,6 +171,17 @@ public CacheHandle lookup(Spawn spawn, SpawnExecutionContext context)
}
} catch (CacheNotFoundException e) {
// Intentionally left blank
} catch (CredentialHelperException e) {
throw new EnvironmentalExecException(
e,
FailureDetail.newBuilder()
.setRemoteOptions(
FailureDetails.RemoteOptions.newBuilder()
.setCode(Code.CREDENTIALS_READ_FAILURE)
.build())
.build());
} catch (ExecIOException e){
throw e.getExecException();
} catch (IOException e) {
if (BulkTransferException.allCausedByCacheNotFoundException(e)) {
// Intentionally left blank
Expand Down
23 changes: 23 additions & 0 deletions src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,15 @@
import com.google.common.util.concurrent.ListeningScheduledExecutorService;
import com.google.devtools.build.lib.actions.ActionInput;
import com.google.devtools.build.lib.actions.CommandLines.ParamFileActionInput;
import com.google.devtools.build.lib.actions.EnvironmentalExecException;
import com.google.devtools.build.lib.actions.ExecException;
import com.google.devtools.build.lib.actions.ForbiddenActionInputException;
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.SpawnMetrics;
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;
Expand All @@ -61,6 +63,7 @@
import com.google.devtools.build.lib.remote.RemoteExecutionService.ServerLogs;
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.ExecIOException;
import com.google.devtools.build.lib.remote.common.OperationObserver;
import com.google.devtools.build.lib.remote.options.RemoteOptions;
import com.google.devtools.build.lib.remote.util.DigestUtil;
Expand Down Expand Up @@ -241,6 +244,10 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionContext context)
}
}
}
} catch (CredentialHelperException e) {
throw createExecExceptionForCredentialHelperException(e);
} catch (ExecIOException e){
throw e.getExecException();
} catch (IOException e) {
return execLocallyAndUploadOrFail(action, spawn, context, uploadLocalResults, e);
}
Expand Down Expand Up @@ -328,11 +335,27 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionContext context)
throw e;
}
});
} catch (CredentialHelperException e) {
throw createExecExceptionForCredentialHelperException(e);
} catch (ExecIOException e){
throw e.getExecException();
} catch (IOException e) {
return execLocallyAndUploadOrFail(action, spawn, context, uploadLocalResults, e);
}
}

private static ExecException createExecExceptionForCredentialHelperException(
CredentialHelperException e) {
return new EnvironmentalExecException(
e,
FailureDetail.newBuilder()
.setRemoteOptions(
FailureDetails.RemoteOptions.newBuilder()
.setCode(FailureDetails.RemoteOptions.Code.CREDENTIALS_READ_FAILURE)
.build())
.build());
}

private static void profileAccounting(
long clampTimeNanos, ExecutedActionMetadata executedActionMetadata) {
MillisSinceEpochToNanosConverter converter =
Expand Down
34 changes: 34 additions & 0 deletions src/main/java/com/google/devtools/build/lib/remote/common/ExecIOException.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// 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 com.google.devtools.build.lib.actions.ExecException;
import java.io.IOException;

/**
* An exception to indicate that the execution of an action has failed and {@link
* #getExecException()} should be propagated.
*/
public class ExecIOException extends IOException {
private final ExecException execException;

public ExecIOException(ExecException execException) {
super(execException);
this.execException = execException;
}

public ExecException getExecException() {
return execException;
}
}
4 changes: 1 addition & 3 deletions src/test/shell/bazel/remote/remote_execution_test.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,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

Expand Down

0 comments on commit 852f2e8

Please sign in to comment.