Skip to content

Commit ef44795

Browse files
authored
xds: treat target server authority opaquely for resolving cluster name (#6767)
Fixes usage of target hostname:port in xDS plugin. The target hostname:port used to construct gRPC channel should be treated opaquely. XdsNameResolver should not try to split it and should use it opaquely for sending LDS requests. In received RouteConfiguration messages, do not stripe off port (if any) for finding the virtual host with domain name matching the requested LDS resource name.
1 parent 4a2c5d6 commit ef44795

File tree

5 files changed

+155
-164
lines changed

5 files changed

+155
-164
lines changed

xds/src/main/java/io/grpc/xds/XdsClient.java

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -391,19 +391,17 @@ interface EndpointWatcher {
391391
abstract void shutdown();
392392

393393
/**
394-
* Registers a watcher to receive {@link ConfigUpdate} for service with the given hostname and
395-
* port.
394+
* Registers a watcher to receive {@link ConfigUpdate} for service with the given target
395+
* authority.
396396
*
397397
* <p>Unlike watchers for cluster data and endpoint data, at most one ConfigWatcher can be
398398
* registered. Once it is registered, it cannot be unregistered.
399399
*
400-
* @param hostName the host name part of the "xds:" URI for the server name that the gRPC client
401-
* targets for. Must NOT contain port.
402-
* @param port the port part of the "xds:" URI for the server name that the gRPC client targets
403-
* for. -1 if not specified.
400+
* @param targetAuthority authority of the "xds:" URI for the server name that the gRPC client
401+
* targets for.
404402
* @param watcher the {@link ConfigWatcher} to receive {@link ConfigUpdate}.
405403
*/
406-
void watchConfigData(String hostName, int port, ConfigWatcher watcher) {
404+
void watchConfigData(String targetAuthority, ConfigWatcher watcher) {
407405
}
408406

409407
/**

xds/src/main/java/io/grpc/xds/XdsClientImpl.java

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -157,9 +157,6 @@ final class XdsClientImpl extends XdsClient {
157157
// never change.
158158
@Nullable
159159
private ConfigWatcher configWatcher;
160-
// The host name portion of "xds:" URI that the gRPC client targets for.
161-
@Nullable
162-
private String hostName;
163160
// The "xds:" URI (including port suffix if present) that the gRPC client targets for.
164161
@Nullable
165162
private String ldsResourceName;
@@ -233,15 +230,10 @@ private void cleanUpResources() {
233230
}
234231

235232
@Override
236-
void watchConfigData(String hostName, int port, ConfigWatcher watcher) {
237-
checkState(configWatcher == null, "watcher for %s already registered", hostName);
233+
void watchConfigData(String targetAuthority, ConfigWatcher watcher) {
234+
checkState(configWatcher == null, "watcher for %s already registered", targetAuthority);
235+
ldsResourceName = checkNotNull(targetAuthority, "targetAuthority");
238236
configWatcher = checkNotNull(watcher, "watcher");
239-
this.hostName = checkNotNull(hostName, "hostName");
240-
if (port == -1) {
241-
ldsResourceName = hostName;
242-
} else {
243-
ldsResourceName = hostName + ":" + port;
244-
}
245237
logger.log(XdsLogLevel.INFO, "Started watching config {0}", ldsResourceName);
246238
if (rpcRetryTimer != null && rpcRetryTimer.isPending()) {
247239
// Currently in retry backoff.
@@ -540,11 +532,12 @@ private void handleLdsResponse(DiscoveryResponse ldsResponse) {
540532
// data or one supersedes the other. TBD.
541533
if (requestedHttpConnManager.hasRouteConfig()) {
542534
RouteConfiguration rc = requestedHttpConnManager.getRouteConfig();
543-
clusterName = findClusterNameInRouteConfig(rc, hostName);
535+
clusterName = findClusterNameInRouteConfig(rc, ldsResourceName);
544536
if (clusterName == null) {
545537
errorMessage =
546538
"Listener " + ldsResourceName + " : cannot find a valid cluster name in any "
547-
+ "virtual hosts inside RouteConfiguration with domains matching: " + hostName;
539+
+ "virtual hosts inside RouteConfiguration with domains matching: "
540+
+ ldsResourceName;
548541
}
549542
} else if (requestedHttpConnManager.hasRds()) {
550543
Rds rds = requestedHttpConnManager.getRds();
@@ -650,14 +643,14 @@ private void handleRdsResponse(DiscoveryResponse rdsResponse) {
650643
// Resolved cluster name for the requested resource, if exists.
651644
String clusterName = null;
652645
if (requestedRouteConfig != null) {
653-
clusterName = findClusterNameInRouteConfig(requestedRouteConfig, hostName);
646+
clusterName = findClusterNameInRouteConfig(requestedRouteConfig, ldsResourceName);
654647
if (clusterName == null) {
655648
adsStream.sendNackRequest(
656649
ADS_TYPE_URL_RDS, ImmutableList.of(adsStream.rdsResourceName),
657650
rdsResponse.getVersionInfo(),
658651
"RouteConfiguration " + requestedRouteConfig.getName() + ": cannot find a "
659652
+ "valid cluster name in any virtual hosts with domains matching: "
660-
+ hostName);
653+
+ ldsResourceName);
661654
return;
662655
}
663656
}

xds/src/main/java/io/grpc/xds/XdsNameResolver.java

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

1717
package io.grpc.xds;
1818

19-
import static com.google.common.base.Preconditions.checkArgument;
2019
import static com.google.common.base.Preconditions.checkNotNull;
2120

2221
import com.google.common.base.Stopwatch;
@@ -31,6 +30,7 @@
3130
import io.grpc.Status.Code;
3231
import io.grpc.SynchronizationContext;
3332
import io.grpc.internal.BackoffPolicy;
33+
import io.grpc.internal.GrpcUtil;
3434
import io.grpc.internal.JsonParser;
3535
import io.grpc.internal.ObjectPool;
3636
import io.grpc.xds.Bootstrapper.BootstrapInfo;
@@ -42,7 +42,6 @@
4242
import io.grpc.xds.XdsClient.XdsClientFactory;
4343
import io.grpc.xds.XdsLogger.XdsLogLevel;
4444
import java.io.IOException;
45-
import java.net.URI;
4645
import java.util.List;
4746
import java.util.Map;
4847
import java.util.concurrent.ScheduledExecutorService;
@@ -60,8 +59,6 @@ final class XdsNameResolver extends NameResolver {
6059

6160
private final XdsLogger logger;
6261
private final String authority;
63-
private final String hostName;
64-
private final int port;
6562
private final XdsChannelFactory channelFactory;
6663
private final SynchronizationContext syncContext;
6764
private final ScheduledExecutorService timeService;
@@ -82,12 +79,7 @@ final class XdsNameResolver extends NameResolver {
8279
Supplier<Stopwatch> stopwatchSupplier,
8380
XdsChannelFactory channelFactory,
8481
Bootstrapper bootstrapper) {
85-
URI nameUri = URI.create("//" + checkNotNull(name, "name"));
86-
checkArgument(nameUri.getHost() != null, "Invalid hostname: %s", name);
87-
authority =
88-
checkNotNull(nameUri.getAuthority(), "nameUri (%s) doesn't have an authority", nameUri);
89-
hostName = nameUri.getHost();
90-
port = nameUri.getPort(); // -1 if not specified
82+
authority = GrpcUtil.checkAuthority(checkNotNull(name, "name"));
9183
this.channelFactory = checkNotNull(channelFactory, "channelFactory");
9284
this.syncContext = checkNotNull(args.getSynchronizationContext(), "syncContext");
9385
this.timeService = checkNotNull(args.getScheduledExecutorService(), "timeService");
@@ -139,7 +131,7 @@ XdsClient createXdsClient() {
139131
};
140132
xdsClientPool = new RefCountedXdsClientObjectPool(xdsClientFactory);
141133
xdsClient = xdsClientPool.getObject();
142-
xdsClient.watchConfigData(hostName, port, new ConfigWatcher() {
134+
xdsClient.watchConfigData(authority, new ConfigWatcher() {
143135
@Override
144136
public void onConfigChanged(ConfigUpdate update) {
145137
logger.log(

0 commit comments

Comments
 (0)