diff --git a/management/internals/modules/agentnetwork/domainlookup_test.go b/management/internals/modules/agentnetwork/domainlookup_test.go new file mode 100644 index 000000000..a77a72070 --- /dev/null +++ b/management/internals/modules/agentnetwork/domainlookup_test.go @@ -0,0 +1,115 @@ +package agentnetwork + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/netbirdio/netbird/management/server/store" +) + +// TestSynthesizeServiceForDomain_ResolvesZoneBasedEndpoint — with a Zone the +// hostname's parent is the zone, not the cluster, so the old "strip the first +// label and match a cluster" prefilter found nothing and every zone-based +// tenant failed to resolve on the auth path. +func TestSynthesizeServiceForDomain_ResolvesZoneBasedEndpoint(t *testing.T) { + ctx := context.Background() + s, cleanup, err := store.NewTestStoreFromSQL(ctx, "", t.TempDir()) + require.NoError(t, err, "real sqlite test store must come up") + defer cleanup() + + settings := newSynthTestSettings() + settings.Cluster = "eu.proxy.netbird.io" + settings.Zone = "gateway.netbird.ai" + settings.Subdomain = "brave-otter" + require.NoError(t, s.SaveAgentNetworkSettings(ctx, settings)) + provider := newSynthTestProvider() + require.NoError(t, s.SaveAgentNetworkProvider(ctx, provider)) + require.NoError(t, s.SaveAgentNetworkPolicy(ctx, newSynthTestPolicy(provider.ID, "grp-eng", ""))) + + domain := "brave-otter.gateway.netbird.ai" + svc, err := SynthesizeServiceForDomain(ctx, s, domain) + require.NoError(t, err) + require.NotNil(t, svc, "zone-based endpoint must resolve to the owning account's service") + assert.Equal(t, domain, svc.Domain) +} + +// TestSynthesizeServiceForDomain_ResolvesLegacyClusterEndpoint — the +// non-breaking guarantee. A row with no Zone still resolves at +// ., because the subdomain is the first label either way. +func TestSynthesizeServiceForDomain_ResolvesLegacyClusterEndpoint(t *testing.T) { + ctx := context.Background() + s, cleanup, err := store.NewTestStoreFromSQL(ctx, "", t.TempDir()) + require.NoError(t, err, "real sqlite test store must come up") + defer cleanup() + + settings := newSynthTestSettings() + settings.Cluster = "eu.proxy.netbird.io" + settings.Zone = "" + settings.Subdomain = "swift-heron" + require.NoError(t, s.SaveAgentNetworkSettings(ctx, settings)) + provider := newSynthTestProvider() + require.NoError(t, s.SaveAgentNetworkProvider(ctx, provider)) + require.NoError(t, s.SaveAgentNetworkPolicy(ctx, newSynthTestPolicy(provider.ID, "grp-eng", ""))) + + domain := "swift-heron.eu.proxy.netbird.io" + svc, err := SynthesizeServiceForDomain(ctx, s, domain) + require.NoError(t, err) + require.NotNil(t, svc, "legacy cluster-based endpoint must still resolve") + assert.Equal(t, domain, svc.Domain) +} + +// TestSynthesizeServiceForDomain_LabelMatchesButParentDoesNot — the label is +// globally unique, so a lookup by first label can hit a row that does NOT own +// the queried hostname. That must resolve to nothing rather than to the wrong +// account's service. +func TestSynthesizeServiceForDomain_LabelMatchesButParentDoesNot(t *testing.T) { + ctx := context.Background() + s, cleanup, err := store.NewTestStoreFromSQL(ctx, "", t.TempDir()) + require.NoError(t, err, "real sqlite test store must come up") + defer cleanup() + + settings := newSynthTestSettings() + settings.Cluster = "eu.proxy.netbird.io" + settings.Zone = "gateway.netbird.ai" + settings.Subdomain = "brave-otter" + require.NoError(t, s.SaveAgentNetworkSettings(ctx, settings)) + provider := newSynthTestProvider() + require.NoError(t, s.SaveAgentNetworkProvider(ctx, provider)) + require.NoError(t, s.SaveAgentNetworkPolicy(ctx, newSynthTestPolicy(provider.ID, "grp-eng", ""))) + + svc, err := SynthesizeServiceForDomain(ctx, s, "brave-otter.someone-elses.zone") + require.NoError(t, err) + assert.Nil(t, svc, "label matched a different endpoint's parent; must not resolve to the wrong account") +} + +// TestSynthesizeServiceForDomain_UnknownLabel — a hostname whose first label +// belongs to no account is a miss, not an error: the caller falls back to the +// persisted-service lookup and a returned error would mask that. +func TestSynthesizeServiceForDomain_UnknownLabel(t *testing.T) { + ctx := context.Background() + s, cleanup, err := store.NewTestStoreFromSQL(ctx, "", t.TempDir()) + require.NoError(t, err, "real sqlite test store must come up") + defer cleanup() + + svc, err := SynthesizeServiceForDomain(ctx, s, "nobody-home.gateway.netbird.ai") + require.NoError(t, err) + assert.Nil(t, svc, "unknown label must be a miss, not an error") +} + +// TestSynthesizeServiceForDomain_DegenerateInput — empty and single-label +// hostnames have no subdomain to look up and must not reach the store. +func TestSynthesizeServiceForDomain_DegenerateInput(t *testing.T) { + ctx := context.Background() + s, cleanup, err := store.NewTestStoreFromSQL(ctx, "", t.TempDir()) + require.NoError(t, err, "real sqlite test store must come up") + defer cleanup() + + for _, domain := range []string{"", "localhost"} { + svc, err := SynthesizeServiceForDomain(ctx, s, domain) + require.NoError(t, err, "domain %q", domain) + assert.Nil(t, svc, "domain %q has no subdomain label to look up", domain) + } +} diff --git a/management/internals/modules/agentnetwork/synthesizer.go b/management/internals/modules/agentnetwork/synthesizer.go index 64711387b..2a3002784 100644 --- a/management/internals/modules/agentnetwork/synthesizer.go +++ b/management/internals/modules/agentnetwork/synthesizer.go @@ -116,45 +116,46 @@ func SynthesizeServicesForCluster(ctx context.Context, s store.Store, clusterAdd } // SynthesizeServiceForDomain resolves a single agent-network service by its -// public endpoint domain. It lists the (few) settings rows on the domain's -// cluster, matches the one whose endpoint equals the domain, and synthesises -// only that account — avoiding full per-account synthesis for every tenant on -// the cluster, which is what auth/session paths previously paid. Returns nil -// (no error) when no account owns the domain. +// endpoint hostname. Both endpoint shapes put the account's label in the first +// DNS label — . and . — and the label is +// globally unique, so this is a single indexed lookup for either shape. It +// synthesises only the owning account rather than every tenant on a cluster, +// which is what auth/session paths previously paid. Returns nil (no error) when +// no account owns the hostname. func SynthesizeServiceForDomain(ctx context.Context, s store.Store, domain string) (*rpservice.Service, error) { domain = strings.TrimSpace(domain) - cluster := clusterFromDomain(domain) - if domain != "" && cluster != "" { - settingsRows, err := s.GetAgentNetworkSettingsByCluster(ctx, store.LockingStrengthNone, cluster) - if err != nil { - return nil, fmt.Errorf("list agent network settings on cluster: %w", err) - } - for _, settings := range settingsRows { - if settings == nil || settings.Endpoint() != domain { - continue - } - services, serr := SynthesizeServices(ctx, s, settings.AccountID) - if serr != nil { - return nil, serr - } - for _, svc := range services { - if svc != nil && svc.Domain == domain { - return svc, nil - } - } - break - } + subdomain, _, found := strings.Cut(domain, ".") + if !found || subdomain == "" { + return nil, nil //nolint:nilnil // no label to resolve: not an owned endpoint } - return nil, nil //nolint:nilnil // optional lookup: no account owns the domain -} -// clusterFromDomain returns the cluster portion of an endpoint domain (every -// label after the first). -func clusterFromDomain(domain string) string { - if i := strings.IndexByte(domain, '.'); i >= 0 { - return domain[i+1:] + settings, err := s.GetAgentNetworkSettingsBySubdomain(ctx, store.LockingStrengthNone, subdomain) + if err != nil { + var sErr *status.Error + if errors.As(err, &sErr) && sErr.Type() == status.NotFound { + return nil, nil //nolint:nilnil // no account owns the label + } + // A real store failure must surface: the caller treats nil as "not an + // agent-network endpoint" and would silently mask a database error. + return nil, fmt.Errorf("get agent network settings by subdomain: %w", err) } - return "" + + // The label is unique but the parent is not implied by it: a row owning + // "brave-otter" does not own "brave-otter.some-other.zone". + if settings.Endpoint() != domain { + return nil, nil //nolint:nilnil // label matched a different endpoint + } + + services, err := SynthesizeServices(ctx, s, settings.AccountID) + if err != nil { + return nil, err + } + for _, svc := range services { + if svc != nil && svc.Domain == domain { + return svc, nil + } + } + return nil, nil //nolint:nilnil // owner found but it emits no service } // SynthesizeServices builds the in-memory reverse-proxy service that diff --git a/management/server/store/sql_store_agentnetwork.go b/management/server/store/sql_store_agentnetwork.go index cdfe743b9..883923156 100644 --- a/management/server/store/sql_store_agentnetwork.go +++ b/management/server/store/sql_store_agentnetwork.go @@ -334,6 +334,30 @@ func (s *SqlStore) GetAgentNetworkSettingsByCluster(ctx context.Context, lockStr return settings, nil } +// GetAgentNetworkSettingsBySubdomain returns the settings row that owns the +// given subdomain label. The label is globally unique (enforced by +// idx_agent_network_settings_subdomain_unique), so at most one row can match, +// which makes this an indexed point lookup rather than a scan. +func (s *SqlStore) GetAgentNetworkSettingsBySubdomain(ctx context.Context, lockStrength LockingStrength, subdomain string) (*agentNetworkTypes.Settings, error) { + tx := s.db + if lockStrength != LockingStrengthNone { + tx = tx.Clauses(clause.Locking{Strength: string(lockStrength)}) + } + + var settings agentNetworkTypes.Settings + result := tx.Take(&settings, "subdomain = ?", subdomain) + if result.Error != nil { + if errors.Is(result.Error, gorm.ErrRecordNotFound) { + return nil, status.Errorf(status.NotFound, "agent network settings for subdomain %s not found", subdomain) + } + + log.WithContext(ctx).Errorf("failed to get agent network settings by subdomain from store: %v", result.Error) + return nil, status.Errorf(status.Internal, "failed to get agent network settings by subdomain from store") + } + + return &settings, nil +} + // SaveAgentNetworkSettings upserts the per-account Agent Network // settings row. func (s *SqlStore) SaveAgentNetworkSettings(ctx context.Context, settings *agentNetworkTypes.Settings) error { diff --git a/management/server/store/store.go b/management/server/store/store.go index 9bb2e8890..c0e086378 100644 --- a/management/server/store/store.go +++ b/management/server/store/store.go @@ -361,6 +361,7 @@ type Store interface { GetAgentNetworkSettings(ctx context.Context, lockStrength LockingStrength, accountID string) (*agentNetworkTypes.Settings, error) GetAllAgentNetworkSettings(ctx context.Context, lockStrength LockingStrength) ([]*agentNetworkTypes.Settings, error) GetAgentNetworkSettingsByCluster(ctx context.Context, lockStrength LockingStrength, cluster string) ([]*agentNetworkTypes.Settings, error) + GetAgentNetworkSettingsBySubdomain(ctx context.Context, lockStrength LockingStrength, subdomain string) (*agentNetworkTypes.Settings, error) SaveAgentNetworkSettings(ctx context.Context, settings *agentNetworkTypes.Settings) error CreateAgentNetworkSettings(ctx context.Context, settings *agentNetworkTypes.Settings) error IncrementAgentNetworkConsumption(ctx context.Context, accountID string, kind agentNetworkTypes.ConsumptionDimension, dimID string, windowSeconds int64, windowStart time.Time, tokensIn, tokensOut int64, costUSD float64) error diff --git a/management/server/store/store_mock.go b/management/server/store/store_mock.go index f006fc00b..b96cd2f08 100644 --- a/management/server/store/store_mock.go +++ b/management/server/store/store_mock.go @@ -1716,6 +1716,21 @@ func (mr *MockStoreMockRecorder) GetAgentNetworkSettingsByCluster(ctx, lockStren return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAgentNetworkSettingsByCluster", reflect.TypeOf((*MockStore)(nil).GetAgentNetworkSettingsByCluster), ctx, lockStrength, cluster) } +// GetAgentNetworkSettingsBySubdomain mocks base method. +func (m *MockStore) GetAgentNetworkSettingsBySubdomain(ctx context.Context, lockStrength LockingStrength, subdomain string) (*types.Settings, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetAgentNetworkSettingsBySubdomain", ctx, lockStrength, subdomain) + ret0, _ := ret[0].(*types.Settings) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetAgentNetworkSettingsBySubdomain indicates an expected call of GetAgentNetworkSettingsBySubdomain. +func (mr *MockStoreMockRecorder) GetAgentNetworkSettingsBySubdomain(ctx, lockStrength, subdomain interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAgentNetworkSettingsBySubdomain", reflect.TypeOf((*MockStore)(nil).GetAgentNetworkSettingsBySubdomain), ctx, lockStrength, subdomain) +} + // GetAgentNetworkUsageRows mocks base method. func (m *MockStore) GetAgentNetworkUsageRows(ctx context.Context, lockStrength LockingStrength, accountID string, filter types.AgentNetworkAccessLogFilter) ([]*types.AgentNetworkUsage, error) { m.ctrl.T.Helper()