From d250f92c435bac83fd55f00fad3ee2c292eee910 Mon Sep 17 00:00:00 2001 From: Maycon Santos Date: Wed, 20 May 2026 10:08:34 +0200 Subject: [PATCH] feat(reverse-proxy): clusters API surfaces type, online status, and capability flags (#6148) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The cluster listing now answers three questions in one round-trip instead of forcing the dashboard to cross-reference the domains API: which clusters can this account see, are they currently up, and what do they support. The ProxyCluster wire type drops the boolean self_hosted in favour of a `type` enum (`account` / `shared`) plus explicit `online`, `supports_custom_ports`, `require_subdomain`, and `supports_crowdsec` fields. Store query reworked so offline clusters still appear (no last_seen WHERE), with online and connected_proxies both derived from the existing 2-min active window via portable CASE expressions; the 1-hour heartbeat reaper still removes long-stale rows. Service manager enriches each cluster with the capability flags via the existing per-cluster lookups (CapabilityProvider now also exposes ClusterSupportsCrowdSec). GetActiveClusterAddresses* keep their tight 2-min filter so service routing and domain enumeration aren't pulled into the wider window. The hard cut removes self_hosted from the response — the dashboard is the only consumer and is updated in the matching PR; no transitional field is shipped. Adds a cross-engine regression test asserting offline clusters surface, connected_proxies counts only fresh proxies, and account-scoped BYOP clusters never leak across accounts. --- .../reverseproxy/proxy/manager/manager.go | 2 +- .../proxy/manager/manager_test.go | 2 +- .../modules/reverseproxy/proxy/proxy.go | 27 ++++- .../modules/reverseproxy/service/interface.go | 2 +- .../reverseproxy/service/interface_mock.go | 72 ++++++------ .../reverseproxy/service/manager/api.go | 14 ++- .../reverseproxy/service/manager/manager.go | 22 +++- .../shared/grpc/proxy_group_access_test.go | 2 +- .../shared/grpc/validate_session_test.go | 2 +- .../proxy/auth_callback_integration_test.go | 2 +- management/server/store/sql_store.go | 64 ++++++++-- .../store/sql_store_proxy_clusters_test.go | 109 ++++++++++++++++++ management/server/store/store.go | 2 +- management/server/store/store_mock.go | 86 +++++++------- proxy/management_integration_test.go | 2 +- shared/management/http/api/openapi.yml | 32 ++++- shared/management/http/api/types.gen.go | 73 +++++++++--- 17 files changed, 393 insertions(+), 122 deletions(-) create mode 100644 management/server/store/sql_store_proxy_clusters_test.go diff --git a/management/internals/modules/reverseproxy/proxy/manager/manager.go b/management/internals/modules/reverseproxy/proxy/manager/manager.go index b72a6ebe5..510500e0c 100644 --- a/management/internals/modules/reverseproxy/proxy/manager/manager.go +++ b/management/internals/modules/reverseproxy/proxy/manager/manager.go @@ -17,7 +17,7 @@ type store interface { UpdateProxyHeartbeat(ctx context.Context, p *proxy.Proxy) error GetActiveProxyClusterAddresses(ctx context.Context) ([]string, error) GetActiveProxyClusterAddressesForAccount(ctx context.Context, accountID string) ([]string, error) - GetActiveProxyClusters(ctx context.Context, accountID string) ([]proxy.Cluster, error) + GetProxyClusters(ctx context.Context, accountID string) ([]proxy.Cluster, error) GetClusterSupportsCustomPorts(ctx context.Context, clusterAddr string) *bool GetClusterRequireSubdomain(ctx context.Context, clusterAddr string) *bool GetClusterSupportsCrowdSec(ctx context.Context, clusterAddr string) *bool diff --git a/management/internals/modules/reverseproxy/proxy/manager/manager_test.go b/management/internals/modules/reverseproxy/proxy/manager/manager_test.go index 3c53fe684..3436216b4 100644 --- a/management/internals/modules/reverseproxy/proxy/manager/manager_test.go +++ b/management/internals/modules/reverseproxy/proxy/manager/manager_test.go @@ -57,7 +57,7 @@ func (m *mockStore) GetActiveProxyClusterAddressesForAccount(ctx context.Context } return nil, nil } -func (m *mockStore) GetActiveProxyClusters(_ context.Context, _ string) ([]proxy.Cluster, error) { +func (m *mockStore) GetProxyClusters(_ context.Context, _ string) ([]proxy.Cluster, error) { return nil, nil } func (m *mockStore) CleanupStaleProxies(ctx context.Context, d time.Duration) error { diff --git a/management/internals/modules/reverseproxy/proxy/proxy.go b/management/internals/modules/reverseproxy/proxy/proxy.go index 64394799e..9da7910df 100644 --- a/management/internals/modules/reverseproxy/proxy/proxy.go +++ b/management/internals/modules/reverseproxy/proxy/proxy.go @@ -42,10 +42,35 @@ func (Proxy) TableName() string { return "proxies" } +// ClusterType is the source of a proxy cluster. +type ClusterType string + +const ( + // ClusterTypeAccount is a cluster operated by the account itself (BYOP) — + // at least one proxy row in the cluster carries a non-NULL account_id. + ClusterTypeAccount ClusterType = "account" + // ClusterTypeShared is a cluster operated by NetBird and shared across + // accounts — all proxy rows in the cluster have account_id IS NULL. + ClusterTypeShared ClusterType = "shared" +) + // Cluster represents a group of proxy nodes serving the same address. +// +// Online and ConnectedProxies derive from the same 2-min active window +// the rest of the module uses, but Cluster rows are not gated on it — +// the cluster listing surfaces offline clusters too so operators can +// see and clean them up. The 1-hour heartbeat reaper still bounds the +// table eventually. type Cluster struct { ID string Address string + Type ClusterType + Online bool ConnectedProxies int - SelfHosted bool + // Capability flags. *bool because nil means "no proxy reported a + // capability for this cluster" — the dashboard renders these as + // unknown rather than false. + SupportsCustomPorts *bool + RequireSubdomain *bool + SupportsCrowdSec *bool } diff --git a/management/internals/modules/reverseproxy/service/interface.go b/management/internals/modules/reverseproxy/service/interface.go index 6a94aa32b..dddf6ae8a 100644 --- a/management/internals/modules/reverseproxy/service/interface.go +++ b/management/internals/modules/reverseproxy/service/interface.go @@ -9,7 +9,7 @@ import ( ) type Manager interface { - GetActiveClusters(ctx context.Context, accountID, userID string) ([]proxy.Cluster, error) + GetClusters(ctx context.Context, accountID, userID string) ([]proxy.Cluster, error) DeleteAccountCluster(ctx context.Context, accountID, userID, clusterAddress string) error GetAllServices(ctx context.Context, accountID, userID string) ([]*Service, error) GetService(ctx context.Context, accountID, userID, serviceID string) (*Service, error) diff --git a/management/internals/modules/reverseproxy/service/interface_mock.go b/management/internals/modules/reverseproxy/service/interface_mock.go index 83b2162ed..24963fe30 100644 --- a/management/internals/modules/reverseproxy/service/interface_mock.go +++ b/management/internals/modules/reverseproxy/service/interface_mock.go @@ -65,20 +65,6 @@ func (mr *MockManagerMockRecorder) CreateServiceFromPeer(ctx, accountID, peerID, return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateServiceFromPeer", reflect.TypeOf((*MockManager)(nil).CreateServiceFromPeer), ctx, accountID, peerID, req) } -// DeleteAllServices mocks base method. -func (m *MockManager) DeleteAllServices(ctx context.Context, accountID, userID string) error { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "DeleteAllServices", ctx, accountID, userID) - ret0, _ := ret[0].(error) - return ret0 -} - -// DeleteAllServices indicates an expected call of DeleteAllServices. -func (mr *MockManagerMockRecorder) DeleteAllServices(ctx, accountID, userID interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteAllServices", reflect.TypeOf((*MockManager)(nil).DeleteAllServices), ctx, accountID, userID) -} - // DeleteAccountCluster mocks base method. func (m *MockManager) DeleteAccountCluster(ctx context.Context, accountID, userID, clusterAddress string) error { m.ctrl.T.Helper() @@ -93,6 +79,20 @@ func (mr *MockManagerMockRecorder) DeleteAccountCluster(ctx, accountID, userID, return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteAccountCluster", reflect.TypeOf((*MockManager)(nil).DeleteAccountCluster), ctx, accountID, userID, clusterAddress) } +// DeleteAllServices mocks base method. +func (m *MockManager) DeleteAllServices(ctx context.Context, accountID, userID string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DeleteAllServices", ctx, accountID, userID) + ret0, _ := ret[0].(error) + return ret0 +} + +// DeleteAllServices indicates an expected call of DeleteAllServices. +func (mr *MockManagerMockRecorder) DeleteAllServices(ctx, accountID, userID interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteAllServices", reflect.TypeOf((*MockManager)(nil).DeleteAllServices), ctx, accountID, userID) +} + // DeleteService mocks base method. func (m *MockManager) DeleteService(ctx context.Context, accountID, userID, serviceID string) error { m.ctrl.T.Helper() @@ -122,21 +122,6 @@ func (mr *MockManagerMockRecorder) GetAccountServices(ctx, accountID interface{} return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAccountServices", reflect.TypeOf((*MockManager)(nil).GetAccountServices), ctx, accountID) } -// GetActiveClusters mocks base method. -func (m *MockManager) GetActiveClusters(ctx context.Context, accountID, userID string) ([]proxy.Cluster, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetActiveClusters", ctx, accountID, userID) - ret0, _ := ret[0].([]proxy.Cluster) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// GetActiveClusters indicates an expected call of GetActiveClusters. -func (mr *MockManagerMockRecorder) GetActiveClusters(ctx, accountID, userID interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetActiveClusters", reflect.TypeOf((*MockManager)(nil).GetActiveClusters), ctx, accountID, userID) -} - // GetAllServices mocks base method. func (m *MockManager) GetAllServices(ctx context.Context, accountID, userID string) ([]*Service, error) { m.ctrl.T.Helper() @@ -152,19 +137,19 @@ func (mr *MockManagerMockRecorder) GetAllServices(ctx, accountID, userID interfa return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAllServices", reflect.TypeOf((*MockManager)(nil).GetAllServices), ctx, accountID, userID) } -// GetServiceByDomain mocks base method. -func (m *MockManager) GetServiceByDomain(ctx context.Context, domain string) (*Service, error) { +// GetClusters mocks base method. +func (m *MockManager) GetClusters(ctx context.Context, accountID, userID string) ([]proxy.Cluster, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetServiceByDomain", ctx, domain) - ret0, _ := ret[0].(*Service) + ret := m.ctrl.Call(m, "GetClusters", ctx, accountID, userID) + ret0, _ := ret[0].([]proxy.Cluster) ret1, _ := ret[1].(error) return ret0, ret1 } -// GetServiceByDomain indicates an expected call of GetServiceByDomain. -func (mr *MockManagerMockRecorder) GetServiceByDomain(ctx, domain interface{}) *gomock.Call { +// GetClusters indicates an expected call of GetClusters. +func (mr *MockManagerMockRecorder) GetClusters(ctx, accountID, userID interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetServiceByDomain", reflect.TypeOf((*MockManager)(nil).GetServiceByDomain), ctx, domain) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetClusters", reflect.TypeOf((*MockManager)(nil).GetClusters), ctx, accountID, userID) } // GetGlobalServices mocks base method. @@ -197,6 +182,21 @@ func (mr *MockManagerMockRecorder) GetService(ctx, accountID, userID, serviceID return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetService", reflect.TypeOf((*MockManager)(nil).GetService), ctx, accountID, userID, serviceID) } +// GetServiceByDomain mocks base method. +func (m *MockManager) GetServiceByDomain(ctx context.Context, domain string) (*Service, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetServiceByDomain", ctx, domain) + ret0, _ := ret[0].(*Service) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetServiceByDomain indicates an expected call of GetServiceByDomain. +func (mr *MockManagerMockRecorder) GetServiceByDomain(ctx, domain interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetServiceByDomain", reflect.TypeOf((*MockManager)(nil).GetServiceByDomain), ctx, domain) +} + // GetServiceByID mocks base method. func (m *MockManager) GetServiceByID(ctx context.Context, accountID, serviceID string) (*Service, error) { m.ctrl.T.Helper() diff --git a/management/internals/modules/reverseproxy/service/manager/api.go b/management/internals/modules/reverseproxy/service/manager/api.go index 08272077c..9d93d52ee 100644 --- a/management/internals/modules/reverseproxy/service/manager/api.go +++ b/management/internals/modules/reverseproxy/service/manager/api.go @@ -187,7 +187,7 @@ func (h *handler) getClusters(w http.ResponseWriter, r *http.Request) { return } - clusters, err := h.manager.GetActiveClusters(r.Context(), userAuth.AccountId, userAuth.UserId) + clusters, err := h.manager.GetClusters(r.Context(), userAuth.AccountId, userAuth.UserId) if err != nil { util.WriteError(r.Context(), err, w) return @@ -196,10 +196,14 @@ func (h *handler) getClusters(w http.ResponseWriter, r *http.Request) { apiClusters := make([]api.ProxyCluster, 0, len(clusters)) for _, c := range clusters { apiClusters = append(apiClusters, api.ProxyCluster{ - Id: c.ID, - Address: c.Address, - ConnectedProxies: c.ConnectedProxies, - SelfHosted: c.SelfHosted, + Id: c.ID, + Address: c.Address, + Type: api.ProxyClusterType(c.Type), + Online: c.Online, + ConnectedProxies: c.ConnectedProxies, + SupportsCustomPorts: c.SupportsCustomPorts, + RequireSubdomain: c.RequireSubdomain, + SupportsCrowdsec: c.SupportsCrowdSec, }) } diff --git a/management/internals/modules/reverseproxy/service/manager/manager.go b/management/internals/modules/reverseproxy/service/manager/manager.go index 4a8598afb..ca0c5540f 100644 --- a/management/internals/modules/reverseproxy/service/manager/manager.go +++ b/management/internals/modules/reverseproxy/service/manager/manager.go @@ -81,6 +81,7 @@ type ClusterDeriver interface { type CapabilityProvider interface { ClusterSupportsCustomPorts(ctx context.Context, clusterAddr string) *bool ClusterRequireSubdomain(ctx context.Context, clusterAddr string) *bool + ClusterSupportsCrowdSec(ctx context.Context, clusterAddr string) *bool } type Manager struct { @@ -112,8 +113,12 @@ func (m *Manager) StartExposeReaper(ctx context.Context) { m.exposeReaper.StartExposeReaper(ctx) } -// GetActiveClusters returns all active proxy clusters with their connected proxy count. -func (m *Manager) GetActiveClusters(ctx context.Context, accountID, userID string) ([]proxy.Cluster, error) { +// GetClusters returns every proxy cluster visible to the account +// (shared + its own BYOP), regardless of whether any proxy in the +// cluster is currently heartbeating. Each cluster is enriched with the +// capability flags reported by its active proxies so the dashboard can +// render feature support without a second round-trip. +func (m *Manager) GetClusters(ctx context.Context, accountID, userID string) ([]proxy.Cluster, error) { ok, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Services, operations.Read) if err != nil { return nil, status.NewPermissionValidationError(err) @@ -122,7 +127,18 @@ func (m *Manager) GetActiveClusters(ctx context.Context, accountID, userID strin return nil, status.NewPermissionDeniedError() } - return m.store.GetActiveProxyClusters(ctx, accountID) + clusters, err := m.store.GetProxyClusters(ctx, accountID) + if err != nil { + return nil, err + } + + for i := range clusters { + clusters[i].SupportsCustomPorts = m.capabilities.ClusterSupportsCustomPorts(ctx, clusters[i].Address) + clusters[i].RequireSubdomain = m.capabilities.ClusterRequireSubdomain(ctx, clusters[i].Address) + clusters[i].SupportsCrowdSec = m.capabilities.ClusterSupportsCrowdSec(ctx, clusters[i].Address) + } + + return clusters, nil } // DeleteAccountCluster removes all proxy registrations for the given cluster address diff --git a/management/internals/shared/grpc/proxy_group_access_test.go b/management/internals/shared/grpc/proxy_group_access_test.go index 46dad5b56..5980f8a30 100644 --- a/management/internals/shared/grpc/proxy_group_access_test.go +++ b/management/internals/shared/grpc/proxy_group_access_test.go @@ -109,7 +109,7 @@ func (m *mockReverseProxyManager) GetServiceByDomain(_ context.Context, domain s return nil, errors.New("service not found for domain: " + domain) } -func (m *mockReverseProxyManager) GetActiveClusters(_ context.Context, _, _ string) ([]proxy.Cluster, error) { +func (m *mockReverseProxyManager) GetClusters(_ context.Context, _, _ string) ([]proxy.Cluster, error) { return nil, nil } diff --git a/management/internals/shared/grpc/validate_session_test.go b/management/internals/shared/grpc/validate_session_test.go index 7b7ffcfb2..774c5d1d3 100644 --- a/management/internals/shared/grpc/validate_session_test.go +++ b/management/internals/shared/grpc/validate_session_test.go @@ -322,7 +322,7 @@ func (m *testValidateSessionServiceManager) GetServiceByDomain(ctx context.Conte return m.store.GetServiceByDomain(ctx, domain) } -func (m *testValidateSessionServiceManager) GetActiveClusters(_ context.Context, _, _ string) ([]proxy.Cluster, error) { +func (m *testValidateSessionServiceManager) GetClusters(_ context.Context, _, _ string) ([]proxy.Cluster, error) { return nil, nil } diff --git a/management/server/http/handlers/proxy/auth_callback_integration_test.go b/management/server/http/handlers/proxy/auth_callback_integration_test.go index 30d8aa0e7..f08d5daf1 100644 --- a/management/server/http/handlers/proxy/auth_callback_integration_test.go +++ b/management/server/http/handlers/proxy/auth_callback_integration_test.go @@ -444,7 +444,7 @@ func (m *testServiceManager) GetServiceByDomain(ctx context.Context, domain stri return m.store.GetServiceByDomain(ctx, domain) } -func (m *testServiceManager) GetActiveClusters(_ context.Context, _, _ string) ([]nbproxy.Cluster, error) { +func (m *testServiceManager) GetClusters(_ context.Context, _, _ string) ([]nbproxy.Cluster, error) { return nil, nil } diff --git a/management/server/store/sql_store.go b/management/server/store/sql_store.go index 8cf37de56..f3c6b741b 100644 --- a/management/server/store/sql_store.go +++ b/management/server/store/sql_store.go @@ -5736,19 +5736,67 @@ func (s *SqlStore) DeleteAccountCluster(ctx context.Context, clusterAddress, acc return nil } -func (s *SqlStore) GetActiveProxyClusters(ctx context.Context, accountID string) ([]proxy.Cluster, error) { - var clusters []proxy.Cluster +// GetProxyClusters returns every cluster the account can see (shared +// plus its own BYOP), regardless of whether any proxy in the cluster +// is currently heartbeating. Online and ConnectedProxies are derived +// from the 2-min active window so the dashboard can render offline +// clusters distinctly; the 1-hour heartbeat reaper still removes rows +// that go quiet for too long. +// +// AccountOwned is determined by whether any proxy row in the group +// carries a non-NULL account_id; the caller maps that to Cluster.Type. +// Capability flags are NOT filled here — the handler enriches them via +// the per-cluster capability lookups. +func (s *SqlStore) GetProxyClusters(ctx context.Context, accountID string) ([]proxy.Cluster, error) { + activeCutoff := time.Now().Add(-proxyActiveThreshold) + type clusterRow struct { + ID string + Address string + ConnectedProxies int + Online bool + AccountOwned bool + } + + var rows []clusterRow result := s.db.Model(&proxy.Proxy{}). - Select("MIN(id) as id, cluster_address as address, COUNT(*) as connected_proxies, COUNT(account_id) > 0 as self_hosted"). - Where("status = ? AND last_seen > ? AND (account_id IS NULL OR account_id = ?)", - proxy.StatusConnected, time.Now().Add(-proxyActiveThreshold), accountID). + Select( + "MIN(id) AS id, "+ + "cluster_address AS address, "+ + // COUNT(CASE WHEN ... THEN 1 END) counts only non-NULL — i.e. only + // rows that satisfy the predicate — so it works portably across + // sqlite/postgres/mysql without dialect-specific FILTER syntax. + "COUNT(CASE WHEN status = ? AND last_seen > ? THEN 1 END) AS connected_proxies, "+ + // MAX(CASE …) > 0 expresses BOOL_OR in a way Postgres tolerates + // (Postgres can't MAX a boolean column). + "MAX(CASE WHEN status = ? AND last_seen > ? THEN 1 ELSE 0 END) > 0 AS online, "+ + "MAX(CASE WHEN account_id IS NOT NULL THEN 1 ELSE 0 END) > 0 AS account_owned", + proxy.StatusConnected, activeCutoff, + proxy.StatusConnected, activeCutoff, + ). + Where("account_id IS NULL OR account_id = ?", accountID). Group("cluster_address"). - Scan(&clusters) + Scan(&rows) if result.Error != nil { - log.WithContext(ctx).Errorf("failed to get active proxy clusters: %v", result.Error) - return nil, status.Errorf(status.Internal, "get active proxy clusters") + log.WithContext(ctx).Errorf("failed to get proxy clusters: %v", result.Error) + return nil, status.Errorf(status.Internal, "get proxy clusters") + } + + clusters := make([]proxy.Cluster, 0, len(rows)) + for _, r := range rows { + c := proxy.Cluster{ + ID: r.ID, + Address: r.Address, + Online: r.Online, + ConnectedProxies: r.ConnectedProxies, + } + if r.AccountOwned { + c.Type = proxy.ClusterTypeAccount + } else { + c.Type = proxy.ClusterTypeShared + } + clusters = append(clusters, c) } return clusters, nil diff --git a/management/server/store/sql_store_proxy_clusters_test.go b/management/server/store/sql_store_proxy_clusters_test.go new file mode 100644 index 000000000..cdacfedae --- /dev/null +++ b/management/server/store/sql_store_proxy_clusters_test.go @@ -0,0 +1,109 @@ +package store + +import ( + "context" + "os" + "runtime" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + rpproxy "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/proxy" +) + +// TestSqlStore_GetProxyClusters_DerivesOnlineAndType guards the +// account-visible cluster list against silent regressions in two +// dimensions: +// +// 1. Online derivation: a cluster with one stale and one fresh proxy +// is online and counts only the fresh proxy; a cluster whose +// proxies all heartbeated outside the 2-min window appears offline +// with connected_proxies = 0 (rather than disappearing, which is +// what the old query did). +// 2. Type derivation: a cluster scoped to the calling account is +// reported as `account`; a cluster with account_id IS NULL is +// reported as `shared`. Clusters scoped to other accounts must not +// leak into the result. +// +// Capability flags are intentionally not asserted here — they're filled +// by the manager (handler) layer from the per-cluster capability +// lookups, not by the store query. +func TestSqlStore_GetProxyClusters_DerivesOnlineAndType(t *testing.T) { + if (os.Getenv("CI") == "true" && runtime.GOOS == "darwin") || runtime.GOOS == "windows" { + t.Skip("skip CI tests on darwin and windows") + } + + runTestForAllEngines(t, "", func(t *testing.T, store Store) { + ctx := context.Background() + accountID := "acct-clusters" + require.NoError(t, store.SaveAccount(ctx, newAccountWithId(ctx, accountID, "user-1", ""))) + + otherAccountID := "acct-other" + require.NoError(t, store.SaveAccount(ctx, newAccountWithId(ctx, otherAccountID, "user-2", ""))) + + acctID := accountID + otherID := otherAccountID + + fresh := time.Now().Add(-30 * time.Second) + stale := time.Now().Add(-30 * time.Minute) + + mustSave := func(id, cluster string, accID *string, status string, lastSeen time.Time) { + require.NoError(t, store.SaveProxy(ctx, &rpproxy.Proxy{ + ID: id, + SessionID: id + "-sess", + ClusterAddress: cluster, + IPAddress: "10.0.0.1", + AccountID: accID, + LastSeen: lastSeen, + Status: status, + })) + } + + // shared-mixed: one fresh + one stale proxy → online, connected=1 + mustSave("p-shared-fresh", "shared-mixed.netbird.io", nil, rpproxy.StatusConnected, fresh) + mustSave("p-shared-stale", "shared-mixed.netbird.io", nil, rpproxy.StatusConnected, stale) + + // shared-offline: only stale proxies → offline, connected=0, + // but row must still appear (this is the new semantic — old + // query would have dropped it entirely). + mustSave("p-shared-off", "shared-offline.netbird.io", nil, rpproxy.StatusConnected, stale) + + // account-online: BYOP cluster owned by acctID, fresh + mustSave("p-acct-fresh", "byop.acct.example", &acctID, rpproxy.StatusConnected, fresh) + + // other-account: must not surface for acctID + mustSave("p-other", "byop.other.example", &otherID, rpproxy.StatusConnected, fresh) + + clusters, err := store.GetProxyClusters(ctx, accountID) + require.NoError(t, err) + + byAddr := map[string]rpproxy.Cluster{} + for _, c := range clusters { + byAddr[c.Address] = c + } + + assert.NotContains(t, byAddr, "byop.other.example", + "another account's BYOP cluster must not leak into this account's listing") + + require.Contains(t, byAddr, "shared-mixed.netbird.io") + mixed := byAddr["shared-mixed.netbird.io"] + assert.Equal(t, rpproxy.ClusterTypeShared, mixed.Type, "shared cluster (account_id IS NULL) must be reported as Type=shared") + assert.True(t, mixed.Online, "cluster with a fresh proxy must be online") + assert.Equal(t, 1, mixed.ConnectedProxies, "connected_proxies must count only fresh proxies; the stale one should not bump the count") + + require.Contains(t, byAddr, "shared-offline.netbird.io", + "offline clusters must still appear so the dashboard can render them — the old GetActiveProxyClusters would have dropped this row, which is the regression this test guards against") + offline := byAddr["shared-offline.netbird.io"] + assert.Equal(t, rpproxy.ClusterTypeShared, offline.Type) + assert.False(t, offline.Online, "no fresh heartbeat → offline") + assert.Equal(t, 0, offline.ConnectedProxies, "no fresh proxies → connected_proxies=0") + + require.Contains(t, byAddr, "byop.acct.example") + acct := byAddr["byop.acct.example"] + assert.Equal(t, rpproxy.ClusterTypeAccount, acct.Type, "BYOP cluster owned by the account must be reported as Type=account") + assert.True(t, acct.Online) + assert.Equal(t, 1, acct.ConnectedProxies) + }) +} diff --git a/management/server/store/store.go b/management/server/store/store.go index 045f1576a..42cdcf36d 100644 --- a/management/server/store/store.go +++ b/management/server/store/store.go @@ -307,7 +307,7 @@ type Store interface { UpdateProxyHeartbeat(ctx context.Context, p *proxy.Proxy) error GetActiveProxyClusterAddresses(ctx context.Context) ([]string, error) GetActiveProxyClusterAddressesForAccount(ctx context.Context, accountID string) ([]string, error) - GetActiveProxyClusters(ctx context.Context, accountID string) ([]proxy.Cluster, error) + GetProxyClusters(ctx context.Context, accountID string) ([]proxy.Cluster, error) GetClusterSupportsCustomPorts(ctx context.Context, clusterAddr string) *bool GetClusterRequireSubdomain(ctx context.Context, clusterAddr string) *bool GetClusterSupportsCrowdSec(ctx context.Context, clusterAddr string) *bool diff --git a/management/server/store/store_mock.go b/management/server/store/store_mock.go index d51629606..4f9d875d2 100644 --- a/management/server/store/store_mock.go +++ b/management/server/store/store_mock.go @@ -380,6 +380,20 @@ func (mr *MockStoreMockRecorder) DeleteAccount(ctx, account interface{}) *gomock return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteAccount", reflect.TypeOf((*MockStore)(nil).DeleteAccount), ctx, account) } +// DeleteAccountCluster mocks base method. +func (m *MockStore) DeleteAccountCluster(ctx context.Context, clusterAddress, accountID string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DeleteAccountCluster", ctx, clusterAddress, accountID) + ret0, _ := ret[0].(error) + return ret0 +} + +// DeleteAccountCluster indicates an expected call of DeleteAccountCluster. +func (mr *MockStoreMockRecorder) DeleteAccountCluster(ctx, clusterAddress, accountID interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteAccountCluster", reflect.TypeOf((*MockStore)(nil).DeleteAccountCluster), ctx, clusterAddress, accountID) +} + // DeleteCustomDomain mocks base method. func (m *MockStore) DeleteCustomDomain(ctx context.Context, accountID, domainID string) error { m.ctrl.T.Helper() @@ -577,20 +591,6 @@ func (mr *MockStoreMockRecorder) DeletePostureChecks(ctx, accountID, postureChec return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeletePostureChecks", reflect.TypeOf((*MockStore)(nil).DeletePostureChecks), ctx, accountID, postureChecksID) } -// DeleteAccountCluster mocks base method. -func (m *MockStore) DeleteAccountCluster(ctx context.Context, clusterAddress, accountID string) error { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "DeleteAccountCluster", ctx, clusterAddress, accountID) - ret0, _ := ret[0].(error) - return ret0 -} - -// DeleteAccountCluster indicates an expected call of DeleteAccountCluster. -func (mr *MockStoreMockRecorder) DeleteAccountCluster(ctx, clusterAddress, accountID interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteAccountCluster", reflect.TypeOf((*MockStore)(nil).DeleteAccountCluster), ctx, clusterAddress, accountID) -} - // DeleteRoute mocks base method. func (m *MockStore) DeleteRoute(ctx context.Context, accountID, routeID string) error { m.ctrl.T.Helper() @@ -731,6 +731,20 @@ func (mr *MockStoreMockRecorder) DeleteZoneDNSRecords(ctx, accountID, zoneID int return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteZoneDNSRecords", reflect.TypeOf((*MockStore)(nil).DeleteZoneDNSRecords), ctx, accountID, zoneID) } +// DisconnectProxy mocks base method. +func (m *MockStore) DisconnectProxy(ctx context.Context, proxyID, sessionID string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DisconnectProxy", ctx, proxyID, sessionID) + ret0, _ := ret[0].(error) + return ret0 +} + +// DisconnectProxy indicates an expected call of DisconnectProxy. +func (mr *MockStoreMockRecorder) DisconnectProxy(ctx, proxyID, sessionID interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DisconnectProxy", reflect.TypeOf((*MockStore)(nil).DisconnectProxy), ctx, proxyID, sessionID) +} + // EphemeralServiceExists mocks base method. func (m *MockStore) EphemeralServiceExists(ctx context.Context, lockStrength LockingStrength, accountID, peerID, domain string) (bool, error) { m.ctrl.T.Helper() @@ -1332,21 +1346,6 @@ func (mr *MockStoreMockRecorder) GetActiveProxyClusterAddressesForAccount(ctx, a return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetActiveProxyClusterAddressesForAccount", reflect.TypeOf((*MockStore)(nil).GetActiveProxyClusterAddressesForAccount), ctx, accountID) } -// GetActiveProxyClusters mocks base method. -func (m *MockStore) GetActiveProxyClusters(ctx context.Context, accountID string) ([]proxy.Cluster, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetActiveProxyClusters", ctx, accountID) - ret0, _ := ret[0].([]proxy.Cluster) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// GetActiveProxyClusters indicates an expected call of GetActiveProxyClusters. -func (mr *MockStoreMockRecorder) GetActiveProxyClusters(ctx, accountID interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetActiveProxyClusters", reflect.TypeOf((*MockStore)(nil).GetActiveProxyClusters), ctx, accountID) -} - // GetAllAccounts mocks base method. func (m *MockStore) GetAllAccounts(ctx context.Context) []*types2.Account { m.ctrl.T.Helper() @@ -2048,6 +2047,21 @@ func (mr *MockStoreMockRecorder) GetProxyByAccountID(ctx, accountID interface{}) return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetProxyByAccountID", reflect.TypeOf((*MockStore)(nil).GetProxyByAccountID), ctx, accountID) } +// GetProxyClusters mocks base method. +func (m *MockStore) GetProxyClusters(ctx context.Context, accountID string) ([]proxy.Cluster, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetProxyClusters", ctx, accountID) + ret0, _ := ret[0].([]proxy.Cluster) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetProxyClusters indicates an expected call of GetProxyClusters. +func (mr *MockStoreMockRecorder) GetProxyClusters(ctx, accountID interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetProxyClusters", reflect.TypeOf((*MockStore)(nil).GetProxyClusters), ctx, accountID) +} + // GetResourceGroups mocks base method. func (m *MockStore) GetResourceGroups(ctx context.Context, lockStrength LockingStrength, accountID, resourceID string) ([]*types2.Group, error) { m.ctrl.T.Helper() @@ -2950,20 +2964,6 @@ func (mr *MockStoreMockRecorder) SaveProxy(ctx, proxy interface{}) *gomock.Call return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SaveProxy", reflect.TypeOf((*MockStore)(nil).SaveProxy), ctx, proxy) } -// DisconnectProxy mocks base method. -func (m *MockStore) DisconnectProxy(ctx context.Context, proxyID, sessionID string) error { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "DisconnectProxy", ctx, proxyID, sessionID) - ret0, _ := ret[0].(error) - return ret0 -} - -// DisconnectProxy indicates an expected call of DisconnectProxy. -func (mr *MockStoreMockRecorder) DisconnectProxy(ctx, proxyID, sessionID interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DisconnectProxy", reflect.TypeOf((*MockStore)(nil).DisconnectProxy), ctx, proxyID, sessionID) -} - // SaveProxyAccessToken mocks base method. func (m *MockStore) SaveProxyAccessToken(ctx context.Context, token *types2.ProxyAccessToken) error { m.ctrl.T.Helper() diff --git a/proxy/management_integration_test.go b/proxy/management_integration_test.go index 9fd3d2ce9..d7e891801 100644 --- a/proxy/management_integration_test.go +++ b/proxy/management_integration_test.go @@ -366,7 +366,7 @@ func (m *storeBackedServiceManager) GetServiceByDomain(ctx context.Context, doma return m.store.GetServiceByDomain(ctx, domain) } -func (m *storeBackedServiceManager) GetActiveClusters(_ context.Context, _, _ string) ([]nbproxy.Cluster, error) { +func (m *storeBackedServiceManager) GetClusters(_ context.Context, _, _ string) ([]nbproxy.Cluster, error) { return nil, nil } diff --git a/shared/management/http/api/openapi.yml b/shared/management/http/api/openapi.yml index 942f3aa45..353aff72d 100644 --- a/shared/management/http/api/openapi.yml +++ b/shared/management/http/api/openapi.yml @@ -3417,19 +3417,43 @@ components: type: string description: Cluster address used for CNAME targets example: "eu.proxy.netbird.io" + type: + $ref: '#/components/schemas/ProxyClusterType' + online: + type: boolean + description: Whether at least one proxy in the cluster has heartbeated within the active window + example: true connected_proxies: type: integer - description: Number of proxy nodes connected in this cluster + description: Number of proxy nodes currently connected (heartbeat within the active window) example: 3 - self_hosted: + supports_custom_ports: type: boolean - description: Whether this cluster is a self-hosted (BYOP) proxy managed by the account owner + description: Whether the cluster supports binding arbitrary TCP/UDP ports + example: true + require_subdomain: + type: boolean + description: Whether services on this cluster must include a subdomain label + example: false + supports_crowdsec: + type: boolean + description: Whether all active proxies in the cluster have CrowdSec configured example: false required: - id - address + - type + - online - connected_proxies - - self_hosted + ProxyClusterType: + type: string + description: | + Source of the proxy cluster. `account` clusters are owned and operated by the account (BYOP); + `shared` clusters are operated by NetBird and shared across accounts. + enum: + - account + - shared + example: shared ReverseProxyDomainType: type: string description: Type of Reverse Proxy Domain diff --git a/shared/management/http/api/types.gen.go b/shared/management/http/api/types.gen.go index b3bb475a9..16e765f8c 100644 --- a/shared/management/http/api/types.gen.go +++ b/shared/management/http/api/types.gen.go @@ -1,6 +1,6 @@ // Package api provides primitives to interact with the openapi HTTP API. // -// Code generated by github.com/oapi-codegen/oapi-codegen/v2 version v2.6.0 DO NOT EDIT. +// Code generated by github.com/oapi-codegen/oapi-codegen/v2 version v2.7.0 DO NOT EDIT. package api import ( @@ -13,8 +13,8 @@ import ( ) const ( - BearerAuthScopes = "BearerAuth.Scopes" - TokenAuthScopes = "TokenAuth.Scopes" + BearerAuthScopes bearerAuthContextKey = "BearerAuth.Scopes" + TokenAuthScopes tokenAuthContextKey = "TokenAuth.Scopes" ) // Defines values for AccessRestrictionsCrowdsecMode. @@ -511,6 +511,7 @@ func (e GroupMinimumIssued) Valid() bool { // Defines values for IdentityProviderType. const ( + IdentityProviderTypeAdfs IdentityProviderType = "adfs" IdentityProviderTypeEntra IdentityProviderType = "entra" IdentityProviderTypeGoogle IdentityProviderType = "google" IdentityProviderTypeMicrosoft IdentityProviderType = "microsoft" @@ -518,12 +519,13 @@ const ( IdentityProviderTypeOkta IdentityProviderType = "okta" IdentityProviderTypePocketid IdentityProviderType = "pocketid" IdentityProviderTypeZitadel IdentityProviderType = "zitadel" - IdentityProviderTypeAdfs IdentityProviderType = "adfs" ) // Valid indicates whether the value is a known member of the IdentityProviderType enum. func (e IdentityProviderType) Valid() bool { switch e { + case IdentityProviderTypeAdfs: + return true case IdentityProviderTypeEntra: return true case IdentityProviderTypeGoogle: @@ -538,8 +540,6 @@ func (e IdentityProviderType) Valid() bool { return true case IdentityProviderTypeZitadel: return true - case IdentityProviderTypeAdfs: - return true default: return false } @@ -878,6 +878,24 @@ func (e PolicyRuleUpdateProtocol) Valid() bool { } } +// Defines values for ProxyClusterType. +const ( + ProxyClusterTypeAccount ProxyClusterType = "account" + ProxyClusterTypeShared ProxyClusterType = "shared" +) + +// Valid indicates whether the value is a known member of the ProxyClusterType enum. +func (e ProxyClusterType) Valid() bool { + switch e { + case ProxyClusterTypeAccount: + return true + case ProxyClusterTypeShared: + return true + default: + return false + } +} + // Defines values for ResourceType. const ( ResourceTypeDomain ResourceType = "domain" @@ -1638,7 +1656,9 @@ type Checks struct { // OsVersionCheck Posture check for the version of operating system OsVersionCheck *OSVersionCheck `json:"os_version_check,omitempty"` - // PeerNetworkRangeCheck Posture check for allow or deny access based on the peer's IP addresses. A range matches when it contains any of the peer's local network interface IPs or its public connection (NAT egress) IP, so ranges may target private subnets, public CIDRs, or single hosts via a /32 or /128. + // PeerNetworkRangeCheck Posture check for allow or deny access based on the peer's IP addresses. A range matches when it + // contains any of the peer's local network interface IPs or its public connection (NAT egress) IP, + // so ranges may target private subnets, public CIDRs, or single hosts via a /32 or /128. PeerNetworkRangeCheck *PeerNetworkRangeCheck `json:"peer_network_range_check,omitempty"` // ProcessCheck Posture Check for binaries exist and are running in the peer’s system @@ -3330,7 +3350,9 @@ type PeerMinimum struct { Name string `json:"name"` } -// PeerNetworkRangeCheck Posture check for allow or deny access based on the peer's IP addresses. A range matches when it contains any of the peer's local network interface IPs or its public connection (NAT egress) IP, so ranges may target private subnets, public CIDRs, or single hosts via a /32 or /128. +// PeerNetworkRangeCheck Posture check for allow or deny access based on the peer's IP addresses. A range matches when it +// contains any of the peer's local network interface IPs or its public connection (NAT egress) IP, +// so ranges may target private subnets, public CIDRs, or single hosts via a /32 or /128. type PeerNetworkRangeCheck struct { // Action Action to take upon policy match Action PeerNetworkRangeCheckAction `json:"action"` @@ -3785,19 +3807,36 @@ type ProxyAccessLogsResponse struct { // ProxyCluster A proxy cluster represents a group of proxy nodes serving the same address type ProxyCluster struct { - // Id Unique identifier of a proxy in this cluster - Id string `json:"id"` - // Address Cluster address used for CNAME targets Address string `json:"address"` - // ConnectedProxies Number of proxy nodes connected in this cluster + // ConnectedProxies Number of proxy nodes currently connected (heartbeat within the active window) ConnectedProxies int `json:"connected_proxies"` - // SelfHosted Whether this cluster is a self-hosted (BYOP) proxy managed by the account owner - SelfHosted bool `json:"self_hosted"` + // Id Unique identifier of a proxy in this cluster + Id string `json:"id"` + + // Online Whether at least one proxy in the cluster has heartbeated within the active window + Online bool `json:"online"` + + // RequireSubdomain Whether services on this cluster must include a subdomain label + RequireSubdomain *bool `json:"require_subdomain,omitempty"` + + // SupportsCrowdsec Whether all active proxies in the cluster have CrowdSec configured + SupportsCrowdsec *bool `json:"supports_crowdsec,omitempty"` + + // SupportsCustomPorts Whether the cluster supports binding arbitrary TCP/UDP ports + SupportsCustomPorts *bool `json:"supports_custom_ports,omitempty"` + + // Type Source of the proxy cluster. `account` clusters are owned and operated by the account (BYOP); + // `shared` clusters are operated by NetBird and shared across accounts. + Type ProxyClusterType `json:"type"` } +// ProxyClusterType Source of the proxy cluster. `account` clusters are owned and operated by the account (BYOP); +// `shared` clusters are operated by NetBird and shared across accounts. +type ProxyClusterType string + // ProxyToken defines model for ProxyToken. type ProxyToken struct { CreatedAt time.Time `json:"created_at"` @@ -4820,6 +4859,12 @@ type ZoneRequest struct { // Conflict Standard error response. Note: The exact structure of this error response is inferred from `util.WriteErrorResponse` and `util.WriteError` usage in the provided Go code, as a specific Go struct for errors was not provided. type Conflict = ErrorResponse +// bearerAuthContextKey is the context key for BearerAuth security scheme +type bearerAuthContextKey string + +// tokenAuthContextKey is the context key for TokenAuth security scheme +type tokenAuthContextKey string + // GetApiEventsNetworkTrafficParams defines parameters for GetApiEventsNetworkTraffic. type GetApiEventsNetworkTrafficParams struct { // Page Page number