From 39f8ea3f70e16c28c7aba1a4af4586a6af2cd24d Mon Sep 17 00:00:00 2001 From: mlsmaycon Date: Sat, 12 Sep 2026 12:53:15 +0000 Subject: [PATCH] [management] Fold casing on agent network identity the legacy schema kept MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The legacy bootstrap stored the cluster as the caller spelled it — trimmed, never folded — and the reshape that shipped in 0.78 copied it into proxy_address, and subdomain.cluster into domain, verbatim. Everything that reads those columns compares against canonical lowercase: proxies canonicalise their address at connect, the gateway-pin check a proxy registration runs matches proxy_address exactly, cluster-scoped synthesis finds an account's row by proxy_address, and the proxy folds the SNI host before matching a mapping's domain. A row that kept capitals is invisible to all of them — its pin protects nothing and its endpoint never routes. The reshape now writes LOWER() for both columns, and an idempotent normaliser lowercases rows a released reshape already copied, registered right after it. The predicate selects only rows that would change, so a normalised table costs one pass over one row per account; on MySQL the default collation compares case-insensitively already and it is a no-op. Reported by cubic on #7402: the exact proxy_address match a proxy registration relies on misses a migrated pin with capitals. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01Sa3DsBDP3VciAi4PPG17L6 --- .../migration/migration_agentnetwork.go | 65 ++++++++++++++++++- management/server/migration/migration_test.go | 52 ++++++++++++++- 2 files changed, 111 insertions(+), 6 deletions(-) diff --git a/management/server/migration/migration_agentnetwork.go b/management/server/migration/migration_agentnetwork.go index c6cda56c2..d81bcc91a 100644 --- a/management/server/migration/migration_agentnetwork.go +++ b/management/server/migration/migration_agentnetwork.go @@ -66,12 +66,18 @@ func MigrateAgentNetworkSettingsToDomain(ctx context.Context, db *gorm.DB) error } if hasCluster { - concat := "subdomain || '.' || cluster" + // 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. + concat := "LOWER(subdomain || '.' || cluster)" if tx.Name() == "mysql" { - concat = "CONCAT(subdomain, '.', cluster)" + concat = "LOWER(CONCAT(subdomain, '.', cluster))" } res := tx.Exec(fmt.Sprintf( - "UPDATE agent_network_settings SET domain = %s, proxy_address = cluster WHERE (domain IS NULL OR domain = '') AND cluster <> '' AND subdomain <> ''", + "UPDATE agent_network_settings SET domain = %s, proxy_address = LOWER(cluster) WHERE (domain IS NULL OR domain = '') AND cluster <> '' AND subdomain <> ''", concat, )) if res.Error != nil { @@ -110,3 +116,56 @@ func MigrateAgentNetworkSettingsToDomain(ctx context.Context, db *gorm.DB) error return nil }) } + +// agentNetworkSettingsIdentity is the post-reshape view of the two identity +// columns, enough for the normaliser to address the table without importing +// the current model. +type agentNetworkSettingsIdentity struct { + AccountID string `gorm:"primaryKey"` + Domain string `gorm:"type:varchar(255)"` + ProxyAddress string `gorm:"type:varchar(255)"` +} + +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. +// +// 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. +// +// 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. +func NormalizeAgentNetworkSettingsIdentity(ctx context.Context, db *gorm.DB) error { + model := &agentNetworkSettingsIdentity{} + migrator := db.Migrator() + + if !migrator.HasTable(model) || !migrator.HasColumn(model, "Domain") || !migrator.HasColumn(model, "ProxyAddress") { + 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 res.Error != nil { + return fmt.Errorf("normalize agent_network_settings identity casing: %w", res.Error) + } + if res.RowsAffected > 0 { + log.WithContext(ctx).Infof("normalized casing on %d agent_network_settings row(s)", res.RowsAffected) + } + + return nil +} diff --git a/management/server/migration/migration_test.go b/management/server/migration/migration_test.go index 868332fdf..65cf3a99e 100644 --- a/management/server/migration/migration_test.go +++ b/management/server/migration/migration_test.go @@ -757,8 +757,9 @@ 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. 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", @@ -770,8 +771,10 @@ func TestMigrateAgentNetworkSettingsToDomain_BackfillsAndDropsLegacyColumns(t *t var one, two agentNetworkTypes.Settings require.NoError(t, db.First(&one, "account_id = ?", "acct-1").Error) - assert.Equal(t, "violet.eu.proxy.netbird.io", one.Domain, "domain must combine subdomain and cluster") - assert.Equal(t, "eu.proxy.netbird.io", one.ProxyAddress, "proxy address must carry the cluster") + assert.Equal(t, "violet.eu.proxy.netbird.io", one.Domain, + "domain must combine subdomain and cluster, folded to the canonical lowercase every reader compares against") + assert.Equal(t, "eu.proxy.netbird.io", one.ProxyAddress, + "proxy address must carry the cluster in canonical lowercase, matching what proxies register under") assert.True(t, one.EnableLogCollection, "non-identity fields must ride through") require.NoError(t, db.First(&two, "account_id = ?", "acct-2").Error) assert.Equal(t, "violet.us.proxy.netbird.io", two.Domain, @@ -858,3 +861,46 @@ func TestMigrateAgentNetworkSettingsToDomain_ResumesAfterPartialDrop(t *testing. assert.Equal(t, "violet.eu.proxy.netbird.io", row.Domain, "migrated values must be untouched") assert.Equal(t, "eu.proxy.netbird.io", row.ProxyAddress, "migrated values must be untouched") } + +// TestNormalizeAgentNetworkSettingsIdentity_LowercasesReshapedRows covers rows +// a released reshape already copied verbatim: capitals kept from the legacy +// cluster spelling are folded in place, canonical rows are left alone, and +// non-identity fields ride through. +func TestNormalizeAgentNetworkSettingsIdentity_LowercasesReshapedRows(t *testing.T) { + ctx := context.Background() + db := setupDatabase(t) + require.NoError(t, db.Migrator().DropTable(&agentNetworkTypes.Settings{})) + require.NoError(t, db.AutoMigrate(&agentNetworkTypes.Settings{})) + require.NoError(t, db.Create(&agentNetworkTypes.Settings{ + AccountID: "acct-legacy", Domain: "Violet.EU.Proxy.NetBird.io", ProxyAddress: "EU.Proxy.NetBird.io", EnableLogCollection: true, + }).Error) + require.NoError(t, db.Create(&agentNetworkTypes.Settings{ + AccountID: "acct-canonical", Domain: "amber.us.proxy.netbird.io", ProxyAddress: "us.proxy.netbird.io", + }).Error) + + require.NoError(t, migration.NormalizeAgentNetworkSettingsIdentity(ctx, db)) + + var legacy, canonical agentNetworkTypes.Settings + require.NoError(t, db.First(&legacy, "account_id = ?", "acct-legacy").Error) + assert.Equal(t, "violet.eu.proxy.netbird.io", legacy.Domain, "a mixed-case endpoint must be folded where it is stored") + assert.Equal(t, "eu.proxy.netbird.io", legacy.ProxyAddress, "a mixed-case pin must be folded so exact lookups find it") + assert.True(t, legacy.EnableLogCollection, "non-identity fields must ride through") + require.NoError(t, db.First(&canonical, "account_id = ?", "acct-canonical").Error) + assert.Equal(t, "amber.us.proxy.netbird.io", canonical.Domain, "a canonical row must be left as it is") + assert.Equal(t, "us.proxy.netbird.io", canonical.ProxyAddress) + + require.NoError(t, migration.NormalizeAgentNetworkSettingsIdentity(ctx, db), + "a second run over a normalised table must be a no-op, not an error") +} + +// TestNormalizeAgentNetworkSettingsIdentity_SkipsMissingTable pins that a +// store which never had agent network settings is left untouched. +func TestNormalizeAgentNetworkSettingsIdentity_SkipsMissingTable(t *testing.T) { + ctx := context.Background() + db := setupDatabase(t) + require.NoError(t, db.Migrator().DropTable(&agentNetworkTypes.Settings{})) + + require.NoError(t, migration.NormalizeAgentNetworkSettingsIdentity(ctx, db), + "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") +}