Skip to content

Commit 82021c2

Browse files
Zgoda91Zgoda91
committed
Postponed ChannelCredentials instantiation.
Lifetime of `ChannelCredentials` is now bounded to `GrpcXdsTransport`
1 parent 52c06c0 commit 82021c2

19 files changed

+134
-98
lines changed

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

Lines changed: 15 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818

1919
import com.google.common.annotations.VisibleForTesting;
2020
import com.google.common.collect.ImmutableMap;
21-
import io.grpc.ChannelCredentials;
21+
import com.google.common.collect.ImmutableSet;
2222
import io.grpc.internal.JsonUtil;
2323
import io.grpc.xds.client.BootstrapperImpl;
2424
import io.grpc.xds.client.XdsInitializationException;
@@ -90,47 +90,43 @@ protected String getJsonContent() throws XdsInitializationException, IOException
9090
}
9191

9292
@Override
93-
protected Object getImplSpecificConfig(Map<String, ?> serverConfig, String serverUri)
93+
protected ImmutableMap<String, ?> getImplSpecificConfig(Map<String, ?> serverConfig,
94+
String serverUri)
9495
throws XdsInitializationException {
95-
return getChannelCredentials(serverConfig, serverUri);
96+
return getChannelCredentialsConfig(serverConfig, serverUri);
9697
}
9798

98-
private static ChannelCredentials getChannelCredentials(Map<String, ?> serverConfig,
99-
String serverUri)
99+
private static ImmutableMap<String, ?> getChannelCredentialsConfig(Map<String, ?> serverConfig,
100+
String serverUri)
100101
throws XdsInitializationException {
101102
List<?> rawChannelCredsList = JsonUtil.getList(serverConfig, "channel_creds");
102103
if (rawChannelCredsList == null || rawChannelCredsList.isEmpty()) {
103104
throw new XdsInitializationException(
104105
"Invalid bootstrap: server " + serverUri + " 'channel_creds' required");
105106
}
106-
ChannelCredentials channelCredentials =
107+
ImmutableMap<String, ?> channelCredentialsConfig =
107108
parseChannelCredentials(JsonUtil.checkObjectList(rawChannelCredsList), serverUri);
108-
if (channelCredentials == null) {
109+
if (channelCredentialsConfig == null) {
109110
throw new XdsInitializationException(
110111
"Server " + serverUri + ": no supported channel credentials found");
111112
}
112-
return channelCredentials;
113+
return channelCredentialsConfig;
113114
}
114115

115116
@Nullable
116-
private static ChannelCredentials parseChannelCredentials(List<Map<String, ?>> jsonList,
117-
String serverUri)
117+
private static ImmutableMap<String, ?> parseChannelCredentials(List<Map<String, ?>> jsonList,
118+
String serverUri)
118119
throws XdsInitializationException {
119120
for (Map<String, ?> channelCreds : jsonList) {
120121
String type = JsonUtil.getString(channelCreds, "type");
121122
if (type == null) {
122123
throw new XdsInitializationException(
123124
"Invalid bootstrap: server " + serverUri + " with 'channel_creds' type unspecified");
124125
}
125-
XdsCredentialsProvider provider = XdsCredentialsRegistry.getDefaultRegistry()
126-
.getProvider(type);
127-
if (provider != null) {
128-
Map<String, ?> config = JsonUtil.getObject(channelCreds, "config");
129-
if (config == null) {
130-
config = ImmutableMap.of();
131-
}
132-
133-
return provider.newChannelCredentials(config);
126+
ImmutableSet<String> supportedNames = XdsCredentialsRegistry.getDefaultRegistry()
127+
.getSupportedCredentialNames();
128+
if (supportedNames.contains(type)) {
129+
return ImmutableMap.copyOf(channelCreds);
134130
}
135131
}
136132
return null;

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

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919
import static com.google.common.base.Preconditions.checkNotNull;
2020

2121
import com.google.common.annotations.VisibleForTesting;
22+
import com.google.common.collect.ImmutableList;
23+
import com.google.common.collect.ImmutableMap;
2224
import io.grpc.CallCredentials;
2325
import io.grpc.CallOptions;
2426
import io.grpc.ChannelCredentials;
@@ -28,9 +30,15 @@
2830
import io.grpc.ManagedChannel;
2931
import io.grpc.Metadata;
3032
import io.grpc.MethodDescriptor;
33+
import io.grpc.ResourceAllocatingChannelCredentials;
3134
import io.grpc.Status;
35+
import io.grpc.internal.GrpcUtil;
36+
import io.grpc.internal.JsonUtil;
3237
import io.grpc.xds.client.Bootstrapper;
38+
import io.grpc.xds.client.XdsInitializationException;
3339
import io.grpc.xds.client.XdsTransportFactory;
40+
import java.io.Closeable;
41+
import java.util.Map;
3442
import java.util.concurrent.TimeUnit;
3543

3644
final class GrpcXdsTransportFactory implements XdsTransportFactory {
@@ -42,7 +50,7 @@ final class GrpcXdsTransportFactory implements XdsTransportFactory {
4250
}
4351

4452
@Override
45-
public XdsTransport create(Bootstrapper.ServerInfo serverInfo) {
53+
public XdsTransport create(Bootstrapper.ServerInfo serverInfo) throws XdsInitializationException {
4654
return new GrpcXdsTransport(serverInfo, callCredentials);
4755
}
4856

@@ -56,8 +64,9 @@ static class GrpcXdsTransport implements XdsTransport {
5664

5765
private final ManagedChannel channel;
5866
private final CallCredentials callCredentials;
67+
private final ImmutableList<Closeable> resources;
5968

60-
public GrpcXdsTransport(Bootstrapper.ServerInfo serverInfo) {
69+
public GrpcXdsTransport(Bootstrapper.ServerInfo serverInfo) throws XdsInitializationException {
6170
this(serverInfo, null);
6271
}
6372

@@ -66,9 +75,27 @@ public GrpcXdsTransport(ManagedChannel channel) {
6675
this(channel, null);
6776
}
6877

69-
public GrpcXdsTransport(Bootstrapper.ServerInfo serverInfo, CallCredentials callCredentials) {
78+
public GrpcXdsTransport(Bootstrapper.ServerInfo serverInfo, CallCredentials callCredentials)
79+
throws XdsInitializationException {
7080
String target = serverInfo.target();
71-
ChannelCredentials channelCredentials = (ChannelCredentials) serverInfo.implSpecificConfig();
81+
Map<String, ?> implSpecificConfig = serverInfo.implSpecificConfig();
82+
String type = JsonUtil.getString(implSpecificConfig, "type");
83+
XdsCredentialsProvider provider = XdsCredentialsRegistry.getDefaultRegistry()
84+
.getProvider(type);
85+
Map<String, ?> config = JsonUtil.getObject(implSpecificConfig, "config");
86+
if (config == null) {
87+
config = ImmutableMap.of();
88+
}
89+
ChannelCredentials channelCredentials = provider.newChannelCredentials(config);
90+
if (channelCredentials == null) {
91+
throw new XdsInitializationException(
92+
"Cannot create channel credentials of type " + type + " for target " + target);
93+
}
94+
// if {@code ChannelCredentials} instance has allocated resource of any type, save them to be
95+
// released once the channel is shutdown
96+
this.resources = (channelCredentials instanceof ResourceAllocatingChannelCredentials)
97+
? ((ResourceAllocatingChannelCredentials) channelCredentials).getAllocatedResources()
98+
: ImmutableList.<Closeable>of();
7299
this.channel = Grpc.newChannelBuilder(target, channelCredentials)
73100
.keepAliveTime(5, TimeUnit.MINUTES)
74101
.build();
@@ -79,6 +106,7 @@ public GrpcXdsTransport(Bootstrapper.ServerInfo serverInfo, CallCredentials call
79106
public GrpcXdsTransport(ManagedChannel channel, CallCredentials callCredentials) {
80107
this.channel = checkNotNull(channel, "channel");
81108
this.callCredentials = callCredentials;
109+
this.resources = ImmutableList.<Closeable>of();
82110
}
83111

84112
@Override
@@ -99,6 +127,9 @@ public <ReqT, RespT> StreamingCall<ReqT, RespT> createStreamingCall(
99127
@Override
100128
public void shutdown() {
101129
channel.shutdown();
130+
for (Closeable resource : resources) {
131+
GrpcUtil.closeQuietly(resource);
132+
}
102133
}
103134

104135
private class XdsStreamingCall<ReqT, RespT> implements

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
import com.google.common.annotations.VisibleForTesting;
2323
import com.google.common.collect.ImmutableMap;
24+
import com.google.common.collect.ImmutableSet;
2425
import com.google.errorprone.annotations.concurrent.GuardedBy;
2526
import io.grpc.InternalServiceProviders;
2627
import java.util.ArrayList;
@@ -147,6 +148,14 @@ public synchronized XdsCredentialsProvider getProvider(String name) {
147148
return effectiveProviders.get(checkNotNull(name, "name"));
148149
}
149150

151+
/**
152+
* Returns list of registered xds credential names. Each provider declares its name via
153+
* {@link XdsCredentialsProvider#getName}.
154+
*/
155+
public synchronized ImmutableSet<String> getSupportedCredentialNames() {
156+
return effectiveProviders.keySet();
157+
}
158+
150159
@VisibleForTesting
151160
static List<Class<?>> getHardCodedClasses() {
152161
// Class.forName(String) is used to remove the need for ProGuard configuration. Note that

‎xds/src/main/java/io/grpc/xds/client/Bootstrapper.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ public BootstrapInfo bootstrap(Map<String, ?> rawData) throws XdsInitializationE
5757
public abstract static class ServerInfo {
5858
public abstract String target();
5959

60-
public abstract Object implSpecificConfig();
60+
public abstract ImmutableMap<String, ?> implSpecificConfig();
6161

6262
public abstract boolean ignoreResourceDeletion();
6363

@@ -66,14 +66,15 @@ public abstract static class ServerInfo {
6666
public abstract boolean resourceTimerIsTransientError();
6767

6868
@VisibleForTesting
69-
public static ServerInfo create(String target, @Nullable Object implSpecificConfig) {
69+
public static ServerInfo create(
70+
String target, @Nullable ImmutableMap<String, ?> implSpecificConfig) {
7071
return new AutoValue_Bootstrapper_ServerInfo(target, implSpecificConfig,
7172
false, false, false);
7273
}
7374

7475
@VisibleForTesting
7576
public static ServerInfo create(
76-
String target, Object implSpecificConfig, boolean ignoreResourceDeletion,
77+
String target, ImmutableMap<String, ?> implSpecificConfig, boolean ignoreResourceDeletion,
7778
boolean isTrustedXdsServer, boolean resourceTimerIsTransientError) {
7879
return new AutoValue_Bootstrapper_ServerInfo(target, implSpecificConfig,
7980
ignoreResourceDeletion, isTrustedXdsServer, resourceTimerIsTransientError);

‎xds/src/main/java/io/grpc/xds/client/BootstrapperImpl.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,8 @@ protected BootstrapperImpl() {
7676

7777
protected abstract String getJsonContent() throws IOException, XdsInitializationException;
7878

79-
protected abstract Object getImplSpecificConfig(Map<String, ?> serverConfig, String serverUri)
80-
throws XdsInitializationException;
79+
protected abstract ImmutableMap<String, ?> getImplSpecificConfig(
80+
Map<String, ?> serverConfig, String serverUri) throws XdsInitializationException;
8181

8282

8383
/**
@@ -253,7 +253,7 @@ private List<ServerInfo> parseServerInfos(List<?> rawServerConfigs, XdsLogger lo
253253
}
254254
logger.log(XdsLogLevel.INFO, "xDS server URI: {0}", serverUri);
255255

256-
Object implSpecificConfig = getImplSpecificConfig(serverConfig, serverUri);
256+
ImmutableMap<String, ?> implSpecificConfig = getImplSpecificConfig(serverConfig, serverUri);
257257

258258
boolean resourceTimerIsTransientError = false;
259259
boolean ignoreResourceDeletion = false;

‎xds/src/main/java/io/grpc/xds/client/XdsTransportFactory.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
*/
2626
@ExperimentalApi("https://github.com/grpc/grpc-java/issues/10823")
2727
public interface XdsTransportFactory {
28-
XdsTransport create(Bootstrapper.ServerInfo serverInfo);
28+
XdsTransport create(Bootstrapper.ServerInfo serverInfo) throws XdsInitializationException;
2929

3030
/**
3131
* Represents transport for xDS communication (e.g., a gRPC channel).

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@
3434
import io.grpc.ConnectivityState;
3535
import io.grpc.ConnectivityStateInfo;
3636
import io.grpc.EquivalentAddressGroup;
37-
import io.grpc.InsecureChannelCredentials;
3837
import io.grpc.LoadBalancer;
3938
import io.grpc.LoadBalancer.CreateSubchannelArgs;
4039
import io.grpc.LoadBalancer.FixedResultPicker;
@@ -116,7 +115,7 @@ public class ClusterImplLoadBalancerTest {
116115
private static final String CLUSTER = "cluster-foo.googleapis.com";
117116
private static final String EDS_SERVICE_NAME = "service.googleapis.com";
118117
private static final ServerInfo LRS_SERVER_INFO =
119-
ServerInfo.create("api.google.com", InsecureChannelCredentials.create());
118+
ServerInfo.create("api.google.com", ImmutableMap.of("type", "insecure"));
120119
private static final Metadata.Key<OrcaLoadReport> ORCA_ENDPOINT_LOAD_METRICS_KEY =
121120
Metadata.Key.of(
122121
"endpoint-load-metrics-bin",

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@
3838
import io.grpc.ConnectivityState;
3939
import io.grpc.EquivalentAddressGroup;
4040
import io.grpc.HttpConnectProxiedSocketAddress;
41-
import io.grpc.InsecureChannelCredentials;
4241
import io.grpc.LoadBalancer;
4342
import io.grpc.LoadBalancer.Helper;
4443
import io.grpc.LoadBalancer.PickResult;
@@ -126,7 +125,7 @@ public class ClusterResolverLoadBalancerTest {
126125
private static final String EDS_SERVICE_NAME2 = "backend-service-bar.googleapis.com";
127126
private static final String DNS_HOST_NAME = "dns-service.googleapis.com";
128127
private static final ServerInfo LRS_SERVER_INFO =
129-
ServerInfo.create("lrs.googleapis.com", InsecureChannelCredentials.create());
128+
ServerInfo.create("lrs.googleapis.com", ImmutableMap.of("type", "insecure"));
130129
private final Locality locality1 =
131130
Locality.create("test-region-1", "test-zone-1", "test-subzone-1");
132131
private final Locality locality2 =

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@
3838
import io.envoyproxy.envoy.service.status.v3.ClientStatusResponse;
3939
import io.envoyproxy.envoy.type.matcher.v3.NodeMatcher;
4040
import io.grpc.Deadline;
41-
import io.grpc.InsecureChannelCredentials;
4241
import io.grpc.MetricRecorder;
4342
import io.grpc.Status;
4443
import io.grpc.Status.Code;
@@ -79,7 +78,7 @@ public class CsdsServiceTest {
7978
EnvoyProtoData.Node.newBuilder().setId(NODE_ID).build();
8079
private static final BootstrapInfo BOOTSTRAP_INFO = BootstrapInfo.builder()
8180
.servers(ImmutableList.of(
82-
ServerInfo.create(SERVER_URI, InsecureChannelCredentials.create())))
81+
ServerInfo.create(SERVER_URI, ImmutableMap.of("type", "insecure"))))
8382
.node(BOOTSTRAP_NODE)
8483
.build();
8584
private static final FakeXdsClient XDS_CLIENT_NO_RESOURCES = new FakeXdsClient();

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

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,6 @@
2424

2525
import com.google.common.collect.ImmutableMap;
2626
import com.google.common.collect.Iterables;
27-
import io.grpc.InsecureChannelCredentials;
28-
import io.grpc.TlsChannelCredentials;
2927
import io.grpc.internal.GrpcUtil;
3028
import io.grpc.internal.GrpcUtil.GrpcBuildVersion;
3129
import io.grpc.xds.client.Bootstrapper;
@@ -115,7 +113,7 @@ public void parseBootstrap_singleXdsServer() throws XdsInitializationException {
115113
assertThat(info.servers()).hasSize(1);
116114
ServerInfo serverInfo = Iterables.getOnlyElement(info.servers());
117115
assertThat(serverInfo.target()).isEqualTo(SERVER_URI);
118-
assertThat(serverInfo.implSpecificConfig()).isInstanceOf(InsecureChannelCredentials.class);
116+
assertThat(serverInfo.implSpecificConfig()).isEqualTo(ImmutableMap.of("type", "insecure"));
119117
assertThat(info.node()).isEqualTo(
120118
getNodeBuilder()
121119
.setId("ENVOY_NODE_ID")
@@ -169,11 +167,11 @@ public void parseBootstrap_multipleXdsServers() throws XdsInitializationExceptio
169167
assertThat(serverInfoList.get(0).target())
170168
.isEqualTo("trafficdirector-foo.googleapis.com:443");
171169
assertThat(serverInfoList.get(0).implSpecificConfig())
172-
.isInstanceOf(TlsChannelCredentials.class);
170+
.isEqualTo(ImmutableMap.of("type", "tls"));
173171
assertThat(serverInfoList.get(1).target())
174172
.isEqualTo("trafficdirector-bar.googleapis.com:443");
175173
assertThat(serverInfoList.get(1).implSpecificConfig())
176-
.isInstanceOf(InsecureChannelCredentials.class);
174+
.isEqualTo(ImmutableMap.of("type", "insecure"));
177175
assertThat(info.node()).isEqualTo(
178176
getNodeBuilder()
179177
.setId("ENVOY_NODE_ID")
@@ -217,7 +215,7 @@ public void parseBootstrap_IgnoreIrrelevantFields() throws XdsInitializationExce
217215
assertThat(info.servers()).hasSize(1);
218216
ServerInfo serverInfo = Iterables.getOnlyElement(info.servers());
219217
assertThat(serverInfo.target()).isEqualTo(SERVER_URI);
220-
assertThat(serverInfo.implSpecificConfig()).isInstanceOf(InsecureChannelCredentials.class);
218+
assertThat(serverInfo.implSpecificConfig()).isEqualTo(ImmutableMap.of("type", "insecure"));;
221219
assertThat(info.node()).isEqualTo(
222220
getNodeBuilder()
223221
.setId("ENVOY_NODE_ID")
@@ -288,7 +286,7 @@ public void parseBootstrap_useFirstSupportedChannelCredentials()
288286
assertThat(info.servers()).hasSize(1);
289287
ServerInfo serverInfo = Iterables.getOnlyElement(info.servers());
290288
assertThat(serverInfo.target()).isEqualTo(SERVER_URI);
291-
assertThat(serverInfo.implSpecificConfig()).isInstanceOf(InsecureChannelCredentials.class);
289+
assertThat(serverInfo.implSpecificConfig()).isEqualTo(ImmutableMap.of("type", "insecure"));;
292290
assertThat(info.node()).isEqualTo(getNodeBuilder().build());
293291
}
294292

@@ -583,7 +581,7 @@ public void useV2ProtocolByDefault() throws XdsInitializationException {
583581
BootstrapInfo info = bootstrapper.bootstrap();
584582
ServerInfo serverInfo = Iterables.getOnlyElement(info.servers());
585583
assertThat(serverInfo.target()).isEqualTo(SERVER_URI);
586-
assertThat(serverInfo.implSpecificConfig()).isInstanceOf(InsecureChannelCredentials.class);
584+
assertThat(serverInfo.implSpecificConfig()).isEqualTo(ImmutableMap.of("type", "insecure"));
587585
assertThat(serverInfo.ignoreResourceDeletion()).isFalse();
588586
}
589587

@@ -605,7 +603,7 @@ public void useV3ProtocolIfV3FeaturePresent() throws XdsInitializationException
605603
BootstrapInfo info = bootstrapper.bootstrap();
606604
ServerInfo serverInfo = Iterables.getOnlyElement(info.servers());
607605
assertThat(serverInfo.target()).isEqualTo(SERVER_URI);
608-
assertThat(serverInfo.implSpecificConfig()).isInstanceOf(InsecureChannelCredentials.class);
606+
assertThat(serverInfo.implSpecificConfig()).isEqualTo(ImmutableMap.of("type", "insecure"));
609607
assertThat(serverInfo.ignoreResourceDeletion()).isFalse();
610608
}
611609

@@ -627,7 +625,7 @@ public void serverFeatureIgnoreResourceDeletion() throws XdsInitializationExcept
627625
BootstrapInfo info = bootstrapper.bootstrap();
628626
ServerInfo serverInfo = Iterables.getOnlyElement(info.servers());
629627
assertThat(serverInfo.target()).isEqualTo(SERVER_URI);
630-
assertThat(serverInfo.implSpecificConfig()).isInstanceOf(InsecureChannelCredentials.class);
628+
assertThat(serverInfo.implSpecificConfig()).isEqualTo(ImmutableMap.of("type", "insecure"));
631629
// Only ignore_resource_deletion feature enabled: confirm it's on, and xds_v3 is off.
632630
assertThat(serverInfo.ignoreResourceDeletion()).isTrue();
633631
}
@@ -650,7 +648,7 @@ public void serverFeatureTrustedXdsServer() throws XdsInitializationException {
650648
BootstrapInfo info = bootstrapper.bootstrap();
651649
ServerInfo serverInfo = Iterables.getOnlyElement(info.servers());
652650
assertThat(serverInfo.target()).isEqualTo(SERVER_URI);
653-
assertThat(serverInfo.implSpecificConfig()).isInstanceOf(InsecureChannelCredentials.class);
651+
assertThat(serverInfo.implSpecificConfig()).isEqualTo(ImmutableMap.of("type", "insecure"));
654652
assertThat(serverInfo.isTrustedXdsServer()).isTrue();
655653
}
656654

@@ -672,7 +670,7 @@ public void serverFeatureIgnoreResourceDeletion_xdsV3() throws XdsInitialization
672670
BootstrapInfo info = bootstrapper.bootstrap();
673671
ServerInfo serverInfo = Iterables.getOnlyElement(info.servers());
674672
assertThat(serverInfo.target()).isEqualTo(SERVER_URI);
675-
assertThat(serverInfo.implSpecificConfig()).isInstanceOf(InsecureChannelCredentials.class);
673+
assertThat(serverInfo.implSpecificConfig()).isEqualTo(ImmutableMap.of("type", "insecure"));
676674
// ignore_resource_deletion features enabled: confirm both are on.
677675
assertThat(serverInfo.ignoreResourceDeletion()).isTrue();
678676
}

0 commit comments

Comments
 (0)