mirror of
https://github.com/netbirdio/netbird.git
synced 2026-03-31 06:34:19 -04:00
[management] Fix DNS label uniqueness check on peer rename (#5679)
This commit is contained in:
@@ -249,7 +249,7 @@ func (am *DefaultAccountManager) UpdatePeer(ctx context.Context, accountID, user
|
|||||||
if err != nil {
|
if err != nil {
|
||||||
newLabel = ""
|
newLabel = ""
|
||||||
} else {
|
} else {
|
||||||
_, err := transaction.GetPeerIdByLabel(ctx, store.LockingStrengthNone, accountID, update.Name)
|
_, err := transaction.GetPeerIdByLabel(ctx, store.LockingStrengthNone, accountID, newLabel)
|
||||||
if err == nil {
|
if err == nil {
|
||||||
newLabel = ""
|
newLabel = ""
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -37,6 +37,7 @@ import (
|
|||||||
"github.com/netbirdio/netbird/management/server/job"
|
"github.com/netbirdio/netbird/management/server/job"
|
||||||
"github.com/netbirdio/netbird/management/server/permissions"
|
"github.com/netbirdio/netbird/management/server/permissions"
|
||||||
"github.com/netbirdio/netbird/management/server/settings"
|
"github.com/netbirdio/netbird/management/server/settings"
|
||||||
|
"github.com/netbirdio/netbird/shared/auth"
|
||||||
"github.com/netbirdio/netbird/shared/management/status"
|
"github.com/netbirdio/netbird/shared/management/status"
|
||||||
|
|
||||||
"github.com/netbirdio/netbird/management/server/util"
|
"github.com/netbirdio/netbird/management/server/util"
|
||||||
@@ -2738,3 +2739,70 @@ func TestProcessPeerAddAuth(t *testing.T) {
|
|||||||
assert.Empty(t, config.GroupsToAdd)
|
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")
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user