[management] Allow concurrent service domain authorization

Service writes only need shared locks on registrations covering their hostname. Hold those locks until commit to preserve the deletion guard while allowing concurrent writes on the same or unrelated domains.

management/internals/modules/reverseproxy/domain/manager/manager.go

@@ -362,7 +362,11 @@ func (m Manager) ValidateServiceDomain(ctx context.Context, tx nbstore.Store, ac
 	if _, ok := ExtractClusterFromFreeDomain(serviceDomain, []string{cluster}); ok {
 		return nil
 	}
-	customDomains, err := tx.LockCustomDomains(ctx, accountID)
+	name, err := nbdomain.FromString(serviceDomain)
+	if err != nil {
+		return fmt.Errorf("parse service domain: %w", err)
+	}
+	customDomains, err := tx.LockCustomDomains(ctx, accountID, name)
 	if err != nil {
 		return err
 	}

management/server/store/sql_store_domain_deletion.go

@@ -4,19 +4,28 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"strings"
 
 	"gorm.io/gorm"
 	"gorm.io/gorm/clause"
 
 	"github.com/netbirdio/netbird/management/internals/modules/reverseproxy/domain"
+	nbdomain "github.com/netbirdio/netbird/shared/management/domain"
 	"github.com/netbirdio/netbird/shared/management/status"
 )
 
-// LockCustomDomains locks an account's registrations until the caller's transaction ends.
+// LockCustomDomains holds shared locks on registrations covering a service until commit.
-func (s *SqlStore) LockCustomDomains(ctx context.Context, accountID string) ([]*domain.Domain, error) {
+func (s *SqlStore) LockCustomDomains(ctx context.Context, accountID string, serviceDomain nbdomain.Domain) ([]*domain.Domain, error) {
+	var names []string
+	for name := serviceDomain.PunycodeString(); name != ""; {
+		names = append(names, name)
+		_, name, _ = strings.Cut(name, ".")
+	}
+
 	var domains []*domain.Domain
-	if err := s.db.WithContext(ctx).Clauses(clause.Locking{Strength: string(LockingStrengthUpdate)}).
+	if err := s.db.WithContext(ctx).Clauses(clause.Locking{Strength: string(LockingStrengthShare)}).
-		Where(accountIDCondition, accountID).Order("id").Find(&domains).Error; err != nil {
+		Where(accountIDCondition, accountID).Where("domain IN ?", names).
+		Order("id").Find(&domains).Error; err != nil {
 		return nil, fmt.Errorf("lock custom domains: %w", err)
 	}
 	return domains, nil
@@ -26,8 +35,8 @@ func (s *SqlStore) LockCustomDomains(ctx context.Context, accountID string) ([]*
 func (s *SqlStore) DeleteCustomDomain(ctx context.Context, accountID string, domainID string) error {
 	return s.db.WithContext(ctx).Transaction(func(tx *gorm.DB) error {
 		var d domain.Domain
-		// Service writes take the same lock before checking validation, so neither
+		// Service writes hold a shared lock on this row through commit, so neither
-		// operation can commit against the other's outdated view of the domain.
+		// operation can proceed against the other's outdated view of the domain.
 		if err := tx.Clauses(clause.Locking{Strength: string(LockingStrengthUpdate)}).
 			Take(&d, accountAndIDQueryCondition, accountID, domainID).Error; err != nil {
 			if errors.Is(err, gorm.ErrRecordNotFound) {

management/server/store/sql_store_domain_deletion_test.go

@@ -10,9 +10,74 @@ import (
 	"github.com/stretchr/testify/require"
 
 	rpservice "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/service"
+	"github.com/netbirdio/netbird/management/server/types"
+	nbdomain "github.com/netbirdio/netbird/shared/management/domain"
 	"github.com/netbirdio/netbird/shared/management/status"
 )
 
+func TestLockCustomDomains_ConcurrentServices(t *testing.T) {
+	runTestForAllEngines(t, "", func(t *testing.T, store Store) {
+		if store.GetStoreEngine() == types.SqliteStoreEngine {
+			t.Skip("SQLite serializes transactions on one connection")
+		}
+		ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second)
+		defer cancel()
+		require.NoError(t, store.SaveAccount(ctx, newAccountWithId(ctx, "owner", "admin", "")))
+		_, err := store.CreateCustomDomain(ctx, "owner", "one.example.com", "cluster", true)
+		require.NoError(t, err)
+		_, err = store.CreateCustomDomain(ctx, "owner", "two.example.com", "cluster", true)
+		require.NoError(t, err)
+
+		locked := make(chan error, 1)
+		release := make(chan struct{})
+		done := make(chan error, 1)
+		go func() {
+			done <- store.ExecuteInTransaction(ctx, func(tx Store) error {
+				_, err := tx.LockCustomDomains(ctx, "owner", "app.one.example.com")
+				locked <- err
+				if err != nil {
+					return err
+				}
+				select {
+				case <-release:
+					return nil
+				case <-ctx.Done():
+					return ctx.Err()
+				}
+			})
+		}()
+		var lockErr error
+		select {
+		case lockErr = <-locked:
+		case err := <-done:
+			t.Fatalf("transaction ended before locking: %v", err)
+		}
+		writeCtx, writeCancel := context.WithTimeout(ctx, 3*time.Second)
+		defer writeCancel()
+		var writeErr error
+		for _, name := range []nbdomain.Domain{"app.one.example.com", "app.two.example.com"} {
+			writeErr = store.ExecuteInTransaction(writeCtx, func(tx Store) error {
+				if _, err := tx.LockCustomDomains(writeCtx, "owner", name); err != nil {
+					return err
+				}
+				return tx.CreateService(writeCtx, &rpservice.Service{
+					ID: name.PunycodeString(), AccountID: "owner", Domain: name.PunycodeString(),
+				})
+			})
+			if writeErr != nil {
+				break
+			}
+		}
+		close(release)
+		require.NoError(t, <-done)
+		require.NoError(t, lockErr)
+		require.NoError(t, writeErr, "domain authorization locks must allow concurrent service writes")
+		services, err := store.GetAccountServices(ctx, LockingStrengthNone, "owner")
+		require.NoError(t, err)
+		assert.Len(t, services, 2, "both services must commit while the first domain is locked")
+	})
+}
+
 func TestDeleteCustomDomain_ServiceDependencies(t *testing.T) {
 	runTestForAllEngines(t, "", func(t *testing.T, store Store) {
 		ctx := context.Background()
@@ -52,14 +117,14 @@ func TestDeleteCustomDomain_ConcurrentServiceCreation(t *testing.T) {
 		for i := range 10 {
 			d, err := store.CreateCustomDomain(ctx, "owner", fmt.Sprintf("app%d.example.com", i), "cluster", true)
 			require.NoError(t, err)
-			svc := &rpservice.Service{ID: fmt.Sprintf("service-%d", i), AccountID: "owner", Domain: d.Domain}
+			svc := &rpservice.Service{ID: fmt.Sprintf("service-%d", i), AccountID: "owner", Domain: "nested." + d.Domain}
 			start := make(chan struct{})
 			created := make(chan error, 1)
 			deleted := make(chan error, 1)
 			go func() {
 				<-start
 				created <- store.ExecuteInTransaction(ctx, func(tx Store) error {
-					domains, err := tx.LockCustomDomains(ctx, "owner")
+					domains, err := tx.LockCustomDomains(ctx, "owner", nbdomain.Domain(svc.Domain))
 					if err != nil {
 						return err
 					}

management/server/store/store.go

@@ -34,6 +34,7 @@ import (
 	"github.com/netbirdio/netbird/management/server/telemetry"
 	"github.com/netbirdio/netbird/management/server/testutil"
 	"github.com/netbirdio/netbird/management/server/types"
+	nbdomain "github.com/netbirdio/netbird/shared/management/domain"
 	"github.com/netbirdio/netbird/util"
 	"github.com/netbirdio/netbird/util/crypt"
 
@@ -302,7 +303,7 @@ type Store interface {
 	GetCustomDomain(ctx context.Context, accountID string, domainID string) (*domain.Domain, error)
 	ListFreeDomains(ctx context.Context, accountID string) ([]string, error)
 	ListCustomDomains(ctx context.Context, accountID string) ([]*domain.Domain, error)
-	LockCustomDomains(ctx context.Context, accountID string) ([]*domain.Domain, error)
+	LockCustomDomains(ctx context.Context, accountID string, serviceDomain nbdomain.Domain) ([]*domain.Domain, error)
 	GetCustomDomainByName(ctx context.Context, domainName string) (*domain.Domain, error)
 	CreateCustomDomain(ctx context.Context, accountID string, domainName string, targetCluster string, validated bool) (*domain.Domain, error)
 	UpdateCustomDomain(ctx context.Context, accountID string, d *domain.Domain) (*domain.Domain, error)

management/server/store/store_mock.go

@@ -31,6 +31,7 @@ import (
 	posture "github.com/netbirdio/netbird/management/server/posture"
 	types3 "github.com/netbirdio/netbird/management/server/types"
 	route "github.com/netbirdio/netbird/route"
+	domain0 "github.com/netbirdio/netbird/shared/management/domain"
 	crypt "github.com/netbirdio/netbird/util/crypt"
 	gomock "go.uber.org/mock/gomock"
 )
@@ -3213,18 +3214,18 @@ func (mr *MockStoreMockRecorder) ListFreeDomains(ctx, accountID any) *gomock.Cal
 }
 
 // LockCustomDomains mocks base method.
-func (m *MockStore) LockCustomDomains(ctx context.Context, accountID string) ([]*domain.Domain, error) {
+func (m *MockStore) LockCustomDomains(ctx context.Context, accountID string, serviceDomain domain0.Domain) ([]*domain.Domain, error) {
 	m.ctrl.T.Helper()
-	ret := m.ctrl.Call(m, "LockCustomDomains", ctx, accountID)
+	ret := m.ctrl.Call(m, "LockCustomDomains", ctx, accountID, serviceDomain)
 	ret0, _ := ret[0].([]*domain.Domain)
 	ret1, _ := ret[1].(error)
 	return ret0, ret1
 }
 
 // LockCustomDomains indicates an expected call of LockCustomDomains.
-func (mr *MockStoreMockRecorder) LockCustomDomains(ctx, accountID any) *gomock.Call {
+func (mr *MockStoreMockRecorder) LockCustomDomains(ctx, accountID, serviceDomain any) *gomock.Call {
 	mr.mock.ctrl.T.Helper()
-	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "LockCustomDomains", reflect.TypeOf((*MockStore)(nil).LockCustomDomains), ctx, accountID)
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "LockCustomDomains", reflect.TypeOf((*MockStore)(nil).LockCustomDomains), ctx, accountID, serviceDomain)
 }
 
 // MarkAccountPrimary mocks base method.