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 d91075c5b5ea3c..006b6f67601b2f 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 @@ -24,16 +24,18 @@ import com.google.devtools.build.lib.authandtls.GoogleAuthUtils; import com.google.devtools.build.lib.buildeventservice.client.BuildEventServiceClient; import com.google.devtools.build.lib.buildeventservice.client.BuildEventServiceGrpcClient; +import io.grpc.ClientInterceptor; import io.grpc.ManagedChannel; +import io.grpc.Metadata; +import io.grpc.stub.MetadataUtils; import java.io.IOException; import java.util.Map; +import java.util.Map.Entry; import java.util.Objects; import java.util.Set; import javax.annotation.Nullable; -/** - * Bazel's BES module. - */ +/** Bazel's BES module. */ public class BazelBuildEventServiceModule extends BuildEventServiceModule { @@ -47,6 +49,15 @@ abstract static class BackendConfig { abstract ImmutableList> besHeaders(); abstract AuthAndTLSOptions authAndTLSOptions(); + + static BackendConfig create( + BuildEventServiceOptions besOptions, AuthAndTLSOptions authAndTLSOptions) { + return new AutoValue_BazelBuildEventServiceModule_BackendConfig( + besOptions.besBackend, + besOptions.besProxy, + ImmutableMap.copyOf(besOptions.besHeaders).entrySet().asList(), + authAndTLSOptions); + } } private BuildEventServiceClient client; @@ -60,12 +71,7 @@ protected Class optionsClass() { @Override protected BuildEventServiceClient getBesClient( BuildEventServiceOptions besOptions, AuthAndTLSOptions authAndTLSOptions) throws IOException { - BackendConfig newConfig = - new AutoValue_BazelBuildEventServiceModule_BackendConfig( - besOptions.besBackend, - besOptions.besProxy, - ImmutableMap.copyOf(besOptions.besHeaders).entrySet().asList(), - authAndTLSOptions); + BackendConfig newConfig = BackendConfig.create(besOptions, authAndTLSOptions); if (client == null || !Objects.equals(config, newConfig)) { clearBesClient(); config = newConfig; @@ -73,11 +79,28 @@ protected BuildEventServiceClient getBesClient( new BuildEventServiceGrpcClient( newGrpcChannel(config), GoogleAuthUtils.newCallCredentials(config.authAndTLSOptions()), - config.besHeaders()); + makeGrpcInterceptor(config)); } return client; } + private static ClientInterceptor makeGrpcInterceptor(BackendConfig config) { + if (config.besHeaders().isEmpty()) { + return null; + } + return MetadataUtils.newAttachHeadersInterceptor(makeGrpcMetadata(config)); + } + + @VisibleForTesting + static Metadata makeGrpcMetadata(BackendConfig config) { + Metadata extraHeaders = new Metadata(); + for (Entry header : config.besHeaders()) { + extraHeaders.put( + Metadata.Key.of(header.getKey(), Metadata.ASCII_STRING_MARSHALLER), header.getValue()); + } + return extraHeaders; + } + // newGrpcChannel is only defined so it can be overridden in tests to not use a real network link. @VisibleForTesting protected ManagedChannel newGrpcChannel(BackendConfig config) throws IOException { diff --git a/src/main/java/com/google/devtools/build/lib/buildeventservice/client/BuildEventServiceGrpcClient.java b/src/main/java/com/google/devtools/build/lib/buildeventservice/client/BuildEventServiceGrpcClient.java index 69598ff254f03c..a71964f7d8a386 100644 --- a/src/main/java/com/google/devtools/build/lib/buildeventservice/client/BuildEventServiceGrpcClient.java +++ b/src/main/java/com/google/devtools/build/lib/buildeventservice/client/BuildEventServiceGrpcClient.java @@ -30,16 +30,12 @@ import io.grpc.CallCredentials; import io.grpc.ClientInterceptor; import io.grpc.ManagedChannel; -import io.grpc.Metadata; import io.grpc.Status; import io.grpc.StatusException; import io.grpc.StatusRuntimeException; import io.grpc.stub.AbstractStub; -import io.grpc.stub.MetadataUtils; import io.grpc.stub.StreamObserver; import java.time.Duration; -import java.util.List; -import java.util.Map; import javax.annotation.Nullable; /** Implementation of BuildEventServiceClient that uploads data using gRPC. */ @@ -55,17 +51,7 @@ public class BuildEventServiceGrpcClient implements BuildEventServiceClient { public BuildEventServiceGrpcClient( ManagedChannel channel, @Nullable CallCredentials callCredentials, - List> headers) { - ClientInterceptor interceptor = null; - if (!headers.isEmpty()) { - Metadata extraHeaders = new Metadata(); - headers.forEach( - header -> - extraHeaders.put( - Metadata.Key.of(header.getKey(), Metadata.ASCII_STRING_MARSHALLER), - header.getValue())); - interceptor = MetadataUtils.newAttachHeadersInterceptor(extraHeaders); - } + ClientInterceptor interceptor) { this.besAsync = configureStub(PublishBuildEventGrpc.newStub(channel), callCredentials, interceptor); this.besBlocking = 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 53b47d9beacf62..9fa5d10224d7ec 100644 --- a/src/test/java/com/google/devtools/build/lib/buildeventservice/BUILD +++ b/src/test/java/com/google/devtools/build/lib/buildeventservice/BUILD @@ -26,7 +26,6 @@ java_test( "//src/test/java/com/google/devtools/build/lib/testutil", "//third_party:guava", "//third_party:junit4", - "//third_party:mockito", "//third_party:truth", "//third_party/grpc:grpc-jar", "//third_party/protobuf:protobuf_java", @@ -48,6 +47,7 @@ java_test( deps = [ "//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/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 01c314ebed38f6..30bd50524a80c7 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 @@ -23,12 +23,15 @@ import com.google.common.base.MoreObjects; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; import com.google.common.collect.Sets; import com.google.common.util.concurrent.Uninterruptibles; 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.bugreport.BugReport; +import com.google.devtools.build.lib.buildeventservice.BazelBuildEventServiceModule.BackendConfig; import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploader; import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos; import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.Aborted; @@ -66,6 +69,7 @@ import com.google.devtools.build.v1.StreamId; import com.google.protobuf.Empty; import io.grpc.ManagedChannel; +import io.grpc.Metadata; import io.grpc.Server; import io.grpc.Status; import io.grpc.StatusRuntimeException; @@ -588,6 +592,26 @@ public void testKeywords() throws Exception { .containsExactly("user_keyword=keyword0", "user_keyword=keyword1"); } + @Test + public void testMakeGrpcMetadata() throws Exception { + runBuildWithOptions(); + BuildEventServiceOptions besOptions = new BuildEventServiceOptions(); + AuthAndTLSOptions authAndTLSOptions = new AuthAndTLSOptions(); + besOptions.besBackend = "bes-backend"; + besOptions.besProxy = "bes-proxy"; + besOptions.besHeaders = + ImmutableMap.of("key1", "val1", "key2", "val2", "key3", "val3").entrySet().asList(); + BackendConfig newConfig = BackendConfig.create(besOptions, authAndTLSOptions); + + Metadata metadata = BazelBuildEventServiceModule.makeGrpcMetadata(newConfig); + assertThat(metadata.get(Metadata.Key.of("key1", Metadata.ASCII_STRING_MARSHALLER))) + .isEqualTo("val1"); + assertThat(metadata.get(Metadata.Key.of("key2", Metadata.ASCII_STRING_MARSHALLER))) + .isEqualTo("val2"); + assertThat(metadata.get(Metadata.Key.of("key3", Metadata.ASCII_STRING_MARSHALLER))) + .isEqualTo("val3"); + } + /** Regression test for b/111653523. */ @Test public void testCoverageFileIncluded() throws Exception { diff --git a/src/test/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceGrpcClientTest.java b/src/test/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceGrpcClientTest.java index 5689c103a70350..80b8d039fb7766 100644 --- a/src/test/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceGrpcClientTest.java +++ b/src/test/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceGrpcClientTest.java @@ -16,12 +16,11 @@ import static com.google.common.truth.Truth.assertThat; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.Maps; import com.google.devtools.build.lib.buildeventservice.client.BuildEventServiceGrpcClient; import com.google.devtools.build.v1.PublishBuildEventGrpc; import com.google.devtools.build.v1.PublishBuildToolEventStreamRequest; import com.google.devtools.build.v1.PublishBuildToolEventStreamResponse; +import io.grpc.ClientInterceptor; import io.grpc.ManagedChannel; import io.grpc.Metadata; import io.grpc.Server; @@ -34,6 +33,7 @@ import io.grpc.StatusException; import io.grpc.inprocess.InProcessChannelBuilder; import io.grpc.inprocess.InProcessServerBuilder; +import io.grpc.stub.MetadataUtils; import io.grpc.stub.StreamObserver; import java.util.ArrayList; import java.util.UUID; @@ -116,11 +116,11 @@ public ServerCall.Listener interceptCall( return next.startCall(call, headers); } }))) { + Metadata extraHeaders = new Metadata(); + extraHeaders.put(Metadata.Key.of("metadata-foo", Metadata.ASCII_STRING_MARSHALLER), "bar"); + ClientInterceptor interceptor = MetadataUtils.newAttachHeadersInterceptor(extraHeaders); BuildEventServiceGrpcClient grpcClient = - new BuildEventServiceGrpcClient( - server.getChannel(), - null, - ImmutableList.of(Maps.immutableEntry("metadata-foo", "bar"))); + new BuildEventServiceGrpcClient(server.getChannel(), null, interceptor); assertThat(grpcClient.openStream(ack -> {}).getStatus().get()).isEqualTo(Status.OK); assertThat(seenHeaders).hasSize(1); Metadata headers = seenHeaders.get(0); @@ -133,7 +133,7 @@ public ServerCall.Listener interceptCall( public void immediateSuccess() throws Exception { try (TestServer server = startTestServer(NOOP_SERVER.bindService())) { assertThat( - new BuildEventServiceGrpcClient(server.getChannel(), null, ImmutableList.of()) + new BuildEventServiceGrpcClient(server.getChannel(), null, null) .openStream(ack -> {}) .getStatus() .get()) @@ -154,7 +154,7 @@ public StreamObserver publishBuildToolEventS } }.bindService())) { assertThat( - new BuildEventServiceGrpcClient(server.getChannel(), null, ImmutableList.of()) + new BuildEventServiceGrpcClient(server.getChannel(), null, null) .openStream(ack -> {}) .getStatus() .get())