Skip to content

Commit 8e02b21

Browse files
kannanjgithubkannanjgithub
committed
Remove handling of grpc service config initial metadata headers as per update in the grfc.
1 parent bdcf52c commit 8e02b21

5 files changed

Lines changed: 3 additions & 185 deletions

File tree

‎xds/src/main/java/io/grpc/xds/ExternalProcessorFilter.java‎

Lines changed: 1 addition & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,6 @@
7676
import io.grpc.internal.SerializingExecutor;
7777
import io.grpc.stub.ClientCallStreamObserver;
7878
import io.grpc.stub.ClientResponseObserver;
79-
import io.grpc.stub.MetadataUtils;
8079
import io.grpc.xds.Filter.FilterConfigParseContext;
8180
import io.grpc.xds.Filter.FilterContext;
8281
import io.grpc.xds.internal.MatcherParser;
@@ -634,29 +633,7 @@ public <ReqT, RespT> ClientCall<ReqT, RespT> interceptCall(
634633
}
635634
}
636635

637-
ImmutableList<HeaderValue> initialMetadata =
638-
filterConfig.getGrpcServiceConfig().initialMetadata();
639-
if (!initialMetadata.isEmpty()) {
640-
Metadata extraHeaders = new Metadata();
641-
for (HeaderValue headerValue : initialMetadata) {
642-
String key = headerValue.key();
643-
if (key.endsWith(Metadata.BINARY_HEADER_SUFFIX)) {
644-
if (headerValue.rawValue().isPresent()) {
645-
Metadata.Key<byte[]> metadataKey =
646-
Metadata.Key.of(key, Metadata.BINARY_BYTE_MARSHALLER);
647-
extraHeaders.put(metadataKey, headerValue.rawValue().get().toByteArray());
648-
}
649-
} else {
650-
if (headerValue.value().isPresent()) {
651-
Metadata.Key<String> metadataKey =
652-
Metadata.Key.of(key, Metadata.ASCII_STRING_MARSHALLER);
653-
extraHeaders.put(metadataKey, headerValue.value().get());
654-
}
655-
}
656-
}
657-
extProcStub = extProcStub.withInterceptors(
658-
MetadataUtils.newAttachHeadersInterceptor(extraHeaders));
659-
}
636+
660637

661638
// The filter chain is preceded by RawMessageClientInterceptor, so ReqT and RespT are
662639
// InputStream.

‎xds/src/main/java/io/grpc/xds/GrpcServiceConfigParser.java‎

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,6 @@
3939
import io.grpc.xds.client.ConfiguredChannelCredentials;
4040
import io.grpc.xds.internal.grpcservice.GrpcServiceConfig;
4141
import io.grpc.xds.internal.grpcservice.GrpcServiceParseException;
42-
import io.grpc.xds.internal.grpcservice.HeaderValue;
43-
import io.grpc.xds.internal.grpcservice.HeaderValueValidationUtils;
4442
import java.net.URI;
4543
import java.net.URISyntaxException;
4644
import java.time.Duration;
@@ -93,22 +91,7 @@ public static GrpcServiceConfig parse(GrpcService grpcServiceProto,
9391

9492
GrpcServiceConfig.Builder builder = GrpcServiceConfig.builder().googleGrpc(googleGrpcConfig);
9593

96-
ImmutableList.Builder<HeaderValue> initialMetadata = ImmutableList.builder();
97-
for (io.envoyproxy.envoy.config.core.v3.HeaderValue header : grpcServiceProto
98-
.getInitialMetadataList()) {
99-
String key = header.getKey();
100-
HeaderValue headerValue;
101-
if (key.endsWith(Metadata.BINARY_HEADER_SUFFIX)) {
102-
headerValue = HeaderValue.create(key, header.getRawValue());
103-
} else {
104-
headerValue = HeaderValue.create(key, header.getValue());
105-
}
106-
if (HeaderValueValidationUtils.isDisallowed(headerValue)) {
107-
throw new GrpcServiceParseException("Invalid initial metadata header: " + key);
108-
}
109-
initialMetadata.add(headerValue);
110-
}
111-
builder.initialMetadata(initialMetadata.build());
94+
11295

11396
if (grpcServiceProto.hasTimeout()) {
11497
com.google.protobuf.Duration timeout = grpcServiceProto.getTimeout();

‎xds/src/main/java/io/grpc/xds/internal/grpcservice/GrpcServiceConfig.java‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
package io.grpc.xds.internal.grpcservice;
1818

1919
import com.google.auto.value.AutoValue;
20-
import com.google.common.collect.ImmutableList;
2120
import io.grpc.CallCredentials;
2221
import io.grpc.xds.client.ConfiguredChannelCredentials;
2322
import java.time.Duration;
@@ -40,16 +39,12 @@ public static Builder builder() {
4039

4140
public abstract Optional<Duration> timeout();
4241

43-
public abstract ImmutableList<HeaderValue> initialMetadata();
44-
4542
@AutoValue.Builder
4643
public abstract static class Builder {
4744
public abstract Builder googleGrpc(GoogleGrpcConfig googleGrpc);
4845

4946
public abstract Builder timeout(Duration timeout);
5047

51-
public abstract Builder initialMetadata(ImmutableList<HeaderValue> initialMetadata);
52-
5348
public abstract GrpcServiceConfig build();
5449
}
5550

‎xds/src/test/java/io/grpc/xds/ExternalProcessorFilterTest.java‎

Lines changed: 0 additions & 108 deletions
Original file line numberDiff line numberDiff line change
@@ -907,115 +907,7 @@ public <ReqT, RespT> ClientCall<ReqT, RespT> interceptCall(
907907
channelManager.close();
908908
}
909909

910-
@Test
911-
@SuppressWarnings("unchecked")
912-
public void givenGrpcServiceWithInitialMetadata_whenCallIntercepted_thenSendsMetadata()
913-
throws Exception {
914-
String uniqueExtProcServerName = InProcessServerBuilder.generateName();
915-
String uniqueDataPlaneServerName = InProcessServerBuilder.generateName();
916-
ExternalProcessor proto = ExternalProcessor.newBuilder()
917-
.setGrpcService(GrpcService.newBuilder()
918-
.setGoogleGrpc(GrpcService.GoogleGrpc.newBuilder()
919-
.setTargetUri("in-process:///" + uniqueExtProcServerName)
920-
.addChannelCredentialsPlugin(Any.newBuilder()
921-
.setTypeUrl("type.googleapis.com/envoy.extensions.grpc_service."
922-
+ "channel_credentials.insecure.v3.InsecureCredentials")
923-
.build())
924-
.build())
925-
.addInitialMetadata(io.envoyproxy.envoy.config.core.v3.HeaderValue.newBuilder()
926-
.setKey("x-init-key").setValue("init-val").build())
927-
.addInitialMetadata(
928-
io.envoyproxy.envoy.config.core.v3.HeaderValue.newBuilder()
929-
.setKey("x-bin-key-bin")
930-
.setRawValue(ByteString.copyFrom(new byte[] {1, 2, 3}))
931-
.build())
932-
.build())
933-
.build();
934-
ConfigOrError<ExternalProcessorFilterConfig> configOrError =
935-
provider.parseFilterConfig(Any.pack(proto), filterContext);
936-
assertThat(configOrError.errorDetail).isNull();
937-
ExternalProcessorFilterConfig filterConfig = configOrError.config;
938-
939-
// External Processor Server
940-
final AtomicReference<Metadata> capturedHeaders = new AtomicReference<>();
941-
ExternalProcessorGrpc.ExternalProcessorImplBase extProcImpl;
942-
extProcImpl = new ExternalProcessorGrpc.ExternalProcessorImplBase() {
943-
@Override
944-
@SuppressWarnings("unchecked")
945-
public StreamObserver<ProcessingRequest> process(
946-
StreamObserver<ProcessingResponse> responseObserver) {
947-
((ServerCallStreamObserver<ProcessingResponse>) responseObserver).request(100);
948-
return new StreamObserver<ProcessingRequest>() {
949-
@Override
950-
public void onNext(ProcessingRequest request) {
951-
}
952910

953-
@Override
954-
public void onError(Throwable t) {
955-
}
956-
957-
@Override
958-
public void onCompleted() {
959-
responseObserver.onCompleted();
960-
}
961-
};
962-
}
963-
};
964-
965-
ServerServiceDefinition interceptedExtProc = ServerInterceptors.intercept(
966-
extProcImpl,
967-
new ServerInterceptor() {
968-
@Override
969-
public <ReqT, RespT> ServerCall.Listener<ReqT> interceptCall(
970-
ServerCall<ReqT, RespT> call, Metadata headers, ServerCallHandler<ReqT, RespT> next) {
971-
capturedHeaders.set(headers);
972-
return next.startCall(call, headers);
973-
}
974-
});
975-
976-
grpcCleanup.register(InProcessServerBuilder.forName(uniqueExtProcServerName)
977-
.addService(interceptedExtProc)
978-
.directExecutor()
979-
.build().start());
980-
981-
CachedChannelManager channelManager = new CachedChannelManager(config -> {
982-
return grpcCleanup.register(
983-
InProcessChannelBuilder.forName(uniqueExtProcServerName).directExecutor().build());
984-
});
985-
986-
ExternalProcessorInterceptor interceptor = new ExternalProcessorInterceptor(
987-
filterConfig, channelManager, scheduler, FAKE_CONTEXT);
988-
989-
MutableHandlerRegistry uniqueRegistry = new MutableHandlerRegistry();
990-
grpcCleanup.register(InProcessServerBuilder.forName(uniqueDataPlaneServerName)
991-
.fallbackHandlerRegistry(uniqueRegistry)
992-
.directExecutor()
993-
.build().start());
994-
995-
ManagedChannel dataPlaneChannel =
996-
grpcCleanup.register(
997-
InProcessChannelBuilder.forName(uniqueDataPlaneServerName).directExecutor().build());
998-
999-
CallOptions callOptions = DEFAULT_CALL_OPTIONS.withExecutor(MoreExecutors.directExecutor());
1000-
ClientCall<String, String> proxyCall =
1001-
interceptCall(interceptor, METHOD_SAY_HELLO, callOptions, dataPlaneChannel);
1002-
proxyCall.start(new ClientCall.Listener<String>() {}, new Metadata());
1003-
1004-
assertThat(capturedHeaders.get()).isNotNull();
1005-
assertThat(
1006-
capturedHeaders
1007-
.get()
1008-
.get(Metadata.Key.of("x-init-key", Metadata.ASCII_STRING_MARSHALLER)))
1009-
.isEqualTo("init-val");
1010-
assertThat(
1011-
capturedHeaders
1012-
.get()
1013-
.get(Metadata.Key.of("x-bin-key-bin", Metadata.BINARY_BYTE_MARSHALLER)))
1014-
.isEqualTo(new byte[] {1, 2, 3});
1015-
1016-
proxyCall.cancel("Cleanup", null);
1017-
channelManager.close();
1018-
}
1019911

1020912
// --- Category 4: Request Header Processing ---
1021913

‎xds/src/test/java/io/grpc/xds/GrpcServiceConfigParserTest.java‎

Lines changed: 1 addition & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -108,15 +108,9 @@ public void parse_success() throws GrpcServiceParseException {
108108
GrpcService.GoogleGrpc googleGrpc = GrpcService.GoogleGrpc.newBuilder().setTargetUri("test_uri")
109109
.addChannelCredentialsPlugin(insecureCreds).addCallCredentialsPlugin(accessTokenCreds)
110110
.build();
111-
HeaderValue asciiHeader =
112-
HeaderValue.newBuilder().setKey("test_key").setValue("test_value").build();
113-
HeaderValue binaryHeader =
114-
HeaderValue.newBuilder().setKey("test_key-bin").setRawValue(ByteString
115-
.copyFrom("test_value_binary".getBytes(StandardCharsets.UTF_8))).build();
116111
Duration timeout = Duration.newBuilder().setSeconds(10).build();
117112
GrpcService grpcService =
118-
GrpcService.newBuilder().setGoogleGrpc(googleGrpc).addInitialMetadata(asciiHeader)
119-
.addInitialMetadata(binaryHeader).setTimeout(timeout).build();
113+
GrpcService.newBuilder().setGoogleGrpc(googleGrpc).setTimeout(timeout).build();
120114

121115
GrpcServiceConfig config = parse(grpcService,
122116
dummyBootstrapInfo(),
@@ -138,14 +132,6 @@ public void parse_success() throws GrpcServiceParseException {
138132
assertThat(config.googleGrpc().callCredentials().get().getClass().getName())
139133
.isEqualTo(CALL_CREDENTIALS_CLASS_NAME);
140134

141-
// Assert initial metadata
142-
assertThat(config.initialMetadata()).isNotEmpty();
143-
assertThat(config.initialMetadata().get(0).key()).isEqualTo("test_key");
144-
assertThat(config.initialMetadata().get(0).value().get()).isEqualTo("test_value");
145-
assertThat(config.initialMetadata().get(1).key()).isEqualTo("test_key-bin");
146-
assertThat(config.initialMetadata().get(1).rawValue().get().toByteArray())
147-
.isEqualTo("test_value_binary".getBytes(StandardCharsets.UTF_8));
148-
149135
// Assert timeout
150136
assertThat(config.timeout().isPresent()).isTrue();
151137
assertThat(config.timeout().get()).isEqualTo(java.time.Duration.ofSeconds(10));
@@ -166,7 +152,6 @@ public void parse_minimalSuccess_defaults() throws GrpcServiceParseException {
166152
dummyServerInfo());
167153

168154
assertThat(config.googleGrpc().target()).isEqualTo("test_uri");
169-
assertThat(config.initialMetadata()).isEmpty();
170155
assertThat(config.timeout().isPresent()).isFalse();
171156
}
172157

@@ -463,20 +448,6 @@ public void parseGoogleGrpcConfig_unsupportedScheme() {
463448
.contains("Target URI scheme is not resolvable");
464449
}
465450

466-
@Test
467-
public void parse_disallowedInitialMetadata() {
468-
Any insecureCreds = Any.pack(InsecureCredentials.getDefaultInstance());
469-
GrpcService.GoogleGrpc googleGrpc = GrpcService.GoogleGrpc.newBuilder().setTargetUri("test_uri")
470-
.addChannelCredentialsPlugin(insecureCreds).build();
471-
HeaderValue disallowedHeader =
472-
HeaderValue.newBuilder().setKey("host").setValue("test_value").build();
473-
GrpcService grpcService = GrpcService.newBuilder().setGoogleGrpc(googleGrpc)
474-
.addInitialMetadata(disallowedHeader).build();
475-
476-
GrpcServiceParseException exception = assertThrows(GrpcServiceParseException.class,
477-
() -> parse(grpcService, dummyBootstrapInfo(), dummyServerInfo()));
478-
assertThat(exception).hasMessageThat().contains("Invalid initial metadata header: host");
479-
}
480451

481452
@Test
482453
public void parse_invalidDuration() {

0 commit comments

Comments
 (0)