diff --git a/management/internals/modules/reverseproxy/proxy/manager/manager.go b/management/internals/modules/reverseproxy/proxy/manager/manager.go index 943766004..f7a70b60f 100644 --- a/management/internals/modules/reverseproxy/proxy/manager/manager.go +++ b/management/internals/modules/reverseproxy/proxy/manager/manager.go @@ -26,6 +26,7 @@ type store interface { GetProxyByAccountID(ctx context.Context, accountID string) (*proxy.Proxy, error) CountProxiesByAccountID(ctx context.Context, accountID string) (int64, error) IsClusterAddressConflicting(ctx context.Context, clusterAddress, accountID string) (bool, error) + HasGatewayPinnedByOtherAccount(ctx context.Context, host, accountID string) (bool, error) DeleteAccountCluster(ctx context.Context, clusterAddress, accountID string) error } @@ -169,12 +170,36 @@ func (m *Manager) CountAccountProxies(ctx context.Context, accountID string) (in return m.store.CountProxiesByAccountID(ctx, accountID) } +// IsClusterAddressAvailable reports whether the account may claim this cluster +// address. +// +// Two kinds of claim make an address unavailable, and both are checked here so +// that no caller can consult one and forget the other. A proxy row is the +// obvious one. An agent network gateway pinned to the address by another +// account is the second: that pin is immutable and is served by whichever +// proxy declares the address, so letting a proxy from a different account take +// it strands the pin — an account-scoped proxy never receives another +// account's mappings. An account claiming an address its own gateway is pinned +// to is the intended order, not a conflict: pin first, deploy the proxy after. func (m *Manager) IsClusterAddressAvailable(ctx context.Context, clusterAddress, accountID string) (bool, error) { conflicting, err := m.store.IsClusterAddressConflicting(ctx, clusterAddress, accountID) if err != nil { return false, err } - return !conflicting, nil + if conflicting { + return false, nil + } + + pinned, err := m.store.HasGatewayPinnedByOtherAccount(ctx, clusterAddress, accountID) + if err != nil { + return false, err + } + if pinned { + log.WithContext(ctx).Infof("cluster address %s is pinned as another account's agent network gateway, refusing claim by account %s", clusterAddress, accountID) + return false, nil + } + + return true, nil } func (m *Manager) DeleteAccountCluster(ctx context.Context, clusterAddress, accountID string) error { diff --git a/management/internals/modules/reverseproxy/proxy/manager/manager_test.go b/management/internals/modules/reverseproxy/proxy/manager/manager_test.go index 5c44470a3..a338a2cc5 100644 --- a/management/internals/modules/reverseproxy/proxy/manager/manager_test.go +++ b/management/internals/modules/reverseproxy/proxy/manager/manager_test.go @@ -24,6 +24,7 @@ type mockStore struct { getProxyByAccountIDFunc func(ctx context.Context, accountID string) (*proxy.Proxy, error) countProxiesByAccountIDFunc func(ctx context.Context, accountID string) (int64, error) isClusterAddressConflictingFunc func(ctx context.Context, clusterAddress, accountID string) (bool, error) + hasGatewayPinnedByOtherAccountFunc func(ctx context.Context, host, accountID string) (bool, error) deleteAccountClusterFunc func(ctx context.Context, clusterAddress, accountID string) error } @@ -84,6 +85,12 @@ func (m *mockStore) IsClusterAddressConflicting(ctx context.Context, clusterAddr } return false, nil } +func (m *mockStore) HasGatewayPinnedByOtherAccount(ctx context.Context, host, accountID string) (bool, error) { + if m.hasGatewayPinnedByOtherAccountFunc != nil { + return m.hasGatewayPinnedByOtherAccountFunc(ctx, host, accountID) + } + return false, nil +} func (m *mockStore) DeleteAccountCluster(ctx context.Context, clusterAddress, accountID string) error { if m.deleteAccountClusterFunc != nil { return m.deleteAccountClusterFunc(ctx, clusterAddress, accountID) @@ -338,3 +345,65 @@ func TestGetActiveClusterAddressesForAccount(t *testing.T) { require.NoError(t, err) assert.Equal(t, expected, result) } + +// TestIsClusterAddressAvailableConsidersGatewayPins pins that a proxy row is +// not the only claim on an address. +// +// An agent network gateway pinned to the address by another account is +// immutable and is served by whichever proxy declares that address, so a proxy +// from a different account taking it strands the pin — the mapping paths never +// hand an account-scoped proxy another account's mappings. Refusing the later +// claimant is what makes the bootstrap-time ownership check hold over time +// rather than only at the instant it runs: without this, an address a gateway +// pinned while no proxy served it could be taken a moment, or a week, later. +func TestIsClusterAddressAvailableConsidersGatewayPins(t *testing.T) { + ctx := context.Background() + + tests := []struct { + name string + conflicting bool + pinned bool + available bool + }{ + {name: "free address", available: true}, + {name: "claimed by a proxy", conflicting: true}, + {name: "pinned by another account's gateway", pinned: true}, + {name: "claimed both ways", conflicting: true, pinned: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + st := &mockStore{ + isClusterAddressConflictingFunc: func(context.Context, string, string) (bool, error) { + return tt.conflicting, nil + }, + hasGatewayPinnedByOtherAccountFunc: func(context.Context, string, string) (bool, error) { + return tt.pinned, nil + }, + } + m, err := NewManager(st, noop.NewMeterProvider().Meter("")) + require.NoError(t, err) + + available, err := m.IsClusterAddressAvailable(ctx, "gw.example.com", "account1") + require.NoError(t, err) + assert.Equal(t, tt.available, available) + }) + } +} + +// TestIsClusterAddressAvailableSurfacesGatewayPinError pins that a failed pin +// lookup refuses the claim rather than falling through to available: this runs +// on the proxy-connect path, where "could not tell" must not read as "yes". +func TestIsClusterAddressAvailableSurfacesGatewayPinError(t *testing.T) { + st := &mockStore{ + hasGatewayPinnedByOtherAccountFunc: func(context.Context, string, string) (bool, error) { + return false, errors.New("db down") + }, + } + m, err := NewManager(st, noop.NewMeterProvider().Meter("")) + require.NoError(t, err) + + available, err := m.IsClusterAddressAvailable(context.Background(), "gw.example.com", "account1") + require.Error(t, err) + assert.False(t, available) +} diff --git a/management/server/store/sql_store_agentnetwork.go b/management/server/store/sql_store_agentnetwork.go index e75e36320..4d3d0222f 100644 --- a/management/server/store/sql_store_agentnetwork.go +++ b/management/server/store/sql_store_agentnetwork.go @@ -315,6 +315,32 @@ func (s *SqlStore) GetAllAgentNetworkSettings(ctx context.Context, lockStrength return settings, nil } +// HasGatewayPinnedByOtherAccount reports whether an account other than the +// given one has its agent network gateway pinned to this host. +// +// A pin is a claim on the host, the same way a proxy row is: the pinned +// endpoint is served by whichever proxy declares that address, and an +// account-scoped proxy only ever receives its own account's mappings. A proxy +// from a different account taking the address therefore cannot serve the pin +// and silently strands it. The pin is immutable, so the account that holds it +// cannot move out of the way — the later claimant is the one to refuse. +// +// Both sides are canonical (settings normalize on write, proxy addresses +// canonicalize at connect), so the match is exact and uses the proxy_address +// index. +func (s *SqlStore) HasGatewayPinnedByOtherAccount(ctx context.Context, host, accountID string) (bool, error) { + var count int64 + result := s.db. + Model(&agentNetworkTypes.Settings{}). + Where("proxy_address = ? AND account_id != ?", host, accountID). + Count(&count) + if result.Error != nil { + log.WithContext(ctx).Errorf("failed to check agent network gateway pins by proxy address: %v", result.Error) + return false, status.Errorf(status.Internal, "check agent network gateway pins") + } + return count > 0, nil +} + // GetAgentNetworkSettingsByProxyAddress returns every Settings row whose // gateway is served by the proxy declaring the given cluster address. Used by // cluster-scoped synthesis to find the accounts a shared proxy serves. diff --git a/management/server/store/store.go b/management/server/store/store.go index 55a8c319c..203ced596 100644 --- a/management/server/store/store.go +++ b/management/server/store/store.go @@ -374,6 +374,7 @@ type Store interface { GetAgentNetworkSettings(ctx context.Context, lockStrength LockingStrength, accountID string) (*agentNetworkTypes.Settings, error) GetAllAgentNetworkSettings(ctx context.Context, lockStrength LockingStrength) ([]*agentNetworkTypes.Settings, error) GetAgentNetworkSettingsByProxyAddress(ctx context.Context, lockStrength LockingStrength, proxyAddress string) ([]*agentNetworkTypes.Settings, error) + HasGatewayPinnedByOtherAccount(ctx context.Context, host, accountID string) (bool, error) GetAgentNetworkSettingsByDomain(ctx context.Context, lockStrength LockingStrength, domain string) (*agentNetworkTypes.Settings, error) CreateAgentNetworkSettings(ctx context.Context, settings *agentNetworkTypes.Settings) error SaveAgentNetworkSettings(ctx context.Context, settings *agentNetworkTypes.Settings) error diff --git a/management/server/store/store_mock.go b/management/server/store/store_mock.go index 4b8284212..f0299a32c 100644 --- a/management/server/store/store_mock.go +++ b/management/server/store/store_mock.go @@ -3080,6 +3080,21 @@ func (mr *MockStoreMockRecorder) HasForeignAccountProxyAtHost(ctx, host, account return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "HasForeignAccountProxyAtHost", reflect.TypeOf((*MockStore)(nil).HasForeignAccountProxyAtHost), ctx, host, accountID) } +// HasGatewayPinnedByOtherAccount mocks base method. +func (m *MockStore) HasGatewayPinnedByOtherAccount(ctx context.Context, host, accountID string) (bool, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "HasGatewayPinnedByOtherAccount", ctx, host, accountID) + ret0, _ := ret[0].(bool) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// HasGatewayPinnedByOtherAccount indicates an expected call of HasGatewayPinnedByOtherAccount. +func (mr *MockStoreMockRecorder) HasGatewayPinnedByOtherAccount(ctx, host, accountID any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "HasGatewayPinnedByOtherAccount", reflect.TypeOf((*MockStore)(nil).HasGatewayPinnedByOtherAccount), ctx, host, accountID) +} + // IncrementAgentNetworkConsumption mocks base method. func (m *MockStore) IncrementAgentNetworkConsumption(ctx context.Context, accountID string, kind types.ConsumptionDimension, dimID string, windowSeconds int64, windowStart time.Time, tokensIn, tokensOut int64, costUSD float64) error { m.ctrl.T.Helper()