[management] Fold migrated agent network identity on MySQL too

The normaliser's predicate never matched under MySQL's default
case-insensitive collation, and the comment called that the right answer
because nothing on MySQL was invisible. Only the SQL lookups are tolerant
there: the domain lookup is followed by an exact Go compare in
SynthesizeServiceForDomain, and the proxy's host map is keyed by the domain
verbatim, so a mixed-case row still missed. Spell the predicate byte-wise on
MySQL so the fold applies.

Two rows whose identities differ only by case would fold onto one endpoint,
which the unique index refuses with a driver message that names no row.
Both the reshape and the normaliser now stop first and name the hostname
that needs a human, in the same voice as the reshape's existing loud
failure.

The comments cited SNI folding as the reason the columns must be lowercase;
the SNI router folds both sides and would have matched. The readers that
miss are the exact ones, and the comments now name those.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Sa3DsBDP3VciAi4PPG17L6

management/server/migration/migration_agentnetwork.go

@@ -3,6 +3,7 @@ package migration
 import (
 	"context"
 	"fmt"
+	"strings"
 
 	log "github.com/sirupsen/logrus"
 	"gorm.io/gorm"
@@ -67,11 +68,12 @@ func MigrateAgentNetworkSettingsToDomain(ctx context.Context, db *gorm.DB) error
 
 		if hasCluster {
 			// The legacy bootstrap stored the cluster as the caller spelled
-			// it (trimmed, never folded), while every path that reads these
-			// columns now compares against canonical lowercase: proxy
-			// addresses are canonicalised at connect and the proxy folds the
-			// SNI host it routes on. Fold here so the reshaped row is
-			// addressable, rather than copying a spelling nothing will match.
+			// it, trimmed but never folded, while every reader of these
+			// columns matches exactly against canonical lowercase: proxy
+			// addresses are canonicalised at connect, and the proxy's host
+			// map is keyed by the domain verbatim. Fold here so the reshaped
+			// row is addressable, rather than copying a spelling nothing
+			// will match.
 			concat := "LOWER(subdomain || '.' || cluster)"
 			if tx.Name() == "mysql" {
 				concat = "LOWER(CONCAT(subdomain, '.', cluster))"
@@ -94,6 +96,9 @@ func MigrateAgentNetworkSettingsToDomain(ctx context.Context, db *gorm.DB) error
 					unmigratable,
 				)
 			}
+			if err := failOnDuplicateAgentNetworkDomains(tx); err != nil {
+				return err
+			}
 
 			if res.RowsAffected > 0 {
 				log.WithContext(ctx).Infof("migrated %d agent_network_settings row(s) to domain/proxy_address", res.RowsAffected)
@@ -128,28 +133,23 @@ type agentNetworkSettingsIdentity struct {
 
 func (agentNetworkSettingsIdentity) TableName() string { return "agent_network_settings" }
 
-// NormalizeAgentNetworkSettingsIdentity lowercases domain and proxy_address
-// on rows already reshaped by a release whose backfill copied the legacy
-// cluster spelling verbatim.
+// NormalizeAgentNetworkSettingsIdentity lowercases domain and proxy_address on
+// rows already reshaped by a release whose backfill copied the legacy cluster
+// spelling verbatim.
 //
-// Both columns are compared exactly against canonical lowercase values: a
-// proxy registering at a host asks whether another account's gateway is
-// pinned there by proxy_address, cluster-scoped mapping synthesis finds the
-// accounts a proxy serves the same way, and the proxy itself folds the SNI
-// host before matching a mapping's domain. A row that kept capitals from the
-// legacy schema is invisible to all three — its pin does not protect the
-// host, and its endpoint is never matched — so the value is repaired where it
-// is stored rather than folded on every read.
+// Every reader of these columns matches exactly against canonical lowercase:
+// the gateway-pin check a proxy registration runs and cluster-scoped mapping
+// synthesis look proxy_address up by the canonical address, the domain lookup
+// is followed by an exact Go compare, and the proxy's host map is keyed by the
+// domain verbatim. A row that kept capitals is invisible to all of them, so
+// the value is repaired where it is stored rather than folded on every read.
 //
-// Idempotent: the WHERE clause selects only rows that would change, so a
-// normalised table costs one pass over a table holding one row per account.
-// On MySQL the default collation already compares case-insensitively, so the
-// predicate never matches there and the statement is a no-op, which is the
-// right answer: nothing on MySQL was invisible to begin with. Runs after the
-// reshape, so the columns exist whenever the table does. Two rows that differ
-// only by case would collapse onto one domain, which the unique index
-// refuses; that state is unreachable through the API and the migration fails
-// loudly rather than guessing which endpoint to keep.
+// MySQL needs the predicate spelled byte-wise: under its default
+// case-insensitive collation `domain <> LOWER(domain)` is false for every row,
+// which would leave the rows unrepaired while the Go-side compares still miss
+// them. Idempotent: the predicate selects only rows that would change, one
+// pass over a table holding one row per account. Runs after the reshape, so
+// the columns exist whenever the table does.
 func NormalizeAgentNetworkSettingsIdentity(ctx context.Context, db *gorm.DB) error {
 	model := &agentNetworkSettingsIdentity{}
 	migrator := db.Migrator()
@@ -158,8 +158,15 @@ func NormalizeAgentNetworkSettingsIdentity(ctx context.Context, db *gorm.DB) err
 		return nil
 	}
 
-	res := db.Exec("UPDATE agent_network_settings SET domain = LOWER(domain), proxy_address = LOWER(proxy_address) " +
-		"WHERE domain <> LOWER(domain) OR proxy_address <> LOWER(proxy_address)")
+	if err := failOnDuplicateAgentNetworkDomains(db); err != nil {
+		return err
+	}
+
+	predicate := "domain <> LOWER(domain) OR proxy_address <> LOWER(proxy_address)"
+	if db.Name() == "mysql" {
+		predicate = "BINARY domain <> BINARY LOWER(domain) OR BINARY proxy_address <> BINARY LOWER(proxy_address)"
+	}
+	res := db.Exec("UPDATE agent_network_settings SET domain = LOWER(domain), proxy_address = LOWER(proxy_address) WHERE " + predicate)
 	if res.Error != nil {
 		return fmt.Errorf("normalize agent_network_settings identity casing: %w", res.Error)
 	}
@@ -169,3 +176,28 @@ func NormalizeAgentNetworkSettingsIdentity(ctx context.Context, db *gorm.DB) err
 
 	return nil
 }
+
+// failOnDuplicateAgentNetworkDomains refuses to continue when two settings
+// rows would fold onto one endpoint hostname. Two accounts cannot share an
+// endpoint, the unique index would refuse the fold with a driver message that
+// names no row, and there is no right answer as to which account keeps the
+// name, so the migration stops and says which hostname needs a human.
+func failOnDuplicateAgentNetworkDomains(db *gorm.DB) error {
+	var rows []struct{ Domain string }
+	err := db.Raw("SELECT LOWER(domain) AS domain FROM agent_network_settings GROUP BY LOWER(domain) HAVING COUNT(*) > 1").
+		Scan(&rows).Error
+	if err != nil {
+		return fmt.Errorf("check agent_network_settings for endpoints differing only by case: %w", err)
+	}
+	if len(rows) == 0 {
+		return nil
+	}
+	duplicates := make([]string, 0, len(rows))
+	for _, row := range rows {
+		duplicates = append(duplicates, row.Domain)
+	}
+	return fmt.Errorf(
+		"agent_network_settings holds endpoints that differ only by case (%s); resolve them manually before upgrading",
+		strings.Join(duplicates, ", "),
+	)
+}

management/server/migration/migration_test.go

@@ -757,9 +757,11 @@ func TestMigrateAgentNetworkSettingsToDomain_BackfillsAndDropsLegacyColumns(t *t
 	db := setupDatabase(t)
 	require.NoError(t, db.Migrator().DropTable(&legacyAgentNetworkSettings{}))
 	require.NoError(t, db.AutoMigrate(&legacyAgentNetworkSettings{}))
-	// Spelled the way the legacy bootstrap kept it: trimmed, never folded.
+	// The cluster is spelled the way the legacy bootstrap kept it: as the
+	// caller typed it, trimmed but never folded. The subdomain was always
+	// server-assigned lowercase.
 	require.NoError(t, db.Create(&legacyAgentNetworkSettings{
-		AccountID: "acct-1", Cluster: "EU.Proxy.NetBird.io", Subdomain: "Violet", EnableLogCollection: true,
+		AccountID: "acct-1", Cluster: "EU.Proxy.NetBird.io", Subdomain: "violet", EnableLogCollection: true,
 	}).Error)
 	require.NoError(t, db.Create(&legacyAgentNetworkSettings{
 		AccountID: "acct-2", Cluster: "us.proxy.netbird.io", Subdomain: "violet",
@@ -904,3 +906,52 @@ func TestNormalizeAgentNetworkSettingsIdentity_SkipsMissingTable(t *testing.T) {
 		"no table must be a no-op, not an error")
 	assert.False(t, db.Migrator().HasTable(&agentNetworkTypes.Settings{}), "the normaliser must not create the table")
 }
+
+// TestNormalizeAgentNetworkSettingsIdentity_RefusesCaseOnlyCollision pins the
+// loud failure: two rows that would fold onto one endpoint stop the migration
+// with the hostname named, and neither row is touched, rather than letting the
+// unique index refuse the fold with a driver message that names no row.
+func TestNormalizeAgentNetworkSettingsIdentity_RefusesCaseOnlyCollision(t *testing.T) {
+	ctx := context.Background()
+	db := setupDatabase(t)
+	if db.Name() == "mysql" {
+		t.Skip("MySQL's default collation refuses two rows differing only by case at insert; the collision cannot exist there")
+	}
+	require.NoError(t, db.Migrator().DropTable(&agentNetworkTypes.Settings{}))
+	require.NoError(t, db.AutoMigrate(&agentNetworkTypes.Settings{}))
+	require.NoError(t, db.Create(&agentNetworkTypes.Settings{
+		AccountID: "acct-1", Domain: "Violet.eu.proxy.netbird.io", ProxyAddress: "eu.proxy.netbird.io",
+	}).Error)
+	require.NoError(t, db.Create(&agentNetworkTypes.Settings{
+		AccountID: "acct-2", Domain: "violet.eu.proxy.netbird.io", ProxyAddress: "eu.proxy.netbird.io",
+	}).Error)
+
+	err := migration.NormalizeAgentNetworkSettingsIdentity(ctx, db)
+	require.Error(t, err, "two rows folding onto one endpoint must stop the migration")
+	assert.Contains(t, err.Error(), "violet.eu.proxy.netbird.io", "the failure must name the colliding hostname")
+
+	var one agentNetworkTypes.Settings
+	require.NoError(t, db.First(&one, "account_id = ?", "acct-1").Error)
+	assert.Equal(t, "Violet.eu.proxy.netbird.io", one.Domain, "a refused normalisation must leave every row as it was")
+}
+
+// TestMigrateAgentNetworkSettingsToDomain_RefusesCaseOnlyCollision pins the
+// same loud failure on the reshape: legacy rows whose identities differ only
+// by case would fold onto one endpoint, and the reshape must say so rather
+// than leave AutoMigrate to fail on the unique index.
+func TestMigrateAgentNetworkSettingsToDomain_RefusesCaseOnlyCollision(t *testing.T) {
+	ctx := context.Background()
+	db := setupDatabase(t)
+	require.NoError(t, db.Migrator().DropTable(&legacyAgentNetworkSettings{}))
+	require.NoError(t, db.AutoMigrate(&legacyAgentNetworkSettings{}))
+	require.NoError(t, db.Create(&legacyAgentNetworkSettings{
+		AccountID: "acct-1", Cluster: "EU.proxy.netbird.io", Subdomain: "violet",
+	}).Error)
+	require.NoError(t, db.Create(&legacyAgentNetworkSettings{
+		AccountID: "acct-2", Cluster: "eu.proxy.netbird.io", Subdomain: "violet",
+	}).Error)
+
+	err := migration.MigrateAgentNetworkSettingsToDomain(ctx, db)
+	require.Error(t, err, "legacy rows folding onto one endpoint must stop the reshape")
+	assert.Contains(t, err.Error(), "violet.eu.proxy.netbird.io", "the failure must name the colliding hostname")
+}