From 618a4de705189ce01179434e85f91f197a36a6b4 Mon Sep 17 00:00:00 2001
From: Eric Anderson <ejona@google.com>
Date: Thu, 18 Aug 2022 17:27:15 -0700
Subject: [PATCH] xds: CHANNEL_ID hash should be random for the channel

The log id is an incrementing value starting from 0. That means the same
binary on two different machines will have the same hashes for each
consecutive Channel. That was not at all the intension of CHANNEL_ID.
From gRFC A42:

> This can be used in similar situations to where Envoy uses
> connection_properties to hash on the source IP address.
---
 xds/src/main/java/io/grpc/xds/XdsNameResolver.java     | 4 +++-
 xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java | 1 +
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/xds/src/main/java/io/grpc/xds/XdsNameResolver.java b/xds/src/main/java/io/grpc/xds/XdsNameResolver.java
index d29132d1b25..0d51d3afa80 100644
--- a/xds/src/main/java/io/grpc/xds/XdsNameResolver.java
+++ b/xds/src/main/java/io/grpc/xds/XdsNameResolver.java
@@ -123,6 +123,7 @@ final class XdsNameResolver extends NameResolver {
   // put()/remove() must be called in SyncContext, and get() can be called in any thread.
   private final ConcurrentMap<String, ClusterRefState> clusterRefs = new ConcurrentHashMap<>();
   private final ConfigSelector configSelector = new ConfigSelector();
+  private final long randomChannelId;
 
   private volatile RoutingConfig routingConfig = RoutingConfig.empty;
   private Listener2 listener;
@@ -162,6 +163,7 @@ final class XdsNameResolver extends NameResolver {
     this.xdsClientPoolFactory.setBootstrapOverride(bootstrapOverride);
     this.random = checkNotNull(random, "random");
     this.filterRegistry = checkNotNull(filterRegistry, "filterRegistry");
+    randomChannelId = random.nextLong();
     logId = InternalLogId.allocate("xds-resolver", name);
     logger = XdsLogger.withLogId(logId);
     logger.log(XdsLogLevel.INFO, "Created resolver for {0}", name);
@@ -586,7 +588,7 @@ private long generateHash(List<HashPolicy> hashPolicies, Metadata headers) {
             newHash = hashFunc.hashAsciiString(value);
           }
         } else if (policy.type() == HashPolicy.Type.CHANNEL_ID) {
-          newHash = hashFunc.hashLong(logId.getId());
+          newHash = hashFunc.hashLong(randomChannelId);
         }
         if (newHash != null ) {
           // Rotating the old value prevents duplicate hash rules from cancelling each other out
diff --git a/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java b/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java
index 8f4a2cf1774..d96fa2b30a8 100644
--- a/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java
+++ b/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java
@@ -802,6 +802,7 @@ public void resolved_rpcHashingByChannelId() {
     // A different resolver/Channel.
     resolver.shutdown();
     reset(mockListener);
+    when(mockRandom.nextLong()).thenReturn(123L);
     resolver = new XdsNameResolver(null, AUTHORITY, null, serviceConfigParser,
         syncContext, scheduler,
         xdsClientPoolFactory, mockRandom, FilterRegistry.getDefaultRegistry(), null);