diff --git a/management/server/peer.go b/management/server/peer.go index 78ecbfcae..f7cb6a0f1 100644 --- a/management/server/peer.go +++ b/management/server/peer.go @@ -249,7 +249,7 @@ func (am *DefaultAccountManager) UpdatePeer(ctx context.Context, accountID, user if err != nil { newLabel = "" } else { - _, err := transaction.GetPeerIdByLabel(ctx, store.LockingStrengthNone, accountID, update.Name) + _, err := transaction.GetPeerIdByLabel(ctx, store.LockingStrengthNone, accountID, newLabel) if err == nil { newLabel = "" } diff --git a/management/server/peer_test.go b/management/server/peer_test.go index b17757ffd..51c16d730 100644 --- a/management/server/peer_test.go +++ b/management/server/peer_test.go @@ -37,6 +37,7 @@ import ( "github.com/netbirdio/netbird/management/server/job" "github.com/netbirdio/netbird/management/server/permissions" "github.com/netbirdio/netbird/management/server/settings" + "github.com/netbirdio/netbird/shared/auth" "github.com/netbirdio/netbird/shared/management/status" "github.com/netbirdio/netbird/management/server/util" @@ -2738,3 +2739,70 @@ func TestProcessPeerAddAuth(t *testing.T) { assert.Empty(t, config.GroupsToAdd) }) } + +func TestUpdatePeer_DnsLabelCollisionWithFQDN(t *testing.T) { + manager, _, err := createManager(t) + require.NoError(t, err, "unable to create account manager") + + accountID, err := manager.GetAccountIDByUserID(context.Background(), auth.UserAuth{UserId: userID}) + require.NoError(t, err, "unable to create an account") + + // Add first peer with hostname that produces DNS label "netbird1" + key1, err := wgtypes.GenerateKey() + require.NoError(t, err) + peer1, _, _, err := manager.AddPeer(context.Background(), "", "", userID, &nbpeer.Peer{ + Key: key1.PublicKey().String(), + Meta: nbpeer.PeerSystemMeta{Hostname: "netbird1.netbird.cloud"}, + }, false) + require.NoError(t, err, "unable to add first peer") + assert.Equal(t, "netbird1", peer1.DNSLabel) + + // Add second peer with a different hostname + key2, err := wgtypes.GenerateKey() + require.NoError(t, err) + peer2, _, _, err := manager.AddPeer(context.Background(), "", "", userID, &nbpeer.Peer{ + Key: key2.PublicKey().String(), + Meta: nbpeer.PeerSystemMeta{Hostname: "ip-10-29-5-130"}, + }, false) + require.NoError(t, err) + + update := peer2.Copy() + update.Name = "netbird1.demo.netbird.cloud" + updated, err := manager.UpdatePeer(context.Background(), accountID, userID, update) + require.NoError(t, err, "renaming peer should not fail with duplicate DNS label error") + assert.Equal(t, "netbird1.demo.netbird.cloud", updated.Name) + assert.NotEqual(t, "netbird1", updated.DNSLabel, "DNS label should not collide with existing peer") + assert.Contains(t, updated.DNSLabel, "netbird1-", "DNS label should be IP-based fallback") +} + +func TestUpdatePeer_DnsLabelUniqueName(t *testing.T) { + manager, _, err := createManager(t) + require.NoError(t, err, "unable to create account manager") + + accountID, err := manager.GetAccountIDByUserID(context.Background(), auth.UserAuth{UserId: userID}) + require.NoError(t, err, "unable to create an account") + + key1, err := wgtypes.GenerateKey() + require.NoError(t, err) + peer1, _, _, err := manager.AddPeer(context.Background(), "", "", userID, &nbpeer.Peer{ + Key: key1.PublicKey().String(), + Meta: nbpeer.PeerSystemMeta{Hostname: "web-server"}, + }, false) + require.NoError(t, err) + assert.Equal(t, "web-server", peer1.DNSLabel) + + // Add second peer and rename it to a unique FQDN whose first label doesn't collide + key2, err := wgtypes.GenerateKey() + require.NoError(t, err) + peer2, _, _, err := manager.AddPeer(context.Background(), "", "", userID, &nbpeer.Peer{ + Key: key2.PublicKey().String(), + Meta: nbpeer.PeerSystemMeta{Hostname: "old-name"}, + }, false) + require.NoError(t, err) + + update := peer2.Copy() + update.Name = "api-server.example.com" + updated, err := manager.UpdatePeer(context.Background(), accountID, userID, update) + require.NoError(t, err, "renaming to unique FQDN should succeed") + assert.Equal(t, "api-server", updated.DNSLabel, "DNS label should be first label of FQDN") +}