Merge remote-tracking branch 'origin/main' into android/gui-integration

# Conflicts:
#	client/android/profile_state_test.go

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

@@ -66,8 +66,8 @@ func TestExtractClusterFromFreeDomain(t *testing.T) {
 
 func TestExtractClusterFromCustomDomains(t *testing.T) {
 	customDomains := []*domain.Domain{
-		{Domain: "example.com", TargetCluster: "eu1.proxy.netbird.io", Validated: true},
-		{Domain: "proxy.corp.io", TargetCluster: "us1.proxy.netbird.io", Validated: true},
+		{Domain: "example.com", TargetCluster: "eu1.proxy.netbird.io"},
+		{Domain: "proxy.corp.io", TargetCluster: "us1.proxy.netbird.io"},
 	}
 
 	tests := []struct {
@@ -120,49 +120,19 @@ func TestExtractClusterFromCustomDomains(t *testing.T) {
 
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
-			cluster, match := extractClusterFromCustomDomains(tc.domain, customDomains)
-			if !tc.wantOK {
-				assert.Equal(t, customDomainNoMatch, match, "unrelated domain should not match any custom domain")
-				return
+			cluster, ok := extractClusterFromCustomDomains(tc.domain, customDomains)
+			assert.Equal(t, tc.wantOK, ok)
+			if ok {
+				assert.Equal(t, tc.wantVal, cluster)
 			}
-			assert.Equal(t, customDomainValidated, match, "validated custom domain should resolve a cluster")
-			assert.Equal(t, tc.wantVal, cluster)
 		})
 	}
 }
 
-// An unvalidated row must never yield a cluster: the account has not shown it
-// controls the name, so no service may be bound to it.
-func TestExtractClusterFromCustomDomains_UnvalidatedDomainRefused(t *testing.T) {
-	customDomains := []*domain.Domain{
-		{Domain: "example.com", TargetCluster: "eu1.proxy.netbird.io", Validated: false},
-	}
-
-	for _, serviceDomain := range []string{"example.com", "app.example.com"} {
-		t.Run(serviceDomain, func(t *testing.T) {
-			cluster, match := extractClusterFromCustomDomains(serviceDomain, customDomains)
-			assert.Equal(t, customDomainUnvalidated, match, "unvalidated row must be reported as such")
-			assert.Empty(t, cluster, "unvalidated row must not resolve a cluster")
-		})
-	}
-}
-
-// A more specific unvalidated row must not shadow a validated parent domain.
-func TestExtractClusterFromCustomDomains_ValidatedParentWinsOverUnvalidatedChild(t *testing.T) {
-	customDomains := []*domain.Domain{
-		{Domain: "example.com", TargetCluster: "cluster-generic", Validated: true},
-		{Domain: "app.example.com", TargetCluster: "cluster-app", Validated: false},
-	}
-
-	cluster, match := extractClusterFromCustomDomains("app.example.com", customDomains)
-	assert.Equal(t, customDomainValidated, match)
-	assert.Equal(t, "cluster-generic", cluster, "validated parent domain should provide the cluster")
-}
-
 func TestExtractClusterFromCustomDomains_OverlappingDomains(t *testing.T) {
 	customDomains := []*domain.Domain{
-		{Domain: "example.com", TargetCluster: "cluster-generic", Validated: true},
-		{Domain: "app.example.com", TargetCluster: "cluster-app", Validated: true},
+		{Domain: "example.com", TargetCluster: "cluster-generic"},
+		{Domain: "app.example.com", TargetCluster: "cluster-app"},
 	}
 
 	tests := []struct {
@@ -194,8 +164,8 @@ func TestExtractClusterFromCustomDomains_OverlappingDomains(t *testing.T) {
 
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
-			cluster, match := extractClusterFromCustomDomains(tc.domain, customDomains)
-			assert.Equal(t, customDomainValidated, match)
+			cluster, ok := extractClusterFromCustomDomains(tc.domain, customDomains)
+			assert.True(t, ok)
 			assert.Equal(t, tc.wantVal, cluster)
 		})
 	}

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

@@ -22,7 +22,6 @@ type store interface {
 	GetAccount(ctx context.Context, accountID string) (*types.Account, error)
 
 	GetCustomDomain(ctx context.Context, accountID string, domainID string) (*domain.Domain, error)
-	GetCustomDomainByName(ctx context.Context, domainName string) (*domain.Domain, error)
 	ListFreeDomains(ctx context.Context, accountID string) ([]string, error)
 	ListCustomDomains(ctx context.Context, accountID string) ([]*domain.Domain, error)
 	CreateCustomDomain(ctx context.Context, accountID string, domainName string, targetCluster string, validated bool) (*domain.Domain, error)
@@ -147,10 +146,6 @@ func (m Manager) CreateDomain(ctx context.Context, accountID, userID, domainName
 		return nil, fmt.Errorf("target cluster %s is not available", targetCluster)
 	}
 
-	if err := m.checkDomainAvailable(ctx, domainName); err != nil {
-		return nil, err
-	}
-
 	// Attempt an initial validation against the specified cluster only
 	var validated bool
 	if m.validator.IsValid(ctx, domainName, []string{targetCluster}) {
@@ -167,23 +162,6 @@ func (m Manager) CreateDomain(ctx context.Context, accountID, userID, domainName
 	return d, nil
 }
 
-// checkDomainAvailable reports whether the domain is free to claim. The unique
-// index on the column is the real guard; this turns the violation into a
-// conflict the caller can act on instead of a database error, and says nothing
-// about which account holds the domain.
-func (m Manager) checkDomainAvailable(ctx context.Context, domainName string) error {
-	_, err := m.store.GetCustomDomainByName(ctx, domainName)
-	if err == nil {
-		return status.Errorf(status.AlreadyExists, "domain %s is already registered", domainName)
-	}
-
-	if sErr, ok := status.FromError(err); ok && sErr.Type() == status.NotFound {
-		return nil
-	}
-
-	return fmt.Errorf("look up domain: %w", err)
-}
-
 func (m Manager) DeleteDomain(ctx context.Context, accountID, userID, domainID string) error {
 	ok, ctx, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Services, operations.Delete)
 	if err != nil {
@@ -316,12 +294,9 @@ func (m Manager) DeriveClusterFromDomain(ctx context.Context, accountID, domain
 		return "", fmt.Errorf("list custom domains: %w", err)
 	}
 
-	targetCluster, match := extractClusterFromCustomDomains(domain, customDomains)
-	switch match {
-	case customDomainValidated:
+	targetCluster, valid := extractClusterFromCustomDomains(domain, customDomains)
+	if valid {
 		return targetCluster, nil
-	case customDomainUnvalidated:
-		return "", status.Errorf(status.PreconditionFailed, "domain %s is not validated", domain)
 	}
 
 	return "", fmt.Errorf("domain %s does not match any available proxy cluster", domain)
@@ -355,46 +330,19 @@ func (m Manager) getClusterAllowList(ctx context.Context, accountID string) ([]s
 	return merged, nil
 }
 
-// customDomainMatch describes how a service domain relates to the account's
-// custom domain rows.
-type customDomainMatch int
-
-const (
-	customDomainNoMatch customDomainMatch = iota
-	customDomainUnvalidated
-	customDomainValidated
-)
-
-// extractClusterFromCustomDomains finds the longest custom domain covering the
-// service domain and reports its target cluster. Only a validated row yields a
-// cluster: until the CNAME check has passed the account has not shown it
-// controls the name, so no traffic may be routed for it.
-func extractClusterFromCustomDomains(serviceDomain string, customDomains []*domain.Domain) (string, customDomainMatch) {
+func extractClusterFromCustomDomains(serviceDomain string, customDomains []*domain.Domain) (string, bool) {
 	bestCluster := ""
 	bestLen := -1
-	matched := false
 	for _, cd := range customDomains {
 		if serviceDomain != cd.Domain && !strings.HasSuffix(serviceDomain, "."+cd.Domain) {
 			continue
 		}
-		matched = true
-		if !cd.Validated {
-			continue
-		}
 		if l := len(cd.Domain); l > bestLen {
 			bestLen = l
 			bestCluster = cd.TargetCluster
 		}
 	}
-
-	switch {
-	case bestLen >= 0:
-		return bestCluster, customDomainValidated
-	case matched:
-		return "", customDomainUnvalidated
-	default:
-		return "", customDomainNoMatch
-	}
+	return bestCluster, bestLen >= 0
 }
 
 // ExtractClusterFromFreeDomain extracts the cluster address from a free domain.

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

@@ -1,249 +0,0 @@
-package manager
-
-import (
-	"context"
-	"fmt"
-	"sync"
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"go.opentelemetry.io/otel/metric/noop"
-
-	"github.com/netbirdio/netbird/management/internals/modules/reverseproxy/domain"
-	proxymanager "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/proxy/manager"
-	"github.com/netbirdio/netbird/management/server/activity"
-	"github.com/netbirdio/netbird/management/server/mock_server"
-	"github.com/netbirdio/netbird/management/server/permissions"
-	nbstore "github.com/netbirdio/netbird/management/server/store"
-	"github.com/netbirdio/netbird/management/server/types"
-	"github.com/netbirdio/netbird/shared/management/status"
-)
-
-const (
-	testCluster  = "eu.proxy.test"
-	accountA     = "account-a"
-	accountAUser = "account-a-admin"
-	accountB     = "account-b"
-	accountBUser = "account-b-admin"
-)
-
-// stubResolver answers CNAME lookups from a table the test controls, so a
-// domain can point at the cluster or nowhere without touching a real resolver.
-type stubResolver struct {
-	mu     sync.Mutex
-	cnames map[string]string
-}
-
-func (r *stubResolver) LookupCNAME(_ context.Context, host string) (string, error) {
-	r.mu.Lock()
-	defer r.mu.Unlock()
-
-	cname, ok := r.cnames[host]
-	if !ok {
-		return "", fmt.Errorf("lookup %s: no such host", host)
-	}
-	return cname + ".", nil
-}
-
-func (r *stubResolver) set(host, cname string) {
-	r.mu.Lock()
-	defer r.mu.Unlock()
-	r.cnames[host] = cname
-}
-
-type domainTestEnv struct {
-	manager  Manager
-	store    nbstore.Store
-	resolver *stubResolver
-}
-
-// setupDomainTest builds the domain manager on a real SQLite store with two
-// accounts and one active public proxy cluster.
-func setupDomainTest(t *testing.T) *domainTestEnv {
-	t.Helper()
-
-	ctx := context.Background()
-	testStore, cleanup, err := nbstore.NewTestStoreFromSQL(ctx, "", t.TempDir())
-	require.NoError(t, err)
-	t.Cleanup(cleanup)
-
-	for accountID, userID := range map[string]string{accountA: accountAUser, accountB: accountBUser} {
-		require.NoError(t, testStore.SaveAccount(ctx, &types.Account{
-			Id:        accountID,
-			CreatedBy: userID,
-			Settings:  &types.Settings{},
-			Users: map[string]*types.User{
-				userID: {
-					Id:        userID,
-					AccountID: accountID,
-					Role:      types.UserRoleAdmin,
-				},
-			},
-		}))
-	}
-
-	proxyMgr, err := proxymanager.NewManager(testStore, noop.NewMeterProvider().Meter(""))
-	require.NoError(t, err)
-
-	_, err = proxyMgr.Connect(ctx, "proxy-1", "session-1", testCluster, "127.0.0.1", nil, nil)
-	require.NoError(t, err)
-
-	resolver := &stubResolver{cnames: make(map[string]string)}
-
-	mgr := Manager{
-		store:              testStore,
-		proxyManager:       proxyMgr,
-		validator:          domain.Validator{Resolver: resolver},
-		permissionsManager: permissions.NewManager(testStore),
-		accountManager: &mock_server.MockAccountManager{
-			StoreEventFunc: func(context.Context, string, string, string, activity.ActivityDescriber, map[string]any) {},
-		},
-	}
-
-	return &domainTestEnv{manager: mgr, store: testStore, resolver: resolver}
-}
-
-// storedDomain reads a domain row back through the store so assertions are made
-// on what was persisted rather than on the value the manager returned.
-func storedDomain(t *testing.T, s nbstore.Store, accountID, domainName string) *domain.Domain {
-	t.Helper()
-
-	domains, err := s.ListCustomDomains(context.Background(), accountID)
-	require.NoError(t, err)
-	for _, d := range domains {
-		if d.Domain == domainName {
-			return d
-		}
-	}
-	return nil
-}
-
-// A domain whose CNAME check fails is stored unvalidated and must not resolve a
-// cluster, which is what service creation gates on.
-func TestCreateDomain_FailedLookupIsNotServable(t *testing.T) {
-	ctx := context.Background()
-	env := setupDomainTest(t)
-
-	created, err := env.manager.CreateDomain(ctx, accountA, accountAUser, "apps.example.com", testCluster)
-	require.NoError(t, err)
-	assert.False(t, created.Validated, "a domain whose CNAME lookup fails must not be created validated")
-
-	stored := storedDomain(t, env.store, accountA, "apps.example.com")
-	require.NotNil(t, stored, "domain row should exist")
-	assert.False(t, stored.Validated, "persisted row must be unvalidated")
-
-	cluster, err := env.manager.DeriveClusterFromDomain(ctx, accountA, "apps.example.com")
-	require.Error(t, err, "an unvalidated domain must not resolve a cluster")
-	assert.Empty(t, cluster)
-	assert.Contains(t, err.Error(), "not validated", "error should tell the caller what to fix")
-
-	sErr, ok := status.FromError(err)
-	require.True(t, ok, "error should be a typed status error")
-	assert.Equal(t, status.PreconditionFailed, sErr.Type())
-
-	_, err = env.manager.DeriveClusterFromDomain(ctx, accountA, "sub.apps.example.com")
-	assert.Error(t, err, "subdomains of an unvalidated custom domain are not servable either")
-}
-
-// A second account claiming a registered domain gets a clean conflict, not a
-// database error surfaced as a 500.
-func TestCreateDomain_DuplicateIsAConflict(t *testing.T) {
-	ctx := context.Background()
-	env := setupDomainTest(t)
-
-	_, err := env.manager.CreateDomain(ctx, accountA, accountAUser, "shared.example.com", testCluster)
-	require.NoError(t, err)
-
-	_, err = env.manager.CreateDomain(ctx, accountB, accountBUser, "shared.example.com", testCluster)
-	require.Error(t, err)
-
-	sErr, ok := status.FromError(err)
-	require.True(t, ok, "conflict must be a typed status error, not a raw database error")
-	assert.Equal(t, status.AlreadyExists, sErr.Type(), "conflict should map to 409, not 500")
-	assert.NotContains(t, sErr.Message, accountA, "the response must not reveal the holding account")
-
-	assert.Nil(t, storedDomain(t, env.store, accountB, "shared.example.com"), "no row should be written on conflict")
-}
-
-// The same account re-adding one of its own domains is a conflict too.
-func TestCreateDomain_SameAccountDuplicateIsAConflict(t *testing.T) {
-	ctx := context.Background()
-	env := setupDomainTest(t)
-
-	_, err := env.manager.CreateDomain(ctx, accountA, accountAUser, "dup.example.com", testCluster)
-	require.NoError(t, err)
-
-	_, err = env.manager.CreateDomain(ctx, accountA, accountAUser, "dup.example.com", testCluster)
-	require.Error(t, err)
-
-	sErr, ok := status.FromError(err)
-	require.True(t, ok)
-	assert.Equal(t, status.AlreadyExists, sErr.Type())
-}
-
-// The negative control: a validated domain still derives its cluster, for the
-// bare name and for subdomains, exactly as before.
-func TestCreateDomain_ValidatedDomainDerivesCluster(t *testing.T) {
-	ctx := context.Background()
-	env := setupDomainTest(t)
-	env.resolver.set("validation.valid.example.com", testCluster)
-
-	created, err := env.manager.CreateDomain(ctx, accountA, accountAUser, "valid.example.com", testCluster)
-	require.NoError(t, err)
-	require.True(t, created.Validated, "a matching CNAME should validate on create")
-
-	cluster, err := env.manager.DeriveClusterFromDomain(ctx, accountA, "valid.example.com")
-	require.NoError(t, err)
-	assert.Equal(t, testCluster, cluster)
-
-	cluster, err = env.manager.DeriveClusterFromDomain(ctx, accountA, "app.valid.example.com")
-	require.NoError(t, err)
-	assert.Equal(t, testCluster, cluster, "subdomains of a validated custom domain resolve too")
-}
-
-// Validating a domain flips the gate: the same lookup that failed before now
-// resolves a cluster.
-func TestValidateDomain_UnlocksClusterDerivation(t *testing.T) {
-	ctx := context.Background()
-	env := setupDomainTest(t)
-
-	created, err := env.manager.CreateDomain(ctx, accountA, accountAUser, "later.example.com", testCluster)
-	require.NoError(t, err)
-	require.False(t, created.Validated)
-
-	_, err = env.manager.DeriveClusterFromDomain(ctx, accountA, "later.example.com")
-	require.Error(t, err)
-
-	env.resolver.set("validation.later.example.com", testCluster)
-	env.manager.ValidateDomain(ctx, accountA, accountAUser, created.ID)
-
-	require.True(t, storedDomain(t, env.store, accountA, "later.example.com").Validated)
-
-	cluster, err := env.manager.DeriveClusterFromDomain(ctx, accountA, "later.example.com")
-	require.NoError(t, err)
-	assert.Equal(t, testCluster, cluster)
-}
-
-// Free cluster domains are unaffected by the custom domain gate.
-func TestDeriveClusterFromDomain_FreeDomainUnaffected(t *testing.T) {
-	ctx := context.Background()
-	env := setupDomainTest(t)
-
-	cluster, err := env.manager.DeriveClusterFromDomain(ctx, accountA, "myapp.abc123."+testCluster)
-	require.NoError(t, err)
-	assert.Equal(t, testCluster, cluster)
-}
-
-// The manager pre-check exists to turn a conflict into a 409, but the unique
-// index on the column is what actually guarantees the domain is claimed once.
-func TestStore_DuplicateDomainRejectedByIndex(t *testing.T) {
-	ctx := context.Background()
-	env := setupDomainTest(t)
-
-	_, err := env.store.CreateCustomDomain(ctx, accountA, "indexed.example.com", testCluster, false)
-	require.NoError(t, err)
-
-	_, err = env.store.CreateCustomDomain(ctx, accountB, "indexed.example.com", testCluster, false)
-	assert.Error(t, err, "the unique index must reject the same domain in a second account")
-}

management/internals/modules/reverseproxy/service/manager/domain_validation_test.go

@@ -1,127 +0,0 @@
-package manager
-
-import (
-	"context"
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"go.opentelemetry.io/otel/metric/noop"
-
-	domainmanager "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/domain/manager"
-	proxymanager "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/proxy/manager"
-	rpservice "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/service"
-	"github.com/netbirdio/netbird/management/server/activity"
-	"github.com/netbirdio/netbird/management/server/mock_server"
-	"github.com/netbirdio/netbird/management/server/permissions"
-	"github.com/netbirdio/netbird/management/server/store"
-	"github.com/netbirdio/netbird/shared/management/status"
-)
-
-const validationTestCluster = "eu.proxy.test"
-
-// withRealDomainManager swaps the stub cluster deriver for the real domain
-// manager backed by the same store, so service creation is gated by the actual
-// domain rows rather than by a test double that always agrees.
-func withRealDomainManager(t *testing.T, mgr *Manager, testStore store.Store) {
-	t.Helper()
-
-	ctx := context.Background()
-	proxyMgr, err := proxymanager.NewManager(testStore, noop.NewMeterProvider().Meter(""))
-	require.NoError(t, err)
-
-	_, err = proxyMgr.Connect(ctx, "proxy-1", "session-1", validationTestCluster, "127.0.0.1", nil, nil)
-	require.NoError(t, err)
-
-	accountMgr := &mock_server.MockAccountManager{
-		StoreEventFunc: func(context.Context, string, string, string, activity.ActivityDescriber, map[string]any) {},
-	}
-	mgr.clusterDeriver = domainmanager.NewManager(testStore, proxyMgr, permissions.NewManager(testStore), accountMgr)
-}
-
-func newTestService(domain string) *rpservice.Service {
-	return &rpservice.Service{
-		Name:    "test-service",
-		Domain:  domain,
-		Enabled: true,
-		Mode:    rpservice.ModeHTTP,
-		Targets: []*rpservice.Target{{
-			Host:       "10.0.0.1",
-			Port:       8080,
-			Protocol:   "http",
-			TargetId:   testPeerID,
-			TargetType: "peer",
-			Enabled:    true,
-		}},
-	}
-}
-
-// A service must not bind to a domain the account has not validated, and
-// nothing may be persisted for the attempt.
-func TestCreateService_RefusesUnvalidatedDomain(t *testing.T) {
-	ctx := context.Background()
-	mgr, testStore := setupIntegrationTest(t)
-	withRealDomainManager(t, mgr, testStore)
-
-	_, err := testStore.CreateCustomDomain(ctx, testAccountID, "unproven.example.com", validationTestCluster, false)
-	require.NoError(t, err)
-
-	_, err = mgr.CreateService(ctx, testAccountID, testUserID, newTestService("unproven.example.com"))
-	require.Error(t, err, "an unvalidated domain must not bind a service")
-	assert.Contains(t, err.Error(), "not validated", "the API error should name the actual problem")
-
-	sErr, ok := status.FromError(err)
-	require.True(t, ok, "error should be a typed status error")
-	assert.Equal(t, status.PreconditionFailed, sErr.Type())
-
-	services, err := testStore.GetAccountServices(ctx, store.LockingStrengthNone, testAccountID)
-	require.NoError(t, err)
-	assert.Empty(t, services, "no service row should be written for a refused domain")
-}
-
-// The negative control: a validated domain still binds a service and derives
-// its cluster exactly as before.
-func TestCreateService_ValidatedDomainBindsService(t *testing.T) {
-	ctx := context.Background()
-	mgr, testStore := setupIntegrationTest(t)
-	withRealDomainManager(t, mgr, testStore)
-
-	_, err := testStore.CreateCustomDomain(ctx, testAccountID, "proven.example.com", validationTestCluster, true)
-	require.NoError(t, err)
-
-	created, err := mgr.CreateService(ctx, testAccountID, testUserID, newTestService("app.proven.example.com"))
-	require.NoError(t, err)
-	assert.Equal(t, validationTestCluster, created.ProxyCluster, "service should bind to the domain's target cluster")
-
-	services, err := testStore.GetAccountServices(ctx, store.LockingStrengthNone, testAccountID)
-	require.NoError(t, err)
-	require.Len(t, services, 1, "the service should be persisted")
-	assert.Equal(t, "app.proven.example.com", services[0].Domain)
-}
-
-// An update must not be a way around the creation gate: moving a live service
-// onto an unvalidated domain has to fail rather than silently keep the old
-// cluster and start serving the new hostname.
-func TestUpdateService_RefusesMoveToUnvalidatedDomain(t *testing.T) {
-	ctx := context.Background()
-	mgr, testStore := setupIntegrationTest(t)
-	withRealDomainManager(t, mgr, testStore)
-
-	_, err := testStore.CreateCustomDomain(ctx, testAccountID, "proven.example.com", validationTestCluster, true)
-	require.NoError(t, err)
-	_, err = testStore.CreateCustomDomain(ctx, testAccountID, "unproven.example.com", validationTestCluster, false)
-	require.NoError(t, err)
-
-	created, err := mgr.CreateService(ctx, testAccountID, testUserID, newTestService("app.proven.example.com"))
-	require.NoError(t, err)
-
-	moved := *created
-	moved.Domain = "app.unproven.example.com"
-	_, err = mgr.UpdateService(ctx, testAccountID, testUserID, &moved)
-	require.Error(t, err, "moving to an unvalidated domain must fail")
-	assert.Contains(t, err.Error(), "not validated")
-
-	stored, err := testStore.GetServiceByID(ctx, store.LockingStrengthNone, testAccountID, created.ID)
-	require.NoError(t, err)
-	assert.Equal(t, "app.proven.example.com", stored.Domain, "the service must keep its original domain")
-}

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

@@ -606,19 +606,16 @@ func (m *Manager) resolveEffectiveCluster(ctx context.Context, accountID string,
 		return existing.ProxyCluster, nil
 	}
 
-	if m.clusterDeriver == nil {
-		return existing.ProxyCluster, nil
+	if m.clusterDeriver != nil {
+		derived, err := m.clusterDeriver.DeriveClusterFromDomain(ctx, accountID, svc.Domain)
+		if err != nil {
+			log.WithError(err).Warnf("could not derive cluster from domain %s", svc.Domain)
+		} else {
+			return derived, nil
+		}
 	}
 
-	// Falling back to the old cluster here would let an update move a service
-	// onto a domain the account has not validated, bypassing the check that
-	// creation makes.
-	derived, err := m.clusterDeriver.DeriveClusterFromDomain(ctx, accountID, svc.Domain)
-	if err != nil {
-		return "", status.Errorf(status.PreconditionFailed, "could not derive cluster from domain %s: %v", svc.Domain, err)
-	}
-
-	return derived, nil
+	return existing.ProxyCluster, nil
 }
 
 func (m *Manager) executeServiceUpdate(ctx context.Context, transaction store.Store, accountID string, service *service.Service, updateInfo *serviceUpdateInfo, customPorts *bool, effectiveCluster string) error {

management/server/store/sql_store.go

@@ -5658,23 +5658,6 @@ func (s *SqlStore) ListCustomDomains(ctx context.Context, accountID string) ([]*
 	return domains, nil
 }
 
-// GetCustomDomainByName returns the custom domain row holding the given name,
-// regardless of which account owns it.
-func (s *SqlStore) GetCustomDomainByName(ctx context.Context, domainName string) (*domain.Domain, error) {
-	customDomain := &domain.Domain{}
-	result := s.db.Take(customDomain, "domain = ?", domainName)
-	if result.Error != nil {
-		if errors.Is(result.Error, gorm.ErrRecordNotFound) {
-			return nil, status.Errorf(status.NotFound, "custom domain %s not found", domainName)
-		}
-
-		log.WithContext(ctx).Errorf("failed to get custom domain by name from store: %v", result.Error)
-		return nil, status.Errorf(status.Internal, "failed to get custom domain from store")
-	}
-
-	return customDomain, nil
-}
-
 func (s *SqlStore) CreateCustomDomain(ctx context.Context, accountID string, domainName string, targetCluster string, validated bool) (*domain.Domain, error) {
 	newDomain := &domain.Domain{
 		ID:            xid.New().String(), // Generate our own ID because gorm doesn't always configure the database to handle this for us.

management/server/store/store.go

@@ -294,7 +294,6 @@ 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)
-	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)
 	DeleteCustomDomain(ctx context.Context, accountID string, domainID string) error

management/server/store/store_mock.go

@@ -1892,21 +1892,6 @@ func (mr *MockStoreMockRecorder) GetCustomDomain(ctx, accountID, domainID interf
 	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetCustomDomain", reflect.TypeOf((*MockStore)(nil).GetCustomDomain), ctx, accountID, domainID)
 }
 
-// GetCustomDomainByName mocks base method.
-func (m *MockStore) GetCustomDomainByName(ctx context.Context, domainName string) (*domain.Domain, error) {
-	m.ctrl.T.Helper()
-	ret := m.ctrl.Call(m, "GetCustomDomainByName", ctx, domainName)
-	ret0, _ := ret[0].(*domain.Domain)
-	ret1, _ := ret[1].(error)
-	return ret0, ret1
-}
-
-// GetCustomDomainByName indicates an expected call of GetCustomDomainByName.
-func (mr *MockStoreMockRecorder) GetCustomDomainByName(ctx, domainName interface{}) *gomock.Call {
-	mr.mock.ctrl.T.Helper()
-	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetCustomDomainByName", reflect.TypeOf((*MockStore)(nil).GetCustomDomainByName), ctx, domainName)
-}
-
 // GetCustomDomainsCounts mocks base method.
 func (m *MockStore) GetCustomDomainsCounts(ctx context.Context) (int64, int64, error) {
 	m.ctrl.T.Helper()