Disallow peering to the same cluster.

This commit is contained in:
Derek Menteer 2022-10-07 13:17:11 -05:00 committed by Derek Menteer
parent 8742fbe14f
commit 1e394da400
2 changed files with 8 additions and 48 deletions

View File

@ -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
}

View File

@ -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)
}