Skip to content

Commit fe9ba89

Browse files
buchgrdslomov
buchgr
authored and
dslomov
committed
grpc: Consolidate gRPC code from BES and Remote Execution. Fixes bazelbuild#3460, bazelbuild#3486
BES and Remote Execution have separate implementations of gRPC channel creation, authentication and TLS. We should merge them, to avoid duplication and bugs. One such bug is bazelbuild#3640, where the BES code had a different implementation for Google Application Default Credentials. RELNOTES: The Build Event Service (BES) client now properly supports Google Applicaton Default Credentials. PiperOrigin-RevId: 164253879
1 parent dba4916 commit fe9ba89

11 files changed

Lines changed: 126 additions & 239 deletions

File tree

‎src/main/java/com/google/devtools/build/lib/BUILD‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -391,7 +391,7 @@ java_library(
391391

392392
java_library(
393393
name = "auth_and_tls_options",
394-
srcs = ["authandtls/AuthAndTLSOptions.java"],
394+
srcs = glob(["authandtls/*.java"]),
395395
visibility = [
396396
"//src/main/java/com/google/devtools/build/lib/remote:__pkg__",
397397
"//src/test/java/com/google/devtools/build/lib:__pkg__",
@@ -401,6 +401,13 @@ java_library(
401401
],
402402
deps = [
403403
"//src/main/java/com/google/devtools/common/options",
404+
"//third_party:apache_httpclient",
405+
"//third_party:apache_httpcore",
406+
"//third_party:auth",
407+
"//third_party:guava",
408+
"//third_party:jsr305",
409+
"//third_party:netty",
410+
"//third_party/grpc:grpc-jar",
404411
],
405412
)
406413

‎src/main/java/com/google/devtools/build/lib/authandtls/AuthAndTLSOptions.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ public class AuthAndTLSOptions extends OptionsBase {
3939

4040
@Option(
4141
name = "auth_scope",
42-
defaultValue = "null",
42+
defaultValue = "https://www.googleapis.com/auth/cloud-build-service",
4343
category = "remote",
4444
documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
4545
effectTags = {OptionEffectTag.UNKNOWN},

src/main/java/com/google/devtools/build/lib/remote/ChannelOptions.java renamed to src/main/java/com/google/devtools/build/lib/authandtls/GrpcUtils.java

Lines changed: 73 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -12,16 +12,19 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15-
package com.google.devtools.build.lib.remote;
15+
package com.google.devtools.build.lib.authandtls;
1616

1717
import com.google.auth.oauth2.GoogleCredentials;
1818
import com.google.common.annotations.VisibleForTesting;
19+
import com.google.common.base.Preconditions;
1920
import com.google.common.collect.ImmutableList;
20-
import com.google.devtools.build.lib.authandtls.AuthAndTLSOptions;
21-
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
2221
import io.grpc.CallCredentials;
22+
import io.grpc.ManagedChannel;
2323
import io.grpc.auth.MoreCallCredentials;
2424
import io.grpc.netty.GrpcSslContexts;
25+
import io.grpc.netty.NegotiationType;
26+
import io.grpc.netty.NettyChannelBuilder;
27+
import io.grpc.util.RoundRobinLoadBalancerFactory;
2528
import io.netty.handler.ssl.SslContext;
2629
import java.io.File;
2730
import java.io.FileInputStream;
@@ -30,70 +33,91 @@
3033
import java.io.InputStream;
3134
import javax.annotation.Nullable;
3235

33-
/** Instantiate all authentication helpers from build options. */
34-
@ThreadSafe
35-
public final class ChannelOptions {
36-
private final boolean tlsEnabled;
37-
private final SslContext sslContext;
38-
private final String tlsAuthorityOverride;
39-
private final CallCredentials credentials;
36+
/**
37+
* Utility methods for using {@link AuthAndTLSOptions} with gRPC.
38+
*/
39+
public final class GrpcUtils {
4040

41-
private ChannelOptions(
42-
boolean tlsEnabled,
43-
SslContext sslContext,
44-
String tlsAuthorityOverride,
45-
CallCredentials credentials) {
46-
this.tlsEnabled = tlsEnabled;
47-
this.sslContext = sslContext;
48-
this.tlsAuthorityOverride = tlsAuthorityOverride;
49-
this.credentials = credentials;
50-
}
41+
/**
42+
* Create a new gRPC {@link ManagedChannel}.
43+
*
44+
* @throws IOException in case the channel can't be constructed.
45+
*/
46+
public static ManagedChannel newChannel(String target, AuthAndTLSOptions options)
47+
throws IOException {
48+
Preconditions.checkNotNull(target);
49+
Preconditions.checkNotNull(options);
5150

52-
public boolean tlsEnabled() {
53-
return tlsEnabled;
54-
}
51+
final SslContext sslContext =
52+
options.tlsEnabled ? createSSlContext(options.tlsCertificate) : null;
5553

56-
public CallCredentials getCallCredentials() {
57-
return credentials;
54+
try {
55+
NettyChannelBuilder builder =
56+
NettyChannelBuilder.forTarget(target)
57+
.negotiationType(
58+
options.tlsEnabled ? NegotiationType.TLS : NegotiationType.PLAINTEXT)
59+
.loadBalancerFactory(RoundRobinLoadBalancerFactory.getInstance());
60+
if (sslContext != null) {
61+
builder.sslContext(sslContext);
62+
if (options.tlsAuthorityOverride != null) {
63+
builder.overrideAuthority(options.tlsAuthorityOverride);
64+
}
65+
}
66+
return builder.build();
67+
} catch (RuntimeException e) {
68+
// gRPC might throw all kinds of RuntimeExceptions: StatusRuntimeException,
69+
// IllegalStateException, NullPointerException, ...
70+
String message = "Failed to connect to '%s': %s";
71+
throw new IOException(String.format(message, target, e.getMessage()));
72+
}
5873
}
5974

60-
public String getTlsAuthorityOverride() {
61-
return tlsAuthorityOverride;
75+
private static SslContext createSSlContext(@Nullable String rootCert) throws IOException {
76+
if (rootCert == null) {
77+
try {
78+
return GrpcSslContexts.forClient().build();
79+
} catch (Exception e) {
80+
String message = "Failed to init TLS infrastructure: "
81+
+ e.getMessage();
82+
throw new IOException(message, e);
83+
}
84+
} else {
85+
try {
86+
return GrpcSslContexts.forClient().trustManager(new File(rootCert)).build();
87+
} catch (Exception e) {
88+
String message = "Failed to init TLS infrastructure using '%s' as root certificate: %s";
89+
message = String.format(message, rootCert, e.getMessage());
90+
throw new IOException(message, e);
91+
}
92+
}
6293
}
6394

64-
public SslContext getSslContext() {
65-
return sslContext;
66-
}
95+
/**
96+
* Create a new {@link CallCredentials} object.
97+
*
98+
* @throws IOException in case the call credentials can't be constructed.
99+
*/
100+
public static CallCredentials newCallCredentials(AuthAndTLSOptions options) throws IOException {
101+
if (!options.authEnabled) {
102+
return null;
103+
}
67104

68-
public static ChannelOptions create(AuthAndTLSOptions options) throws IOException {
69105
if (options.authCredentials != null) {
106+
// Credentials from file
70107
try (InputStream authFile = new FileInputStream(options.authCredentials)) {
71-
return create(options, authFile);
108+
return newCallCredentials(authFile, options.authScope);
72109
} catch (FileNotFoundException e) {
73110
String message = String.format("Could not open auth credentials file '%s': %s",
74111
options.authCredentials, e.getMessage());
75112
throw new IOException(message, e);
76113
}
77-
} else {
78-
return create(options, null);
79114
}
115+
// Google Application Default Credentials
116+
return newCallCredentials(null, options.authScope);
80117
}
81118

82119
@VisibleForTesting
83-
static ChannelOptions create(
84-
AuthAndTLSOptions options,
85-
@Nullable InputStream credentialsFile) throws IOException {
86-
final SslContext sslContext =
87-
options.tlsEnabled ? createSSlContext(options.tlsCertificate) : null;
88-
89-
final CallCredentials callCredentials =
90-
options.authEnabled ? createCallCredentials(credentialsFile, options.authScope) : null;
91-
92-
return new ChannelOptions(
93-
sslContext != null, sslContext, options.tlsAuthorityOverride, callCredentials);
94-
}
95-
96-
private static CallCredentials createCallCredentials(@Nullable InputStream credentialsFile,
120+
public static CallCredentials newCallCredentials(@Nullable InputStream credentialsFile,
97121
@Nullable String authScope) throws IOException {
98122
try {
99123
GoogleCredentials creds =
@@ -105,30 +129,9 @@ private static CallCredentials createCallCredentials(@Nullable InputStream crede
105129
}
106130
return MoreCallCredentials.from(creds);
107131
} catch (IOException e) {
108-
String message = "Failed to init auth credentials for remote caching/execution: "
132+
String message = "Failed to init auth credentials: "
109133
+ e.getMessage();
110134
throw new IOException(message, e);
111135
}
112136
}
113-
114-
private static SslContext createSSlContext(@Nullable String rootCert) throws IOException {
115-
if (rootCert == null) {
116-
try {
117-
return GrpcSslContexts.forClient().build();
118-
} catch (Exception e) {
119-
String message = "Failed to init TLS infrastructure for remote caching/execution: "
120-
+ e.getMessage();
121-
throw new IOException(message, e);
122-
}
123-
} else {
124-
try {
125-
return GrpcSslContexts.forClient().trustManager(new File(rootCert)).build();
126-
} catch (Exception e) {
127-
String message = "Failed to init TLS infrastructure for remote caching/execution using "
128-
+ "'%s' as root certificate: %s";
129-
message = String.format(message, rootCert, e.getMessage());
130-
throw new IOException(message, e);
131-
}
132-
}
133-
}
134137
}

‎src/main/java/com/google/devtools/build/lib/buildeventservice/BazelBuildEventServiceModule.java‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,10 @@
1616

1717
import com.google.common.collect.ImmutableSet;
1818
import com.google.devtools.build.lib.authandtls.AuthAndTLSOptions;
19+
import com.google.devtools.build.lib.authandtls.GrpcUtils;
1920
import com.google.devtools.build.lib.buildeventservice.client.BuildEventServiceClient;
2021
import com.google.devtools.build.lib.buildeventservice.client.BuildEventServiceGrpcClient;
22+
import java.io.IOException;
2123
import java.util.Set;
2224

2325
/**
@@ -33,11 +35,10 @@ protected Class<BuildEventServiceOptions> optionsClass() {
3335

3436
@Override
3537
protected BuildEventServiceClient createBesClient(BuildEventServiceOptions besOptions,
36-
AuthAndTLSOptions authAndTLSOptions) {
38+
AuthAndTLSOptions authAndTLSOptions) throws IOException {
3739
return new BuildEventServiceGrpcClient(
38-
besOptions.besBackend, authAndTLSOptions.tlsEnabled, authAndTLSOptions.tlsCertificate,
39-
authAndTLSOptions.tlsAuthorityOverride, authAndTLSOptions.authCredentials,
40-
authAndTLSOptions.authScope);
40+
GrpcUtils.newChannel(besOptions.besBackend, authAndTLSOptions),
41+
GrpcUtils.newCallCredentials(authAndTLSOptions));
4142
}
4243

4344
@Override

‎src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceModule.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
import com.google.devtools.build.lib.util.io.OutErr;
4343
import com.google.devtools.common.options.OptionsBase;
4444
import com.google.devtools.common.options.OptionsProvider;
45+
import java.io.IOException;
4546
import java.util.Set;
4647
import java.util.UUID;
4748
import java.util.logging.Logger;
@@ -184,7 +185,7 @@ BuildEventStreamer tryCreateStreamer(
184185
@Nullable
185186
private BuildEventTransport tryCreateBesTransport(T besOptions, AuthAndTLSOptions authTlsOptions,
186187
String buildRequestId, String invocationId, ModuleEnvironment moduleEnvironment, Clock clock,
187-
PathConverter pathConverter, EventHandler commandLineReporter) {
188+
PathConverter pathConverter, EventHandler commandLineReporter) throws IOException {
188189
if (isNullOrEmpty(besOptions.besBackend)) {
189190
logger.fine("BuildEventServiceTransport is disabled.");
190191
return null;
@@ -222,7 +223,7 @@ private BuildEventTransport tryCreateBesTransport(T besOptions, AuthAndTLSOption
222223
protected abstract Class<T> optionsClass();
223224

224225
protected abstract BuildEventServiceClient createBesClient(T besOptions,
225-
AuthAndTLSOptions authAndTLSOptions);
226+
AuthAndTLSOptions authAndTLSOptions) throws IOException;
226227

227228
protected abstract Set<String> whitelistedCommands();
228229
}

‎src/main/java/com/google/devtools/build/lib/buildeventservice/client/BuildEventServiceGrpcClient.java‎

Lines changed: 0 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -14,17 +14,12 @@
1414

1515
package com.google.devtools.build.lib.buildeventservice.client;
1616

17-
import static com.google.common.base.Strings.isNullOrEmpty;
1817
import static com.google.devtools.build.lib.util.Preconditions.checkNotNull;
1918
import static com.google.devtools.build.lib.util.Preconditions.checkState;
20-
import static java.lang.System.getenv;
21-
import static java.nio.file.Files.newInputStream;
2219
import static java.util.concurrent.TimeUnit.MILLISECONDS;
2320

24-
import com.google.auth.oauth2.GoogleCredentials;
2521
import com.google.common.base.Function;
2622
import com.google.common.base.Throwables;
27-
import com.google.common.collect.ImmutableList;
2823
import com.google.common.util.concurrent.ListenableFuture;
2924
import com.google.common.util.concurrent.SettableFuture;
3025
import com.google.devtools.build.v1.PublishBuildEventGrpc;
@@ -37,49 +32,23 @@
3732
import io.grpc.ManagedChannel;
3833
import io.grpc.Status;
3934
import io.grpc.StatusRuntimeException;
40-
import io.grpc.auth.MoreCallCredentials;
41-
import io.grpc.netty.GrpcSslContexts;
42-
import io.grpc.netty.NegotiationType;
43-
import io.grpc.netty.NettyChannelBuilder;
4435
import io.grpc.stub.AbstractStub;
4536
import io.grpc.stub.StreamObserver;
46-
import io.netty.handler.ssl.SslContext;
47-
import java.io.File;
48-
import java.io.IOException;
49-
import java.nio.file.Paths;
5037
import java.util.concurrent.atomic.AtomicReference;
51-
import java.util.logging.Level;
52-
import java.util.logging.Logger;
5338
import javax.annotation.Nullable;
54-
import javax.net.ssl.SSLException;
5539
import org.joda.time.Duration;
5640

5741
/** Implementation of BuildEventServiceClient that uploads data using gRPC. */
5842
public class BuildEventServiceGrpcClient implements BuildEventServiceClient {
5943

60-
private static final Logger logger =
61-
Logger.getLogger(BuildEventServiceGrpcClient.class.getName());
62-
6344
/** Max wait time for a single non-streaming RPC to finish */
6445
private static final Duration RPC_TIMEOUT = Duration.standardSeconds(15);
65-
/** See https://developers.google.com/identity/protocols/application-default-credentials * */
66-
private static final String DEFAULT_APP_CREDENTIALS_ENV_VAR = "GOOGLE_APPLICATION_CREDENTIALS";
67-
/** TODO(eduardocolaco): Scope documentation.* */
68-
private static final String CREDENTIALS_SCOPE =
69-
"https://www.googleapis.com/auth/cloud-build-service";
7046

7147
private final PublishBuildEventStub besAsync;
7248
private final PublishBuildEventBlockingStub besBlocking;
7349
private final ManagedChannel channel;
7450
private final AtomicReference<StreamObserver<PublishBuildToolEventStreamRequest>> streamReference;
7551

76-
public BuildEventServiceGrpcClient(String serverSpec, boolean tlsEnabled,
77-
@Nullable String tlsCertificateFile, @Nullable String tlsAuthorityOverride,
78-
@Nullable String credentialsFile, @Nullable String credentialsScope) {
79-
this(getChannel(serverSpec, tlsEnabled, tlsCertificateFile, tlsAuthorityOverride),
80-
getCallCredentials(credentialsFile, credentialsScope));
81-
}
82-
8352
public BuildEventServiceGrpcClient(
8453
ManagedChannel channel,
8554
@Nullable CallCredentials callCredentials) {
@@ -183,52 +152,4 @@ public String userReadableError(Throwable t) {
183152
return t.getMessage();
184153
}
185154
}
186-
187-
/**
188-
* Returns call credentials read from the specified file (if non-empty) or from
189-
* env(GOOGLE_APPLICATION_CREDENTIALS) otherwise.
190-
*/
191-
@Nullable
192-
private static CallCredentials getCallCredentials(@Nullable String credentialsFile,
193-
@Nullable String credentialsScope) {
194-
String effectiveScope = credentialsScope != null ? credentialsScope : CREDENTIALS_SCOPE;
195-
try {
196-
if (!isNullOrEmpty(credentialsFile)) {
197-
return MoreCallCredentials.from(
198-
GoogleCredentials.fromStream(newInputStream(Paths.get(credentialsFile)))
199-
.createScoped(ImmutableList.of(effectiveScope)));
200-
201-
} else if (!isNullOrEmpty(getenv(DEFAULT_APP_CREDENTIALS_ENV_VAR))) {
202-
return MoreCallCredentials.from(
203-
GoogleCredentials.getApplicationDefault()
204-
.createScoped(ImmutableList.of(effectiveScope)));
205-
}
206-
} catch (IOException e) {
207-
logger.log(Level.WARNING, "Failed to read credentials", e);
208-
}
209-
return null;
210-
}
211-
212-
/**
213-
* Returns a ManagedChannel to the specified server.
214-
*/
215-
private static ManagedChannel getChannel(String serverSpec, boolean tlsEnabled,
216-
@Nullable String tlsCertificateFile, @Nullable String tlsAuthorityOverride) {
217-
//TODO(buchgr): Use ManagedChannelBuilder once bazel uses a newer gRPC version.
218-
NettyChannelBuilder builder = NettyChannelBuilder.forTarget(serverSpec);
219-
builder.negotiationType(tlsEnabled ? NegotiationType.TLS : NegotiationType.PLAINTEXT);
220-
if (tlsCertificateFile != null) {
221-
try {
222-
SslContext sslContext =
223-
GrpcSslContexts.forClient().trustManager(new File(tlsCertificateFile)).build();
224-
builder.sslContext(sslContext);
225-
} catch (SSLException e) {
226-
throw new RuntimeException(e);
227-
}
228-
}
229-
if (tlsAuthorityOverride != null) {
230-
builder.overrideAuthority(tlsAuthorityOverride);
231-
}
232-
return builder.build();
233-
}
234155
}

0 commit comments

Comments
 (0)