Skip to content

ROX-28151: limit the number of registries #16773

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
parametalol merged 2 commits into from
Sep 26, 2025
Merged

ROX-28151: limit the number of registries #16773

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
eaa4efa
max registries guard
parametalol Sep 11, 2025
b5f3477
fix rebase
parametalol Sep 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion central/metrics/custom/image_vulnerabilities/tracker_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,10 @@ func TestQueryDeploymentsAndImages(t *testing.T) {
req := httptest.NewRequestWithContext(makeAdminContext(t),
"GET", "/metrics", nil)

metrics.GetCustomRegistry("Admin").ServeHTTP(rec, req)
r, err := metrics.GetCustomRegistry("Admin")
if assert.NoError(t, err) {
r.ServeHTTP(rec, req)
}

result := rec.Result()
assert.Equal(t, 200, result.StatusCode)
Expand Down
7 changes: 6 additions & 1 deletion central/metrics/custom/runner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"github.com/stackrox/rox/central/telemetry/centralclient"
"github.com/stackrox/rox/generated/storage"
"github.com/stackrox/rox/pkg/grpc/authn"
"github.com/stackrox/rox/pkg/httputil"
"github.com/stackrox/rox/pkg/logging"
"github.com/stackrox/rox/pkg/sac"
"github.com/stackrox/rox/pkg/telemetry/phonehome/telemeter"
Expand Down Expand Up @@ -156,7 +157,11 @@ func (tr trackerRunner) ServeHTTP(w http.ResponseWriter, req *http.Request) {
go tracker.Gather(newCtx)
}
}
registry := metrics.GetCustomRegistry(userID)
registry, err := metrics.GetCustomRegistry(userID)
if err != nil {
httputil.WriteError(w, err)
return
}
registry.Lock()
defer registry.Unlock()
registry.ServeHTTP(w, req)
Expand Down
8 changes: 6 additions & 2 deletions central/metrics/custom/tracker/tracker_base.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ type TrackerBase[F Finding] struct {
gatherers sync.Map // map[user ID]*gatherer
cleanupWG sync.WaitGroup // for sync in testing.

registryFactory func(userID string) metrics.CustomRegistry // for mocking in tests.
registryFactory func(userID string) (metrics.CustomRegistry, error) // for mocking in tests.
}

// makeGettersMap transforms a list of label names with their getters to a map.
Expand Down Expand Up @@ -316,7 +316,11 @@ func (tracker *TrackerBase[Finding]) getGatherer(userID string, cfg *Configurati
defer tracker.cleanupInactiveGatherers()
var gr *gatherer
if g, ok := tracker.gatherers.Load(userID); !ok {
r := tracker.registryFactory(userID)
r, err := tracker.registryFactory(userID)
if err != nil {
log.Errorw("failed to create custom registry for user", userID, logging.Err(err))
return nil
}
gr = &gatherer{
registry: r,
}
Expand Down
14 changes: 7 additions & 7 deletions central/metrics/custom/tracker/tracker_base_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ func TestTrackerBase_Reconfigure(t *testing.T) {
makeTestGatherFunc(testData))

rf := mocks.NewMockCustomRegistry(ctrl)
tracker.registryFactory = func(string) metrics.CustomRegistry { return rf }
tracker.registryFactory = func(string) (metrics.CustomRegistry, error) { return rf, nil }

var registered, unregistered []MetricName
rf.EXPECT().RegisterMetric(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).
Expand Down Expand Up @@ -181,7 +181,7 @@ func TestTrackerBase_Track(t *testing.T) {
tracker := MakeTrackerBase("test", "Test",
testLabelGetters,
makeTestGatherFunc(testData))
tracker.registryFactory = func(string) metrics.CustomRegistry { return rf }
tracker.registryFactory = func(string) (metrics.CustomRegistry, error) { return rf, nil }

result := make(map[string][]*aggregatedRecord)
rf.EXPECT().SetTotal(gomock.Any(), gomock.Any(), gomock.Any()).
Expand Down Expand Up @@ -252,7 +252,7 @@ func TestTrackerBase_error(t *testing.T) {
}
},
)
tracker.registryFactory = func(string) metrics.CustomRegistry { return rf }
tracker.registryFactory = func(string) (metrics.CustomRegistry, error) { return rf, nil }

tracker.config = &Configuration{
metrics: makeTestMetricDescriptors(t),
Expand All @@ -268,7 +268,7 @@ func TestTrackerBase_Gather(t *testing.T) {
testLabelGetters,
makeTestGatherFunc(testData),
)
tracker.registryFactory = func(string) metrics.CustomRegistry { return rf }
tracker.registryFactory = func(string) (metrics.CustomRegistry, error) { return rf, nil }

result := make(map[string][]*aggregatedRecord)
{ // Capture result with a mock registry.
Expand Down Expand Up @@ -308,7 +308,7 @@ func TestTrackerBase_getGatherer(t *testing.T) {
testLabelGetters,
makeTestGatherFunc(testData),
)
tracker.registryFactory = func(string) metrics.CustomRegistry { return rf }
tracker.registryFactory = func(string) (metrics.CustomRegistry, error) { return rf, nil }

md := makeTestMetricDescriptors(t)
tracker.Reconfigure(&Configuration{
Expand Down Expand Up @@ -349,7 +349,7 @@ func TestTrackerBase_cleanup(t *testing.T) {
testLabelGetters,
makeTestGatherFunc(testData),
)
tracker.registryFactory = func(string) metrics.CustomRegistry { return rf }
tracker.registryFactory = func(string) (metrics.CustomRegistry, error) { return rf, nil }

cfg := &Configuration{
metrics: makeTestMetricDescriptors(t),
Expand Down Expand Up @@ -405,7 +405,7 @@ func Test_makeProps(t *testing.T) {
tracker := MakeTrackerBase("test", "telemetry test",
testLabelGetters,
makeTestGatherFunc(testData))
tracker.registryFactory = func(string) metrics.CustomRegistry { return rf }
tracker.registryFactory = func(string) (metrics.CustomRegistry, error) { return rf, nil }

md := makeTestMetricDescriptors(t)
tracker.Reconfigure(&Configuration{
Expand Down
13 changes: 11 additions & 2 deletions central/metrics/custom_registry.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/promhttp"
"github.com/stackrox/rox/pkg/errox"
"github.com/stackrox/rox/pkg/metrics"
"github.com/stackrox/rox/pkg/sync"
)
Expand All @@ -21,6 +22,10 @@ Metrics are immutable. To change anything, the metric has to be deleted and
recreated with separate requests.
*/

// This is the maximum number of registries, created in memory to serve
// different user identities.
const maxCustomRegistries = 100

//go:generate mockgen-wrapper
type CustomRegistry interface {
prometheus.Gatherer
Expand All @@ -43,22 +48,26 @@ type customRegistry struct {
var (
userRegistries map[string]*customRegistry = make(map[string]*customRegistry)
registriesMux sync.Mutex
ErrTooMany = errox.ResourceExhausted.New("too many custom registries")
)

// GetCustomRegistry is a CustomRegistry factory that returns the existing or
// a new registry for the user.
func GetCustomRegistry(userID string) CustomRegistry {
func GetCustomRegistry(userID string) (CustomRegistry, error) {
registriesMux.Lock()
defer registriesMux.Unlock()
registry, ok := userRegistries[userID]
if !ok {
if len(userRegistries) >= maxCustomRegistries {
return nil, ErrTooMany
}
registry = &customRegistry{
Registry: prometheus.NewRegistry(),
}
registry.Handler = promhttp.HandlerFor(registry, promhttp.HandlerOpts{})
userRegistries[userID] = registry
}
return registry
return registry, nil
}

// DeleteCustomRegistry unregisters all metrics and deletes a registry for the
Expand Down
36 changes: 30 additions & 6 deletions central/metrics/custom_registry_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package metrics

import (
"strconv"
"testing"
"time"

Expand All @@ -24,8 +25,10 @@ func getMetricValue(_ *testing.T, registry CustomRegistry, metricName string) (f
}

func TestMakeCustomRegistry(t *testing.T) {
cr1 := GetCustomRegistry("user1")
cr2 := GetCustomRegistry("user2")
cr1, err := GetCustomRegistry("user1")
assert.NoError(t, err)
cr2, err := GetCustomRegistry("user2")
assert.NoError(t, err)
assert.NotSame(t, cr1, cr2)

assert.NoError(t, cr1.RegisterMetric("test", "test", 10*time.Minute, []string{"Test1", "Test2"}))
Expand All @@ -49,7 +52,8 @@ func TestMakeCustomRegistry(t *testing.T) {
}

func TestCustomRegistry_Reset(t *testing.T) {
cr := GetCustomRegistry("user1")
cr, err := GetCustomRegistry("user1")
assert.NoError(t, err)
assert.NoError(t, cr.RegisterMetric("test1", "test", 10*time.Minute, []string{"Test1", "Test2"}))
assert.NoError(t, cr.RegisterMetric("test2", "test", 10*time.Minute, []string{"Test1", "Test2"}))
cr.SetTotal("test1", map[string]string{"Test1": "value1", "Test2": "value2"}, 42)
Expand All @@ -67,8 +71,10 @@ func TestCustomRegistry_Reset(t *testing.T) {
}

func TestDeleteCustomRegistry(t *testing.T) {
cr1 := GetCustomRegistry("user1")
cr2 := GetCustomRegistry("user2")
cr1, err := GetCustomRegistry("user1")
assert.NoError(t, err)
cr2, err := GetCustomRegistry("user2")
assert.NoError(t, err)
_ = cr1.RegisterMetric("test", "test", time.Hour, []string{"Test1", "Test2"})
_ = cr2.RegisterMetric("test", "test", time.Hour, []string{"Test1", "Test2"})
cr1.SetTotal("test", map[string]string{"Test1": "value1", "Test2": "value2"}, 42)
Expand All @@ -80,7 +86,8 @@ func TestDeleteCustomRegistry(t *testing.T) {
assert.Error(t, err)
assert.Equal(t, float64(0), value)

cr1 = GetCustomRegistry("user1")
cr1, err = GetCustomRegistry("user1")
assert.NoError(t, err)
value, err = getMetricValue(t, cr1, "test")
assert.Error(t, err)
assert.Equal(t, float64(0), value)
Expand All @@ -95,3 +102,20 @@ func TestDeleteCustomRegistry(t *testing.T) {
DeleteCustomRegistry("user100")
})
}

func TestErrTooMany(t *testing.T) {
// Saturate.
for i := range maxCustomRegistries {
_, err := GetCustomRegistry("user" + strconv.Itoa(i))
assert.NoError(t, err)
}
_, err := GetCustomRegistry("user" + strconv.Itoa(maxCustomRegistries))
assert.ErrorIs(t, err, ErrTooMany)
DeleteCustomRegistry("user0")
_, err = GetCustomRegistry("user0")
assert.NoError(t, err)
// Cleanup.
for i := range maxCustomRegistries {
DeleteCustomRegistry("user" + strconv.Itoa(i))
}
}
Loading