From 1e394da4001eac74cdfda969c8a747a02b154c06 Mon Sep 17 00:00:00 2001
From: Derek Menteer <derek.menteer@hashicorp.com>
Date: Fri, 7 Oct 2022 13:17:11 -0500
Subject: [PATCH] Disallow peering to the same cluster.

---
 agent/rpc/peering/service.go      | 28 ++++++----------------------
 agent/rpc/peering/service_test.go | 28 ++--------------------------
 2 files changed, 8 insertions(+), 48 deletions(-)

diff --git a/agent/rpc/peering/service.go b/agent/rpc/peering/service.go
index 18336d33a6..0eb381acfe 100644
--- a/agent/rpc/peering/service.go
+++ b/agent/rpc/peering/service.go
@@ -386,7 +386,7 @@ func (s *Server) Establish(
 		return nil, err
 	}
 
-	if err := s.validatePeeringLocality(tok, entMeta.PartitionOrEmpty()); err != nil {
+	if err := s.validatePeeringLocality(tok); err != nil {
 		return nil, err
 	}
 
@@ -478,32 +478,16 @@ func (s *Server) Establish(
 	return resp, nil
 }
 
-// validatePeeringLocality makes sure that we don't create a peering in the cluster/partition it was generated.
-// We validate by looking at the remote PeerID from the PeeringToken and looking up that peering in the partition.
-// If there is one and the request partition is the same, then we are attempting to peer within the partition, which we shouldn't.
-// We also perform a check to verify if the ServerName of the PeeringToken overlaps with our own, we do not process it
-// unless we've been able to find the peering in the store, i.e. this peering is between two local partitions.
-func (s *Server) validatePeeringLocality(token *structs.PeeringToken, partition string) error {
-	_, peering, err := s.Backend.Store().PeeringReadByID(nil, token.PeerID)
-	if err != nil {
-		return fmt.Errorf("cannot read peering by ID: %w", err)
-	}
-
-	// If the token has the same server name as this cluster, but we can't find the peering
-	// in our store, it indicates a naming conflict.
+// validatePeeringLocality makes sure that we don't create a peering in the same cluster it was generated.
+// If the ServerName of the PeeringToken overlaps with our own, we do not accept it.
+func (s *Server) validatePeeringLocality(token *structs.PeeringToken) error {
 	serverName, _, err := s.Backend.GetTLSMaterials(false)
 	if err != nil {
 		return fmt.Errorf("failed to fetch TLS materials: %w", err)
 	}
-
-	if serverName == token.ServerName && peering == nil {
-		return fmt.Errorf("conflict - peering token's server name matches the current cluster's server name, %q, but there is no record in the database", serverName)
+	if serverName == token.ServerName {
+		return fmt.Errorf("cannot create a peering within the same cluster %q", serverName)
 	}
-
-	if peering != nil && acl.EqualPartitions(peering.GetPartition(), partition) {
-		return fmt.Errorf("cannot create a peering within the same partition (ENT) or cluster (OSS)")
-	}
-
 	return nil
 }
 
diff --git a/agent/rpc/peering/service_test.go b/agent/rpc/peering/service_test.go
index 06d93369c0..300e463bee 100644
--- a/agent/rpc/peering/service_test.go
+++ b/agent/rpc/peering/service_test.go
@@ -364,31 +364,7 @@ func TestPeeringService_Establish_Validation(t *testing.T) {
 	}
 }
 
-// Loopback peering within the same cluster/partion should throw an error
-func TestPeeringService_Establish_invalidPeeringInSamePartition(t *testing.T) {
-	// TODO(peering): see note on newTestServer, refactor to not use this
-	s := newTestServer(t, nil)
-	client := pbpeering.NewPeeringServiceClient(s.ClientConn(t))
-
-	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
-	t.Cleanup(cancel)
-
-	req := pbpeering.GenerateTokenRequest{PeerName: "peerOne"}
-	resp, err := client.GenerateToken(ctx, &req)
-	require.NoError(t, err)
-	require.NotEmpty(t, resp)
-
-	establishReq := &pbpeering.EstablishRequest{
-		PeerName:     "peerTwo",
-		PeeringToken: resp.PeeringToken}
-
-	respE, errE := client.Establish(ctx, establishReq)
-	require.Error(t, errE)
-	require.Contains(t, errE.Error(), "cannot create a peering within the same partition (ENT) or cluster (OSS)")
-	require.Nil(t, respE)
-}
-
-// When tokens have the same name as the dialing cluster but are unknown by ID, we
+// When tokens have the same name as the dialing cluster, we
 // should be throwing an error to note the server name conflict.
 func TestPeeringService_Establish_serverNameConflict(t *testing.T) {
 	// TODO(peering): see note on newTestServer, refactor to not use this
@@ -423,7 +399,7 @@ func TestPeeringService_Establish_serverNameConflict(t *testing.T) {
 
 	respE, errE := client.Establish(ctx, establishReq)
 	require.Error(t, errE)
-	require.Contains(t, errE.Error(), "conflict - peering token's server name matches the current cluster's server name")
+	require.Contains(t, errE.Error(), "cannot create a peering within the same cluster")
 	require.Nil(t, respE)
 }