From d8299670cc3e8605017c1e673463344345301ede Mon Sep 17 00:00:00 2001
From: Daniel Nephin <dnephin@hashicorp.com>
Date: Tue, 13 Oct 2020 18:45:17 -0400
Subject: [PATCH] agent/grpc/resolver: namespace the server ID with the DC name

So that if two datacenters end up with overlapping serverIDs we don't send requests to the wrong server
---
 agent/grpc/client_test.go       |  3 ++-
 agent/grpc/resolver/resolver.go | 18 ++++++++++++++----
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/agent/grpc/client_test.go b/agent/grpc/client_test.go
index 400e0a815d..4bd82b6667 100644
--- a/agent/grpc/client_test.go
+++ b/agent/grpc/client_test.go
@@ -8,11 +8,12 @@ import (
 	"testing"
 	"time"
 
+	"github.com/stretchr/testify/require"
+
 	"github.com/hashicorp/consul/agent/grpc/internal/testservice"
 	"github.com/hashicorp/consul/agent/grpc/resolver"
 	"github.com/hashicorp/consul/agent/metadata"
 	"github.com/hashicorp/consul/sdk/testutil/retry"
-	"github.com/stretchr/testify/require"
 )
 
 func TestNewDialer_WithTLSWrapper(t *testing.T) {
diff --git a/agent/grpc/resolver/resolver.go b/agent/grpc/resolver/resolver.go
index b34aad72f4..4c6874ddca 100644
--- a/agent/grpc/resolver/resolver.go
+++ b/agent/grpc/resolver/resolver.go
@@ -7,8 +7,9 @@ import (
 	"sync"
 	"time"
 
-	"github.com/hashicorp/consul/agent/metadata"
 	"google.golang.org/grpc/resolver"
+
+	"github.com/hashicorp/consul/agent/metadata"
 )
 
 var registerLock sync.Mutex
@@ -31,7 +32,7 @@ type ServerResolverBuilder struct {
 	// parallel testing because gRPC registers resolvers globally.
 	scheme string
 	// servers is an index of Servers by Server.ID. The map contains server IDs
-	// for all datacenters, so it assumes the ID is globally unique.
+	// for all datacenters.
 	servers map[string]*metadata.Server
 	// resolvers is an index of connections to the serverResolver which manages
 	// addresses of servers for that connection.
@@ -131,7 +132,7 @@ func (s *ServerResolverBuilder) AddServer(server *metadata.Server) {
 	s.lock.Lock()
 	defer s.lock.Unlock()
 
-	s.servers[server.ID] = server
+	s.servers[uniqueID(server)] = server
 
 	addrs := s.getDCAddrs(server.Datacenter)
 	for _, resolver := range s.resolvers {
@@ -141,12 +142,21 @@ func (s *ServerResolverBuilder) AddServer(server *metadata.Server) {
 	}
 }
 
+// uniqueID returns a unique identifier for the server which includes the
+// Datacenter and the ID.
+//
+// In practice it is expected that the server.ID is already a globally unique
+// UUID. This function is an extra safeguard in case that ever changes.
+func uniqueID(server *metadata.Server) string {
+	return server.Datacenter + "-" + server.ID
+}
+
 // RemoveServer updates the resolvers' states with the given server removed.
 func (s *ServerResolverBuilder) RemoveServer(server *metadata.Server) {
 	s.lock.Lock()
 	defer s.lock.Unlock()
 
-	delete(s.servers, server.ID)
+	delete(s.servers, uniqueID(server))
 
 	addrs := s.getDCAddrs(server.Datacenter)
 	for _, resolver := range s.resolvers {