From 832e16886984ece3191d41f8e7ba178514956bc4 Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Fri, 22 Nov 2024 16:16:52 +0100 Subject: [PATCH 01/38] refactor jwtValidator and geo db to interfaces + first component test for setup-keys --- management/cmd/management.go | 2 +- management/server/account.go | 8 +- management/server/event.go | 4 +- management/server/event_test.go | 8 +- management/server/geolocation/geolocation.go | 39 ++++- management/server/grpcserver.go | 4 +- .../server/http/geolocations_handler.go | 4 +- management/server/http/handler.go | 4 +- .../server/http/posture_checks_handler.go | 4 +- .../http/posture_checks_handler_test.go | 2 +- .../server/http/setupkeys_component_test.go | 159 ++++++++++++++++++ management/server/integrated_validator.go | 43 +++++ management/server/jwtclaims/jwtValidator.go | 26 ++- management/server/user_test.go | 49 +++--- 14 files changed, 300 insertions(+), 56 deletions(-) create mode 100644 management/server/http/setupkeys_component_test.go diff --git a/management/cmd/management.go b/management/cmd/management.go index 719d1a78c1a..0340e09661d 100644 --- a/management/cmd/management.go +++ b/management/cmd/management.go @@ -264,7 +264,7 @@ var ( KeysLocation: config.HttpConfig.AuthKeysLocation, } - httpAPIHandler, err := httpapi.APIHandler(ctx, accountManager, geo, *jwtValidator, appMetrics, httpAPIAuthCfg, integratedPeerValidator) + httpAPIHandler, err := httpapi.APIHandler(ctx, accountManager, geo, jwtValidator, appMetrics, httpAPIAuthCfg, integratedPeerValidator) if err != nil { return fmt.Errorf("failed creating HTTP API handler: %v", err) } diff --git a/management/server/account.go b/management/server/account.go index 0ab123655a6..37bc0529fe1 100644 --- a/management/server/account.go +++ b/management/server/account.go @@ -166,8 +166,8 @@ type DefaultAccountManager struct { cacheManager cache.CacheInterface[[]*idp.UserData] externalCacheManager ExternalCacheManager ctx context.Context - eventStore activity.Store - geo *geolocation.Geolocation + EventStore activity.Store + geo geolocation.Geolocation requestBuffer *AccountRequestBuffer @@ -1041,7 +1041,7 @@ func BuildManager( singleAccountModeDomain string, dnsDomain string, eventStore activity.Store, - geo *geolocation.Geolocation, + geo geolocation.Geolocation, userDeleteFromIDPEnabled bool, integratedPeerValidator integrated_validator.IntegratedValidator, metrics telemetry.AppMetrics, @@ -1055,7 +1055,7 @@ func BuildManager( cacheMux: sync.Mutex{}, cacheLoading: map[string]chan struct{}{}, dnsDomain: dnsDomain, - eventStore: eventStore, + EventStore: eventStore, peerLoginExpiry: NewDefaultScheduler(), peerInactivityExpiry: NewDefaultScheduler(), userDeleteFromIDPEnabled: userDeleteFromIDPEnabled, diff --git a/management/server/event.go b/management/server/event.go index 93b80922678..a85ff2ff201 100644 --- a/management/server/event.go +++ b/management/server/event.go @@ -30,7 +30,7 @@ func (am *DefaultAccountManager) GetEvents(ctx context.Context, accountID, userI return nil, status.Errorf(status.PermissionDenied, "only users with admin power can view events") } - events, err := am.eventStore.Get(ctx, accountID, 0, 10000, true) + events, err := am.EventStore.Get(ctx, accountID, 0, 10000, true) if err != nil { return nil, err } @@ -58,7 +58,7 @@ func (am *DefaultAccountManager) GetEvents(ctx context.Context, accountID, userI func (am *DefaultAccountManager) StoreEvent(ctx context.Context, initiatorID, targetID, accountID string, activityID activity.ActivityDescriber, meta map[string]any) { go func() { - _, err := am.eventStore.Save(ctx, &activity.Event{ + _, err := am.EventStore.Save(ctx, &activity.Event{ Timestamp: time.Now().UTC(), Activity: activityID, InitiatorID: initiatorID, diff --git a/management/server/event_test.go b/management/server/event_test.go index 8c56fd3f6ec..4cbec56dc75 100644 --- a/management/server/event_test.go +++ b/management/server/event_test.go @@ -14,7 +14,7 @@ func generateAndStoreEvents(t *testing.T, manager *DefaultAccountManager, typ ac accountID string, count int) { t.Helper() for i := 0; i < count; i++ { - _, err := manager.eventStore.Save(context.Background(), &activity.Event{ + _, err := manager.EventStore.Save(context.Background(), &activity.Event{ Timestamp: time.Now().UTC(), Activity: typ, InitiatorID: initiatorID, @@ -41,7 +41,7 @@ func TestDefaultAccountManager_GetEvents(t *testing.T) { return } assert.Len(t, events, 0) - _ = manager.eventStore.Close(context.Background()) //nolint + _ = manager.EventStore.Close(context.Background()) //nolint }) t.Run("get events", func(t *testing.T) { @@ -52,7 +52,7 @@ func TestDefaultAccountManager_GetEvents(t *testing.T) { } assert.Len(t, events, 10) - _ = manager.eventStore.Close(context.Background()) //nolint + _ = manager.EventStore.Close(context.Background()) //nolint }) t.Run("get events without duplicates", func(t *testing.T) { @@ -62,6 +62,6 @@ func TestDefaultAccountManager_GetEvents(t *testing.T) { return } assert.Len(t, events, 1) - _ = manager.eventStore.Close(context.Background()) //nolint + _ = manager.EventStore.Close(context.Background()) //nolint }) } diff --git a/management/server/geolocation/geolocation.go b/management/server/geolocation/geolocation.go index 553a3158187..f209cc715ff 100644 --- a/management/server/geolocation/geolocation.go +++ b/management/server/geolocation/geolocation.go @@ -14,7 +14,14 @@ import ( log "github.com/sirupsen/logrus" ) -type Geolocation struct { +type Geolocation interface { + Lookup(ip net.IP) (*Record, error) + GetAllCountries() ([]Country, error) + GetCitiesByCountry(countryISOCode string) ([]City, error) + Stop() error +} + +type GeolocationImpl struct { mmdbPath string mux sync.RWMutex db *maxminddb.Reader @@ -54,7 +61,7 @@ const ( geonamesdbPattern = "geonames_*.db" ) -func NewGeolocation(ctx context.Context, dataDir string, autoUpdate bool) (*Geolocation, error) { +func NewGeolocation(ctx context.Context, dataDir string, autoUpdate bool) (Geolocation, error) { mmdbGlobPattern := filepath.Join(dataDir, mmdbPattern) mmdbFile, err := getDatabaseFilename(ctx, geoLiteCityTarGZURL, mmdbGlobPattern, autoUpdate) if err != nil { @@ -86,7 +93,7 @@ func NewGeolocation(ctx context.Context, dataDir string, autoUpdate bool) (*Geol return nil, err } - geo := &Geolocation{ + geo := &GeolocationImpl{ mmdbPath: mmdbPath, mux: sync.RWMutex{}, db: db, @@ -113,7 +120,7 @@ func openDB(mmdbPath string) (*maxminddb.Reader, error) { return db, nil } -func (gl *Geolocation) Lookup(ip net.IP) (*Record, error) { +func (gl *GeolocationImpl) Lookup(ip net.IP) (*Record, error) { gl.mux.RLock() defer gl.mux.RUnlock() @@ -127,7 +134,7 @@ func (gl *Geolocation) Lookup(ip net.IP) (*Record, error) { } // GetAllCountries retrieves a list of all countries. -func (gl *Geolocation) GetAllCountries() ([]Country, error) { +func (gl *GeolocationImpl) GetAllCountries() ([]Country, error) { allCountries, err := gl.locationDB.GetAllCountries() if err != nil { return nil, err @@ -143,7 +150,7 @@ func (gl *Geolocation) GetAllCountries() ([]Country, error) { } // GetCitiesByCountry retrieves a list of cities in a specific country based on the country's ISO code. -func (gl *Geolocation) GetCitiesByCountry(countryISOCode string) ([]City, error) { +func (gl *GeolocationImpl) GetCitiesByCountry(countryISOCode string) ([]City, error) { allCities, err := gl.locationDB.GetCitiesByCountry(countryISOCode) if err != nil { return nil, err @@ -158,7 +165,7 @@ func (gl *Geolocation) GetCitiesByCountry(countryISOCode string) ([]City, error) return cities, nil } -func (gl *Geolocation) Stop() error { +func (gl *GeolocationImpl) Stop() error { close(gl.stopCh) if gl.db != nil { if err := gl.db.Close(); err != nil { @@ -259,3 +266,21 @@ func cleanupMaxMindDatabases(ctx context.Context, dataDir string, mmdbFile strin } return nil } + +type GeolocationMock struct{} + +func (g *GeolocationMock) Lookup(ip net.IP) (*Record, error) { + return &Record{}, nil +} + +func (g *GeolocationMock) GetAllCountries() ([]Country, error) { + return []Country{}, nil +} + +func (g *GeolocationMock) GetCitiesByCountry(countryISOCode string) ([]City, error) { + return []City{}, nil +} + +func (g *GeolocationMock) Stop() error { + return nil +} diff --git a/management/server/grpcserver.go b/management/server/grpcserver.go index 9c12336f80f..7ea5c635981 100644 --- a/management/server/grpcserver.go +++ b/management/server/grpcserver.go @@ -35,7 +35,7 @@ type GRPCServer struct { peersUpdateManager *PeersUpdateManager config *Config secretsManager SecretsManager - jwtValidator *jwtclaims.JWTValidator + jwtValidator jwtclaims.JWTValidator jwtClaimsExtractor *jwtclaims.ClaimsExtractor appMetrics telemetry.AppMetrics ephemeralManager *EphemeralManager @@ -57,7 +57,7 @@ func NewServer( return nil, err } - var jwtValidator *jwtclaims.JWTValidator + var jwtValidator jwtclaims.JWTValidator if config.HttpConfig != nil && config.HttpConfig.AuthIssuer != "" && config.HttpConfig.AuthAudience != "" && validateURL(config.HttpConfig.AuthKeysLocation) { jwtValidator, err = jwtclaims.NewJWTValidator( diff --git a/management/server/http/geolocations_handler.go b/management/server/http/geolocations_handler.go index 418228abfe6..2805770dfea 100644 --- a/management/server/http/geolocations_handler.go +++ b/management/server/http/geolocations_handler.go @@ -21,12 +21,12 @@ var ( // GeolocationsHandler is a handler that returns locations. type GeolocationsHandler struct { accountManager server.AccountManager - geolocationManager *geolocation.Geolocation + geolocationManager geolocation.Geolocation claimsExtractor *jwtclaims.ClaimsExtractor } // NewGeolocationsHandlerHandler creates a new Geolocations handler -func NewGeolocationsHandlerHandler(accountManager server.AccountManager, geolocationManager *geolocation.Geolocation, authCfg AuthCfg) *GeolocationsHandler { +func NewGeolocationsHandlerHandler(accountManager server.AccountManager, geolocationManager geolocation.Geolocation, authCfg AuthCfg) *GeolocationsHandler { return &GeolocationsHandler{ accountManager: accountManager, geolocationManager: geolocationManager, diff --git a/management/server/http/handler.go b/management/server/http/handler.go index c3928bff681..a4022c6b09b 100644 --- a/management/server/http/handler.go +++ b/management/server/http/handler.go @@ -31,7 +31,7 @@ type AuthCfg struct { type apiHandler struct { Router *mux.Router AccountManager s.AccountManager - geolocationManager *geolocation.Geolocation + geolocationManager geolocation.Geolocation AuthCfg AuthCfg } @@ -40,7 +40,7 @@ type emptyObject struct { } // APIHandler creates the Management service HTTP API handler registering all the available endpoints. -func APIHandler(ctx context.Context, accountManager s.AccountManager, LocationManager *geolocation.Geolocation, jwtValidator jwtclaims.JWTValidator, appMetrics telemetry.AppMetrics, authCfg AuthCfg, integratedValidator integrated_validator.IntegratedValidator) (http.Handler, error) { +func APIHandler(ctx context.Context, accountManager s.AccountManager, LocationManager geolocation.Geolocation, jwtValidator jwtclaims.JWTValidator, appMetrics telemetry.AppMetrics, authCfg AuthCfg, integratedValidator integrated_validator.IntegratedValidator) (http.Handler, error) { claimsExtractor := jwtclaims.NewClaimsExtractor( jwtclaims.WithAudience(authCfg.Audience), jwtclaims.WithUserIDClaim(authCfg.UserIDClaim), diff --git a/management/server/http/posture_checks_handler.go b/management/server/http/posture_checks_handler.go index 1d020e9bcb7..c3536755b8c 100644 --- a/management/server/http/posture_checks_handler.go +++ b/management/server/http/posture_checks_handler.go @@ -18,12 +18,12 @@ import ( // PostureChecksHandler is a handler that returns posture checks of the account. type PostureChecksHandler struct { accountManager server.AccountManager - geolocationManager *geolocation.Geolocation + geolocationManager geolocation.Geolocation claimsExtractor *jwtclaims.ClaimsExtractor } // NewPostureChecksHandler creates a new PostureChecks handler -func NewPostureChecksHandler(accountManager server.AccountManager, geolocationManager *geolocation.Geolocation, authCfg AuthCfg) *PostureChecksHandler { +func NewPostureChecksHandler(accountManager server.AccountManager, geolocationManager geolocation.Geolocation, authCfg AuthCfg) *PostureChecksHandler { return &PostureChecksHandler{ accountManager: accountManager, geolocationManager: geolocationManager, diff --git a/management/server/http/posture_checks_handler_test.go b/management/server/http/posture_checks_handler_test.go index 02f0f0d8308..649649a2a58 100644 --- a/management/server/http/posture_checks_handler_test.go +++ b/management/server/http/posture_checks_handler_test.go @@ -70,7 +70,7 @@ func initPostureChecksTestData(postureChecks ...*posture.Checks) *PostureChecksH return claims.AccountId, claims.UserId, nil }, }, - geolocationManager: &geolocation.Geolocation{}, + geolocationManager: &geolocation.GeolocationImpl{}, claimsExtractor: jwtclaims.NewClaimsExtractor( jwtclaims.WithFromRequestContext(func(r *http.Request) jwtclaims.AuthorizationClaims { return jwtclaims.AuthorizationClaims{ diff --git a/management/server/http/setupkeys_component_test.go b/management/server/http/setupkeys_component_test.go new file mode 100644 index 00000000000..1813ac7ba7c --- /dev/null +++ b/management/server/http/setupkeys_component_test.go @@ -0,0 +1,159 @@ +//go:build component + +package http + +import ( + "bytes" + "context" + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/stretchr/testify/assert" + + "github.com/netbirdio/netbird/management/server" + "github.com/netbirdio/netbird/management/server/activity" + "github.com/netbirdio/netbird/management/server/geolocation" + "github.com/netbirdio/netbird/management/server/http/api" + "github.com/netbirdio/netbird/management/server/jwtclaims" + "github.com/netbirdio/netbird/management/server/telemetry" +) + +const ( + testAccountId = "testUserId" + testUserId = "testAccountId" + + newKeyName = "newKey" + expiresIn = 3600 + + existingKeyName = "existingKey" +) + +func Test_SetupKeys_Create_Success(t *testing.T) { + tt := []struct { + name string + expectedStatus int + expectedSetupKey *api.SetupKey + requestBody *api.CreateSetupKeyRequest + requestType string + requestPath string + }{ + { + name: "Create Setup Key", + requestType: http.MethodPost, + requestPath: "/api/setup-keys", + requestBody: &api.CreateSetupKeyRequest{ + AutoGroups: nil, + ExpiresIn: expiresIn, + Name: newKeyName, + Type: "reusable", + UsageLimit: 0, + }, + expectedStatus: http.StatusOK, + expectedSetupKey: &api.SetupKey{ + AutoGroups: []string{}, + Ephemeral: false, + Expires: time.Time{}, + Id: "", + Key: "", + LastUsed: time.Time{}, + Name: newKeyName, + Revoked: false, + State: "valid", + Type: "reusable", + UpdatedAt: time.Now(), + UsageLimit: 0, + UsedTimes: 0, + Valid: true, + }, + }, + } + + for _, tc := range tt { + t.Run(tc.name, func(t *testing.T) { + store, cleanup, err := server.NewTestStoreFromSQL(context.Background(), "testdata/setup_keys.sql", t.TempDir()) + if err != nil { + t.Fatalf("Failed to create test store: %v", err) + } + t.Cleanup(cleanup) + + metrics, err := telemetry.NewDefaultAppMetrics(context.Background()) + am := server.DefaultAccountManager{ + Store: store, + EventStore: &activity.InMemoryEventStore{}, + } + apiHandler, err := APIHandler(context.Background(), &am, &geolocation.GeolocationMock{}, &jwtclaims.JwtValidatorMock{}, metrics, AuthCfg{}, server.MocIntegratedValidator{}) + if err != nil { + t.Fatalf("Failed to create API handler: %v", err) + } + + body, err := json.Marshal(tc.requestBody) + if err != nil { + t.Fatalf("Failed to marshal request body: %v", err) + } + + recorder := httptest.NewRecorder() + req := httptest.NewRequest(tc.requestType, tc.requestPath, bytes.NewBuffer(body)) + req.Header.Set("Authorization", "Bearer "+"my.dummy.token") + + apiHandler.ServeHTTP(recorder, req) + + res := recorder.Result() + defer res.Body.Close() + + content, err := io.ReadAll(res.Body) + if err != nil { + t.Fatalf("I don't know what I expected; %v", err) + } + + if status := recorder.Code; status != tc.expectedStatus { + t.Errorf("handler returned wrong status code: got %v want %v, content: %s", + status, tc.expectedStatus, string(content)) + return + } + + got := &api.SetupKey{} + if err = json.Unmarshal(content, &got); err != nil { + t.Fatalf("Sent content is not in correct json format; %v", err) + } + + validateCreatedKey(t, tc.expectedSetupKey, got) + + key, err := am.GetSetupKey(context.Background(), testAccountId, testUserId, got.Id) + if err != nil { + return + } + + validateCreatedKey(t, tc.expectedSetupKey, toResponseBody(key)) + }) + } +} + +func validateCreatedKey(t *testing.T, expectedKey *api.SetupKey, got *api.SetupKey) { + t.Helper() + + if got.Expires.After(time.Now().Add(-1*time.Minute)) && got.Expires.Before(time.Now().Add(expiresIn*time.Second)) { + got.Expires = time.Time{} + expectedKey.Expires = time.Time{} + } + + if got.Id == "" { + t.Error("Expected key to have an ID") + } + got.Id = "" + + if got.Key == "" { + t.Error("Expected key to have a key") + } + got.Key = "" + + if got.UpdatedAt.After(time.Now().Add(-1*time.Minute)) && got.UpdatedAt.Before(time.Now().Add(+1*time.Minute)) { + got.UpdatedAt = time.Time{} + expectedKey.UpdatedAt = time.Time{} + } + + assert.Equal(t, expectedKey, got) +} diff --git a/management/server/integrated_validator.go b/management/server/integrated_validator.go index 0c70b702a01..48ba60bfaeb 100644 --- a/management/server/integrated_validator.go +++ b/management/server/integrated_validator.go @@ -7,6 +7,8 @@ import ( log "github.com/sirupsen/logrus" "github.com/netbirdio/netbird/management/server/account" + "github.com/netbirdio/netbird/management/server/group" + nbpeer "github.com/netbirdio/netbird/management/server/peer" ) // UpdateIntegratedValidatorGroups updates the integrated validator groups for a specified account. @@ -76,3 +78,44 @@ func (am *DefaultAccountManager) GroupValidation(ctx context.Context, accountID func (am *DefaultAccountManager) GetValidatedPeers(account *Account) (map[string]struct{}, error) { return am.integratedPeerValidator.GetValidatedPeers(account.Id, account.Groups, account.Peers, account.Settings.Extra) } + +type MocIntegratedValidator struct { + ValidatePeerFunc func(_ context.Context, update *nbpeer.Peer, peer *nbpeer.Peer, userID string, accountID string, dnsDomain string, peersGroup []string, extraSettings *account.ExtraSettings) (*nbpeer.Peer, bool, error) +} + +func (a MocIntegratedValidator) ValidateExtraSettings(_ context.Context, newExtraSettings *account.ExtraSettings, oldExtraSettings *account.ExtraSettings, peers map[string]*nbpeer.Peer, userID string, accountID string) error { + return nil +} + +func (a MocIntegratedValidator) ValidatePeer(_ context.Context, update *nbpeer.Peer, peer *nbpeer.Peer, userID string, accountID string, dnsDomain string, peersGroup []string, extraSettings *account.ExtraSettings) (*nbpeer.Peer, bool, error) { + if a.ValidatePeerFunc != nil { + return a.ValidatePeerFunc(context.Background(), update, peer, userID, accountID, dnsDomain, peersGroup, extraSettings) + } + return update, false, nil +} +func (a MocIntegratedValidator) GetValidatedPeers(accountID string, groups map[string]*group.Group, peers map[string]*nbpeer.Peer, extraSettings *account.ExtraSettings) (map[string]struct{}, error) { + validatedPeers := make(map[string]struct{}) + for _, peer := range peers { + validatedPeers[peer.ID] = struct{}{} + } + return validatedPeers, nil +} + +func (MocIntegratedValidator) PreparePeer(_ context.Context, accountID string, peer *nbpeer.Peer, peersGroup []string, extraSettings *account.ExtraSettings) *nbpeer.Peer { + return peer +} + +func (MocIntegratedValidator) IsNotValidPeer(_ context.Context, accountID string, peer *nbpeer.Peer, peersGroup []string, extraSettings *account.ExtraSettings) (bool, bool, error) { + return false, false, nil +} + +func (MocIntegratedValidator) PeerDeleted(_ context.Context, _, _ string) error { + return nil +} + +func (MocIntegratedValidator) SetPeerInvalidationListener(func(accountID string)) { + +} + +func (MocIntegratedValidator) Stop(_ context.Context) { +} diff --git a/management/server/jwtclaims/jwtValidator.go b/management/server/jwtclaims/jwtValidator.go index d5c1e7c9e93..ab43c7fe9a8 100644 --- a/management/server/jwtclaims/jwtValidator.go +++ b/management/server/jwtclaims/jwtValidator.go @@ -72,13 +72,17 @@ type JSONWebKey struct { X5c []string `json:"x5c"` } -// JWTValidator struct to handle token validation and parsing -type JWTValidator struct { +type JWTValidator interface { + ValidateAndParse(ctx context.Context, token string) (*jwt.Token, error) +} + +// JWTValidatorImpl struct to handle token validation and parsing +type JWTValidatorImpl struct { options Options } // NewJWTValidator constructor -func NewJWTValidator(ctx context.Context, issuer string, audienceList []string, keysLocation string, idpSignkeyRefreshEnabled bool) (*JWTValidator, error) { +func NewJWTValidator(ctx context.Context, issuer string, audienceList []string, keysLocation string, idpSignkeyRefreshEnabled bool) (JWTValidator, error) { keys, err := getPemKeys(ctx, keysLocation) if err != nil { return nil, err @@ -138,13 +142,13 @@ func NewJWTValidator(ctx context.Context, issuer string, audienceList []string, options.UserProperty = "user" } - return &JWTValidator{ + return &JWTValidatorImpl{ options: options, }, nil } // ValidateAndParse validates the token and returns the parsed token -func (m *JWTValidator) ValidateAndParse(ctx context.Context, token string) (*jwt.Token, error) { +func (m *JWTValidatorImpl) ValidateAndParse(ctx context.Context, token string) (*jwt.Token, error) { // If the token is empty... if token == "" { // Check if it was required @@ -311,3 +315,15 @@ func getMaxAgeFromCacheHeader(ctx context.Context, cacheControl string) int { return 0 } +type JwtValidatorMock struct{} + +func (j *JwtValidatorMock) ValidateAndParse(ctx context.Context, token string) (*jwt.Token, error) { + claimMaps := jwt.MapClaims{} + claimMaps[UserIDClaim] = "testUserId" + claimMaps[AccountIDSuffix] = "testAccountId" + claimMaps[DomainIDSuffix] = "test.com" + claimMaps[DomainCategorySuffix] = "private" + jwtToken := jwt.NewWithClaims(jwt.SigningMethodHS256, claimMaps) + + return jwtToken, nil +} diff --git a/management/server/user_test.go b/management/server/user_test.go index d4f560a54c7..b025821859b 100644 --- a/management/server/user_test.go +++ b/management/server/user_test.go @@ -10,13 +10,14 @@ import ( "github.com/eko/gocache/v3/cache" cacheStore "github.com/eko/gocache/v3/store" "github.com/google/go-cmp/cmp" - nbgroup "github.com/netbirdio/netbird/management/server/group" - nbpeer "github.com/netbirdio/netbird/management/server/peer" gocache "github.com/patrickmn/go-cache" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.zx2c4.com/wireguard/wgctrl/wgtypes" + nbgroup "github.com/netbirdio/netbird/management/server/group" + nbpeer "github.com/netbirdio/netbird/management/server/peer" + "github.com/netbirdio/netbird/management/server/activity" "github.com/netbirdio/netbird/management/server/idp" "github.com/netbirdio/netbird/management/server/integration_reference" @@ -52,7 +53,7 @@ func TestUser_CreatePAT_ForSameUser(t *testing.T) { am := DefaultAccountManager{ Store: store, - eventStore: &activity.InMemoryEventStore{}, + EventStore: &activity.InMemoryEventStore{}, } pat, err := am.CreatePAT(context.Background(), mockAccountID, mockUserID, mockUserID, mockTokenName, mockExpiresIn) @@ -96,7 +97,7 @@ func TestUser_CreatePAT_ForDifferentUser(t *testing.T) { am := DefaultAccountManager{ Store: store, - eventStore: &activity.InMemoryEventStore{}, + EventStore: &activity.InMemoryEventStore{}, } _, err = am.CreatePAT(context.Background(), mockAccountID, mockUserID, mockTargetUserId, mockTokenName, mockExpiresIn) @@ -118,7 +119,7 @@ func TestUser_CreatePAT_ForServiceUser(t *testing.T) { am := DefaultAccountManager{ Store: store, - eventStore: &activity.InMemoryEventStore{}, + EventStore: &activity.InMemoryEventStore{}, } pat, err := am.CreatePAT(context.Background(), mockAccountID, mockUserID, mockTargetUserId, mockTokenName, mockExpiresIn) @@ -141,7 +142,7 @@ func TestUser_CreatePAT_WithWrongExpiration(t *testing.T) { am := DefaultAccountManager{ Store: store, - eventStore: &activity.InMemoryEventStore{}, + EventStore: &activity.InMemoryEventStore{}, } _, err = am.CreatePAT(context.Background(), mockAccountID, mockUserID, mockUserID, mockTokenName, mockWrongExpiresIn) @@ -160,7 +161,7 @@ func TestUser_CreatePAT_WithEmptyName(t *testing.T) { am := DefaultAccountManager{ Store: store, - eventStore: &activity.InMemoryEventStore{}, + EventStore: &activity.InMemoryEventStore{}, } _, err = am.CreatePAT(context.Background(), mockAccountID, mockUserID, mockUserID, mockEmptyTokenName, mockExpiresIn) @@ -187,7 +188,7 @@ func TestUser_DeletePAT(t *testing.T) { am := DefaultAccountManager{ Store: store, - eventStore: &activity.InMemoryEventStore{}, + EventStore: &activity.InMemoryEventStore{}, } err = am.DeletePAT(context.Background(), mockAccountID, mockUserID, mockUserID, mockTokenID1) @@ -224,7 +225,7 @@ func TestUser_GetPAT(t *testing.T) { am := DefaultAccountManager{ Store: store, - eventStore: &activity.InMemoryEventStore{}, + EventStore: &activity.InMemoryEventStore{}, } pat, err := am.GetPAT(context.Background(), mockAccountID, mockUserID, mockUserID, mockTokenID1) @@ -261,7 +262,7 @@ func TestUser_GetAllPATs(t *testing.T) { am := DefaultAccountManager{ Store: store, - eventStore: &activity.InMemoryEventStore{}, + EventStore: &activity.InMemoryEventStore{}, } pats, err := am.GetAllPATs(context.Background(), mockAccountID, mockUserID, mockUserID) @@ -351,7 +352,7 @@ func TestUser_CreateServiceUser(t *testing.T) { am := DefaultAccountManager{ Store: store, - eventStore: &activity.InMemoryEventStore{}, + EventStore: &activity.InMemoryEventStore{}, } user, err := am.createServiceUser(context.Background(), mockAccountID, mockUserID, mockRole, mockServiceUserName, false, []string{"group1", "group2"}) @@ -392,7 +393,7 @@ func TestUser_CreateUser_ServiceUser(t *testing.T) { am := DefaultAccountManager{ Store: store, - eventStore: &activity.InMemoryEventStore{}, + EventStore: &activity.InMemoryEventStore{}, } user, err := am.CreateUser(context.Background(), mockAccountID, mockUserID, &UserInfo{ @@ -434,7 +435,7 @@ func TestUser_CreateUser_RegularUser(t *testing.T) { am := DefaultAccountManager{ Store: store, - eventStore: &activity.InMemoryEventStore{}, + EventStore: &activity.InMemoryEventStore{}, } _, err = am.CreateUser(context.Background(), mockAccountID, mockUserID, &UserInfo{ @@ -459,7 +460,7 @@ func TestUser_InviteNewUser(t *testing.T) { am := DefaultAccountManager{ Store: store, - eventStore: &activity.InMemoryEventStore{}, + EventStore: &activity.InMemoryEventStore{}, cacheLoading: map[string]chan struct{}{}, } @@ -559,7 +560,7 @@ func TestUser_DeleteUser_ServiceUser(t *testing.T) { am := DefaultAccountManager{ Store: store, - eventStore: &activity.InMemoryEventStore{}, + EventStore: &activity.InMemoryEventStore{}, } err = am.DeleteUser(context.Background(), mockAccountID, mockUserID, mockServiceUserID) @@ -591,7 +592,7 @@ func TestUser_DeleteUser_SelfDelete(t *testing.T) { am := DefaultAccountManager{ Store: store, - eventStore: &activity.InMemoryEventStore{}, + EventStore: &activity.InMemoryEventStore{}, } err = am.DeleteUser(context.Background(), mockAccountID, mockUserID, mockUserID) @@ -639,7 +640,7 @@ func TestUser_DeleteUser_regularUser(t *testing.T) { am := DefaultAccountManager{ Store: store, - eventStore: &activity.InMemoryEventStore{}, + EventStore: &activity.InMemoryEventStore{}, integratedPeerValidator: MocIntegratedValidator{}, } @@ -743,7 +744,7 @@ func TestUser_DeleteUser_RegularUsers(t *testing.T) { am := DefaultAccountManager{ Store: store, - eventStore: &activity.InMemoryEventStore{}, + EventStore: &activity.InMemoryEventStore{}, integratedPeerValidator: MocIntegratedValidator{}, } @@ -845,7 +846,7 @@ func TestDefaultAccountManager_GetUser(t *testing.T) { am := DefaultAccountManager{ Store: store, - eventStore: &activity.InMemoryEventStore{}, + EventStore: &activity.InMemoryEventStore{}, } claims := jwtclaims.AuthorizationClaims{ @@ -876,7 +877,7 @@ func TestDefaultAccountManager_ListUsers(t *testing.T) { am := DefaultAccountManager{ Store: store, - eventStore: &activity.InMemoryEventStore{}, + EventStore: &activity.InMemoryEventStore{}, } users, err := am.ListUsers(context.Background(), mockAccountID) @@ -958,7 +959,7 @@ func TestDefaultAccountManager_ListUsers_DashboardPermissions(t *testing.T) { am := DefaultAccountManager{ Store: store, - eventStore: &activity.InMemoryEventStore{}, + EventStore: &activity.InMemoryEventStore{}, } users, err := am.ListUsers(context.Background(), mockAccountID) @@ -997,7 +998,7 @@ func TestDefaultAccountManager_ExternalCache(t *testing.T) { am := DefaultAccountManager{ Store: store, - eventStore: &activity.InMemoryEventStore{}, + EventStore: &activity.InMemoryEventStore{}, idpManager: &idp.GoogleWorkspaceManager{}, // empty manager cacheLoading: map[string]chan struct{}{}, cacheManager: cache.New[[]*idp.UserData]( @@ -1056,7 +1057,7 @@ func TestUser_GetUsersFromAccount_ForAdmin(t *testing.T) { am := DefaultAccountManager{ Store: store, - eventStore: &activity.InMemoryEventStore{}, + EventStore: &activity.InMemoryEventStore{}, } users, err := am.GetUsersFromAccount(context.Background(), mockAccountID, mockUserID) @@ -1085,7 +1086,7 @@ func TestUser_GetUsersFromAccount_ForUser(t *testing.T) { am := DefaultAccountManager{ Store: store, - eventStore: &activity.InMemoryEventStore{}, + EventStore: &activity.InMemoryEventStore{}, } users, err := am.GetUsersFromAccount(context.Background(), mockAccountID, mockServiceUserID) From 72157b55301e52de3879a7ad93069958f539a03e Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Fri, 22 Nov 2024 16:40:19 +0100 Subject: [PATCH 02/38] unexport event store and add peerUpdateManager --- management/server/account.go | 4 +- management/server/event.go | 4 +- management/server/event_test.go | 8 ++-- ..._test.go => setupkeys_integration_test.go} | 13 ++++-- management/server/user_test.go | 44 +++++++++---------- 5 files changed, 39 insertions(+), 34 deletions(-) rename management/server/http/{setupkeys_component_test.go => setupkeys_integration_test.go} (88%) diff --git a/management/server/account.go b/management/server/account.go index 37bc0529fe1..68e0035d386 100644 --- a/management/server/account.go +++ b/management/server/account.go @@ -166,7 +166,7 @@ type DefaultAccountManager struct { cacheManager cache.CacheInterface[[]*idp.UserData] externalCacheManager ExternalCacheManager ctx context.Context - EventStore activity.Store + eventStore activity.Store geo geolocation.Geolocation requestBuffer *AccountRequestBuffer @@ -1055,7 +1055,7 @@ func BuildManager( cacheMux: sync.Mutex{}, cacheLoading: map[string]chan struct{}{}, dnsDomain: dnsDomain, - EventStore: eventStore, + eventStore: eventStore, peerLoginExpiry: NewDefaultScheduler(), peerInactivityExpiry: NewDefaultScheduler(), userDeleteFromIDPEnabled: userDeleteFromIDPEnabled, diff --git a/management/server/event.go b/management/server/event.go index a85ff2ff201..93b80922678 100644 --- a/management/server/event.go +++ b/management/server/event.go @@ -30,7 +30,7 @@ func (am *DefaultAccountManager) GetEvents(ctx context.Context, accountID, userI return nil, status.Errorf(status.PermissionDenied, "only users with admin power can view events") } - events, err := am.EventStore.Get(ctx, accountID, 0, 10000, true) + events, err := am.eventStore.Get(ctx, accountID, 0, 10000, true) if err != nil { return nil, err } @@ -58,7 +58,7 @@ func (am *DefaultAccountManager) GetEvents(ctx context.Context, accountID, userI func (am *DefaultAccountManager) StoreEvent(ctx context.Context, initiatorID, targetID, accountID string, activityID activity.ActivityDescriber, meta map[string]any) { go func() { - _, err := am.EventStore.Save(ctx, &activity.Event{ + _, err := am.eventStore.Save(ctx, &activity.Event{ Timestamp: time.Now().UTC(), Activity: activityID, InitiatorID: initiatorID, diff --git a/management/server/event_test.go b/management/server/event_test.go index 4cbec56dc75..8c56fd3f6ec 100644 --- a/management/server/event_test.go +++ b/management/server/event_test.go @@ -14,7 +14,7 @@ func generateAndStoreEvents(t *testing.T, manager *DefaultAccountManager, typ ac accountID string, count int) { t.Helper() for i := 0; i < count; i++ { - _, err := manager.EventStore.Save(context.Background(), &activity.Event{ + _, err := manager.eventStore.Save(context.Background(), &activity.Event{ Timestamp: time.Now().UTC(), Activity: typ, InitiatorID: initiatorID, @@ -41,7 +41,7 @@ func TestDefaultAccountManager_GetEvents(t *testing.T) { return } assert.Len(t, events, 0) - _ = manager.EventStore.Close(context.Background()) //nolint + _ = manager.eventStore.Close(context.Background()) //nolint }) t.Run("get events", func(t *testing.T) { @@ -52,7 +52,7 @@ func TestDefaultAccountManager_GetEvents(t *testing.T) { } assert.Len(t, events, 10) - _ = manager.EventStore.Close(context.Background()) //nolint + _ = manager.eventStore.Close(context.Background()) //nolint }) t.Run("get events without duplicates", func(t *testing.T) { @@ -62,6 +62,6 @@ func TestDefaultAccountManager_GetEvents(t *testing.T) { return } assert.Len(t, events, 1) - _ = manager.EventStore.Close(context.Background()) //nolint + _ = manager.eventStore.Close(context.Background()) //nolint }) } diff --git a/management/server/http/setupkeys_component_test.go b/management/server/http/setupkeys_integration_test.go similarity index 88% rename from management/server/http/setupkeys_component_test.go rename to management/server/http/setupkeys_integration_test.go index 1813ac7ba7c..e32ea225e63 100644 --- a/management/server/http/setupkeys_component_test.go +++ b/management/server/http/setupkeys_integration_test.go @@ -81,11 +81,16 @@ func Test_SetupKeys_Create_Success(t *testing.T) { t.Cleanup(cleanup) metrics, err := telemetry.NewDefaultAppMetrics(context.Background()) - am := server.DefaultAccountManager{ - Store: store, - EventStore: &activity.InMemoryEventStore{}, + + peersUpdateManager := &server.PeersUpdateManager{} + geoMock := &geolocation.GeolocationMock{} + validatorMock := server.MocIntegratedValidator{} + am, err := server.BuildManager(context.Background(), store, peersUpdateManager, nil, "", "", &activity.InMemoryEventStore{}, geoMock, false, validatorMock, metrics) + if err != nil { + t.Fatalf("Failed to create manager: %v", err) } - apiHandler, err := APIHandler(context.Background(), &am, &geolocation.GeolocationMock{}, &jwtclaims.JwtValidatorMock{}, metrics, AuthCfg{}, server.MocIntegratedValidator{}) + + apiHandler, err := APIHandler(context.Background(), am, geoMock, &jwtclaims.JwtValidatorMock{}, metrics, AuthCfg{}, validatorMock) if err != nil { t.Fatalf("Failed to create API handler: %v", err) } diff --git a/management/server/user_test.go b/management/server/user_test.go index b025821859b..cea326258ad 100644 --- a/management/server/user_test.go +++ b/management/server/user_test.go @@ -53,7 +53,7 @@ func TestUser_CreatePAT_ForSameUser(t *testing.T) { am := DefaultAccountManager{ Store: store, - EventStore: &activity.InMemoryEventStore{}, + eventStore: &activity.InMemoryEventStore{}, } pat, err := am.CreatePAT(context.Background(), mockAccountID, mockUserID, mockUserID, mockTokenName, mockExpiresIn) @@ -97,7 +97,7 @@ func TestUser_CreatePAT_ForDifferentUser(t *testing.T) { am := DefaultAccountManager{ Store: store, - EventStore: &activity.InMemoryEventStore{}, + eventStore: &activity.InMemoryEventStore{}, } _, err = am.CreatePAT(context.Background(), mockAccountID, mockUserID, mockTargetUserId, mockTokenName, mockExpiresIn) @@ -119,7 +119,7 @@ func TestUser_CreatePAT_ForServiceUser(t *testing.T) { am := DefaultAccountManager{ Store: store, - EventStore: &activity.InMemoryEventStore{}, + eventStore: &activity.InMemoryEventStore{}, } pat, err := am.CreatePAT(context.Background(), mockAccountID, mockUserID, mockTargetUserId, mockTokenName, mockExpiresIn) @@ -142,7 +142,7 @@ func TestUser_CreatePAT_WithWrongExpiration(t *testing.T) { am := DefaultAccountManager{ Store: store, - EventStore: &activity.InMemoryEventStore{}, + eventStore: &activity.InMemoryEventStore{}, } _, err = am.CreatePAT(context.Background(), mockAccountID, mockUserID, mockUserID, mockTokenName, mockWrongExpiresIn) @@ -161,7 +161,7 @@ func TestUser_CreatePAT_WithEmptyName(t *testing.T) { am := DefaultAccountManager{ Store: store, - EventStore: &activity.InMemoryEventStore{}, + eventStore: &activity.InMemoryEventStore{}, } _, err = am.CreatePAT(context.Background(), mockAccountID, mockUserID, mockUserID, mockEmptyTokenName, mockExpiresIn) @@ -188,7 +188,7 @@ func TestUser_DeletePAT(t *testing.T) { am := DefaultAccountManager{ Store: store, - EventStore: &activity.InMemoryEventStore{}, + eventStore: &activity.InMemoryEventStore{}, } err = am.DeletePAT(context.Background(), mockAccountID, mockUserID, mockUserID, mockTokenID1) @@ -225,7 +225,7 @@ func TestUser_GetPAT(t *testing.T) { am := DefaultAccountManager{ Store: store, - EventStore: &activity.InMemoryEventStore{}, + eventStore: &activity.InMemoryEventStore{}, } pat, err := am.GetPAT(context.Background(), mockAccountID, mockUserID, mockUserID, mockTokenID1) @@ -262,7 +262,7 @@ func TestUser_GetAllPATs(t *testing.T) { am := DefaultAccountManager{ Store: store, - EventStore: &activity.InMemoryEventStore{}, + eventStore: &activity.InMemoryEventStore{}, } pats, err := am.GetAllPATs(context.Background(), mockAccountID, mockUserID, mockUserID) @@ -352,7 +352,7 @@ func TestUser_CreateServiceUser(t *testing.T) { am := DefaultAccountManager{ Store: store, - EventStore: &activity.InMemoryEventStore{}, + eventStore: &activity.InMemoryEventStore{}, } user, err := am.createServiceUser(context.Background(), mockAccountID, mockUserID, mockRole, mockServiceUserName, false, []string{"group1", "group2"}) @@ -393,7 +393,7 @@ func TestUser_CreateUser_ServiceUser(t *testing.T) { am := DefaultAccountManager{ Store: store, - EventStore: &activity.InMemoryEventStore{}, + eventStore: &activity.InMemoryEventStore{}, } user, err := am.CreateUser(context.Background(), mockAccountID, mockUserID, &UserInfo{ @@ -435,7 +435,7 @@ func TestUser_CreateUser_RegularUser(t *testing.T) { am := DefaultAccountManager{ Store: store, - EventStore: &activity.InMemoryEventStore{}, + eventStore: &activity.InMemoryEventStore{}, } _, err = am.CreateUser(context.Background(), mockAccountID, mockUserID, &UserInfo{ @@ -460,7 +460,7 @@ func TestUser_InviteNewUser(t *testing.T) { am := DefaultAccountManager{ Store: store, - EventStore: &activity.InMemoryEventStore{}, + eventStore: &activity.InMemoryEventStore{}, cacheLoading: map[string]chan struct{}{}, } @@ -560,7 +560,7 @@ func TestUser_DeleteUser_ServiceUser(t *testing.T) { am := DefaultAccountManager{ Store: store, - EventStore: &activity.InMemoryEventStore{}, + eventStore: &activity.InMemoryEventStore{}, } err = am.DeleteUser(context.Background(), mockAccountID, mockUserID, mockServiceUserID) @@ -592,7 +592,7 @@ func TestUser_DeleteUser_SelfDelete(t *testing.T) { am := DefaultAccountManager{ Store: store, - EventStore: &activity.InMemoryEventStore{}, + eventStore: &activity.InMemoryEventStore{}, } err = am.DeleteUser(context.Background(), mockAccountID, mockUserID, mockUserID) @@ -640,7 +640,7 @@ func TestUser_DeleteUser_regularUser(t *testing.T) { am := DefaultAccountManager{ Store: store, - EventStore: &activity.InMemoryEventStore{}, + eventStore: &activity.InMemoryEventStore{}, integratedPeerValidator: MocIntegratedValidator{}, } @@ -744,7 +744,7 @@ func TestUser_DeleteUser_RegularUsers(t *testing.T) { am := DefaultAccountManager{ Store: store, - EventStore: &activity.InMemoryEventStore{}, + eventStore: &activity.InMemoryEventStore{}, integratedPeerValidator: MocIntegratedValidator{}, } @@ -846,7 +846,7 @@ func TestDefaultAccountManager_GetUser(t *testing.T) { am := DefaultAccountManager{ Store: store, - EventStore: &activity.InMemoryEventStore{}, + eventStore: &activity.InMemoryEventStore{}, } claims := jwtclaims.AuthorizationClaims{ @@ -877,7 +877,7 @@ func TestDefaultAccountManager_ListUsers(t *testing.T) { am := DefaultAccountManager{ Store: store, - EventStore: &activity.InMemoryEventStore{}, + eventStore: &activity.InMemoryEventStore{}, } users, err := am.ListUsers(context.Background(), mockAccountID) @@ -959,7 +959,7 @@ func TestDefaultAccountManager_ListUsers_DashboardPermissions(t *testing.T) { am := DefaultAccountManager{ Store: store, - EventStore: &activity.InMemoryEventStore{}, + eventStore: &activity.InMemoryEventStore{}, } users, err := am.ListUsers(context.Background(), mockAccountID) @@ -998,7 +998,7 @@ func TestDefaultAccountManager_ExternalCache(t *testing.T) { am := DefaultAccountManager{ Store: store, - EventStore: &activity.InMemoryEventStore{}, + eventStore: &activity.InMemoryEventStore{}, idpManager: &idp.GoogleWorkspaceManager{}, // empty manager cacheLoading: map[string]chan struct{}{}, cacheManager: cache.New[[]*idp.UserData]( @@ -1057,7 +1057,7 @@ func TestUser_GetUsersFromAccount_ForAdmin(t *testing.T) { am := DefaultAccountManager{ Store: store, - EventStore: &activity.InMemoryEventStore{}, + eventStore: &activity.InMemoryEventStore{}, } users, err := am.GetUsersFromAccount(context.Background(), mockAccountID, mockUserID) @@ -1086,7 +1086,7 @@ func TestUser_GetUsersFromAccount_ForUser(t *testing.T) { am := DefaultAccountManager{ Store: store, - EventStore: &activity.InMemoryEventStore{}, + eventStore: &activity.InMemoryEventStore{}, } users, err := am.GetUsersFromAccount(context.Background(), mockAccountID, mockServiceUserID) From bc6428c859e163c5e3cc6eae545d0ceee48491b9 Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Fri, 22 Nov 2024 17:02:55 +0100 Subject: [PATCH 03/38] wait if network map update is received and allow validating network map + remove MocIntegrated Validator from tests --- management/server/account_test.go | 48 ++----------------- .../server/http/setupkeys_integration_test.go | 40 +++++++++++++++- management/server/management_test.go | 42 +--------------- 3 files changed, 43 insertions(+), 87 deletions(-) diff --git a/management/server/account_test.go b/management/server/account_test.go index 97e0d45f016..fb1f39868d4 100644 --- a/management/server/account_test.go +++ b/management/server/account_test.go @@ -18,7 +18,6 @@ import ( "golang.zx2c4.com/wireguard/wgctrl/wgtypes" nbdns "github.com/netbirdio/netbird/dns" - "github.com/netbirdio/netbird/management/server/account" "github.com/netbirdio/netbird/management/server/activity" "github.com/netbirdio/netbird/management/server/group" "github.com/netbirdio/netbird/management/server/jwtclaims" @@ -28,47 +27,6 @@ import ( "github.com/netbirdio/netbird/route" ) -type MocIntegratedValidator struct { - ValidatePeerFunc func(_ context.Context, update *nbpeer.Peer, peer *nbpeer.Peer, userID string, accountID string, dnsDomain string, peersGroup []string, extraSettings *account.ExtraSettings) (*nbpeer.Peer, bool, error) -} - -func (a MocIntegratedValidator) ValidateExtraSettings(_ context.Context, newExtraSettings *account.ExtraSettings, oldExtraSettings *account.ExtraSettings, peers map[string]*nbpeer.Peer, userID string, accountID string) error { - return nil -} - -func (a MocIntegratedValidator) ValidatePeer(_ context.Context, update *nbpeer.Peer, peer *nbpeer.Peer, userID string, accountID string, dnsDomain string, peersGroup []string, extraSettings *account.ExtraSettings) (*nbpeer.Peer, bool, error) { - if a.ValidatePeerFunc != nil { - return a.ValidatePeerFunc(context.Background(), update, peer, userID, accountID, dnsDomain, peersGroup, extraSettings) - } - return update, false, nil -} -func (a MocIntegratedValidator) GetValidatedPeers(accountID string, groups map[string]*group.Group, peers map[string]*nbpeer.Peer, extraSettings *account.ExtraSettings) (map[string]struct{}, error) { - validatedPeers := make(map[string]struct{}) - for _, peer := range peers { - validatedPeers[peer.ID] = struct{}{} - } - return validatedPeers, nil -} - -func (MocIntegratedValidator) PreparePeer(_ context.Context, accountID string, peer *nbpeer.Peer, peersGroup []string, extraSettings *account.ExtraSettings) *nbpeer.Peer { - return peer -} - -func (MocIntegratedValidator) IsNotValidPeer(_ context.Context, accountID string, peer *nbpeer.Peer, peersGroup []string, extraSettings *account.ExtraSettings) (bool, bool, error) { - return false, false, nil -} - -func (MocIntegratedValidator) PeerDeleted(_ context.Context, _, _ string) error { - return nil -} - -func (MocIntegratedValidator) SetPeerInvalidationListener(func(accountID string)) { - -} - -func (MocIntegratedValidator) Stop(_ context.Context) { -} - func verifyCanAddPeerToAccount(t *testing.T, manager AccountManager, account *Account, userID string) { t.Helper() peer := &nbpeer.Peer{ @@ -1038,7 +996,7 @@ func BenchmarkTest_GetAccountWithclaims(b *testing.B) { } b.Run("public without account ID", func(b *testing.B) { - //b.ResetTimer() + // b.ResetTimer() for i := 0; i < b.N; i++ { _, err := am.getAccountIDWithAuthorizationClaims(context.Background(), publicClaims) if err != nil { @@ -1048,7 +1006,7 @@ func BenchmarkTest_GetAccountWithclaims(b *testing.B) { }) b.Run("private without account ID", func(b *testing.B) { - //b.ResetTimer() + // b.ResetTimer() for i := 0; i < b.N; i++ { _, err := am.getAccountIDWithAuthorizationClaims(context.Background(), claims) if err != nil { @@ -1059,7 +1017,7 @@ func BenchmarkTest_GetAccountWithclaims(b *testing.B) { b.Run("private with account ID", func(b *testing.B) { claims.AccountId = id - //b.ResetTimer() + // b.ResetTimer() for i := 0; i < b.N; i++ { _, err := am.getAccountIDWithAuthorizationClaims(context.Background(), claims) if err != nil { diff --git a/management/server/http/setupkeys_integration_test.go b/management/server/http/setupkeys_integration_test.go index e32ea225e63..eb3fc1b0186 100644 --- a/management/server/http/setupkeys_integration_test.go +++ b/management/server/http/setupkeys_integration_test.go @@ -25,6 +25,7 @@ import ( const ( testAccountId = "testUserId" testUserId = "testAccountId" + testPeerId = "testPeerId" newKeyName = "newKey" expiresIn = 3600 @@ -82,7 +83,14 @@ func Test_SetupKeys_Create_Success(t *testing.T) { metrics, err := telemetry.NewDefaultAppMetrics(context.Background()) - peersUpdateManager := &server.PeersUpdateManager{} + peersUpdateManager := server.NewPeersUpdateManager(nil) + updMsg := peersUpdateManager.CreateChannel(context.Background(), testPeerId) + done := make(chan struct{}) + go func() { + peerShouldNotReceiveUpdate(t, updMsg) + close(done) + }() + geoMock := &geolocation.GeolocationMock{} validatorMock := server.MocIntegratedValidator{} am, err := server.BuildManager(context.Background(), store, peersUpdateManager, nil, "", "", &activity.InMemoryEventStore{}, geoMock, false, validatorMock, metrics) @@ -133,6 +141,12 @@ func Test_SetupKeys_Create_Success(t *testing.T) { } validateCreatedKey(t, tc.expectedSetupKey, toResponseBody(key)) + + select { + case <-done: + case <-time.After(time.Second): + t.Error("timeout waiting for peerShouldNotReceiveUpdate") + } }) } } @@ -162,3 +176,27 @@ func validateCreatedKey(t *testing.T, expectedKey *api.SetupKey, got *api.SetupK assert.Equal(t, expectedKey, got) } + +func peerShouldNotReceiveUpdate(t *testing.T, updateMessage <-chan *server.UpdateMessage) { + t.Helper() + select { + case msg := <-updateMessage: + t.Errorf("Unexpected message received: %+v", msg) + case <-time.After(500 * time.Millisecond): + return + } +} + +func peerShouldReceiveUpdate(t *testing.T, updateMessage <-chan *server.UpdateMessage, expected *server.UpdateMessage) { + t.Helper() + + select { + case msg := <-updateMessage: + if msg == nil { + t.Errorf("Received nil update message, expected valid message") + } + assert.Equal(t, expected, msg) + case <-time.After(500 * time.Millisecond): + t.Error("Timed out waiting for update message") + } +} diff --git a/management/server/management_test.go b/management/server/management_test.go index 5361da53fd5..cc10449c891 100644 --- a/management/server/management_test.go +++ b/management/server/management_test.go @@ -21,10 +21,7 @@ import ( "github.com/netbirdio/netbird/encryption" mgmtProto "github.com/netbirdio/netbird/management/proto" "github.com/netbirdio/netbird/management/server" - "github.com/netbirdio/netbird/management/server/account" "github.com/netbirdio/netbird/management/server/activity" - "github.com/netbirdio/netbird/management/server/group" - nbpeer "github.com/netbirdio/netbird/management/server/peer" "github.com/netbirdio/netbird/management/server/telemetry" "github.com/netbirdio/netbird/util" ) @@ -446,43 +443,6 @@ var _ = Describe("Management service", func() { }) }) -type MocIntegratedValidator struct { -} - -func (a MocIntegratedValidator) ValidateExtraSettings(_ context.Context, newExtraSettings *account.ExtraSettings, oldExtraSettings *account.ExtraSettings, peers map[string]*nbpeer.Peer, userID string, accountID string) error { - return nil -} - -func (a MocIntegratedValidator) ValidatePeer(_ context.Context, update *nbpeer.Peer, peer *nbpeer.Peer, userID string, accountID string, dnsDomain string, peersGroup []string, extraSettings *account.ExtraSettings) (*nbpeer.Peer, bool, error) { - return update, false, nil -} - -func (a MocIntegratedValidator) GetValidatedPeers(accountID string, groups map[string]*group.Group, peers map[string]*nbpeer.Peer, extraSettings *account.ExtraSettings) (map[string]struct{}, error) { - validatedPeers := make(map[string]struct{}) - for p := range peers { - validatedPeers[p] = struct{}{} - } - return validatedPeers, nil -} - -func (MocIntegratedValidator) PreparePeer(_ context.Context, accountID string, peer *nbpeer.Peer, peersGroup []string, extraSettings *account.ExtraSettings) *nbpeer.Peer { - return peer -} - -func (MocIntegratedValidator) IsNotValidPeer(_ context.Context, accountID string, peer *nbpeer.Peer, peersGroup []string, extraSettings *account.ExtraSettings) (bool, bool, error) { - return false, false, nil -} - -func (MocIntegratedValidator) PeerDeleted(_ context.Context, _, _ string) error { - return nil -} - -func (MocIntegratedValidator) SetPeerInvalidationListener(func(accountID string)) { - -} - -func (MocIntegratedValidator) Stop(_ context.Context) {} - func loginPeerWithValidSetupKey(serverPubKey wgtypes.Key, key wgtypes.Key, client mgmtProto.ManagementServiceClient) *mgmtProto.LoginResponse { defer GinkgoRecover() @@ -545,7 +505,7 @@ func startServer(config *server.Config, dataDir string, testFile string) (*grpc. log.Fatalf("failed creating metrics: %v", err) } - accountManager, err := server.BuildManager(context.Background(), store, peersUpdateManager, nil, "", "netbird.selfhosted", eventStore, nil, false, MocIntegratedValidator{}, metrics) + accountManager, err := server.BuildManager(context.Background(), store, peersUpdateManager, nil, "", "netbird.selfhosted", eventStore, nil, false, server.MocIntegratedValidator{}, metrics) if err != nil { log.Fatalf("failed creating a manager: %v", err) } From 0928382ebcf1f626937b75a375d3299aeb3cf981 Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Fri, 22 Nov 2024 17:04:44 +0100 Subject: [PATCH 04/38] add testdata --- management/server/http/testdata/setup_keys.sql | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 management/server/http/testdata/setup_keys.sql diff --git a/management/server/http/testdata/setup_keys.sql b/management/server/http/testdata/setup_keys.sql new file mode 100644 index 00000000000..f8d96c27c0d --- /dev/null +++ b/management/server/http/testdata/setup_keys.sql @@ -0,0 +1,8 @@ +CREATE TABLE `accounts` (`id` text,`created_by` text,`created_at` datetime,`domain` text,`domain_category` text,`is_domain_primary_account` numeric,`network_identifier` text,`network_net` text,`network_dns` text,`network_serial` integer,`dns_settings_disabled_management_groups` text,`settings_peer_login_expiration_enabled` numeric,`settings_peer_login_expiration` integer,`settings_regular_users_view_blocked` numeric,`settings_groups_propagation_enabled` numeric,`settings_jwt_groups_enabled` numeric,`settings_jwt_groups_claim_name` text,`settings_jwt_allow_groups` text,`settings_extra_peer_approval_enabled` numeric,`settings_extra_integrated_validator_groups` text,PRIMARY KEY (`id`)); +CREATE TABLE `setup_keys` (`id` text,`account_id` text,`key` text,`name` text,`type` text,`created_at` datetime,`expires_at` datetime,`updated_at` datetime,`revoked` numeric,`used_times` integer,`last_used` datetime,`auto_groups` text,`usage_limit` integer,`ephemeral` numeric,PRIMARY KEY (`id`),CONSTRAINT `fk_accounts_setup_keys_g` FOREIGN KEY (`account_id`) REFERENCES `accounts`(`id`)); +CREATE TABLE `users` (`id` text,`account_id` text,`role` text,`is_service_user` numeric,`non_deletable` numeric,`service_user_name` text,`auto_groups` text,`blocked` numeric,`last_login` datetime,`created_at` datetime,`issued` text DEFAULT "api",`integration_ref_id` integer,`integration_ref_integration_type` text,PRIMARY KEY (`id`),CONSTRAINT `fk_accounts_users_g` FOREIGN KEY (`account_id`) REFERENCES `accounts`(`id`)); +CREATE TABLE `peers` (`id` text,`account_id` text,`key` text,`setup_key` text,`ip` text,`meta_hostname` text,`meta_go_os` text,`meta_kernel` text,`meta_core` text,`meta_platform` text,`meta_os` text,`meta_os_version` text,`meta_wt_version` text,`meta_ui_version` text,`meta_kernel_version` text,`meta_network_addresses` text,`meta_system_serial_number` text,`meta_system_product_name` text,`meta_system_manufacturer` text,`meta_environment` text,`meta_files` text,`name` text,`dns_label` text,`peer_status_last_seen` datetime,`peer_status_connected` numeric,`peer_status_login_expired` numeric,`peer_status_requires_approval` numeric,`user_id` text,`ssh_key` text,`ssh_enabled` numeric,`login_expiration_enabled` numeric,`last_login` datetime,`created_at` datetime,`ephemeral` numeric,`location_connection_ip` text,`location_country_code` text,`location_city_name` text,`location_geo_name_id` integer,PRIMARY KEY (`id`),CONSTRAINT `fk_accounts_peers_g` FOREIGN KEY (`account_id`) REFERENCES `accounts`(`id`)); + +INSERT INTO accounts VALUES('testAccountId','','2024-10-02 16:01:38.210014+02:00','test.com','private',1,'testNetworkIdentifier','{"IP":"100.64.0.0","Mask":"//8AAA=="}','',0,'[]',0,86400000000000,0,0,0,'',NULL,NULL,NULL); +INSERT INTO users VALUES('testUserId','testAccountId','admin',0,0,'','[]',0,'0001-01-01 00:00:00+00:00','2024-10-02 16:01:38.210678+02:00','api',0,''); +INSERT INTO peers VALUES('testPeerId','testAccountId','5rvhvriKJZ3S9oxYToVj5TzDM9u9y8cxg7htIMWlYAg=','72546A29-6BC8-4311-BCFC-9CDBF33F1A48','"100.64.114.31"','f2a34f6a4731','linux','Linux','11','unknown','Debian GNU/Linux','','0.12.0','','',NULL,'','','','{"Cloud":"","Platform":""}',NULL,'f2a34f6a4731','f2a34f6a4731','2023-03-02 09:21:02.189035775+01:00',0,0,0,'','ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAILzUUSYG/LGnV8zarb2SGN+tib/PZ+M7cL4WtTzUrTpk',0,1,'2023-03-01 19:48:19.817799698+01:00','2024-10-02 17:00:32.527947+02:00',0,'""','','',0); From 5485b68bbb7bb62e6cdb2e419a4c9b30d494ea99 Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Fri, 22 Nov 2024 18:16:43 +0100 Subject: [PATCH 05/38] add more key creation tests --- .../server/http/setupkeys_handler_test.go | 2 +- .../server/http/setupkeys_integration_test.go | 267 +++++++++++++++--- .../server/http/testdata/setup_keys.sql | 5 + management/server/setupkey.go | 2 +- 4 files changed, 227 insertions(+), 49 deletions(-) diff --git a/management/server/http/setupkeys_handler_test.go b/management/server/http/setupkeys_handler_test.go index 09256d0ea5e..b8dcae2395e 100644 --- a/management/server/http/setupkeys_handler_test.go +++ b/management/server/http/setupkeys_handler_test.go @@ -227,7 +227,7 @@ func TestSetupKeysHandlers(t *testing.T) { func assertKeys(t *testing.T, got *api.SetupKey, expected *api.SetupKey) { t.Helper() // this comparison is done manually because when converting to JSON dates formatted differently - // assert.Equal(t, got.UpdatedAt, tc.expectedSetupKey.UpdatedAt) //doesn't work + // assert.Equal(t, got.UpdatedAt, tc.expectedResponse.UpdatedAt) //doesn't work assert.WithinDurationf(t, got.UpdatedAt, expected.UpdatedAt, 0, "") assert.WithinDurationf(t, got.Expires, expected.Expires, 0, "") assert.Equal(t, got.Name, expected.Name) diff --git a/management/server/http/setupkeys_integration_test.go b/management/server/http/setupkeys_integration_test.go index eb3fc1b0186..daea699155e 100644 --- a/management/server/http/setupkeys_integration_test.go +++ b/management/server/http/setupkeys_integration_test.go @@ -33,11 +33,12 @@ const ( existingKeyName = "existingKey" ) -func Test_SetupKeys_Create_Success(t *testing.T) { +func Test_SetupKeys(t *testing.T) { + truePointer := true tt := []struct { name string expectedStatus int - expectedSetupKey *api.SetupKey + expectedResponse *api.SetupKey requestBody *api.CreateSetupKeyRequest requestType string requestPath string @@ -54,7 +55,7 @@ func Test_SetupKeys_Create_Success(t *testing.T) { UsageLimit: 0, }, expectedStatus: http.StatusOK, - expectedSetupKey: &api.SetupKey{ + expectedResponse: &api.SetupKey{ AutoGroups: []string{}, Ephemeral: false, Expires: time.Time{}, @@ -71,76 +72,184 @@ func Test_SetupKeys_Create_Success(t *testing.T) { Valid: true, }, }, + { + name: "Create Setup Key with already existing name", + requestType: http.MethodPost, + requestPath: "/api/setup-keys", + requestBody: &api.CreateSetupKeyRequest{ + AutoGroups: nil, + ExpiresIn: expiresIn, + Name: existingKeyName, + Type: "one-off", + UsageLimit: 0, + }, + expectedStatus: http.StatusOK, + expectedResponse: &api.SetupKey{ + AutoGroups: []string{}, + Ephemeral: false, + Expires: time.Time{}, + Id: "", + Key: "", + LastUsed: time.Time{}, + Name: existingKeyName, + Revoked: false, + State: "valid", + Type: "one-off", + UpdatedAt: time.Now(), + UsageLimit: 1, + UsedTimes: 0, + Valid: true, + }, + }, + { + name: "Create Setup Key as on-off with more than one usage", + requestType: http.MethodPost, + requestPath: "/api/setup-keys", + requestBody: &api.CreateSetupKeyRequest{ + AutoGroups: nil, + ExpiresIn: expiresIn, + Name: newKeyName, + Type: "one-off", + UsageLimit: 3, + }, + expectedStatus: http.StatusOK, + expectedResponse: &api.SetupKey{ + AutoGroups: []string{}, + Ephemeral: false, + Expires: time.Time{}, + Id: "", + Key: "", + LastUsed: time.Time{}, + Name: newKeyName, + Revoked: false, + State: "valid", + Type: "one-off", + UpdatedAt: time.Now(), + UsageLimit: 1, + UsedTimes: 0, + Valid: true, + }, + }, + { + name: "Create Setup Key with expiration in the past", + requestType: http.MethodPost, + requestPath: "/api/setup-keys", + requestBody: &api.CreateSetupKeyRequest{ + AutoGroups: nil, + ExpiresIn: -expiresIn, + Name: newKeyName, + Type: "one-off", + UsageLimit: 0, + }, + expectedStatus: http.StatusUnprocessableEntity, + expectedResponse: nil, + }, + { + name: "Create Setup Key with AutoGroups that do exist", + requestType: http.MethodPost, + requestPath: "/api/setup-keys", + requestBody: &api.CreateSetupKeyRequest{ + AutoGroups: []string{"testGroup"}, + ExpiresIn: expiresIn, + Name: newKeyName, + Type: "reusable", + UsageLimit: 0, + }, + expectedStatus: http.StatusOK, + expectedResponse: &api.SetupKey{ + AutoGroups: []string{"testGroup"}, + Ephemeral: false, + Expires: time.Time{}, + Id: "", + Key: "", + LastUsed: time.Time{}, + Name: newKeyName, + Revoked: false, + State: "valid", + Type: "reusable", + UpdatedAt: time.Now(), + UsageLimit: 1, + UsedTimes: 0, + Valid: true, + }, + }, + { + name: "Create Setup Key for ephemeral peers", + requestType: http.MethodPost, + requestPath: "/api/setup-keys", + requestBody: &api.CreateSetupKeyRequest{ + AutoGroups: []string{}, + ExpiresIn: expiresIn, + Name: newKeyName, + Type: "reusable", + Ephemeral: &truePointer, + UsageLimit: 1, + }, + expectedStatus: http.StatusOK, + expectedResponse: &api.SetupKey{ + AutoGroups: []string{}, + Ephemeral: true, + Expires: time.Time{}, + Id: "", + Key: "", + LastUsed: time.Time{}, + Name: newKeyName, + Revoked: false, + State: "valid", + Type: "reusable", + UpdatedAt: time.Now(), + UsageLimit: 1, + UsedTimes: 0, + Valid: true, + }, + }, + { + name: "Create Setup Key with AutoGroups that do not exist", + requestType: http.MethodPost, + requestPath: "/api/setup-keys", + requestBody: &api.CreateSetupKeyRequest{ + AutoGroups: []string{"someGroupID"}, + ExpiresIn: expiresIn, + Name: newKeyName, + Type: "reusable", + UsageLimit: 0, + }, + expectedStatus: http.StatusUnprocessableEntity, + expectedResponse: nil, + }, } for _, tc := range tt { t.Run(tc.name, func(t *testing.T) { - store, cleanup, err := server.NewTestStoreFromSQL(context.Background(), "testdata/setup_keys.sql", t.TempDir()) - if err != nil { - t.Fatalf("Failed to create test store: %v", err) - } - t.Cleanup(cleanup) - - metrics, err := telemetry.NewDefaultAppMetrics(context.Background()) - - peersUpdateManager := server.NewPeersUpdateManager(nil) - updMsg := peersUpdateManager.CreateChannel(context.Background(), testPeerId) - done := make(chan struct{}) - go func() { - peerShouldNotReceiveUpdate(t, updMsg) - close(done) - }() - - geoMock := &geolocation.GeolocationMock{} - validatorMock := server.MocIntegratedValidator{} - am, err := server.BuildManager(context.Background(), store, peersUpdateManager, nil, "", "", &activity.InMemoryEventStore{}, geoMock, false, validatorMock, metrics) - if err != nil { - t.Fatalf("Failed to create manager: %v", err) - } - - apiHandler, err := APIHandler(context.Background(), am, geoMock, &jwtclaims.JwtValidatorMock{}, metrics, AuthCfg{}, validatorMock) - if err != nil { - t.Fatalf("Failed to create API handler: %v", err) - } + apiHandler, am, done := buildApiBlackBoxWithDBState(t, "testdata/setup_keys.sql", nil) body, err := json.Marshal(tc.requestBody) if err != nil { t.Fatalf("Failed to marshal request body: %v", err) } + req := buildRequest(t, body, tc.requestType, tc.requestPath) recorder := httptest.NewRecorder() - req := httptest.NewRequest(tc.requestType, tc.requestPath, bytes.NewBuffer(body)) - req.Header.Set("Authorization", "Bearer "+"my.dummy.token") apiHandler.ServeHTTP(recorder, req) - res := recorder.Result() - defer res.Body.Close() - - content, err := io.ReadAll(res.Body) - if err != nil { - t.Fatalf("I don't know what I expected; %v", err) - } - - if status := recorder.Code; status != tc.expectedStatus { - t.Errorf("handler returned wrong status code: got %v want %v, content: %s", - status, tc.expectedStatus, string(content)) + content, noResponseExpected := readResponse(t, recorder, tc.expectedStatus) + if noResponseExpected { return } - got := &api.SetupKey{} - if err = json.Unmarshal(content, &got); err != nil { + if err := json.Unmarshal(content, &got); err != nil { t.Fatalf("Sent content is not in correct json format; %v", err) } - validateCreatedKey(t, tc.expectedSetupKey, got) + validateCreatedKey(t, tc.expectedResponse, got) key, err := am.GetSetupKey(context.Background(), testAccountId, testUserId, got.Id) if err != nil { return } - validateCreatedKey(t, tc.expectedSetupKey, toResponseBody(key)) + validateCreatedKey(t, tc.expectedResponse, toResponseBody(key)) select { case <-done: @@ -151,6 +260,70 @@ func Test_SetupKeys_Create_Success(t *testing.T) { } } +func buildApiBlackBoxWithDBState(t *testing.T, sqlFile string, expectedPeerUpdate *server.UpdateMessage) (http.Handler, server.AccountManager, chan struct{}) { + store, cleanup, err := server.NewTestStoreFromSQL(context.Background(), sqlFile, t.TempDir()) + if err != nil { + t.Fatalf("Failed to create test store: %v", err) + } + t.Cleanup(cleanup) + + metrics, err := telemetry.NewDefaultAppMetrics(context.Background()) + + peersUpdateManager := server.NewPeersUpdateManager(nil) + updMsg := peersUpdateManager.CreateChannel(context.Background(), testPeerId) + done := make(chan struct{}) + go func() { + if expectedPeerUpdate != nil { + peerShouldReceiveUpdate(t, updMsg, expectedPeerUpdate) + } else { + peerShouldNotReceiveUpdate(t, updMsg) + } + close(done) + }() + + geoMock := &geolocation.GeolocationMock{} + validatorMock := server.MocIntegratedValidator{} + am, err := server.BuildManager(context.Background(), store, peersUpdateManager, nil, "", "", &activity.InMemoryEventStore{}, geoMock, false, validatorMock, metrics) + if err != nil { + t.Fatalf("Failed to create manager: %v", err) + } + + apiHandler, err := APIHandler(context.Background(), am, geoMock, &jwtclaims.JwtValidatorMock{}, metrics, AuthCfg{}, validatorMock) + if err != nil { + t.Fatalf("Failed to create API handler: %v", err) + } + + return apiHandler, am, done +} + +func buildRequest(t *testing.T, requestBody []byte, requestType, requestPath string) *http.Request { + t.Helper() + + req := httptest.NewRequest(requestType, requestPath, bytes.NewBuffer(requestBody)) + req.Header.Set("Authorization", "Bearer "+"my.dummy.token") + + return req +} + +func readResponse(t *testing.T, recorder *httptest.ResponseRecorder, expectedStatus int) ([]byte, bool) { + t.Helper() + + res := recorder.Result() + defer res.Body.Close() + + content, err := io.ReadAll(res.Body) + if err != nil { + t.Fatalf("Failed to read response body: %v", err) + } + + if status := recorder.Code; status != expectedStatus { + t.Fatalf("handler returned wrong status code: got %v want %v, content: %s", + status, expectedStatus, string(content)) + } + + return content, expectedStatus != http.StatusOK +} + func validateCreatedKey(t *testing.T, expectedKey *api.SetupKey, got *api.SetupKey) { t.Helper() diff --git a/management/server/http/testdata/setup_keys.sql b/management/server/http/testdata/setup_keys.sql index f8d96c27c0d..81d845d21ad 100644 --- a/management/server/http/testdata/setup_keys.sql +++ b/management/server/http/testdata/setup_keys.sql @@ -2,7 +2,12 @@ CREATE TABLE `accounts` (`id` text,`created_by` text,`created_at` datetime,`doma CREATE TABLE `setup_keys` (`id` text,`account_id` text,`key` text,`name` text,`type` text,`created_at` datetime,`expires_at` datetime,`updated_at` datetime,`revoked` numeric,`used_times` integer,`last_used` datetime,`auto_groups` text,`usage_limit` integer,`ephemeral` numeric,PRIMARY KEY (`id`),CONSTRAINT `fk_accounts_setup_keys_g` FOREIGN KEY (`account_id`) REFERENCES `accounts`(`id`)); CREATE TABLE `users` (`id` text,`account_id` text,`role` text,`is_service_user` numeric,`non_deletable` numeric,`service_user_name` text,`auto_groups` text,`blocked` numeric,`last_login` datetime,`created_at` datetime,`issued` text DEFAULT "api",`integration_ref_id` integer,`integration_ref_integration_type` text,PRIMARY KEY (`id`),CONSTRAINT `fk_accounts_users_g` FOREIGN KEY (`account_id`) REFERENCES `accounts`(`id`)); CREATE TABLE `peers` (`id` text,`account_id` text,`key` text,`setup_key` text,`ip` text,`meta_hostname` text,`meta_go_os` text,`meta_kernel` text,`meta_core` text,`meta_platform` text,`meta_os` text,`meta_os_version` text,`meta_wt_version` text,`meta_ui_version` text,`meta_kernel_version` text,`meta_network_addresses` text,`meta_system_serial_number` text,`meta_system_product_name` text,`meta_system_manufacturer` text,`meta_environment` text,`meta_files` text,`name` text,`dns_label` text,`peer_status_last_seen` datetime,`peer_status_connected` numeric,`peer_status_login_expired` numeric,`peer_status_requires_approval` numeric,`user_id` text,`ssh_key` text,`ssh_enabled` numeric,`login_expiration_enabled` numeric,`last_login` datetime,`created_at` datetime,`ephemeral` numeric,`location_connection_ip` text,`location_country_code` text,`location_city_name` text,`location_geo_name_id` integer,PRIMARY KEY (`id`),CONSTRAINT `fk_accounts_peers_g` FOREIGN KEY (`account_id`) REFERENCES `accounts`(`id`)); +CREATE TABLE `groups` (`id` text,`account_id` text,`name` text,`issued` text,`peers` text,`integration_ref_id` integer,`integration_ref_integration_type` text,PRIMARY KEY (`id`),CONSTRAINT `fk_accounts_groups_g` FOREIGN KEY (`account_id`) REFERENCES `accounts`(`id`)); INSERT INTO accounts VALUES('testAccountId','','2024-10-02 16:01:38.210014+02:00','test.com','private',1,'testNetworkIdentifier','{"IP":"100.64.0.0","Mask":"//8AAA=="}','',0,'[]',0,86400000000000,0,0,0,'',NULL,NULL,NULL); INSERT INTO users VALUES('testUserId','testAccountId','admin',0,0,'','[]',0,'0001-01-01 00:00:00+00:00','2024-10-02 16:01:38.210678+02:00','api',0,''); INSERT INTO peers VALUES('testPeerId','testAccountId','5rvhvriKJZ3S9oxYToVj5TzDM9u9y8cxg7htIMWlYAg=','72546A29-6BC8-4311-BCFC-9CDBF33F1A48','"100.64.114.31"','f2a34f6a4731','linux','Linux','11','unknown','Debian GNU/Linux','','0.12.0','','',NULL,'','','','{"Cloud":"","Platform":""}',NULL,'f2a34f6a4731','f2a34f6a4731','2023-03-02 09:21:02.189035775+01:00',0,0,0,'','ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAILzUUSYG/LGnV8zarb2SGN+tib/PZ+M7cL4WtTzUrTpk',0,1,'2023-03-01 19:48:19.817799698+01:00','2024-10-02 17:00:32.527947+02:00',0,'""','','',0); +INSERT INTO "groups" VALUES('testGroup','testAccountId','testGroupName','api','[]',0,''); + +INSERT INTO setup_keys VALUES('testKey','testAccountId','testKey','existingKey','reusable','2021-08-19 20:46:20.005936822+02:00','2321-09-18 20:46:20.005936822+02:00','2021-08-19 20:46:20.005936822+02:00',0,0,'0001-01-01 00:00:00+00:00','[]',0,0); + diff --git a/management/server/setupkey.go b/management/server/setupkey.go index cae0dfecbde..5455609275a 100644 --- a/management/server/setupkey.go +++ b/management/server/setupkey.go @@ -248,7 +248,7 @@ func (am *DefaultAccountManager) CreateSetupKey(ctx context.Context, accountID s err = am.Store.ExecuteInTransaction(ctx, func(transaction Store) error { if err = validateSetupKeyAutoGroups(ctx, transaction, accountID, autoGroups); err != nil { - return err + return status.Errorf(status.InvalidArgument, "invalid auto groups: %v", err) } setupKey, plainKey = GenerateSetupKey(keyName, keyType, expiresIn, autoGroups, usageLimit, ephemeral) From b3f09598ac0faf04ff96637f8fb39676f4e936ec Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Fri, 22 Nov 2024 19:39:34 +0100 Subject: [PATCH 06/38] add all setup keys tests --- .../server/http/setupkeys_integration_test.go | 650 +++++++++++++++++- .../server/http/testdata/setup_keys.sql | 9 +- management/server/setupkey.go | 2 +- 3 files changed, 648 insertions(+), 13 deletions(-) diff --git a/management/server/http/setupkeys_integration_test.go b/management/server/http/setupkeys_integration_test.go index daea699155e..dcbe13a5b71 100644 --- a/management/server/http/setupkeys_integration_test.go +++ b/management/server/http/setupkeys_integration_test.go @@ -9,6 +9,8 @@ import ( "io" "net/http" "net/http/httptest" + "sort" + "strings" "testing" "time" @@ -26,14 +28,19 @@ const ( testAccountId = "testUserId" testUserId = "testAccountId" testPeerId = "testPeerId" + testGroupId = "testGroupId" + testKeyId = "testKeyId" - newKeyName = "newKey" - expiresIn = 3600 + newKeyName = "newKey" + newGroupId = "newGroupId" + expiresIn = 3600 + revokedKeyId = "revokedKeyId" + expiredKeyId = "expiredKeyId" existingKeyName = "existingKey" ) -func Test_SetupKeys(t *testing.T) { +func Test_SetupKeys_Create(t *testing.T) { truePointer := true tt := []struct { name string @@ -149,15 +156,15 @@ func Test_SetupKeys(t *testing.T) { requestType: http.MethodPost, requestPath: "/api/setup-keys", requestBody: &api.CreateSetupKeyRequest{ - AutoGroups: []string{"testGroup"}, + AutoGroups: []string{testGroupId}, ExpiresIn: expiresIn, Name: newKeyName, Type: "reusable", - UsageLimit: 0, + UsageLimit: 1, }, expectedStatus: http.StatusOK, expectedResponse: &api.SetupKey{ - AutoGroups: []string{"testGroup"}, + AutoGroups: []string{testGroupId}, Ephemeral: false, Expires: time.Time{}, Id: "", @@ -217,6 +224,35 @@ func Test_SetupKeys(t *testing.T) { expectedStatus: http.StatusUnprocessableEntity, expectedResponse: nil, }, + { + name: "Create Setup Key", + requestType: http.MethodPost, + requestPath: "/api/setup-keys", + requestBody: &api.CreateSetupKeyRequest{ + AutoGroups: nil, + ExpiresIn: expiresIn, + Name: newKeyName, + Type: "reusable", + UsageLimit: 0, + }, + expectedStatus: http.StatusOK, + expectedResponse: &api.SetupKey{ + AutoGroups: []string{}, + Ephemeral: false, + Expires: time.Time{}, + Id: "", + Key: "", + LastUsed: time.Time{}, + Name: newKeyName, + Revoked: false, + State: "valid", + Type: "reusable", + UpdatedAt: time.Now(), + UsageLimit: 0, + UsedTimes: 0, + Valid: true, + }, + }, } for _, tc := range tt { @@ -260,6 +296,600 @@ func Test_SetupKeys(t *testing.T) { } } +func Test_SetupKeys_Update(t *testing.T) { + tt := []struct { + name string + expectedStatus int + expectedResponse *api.SetupKey + requestBody *api.SetupKeyRequest + requestType string + requestPath string + requestId string + }{ + { + name: "Add existing Group to existing Setup Key", + requestType: http.MethodPut, + requestPath: "/api/setup-keys/{id}", + requestId: testKeyId, + requestBody: &api.SetupKeyRequest{ + AutoGroups: []string{testGroupId, newGroupId}, + Revoked: false, + }, + expectedStatus: http.StatusOK, + expectedResponse: &api.SetupKey{ + AutoGroups: []string{testGroupId, newGroupId}, + Ephemeral: false, + Expires: time.Time{}, + Id: "", + Key: "", + LastUsed: time.Time{}, + Name: existingKeyName, + Revoked: false, + State: "valid", + Type: "one-off", + UpdatedAt: time.Now(), + UsageLimit: 1, + UsedTimes: 0, + Valid: true, + }, + }, + { + name: "Add non-existing Group to existing Setup Key", + requestType: http.MethodPut, + requestPath: "/api/setup-keys/{id}", + requestId: testKeyId, + requestBody: &api.SetupKeyRequest{ + AutoGroups: []string{testGroupId, "someGroupId"}, + Revoked: false, + }, + expectedStatus: http.StatusUnprocessableEntity, + expectedResponse: nil, + }, + { + name: "Add existing Group to non-existing Setup Key", + requestType: http.MethodPut, + requestPath: "/api/setup-keys/{id}", + requestId: "someId", + requestBody: &api.SetupKeyRequest{ + AutoGroups: []string{testGroupId, newGroupId}, + Revoked: false, + }, + expectedStatus: http.StatusNotFound, + expectedResponse: nil, + }, + { + name: "Remove existing Group from existing Setup Key", + requestType: http.MethodPut, + requestPath: "/api/setup-keys/{id}", + requestId: testKeyId, + requestBody: &api.SetupKeyRequest{ + AutoGroups: []string{}, + Revoked: false, + }, + expectedStatus: http.StatusOK, + expectedResponse: &api.SetupKey{ + AutoGroups: []string{}, + Ephemeral: false, + Expires: time.Time{}, + Id: "", + Key: "", + LastUsed: time.Time{}, + Name: existingKeyName, + Revoked: false, + State: "valid", + Type: "one-off", + UpdatedAt: time.Now(), + UsageLimit: 1, + UsedTimes: 0, + Valid: true, + }, + }, + { + name: "Remove existing Group to non-existing Setup Key", + requestType: http.MethodPut, + requestPath: "/api/setup-keys/{id}", + requestId: "someID", + requestBody: &api.SetupKeyRequest{ + AutoGroups: []string{}, + Revoked: false, + }, + expectedStatus: http.StatusNotFound, + expectedResponse: nil, + }, + { + name: "Revoke existing valid Setup Key", + requestType: http.MethodPut, + requestPath: "/api/setup-keys/{id}", + requestId: testKeyId, + requestBody: &api.SetupKeyRequest{ + AutoGroups: []string{testGroupId}, + Revoked: true, + }, + expectedStatus: http.StatusOK, + expectedResponse: &api.SetupKey{ + AutoGroups: []string{testGroupId}, + Ephemeral: false, + Expires: time.Time{}, + Id: "", + Key: "", + LastUsed: time.Time{}, + Name: existingKeyName, + Revoked: true, + State: "revoked", + Type: "one-off", + UpdatedAt: time.Now(), + UsageLimit: 1, + UsedTimes: 0, + Valid: false, + }, + }, + { + name: "Revoke existing revoked Setup Key", + requestType: http.MethodPut, + requestPath: "/api/setup-keys/{id}", + requestId: revokedKeyId, + requestBody: &api.SetupKeyRequest{ + AutoGroups: []string{testGroupId}, + Revoked: true, + }, + expectedStatus: http.StatusOK, + expectedResponse: &api.SetupKey{ + AutoGroups: []string{testGroupId}, + Ephemeral: false, + Expires: time.Time{}, + Id: "", + Key: "", + LastUsed: time.Time{}, + Name: existingKeyName, + Revoked: true, + State: "revoked", + Type: "reusable", + UpdatedAt: time.Now(), + UsageLimit: 3, + UsedTimes: 0, + Valid: false, + }, + }, + { + name: "Un-Revoke existing revoked Setup Key", + requestType: http.MethodPut, + requestPath: "/api/setup-keys/{id}", + requestId: revokedKeyId, + requestBody: &api.SetupKeyRequest{ + AutoGroups: []string{testGroupId}, + Revoked: false, + }, + expectedStatus: http.StatusUnprocessableEntity, + expectedResponse: nil, + }, + { + name: "Revoke existing expired Setup Key", + requestType: http.MethodPut, + requestPath: "/api/setup-keys/{id}", + requestId: expiredKeyId, + requestBody: &api.SetupKeyRequest{ + AutoGroups: []string{testGroupId}, + Revoked: true, + }, + expectedStatus: http.StatusOK, + expectedResponse: &api.SetupKey{ + AutoGroups: []string{testGroupId}, + Ephemeral: true, + Expires: time.Time{}, + Id: "", + Key: "", + LastUsed: time.Time{}, + Name: existingKeyName, + Revoked: true, + State: "expired", + Type: "reusable", + UpdatedAt: time.Now(), + UsageLimit: 5, + UsedTimes: 1, + Valid: false, + }, + }, + } + + for _, tc := range tt { + t.Run(tc.name, func(t *testing.T) { + apiHandler, am, done := buildApiBlackBoxWithDBState(t, "testdata/setup_keys.sql", nil) + + body, err := json.Marshal(tc.requestBody) + if err != nil { + t.Fatalf("Failed to marshal request body: %v", err) + } + + req := buildRequest(t, body, tc.requestType, strings.Replace(tc.requestPath, "{id}", tc.requestId, 1)) + + recorder := httptest.NewRecorder() + + apiHandler.ServeHTTP(recorder, req) + + content, noResponseExpected := readResponse(t, recorder, tc.expectedStatus) + if noResponseExpected { + return + } + got := &api.SetupKey{} + if err := json.Unmarshal(content, &got); err != nil { + t.Fatalf("Sent content is not in correct json format; %v", err) + } + + validateCreatedKey(t, tc.expectedResponse, got) + + key, err := am.GetSetupKey(context.Background(), testAccountId, testUserId, got.Id) + if err != nil { + return + } + + validateCreatedKey(t, tc.expectedResponse, toResponseBody(key)) + + select { + case <-done: + case <-time.After(time.Second): + t.Error("timeout waiting for peerShouldNotReceiveUpdate") + } + }) + } +} + +func Test_SetupKeys_Get(t *testing.T) { + tt := []struct { + name string + expectedStatus int + expectedResponse *api.SetupKey + requestType string + requestPath string + requestId string + }{ + { + name: "Get existing valid Setup Key", + requestType: http.MethodGet, + requestPath: "/api/setup-keys/{id}", + requestId: testKeyId, + expectedStatus: http.StatusOK, + expectedResponse: &api.SetupKey{ + AutoGroups: []string{testGroupId}, + Ephemeral: false, + Expires: time.Time{}, + Id: "", + Key: "", + LastUsed: time.Time{}, + Name: existingKeyName, + Revoked: false, + State: "valid", + Type: "one-off", + UpdatedAt: time.Date(2021, time.August, 19, 20, 46, 20, 5936822, time.Local), + UsageLimit: 1, + UsedTimes: 0, + Valid: true, + }, + }, + { + name: "Get existing expired Setup Key", + requestType: http.MethodGet, + requestPath: "/api/setup-keys/{id}", + requestId: expiredKeyId, + expectedStatus: http.StatusOK, + expectedResponse: &api.SetupKey{ + AutoGroups: []string{testGroupId}, + Ephemeral: true, + Expires: time.Time{}, + Id: "", + Key: "", + LastUsed: time.Time{}, + Name: existingKeyName, + Revoked: false, + State: "expired", + Type: "reusable", + UpdatedAt: time.Date(2021, time.August, 19, 20, 46, 20, 5936822, time.Local), + UsageLimit: 5, + UsedTimes: 1, + Valid: false, + }, + }, + { + name: "Get existing revoked Setup Key", + requestType: http.MethodGet, + requestPath: "/api/setup-keys/{id}", + requestId: revokedKeyId, + expectedStatus: http.StatusOK, + expectedResponse: &api.SetupKey{ + AutoGroups: []string{testGroupId}, + Ephemeral: false, + Expires: time.Time{}, + Id: "", + Key: "", + LastUsed: time.Time{}, + Name: existingKeyName, + Revoked: true, + State: "revoked", + Type: "reusable", + UpdatedAt: time.Date(2021, time.August, 19, 20, 46, 20, 5936822, time.Local), + UsageLimit: 3, + UsedTimes: 0, + Valid: false, + }, + }, + { + name: "Get non-existing Setup Key", + requestType: http.MethodGet, + requestPath: "/api/setup-keys/{id}", + requestId: "someId", + expectedStatus: http.StatusNotFound, + expectedResponse: nil, + }, + } + + for _, tc := range tt { + t.Run(tc.name, func(t *testing.T) { + apiHandler, am, done := buildApiBlackBoxWithDBState(t, "testdata/setup_keys.sql", nil) + + req := buildRequest(t, []byte{}, tc.requestType, strings.Replace(tc.requestPath, "{id}", tc.requestId, 1)) + + recorder := httptest.NewRecorder() + + apiHandler.ServeHTTP(recorder, req) + + content, noResponseExpected := readResponse(t, recorder, tc.expectedStatus) + if noResponseExpected { + return + } + got := &api.SetupKey{} + if err := json.Unmarshal(content, &got); err != nil { + t.Fatalf("Sent content is not in correct json format; %v", err) + } + + validateCreatedKey(t, tc.expectedResponse, got) + + key, err := am.GetSetupKey(context.Background(), testAccountId, testUserId, got.Id) + if err != nil { + return + } + + validateCreatedKey(t, tc.expectedResponse, toResponseBody(key)) + + select { + case <-done: + case <-time.After(time.Second): + t.Error("timeout waiting for peerShouldNotReceiveUpdate") + } + }) + } +} + +func Test_SetupKeys_GetAll(t *testing.T) { + tt := []struct { + name string + expectedStatus int + expectedResponse []*api.SetupKey + requestType string + requestPath string + }{ + { + name: "Get all Setup Keys", + requestType: http.MethodGet, + requestPath: "/api/setup-keys", + expectedStatus: http.StatusOK, + expectedResponse: []*api.SetupKey{ + { + AutoGroups: []string{testGroupId}, + Ephemeral: false, + Expires: time.Time{}, + Id: "", + Key: "", + LastUsed: time.Time{}, + Name: existingKeyName, + Revoked: false, + State: "valid", + Type: "one-off", + UpdatedAt: time.Date(2021, time.August, 19, 20, 46, 20, 5936822, time.Local), + UsageLimit: 1, + UsedTimes: 0, + Valid: true, + }, + { + AutoGroups: []string{testGroupId}, + Ephemeral: false, + Expires: time.Time{}, + Id: "", + Key: "", + LastUsed: time.Time{}, + Name: existingKeyName, + Revoked: true, + State: "revoked", + Type: "reusable", + UpdatedAt: time.Date(2021, time.August, 19, 20, 46, 20, 5936822, time.Local), + UsageLimit: 3, + UsedTimes: 0, + Valid: false, + }, + { + AutoGroups: []string{testGroupId}, + Ephemeral: true, + Expires: time.Time{}, + Id: "", + Key: "", + LastUsed: time.Time{}, + Name: existingKeyName, + Revoked: false, + State: "expired", + Type: "reusable", + UpdatedAt: time.Date(2021, time.August, 19, 20, 46, 20, 5936822, time.Local), + UsageLimit: 5, + UsedTimes: 1, + Valid: false, + }, + }, + }, + } + + for _, tc := range tt { + t.Run(tc.name, func(t *testing.T) { + apiHandler, am, done := buildApiBlackBoxWithDBState(t, "testdata/setup_keys.sql", nil) + + req := buildRequest(t, []byte{}, tc.requestType, tc.requestPath) + + recorder := httptest.NewRecorder() + + apiHandler.ServeHTTP(recorder, req) + + content, noResponseExpected := readResponse(t, recorder, tc.expectedStatus) + if noResponseExpected { + return + } + got := []api.SetupKey{} + if err := json.Unmarshal(content, &got); err != nil { + t.Fatalf("Sent content is not in correct json format; %v", err) + } + + sort.Slice(got, func(i, j int) bool { + return got[i].UsageLimit < got[j].UsageLimit + }) + + sort.Slice(tc.expectedResponse, func(i, j int) bool { + return tc.expectedResponse[i].UsageLimit < tc.expectedResponse[j].UsageLimit + }) + + for i, _ := range tc.expectedResponse { + validateCreatedKey(t, tc.expectedResponse[i], &got[i]) + + key, err := am.GetSetupKey(context.Background(), testAccountId, testUserId, got[i].Id) + if err != nil { + return + } + + validateCreatedKey(t, tc.expectedResponse[i], toResponseBody(key)) + } + + select { + case <-done: + case <-time.After(time.Second): + t.Error("timeout waiting for peerShouldNotReceiveUpdate") + } + }) + } +} + +func Test_SetupKeys_Delete(t *testing.T) { + tt := []struct { + name string + expectedStatus int + expectedResponse *api.SetupKey + requestType string + requestPath string + requestId string + }{ + { + name: "Delete existing valid Setup Key", + requestType: http.MethodDelete, + requestPath: "/api/setup-keys/{id}", + requestId: testKeyId, + expectedStatus: http.StatusOK, + expectedResponse: &api.SetupKey{ + AutoGroups: []string{testGroupId}, + Ephemeral: false, + Expires: time.Time{}, + Id: "", + Key: "", + LastUsed: time.Time{}, + Name: existingKeyName, + Revoked: false, + State: "valid", + Type: "one-off", + UpdatedAt: time.Date(2021, time.August, 19, 20, 46, 20, 5936822, time.Local), + UsageLimit: 1, + UsedTimes: 0, + Valid: true, + }, + }, + { + name: "Delete existing expired Setup Key", + requestType: http.MethodDelete, + requestPath: "/api/setup-keys/{id}", + requestId: expiredKeyId, + expectedStatus: http.StatusOK, + expectedResponse: &api.SetupKey{ + AutoGroups: []string{testGroupId}, + Ephemeral: true, + Expires: time.Time{}, + Id: "", + Key: "", + LastUsed: time.Time{}, + Name: existingKeyName, + Revoked: false, + State: "expired", + Type: "reusable", + UpdatedAt: time.Date(2021, time.August, 19, 20, 46, 20, 5936822, time.Local), + UsageLimit: 5, + UsedTimes: 1, + Valid: false, + }, + }, + { + name: "Delete existing revoked Setup Key", + requestType: http.MethodDelete, + requestPath: "/api/setup-keys/{id}", + requestId: revokedKeyId, + expectedStatus: http.StatusOK, + expectedResponse: &api.SetupKey{ + AutoGroups: []string{testGroupId}, + Ephemeral: false, + Expires: time.Time{}, + Id: "", + Key: "", + LastUsed: time.Time{}, + Name: existingKeyName, + Revoked: true, + State: "revoked", + Type: "reusable", + UpdatedAt: time.Date(2021, time.August, 19, 20, 46, 20, 5936822, time.Local), + UsageLimit: 3, + UsedTimes: 0, + Valid: false, + }, + }, + { + name: "Delete non-existing Setup Key", + requestType: http.MethodDelete, + requestPath: "/api/setup-keys/{id}", + requestId: "someId", + expectedStatus: http.StatusNotFound, + expectedResponse: nil, + }, + } + + for _, tc := range tt { + t.Run(tc.name, func(t *testing.T) { + apiHandler, am, done := buildApiBlackBoxWithDBState(t, "testdata/setup_keys.sql", nil) + + req := buildRequest(t, []byte{}, tc.requestType, strings.Replace(tc.requestPath, "{id}", tc.requestId, 1)) + + recorder := httptest.NewRecorder() + + apiHandler.ServeHTTP(recorder, req) + + content, noResponseExpected := readResponse(t, recorder, tc.expectedStatus) + if noResponseExpected { + return + } + got := &api.SetupKey{} + if err := json.Unmarshal(content, &got); err != nil { + t.Fatalf("Sent content is not in correct json format; %v", err) + } + + _, err := am.GetSetupKey(context.Background(), testAccountId, testUserId, got.Id) + assert.Errorf(t, err, "Expected error when trying to get deleted key") + + select { + case <-done: + case <-time.After(time.Second): + t.Error("timeout waiting for peerShouldNotReceiveUpdate") + } + }) + } +} + func buildApiBlackBoxWithDBState(t *testing.T, sqlFile string, expectedPeerUpdate *server.UpdateMessage) (http.Handler, server.AccountManager, chan struct{}) { store, cleanup, err := server.NewTestStoreFromSQL(context.Background(), sqlFile, t.TempDir()) if err != nil { @@ -327,18 +957,20 @@ func readResponse(t *testing.T, recorder *httptest.ResponseRecorder, expectedSta func validateCreatedKey(t *testing.T, expectedKey *api.SetupKey, got *api.SetupKey) { t.Helper() - if got.Expires.After(time.Now().Add(-1*time.Minute)) && got.Expires.Before(time.Now().Add(expiresIn*time.Second)) { + if got.Expires.After(time.Now().Add(-1*time.Minute)) && got.Expires.Before(time.Now().Add(expiresIn*time.Second)) || + got.Expires.After(time.Date(2300, 01, 01, 0, 0, 0, 0, time.Local)) || + got.Expires.Before(time.Date(1950, 01, 01, 0, 0, 0, 0, time.Local)) { got.Expires = time.Time{} expectedKey.Expires = time.Time{} } if got.Id == "" { - t.Error("Expected key to have an ID") + t.Fatalf("Expected key to have an ID") } got.Id = "" if got.Key == "" { - t.Error("Expected key to have a key") + t.Fatalf("Expected key to have a key") } got.Key = "" diff --git a/management/server/http/testdata/setup_keys.sql b/management/server/http/testdata/setup_keys.sql index 81d845d21ad..6fe374a6e63 100644 --- a/management/server/http/testdata/setup_keys.sql +++ b/management/server/http/testdata/setup_keys.sql @@ -1,5 +1,5 @@ CREATE TABLE `accounts` (`id` text,`created_by` text,`created_at` datetime,`domain` text,`domain_category` text,`is_domain_primary_account` numeric,`network_identifier` text,`network_net` text,`network_dns` text,`network_serial` integer,`dns_settings_disabled_management_groups` text,`settings_peer_login_expiration_enabled` numeric,`settings_peer_login_expiration` integer,`settings_regular_users_view_blocked` numeric,`settings_groups_propagation_enabled` numeric,`settings_jwt_groups_enabled` numeric,`settings_jwt_groups_claim_name` text,`settings_jwt_allow_groups` text,`settings_extra_peer_approval_enabled` numeric,`settings_extra_integrated_validator_groups` text,PRIMARY KEY (`id`)); -CREATE TABLE `setup_keys` (`id` text,`account_id` text,`key` text,`name` text,`type` text,`created_at` datetime,`expires_at` datetime,`updated_at` datetime,`revoked` numeric,`used_times` integer,`last_used` datetime,`auto_groups` text,`usage_limit` integer,`ephemeral` numeric,PRIMARY KEY (`id`),CONSTRAINT `fk_accounts_setup_keys_g` FOREIGN KEY (`account_id`) REFERENCES `accounts`(`id`)); +CREATE TABLE `setup_keys` (`id` text,`account_id` text,`key` text,`key_secret` text,`name` text,`type` text,`created_at` datetime,`expires_at` datetime,`updated_at` datetime,`revoked` numeric,`used_times` integer,`last_used` datetime,`auto_groups` text,`usage_limit` integer,`ephemeral` numeric,PRIMARY KEY (`id`),CONSTRAINT `fk_accounts_setup_keys_g` FOREIGN KEY (`account_id`) REFERENCES `accounts`(`id`)); CREATE TABLE `users` (`id` text,`account_id` text,`role` text,`is_service_user` numeric,`non_deletable` numeric,`service_user_name` text,`auto_groups` text,`blocked` numeric,`last_login` datetime,`created_at` datetime,`issued` text DEFAULT "api",`integration_ref_id` integer,`integration_ref_integration_type` text,PRIMARY KEY (`id`),CONSTRAINT `fk_accounts_users_g` FOREIGN KEY (`account_id`) REFERENCES `accounts`(`id`)); CREATE TABLE `peers` (`id` text,`account_id` text,`key` text,`setup_key` text,`ip` text,`meta_hostname` text,`meta_go_os` text,`meta_kernel` text,`meta_core` text,`meta_platform` text,`meta_os` text,`meta_os_version` text,`meta_wt_version` text,`meta_ui_version` text,`meta_kernel_version` text,`meta_network_addresses` text,`meta_system_serial_number` text,`meta_system_product_name` text,`meta_system_manufacturer` text,`meta_environment` text,`meta_files` text,`name` text,`dns_label` text,`peer_status_last_seen` datetime,`peer_status_connected` numeric,`peer_status_login_expired` numeric,`peer_status_requires_approval` numeric,`user_id` text,`ssh_key` text,`ssh_enabled` numeric,`login_expiration_enabled` numeric,`last_login` datetime,`created_at` datetime,`ephemeral` numeric,`location_connection_ip` text,`location_country_code` text,`location_city_name` text,`location_geo_name_id` integer,PRIMARY KEY (`id`),CONSTRAINT `fk_accounts_peers_g` FOREIGN KEY (`account_id`) REFERENCES `accounts`(`id`)); CREATE TABLE `groups` (`id` text,`account_id` text,`name` text,`issued` text,`peers` text,`integration_ref_id` integer,`integration_ref_integration_type` text,PRIMARY KEY (`id`),CONSTRAINT `fk_accounts_groups_g` FOREIGN KEY (`account_id`) REFERENCES `accounts`(`id`)); @@ -7,7 +7,10 @@ CREATE TABLE `groups` (`id` text,`account_id` text,`name` text,`issued` text,`pe INSERT INTO accounts VALUES('testAccountId','','2024-10-02 16:01:38.210014+02:00','test.com','private',1,'testNetworkIdentifier','{"IP":"100.64.0.0","Mask":"//8AAA=="}','',0,'[]',0,86400000000000,0,0,0,'',NULL,NULL,NULL); INSERT INTO users VALUES('testUserId','testAccountId','admin',0,0,'','[]',0,'0001-01-01 00:00:00+00:00','2024-10-02 16:01:38.210678+02:00','api',0,''); INSERT INTO peers VALUES('testPeerId','testAccountId','5rvhvriKJZ3S9oxYToVj5TzDM9u9y8cxg7htIMWlYAg=','72546A29-6BC8-4311-BCFC-9CDBF33F1A48','"100.64.114.31"','f2a34f6a4731','linux','Linux','11','unknown','Debian GNU/Linux','','0.12.0','','',NULL,'','','','{"Cloud":"","Platform":""}',NULL,'f2a34f6a4731','f2a34f6a4731','2023-03-02 09:21:02.189035775+01:00',0,0,0,'','ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAILzUUSYG/LGnV8zarb2SGN+tib/PZ+M7cL4WtTzUrTpk',0,1,'2023-03-01 19:48:19.817799698+01:00','2024-10-02 17:00:32.527947+02:00',0,'""','','',0); -INSERT INTO "groups" VALUES('testGroup','testAccountId','testGroupName','api','[]',0,''); +INSERT INTO "groups" VALUES('testGroupId','testAccountId','testGroupName','api','[]',0,''); +INSERT INTO "groups" VALUES('newGroupId','testAccountId','newGroupName','api','[]',0,''); -INSERT INTO setup_keys VALUES('testKey','testAccountId','testKey','existingKey','reusable','2021-08-19 20:46:20.005936822+02:00','2321-09-18 20:46:20.005936822+02:00','2021-08-19 20:46:20.005936822+02:00',0,0,'0001-01-01 00:00:00+00:00','[]',0,0); +INSERT INTO setup_keys VALUES('testKeyId','testAccountId','testKey','testK****','existingKey','one-off','2021-08-19 20:46:20.005936822+02:00','2321-09-18 20:46:20.005936822+02:00','2021-08-19 20:46:20.005936822+02:00',0,0,'0001-01-01 00:00:00+00:00','["testGroupId"]',1,0); +INSERT INTO setup_keys VALUES('revokedKeyId','testAccountId','revokedKey','testK****','existingKey','reusable','2021-08-19 20:46:20.005936822+02:00','2321-09-18 20:46:20.005936822+02:00','2021-08-19 20:46:20.005936822+02:00',1,0,'0001-01-01 00:00:00+00:00','["testGroupId"]',3,0); +INSERT INTO setup_keys VALUES('expiredKeyId','testAccountId','expiredKey','testK****','existingKey','reusable','2021-08-19 20:46:20.005936822+02:00','1921-09-18 20:46:20.005936822+02:00','2021-08-19 20:46:20.005936822+02:00',0,1,'0001-01-01 00:00:00+00:00','["testGroupId"]',5,1); diff --git a/management/server/setupkey.go b/management/server/setupkey.go index b2b3e9bf4c3..335a7874e29 100644 --- a/management/server/setupkey.go +++ b/management/server/setupkey.go @@ -305,7 +305,7 @@ func (am *DefaultAccountManager) SaveSetupKey(ctx context.Context, accountID str err = am.Store.ExecuteInTransaction(ctx, func(transaction Store) error { if err = validateSetupKeyAutoGroups(ctx, transaction, accountID, keyToSave.AutoGroups); err != nil { - return err + return status.Errorf(status.InvalidArgument, "invalid auto groups: %v", err) } oldKey, err = transaction.GetSetupKeyByID(ctx, LockingStrengthShare, accountID, keyToSave.Id) From dcbadd47c0fc692311aeb97224cfc895443ab2d0 Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Fri, 22 Nov 2024 19:52:50 +0100 Subject: [PATCH 07/38] fix linter --- management/server/geolocation/geolocation_test.go | 2 +- management/server/integrated_validator.go | 3 ++- relay/client/picker_test.go | 1 + 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/management/server/geolocation/geolocation_test.go b/management/server/geolocation/geolocation_test.go index 9bdefd268ac..1980e32fc6c 100644 --- a/management/server/geolocation/geolocation_test.go +++ b/management/server/geolocation/geolocation_test.go @@ -24,7 +24,7 @@ func TestGeoLite_Lookup(t *testing.T) { db, err := openDB(filename) assert.NoError(t, err) - geo := &Geolocation{ + geo := &GeolocationImpl{ mux: sync.RWMutex{}, db: db, stopCh: make(chan struct{}), diff --git a/management/server/integrated_validator.go b/management/server/integrated_validator.go index 48ba60bfaeb..1a28d271058 100644 --- a/management/server/integrated_validator.go +++ b/management/server/integrated_validator.go @@ -114,8 +114,9 @@ func (MocIntegratedValidator) PeerDeleted(_ context.Context, _, _ string) error } func (MocIntegratedValidator) SetPeerInvalidationListener(func(accountID string)) { - + // just a dummy } func (MocIntegratedValidator) Stop(_ context.Context) { + // just a dummy } diff --git a/relay/client/picker_test.go b/relay/client/picker_test.go index 20a03e64d1b..28167c5ce3b 100644 --- a/relay/client/picker_test.go +++ b/relay/client/picker_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" "testing" + "time" ) func TestServerPicker_UnavailableServers(t *testing.T) { From f2280edecd6f0914ffaac71128ebd0c5d72333ef Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Fri, 22 Nov 2024 19:57:58 +0100 Subject: [PATCH 08/38] unexport impl structs --- management/server/geolocation/geolocation.go | 12 ++++++------ management/server/geolocation/geolocation_test.go | 2 +- management/server/jwtclaims/jwtValidator.go | 8 ++++---- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/management/server/geolocation/geolocation.go b/management/server/geolocation/geolocation.go index f209cc715ff..b1d7e7b93e0 100644 --- a/management/server/geolocation/geolocation.go +++ b/management/server/geolocation/geolocation.go @@ -21,7 +21,7 @@ type Geolocation interface { Stop() error } -type GeolocationImpl struct { +type geolocationImpl struct { mmdbPath string mux sync.RWMutex db *maxminddb.Reader @@ -93,7 +93,7 @@ func NewGeolocation(ctx context.Context, dataDir string, autoUpdate bool) (Geolo return nil, err } - geo := &GeolocationImpl{ + geo := &geolocationImpl{ mmdbPath: mmdbPath, mux: sync.RWMutex{}, db: db, @@ -120,7 +120,7 @@ func openDB(mmdbPath string) (*maxminddb.Reader, error) { return db, nil } -func (gl *GeolocationImpl) Lookup(ip net.IP) (*Record, error) { +func (gl *geolocationImpl) Lookup(ip net.IP) (*Record, error) { gl.mux.RLock() defer gl.mux.RUnlock() @@ -134,7 +134,7 @@ func (gl *GeolocationImpl) Lookup(ip net.IP) (*Record, error) { } // GetAllCountries retrieves a list of all countries. -func (gl *GeolocationImpl) GetAllCountries() ([]Country, error) { +func (gl *geolocationImpl) GetAllCountries() ([]Country, error) { allCountries, err := gl.locationDB.GetAllCountries() if err != nil { return nil, err @@ -150,7 +150,7 @@ func (gl *GeolocationImpl) GetAllCountries() ([]Country, error) { } // GetCitiesByCountry retrieves a list of cities in a specific country based on the country's ISO code. -func (gl *GeolocationImpl) GetCitiesByCountry(countryISOCode string) ([]City, error) { +func (gl *geolocationImpl) GetCitiesByCountry(countryISOCode string) ([]City, error) { allCities, err := gl.locationDB.GetCitiesByCountry(countryISOCode) if err != nil { return nil, err @@ -165,7 +165,7 @@ func (gl *GeolocationImpl) GetCitiesByCountry(countryISOCode string) ([]City, er return cities, nil } -func (gl *GeolocationImpl) Stop() error { +func (gl *geolocationImpl) Stop() error { close(gl.stopCh) if gl.db != nil { if err := gl.db.Close(); err != nil { diff --git a/management/server/geolocation/geolocation_test.go b/management/server/geolocation/geolocation_test.go index 1980e32fc6c..fecd715be07 100644 --- a/management/server/geolocation/geolocation_test.go +++ b/management/server/geolocation/geolocation_test.go @@ -24,7 +24,7 @@ func TestGeoLite_Lookup(t *testing.T) { db, err := openDB(filename) assert.NoError(t, err) - geo := &GeolocationImpl{ + geo := &geolocationImpl{ mux: sync.RWMutex{}, db: db, stopCh: make(chan struct{}), diff --git a/management/server/jwtclaims/jwtValidator.go b/management/server/jwtclaims/jwtValidator.go index ab43c7fe9a8..b7b05d5e2a1 100644 --- a/management/server/jwtclaims/jwtValidator.go +++ b/management/server/jwtclaims/jwtValidator.go @@ -76,8 +76,8 @@ type JWTValidator interface { ValidateAndParse(ctx context.Context, token string) (*jwt.Token, error) } -// JWTValidatorImpl struct to handle token validation and parsing -type JWTValidatorImpl struct { +// jwtValidatorImpl struct to handle token validation and parsing +type jwtValidatorImpl struct { options Options } @@ -142,13 +142,13 @@ func NewJWTValidator(ctx context.Context, issuer string, audienceList []string, options.UserProperty = "user" } - return &JWTValidatorImpl{ + return &jwtValidatorImpl{ options: options, }, nil } // ValidateAndParse validates the token and returns the parsed token -func (m *JWTValidatorImpl) ValidateAndParse(ctx context.Context, token string) (*jwt.Token, error) { +func (m *jwtValidatorImpl) ValidateAndParse(ctx context.Context, token string) (*jwt.Token, error) { // If the token is empty... if token == "" { // Check if it was required From 426b38a2e018e01f1f54405aac0a28f45f2916a4 Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Fri, 22 Nov 2024 20:08:52 +0100 Subject: [PATCH 09/38] unexport impl structs --- management/server/http/posture_checks_handler_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/management/server/http/posture_checks_handler_test.go b/management/server/http/posture_checks_handler_test.go index 649649a2a58..9f7996e05b7 100644 --- a/management/server/http/posture_checks_handler_test.go +++ b/management/server/http/posture_checks_handler_test.go @@ -70,7 +70,7 @@ func initPostureChecksTestData(postureChecks ...*posture.Checks) *PostureChecksH return claims.AccountId, claims.UserId, nil }, }, - geolocationManager: &geolocation.GeolocationImpl{}, + geolocationManager: &geolocation.geolocationImpl{}, claimsExtractor: jwtclaims.NewClaimsExtractor( jwtclaims.WithFromRequestContext(func(r *http.Request) jwtclaims.AuthorizationClaims { return jwtclaims.AuthorizationClaims{ From 63b000c3b70f33396eea7f294a03bf6a8d85770d Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Mon, 25 Nov 2024 15:07:58 +0100 Subject: [PATCH 10/38] add different users --- .../http/posture_checks_handler_test.go | 2 +- .../server/http/setupkeys_integration_test.go | 561 +++++++++++++----- .../server/http/testdata/setup_keys.sql | 12 +- management/server/jwtclaims/jwtValidator.go | 21 +- 4 files changed, 437 insertions(+), 159 deletions(-) diff --git a/management/server/http/posture_checks_handler_test.go b/management/server/http/posture_checks_handler_test.go index 9f7996e05b7..433205de2ae 100644 --- a/management/server/http/posture_checks_handler_test.go +++ b/management/server/http/posture_checks_handler_test.go @@ -70,7 +70,7 @@ func initPostureChecksTestData(postureChecks ...*posture.Checks) *PostureChecksH return claims.AccountId, claims.UserId, nil }, }, - geolocationManager: &geolocation.geolocationImpl{}, + geolocationManager: &geolocation.GeolocationMock{}, claimsExtractor: jwtclaims.NewClaimsExtractor( jwtclaims.WithFromRequestContext(func(r *http.Request) jwtclaims.AuthorizationClaims { return jwtclaims.AuthorizationClaims{ diff --git a/management/server/http/setupkeys_integration_test.go b/management/server/http/setupkeys_integration_test.go index dcbe13a5b71..15153391b3a 100644 --- a/management/server/http/setupkeys_integration_test.go +++ b/management/server/http/setupkeys_integration_test.go @@ -25,12 +25,20 @@ import ( ) const ( - testAccountId = "testUserId" - testUserId = "testAccountId" + testAccountId = "testAccountId" testPeerId = "testPeerId" testGroupId = "testGroupId" testKeyId = "testKeyId" + testUserId = "testUserId" + testAdminId = "testAdminId" + testOwnerId = "testOwnerId" + testServiceUserId = "testServiceUserId" + testServiceAdminId = "testServiceAdminId" + blockedUserId = "blockedUserId" + otherUserId = "otherUserId" + invalidToken = "invalidToken" + newKeyName = "newKey" newGroupId = "newGroupId" expiresIn = 3600 @@ -42,6 +50,54 @@ const ( func Test_SetupKeys_Create(t *testing.T) { truePointer := true + + users := []struct { + name string + userId string + expectResponse bool + }{ + { + name: "Regular user", + userId: testUserId, + expectResponse: false, + }, + { + name: "Admin user", + userId: testAdminId, + expectResponse: true, + }, + { + name: "Owner user", + userId: testOwnerId, + expectResponse: true, + }, + { + name: "Regular service user", + userId: testServiceUserId, + expectResponse: false, + }, + { + name: "Admin service user", + userId: testServiceAdminId, + expectResponse: true, + }, + { + name: "Blocked user", + userId: blockedUserId, + expectResponse: false, + }, + { + name: "Other user", + userId: otherUserId, + expectResponse: false, + }, + { + name: "Invalid token", + userId: invalidToken, + expectResponse: false, + }, + } + tt := []struct { name string expectedStatus int @@ -49,6 +105,7 @@ func Test_SetupKeys_Create(t *testing.T) { requestBody *api.CreateSetupKeyRequest requestType string requestPath string + userId string }{ { name: "Create Setup Key", @@ -256,47 +313,96 @@ func Test_SetupKeys_Create(t *testing.T) { } for _, tc := range tt { - t.Run(tc.name, func(t *testing.T) { - apiHandler, am, done := buildApiBlackBoxWithDBState(t, "testdata/setup_keys.sql", nil) - - body, err := json.Marshal(tc.requestBody) - if err != nil { - t.Fatalf("Failed to marshal request body: %v", err) - } - req := buildRequest(t, body, tc.requestType, tc.requestPath) - - recorder := httptest.NewRecorder() - - apiHandler.ServeHTTP(recorder, req) - - content, noResponseExpected := readResponse(t, recorder, tc.expectedStatus) - if noResponseExpected { - return - } - got := &api.SetupKey{} - if err := json.Unmarshal(content, &got); err != nil { - t.Fatalf("Sent content is not in correct json format; %v", err) - } - - validateCreatedKey(t, tc.expectedResponse, got) - - key, err := am.GetSetupKey(context.Background(), testAccountId, testUserId, got.Id) - if err != nil { - return - } - - validateCreatedKey(t, tc.expectedResponse, toResponseBody(key)) - - select { - case <-done: - case <-time.After(time.Second): - t.Error("timeout waiting for peerShouldNotReceiveUpdate") - } - }) + for _, user := range users { + t.Run(user.name+" - "+tc.name, func(t *testing.T) { + apiHandler, am, done := buildApiBlackBoxWithDBState(t, "testdata/setup_keys.sql", nil) + + body, err := json.Marshal(tc.requestBody) + if err != nil { + t.Fatalf("Failed to marshal request body: %v", err) + } + req := buildRequest(t, body, tc.requestType, tc.requestPath, user.userId) + + recorder := httptest.NewRecorder() + + apiHandler.ServeHTTP(recorder, req) + + content, expectResponse := readResponse(t, recorder, tc.expectedStatus, user.expectResponse) + if !expectResponse { + return + } + got := &api.SetupKey{} + if err := json.Unmarshal(content, &got); err != nil { + t.Fatalf("Sent content is not in correct json format; %v", err) + } + + validateCreatedKey(t, tc.expectedResponse, got) + + key, err := am.GetSetupKey(context.Background(), testAccountId, testUserId, got.Id) + if err != nil { + return + } + + validateCreatedKey(t, tc.expectedResponse, toResponseBody(key)) + + select { + case <-done: + case <-time.After(time.Second): + t.Error("timeout waiting for peerShouldNotReceiveUpdate") + } + }) + } } } func Test_SetupKeys_Update(t *testing.T) { + users := []struct { + name string + userId string + expectResponse bool + }{ + { + name: "Regular user", + userId: testUserId, + expectResponse: false, + }, + { + name: "Admin user", + userId: testAdminId, + expectResponse: true, + }, + { + name: "Owner user", + userId: testOwnerId, + expectResponse: true, + }, + { + name: "Regular service user", + userId: testServiceUserId, + expectResponse: false, + }, + { + name: "Admin service user", + userId: testServiceAdminId, + expectResponse: true, + }, + { + name: "Blocked user", + userId: blockedUserId, + expectResponse: false, + }, + { + name: "Other user", + userId: otherUserId, + expectResponse: false, + }, + { + name: "Invalid token", + userId: invalidToken, + expectResponse: false, + }, + } + tt := []struct { name string expectedStatus int @@ -492,48 +598,97 @@ func Test_SetupKeys_Update(t *testing.T) { } for _, tc := range tt { - t.Run(tc.name, func(t *testing.T) { - apiHandler, am, done := buildApiBlackBoxWithDBState(t, "testdata/setup_keys.sql", nil) + for _, user := range users { + t.Run(tc.name, func(t *testing.T) { + apiHandler, am, done := buildApiBlackBoxWithDBState(t, "testdata/setup_keys.sql", nil) - body, err := json.Marshal(tc.requestBody) - if err != nil { - t.Fatalf("Failed to marshal request body: %v", err) - } + body, err := json.Marshal(tc.requestBody) + if err != nil { + t.Fatalf("Failed to marshal request body: %v", err) + } - req := buildRequest(t, body, tc.requestType, strings.Replace(tc.requestPath, "{id}", tc.requestId, 1)) + req := buildRequest(t, body, tc.requestType, strings.Replace(tc.requestPath, "{id}", tc.requestId, 1), user.userId) - recorder := httptest.NewRecorder() + recorder := httptest.NewRecorder() - apiHandler.ServeHTTP(recorder, req) + apiHandler.ServeHTTP(recorder, req) - content, noResponseExpected := readResponse(t, recorder, tc.expectedStatus) - if noResponseExpected { - return - } - got := &api.SetupKey{} - if err := json.Unmarshal(content, &got); err != nil { - t.Fatalf("Sent content is not in correct json format; %v", err) - } + content, expectResponse := readResponse(t, recorder, tc.expectedStatus, user.expectResponse) + if !expectResponse { + return + } + got := &api.SetupKey{} + if err := json.Unmarshal(content, &got); err != nil { + t.Fatalf("Sent content is not in correct json format; %v", err) + } - validateCreatedKey(t, tc.expectedResponse, got) + validateCreatedKey(t, tc.expectedResponse, got) - key, err := am.GetSetupKey(context.Background(), testAccountId, testUserId, got.Id) - if err != nil { - return - } + key, err := am.GetSetupKey(context.Background(), testAccountId, testUserId, got.Id) + if err != nil { + return + } - validateCreatedKey(t, tc.expectedResponse, toResponseBody(key)) + validateCreatedKey(t, tc.expectedResponse, toResponseBody(key)) - select { - case <-done: - case <-time.After(time.Second): - t.Error("timeout waiting for peerShouldNotReceiveUpdate") - } - }) + select { + case <-done: + case <-time.After(time.Second): + t.Error("timeout waiting for peerShouldNotReceiveUpdate") + } + }) + } } } func Test_SetupKeys_Get(t *testing.T) { + users := []struct { + name string + userId string + expectResponse bool + }{ + { + name: "Regular user", + userId: testUserId, + expectResponse: false, + }, + { + name: "Admin user", + userId: testAdminId, + expectResponse: true, + }, + { + name: "Owner user", + userId: testOwnerId, + expectResponse: true, + }, + { + name: "Regular service user", + userId: testServiceUserId, + expectResponse: false, + }, + { + name: "Admin service user", + userId: testServiceAdminId, + expectResponse: true, + }, + { + name: "Blocked user", + userId: blockedUserId, + expectResponse: false, + }, + { + name: "Other user", + userId: otherUserId, + expectResponse: false, + }, + { + name: "Invalid token", + userId: invalidToken, + expectResponse: false, + }, + } + tt := []struct { name string expectedStatus int @@ -622,43 +777,92 @@ func Test_SetupKeys_Get(t *testing.T) { } for _, tc := range tt { - t.Run(tc.name, func(t *testing.T) { - apiHandler, am, done := buildApiBlackBoxWithDBState(t, "testdata/setup_keys.sql", nil) + for _, user := range users { + t.Run(tc.name, func(t *testing.T) { + apiHandler, am, done := buildApiBlackBoxWithDBState(t, "testdata/setup_keys.sql", nil) - req := buildRequest(t, []byte{}, tc.requestType, strings.Replace(tc.requestPath, "{id}", tc.requestId, 1)) + req := buildRequest(t, []byte{}, tc.requestType, strings.Replace(tc.requestPath, "{id}", tc.requestId, 1), user.userId) - recorder := httptest.NewRecorder() + recorder := httptest.NewRecorder() - apiHandler.ServeHTTP(recorder, req) + apiHandler.ServeHTTP(recorder, req) - content, noResponseExpected := readResponse(t, recorder, tc.expectedStatus) - if noResponseExpected { - return - } - got := &api.SetupKey{} - if err := json.Unmarshal(content, &got); err != nil { - t.Fatalf("Sent content is not in correct json format; %v", err) - } + content, noResponseExpected := readResponse(t, recorder, tc.expectedStatus, user.expectResponse) + if noResponseExpected { + return + } + got := &api.SetupKey{} + if err := json.Unmarshal(content, &got); err != nil { + t.Fatalf("Sent content is not in correct json format; %v", err) + } - validateCreatedKey(t, tc.expectedResponse, got) + validateCreatedKey(t, tc.expectedResponse, got) - key, err := am.GetSetupKey(context.Background(), testAccountId, testUserId, got.Id) - if err != nil { - return - } + key, err := am.GetSetupKey(context.Background(), testAccountId, testUserId, got.Id) + if err != nil { + return + } - validateCreatedKey(t, tc.expectedResponse, toResponseBody(key)) + validateCreatedKey(t, tc.expectedResponse, toResponseBody(key)) - select { - case <-done: - case <-time.After(time.Second): - t.Error("timeout waiting for peerShouldNotReceiveUpdate") - } - }) + select { + case <-done: + case <-time.After(time.Second): + t.Error("timeout waiting for peerShouldNotReceiveUpdate") + } + }) + } } } func Test_SetupKeys_GetAll(t *testing.T) { + users := []struct { + name string + userId string + expectResponse bool + }{ + { + name: "Regular user", + userId: testUserId, + expectResponse: false, + }, + { + name: "Admin user", + userId: testAdminId, + expectResponse: true, + }, + { + name: "Owner user", + userId: testOwnerId, + expectResponse: true, + }, + { + name: "Regular service user", + userId: testServiceUserId, + expectResponse: false, + }, + { + name: "Admin service user", + userId: testServiceAdminId, + expectResponse: true, + }, + { + name: "Blocked user", + userId: blockedUserId, + expectResponse: false, + }, + { + name: "Other user", + userId: otherUserId, + expectResponse: false, + }, + { + name: "Invalid token", + userId: invalidToken, + expectResponse: false, + }, + } + tt := []struct { name string expectedStatus int @@ -725,53 +929,102 @@ func Test_SetupKeys_GetAll(t *testing.T) { } for _, tc := range tt { - t.Run(tc.name, func(t *testing.T) { - apiHandler, am, done := buildApiBlackBoxWithDBState(t, "testdata/setup_keys.sql", nil) + for _, user := range users { + t.Run(tc.name, func(t *testing.T) { + apiHandler, am, done := buildApiBlackBoxWithDBState(t, "testdata/setup_keys.sql", nil) - req := buildRequest(t, []byte{}, tc.requestType, tc.requestPath) + req := buildRequest(t, []byte{}, tc.requestType, tc.requestPath, user.userId) - recorder := httptest.NewRecorder() + recorder := httptest.NewRecorder() - apiHandler.ServeHTTP(recorder, req) + apiHandler.ServeHTTP(recorder, req) - content, noResponseExpected := readResponse(t, recorder, tc.expectedStatus) - if noResponseExpected { - return - } - got := []api.SetupKey{} - if err := json.Unmarshal(content, &got); err != nil { - t.Fatalf("Sent content is not in correct json format; %v", err) - } + content, expectResponse := readResponse(t, recorder, tc.expectedStatus, user.expectResponse) + if !expectResponse { + return + } + got := []api.SetupKey{} + if err := json.Unmarshal(content, &got); err != nil { + t.Fatalf("Sent content is not in correct json format; %v", err) + } - sort.Slice(got, func(i, j int) bool { - return got[i].UsageLimit < got[j].UsageLimit - }) + sort.Slice(got, func(i, j int) bool { + return got[i].UsageLimit < got[j].UsageLimit + }) - sort.Slice(tc.expectedResponse, func(i, j int) bool { - return tc.expectedResponse[i].UsageLimit < tc.expectedResponse[j].UsageLimit - }) + sort.Slice(tc.expectedResponse, func(i, j int) bool { + return tc.expectedResponse[i].UsageLimit < tc.expectedResponse[j].UsageLimit + }) - for i, _ := range tc.expectedResponse { - validateCreatedKey(t, tc.expectedResponse[i], &got[i]) + for i, _ := range tc.expectedResponse { + validateCreatedKey(t, tc.expectedResponse[i], &got[i]) - key, err := am.GetSetupKey(context.Background(), testAccountId, testUserId, got[i].Id) - if err != nil { - return - } + key, err := am.GetSetupKey(context.Background(), testAccountId, testUserId, got[i].Id) + if err != nil { + return + } - validateCreatedKey(t, tc.expectedResponse[i], toResponseBody(key)) - } + validateCreatedKey(t, tc.expectedResponse[i], toResponseBody(key)) + } - select { - case <-done: - case <-time.After(time.Second): - t.Error("timeout waiting for peerShouldNotReceiveUpdate") - } - }) + select { + case <-done: + case <-time.After(time.Second): + t.Error("timeout waiting for peerShouldNotReceiveUpdate") + } + }) + } } } func Test_SetupKeys_Delete(t *testing.T) { + users := []struct { + name string + userId string + expectResponse bool + }{ + { + name: "Regular user", + userId: testUserId, + expectResponse: false, + }, + { + name: "Admin user", + userId: testAdminId, + expectResponse: true, + }, + { + name: "Owner user", + userId: testOwnerId, + expectResponse: true, + }, + { + name: "Regular service user", + userId: testServiceUserId, + expectResponse: false, + }, + { + name: "Admin service user", + userId: testServiceAdminId, + expectResponse: true, + }, + { + name: "Blocked user", + userId: blockedUserId, + expectResponse: false, + }, + { + name: "Other user", + userId: otherUserId, + expectResponse: false, + }, + { + name: "Invalid token", + userId: invalidToken, + expectResponse: false, + }, + } + tt := []struct { name string expectedStatus int @@ -860,33 +1113,35 @@ func Test_SetupKeys_Delete(t *testing.T) { } for _, tc := range tt { - t.Run(tc.name, func(t *testing.T) { - apiHandler, am, done := buildApiBlackBoxWithDBState(t, "testdata/setup_keys.sql", nil) + for _, user := range users { + t.Run(tc.name, func(t *testing.T) { + apiHandler, am, done := buildApiBlackBoxWithDBState(t, "testdata/setup_keys.sql", nil) - req := buildRequest(t, []byte{}, tc.requestType, strings.Replace(tc.requestPath, "{id}", tc.requestId, 1)) + req := buildRequest(t, []byte{}, tc.requestType, strings.Replace(tc.requestPath, "{id}", tc.requestId, 1), user.userId) - recorder := httptest.NewRecorder() + recorder := httptest.NewRecorder() - apiHandler.ServeHTTP(recorder, req) + apiHandler.ServeHTTP(recorder, req) - content, noResponseExpected := readResponse(t, recorder, tc.expectedStatus) - if noResponseExpected { - return - } - got := &api.SetupKey{} - if err := json.Unmarshal(content, &got); err != nil { - t.Fatalf("Sent content is not in correct json format; %v", err) - } + content, expectResponse := readResponse(t, recorder, tc.expectedStatus, user.expectResponse) + if !expectResponse { + return + } + got := &api.SetupKey{} + if err := json.Unmarshal(content, &got); err != nil { + t.Fatalf("Sent content is not in correct json format; %v", err) + } - _, err := am.GetSetupKey(context.Background(), testAccountId, testUserId, got.Id) - assert.Errorf(t, err, "Expected error when trying to get deleted key") + _, err := am.GetSetupKey(context.Background(), testAccountId, testUserId, got.Id) + assert.Errorf(t, err, "Expected error when trying to get deleted key") - select { - case <-done: - case <-time.After(time.Second): - t.Error("timeout waiting for peerShouldNotReceiveUpdate") - } - }) + select { + case <-done: + case <-time.After(time.Second): + t.Error("timeout waiting for peerShouldNotReceiveUpdate") + } + }) + } } } @@ -926,16 +1181,16 @@ func buildApiBlackBoxWithDBState(t *testing.T, sqlFile string, expectedPeerUpdat return apiHandler, am, done } -func buildRequest(t *testing.T, requestBody []byte, requestType, requestPath string) *http.Request { +func buildRequest(t *testing.T, requestBody []byte, requestType, requestPath, user string) *http.Request { t.Helper() req := httptest.NewRequest(requestType, requestPath, bytes.NewBuffer(requestBody)) - req.Header.Set("Authorization", "Bearer "+"my.dummy.token") + req.Header.Set("Authorization", "Bearer "+user) return req } -func readResponse(t *testing.T, recorder *httptest.ResponseRecorder, expectedStatus int) ([]byte, bool) { +func readResponse(t *testing.T, recorder *httptest.ResponseRecorder, expectedStatus int, expectResponse bool) ([]byte, bool) { t.Helper() res := recorder.Result() @@ -946,12 +1201,16 @@ func readResponse(t *testing.T, recorder *httptest.ResponseRecorder, expectedSta t.Fatalf("Failed to read response body: %v", err) } + if !expectResponse { + return nil, false + } + if status := recorder.Code; status != expectedStatus { t.Fatalf("handler returned wrong status code: got %v want %v, content: %s", status, expectedStatus, string(content)) } - return content, expectedStatus != http.StatusOK + return content, expectedStatus == http.StatusOK } func validateCreatedKey(t *testing.T, expectedKey *api.SetupKey, got *api.SetupKey) { diff --git a/management/server/http/testdata/setup_keys.sql b/management/server/http/testdata/setup_keys.sql index 6fe374a6e63..c0add32680b 100644 --- a/management/server/http/testdata/setup_keys.sql +++ b/management/server/http/testdata/setup_keys.sql @@ -1,15 +1,23 @@ CREATE TABLE `accounts` (`id` text,`created_by` text,`created_at` datetime,`domain` text,`domain_category` text,`is_domain_primary_account` numeric,`network_identifier` text,`network_net` text,`network_dns` text,`network_serial` integer,`dns_settings_disabled_management_groups` text,`settings_peer_login_expiration_enabled` numeric,`settings_peer_login_expiration` integer,`settings_regular_users_view_blocked` numeric,`settings_groups_propagation_enabled` numeric,`settings_jwt_groups_enabled` numeric,`settings_jwt_groups_claim_name` text,`settings_jwt_allow_groups` text,`settings_extra_peer_approval_enabled` numeric,`settings_extra_integrated_validator_groups` text,PRIMARY KEY (`id`)); -CREATE TABLE `setup_keys` (`id` text,`account_id` text,`key` text,`key_secret` text,`name` text,`type` text,`created_at` datetime,`expires_at` datetime,`updated_at` datetime,`revoked` numeric,`used_times` integer,`last_used` datetime,`auto_groups` text,`usage_limit` integer,`ephemeral` numeric,PRIMARY KEY (`id`),CONSTRAINT `fk_accounts_setup_keys_g` FOREIGN KEY (`account_id`) REFERENCES `accounts`(`id`)); CREATE TABLE `users` (`id` text,`account_id` text,`role` text,`is_service_user` numeric,`non_deletable` numeric,`service_user_name` text,`auto_groups` text,`blocked` numeric,`last_login` datetime,`created_at` datetime,`issued` text DEFAULT "api",`integration_ref_id` integer,`integration_ref_integration_type` text,PRIMARY KEY (`id`),CONSTRAINT `fk_accounts_users_g` FOREIGN KEY (`account_id`) REFERENCES `accounts`(`id`)); CREATE TABLE `peers` (`id` text,`account_id` text,`key` text,`setup_key` text,`ip` text,`meta_hostname` text,`meta_go_os` text,`meta_kernel` text,`meta_core` text,`meta_platform` text,`meta_os` text,`meta_os_version` text,`meta_wt_version` text,`meta_ui_version` text,`meta_kernel_version` text,`meta_network_addresses` text,`meta_system_serial_number` text,`meta_system_product_name` text,`meta_system_manufacturer` text,`meta_environment` text,`meta_files` text,`name` text,`dns_label` text,`peer_status_last_seen` datetime,`peer_status_connected` numeric,`peer_status_login_expired` numeric,`peer_status_requires_approval` numeric,`user_id` text,`ssh_key` text,`ssh_enabled` numeric,`login_expiration_enabled` numeric,`last_login` datetime,`created_at` datetime,`ephemeral` numeric,`location_connection_ip` text,`location_country_code` text,`location_city_name` text,`location_geo_name_id` integer,PRIMARY KEY (`id`),CONSTRAINT `fk_accounts_peers_g` FOREIGN KEY (`account_id`) REFERENCES `accounts`(`id`)); CREATE TABLE `groups` (`id` text,`account_id` text,`name` text,`issued` text,`peers` text,`integration_ref_id` integer,`integration_ref_integration_type` text,PRIMARY KEY (`id`),CONSTRAINT `fk_accounts_groups_g` FOREIGN KEY (`account_id`) REFERENCES `accounts`(`id`)); INSERT INTO accounts VALUES('testAccountId','','2024-10-02 16:01:38.210014+02:00','test.com','private',1,'testNetworkIdentifier','{"IP":"100.64.0.0","Mask":"//8AAA=="}','',0,'[]',0,86400000000000,0,0,0,'',NULL,NULL,NULL); -INSERT INTO users VALUES('testUserId','testAccountId','admin',0,0,'','[]',0,'0001-01-01 00:00:00+00:00','2024-10-02 16:01:38.210678+02:00','api',0,''); +INSERT INTO users VALUES('testUserId','testAccountId','user',0,0,'','[]',0,'0001-01-01 00:00:00+00:00','2024-10-02 16:01:38.210678+02:00','api',0,''); +INSERT INTO users VALUES('testAdminId','testAccountId','admin',0,0,'','[]',0,'0001-01-01 00:00:00+00:00','2024-10-02 16:01:38.210678+02:00','api',0,''); +INSERT INTO users VALUES('testOwnerId','testAccountId','owner',0,0,'','[]',0,'0001-01-01 00:00:00+00:00','2024-10-02 16:01:38.210678+02:00','api',0,''); +INSERT INTO users VALUES('testServiceUserId','testAccountId','user',1,0,'','[]',0,'0001-01-01 00:00:00+00:00','2024-10-02 16:01:38.210678+02:00','api',0,''); +INSERT INTO users VALUES('testServiceAdminId','testAccountId','admin',1,0,'','[]',0,'0001-01-01 00:00:00+00:00','2024-10-02 16:01:38.210678+02:00','api',0,''); +INSERT INTO users VALUES('blockedUserId','testAccountId','admin',0,0,'','[]',1,'0001-01-01 00:00:00+00:00','2024-10-02 16:01:38.210678+02:00','api',0,''); +INSERT INTO users VALUES('otherUserId','otherAccountId','admin',0,0,'','[]',0,'0001-01-01 00:00:00+00:00','2024-10-02 16:01:38.210678+02:00','api',0,''); INSERT INTO peers VALUES('testPeerId','testAccountId','5rvhvriKJZ3S9oxYToVj5TzDM9u9y8cxg7htIMWlYAg=','72546A29-6BC8-4311-BCFC-9CDBF33F1A48','"100.64.114.31"','f2a34f6a4731','linux','Linux','11','unknown','Debian GNU/Linux','','0.12.0','','',NULL,'','','','{"Cloud":"","Platform":""}',NULL,'f2a34f6a4731','f2a34f6a4731','2023-03-02 09:21:02.189035775+01:00',0,0,0,'','ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAILzUUSYG/LGnV8zarb2SGN+tib/PZ+M7cL4WtTzUrTpk',0,1,'2023-03-01 19:48:19.817799698+01:00','2024-10-02 17:00:32.527947+02:00',0,'""','','',0); INSERT INTO "groups" VALUES('testGroupId','testAccountId','testGroupName','api','[]',0,''); INSERT INTO "groups" VALUES('newGroupId','testAccountId','newGroupName','api','[]',0,''); + +CREATE TABLE `setup_keys` (`id` text,`account_id` text,`key` text,`key_secret` text,`name` text,`type` text,`created_at` datetime,`expires_at` datetime,`updated_at` datetime,`revoked` numeric,`used_times` integer,`last_used` datetime,`auto_groups` text,`usage_limit` integer,`ephemeral` numeric,PRIMARY KEY (`id`),CONSTRAINT `fk_accounts_setup_keys_g` FOREIGN KEY (`account_id`) REFERENCES `accounts`(`id`)); + INSERT INTO setup_keys VALUES('testKeyId','testAccountId','testKey','testK****','existingKey','one-off','2021-08-19 20:46:20.005936822+02:00','2321-09-18 20:46:20.005936822+02:00','2021-08-19 20:46:20.005936822+02:00',0,0,'0001-01-01 00:00:00+00:00','["testGroupId"]',1,0); INSERT INTO setup_keys VALUES('revokedKeyId','testAccountId','revokedKey','testK****','existingKey','reusable','2021-08-19 20:46:20.005936822+02:00','2321-09-18 20:46:20.005936822+02:00','2021-08-19 20:46:20.005936822+02:00',1,0,'0001-01-01 00:00:00+00:00','["testGroupId"]',3,0); INSERT INTO setup_keys VALUES('expiredKeyId','testAccountId','expiredKey','testK****','existingKey','reusable','2021-08-19 20:46:20.005936822+02:00','1921-09-18 20:46:20.005936822+02:00','2021-08-19 20:46:20.005936822+02:00',0,1,'0001-01-01 00:00:00+00:00','["testGroupId"]',5,1); diff --git a/management/server/jwtclaims/jwtValidator.go b/management/server/jwtclaims/jwtValidator.go index b7b05d5e2a1..da61fda8e25 100644 --- a/management/server/jwtclaims/jwtValidator.go +++ b/management/server/jwtclaims/jwtValidator.go @@ -319,11 +319,22 @@ type JwtValidatorMock struct{} func (j *JwtValidatorMock) ValidateAndParse(ctx context.Context, token string) (*jwt.Token, error) { claimMaps := jwt.MapClaims{} - claimMaps[UserIDClaim] = "testUserId" - claimMaps[AccountIDSuffix] = "testAccountId" - claimMaps[DomainIDSuffix] = "test.com" - claimMaps[DomainCategorySuffix] = "private" - jwtToken := jwt.NewWithClaims(jwt.SigningMethodHS256, claimMaps) + switch token { + case "testUserId", "testAdminId", "testOwnerId", "testServiceUserId", "testServiceAdminId", "blockedUserId": + claimMaps[UserIDClaim] = token + claimMaps[AccountIDSuffix] = "testAccountId" + claimMaps[DomainIDSuffix] = "test.com" + claimMaps[DomainCategorySuffix] = "private" + case "otherUserId": + claimMaps[UserIDClaim] = "otherUserId" + claimMaps[AccountIDSuffix] = "otherAccountId" + claimMaps[DomainIDSuffix] = "other.com" + claimMaps[DomainCategorySuffix] = "private" + case "invalidToken": + return nil, errors.New("invalid token") + } + + jwtToken := jwt.NewWithClaims(jwt.SigningMethodHS256, claimMaps) return jwtToken, nil } From 4a1b1799dcbebe79983ede1ef7937e30e45e4a53 Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Mon, 25 Nov 2024 15:38:15 +0100 Subject: [PATCH 11/38] rename geo mock --- management/server/geolocation/geolocation.go | 10 +++++----- management/server/http/posture_checks_handler_test.go | 2 +- management/server/http/setupkeys_integration_test.go | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/management/server/geolocation/geolocation.go b/management/server/geolocation/geolocation.go index b1d7e7b93e0..c0179a1c481 100644 --- a/management/server/geolocation/geolocation.go +++ b/management/server/geolocation/geolocation.go @@ -267,20 +267,20 @@ func cleanupMaxMindDatabases(ctx context.Context, dataDir string, mmdbFile strin return nil } -type GeolocationMock struct{} +type Mock struct{} -func (g *GeolocationMock) Lookup(ip net.IP) (*Record, error) { +func (g *Mock) Lookup(ip net.IP) (*Record, error) { return &Record{}, nil } -func (g *GeolocationMock) GetAllCountries() ([]Country, error) { +func (g *Mock) GetAllCountries() ([]Country, error) { return []Country{}, nil } -func (g *GeolocationMock) GetCitiesByCountry(countryISOCode string) ([]City, error) { +func (g *Mock) GetCitiesByCountry(countryISOCode string) ([]City, error) { return []City{}, nil } -func (g *GeolocationMock) Stop() error { +func (g *Mock) Stop() error { return nil } diff --git a/management/server/http/posture_checks_handler_test.go b/management/server/http/posture_checks_handler_test.go index 433205de2ae..09c1d1e087f 100644 --- a/management/server/http/posture_checks_handler_test.go +++ b/management/server/http/posture_checks_handler_test.go @@ -70,7 +70,7 @@ func initPostureChecksTestData(postureChecks ...*posture.Checks) *PostureChecksH return claims.AccountId, claims.UserId, nil }, }, - geolocationManager: &geolocation.GeolocationMock{}, + geolocationManager: &geolocation.Mock{}, claimsExtractor: jwtclaims.NewClaimsExtractor( jwtclaims.WithFromRequestContext(func(r *http.Request) jwtclaims.AuthorizationClaims { return jwtclaims.AuthorizationClaims{ diff --git a/management/server/http/setupkeys_integration_test.go b/management/server/http/setupkeys_integration_test.go index 15153391b3a..ecf25b19ba5 100644 --- a/management/server/http/setupkeys_integration_test.go +++ b/management/server/http/setupkeys_integration_test.go @@ -1166,7 +1166,7 @@ func buildApiBlackBoxWithDBState(t *testing.T, sqlFile string, expectedPeerUpdat close(done) }() - geoMock := &geolocation.GeolocationMock{} + geoMock := &geolocation.Mock{} validatorMock := server.MocIntegratedValidator{} am, err := server.BuildManager(context.Background(), store, peersUpdateManager, nil, "", "", &activity.InMemoryEventStore{}, geoMock, false, validatorMock, metrics) if err != nil { From ada2dd58b6d76379345265387a74a804610dad3d Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Thu, 28 Nov 2024 14:58:10 +0100 Subject: [PATCH 12/38] add benchmark tests for setup keys --- .../http/setupkeys_handler_benchmark_test.go | 291 ++++++++++++++++++ ... => setupkeys_handler_integration_test.go} | 125 +------- .../server/http/setupkeys_handler_test.go | 24 ++ management/server/http/tools_test.go | 226 ++++++++++++++ 4 files changed, 542 insertions(+), 124 deletions(-) create mode 100644 management/server/http/setupkeys_handler_benchmark_test.go rename management/server/http/{setupkeys_integration_test.go => setupkeys_handler_integration_test.go} (88%) create mode 100644 management/server/http/tools_test.go diff --git a/management/server/http/setupkeys_handler_benchmark_test.go b/management/server/http/setupkeys_handler_benchmark_test.go new file mode 100644 index 00000000000..ffa6456ab9c --- /dev/null +++ b/management/server/http/setupkeys_handler_benchmark_test.go @@ -0,0 +1,291 @@ +//go:build benchmark + +package http + +import ( + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "os" + "strconv" + "testing" + "time" + + log "github.com/sirupsen/logrus" + "github.com/stretchr/testify/assert" + + "github.com/netbirdio/netbird/management/server" + "github.com/netbirdio/netbird/management/server/http/api" +) + +func BenchmarkCreateSetupKey(b *testing.B) { + benchCases := []struct { + name string + peers int + groups int + users int + setupKeys int + minMsPerOp float64 + maxMsPerOp float64 + }{ + {"Setup Keys - XS", 5, 5, 5, 5, 0.5, 2}, + {"Setup Keys - S", 5, 5, 5, 5, 0.5, 2}, + {"Setup Keys - M", 100, 20, 20, 100, 0.5, 2}, + {"Setup Keys - L", 500, 50, 100, 1000, 0.5, 2}, + {"Setup Keys - XL", 500, 50, 100, 5000, 0.5, 2}, + } + + log.SetOutput(io.Discard) + defer log.SetOutput(os.Stderr) + + recorder := httptest.NewRecorder() + + for _, bc := range benchCases { + b.Run(bc.name, func(b *testing.B) { + apiHandler, am, _ := buildApiBlackBoxWithDBState(b, "testdata/setup_keys.sql", nil) + populateTestData(b, am.(*server.DefaultAccountManager), bc.peers, bc.groups, bc.users, bc.setupKeys) + + b.ResetTimer() + start := time.Now() + for i := 0; i < b.N; i++ { + requestBody := api.CreateSetupKeyRequest{ + AutoGroups: []string{"someGroupID"}, + ExpiresIn: expiresIn, + Name: newKeyName + strconv.Itoa(i), + Type: "reusable", + UsageLimit: 0, + } + + // the time marshal will be recorded as well but for our use case that is ok + body, err := json.Marshal(requestBody) + assert.NoError(b, err) + + req := buildRequest(b, body, http.MethodPost, "/api/setup-keys", testAdminId) + apiHandler.ServeHTTP(recorder, req) + } + + duration := time.Since(start) + msPerOp := float64(duration.Nanoseconds()) / float64(b.N) / 1e6 + b.ReportMetric(msPerOp, "ms/op") + + if msPerOp < bc.minMsPerOp { + b.Fatalf("Benchmark %s failed: too fast (%.2f ms/op, minimum %.2f ms/op)", bc.name, msPerOp, bc.minMsPerOp) + } + + if msPerOp > bc.maxMsPerOp { + b.Fatalf("Benchmark %s failed: too slow (%.2f ms/op, maximum %.2f ms/op)", bc.name, msPerOp, bc.maxMsPerOp) + } + }) + } +} + +func BenchmarkUpdateSetupKey(b *testing.B) { + benchCases := []struct { + name string + peers int + groups int + users int + setupKeys int + minMsPerOp float64 + maxMsPerOp float64 + }{ + {"Setup Keys - XS", 5, 5, 5, 5, 0.5, 2}, + {"Setup Keys - S", 5, 5, 5, 5, 0.5, 2}, + {"Setup Keys - M", 100, 20, 20, 100, 0.5, 2}, + {"Setup Keys - L", 500, 50, 100, 1000, 0.5, 2}, + {"Setup Keys - XL", 500, 50, 100, 5000, 0.5, 2}, + } + + log.SetOutput(io.Discard) + defer log.SetOutput(os.Stderr) + + recorder := httptest.NewRecorder() + + for _, bc := range benchCases { + b.Run(bc.name, func(b *testing.B) { + apiHandler, am, _ := buildApiBlackBoxWithDBState(b, "testdata/setup_keys.sql", nil) + populateTestData(b, am.(*server.DefaultAccountManager), bc.peers, bc.groups, bc.users, bc.setupKeys) + + b.ResetTimer() + start := time.Now() + for i := 0; i < b.N; i++ { + groupId := testGroupId + if i%2 == 0 { + groupId = newGroupId + } + requestBody := api.SetupKeyRequest{ + AutoGroups: []string{groupId}, + Revoked: false, + } + + // the time marshal will be recorded as well but for our use case that is ok + body, err := json.Marshal(requestBody) + assert.NoError(b, err) + + req := buildRequest(b, body, http.MethodPut, "/api/setup-keys/"+testKeyId, testAdminId) + apiHandler.ServeHTTP(recorder, req) + } + + duration := time.Since(start) + msPerOp := float64(duration.Nanoseconds()) / float64(b.N) / 1e6 + b.ReportMetric(msPerOp, "ms/op") + + if msPerOp < bc.minMsPerOp { + b.Fatalf("Benchmark %s failed: too fast (%.2f ms/op, minimum %.2f ms/op)", bc.name, msPerOp, bc.minMsPerOp) + } + + if msPerOp > bc.maxMsPerOp { + b.Fatalf("Benchmark %s failed: too slow (%.2f ms/op, maximum %.2f ms/op)", bc.name, msPerOp, bc.maxMsPerOp) + } + }) + } +} + +func BenchmarkGetOneSetupKey(b *testing.B) { + benchCases := []struct { + name string + peers int + groups int + users int + setupKeys int + minMsPerOp float64 + maxMsPerOp float64 + }{ + {"Setup Keys - XS", 5, 5, 5, 5, 0.5, 2}, + {"Setup Keys - S", 5, 5, 5, 5, 0.5, 2}, + {"Setup Keys - M", 100, 20, 20, 100, 0.5, 2}, + {"Setup Keys - L", 500, 50, 100, 1000, 0.5, 2}, + {"Setup Keys - XL", 500, 50, 100, 5000, 0.5, 2}, + } + + log.SetOutput(io.Discard) + defer log.SetOutput(os.Stderr) + + recorder := httptest.NewRecorder() + + for _, bc := range benchCases { + b.Run(bc.name, func(b *testing.B) { + apiHandler, am, _ := buildApiBlackBoxWithDBState(b, "testdata/setup_keys.sql", nil) + populateTestData(b, am.(*server.DefaultAccountManager), bc.peers, bc.groups, bc.users, bc.setupKeys) + + b.ResetTimer() + start := time.Now() + for i := 0; i < b.N; i++ { + req := buildRequest(b, nil, http.MethodGet, "/api/setup-keys/"+testKeyId, testAdminId) + apiHandler.ServeHTTP(recorder, req) + } + + duration := time.Since(start) + msPerOp := float64(duration.Nanoseconds()) / float64(b.N) / 1e6 + b.ReportMetric(msPerOp, "ms/op") + + if msPerOp < bc.minMsPerOp { + b.Fatalf("Benchmark %s failed: too fast (%.2f ms/op, minimum %.2f ms/op)", bc.name, msPerOp, bc.minMsPerOp) + } + + if msPerOp > bc.maxMsPerOp { + b.Fatalf("Benchmark %s failed: too slow (%.2f ms/op, maximum %.2f ms/op)", bc.name, msPerOp, bc.maxMsPerOp) + } + }) + } +} + +func BenchmarkGetAllSetupKeys(b *testing.B) { + benchCases := []struct { + name string + peers int + groups int + users int + setupKeys int + minMsPerOp float64 + maxMsPerOp float64 + }{ + {"Setup Keys - XS", 5, 5, 5, 5, 0.5, 2}, + {"Setup Keys - S", 5, 5, 5, 5, 0.5, 2}, + {"Setup Keys - M", 100, 20, 20, 100, 0.5, 2}, + {"Setup Keys - L", 500, 50, 100, 1000, 5, 10}, + {"Setup Keys - XL", 500, 50, 100, 5000, 25, 45}, + } + + log.SetOutput(io.Discard) + defer log.SetOutput(os.Stderr) + + recorder := httptest.NewRecorder() + + for _, bc := range benchCases { + b.Run(bc.name, func(b *testing.B) { + apiHandler, am, _ := buildApiBlackBoxWithDBState(b, "testdata/setup_keys.sql", nil) + populateTestData(b, am.(*server.DefaultAccountManager), bc.peers, bc.groups, bc.users, bc.setupKeys) + + b.ResetTimer() + start := time.Now() + for i := 0; i < b.N; i++ { + req := buildRequest(b, nil, http.MethodGet, "/api/setup-keys", testAdminId) + apiHandler.ServeHTTP(recorder, req) + } + + duration := time.Since(start) + msPerOp := float64(duration.Nanoseconds()) / float64(b.N) / 1e6 + b.ReportMetric(msPerOp, "ms/op") + + if msPerOp < bc.minMsPerOp { + b.Fatalf("Benchmark %s failed: too fast (%.2f ms/op, minimum %.2f ms/op)", bc.name, msPerOp, bc.minMsPerOp) + } + + if msPerOp > bc.maxMsPerOp { + b.Fatalf("Benchmark %s failed: too slow (%.2f ms/op, maximum %.2f ms/op)", bc.name, msPerOp, bc.maxMsPerOp) + } + }) + } +} + +func BenchmarkDeleteSetupKey(b *testing.B) { + benchCases := []struct { + name string + peers int + groups int + users int + setupKeys int + minMsPerOp float64 + maxMsPerOp float64 + }{ + {"Setup Keys - XS", 5, 5, 5, 5, 0.5, 2}, + {"Setup Keys - S", 5, 5, 5, 5, 0.5, 2}, + {"Setup Keys - M", 100, 20, 20, 100, 0.5, 2}, + {"Setup Keys - L", 500, 50, 100, 1000, 0.5, 2}, + {"Setup Keys - XL", 500, 50, 100, 5000, 0.5, 2}, + } + + log.SetOutput(io.Discard) + defer log.SetOutput(os.Stderr) + + recorder := httptest.NewRecorder() + + for _, bc := range benchCases { + b.Run(bc.name, func(b *testing.B) { + apiHandler, am, _ := buildApiBlackBoxWithDBState(b, "testdata/setup_keys.sql", nil) + populateTestData(b, am.(*server.DefaultAccountManager), bc.peers, bc.groups, bc.users, bc.setupKeys) + + b.ResetTimer() + start := time.Now() + for i := 0; i < b.N; i++ { + // depending on the test case we may fail do delete keys as no more keys are there + req := buildRequest(b, nil, http.MethodGet, "/api/setup-keys/"+"oldkey-"+strconv.Itoa(i), testAdminId) + apiHandler.ServeHTTP(recorder, req) + } + + duration := time.Since(start) + msPerOp := float64(duration.Nanoseconds()) / float64(b.N) / 1e6 + b.ReportMetric(msPerOp, "ms/op") + + if msPerOp < bc.minMsPerOp { + b.Fatalf("Benchmark %s failed: too fast (%.2f ms/op, minimum %.2f ms/op)", bc.name, msPerOp, bc.minMsPerOp) + } + + if msPerOp > bc.maxMsPerOp { + b.Fatalf("Benchmark %s failed: too slow (%.2f ms/op, maximum %.2f ms/op)", bc.name, msPerOp, bc.maxMsPerOp) + } + }) + } +} diff --git a/management/server/http/setupkeys_integration_test.go b/management/server/http/setupkeys_handler_integration_test.go similarity index 88% rename from management/server/http/setupkeys_integration_test.go rename to management/server/http/setupkeys_handler_integration_test.go index ecf25b19ba5..ebaf671849d 100644 --- a/management/server/http/setupkeys_integration_test.go +++ b/management/server/http/setupkeys_handler_integration_test.go @@ -1,12 +1,10 @@ -//go:build component +//go:build integration package http import ( - "bytes" "context" "encoding/json" - "io" "net/http" "net/http/httptest" "sort" @@ -16,36 +14,7 @@ import ( "github.com/stretchr/testify/assert" - "github.com/netbirdio/netbird/management/server" - "github.com/netbirdio/netbird/management/server/activity" - "github.com/netbirdio/netbird/management/server/geolocation" "github.com/netbirdio/netbird/management/server/http/api" - "github.com/netbirdio/netbird/management/server/jwtclaims" - "github.com/netbirdio/netbird/management/server/telemetry" -) - -const ( - testAccountId = "testAccountId" - testPeerId = "testPeerId" - testGroupId = "testGroupId" - testKeyId = "testKeyId" - - testUserId = "testUserId" - testAdminId = "testAdminId" - testOwnerId = "testOwnerId" - testServiceUserId = "testServiceUserId" - testServiceAdminId = "testServiceAdminId" - blockedUserId = "blockedUserId" - otherUserId = "otherUserId" - invalidToken = "invalidToken" - - newKeyName = "newKey" - newGroupId = "newGroupId" - expiresIn = 3600 - revokedKeyId = "revokedKeyId" - expiredKeyId = "expiredKeyId" - - existingKeyName = "existingKey" ) func Test_SetupKeys_Create(t *testing.T) { @@ -1145,74 +1114,6 @@ func Test_SetupKeys_Delete(t *testing.T) { } } -func buildApiBlackBoxWithDBState(t *testing.T, sqlFile string, expectedPeerUpdate *server.UpdateMessage) (http.Handler, server.AccountManager, chan struct{}) { - store, cleanup, err := server.NewTestStoreFromSQL(context.Background(), sqlFile, t.TempDir()) - if err != nil { - t.Fatalf("Failed to create test store: %v", err) - } - t.Cleanup(cleanup) - - metrics, err := telemetry.NewDefaultAppMetrics(context.Background()) - - peersUpdateManager := server.NewPeersUpdateManager(nil) - updMsg := peersUpdateManager.CreateChannel(context.Background(), testPeerId) - done := make(chan struct{}) - go func() { - if expectedPeerUpdate != nil { - peerShouldReceiveUpdate(t, updMsg, expectedPeerUpdate) - } else { - peerShouldNotReceiveUpdate(t, updMsg) - } - close(done) - }() - - geoMock := &geolocation.Mock{} - validatorMock := server.MocIntegratedValidator{} - am, err := server.BuildManager(context.Background(), store, peersUpdateManager, nil, "", "", &activity.InMemoryEventStore{}, geoMock, false, validatorMock, metrics) - if err != nil { - t.Fatalf("Failed to create manager: %v", err) - } - - apiHandler, err := APIHandler(context.Background(), am, geoMock, &jwtclaims.JwtValidatorMock{}, metrics, AuthCfg{}, validatorMock) - if err != nil { - t.Fatalf("Failed to create API handler: %v", err) - } - - return apiHandler, am, done -} - -func buildRequest(t *testing.T, requestBody []byte, requestType, requestPath, user string) *http.Request { - t.Helper() - - req := httptest.NewRequest(requestType, requestPath, bytes.NewBuffer(requestBody)) - req.Header.Set("Authorization", "Bearer "+user) - - return req -} - -func readResponse(t *testing.T, recorder *httptest.ResponseRecorder, expectedStatus int, expectResponse bool) ([]byte, bool) { - t.Helper() - - res := recorder.Result() - defer res.Body.Close() - - content, err := io.ReadAll(res.Body) - if err != nil { - t.Fatalf("Failed to read response body: %v", err) - } - - if !expectResponse { - return nil, false - } - - if status := recorder.Code; status != expectedStatus { - t.Fatalf("handler returned wrong status code: got %v want %v, content: %s", - status, expectedStatus, string(content)) - } - - return content, expectedStatus == http.StatusOK -} - func validateCreatedKey(t *testing.T, expectedKey *api.SetupKey, got *api.SetupKey) { t.Helper() @@ -1240,27 +1141,3 @@ func validateCreatedKey(t *testing.T, expectedKey *api.SetupKey, got *api.SetupK assert.Equal(t, expectedKey, got) } - -func peerShouldNotReceiveUpdate(t *testing.T, updateMessage <-chan *server.UpdateMessage) { - t.Helper() - select { - case msg := <-updateMessage: - t.Errorf("Unexpected message received: %+v", msg) - case <-time.After(500 * time.Millisecond): - return - } -} - -func peerShouldReceiveUpdate(t *testing.T, updateMessage <-chan *server.UpdateMessage, expected *server.UpdateMessage) { - t.Helper() - - select { - case msg := <-updateMessage: - if msg == nil { - t.Errorf("Received nil update message, expected valid message") - } - assert.Equal(t, expected, msg) - case <-time.After(500 * time.Millisecond): - t.Error("Timed out waiting for update message") - } -} diff --git a/management/server/http/setupkeys_handler_test.go b/management/server/http/setupkeys_handler_test.go index b8dcae2395e..6d78e73e343 100644 --- a/management/server/http/setupkeys_handler_test.go +++ b/management/server/http/setupkeys_handler_test.go @@ -28,6 +28,30 @@ const ( notFoundSetupKeyID = "notFoundSetupKeyID" ) +const ( + testAccountId = "testAccountId" + testPeerId = "testPeerId" + testGroupId = "testGroupId" + testKeyId = "testKeyId" + + testUserId = "testUserId" + testAdminId = "testAdminId" + testOwnerId = "testOwnerId" + testServiceUserId = "testServiceUserId" + testServiceAdminId = "testServiceAdminId" + blockedUserId = "blockedUserId" + otherUserId = "otherUserId" + invalidToken = "invalidToken" + + newKeyName = "newKey" + newGroupId = "newGroupId" + expiresIn = 3600 + revokedKeyId = "revokedKeyId" + expiredKeyId = "expiredKeyId" + + existingKeyName = "existingKey" +) + func initSetupKeysTestMetaData(defaultKey *server.SetupKey, newKey *server.SetupKey, updatedSetupKey *server.SetupKey, user *server.User, ) *SetupKeysHandler { diff --git a/management/server/http/tools_test.go b/management/server/http/tools_test.go new file mode 100644 index 00000000000..f525790621c --- /dev/null +++ b/management/server/http/tools_test.go @@ -0,0 +1,226 @@ +package http + +import ( + "bytes" + "context" + "fmt" + "io" + "net" + "net/http" + "net/http/httptest" + "strconv" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "golang.zx2c4.com/wireguard/wgctrl/wgtypes" + + "github.com/netbirdio/netbird/management/server" + "github.com/netbirdio/netbird/management/server/activity" + "github.com/netbirdio/netbird/management/server/geolocation" + nbgroup "github.com/netbirdio/netbird/management/server/group" + "github.com/netbirdio/netbird/management/server/jwtclaims" + nbpeer "github.com/netbirdio/netbird/management/server/peer" + "github.com/netbirdio/netbird/management/server/posture" + "github.com/netbirdio/netbird/management/server/telemetry" +) + +type TB interface { + Cleanup(func()) + Helper() + Errorf(format string, args ...any) + Fatalf(format string, args ...any) + TempDir() string +} + +func buildApiBlackBoxWithDBState(t TB, sqlFile string, expectedPeerUpdate *server.UpdateMessage) (http.Handler, server.AccountManager, chan struct{}) { + store, cleanup, err := server.NewTestStoreFromSQL(context.Background(), sqlFile, t.TempDir()) + if err != nil { + t.Fatalf("Failed to create test store: %v", err) + } + t.Cleanup(cleanup) + + metrics, err := telemetry.NewDefaultAppMetrics(context.Background()) + + peersUpdateManager := server.NewPeersUpdateManager(nil) + updMsg := peersUpdateManager.CreateChannel(context.Background(), testPeerId) + done := make(chan struct{}) + go func() { + if expectedPeerUpdate != nil { + peerShouldReceiveUpdate(t, updMsg, expectedPeerUpdate) + } else { + peerShouldNotReceiveUpdate(t, updMsg) + } + close(done) + }() + + geoMock := &geolocation.Mock{} + validatorMock := server.MocIntegratedValidator{} + am, err := server.BuildManager(context.Background(), store, peersUpdateManager, nil, "", "", &activity.InMemoryEventStore{}, geoMock, false, validatorMock, metrics) + if err != nil { + t.Fatalf("Failed to create manager: %v", err) + } + + apiHandler, err := APIHandler(context.Background(), am, geoMock, &jwtclaims.JwtValidatorMock{}, metrics, AuthCfg{}, validatorMock) + if err != nil { + t.Fatalf("Failed to create API handler: %v", err) + } + + return apiHandler, am, done +} + +func peerShouldNotReceiveUpdate(t TB, updateMessage <-chan *server.UpdateMessage) { + t.Helper() + select { + case msg := <-updateMessage: + t.Errorf("Unexpected message received: %+v", msg) + case <-time.After(500 * time.Millisecond): + return + } +} + +func peerShouldReceiveUpdate(t TB, updateMessage <-chan *server.UpdateMessage, expected *server.UpdateMessage) { + t.Helper() + + select { + case msg := <-updateMessage: + if msg == nil { + t.Errorf("Received nil update message, expected valid message") + } + assert.Equal(t, expected, msg) + case <-time.After(500 * time.Millisecond): + t.Errorf("Timed out waiting for update message") + } +} + +func buildRequest(t TB, requestBody []byte, requestType, requestPath, user string) *http.Request { + t.Helper() + + req := httptest.NewRequest(requestType, requestPath, bytes.NewBuffer(requestBody)) + req.Header.Set("Authorization", "Bearer "+user) + + return req +} + +func readResponse(t *testing.T, recorder *httptest.ResponseRecorder, expectedStatus int, expectResponse bool) ([]byte, bool) { + t.Helper() + + res := recorder.Result() + defer res.Body.Close() + + content, err := io.ReadAll(res.Body) + if err != nil { + t.Fatalf("Failed to read response body: %v", err) + } + + if !expectResponse { + return nil, false + } + + if status := recorder.Code; status != expectedStatus { + t.Fatalf("handler returned wrong status code: got %v want %v, content: %s", + status, expectedStatus, string(content)) + } + + return content, expectedStatus == http.StatusOK +} + +func populateTestData(b *testing.B, am *server.DefaultAccountManager, peers, groups, users, setupKeys int) { + b.Helper() + + ctx := context.Background() + account, err := am.GetAccount(ctx, testAccountId) + if err != nil { + b.Fatalf("Failed to get account: %v", err) + } + + // Create peers + for i := 0; i < peers; i++ { + peerKey, _ := wgtypes.GeneratePrivateKey() + peer := &nbpeer.Peer{ + ID: fmt.Sprintf("oldpeer-%d", i), + DNSLabel: fmt.Sprintf("oldpeer-%d", i), + Key: peerKey.PublicKey().String(), + IP: net.ParseIP(fmt.Sprintf("100.64.%d.%d", i/256, i%256)), + Status: &nbpeer.PeerStatus{}, + UserID: regularUser, + } + account.Peers[peer.ID] = peer + } + + // Create users + for i := 0; i < users; i++ { + user := &server.User{ + Id: fmt.Sprintf("olduser-%d", i), + AccountID: account.Id, + Role: server.UserRoleUser, + } + account.Users[user.Id] = user + } + + for i := 0; i < setupKeys; i++ { + key := &server.SetupKey{ + Id: fmt.Sprintf("oldkey-%d", i), + AccountID: account.Id, + AutoGroups: []string{"someGroupID"}, + ExpiresAt: time.Now().Add(expiresIn * time.Second), + Name: newKeyName + strconv.Itoa(i), + Type: "reusable", + UsageLimit: 0, + } + account.SetupKeys[key.Id] = key + } + + // Create groups and policies + account.Policies = make([]*server.Policy, 0, groups) + for i := 0; i < groups; i++ { + groupID := fmt.Sprintf("group-%d", i) + group := &nbgroup.Group{ + ID: groupID, + Name: fmt.Sprintf("Group %d", i), + } + for j := 0; j < peers/groups; j++ { + peerIndex := i*(peers/groups) + j + group.Peers = append(group.Peers, fmt.Sprintf("peer-%d", peerIndex)) + } + account.Groups[groupID] = group + + // Create a policy for this group + policy := &server.Policy{ + ID: fmt.Sprintf("policy-%d", i), + Name: fmt.Sprintf("Policy for Group %d", i), + Enabled: true, + Rules: []*server.PolicyRule{ + { + ID: fmt.Sprintf("rule-%d", i), + Name: fmt.Sprintf("Rule for Group %d", i), + Enabled: true, + Sources: []string{groupID}, + Destinations: []string{groupID}, + Bidirectional: true, + Protocol: server.PolicyRuleProtocolALL, + Action: server.PolicyTrafficActionAccept, + }, + }, + } + account.Policies = append(account.Policies, policy) + } + + account.PostureChecks = []*posture.Checks{ + { + ID: "PostureChecksAll", + Name: "All", + Checks: posture.ChecksDefinition{ + NBVersionCheck: &posture.NBVersionCheck{ + MinVersion: "0.0.1", + }, + }, + }, + } + + err = am.Store.SaveAccount(context.Background(), account) + if err != nil { + b.Fatalf("Failed to save account: %v", err) + } + +} From dfc429dbd0c1ecad3943a5756ac9764a9ef105e2 Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Thu, 28 Nov 2024 14:59:10 +0100 Subject: [PATCH 13/38] remove build tags --- management/server/http/setupkeys_handler_benchmark_test.go | 2 -- management/server/http/setupkeys_handler_integration_test.go | 2 -- 2 files changed, 4 deletions(-) diff --git a/management/server/http/setupkeys_handler_benchmark_test.go b/management/server/http/setupkeys_handler_benchmark_test.go index ffa6456ab9c..a6682b474eb 100644 --- a/management/server/http/setupkeys_handler_benchmark_test.go +++ b/management/server/http/setupkeys_handler_benchmark_test.go @@ -1,5 +1,3 @@ -//go:build benchmark - package http import ( diff --git a/management/server/http/setupkeys_handler_integration_test.go b/management/server/http/setupkeys_handler_integration_test.go index ebaf671849d..d946a7a979a 100644 --- a/management/server/http/setupkeys_handler_integration_test.go +++ b/management/server/http/setupkeys_handler_integration_test.go @@ -1,5 +1,3 @@ -//go:build integration - package http import ( From 7c3e4a1516c2cfe57d19ed73d187f54671f0f2ed Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Thu, 28 Nov 2024 15:50:19 +0100 Subject: [PATCH 14/38] fix get test expectation --- management/server/http/setupkeys_handler_integration_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/management/server/http/setupkeys_handler_integration_test.go b/management/server/http/setupkeys_handler_integration_test.go index d946a7a979a..0759c16e834 100644 --- a/management/server/http/setupkeys_handler_integration_test.go +++ b/management/server/http/setupkeys_handler_integration_test.go @@ -754,8 +754,8 @@ func Test_SetupKeys_Get(t *testing.T) { apiHandler.ServeHTTP(recorder, req) - content, noResponseExpected := readResponse(t, recorder, tc.expectedStatus, user.expectResponse) - if noResponseExpected { + content, expectRespnose := readResponse(t, recorder, tc.expectedStatus, user.expectResponse) + if !expectRespnose { return } got := &api.SetupKey{} From 1e0ce801d05e97c5850636c4e8c01893b8bd7945 Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Thu, 28 Nov 2024 15:55:43 +0100 Subject: [PATCH 15/38] fix error handling in metrics dummy --- management/server/http/tools_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/management/server/http/tools_test.go b/management/server/http/tools_test.go index f525790621c..2784fed7bba 100644 --- a/management/server/http/tools_test.go +++ b/management/server/http/tools_test.go @@ -41,6 +41,9 @@ func buildApiBlackBoxWithDBState(t TB, sqlFile string, expectedPeerUpdate *serve t.Cleanup(cleanup) metrics, err := telemetry.NewDefaultAppMetrics(context.Background()) + if err != nil { + t.Fatalf("Failed to create metrics: %v", err) + } peersUpdateManager := server.NewPeersUpdateManager(nil) updMsg := peersUpdateManager.CreateChannel(context.Background(), testPeerId) From e4bdb1e757e0a06dac98ac7d3dd8e0961c189682 Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Thu, 28 Nov 2024 16:19:42 +0100 Subject: [PATCH 16/38] fix time location --- management/server/http/setupkeys_handler_integration_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/management/server/http/setupkeys_handler_integration_test.go b/management/server/http/setupkeys_handler_integration_test.go index 0759c16e834..0ee55b9b300 100644 --- a/management/server/http/setupkeys_handler_integration_test.go +++ b/management/server/http/setupkeys_handler_integration_test.go @@ -1137,5 +1137,8 @@ func validateCreatedKey(t *testing.T, expectedKey *api.SetupKey, got *api.SetupK expectedKey.UpdatedAt = time.Time{} } + expectedKey.UpdatedAt = expectedKey.UpdatedAt.In(time.UTC) + got.UpdatedAt = got.UpdatedAt.In(time.UTC) + assert.Equal(t, expectedKey, got) } From 0149e751ce447a744c8401dc0555141cec16dcea Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Thu, 28 Nov 2024 19:06:55 +0100 Subject: [PATCH 17/38] fix time location --- .../http/setupkeys_handler_integration_test.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/management/server/http/setupkeys_handler_integration_test.go b/management/server/http/setupkeys_handler_integration_test.go index 0ee55b9b300..85ed3dbf9e4 100644 --- a/management/server/http/setupkeys_handler_integration_test.go +++ b/management/server/http/setupkeys_handler_integration_test.go @@ -681,7 +681,7 @@ func Test_SetupKeys_Get(t *testing.T) { Revoked: false, State: "valid", Type: "one-off", - UpdatedAt: time.Date(2021, time.August, 19, 20, 46, 20, 5936822, time.Local), + UpdatedAt: time.Date(2021, time.August, 19, 18, 46, 20, 5936822, time.UTC), UsageLimit: 1, UsedTimes: 0, Valid: true, @@ -704,7 +704,7 @@ func Test_SetupKeys_Get(t *testing.T) { Revoked: false, State: "expired", Type: "reusable", - UpdatedAt: time.Date(2021, time.August, 19, 20, 46, 20, 5936822, time.Local), + UpdatedAt: time.Date(2021, time.August, 19, 18, 46, 20, 5936822, time.UTC), UsageLimit: 5, UsedTimes: 1, Valid: false, @@ -727,7 +727,7 @@ func Test_SetupKeys_Get(t *testing.T) { Revoked: true, State: "revoked", Type: "reusable", - UpdatedAt: time.Date(2021, time.August, 19, 20, 46, 20, 5936822, time.Local), + UpdatedAt: time.Date(2021, time.August, 19, 18, 46, 20, 5936822, time.UTC), UsageLimit: 3, UsedTimes: 0, Valid: false, @@ -854,7 +854,7 @@ func Test_SetupKeys_GetAll(t *testing.T) { Revoked: false, State: "valid", Type: "one-off", - UpdatedAt: time.Date(2021, time.August, 19, 20, 46, 20, 5936822, time.Local), + UpdatedAt: time.Date(2021, time.August, 19, 18, 46, 20, 5936822, time.UTC), UsageLimit: 1, UsedTimes: 0, Valid: true, @@ -870,7 +870,7 @@ func Test_SetupKeys_GetAll(t *testing.T) { Revoked: true, State: "revoked", Type: "reusable", - UpdatedAt: time.Date(2021, time.August, 19, 20, 46, 20, 5936822, time.Local), + UpdatedAt: time.Date(2021, time.August, 19, 18, 46, 20, 5936822, time.UTC), UsageLimit: 3, UsedTimes: 0, Valid: false, @@ -886,7 +886,7 @@ func Test_SetupKeys_GetAll(t *testing.T) { Revoked: false, State: "expired", Type: "reusable", - UpdatedAt: time.Date(2021, time.August, 19, 20, 46, 20, 5936822, time.Local), + UpdatedAt: time.Date(2021, time.August, 19, 18, 46, 20, 5936822, time.UTC), UsageLimit: 5, UsedTimes: 1, Valid: false, @@ -1017,7 +1017,7 @@ func Test_SetupKeys_Delete(t *testing.T) { Revoked: false, State: "valid", Type: "one-off", - UpdatedAt: time.Date(2021, time.August, 19, 20, 46, 20, 5936822, time.Local), + UpdatedAt: time.Date(2021, time.August, 19, 20, 46, 20, 5936822, time.UTC), UsageLimit: 1, UsedTimes: 0, Valid: true, @@ -1040,7 +1040,7 @@ func Test_SetupKeys_Delete(t *testing.T) { Revoked: false, State: "expired", Type: "reusable", - UpdatedAt: time.Date(2021, time.August, 19, 20, 46, 20, 5936822, time.Local), + UpdatedAt: time.Date(2021, time.August, 19, 20, 46, 20, 5936822, time.UTC), UsageLimit: 5, UsedTimes: 1, Valid: false, @@ -1063,7 +1063,7 @@ func Test_SetupKeys_Delete(t *testing.T) { Revoked: true, State: "revoked", Type: "reusable", - UpdatedAt: time.Date(2021, time.August, 19, 20, 46, 20, 5936822, time.Local), + UpdatedAt: time.Date(2021, time.August, 19, 20, 46, 20, 5936822, time.UTC), UsageLimit: 3, UsedTimes: 0, Valid: false, From f389150d7e6c9f3f171a47d41ed3bf0fc5acf57c Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Thu, 28 Nov 2024 19:41:24 +0100 Subject: [PATCH 18/38] fix time location --- .../setupkeys_handler_integration_test.go | 18 +++++++-------- .../server/http/testdata/setup_keys.sql | 22 +++++++++---------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/management/server/http/setupkeys_handler_integration_test.go b/management/server/http/setupkeys_handler_integration_test.go index 85ed3dbf9e4..73097816714 100644 --- a/management/server/http/setupkeys_handler_integration_test.go +++ b/management/server/http/setupkeys_handler_integration_test.go @@ -681,7 +681,7 @@ func Test_SetupKeys_Get(t *testing.T) { Revoked: false, State: "valid", Type: "one-off", - UpdatedAt: time.Date(2021, time.August, 19, 18, 46, 20, 5936822, time.UTC), + UpdatedAt: time.Date(2021, time.August, 19, 20, 46, 20, 0, time.UTC), UsageLimit: 1, UsedTimes: 0, Valid: true, @@ -704,7 +704,7 @@ func Test_SetupKeys_Get(t *testing.T) { Revoked: false, State: "expired", Type: "reusable", - UpdatedAt: time.Date(2021, time.August, 19, 18, 46, 20, 5936822, time.UTC), + UpdatedAt: time.Date(2021, time.August, 19, 20, 46, 20, 0, time.UTC), UsageLimit: 5, UsedTimes: 1, Valid: false, @@ -727,7 +727,7 @@ func Test_SetupKeys_Get(t *testing.T) { Revoked: true, State: "revoked", Type: "reusable", - UpdatedAt: time.Date(2021, time.August, 19, 18, 46, 20, 5936822, time.UTC), + UpdatedAt: time.Date(2021, time.August, 19, 20, 46, 20, 0, time.UTC), UsageLimit: 3, UsedTimes: 0, Valid: false, @@ -854,7 +854,7 @@ func Test_SetupKeys_GetAll(t *testing.T) { Revoked: false, State: "valid", Type: "one-off", - UpdatedAt: time.Date(2021, time.August, 19, 18, 46, 20, 5936822, time.UTC), + UpdatedAt: time.Date(2021, time.August, 19, 20, 46, 20, 0, time.UTC), UsageLimit: 1, UsedTimes: 0, Valid: true, @@ -870,7 +870,7 @@ func Test_SetupKeys_GetAll(t *testing.T) { Revoked: true, State: "revoked", Type: "reusable", - UpdatedAt: time.Date(2021, time.August, 19, 18, 46, 20, 5936822, time.UTC), + UpdatedAt: time.Date(2021, time.August, 19, 20, 46, 20, 0, time.UTC), UsageLimit: 3, UsedTimes: 0, Valid: false, @@ -886,7 +886,7 @@ func Test_SetupKeys_GetAll(t *testing.T) { Revoked: false, State: "expired", Type: "reusable", - UpdatedAt: time.Date(2021, time.August, 19, 18, 46, 20, 5936822, time.UTC), + UpdatedAt: time.Date(2021, time.August, 19, 20, 46, 20, 0, time.UTC), UsageLimit: 5, UsedTimes: 1, Valid: false, @@ -1017,7 +1017,7 @@ func Test_SetupKeys_Delete(t *testing.T) { Revoked: false, State: "valid", Type: "one-off", - UpdatedAt: time.Date(2021, time.August, 19, 20, 46, 20, 5936822, time.UTC), + UpdatedAt: time.Date(2021, time.August, 19, 20, 46, 20, 0, time.UTC), UsageLimit: 1, UsedTimes: 0, Valid: true, @@ -1040,7 +1040,7 @@ func Test_SetupKeys_Delete(t *testing.T) { Revoked: false, State: "expired", Type: "reusable", - UpdatedAt: time.Date(2021, time.August, 19, 20, 46, 20, 5936822, time.UTC), + UpdatedAt: time.Date(2021, time.August, 19, 20, 46, 20, 0, time.UTC), UsageLimit: 5, UsedTimes: 1, Valid: false, @@ -1063,7 +1063,7 @@ func Test_SetupKeys_Delete(t *testing.T) { Revoked: true, State: "revoked", Type: "reusable", - UpdatedAt: time.Date(2021, time.August, 19, 20, 46, 20, 5936822, time.UTC), + UpdatedAt: time.Date(2021, time.August, 19, 20, 46, 20, 0, time.UTC), UsageLimit: 3, UsedTimes: 0, Valid: false, diff --git a/management/server/http/testdata/setup_keys.sql b/management/server/http/testdata/setup_keys.sql index c0add32680b..a315ea0f701 100644 --- a/management/server/http/testdata/setup_keys.sql +++ b/management/server/http/testdata/setup_keys.sql @@ -3,14 +3,14 @@ CREATE TABLE `users` (`id` text,`account_id` text,`role` text,`is_service_user` CREATE TABLE `peers` (`id` text,`account_id` text,`key` text,`setup_key` text,`ip` text,`meta_hostname` text,`meta_go_os` text,`meta_kernel` text,`meta_core` text,`meta_platform` text,`meta_os` text,`meta_os_version` text,`meta_wt_version` text,`meta_ui_version` text,`meta_kernel_version` text,`meta_network_addresses` text,`meta_system_serial_number` text,`meta_system_product_name` text,`meta_system_manufacturer` text,`meta_environment` text,`meta_files` text,`name` text,`dns_label` text,`peer_status_last_seen` datetime,`peer_status_connected` numeric,`peer_status_login_expired` numeric,`peer_status_requires_approval` numeric,`user_id` text,`ssh_key` text,`ssh_enabled` numeric,`login_expiration_enabled` numeric,`last_login` datetime,`created_at` datetime,`ephemeral` numeric,`location_connection_ip` text,`location_country_code` text,`location_city_name` text,`location_geo_name_id` integer,PRIMARY KEY (`id`),CONSTRAINT `fk_accounts_peers_g` FOREIGN KEY (`account_id`) REFERENCES `accounts`(`id`)); CREATE TABLE `groups` (`id` text,`account_id` text,`name` text,`issued` text,`peers` text,`integration_ref_id` integer,`integration_ref_integration_type` text,PRIMARY KEY (`id`),CONSTRAINT `fk_accounts_groups_g` FOREIGN KEY (`account_id`) REFERENCES `accounts`(`id`)); -INSERT INTO accounts VALUES('testAccountId','','2024-10-02 16:01:38.210014+02:00','test.com','private',1,'testNetworkIdentifier','{"IP":"100.64.0.0","Mask":"//8AAA=="}','',0,'[]',0,86400000000000,0,0,0,'',NULL,NULL,NULL); -INSERT INTO users VALUES('testUserId','testAccountId','user',0,0,'','[]',0,'0001-01-01 00:00:00+00:00','2024-10-02 16:01:38.210678+02:00','api',0,''); -INSERT INTO users VALUES('testAdminId','testAccountId','admin',0,0,'','[]',0,'0001-01-01 00:00:00+00:00','2024-10-02 16:01:38.210678+02:00','api',0,''); -INSERT INTO users VALUES('testOwnerId','testAccountId','owner',0,0,'','[]',0,'0001-01-01 00:00:00+00:00','2024-10-02 16:01:38.210678+02:00','api',0,''); -INSERT INTO users VALUES('testServiceUserId','testAccountId','user',1,0,'','[]',0,'0001-01-01 00:00:00+00:00','2024-10-02 16:01:38.210678+02:00','api',0,''); -INSERT INTO users VALUES('testServiceAdminId','testAccountId','admin',1,0,'','[]',0,'0001-01-01 00:00:00+00:00','2024-10-02 16:01:38.210678+02:00','api',0,''); -INSERT INTO users VALUES('blockedUserId','testAccountId','admin',0,0,'','[]',1,'0001-01-01 00:00:00+00:00','2024-10-02 16:01:38.210678+02:00','api',0,''); -INSERT INTO users VALUES('otherUserId','otherAccountId','admin',0,0,'','[]',0,'0001-01-01 00:00:00+00:00','2024-10-02 16:01:38.210678+02:00','api',0,''); +INSERT INTO accounts VALUES('testAccountId','','2024-10-02 16:01:38.000000000+00:00','test.com','private',1,'testNetworkIdentifier','{"IP":"100.64.0.0","Mask":"//8AAA=="}','',0,'[]',0,86400000000000,0,0,0,'',NULL,NULL,NULL); +INSERT INTO users VALUES('testUserId','testAccountId','user',0,0,'','[]',0,'0001-01-01 00:00:00+00:00','2024-10-02 16:01:38.000000000+00:00','api',0,''); +INSERT INTO users VALUES('testAdminId','testAccountId','admin',0,0,'','[]',0,'0001-01-01 00:00:00+00:00','2024-10-02 16:01:38.000000000+00:00','api',0,''); +INSERT INTO users VALUES('testOwnerId','testAccountId','owner',0,0,'','[]',0,'0001-01-01 00:00:00+00:00','2024-10-02 16:01:38.000000000+00:00','api',0,''); +INSERT INTO users VALUES('testServiceUserId','testAccountId','user',1,0,'','[]',0,'0001-01-01 00:00:00+00:00','2024-10-02 16:01:38.000000000+00:00','api',0,''); +INSERT INTO users VALUES('testServiceAdminId','testAccountId','admin',1,0,'','[]',0,'0001-01-01 00:00:00+00:00','2024-10-02 16:01:38.000000000+00:00','api',0,''); +INSERT INTO users VALUES('blockedUserId','testAccountId','admin',0,0,'','[]',1,'0001-01-01 00:00:00+00:00','2024-10-02 16:01:38.000000000+00:00','api',0,''); +INSERT INTO users VALUES('otherUserId','otherAccountId','admin',0,0,'','[]',0,'0001-01-01 00:00:00+00:00','2024-10-02 16:01:38.000000000+00:00','api',0,''); INSERT INTO peers VALUES('testPeerId','testAccountId','5rvhvriKJZ3S9oxYToVj5TzDM9u9y8cxg7htIMWlYAg=','72546A29-6BC8-4311-BCFC-9CDBF33F1A48','"100.64.114.31"','f2a34f6a4731','linux','Linux','11','unknown','Debian GNU/Linux','','0.12.0','','',NULL,'','','','{"Cloud":"","Platform":""}',NULL,'f2a34f6a4731','f2a34f6a4731','2023-03-02 09:21:02.189035775+01:00',0,0,0,'','ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAILzUUSYG/LGnV8zarb2SGN+tib/PZ+M7cL4WtTzUrTpk',0,1,'2023-03-01 19:48:19.817799698+01:00','2024-10-02 17:00:32.527947+02:00',0,'""','','',0); INSERT INTO "groups" VALUES('testGroupId','testAccountId','testGroupName','api','[]',0,''); INSERT INTO "groups" VALUES('newGroupId','testAccountId','newGroupName','api','[]',0,''); @@ -18,7 +18,7 @@ INSERT INTO "groups" VALUES('newGroupId','testAccountId','newGroupName','api','[ CREATE TABLE `setup_keys` (`id` text,`account_id` text,`key` text,`key_secret` text,`name` text,`type` text,`created_at` datetime,`expires_at` datetime,`updated_at` datetime,`revoked` numeric,`used_times` integer,`last_used` datetime,`auto_groups` text,`usage_limit` integer,`ephemeral` numeric,PRIMARY KEY (`id`),CONSTRAINT `fk_accounts_setup_keys_g` FOREIGN KEY (`account_id`) REFERENCES `accounts`(`id`)); -INSERT INTO setup_keys VALUES('testKeyId','testAccountId','testKey','testK****','existingKey','one-off','2021-08-19 20:46:20.005936822+02:00','2321-09-18 20:46:20.005936822+02:00','2021-08-19 20:46:20.005936822+02:00',0,0,'0001-01-01 00:00:00+00:00','["testGroupId"]',1,0); -INSERT INTO setup_keys VALUES('revokedKeyId','testAccountId','revokedKey','testK****','existingKey','reusable','2021-08-19 20:46:20.005936822+02:00','2321-09-18 20:46:20.005936822+02:00','2021-08-19 20:46:20.005936822+02:00',1,0,'0001-01-01 00:00:00+00:00','["testGroupId"]',3,0); -INSERT INTO setup_keys VALUES('expiredKeyId','testAccountId','expiredKey','testK****','existingKey','reusable','2021-08-19 20:46:20.005936822+02:00','1921-09-18 20:46:20.005936822+02:00','2021-08-19 20:46:20.005936822+02:00',0,1,'0001-01-01 00:00:00+00:00','["testGroupId"]',5,1); +INSERT INTO setup_keys VALUES('testKeyId','testAccountId','testKey','testK****','existingKey','one-off','2021-08-19 20:46:20.000000000+00:00','2321-09-18 20:46:20.000000000+00:00','2021-08-19 20:46:20.000000000+00:000',0,0,'0001-01-01 00:00:00+00:00','["testGroupId"]',1,0); +INSERT INTO setup_keys VALUES('revokedKeyId','testAccountId','revokedKey','testK****','existingKey','reusable','2021-08-19 20:46:20.000000000+00:00','2321-09-18 20:46:20.000000000+00:00','2021-08-19 20:46:20.000000000+00:00',1,0,'0001-01-01 00:00:00+00:00','["testGroupId"]',3,0); +INSERT INTO setup_keys VALUES('expiredKeyId','testAccountId','expiredKey','testK****','existingKey','reusable','2021-08-19 20:46:20.000000000+00:00','1921-09-18 20:46:20.000000000+00:00','2021-08-19 20:46:20.000000000+00:00',0,1,'0001-01-01 00:00:00+00:00','["testGroupId"]',5,1); From 95be512982442cacac57eddd09b93c3299bca068 Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Fri, 29 Nov 2024 12:04:36 +0100 Subject: [PATCH 19/38] update expected benchmark timings --- management/server/account_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/management/server/account_test.go b/management/server/account_test.go index 2c7d3e62498..8d44c664b7e 100644 --- a/management/server/account_test.go +++ b/management/server/account_test.go @@ -2956,10 +2956,10 @@ func BenchmarkSyncAndMarkPeer(b *testing.B) { minMsPerOpCICD float64 maxMsPerOpCICD float64 }{ - {"Small", 50, 5, 1, 3, 4, 10}, + {"Small", 50, 5, 1, 3, 3, 10}, {"Medium", 500, 100, 7, 13, 10, 60}, {"Large", 5000, 200, 65, 80, 60, 170}, - {"Small single", 50, 10, 1, 3, 4, 60}, + {"Small single", 50, 10, 1, 3, 3, 60}, {"Medium single", 500, 10, 7, 13, 10, 26}, {"Large 5", 5000, 15, 65, 80, 60, 170}, } From 83d2af3139d5c91d38401995f611bf493c9fe8b0 Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Fri, 29 Nov 2024 12:26:42 +0100 Subject: [PATCH 20/38] update expected benchmark timings --- .../http/setupkeys_handler_benchmark_test.go | 210 +++++++++++------- 1 file changed, 130 insertions(+), 80 deletions(-) diff --git a/management/server/http/setupkeys_handler_benchmark_test.go b/management/server/http/setupkeys_handler_benchmark_test.go index a6682b474eb..4c2cd89e8cd 100644 --- a/management/server/http/setupkeys_handler_benchmark_test.go +++ b/management/server/http/setupkeys_handler_benchmark_test.go @@ -19,19 +19,22 @@ import ( func BenchmarkCreateSetupKey(b *testing.B) { benchCases := []struct { - name string - peers int - groups int - users int - setupKeys int - minMsPerOp float64 - maxMsPerOp float64 + name string + peers int + groups int + users int + setupKeys int + // We need different expectations for CI/CD and local runs because of the different performance characteristics + minMsPerOpLocal float64 + maxMsPerOpLocal float64 + minMsPerOpCICD float64 + maxMsPerOpCICD float64 }{ - {"Setup Keys - XS", 5, 5, 5, 5, 0.5, 2}, - {"Setup Keys - S", 5, 5, 5, 5, 0.5, 2}, - {"Setup Keys - M", 100, 20, 20, 100, 0.5, 2}, - {"Setup Keys - L", 500, 50, 100, 1000, 0.5, 2}, - {"Setup Keys - XL", 500, 50, 100, 5000, 0.5, 2}, + {"Setup Keys - XS", 5, 5, 5, 5, 0.5, 2, 2, 5}, + {"Setup Keys - S", 5, 5, 5, 5, 0.5, 2, 2, 5}, + {"Setup Keys - M", 100, 20, 20, 100, 0.5, 2, 2, 5}, + {"Setup Keys - L", 500, 50, 100, 1000, 0.5, 2, 2, 5}, + {"Setup Keys - XL", 500, 50, 100, 5000, 0.5, 2, 2, 5}, } log.SetOutput(io.Discard) @@ -67,12 +70,19 @@ func BenchmarkCreateSetupKey(b *testing.B) { msPerOp := float64(duration.Nanoseconds()) / float64(b.N) / 1e6 b.ReportMetric(msPerOp, "ms/op") - if msPerOp < bc.minMsPerOp { - b.Fatalf("Benchmark %s failed: too fast (%.2f ms/op, minimum %.2f ms/op)", bc.name, msPerOp, bc.minMsPerOp) + minExpected := bc.minMsPerOpLocal + maxExpected := bc.maxMsPerOpLocal + if os.Getenv("CI") == "true" { + minExpected = bc.minMsPerOpCICD + maxExpected = bc.maxMsPerOpCICD } - if msPerOp > bc.maxMsPerOp { - b.Fatalf("Benchmark %s failed: too slow (%.2f ms/op, maximum %.2f ms/op)", bc.name, msPerOp, bc.maxMsPerOp) + if msPerOp < minExpected { + b.Fatalf("Benchmark %s failed: too fast (%.2f ms/op, minimum %.2f ms/op)", bc.name, msPerOp, minExpected) + } + + if msPerOp > maxExpected { + b.Fatalf("Benchmark %s failed: too slow (%.2f ms/op, maximum %.2f ms/op)", bc.name, msPerOp, maxExpected) } }) } @@ -80,19 +90,22 @@ func BenchmarkCreateSetupKey(b *testing.B) { func BenchmarkUpdateSetupKey(b *testing.B) { benchCases := []struct { - name string - peers int - groups int - users int - setupKeys int - minMsPerOp float64 - maxMsPerOp float64 + name string + peers int + groups int + users int + setupKeys int + // We need different expectations for CI/CD and local runs because of the different performance characteristics + minMsPerOpLocal float64 + maxMsPerOpLocal float64 + minMsPerOpCICD float64 + maxMsPerOpCICD float64 }{ - {"Setup Keys - XS", 5, 5, 5, 5, 0.5, 2}, - {"Setup Keys - S", 5, 5, 5, 5, 0.5, 2}, - {"Setup Keys - M", 100, 20, 20, 100, 0.5, 2}, - {"Setup Keys - L", 500, 50, 100, 1000, 0.5, 2}, - {"Setup Keys - XL", 500, 50, 100, 5000, 0.5, 2}, + {"Setup Keys - XS", 5, 5, 5, 5, 0.5, 2, 3, 7}, + {"Setup Keys - S", 5, 5, 5, 5, 0.5, 2, 3, 7}, + {"Setup Keys - M", 100, 20, 20, 100, 0.5, 2, 3, 7}, + {"Setup Keys - L", 500, 50, 100, 1000, 0.5, 2, 3, 7}, + {"Setup Keys - XL", 500, 50, 100, 5000, 0.5, 2, 3, 7}, } log.SetOutput(io.Discard) @@ -129,12 +142,19 @@ func BenchmarkUpdateSetupKey(b *testing.B) { msPerOp := float64(duration.Nanoseconds()) / float64(b.N) / 1e6 b.ReportMetric(msPerOp, "ms/op") - if msPerOp < bc.minMsPerOp { - b.Fatalf("Benchmark %s failed: too fast (%.2f ms/op, minimum %.2f ms/op)", bc.name, msPerOp, bc.minMsPerOp) + minExpected := bc.minMsPerOpLocal + maxExpected := bc.maxMsPerOpLocal + if os.Getenv("CI") == "true" { + minExpected = bc.minMsPerOpCICD + maxExpected = bc.maxMsPerOpCICD } - if msPerOp > bc.maxMsPerOp { - b.Fatalf("Benchmark %s failed: too slow (%.2f ms/op, maximum %.2f ms/op)", bc.name, msPerOp, bc.maxMsPerOp) + if msPerOp < minExpected { + b.Fatalf("Benchmark %s failed: too fast (%.2f ms/op, minimum %.2f ms/op)", bc.name, msPerOp, minExpected) + } + + if msPerOp > maxExpected { + b.Fatalf("Benchmark %s failed: too slow (%.2f ms/op, maximum %.2f ms/op)", bc.name, msPerOp, maxExpected) } }) } @@ -142,19 +162,22 @@ func BenchmarkUpdateSetupKey(b *testing.B) { func BenchmarkGetOneSetupKey(b *testing.B) { benchCases := []struct { - name string - peers int - groups int - users int - setupKeys int - minMsPerOp float64 - maxMsPerOp float64 + name string + peers int + groups int + users int + setupKeys int + // We need different expectations for CI/CD and local runs because of the different performance characteristics + minMsPerOpLocal float64 + maxMsPerOpLocal float64 + minMsPerOpCICD float64 + maxMsPerOpCICD float64 }{ - {"Setup Keys - XS", 5, 5, 5, 5, 0.5, 2}, - {"Setup Keys - S", 5, 5, 5, 5, 0.5, 2}, - {"Setup Keys - M", 100, 20, 20, 100, 0.5, 2}, - {"Setup Keys - L", 500, 50, 100, 1000, 0.5, 2}, - {"Setup Keys - XL", 500, 50, 100, 5000, 0.5, 2}, + {"Setup Keys - XS", 5, 5, 5, 5, 0.5, 2, 2, 4}, + {"Setup Keys - S", 5, 5, 5, 5, 0.5, 2, 2, 4}, + {"Setup Keys - M", 100, 20, 20, 100, 0.5, 2, 2, 4}, + {"Setup Keys - L", 500, 50, 100, 1000, 0.5, 2, 2, 4}, + {"Setup Keys - XL", 500, 50, 100, 5000, 0.5, 2, 2, 4}, } log.SetOutput(io.Discard) @@ -178,12 +201,19 @@ func BenchmarkGetOneSetupKey(b *testing.B) { msPerOp := float64(duration.Nanoseconds()) / float64(b.N) / 1e6 b.ReportMetric(msPerOp, "ms/op") - if msPerOp < bc.minMsPerOp { - b.Fatalf("Benchmark %s failed: too fast (%.2f ms/op, minimum %.2f ms/op)", bc.name, msPerOp, bc.minMsPerOp) + minExpected := bc.minMsPerOpLocal + maxExpected := bc.maxMsPerOpLocal + if os.Getenv("CI") == "true" { + minExpected = bc.minMsPerOpCICD + maxExpected = bc.maxMsPerOpCICD + } + + if msPerOp < minExpected { + b.Fatalf("Benchmark %s failed: too fast (%.2f ms/op, minimum %.2f ms/op)", bc.name, msPerOp, minExpected) } - if msPerOp > bc.maxMsPerOp { - b.Fatalf("Benchmark %s failed: too slow (%.2f ms/op, maximum %.2f ms/op)", bc.name, msPerOp, bc.maxMsPerOp) + if msPerOp > maxExpected { + b.Fatalf("Benchmark %s failed: too slow (%.2f ms/op, maximum %.2f ms/op)", bc.name, msPerOp, maxExpected) } }) } @@ -191,19 +221,22 @@ func BenchmarkGetOneSetupKey(b *testing.B) { func BenchmarkGetAllSetupKeys(b *testing.B) { benchCases := []struct { - name string - peers int - groups int - users int - setupKeys int - minMsPerOp float64 - maxMsPerOp float64 + name string + peers int + groups int + users int + setupKeys int + // We need different expectations for CI/CD and local runs because of the different performance characteristics + minMsPerOpLocal float64 + maxMsPerOpLocal float64 + minMsPerOpCICD float64 + maxMsPerOpCICD float64 }{ - {"Setup Keys - XS", 5, 5, 5, 5, 0.5, 2}, - {"Setup Keys - S", 5, 5, 5, 5, 0.5, 2}, - {"Setup Keys - M", 100, 20, 20, 100, 0.5, 2}, - {"Setup Keys - L", 500, 50, 100, 1000, 5, 10}, - {"Setup Keys - XL", 500, 50, 100, 5000, 25, 45}, + {"Setup Keys - XS", 5, 5, 5, 5, 0.5, 2, 1, 10}, + {"Setup Keys - S", 5, 5, 5, 5, 0.5, 2, 1, 10}, + {"Setup Keys - M", 100, 20, 20, 100, 0.5, 2, 3, 15}, + {"Setup Keys - L", 500, 50, 100, 1000, 5, 10, 10, 25}, + {"Setup Keys - XL", 500, 50, 100, 5000, 25, 45, 50, 150}, } log.SetOutput(io.Discard) @@ -227,12 +260,19 @@ func BenchmarkGetAllSetupKeys(b *testing.B) { msPerOp := float64(duration.Nanoseconds()) / float64(b.N) / 1e6 b.ReportMetric(msPerOp, "ms/op") - if msPerOp < bc.minMsPerOp { - b.Fatalf("Benchmark %s failed: too fast (%.2f ms/op, minimum %.2f ms/op)", bc.name, msPerOp, bc.minMsPerOp) + minExpected := bc.minMsPerOpLocal + maxExpected := bc.maxMsPerOpLocal + if os.Getenv("CI") == "true" { + minExpected = bc.minMsPerOpCICD + maxExpected = bc.maxMsPerOpCICD + } + + if msPerOp < minExpected { + b.Fatalf("Benchmark %s failed: too fast (%.2f ms/op, minimum %.2f ms/op)", bc.name, msPerOp, minExpected) } - if msPerOp > bc.maxMsPerOp { - b.Fatalf("Benchmark %s failed: too slow (%.2f ms/op, maximum %.2f ms/op)", bc.name, msPerOp, bc.maxMsPerOp) + if msPerOp > maxExpected { + b.Fatalf("Benchmark %s failed: too slow (%.2f ms/op, maximum %.2f ms/op)", bc.name, msPerOp, maxExpected) } }) } @@ -240,19 +280,22 @@ func BenchmarkGetAllSetupKeys(b *testing.B) { func BenchmarkDeleteSetupKey(b *testing.B) { benchCases := []struct { - name string - peers int - groups int - users int - setupKeys int - minMsPerOp float64 - maxMsPerOp float64 + name string + peers int + groups int + users int + setupKeys int + // We need different expectations for CI/CD and local runs because of the different performance characteristics + minMsPerOpLocal float64 + maxMsPerOpLocal float64 + minMsPerOpCICD float64 + maxMsPerOpCICD float64 }{ - {"Setup Keys - XS", 5, 5, 5, 5, 0.5, 2}, - {"Setup Keys - S", 5, 5, 5, 5, 0.5, 2}, - {"Setup Keys - M", 100, 20, 20, 100, 0.5, 2}, - {"Setup Keys - L", 500, 50, 100, 1000, 0.5, 2}, - {"Setup Keys - XL", 500, 50, 100, 5000, 0.5, 2}, + {"Setup Keys - XS", 5, 5, 5, 5, 0.5, 2, 2, 5}, + {"Setup Keys - S", 5, 5, 5, 5, 0.5, 2, 2, 5}, + {"Setup Keys - M", 100, 20, 20, 100, 0.5, 2, 2, 5}, + {"Setup Keys - L", 500, 50, 100, 1000, 0.5, 2, 2, 5}, + {"Setup Keys - XL", 500, 50, 100, 5000, 0.5, 2, 2, 5}, } log.SetOutput(io.Discard) @@ -277,12 +320,19 @@ func BenchmarkDeleteSetupKey(b *testing.B) { msPerOp := float64(duration.Nanoseconds()) / float64(b.N) / 1e6 b.ReportMetric(msPerOp, "ms/op") - if msPerOp < bc.minMsPerOp { - b.Fatalf("Benchmark %s failed: too fast (%.2f ms/op, minimum %.2f ms/op)", bc.name, msPerOp, bc.minMsPerOp) + minExpected := bc.minMsPerOpLocal + maxExpected := bc.maxMsPerOpLocal + if os.Getenv("CI") == "true" { + minExpected = bc.minMsPerOpCICD + maxExpected = bc.maxMsPerOpCICD + } + + if msPerOp < minExpected { + b.Fatalf("Benchmark %s failed: too fast (%.2f ms/op, minimum %.2f ms/op)", bc.name, msPerOp, minExpected) } - if msPerOp > bc.maxMsPerOp { - b.Fatalf("Benchmark %s failed: too slow (%.2f ms/op, maximum %.2f ms/op)", bc.name, msPerOp, bc.maxMsPerOp) + if msPerOp > maxExpected { + b.Fatalf("Benchmark %s failed: too slow (%.2f ms/op, maximum %.2f ms/op)", bc.name, msPerOp, maxExpected) } }) } From 49037a7ac8b316fa13427d39d8deaa28ec3e4bd1 Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Fri, 29 Nov 2024 13:38:33 +0100 Subject: [PATCH 21/38] update expected benchmark timings --- .../http/setupkeys_handler_benchmark_test.go | 40 +++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/management/server/http/setupkeys_handler_benchmark_test.go b/management/server/http/setupkeys_handler_benchmark_test.go index 4c2cd89e8cd..ad0e51e26a6 100644 --- a/management/server/http/setupkeys_handler_benchmark_test.go +++ b/management/server/http/setupkeys_handler_benchmark_test.go @@ -30,11 +30,11 @@ func BenchmarkCreateSetupKey(b *testing.B) { minMsPerOpCICD float64 maxMsPerOpCICD float64 }{ - {"Setup Keys - XS", 5, 5, 5, 5, 0.5, 2, 2, 5}, - {"Setup Keys - S", 5, 5, 5, 5, 0.5, 2, 2, 5}, - {"Setup Keys - M", 100, 20, 20, 100, 0.5, 2, 2, 5}, - {"Setup Keys - L", 500, 50, 100, 1000, 0.5, 2, 2, 5}, - {"Setup Keys - XL", 500, 50, 100, 5000, 0.5, 2, 2, 5}, + {"Setup Keys - XS", 5, 5, 5, 5, 0.5, 2, 2, 12}, + {"Setup Keys - S", 5, 5, 5, 5, 0.5, 2, 2, 12}, + {"Setup Keys - M", 100, 20, 20, 100, 0.5, 2, 2, 12}, + {"Setup Keys - L", 500, 50, 100, 1000, 0.5, 2, 2, 12}, + {"Setup Keys - XL", 500, 50, 100, 5000, 0.5, 2, 2, 12}, } log.SetOutput(io.Discard) @@ -101,11 +101,11 @@ func BenchmarkUpdateSetupKey(b *testing.B) { minMsPerOpCICD float64 maxMsPerOpCICD float64 }{ - {"Setup Keys - XS", 5, 5, 5, 5, 0.5, 2, 3, 7}, - {"Setup Keys - S", 5, 5, 5, 5, 0.5, 2, 3, 7}, - {"Setup Keys - M", 100, 20, 20, 100, 0.5, 2, 3, 7}, - {"Setup Keys - L", 500, 50, 100, 1000, 0.5, 2, 3, 7}, - {"Setup Keys - XL", 500, 50, 100, 5000, 0.5, 2, 3, 7}, + {"Setup Keys - XS", 5, 5, 5, 5, 0.5, 2, 3, 15}, + {"Setup Keys - S", 5, 5, 5, 5, 0.5, 2, 3, 15}, + {"Setup Keys - M", 100, 20, 20, 100, 0.5, 2, 3, 15}, + {"Setup Keys - L", 500, 50, 100, 1000, 0.5, 2, 3, 15}, + {"Setup Keys - XL", 500, 50, 100, 5000, 0.5, 2, 3, 15}, } log.SetOutput(io.Discard) @@ -173,11 +173,11 @@ func BenchmarkGetOneSetupKey(b *testing.B) { minMsPerOpCICD float64 maxMsPerOpCICD float64 }{ - {"Setup Keys - XS", 5, 5, 5, 5, 0.5, 2, 2, 4}, - {"Setup Keys - S", 5, 5, 5, 5, 0.5, 2, 2, 4}, - {"Setup Keys - M", 100, 20, 20, 100, 0.5, 2, 2, 4}, - {"Setup Keys - L", 500, 50, 100, 1000, 0.5, 2, 2, 4}, - {"Setup Keys - XL", 500, 50, 100, 5000, 0.5, 2, 2, 4}, + {"Setup Keys - XS", 5, 5, 5, 5, 0.5, 2, 2, 12}, + {"Setup Keys - S", 5, 5, 5, 5, 0.5, 2, 2, 12}, + {"Setup Keys - M", 100, 20, 20, 100, 0.5, 2, 2, 12}, + {"Setup Keys - L", 500, 50, 100, 1000, 0.5, 2, 2, 12}, + {"Setup Keys - XL", 500, 50, 100, 5000, 0.5, 2, 2, 12}, } log.SetOutput(io.Discard) @@ -291,11 +291,11 @@ func BenchmarkDeleteSetupKey(b *testing.B) { minMsPerOpCICD float64 maxMsPerOpCICD float64 }{ - {"Setup Keys - XS", 5, 5, 5, 5, 0.5, 2, 2, 5}, - {"Setup Keys - S", 5, 5, 5, 5, 0.5, 2, 2, 5}, - {"Setup Keys - M", 100, 20, 20, 100, 0.5, 2, 2, 5}, - {"Setup Keys - L", 500, 50, 100, 1000, 0.5, 2, 2, 5}, - {"Setup Keys - XL", 500, 50, 100, 5000, 0.5, 2, 2, 5}, + {"Setup Keys - XS", 5, 5, 5, 5, 0.5, 2, 1, 12}, + {"Setup Keys - S", 5, 5, 5, 5, 0.5, 2, 1, 12}, + {"Setup Keys - M", 100, 20, 20, 100, 0.5, 2, 1, 12}, + {"Setup Keys - L", 500, 50, 100, 1000, 0.5, 2, 1, 12}, + {"Setup Keys - XL", 500, 50, 100, 5000, 0.5, 2, 1, 12}, } log.SetOutput(io.Discard) From a931fd7b3cd1e9985e3fa0fe3e7dfde9e71a9deb Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Fri, 29 Nov 2024 14:13:33 +0100 Subject: [PATCH 22/38] update expected benchmark timings --- .../http/setupkeys_handler_benchmark_test.go | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/management/server/http/setupkeys_handler_benchmark_test.go b/management/server/http/setupkeys_handler_benchmark_test.go index ad0e51e26a6..14fd9ffda2c 100644 --- a/management/server/http/setupkeys_handler_benchmark_test.go +++ b/management/server/http/setupkeys_handler_benchmark_test.go @@ -30,11 +30,11 @@ func BenchmarkCreateSetupKey(b *testing.B) { minMsPerOpCICD float64 maxMsPerOpCICD float64 }{ - {"Setup Keys - XS", 5, 5, 5, 5, 0.5, 2, 2, 12}, - {"Setup Keys - S", 5, 5, 5, 5, 0.5, 2, 2, 12}, - {"Setup Keys - M", 100, 20, 20, 100, 0.5, 2, 2, 12}, - {"Setup Keys - L", 500, 50, 100, 1000, 0.5, 2, 2, 12}, - {"Setup Keys - XL", 500, 50, 100, 5000, 0.5, 2, 2, 12}, + {"Setup Keys - XS", 5, 5, 5, 5, 0.5, 2, 1, 12}, + {"Setup Keys - S", 5, 5, 5, 5, 0.5, 2, 1, 12}, + {"Setup Keys - M", 100, 20, 20, 100, 0.5, 2, 1, 12}, + {"Setup Keys - L", 500, 50, 100, 1000, 0.5, 2, 1, 12}, + {"Setup Keys - XL", 500, 50, 100, 5000, 0.5, 2, 1, 12}, } log.SetOutput(io.Discard) @@ -101,7 +101,7 @@ func BenchmarkUpdateSetupKey(b *testing.B) { minMsPerOpCICD float64 maxMsPerOpCICD float64 }{ - {"Setup Keys - XS", 5, 5, 5, 5, 0.5, 2, 3, 15}, + {"Setup Keys - XS", 5, 5, 5, 5, 0.5, 2, 2, 15}, {"Setup Keys - S", 5, 5, 5, 5, 0.5, 2, 3, 15}, {"Setup Keys - M", 100, 20, 20, 100, 0.5, 2, 3, 15}, {"Setup Keys - L", 500, 50, 100, 1000, 0.5, 2, 3, 15}, @@ -173,11 +173,11 @@ func BenchmarkGetOneSetupKey(b *testing.B) { minMsPerOpCICD float64 maxMsPerOpCICD float64 }{ - {"Setup Keys - XS", 5, 5, 5, 5, 0.5, 2, 2, 12}, - {"Setup Keys - S", 5, 5, 5, 5, 0.5, 2, 2, 12}, - {"Setup Keys - M", 100, 20, 20, 100, 0.5, 2, 2, 12}, - {"Setup Keys - L", 500, 50, 100, 1000, 0.5, 2, 2, 12}, - {"Setup Keys - XL", 500, 50, 100, 5000, 0.5, 2, 2, 12}, + {"Setup Keys - XS", 5, 5, 5, 5, 0.5, 2, 1, 12}, + {"Setup Keys - S", 5, 5, 5, 5, 0.5, 2, 1, 12}, + {"Setup Keys - M", 100, 20, 20, 100, 0.5, 2, 1, 12}, + {"Setup Keys - L", 500, 50, 100, 1000, 0.5, 2, 1, 12}, + {"Setup Keys - XL", 500, 50, 100, 5000, 0.5, 2, 1, 12}, } log.SetOutput(io.Discard) @@ -233,10 +233,10 @@ func BenchmarkGetAllSetupKeys(b *testing.B) { maxMsPerOpCICD float64 }{ {"Setup Keys - XS", 5, 5, 5, 5, 0.5, 2, 1, 10}, - {"Setup Keys - S", 5, 5, 5, 5, 0.5, 2, 1, 10}, - {"Setup Keys - M", 100, 20, 20, 100, 0.5, 2, 3, 15}, + {"Setup Keys - S", 5, 5, 5, 50, 0.5, 2, 1, 12}, + {"Setup Keys - M", 100, 20, 20, 100, 0.5, 2, 2, 15}, {"Setup Keys - L", 500, 50, 100, 1000, 5, 10, 10, 25}, - {"Setup Keys - XL", 500, 50, 100, 5000, 25, 45, 50, 150}, + {"Setup Keys - XL", 500, 50, 100, 5000, 25, 45, 35, 150}, } log.SetOutput(io.Discard) From 9c783e574bea34c2d90f4a2361bc5519fc826c70 Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Fri, 29 Nov 2024 14:48:25 +0100 Subject: [PATCH 23/38] update expected benchmark timings --- .../http/setupkeys_handler_benchmark_test.go | 50 +++++++++---------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/management/server/http/setupkeys_handler_benchmark_test.go b/management/server/http/setupkeys_handler_benchmark_test.go index 14fd9ffda2c..f58dadb6341 100644 --- a/management/server/http/setupkeys_handler_benchmark_test.go +++ b/management/server/http/setupkeys_handler_benchmark_test.go @@ -30,11 +30,11 @@ func BenchmarkCreateSetupKey(b *testing.B) { minMsPerOpCICD float64 maxMsPerOpCICD float64 }{ - {"Setup Keys - XS", 5, 5, 5, 5, 0.5, 2, 1, 12}, - {"Setup Keys - S", 5, 5, 5, 5, 0.5, 2, 1, 12}, - {"Setup Keys - M", 100, 20, 20, 100, 0.5, 2, 1, 12}, - {"Setup Keys - L", 500, 50, 100, 1000, 0.5, 2, 1, 12}, - {"Setup Keys - XL", 500, 50, 100, 5000, 0.5, 2, 1, 12}, + {"Setup Keys - XS", 10000, 10000, 10000, 5, 0.5, 2, 1, 12}, + {"Setup Keys - S", 5, 5, 5, 100, 0.5, 2, 1, 12}, + {"Setup Keys - M", 100, 20, 20, 1000, 0.5, 2, 1, 12}, + {"Setup Keys - L", 500, 50, 100, 5000, 0.5, 2, 1, 12}, + {"Setup Keys - XL", 500, 50, 100, 25000, 0.5, 2, 1, 12}, } log.SetOutput(io.Discard) @@ -101,11 +101,11 @@ func BenchmarkUpdateSetupKey(b *testing.B) { minMsPerOpCICD float64 maxMsPerOpCICD float64 }{ - {"Setup Keys - XS", 5, 5, 5, 5, 0.5, 2, 2, 15}, - {"Setup Keys - S", 5, 5, 5, 5, 0.5, 2, 3, 15}, - {"Setup Keys - M", 100, 20, 20, 100, 0.5, 2, 3, 15}, - {"Setup Keys - L", 500, 50, 100, 1000, 0.5, 2, 3, 15}, - {"Setup Keys - XL", 500, 50, 100, 5000, 0.5, 2, 3, 15}, + {"Setup Keys - XS", 10000, 10000, 10000, 5, 0.5, 3, 2, 15}, + {"Setup Keys - S", 5, 5, 5, 100, 0.5, 3, 3, 15}, + {"Setup Keys - M", 100, 20, 20, 1000, 0.5, 3, 3, 15}, + {"Setup Keys - L", 500, 50, 100, 5000, 0.5, 3, 3, 15}, + {"Setup Keys - XL", 500, 50, 100, 25000, 0.5, 3, 3, 15}, } log.SetOutput(io.Discard) @@ -173,11 +173,11 @@ func BenchmarkGetOneSetupKey(b *testing.B) { minMsPerOpCICD float64 maxMsPerOpCICD float64 }{ - {"Setup Keys - XS", 5, 5, 5, 5, 0.5, 2, 1, 12}, - {"Setup Keys - S", 5, 5, 5, 5, 0.5, 2, 1, 12}, - {"Setup Keys - M", 100, 20, 20, 100, 0.5, 2, 1, 12}, - {"Setup Keys - L", 500, 50, 100, 1000, 0.5, 2, 1, 12}, - {"Setup Keys - XL", 500, 50, 100, 5000, 0.5, 2, 1, 12}, + {"Setup Keys - XS", 10000, 10000, 10000, 5, 0.5, 2, 1, 12}, + {"Setup Keys - S", 5, 5, 5, 100, 0.5, 2, 1, 12}, + {"Setup Keys - M", 100, 20, 20, 1000, 0.5, 2, 1, 12}, + {"Setup Keys - L", 500, 50, 100, 5000, 0.5, 2, 1, 12}, + {"Setup Keys - XL", 500, 50, 100, 25000, 0.5, 2, 1, 12}, } log.SetOutput(io.Discard) @@ -232,11 +232,11 @@ func BenchmarkGetAllSetupKeys(b *testing.B) { minMsPerOpCICD float64 maxMsPerOpCICD float64 }{ - {"Setup Keys - XS", 5, 5, 5, 5, 0.5, 2, 1, 10}, - {"Setup Keys - S", 5, 5, 5, 50, 0.5, 2, 1, 12}, - {"Setup Keys - M", 100, 20, 20, 100, 0.5, 2, 2, 15}, - {"Setup Keys - L", 500, 50, 100, 1000, 5, 10, 10, 25}, - {"Setup Keys - XL", 500, 50, 100, 5000, 25, 45, 35, 150}, + {"Setup Keys - XS", 10000, 10000, 10000, 5, 0.5, 2, 1, 10}, + {"Setup Keys - S", 5, 5, 5, 10, 0.5, 2, 1, 12}, + {"Setup Keys - M", 100, 20, 20, 1000, 5, 10, 2, 15}, + {"Setup Keys - L", 500, 50, 100, 5000, 30, 50, 10, 25}, + {"Setup Keys - XL", 500, 50, 100, 25000, 140, 220, 35, 150}, } log.SetOutput(io.Discard) @@ -291,11 +291,11 @@ func BenchmarkDeleteSetupKey(b *testing.B) { minMsPerOpCICD float64 maxMsPerOpCICD float64 }{ - {"Setup Keys - XS", 5, 5, 5, 5, 0.5, 2, 1, 12}, - {"Setup Keys - S", 5, 5, 5, 5, 0.5, 2, 1, 12}, - {"Setup Keys - M", 100, 20, 20, 100, 0.5, 2, 1, 12}, - {"Setup Keys - L", 500, 50, 100, 1000, 0.5, 2, 1, 12}, - {"Setup Keys - XL", 500, 50, 100, 5000, 0.5, 2, 1, 12}, + {"Setup Keys - XS", 10000, 10000, 10000, 5, 0.5, 2, 1, 12}, + {"Setup Keys - S", 5, 5, 5, 100, 0.5, 2, 1, 12}, + {"Setup Keys - M", 100, 20, 20, 1000, 0.5, 2, 1, 12}, + {"Setup Keys - L", 500, 50, 100, 5000, 0.5, 2, 1, 12}, + {"Setup Keys - XL", 500, 50, 100, 25000, 0.5, 2, 1, 12}, } log.SetOutput(io.Discard) From 06d18c0fbe0d8f942185fd11e4c95190a3192e21 Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Fri, 29 Nov 2024 15:05:12 +0100 Subject: [PATCH 24/38] add additional cases for other resources with high counts --- .../http/setupkeys_handler_benchmark_test.go | 25 +++++++++++++++---- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/management/server/http/setupkeys_handler_benchmark_test.go b/management/server/http/setupkeys_handler_benchmark_test.go index f58dadb6341..ae6c84a1f5f 100644 --- a/management/server/http/setupkeys_handler_benchmark_test.go +++ b/management/server/http/setupkeys_handler_benchmark_test.go @@ -33,7 +33,10 @@ func BenchmarkCreateSetupKey(b *testing.B) { {"Setup Keys - XS", 10000, 10000, 10000, 5, 0.5, 2, 1, 12}, {"Setup Keys - S", 5, 5, 5, 100, 0.5, 2, 1, 12}, {"Setup Keys - M", 100, 20, 20, 1000, 0.5, 2, 1, 12}, - {"Setup Keys - L", 500, 50, 100, 5000, 0.5, 2, 1, 12}, + {"Setup Keys - L", 5, 5, 5, 5000, 0.5, 2, 1, 12}, + {"Peers - L", 10000, 5, 5, 5000, 0.5, 2, 1, 12}, + {"Groups - L", 5, 10000, 5, 5000, 0.5, 2, 1, 12}, + {"Users - L", 5, 5, 10000, 5000, 0.5, 2, 1, 12}, {"Setup Keys - XL", 500, 50, 100, 25000, 0.5, 2, 1, 12}, } @@ -104,7 +107,10 @@ func BenchmarkUpdateSetupKey(b *testing.B) { {"Setup Keys - XS", 10000, 10000, 10000, 5, 0.5, 3, 2, 15}, {"Setup Keys - S", 5, 5, 5, 100, 0.5, 3, 3, 15}, {"Setup Keys - M", 100, 20, 20, 1000, 0.5, 3, 3, 15}, - {"Setup Keys - L", 500, 50, 100, 5000, 0.5, 3, 3, 15}, + {"Setup Keys - L", 5, 5, 5, 5000, 0.5, 3, 3, 15}, + {"Peers - L", 10000, 5, 5, 5000, 0.5, 3, 3, 15}, + {"Groups - L", 5, 10000, 5, 5000, 0.5, 3, 3, 15}, + {"Users - L", 5, 5, 10000, 5000, 0.5, 3, 3, 15}, {"Setup Keys - XL", 500, 50, 100, 25000, 0.5, 3, 3, 15}, } @@ -176,7 +182,10 @@ func BenchmarkGetOneSetupKey(b *testing.B) { {"Setup Keys - XS", 10000, 10000, 10000, 5, 0.5, 2, 1, 12}, {"Setup Keys - S", 5, 5, 5, 100, 0.5, 2, 1, 12}, {"Setup Keys - M", 100, 20, 20, 1000, 0.5, 2, 1, 12}, - {"Setup Keys - L", 500, 50, 100, 5000, 0.5, 2, 1, 12}, + {"Setup Keys - L", 5, 5, 5, 5000, 0.5, 2, 1, 12}, + {"Peers - L", 10000, 5, 5, 5000, 0.5, 2, 1, 12}, + {"Groups - L", 5, 10000, 5, 5000, 0.5, 2, 1, 12}, + {"Users - L", 5, 5, 10000, 5000, 0.5, 2, 1, 12}, {"Setup Keys - XL", 500, 50, 100, 25000, 0.5, 2, 1, 12}, } @@ -235,7 +244,10 @@ func BenchmarkGetAllSetupKeys(b *testing.B) { {"Setup Keys - XS", 10000, 10000, 10000, 5, 0.5, 2, 1, 10}, {"Setup Keys - S", 5, 5, 5, 10, 0.5, 2, 1, 12}, {"Setup Keys - M", 100, 20, 20, 1000, 5, 10, 2, 15}, - {"Setup Keys - L", 500, 50, 100, 5000, 30, 50, 10, 25}, + {"Setup Keys - L", 5, 5, 5, 5000, 30, 50, 10, 25}, + {"Peers - L", 10000, 5, 5, 5000, 30, 50, 2, 5}, + {"Groups - L", 5, 10000, 5, 5000, 30, 50, 2, 5}, + {"Users - L", 5, 5, 10000, 5000, 30, 50, 2, 5}, {"Setup Keys - XL", 500, 50, 100, 25000, 140, 220, 35, 150}, } @@ -294,7 +306,10 @@ func BenchmarkDeleteSetupKey(b *testing.B) { {"Setup Keys - XS", 10000, 10000, 10000, 5, 0.5, 2, 1, 12}, {"Setup Keys - S", 5, 5, 5, 100, 0.5, 2, 1, 12}, {"Setup Keys - M", 100, 20, 20, 1000, 0.5, 2, 1, 12}, - {"Setup Keys - L", 500, 50, 100, 5000, 0.5, 2, 1, 12}, + {"Setup Keys - L", 5, 5, 5, 5000, 0.5, 2, 1, 12}, + {"Peers - L", 10000, 5, 5, 5000, 0.5, 2, 1, 12}, + {"Groups - L", 5, 10000, 5, 5000, 0.5, 2, 1, 12}, + {"Users - L", 5, 5, 10000, 5000, 0.5, 2, 1, 12}, {"Setup Keys - XL", 500, 50, 100, 25000, 0.5, 2, 1, 12}, } From 39f8eb72d010b1de3be96273d76ac9032dcecb14 Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Fri, 29 Nov 2024 15:35:59 +0100 Subject: [PATCH 25/38] update expected benchmark timings --- .../server/http/setupkeys_handler_benchmark_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/management/server/http/setupkeys_handler_benchmark_test.go b/management/server/http/setupkeys_handler_benchmark_test.go index ae6c84a1f5f..1a6cc91c1be 100644 --- a/management/server/http/setupkeys_handler_benchmark_test.go +++ b/management/server/http/setupkeys_handler_benchmark_test.go @@ -243,12 +243,12 @@ func BenchmarkGetAllSetupKeys(b *testing.B) { }{ {"Setup Keys - XS", 10000, 10000, 10000, 5, 0.5, 2, 1, 10}, {"Setup Keys - S", 5, 5, 5, 10, 0.5, 2, 1, 12}, - {"Setup Keys - M", 100, 20, 20, 1000, 5, 10, 2, 15}, - {"Setup Keys - L", 5, 5, 5, 5000, 30, 50, 10, 25}, - {"Peers - L", 10000, 5, 5, 5000, 30, 50, 2, 5}, - {"Groups - L", 5, 10000, 5, 5000, 30, 50, 2, 5}, - {"Users - L", 5, 5, 10000, 5000, 30, 50, 2, 5}, - {"Setup Keys - XL", 500, 50, 100, 25000, 140, 220, 35, 150}, + {"Setup Keys - M", 100, 20, 20, 1000, 5, 10, 5, 20}, + {"Setup Keys - L", 5, 5, 5, 5000, 30, 50, 50, 100}, + {"Peers - L", 10000, 5, 5, 5000, 30, 50, 50, 100}, + {"Groups - L", 5, 10000, 5, 5000, 30, 50, 50, 100}, + {"Users - L", 5, 5, 10000, 5000, 30, 50, 50, 100}, + {"Setup Keys - XL", 500, 50, 100, 25000, 140, 220, 300, 500}, } log.SetOutput(io.Discard) From 6e2471d6238d1260e4b5e494e659a19b64138fb0 Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Fri, 29 Nov 2024 16:21:10 +0100 Subject: [PATCH 26/38] update expected benchmark timings --- .../http/setupkeys_handler_benchmark_test.go | 282 +++++------------- management/server/http/tools_test.go | 37 +++ 2 files changed, 114 insertions(+), 205 deletions(-) diff --git a/management/server/http/setupkeys_handler_benchmark_test.go b/management/server/http/setupkeys_handler_benchmark_test.go index 1a6cc91c1be..1a6d1054bae 100644 --- a/management/server/http/setupkeys_handler_benchmark_test.go +++ b/management/server/http/setupkeys_handler_benchmark_test.go @@ -17,27 +17,28 @@ import ( "github.com/netbirdio/netbird/management/server/http/api" ) +// Map to store peers, groups, users, and setupKeys by name +var benchCases = map[string]BenchmarkCase{ + "Setup Keys - XS": {Peers: 10000, Groups: 10000, Users: 10000, SetupKeys: 5}, + "Setup Keys - S": {Peers: 5, Groups: 5, Users: 5, SetupKeys: 100}, + "Setup Keys - M": {Peers: 100, Groups: 20, Users: 20, SetupKeys: 1000}, + "Setup Keys - L": {Peers: 5, Groups: 5, Users: 5, SetupKeys: 5000}, + "Peers - L": {Peers: 10000, Groups: 5, Users: 5, SetupKeys: 5000}, + "Groups - L": {Peers: 5, Groups: 10000, Users: 5, SetupKeys: 5000}, + "Users - L": {Peers: 5, Groups: 5, Users: 10000, SetupKeys: 5000}, + "Setup Keys - XL": {Peers: 500, Groups: 50, Users: 100, SetupKeys: 25000}, +} + func BenchmarkCreateSetupKey(b *testing.B) { - benchCases := []struct { - name string - peers int - groups int - users int - setupKeys int - // We need different expectations for CI/CD and local runs because of the different performance characteristics - minMsPerOpLocal float64 - maxMsPerOpLocal float64 - minMsPerOpCICD float64 - maxMsPerOpCICD float64 - }{ - {"Setup Keys - XS", 10000, 10000, 10000, 5, 0.5, 2, 1, 12}, - {"Setup Keys - S", 5, 5, 5, 100, 0.5, 2, 1, 12}, - {"Setup Keys - M", 100, 20, 20, 1000, 0.5, 2, 1, 12}, - {"Setup Keys - L", 5, 5, 5, 5000, 0.5, 2, 1, 12}, - {"Peers - L", 10000, 5, 5, 5000, 0.5, 2, 1, 12}, - {"Groups - L", 5, 10000, 5, 5000, 0.5, 2, 1, 12}, - {"Users - L", 5, 5, 10000, 5000, 0.5, 2, 1, 12}, - {"Setup Keys - XL", 500, 50, 100, 25000, 0.5, 2, 1, 12}, + var expectedMetrics = map[string]PerformanceMetrics{ + "Setup Keys - XS": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, + "Setup Keys - S": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, + "Setup Keys - M": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, + "Setup Keys - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, + "Peers - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, + "Groups - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, + "Users - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, + "Setup Keys - XL": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, } log.SetOutput(io.Discard) @@ -45,10 +46,10 @@ func BenchmarkCreateSetupKey(b *testing.B) { recorder := httptest.NewRecorder() - for _, bc := range benchCases { - b.Run(bc.name, func(b *testing.B) { + for name, bc := range benchCases { + b.Run(name, func(b *testing.B) { apiHandler, am, _ := buildApiBlackBoxWithDBState(b, "testdata/setup_keys.sql", nil) - populateTestData(b, am.(*server.DefaultAccountManager), bc.peers, bc.groups, bc.users, bc.setupKeys) + populateTestData(b, am.(*server.DefaultAccountManager), bc.Peers, bc.Groups, bc.Users, bc.SetupKeys) b.ResetTimer() start := time.Now() @@ -69,49 +70,21 @@ func BenchmarkCreateSetupKey(b *testing.B) { apiHandler.ServeHTTP(recorder, req) } - duration := time.Since(start) - msPerOp := float64(duration.Nanoseconds()) / float64(b.N) / 1e6 - b.ReportMetric(msPerOp, "ms/op") - - minExpected := bc.minMsPerOpLocal - maxExpected := bc.maxMsPerOpLocal - if os.Getenv("CI") == "true" { - minExpected = bc.minMsPerOpCICD - maxExpected = bc.maxMsPerOpCICD - } - - if msPerOp < minExpected { - b.Fatalf("Benchmark %s failed: too fast (%.2f ms/op, minimum %.2f ms/op)", bc.name, msPerOp, minExpected) - } - - if msPerOp > maxExpected { - b.Fatalf("Benchmark %s failed: too slow (%.2f ms/op, maximum %.2f ms/op)", bc.name, msPerOp, maxExpected) - } + evaluateBenchmarkResults(b, name, time.Since(start), expectedMetrics[name]) }) } } func BenchmarkUpdateSetupKey(b *testing.B) { - benchCases := []struct { - name string - peers int - groups int - users int - setupKeys int - // We need different expectations for CI/CD and local runs because of the different performance characteristics - minMsPerOpLocal float64 - maxMsPerOpLocal float64 - minMsPerOpCICD float64 - maxMsPerOpCICD float64 - }{ - {"Setup Keys - XS", 10000, 10000, 10000, 5, 0.5, 3, 2, 15}, - {"Setup Keys - S", 5, 5, 5, 100, 0.5, 3, 3, 15}, - {"Setup Keys - M", 100, 20, 20, 1000, 0.5, 3, 3, 15}, - {"Setup Keys - L", 5, 5, 5, 5000, 0.5, 3, 3, 15}, - {"Peers - L", 10000, 5, 5, 5000, 0.5, 3, 3, 15}, - {"Groups - L", 5, 10000, 5, 5000, 0.5, 3, 3, 15}, - {"Users - L", 5, 5, 10000, 5000, 0.5, 3, 3, 15}, - {"Setup Keys - XL", 500, 50, 100, 25000, 0.5, 3, 3, 15}, + var expectedMetrics = map[string]PerformanceMetrics{ + "Setup Keys - XS": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 3, MaxMsPerOpCICD: 15}, + "Setup Keys - S": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 3, MaxMsPerOpCICD: 15}, + "Setup Keys - M": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 3, MaxMsPerOpCICD: 15}, + "Setup Keys - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 3, MaxMsPerOpCICD: 15}, + "Peers - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 3, MaxMsPerOpCICD: 15}, + "Groups - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 3, MaxMsPerOpCICD: 15}, + "Users - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 3, MaxMsPerOpCICD: 15}, + "Setup Keys - XL": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 3, MaxMsPerOpCICD: 15}, } log.SetOutput(io.Discard) @@ -119,10 +92,10 @@ func BenchmarkUpdateSetupKey(b *testing.B) { recorder := httptest.NewRecorder() - for _, bc := range benchCases { - b.Run(bc.name, func(b *testing.B) { + for name, bc := range benchCases { + b.Run(name, func(b *testing.B) { apiHandler, am, _ := buildApiBlackBoxWithDBState(b, "testdata/setup_keys.sql", nil) - populateTestData(b, am.(*server.DefaultAccountManager), bc.peers, bc.groups, bc.users, bc.setupKeys) + populateTestData(b, am.(*server.DefaultAccountManager), bc.Peers, bc.Groups, bc.Users, bc.SetupKeys) b.ResetTimer() start := time.Now() @@ -144,49 +117,21 @@ func BenchmarkUpdateSetupKey(b *testing.B) { apiHandler.ServeHTTP(recorder, req) } - duration := time.Since(start) - msPerOp := float64(duration.Nanoseconds()) / float64(b.N) / 1e6 - b.ReportMetric(msPerOp, "ms/op") - - minExpected := bc.minMsPerOpLocal - maxExpected := bc.maxMsPerOpLocal - if os.Getenv("CI") == "true" { - minExpected = bc.minMsPerOpCICD - maxExpected = bc.maxMsPerOpCICD - } - - if msPerOp < minExpected { - b.Fatalf("Benchmark %s failed: too fast (%.2f ms/op, minimum %.2f ms/op)", bc.name, msPerOp, minExpected) - } - - if msPerOp > maxExpected { - b.Fatalf("Benchmark %s failed: too slow (%.2f ms/op, maximum %.2f ms/op)", bc.name, msPerOp, maxExpected) - } + evaluateBenchmarkResults(b, name, time.Since(start), expectedMetrics[name]) }) } } func BenchmarkGetOneSetupKey(b *testing.B) { - benchCases := []struct { - name string - peers int - groups int - users int - setupKeys int - // We need different expectations for CI/CD and local runs because of the different performance characteristics - minMsPerOpLocal float64 - maxMsPerOpLocal float64 - minMsPerOpCICD float64 - maxMsPerOpCICD float64 - }{ - {"Setup Keys - XS", 10000, 10000, 10000, 5, 0.5, 2, 1, 12}, - {"Setup Keys - S", 5, 5, 5, 100, 0.5, 2, 1, 12}, - {"Setup Keys - M", 100, 20, 20, 1000, 0.5, 2, 1, 12}, - {"Setup Keys - L", 5, 5, 5, 5000, 0.5, 2, 1, 12}, - {"Peers - L", 10000, 5, 5, 5000, 0.5, 2, 1, 12}, - {"Groups - L", 5, 10000, 5, 5000, 0.5, 2, 1, 12}, - {"Users - L", 5, 5, 10000, 5000, 0.5, 2, 1, 12}, - {"Setup Keys - XL", 500, 50, 100, 25000, 0.5, 2, 1, 12}, + var expectedMetrics = map[string]PerformanceMetrics{ + "Setup Keys - XS": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, + "Setup Keys - S": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, + "Setup Keys - M": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, + "Setup Keys - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, + "Peers - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, + "Groups - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, + "Users - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, + "Setup Keys - XL": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, } log.SetOutput(io.Discard) @@ -194,10 +139,10 @@ func BenchmarkGetOneSetupKey(b *testing.B) { recorder := httptest.NewRecorder() - for _, bc := range benchCases { - b.Run(bc.name, func(b *testing.B) { + for name, bc := range benchCases { + b.Run(name, func(b *testing.B) { apiHandler, am, _ := buildApiBlackBoxWithDBState(b, "testdata/setup_keys.sql", nil) - populateTestData(b, am.(*server.DefaultAccountManager), bc.peers, bc.groups, bc.users, bc.setupKeys) + populateTestData(b, am.(*server.DefaultAccountManager), bc.Peers, bc.Groups, bc.Users, bc.SetupKeys) b.ResetTimer() start := time.Now() @@ -206,49 +151,21 @@ func BenchmarkGetOneSetupKey(b *testing.B) { apiHandler.ServeHTTP(recorder, req) } - duration := time.Since(start) - msPerOp := float64(duration.Nanoseconds()) / float64(b.N) / 1e6 - b.ReportMetric(msPerOp, "ms/op") - - minExpected := bc.minMsPerOpLocal - maxExpected := bc.maxMsPerOpLocal - if os.Getenv("CI") == "true" { - minExpected = bc.minMsPerOpCICD - maxExpected = bc.maxMsPerOpCICD - } - - if msPerOp < minExpected { - b.Fatalf("Benchmark %s failed: too fast (%.2f ms/op, minimum %.2f ms/op)", bc.name, msPerOp, minExpected) - } - - if msPerOp > maxExpected { - b.Fatalf("Benchmark %s failed: too slow (%.2f ms/op, maximum %.2f ms/op)", bc.name, msPerOp, maxExpected) - } + evaluateBenchmarkResults(b, name, time.Since(start), expectedMetrics[name]) }) } } func BenchmarkGetAllSetupKeys(b *testing.B) { - benchCases := []struct { - name string - peers int - groups int - users int - setupKeys int - // We need different expectations for CI/CD and local runs because of the different performance characteristics - minMsPerOpLocal float64 - maxMsPerOpLocal float64 - minMsPerOpCICD float64 - maxMsPerOpCICD float64 - }{ - {"Setup Keys - XS", 10000, 10000, 10000, 5, 0.5, 2, 1, 10}, - {"Setup Keys - S", 5, 5, 5, 10, 0.5, 2, 1, 12}, - {"Setup Keys - M", 100, 20, 20, 1000, 5, 10, 5, 20}, - {"Setup Keys - L", 5, 5, 5, 5000, 30, 50, 50, 100}, - {"Peers - L", 10000, 5, 5, 5000, 30, 50, 50, 100}, - {"Groups - L", 5, 10000, 5, 5000, 30, 50, 50, 100}, - {"Users - L", 5, 5, 10000, 5000, 30, 50, 50, 100}, - {"Setup Keys - XL", 500, 50, 100, 25000, 140, 220, 300, 500}, + var expectedMetrics = map[string]PerformanceMetrics{ + "Setup Keys - XS": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 10}, + "Setup Keys - S": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, + "Setup Keys - M": {MinMsPerOpLocal: 5, MaxMsPerOpLocal: 10, MinMsPerOpCICD: 5, MaxMsPerOpCICD: 20}, + "Setup Keys - L": {MinMsPerOpLocal: 30, MaxMsPerOpLocal: 50, MinMsPerOpCICD: 50, MaxMsPerOpCICD: 100}, + "Peers - L": {MinMsPerOpLocal: 30, MaxMsPerOpLocal: 50, MinMsPerOpCICD: 50, MaxMsPerOpCICD: 100}, + "Groups - L": {MinMsPerOpLocal: 30, MaxMsPerOpLocal: 50, MinMsPerOpCICD: 50, MaxMsPerOpCICD: 100}, + "Users - L": {MinMsPerOpLocal: 30, MaxMsPerOpLocal: 50, MinMsPerOpCICD: 50, MaxMsPerOpCICD: 100}, + "Setup Keys - XL": {MinMsPerOpLocal: 140, MaxMsPerOpLocal: 220, MinMsPerOpCICD: 300, MaxMsPerOpCICD: 500}, } log.SetOutput(io.Discard) @@ -256,10 +173,10 @@ func BenchmarkGetAllSetupKeys(b *testing.B) { recorder := httptest.NewRecorder() - for _, bc := range benchCases { - b.Run(bc.name, func(b *testing.B) { + for name, bc := range benchCases { + b.Run(name, func(b *testing.B) { apiHandler, am, _ := buildApiBlackBoxWithDBState(b, "testdata/setup_keys.sql", nil) - populateTestData(b, am.(*server.DefaultAccountManager), bc.peers, bc.groups, bc.users, bc.setupKeys) + populateTestData(b, am.(*server.DefaultAccountManager), bc.Peers, bc.Groups, bc.Users, bc.SetupKeys) b.ResetTimer() start := time.Now() @@ -268,49 +185,21 @@ func BenchmarkGetAllSetupKeys(b *testing.B) { apiHandler.ServeHTTP(recorder, req) } - duration := time.Since(start) - msPerOp := float64(duration.Nanoseconds()) / float64(b.N) / 1e6 - b.ReportMetric(msPerOp, "ms/op") - - minExpected := bc.minMsPerOpLocal - maxExpected := bc.maxMsPerOpLocal - if os.Getenv("CI") == "true" { - minExpected = bc.minMsPerOpCICD - maxExpected = bc.maxMsPerOpCICD - } - - if msPerOp < minExpected { - b.Fatalf("Benchmark %s failed: too fast (%.2f ms/op, minimum %.2f ms/op)", bc.name, msPerOp, minExpected) - } - - if msPerOp > maxExpected { - b.Fatalf("Benchmark %s failed: too slow (%.2f ms/op, maximum %.2f ms/op)", bc.name, msPerOp, maxExpected) - } + evaluateBenchmarkResults(b, name, time.Since(start), expectedMetrics[name]) }) } } func BenchmarkDeleteSetupKey(b *testing.B) { - benchCases := []struct { - name string - peers int - groups int - users int - setupKeys int - // We need different expectations for CI/CD and local runs because of the different performance characteristics - minMsPerOpLocal float64 - maxMsPerOpLocal float64 - minMsPerOpCICD float64 - maxMsPerOpCICD float64 - }{ - {"Setup Keys - XS", 10000, 10000, 10000, 5, 0.5, 2, 1, 12}, - {"Setup Keys - S", 5, 5, 5, 100, 0.5, 2, 1, 12}, - {"Setup Keys - M", 100, 20, 20, 1000, 0.5, 2, 1, 12}, - {"Setup Keys - L", 5, 5, 5, 5000, 0.5, 2, 1, 12}, - {"Peers - L", 10000, 5, 5, 5000, 0.5, 2, 1, 12}, - {"Groups - L", 5, 10000, 5, 5000, 0.5, 2, 1, 12}, - {"Users - L", 5, 5, 10000, 5000, 0.5, 2, 1, 12}, - {"Setup Keys - XL", 500, 50, 100, 25000, 0.5, 2, 1, 12}, + var expectedMetrics = map[string]PerformanceMetrics{ + "Setup Keys - XS": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, + "Setup Keys - S": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, + "Setup Keys - M": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, + "Setup Keys - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, + "Peers - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, + "Groups - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, + "Users - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, + "Setup Keys - XL": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, } log.SetOutput(io.Discard) @@ -318,10 +207,10 @@ func BenchmarkDeleteSetupKey(b *testing.B) { recorder := httptest.NewRecorder() - for _, bc := range benchCases { - b.Run(bc.name, func(b *testing.B) { + for name, bc := range benchCases { + b.Run(name, func(b *testing.B) { apiHandler, am, _ := buildApiBlackBoxWithDBState(b, "testdata/setup_keys.sql", nil) - populateTestData(b, am.(*server.DefaultAccountManager), bc.peers, bc.groups, bc.users, bc.setupKeys) + populateTestData(b, am.(*server.DefaultAccountManager), bc.Peers, bc.Groups, bc.Users, bc.SetupKeys) b.ResetTimer() start := time.Now() @@ -331,24 +220,7 @@ func BenchmarkDeleteSetupKey(b *testing.B) { apiHandler.ServeHTTP(recorder, req) } - duration := time.Since(start) - msPerOp := float64(duration.Nanoseconds()) / float64(b.N) / 1e6 - b.ReportMetric(msPerOp, "ms/op") - - minExpected := bc.minMsPerOpLocal - maxExpected := bc.maxMsPerOpLocal - if os.Getenv("CI") == "true" { - minExpected = bc.minMsPerOpCICD - maxExpected = bc.maxMsPerOpCICD - } - - if msPerOp < minExpected { - b.Fatalf("Benchmark %s failed: too fast (%.2f ms/op, minimum %.2f ms/op)", bc.name, msPerOp, minExpected) - } - - if msPerOp > maxExpected { - b.Fatalf("Benchmark %s failed: too slow (%.2f ms/op, maximum %.2f ms/op)", bc.name, msPerOp, maxExpected) - } + evaluateBenchmarkResults(b, name, time.Since(start), expectedMetrics[name]) }) } } diff --git a/management/server/http/tools_test.go b/management/server/http/tools_test.go index 2784fed7bba..2f5464cbf41 100644 --- a/management/server/http/tools_test.go +++ b/management/server/http/tools_test.go @@ -8,6 +8,7 @@ import ( "net" "net/http" "net/http/httptest" + "os" "strconv" "testing" "time" @@ -33,6 +34,22 @@ type TB interface { TempDir() string } +// BenchmarkCase defines a single benchmark test case +type BenchmarkCase struct { + Peers int + Groups int + Users int + SetupKeys int +} + +// PerformanceMetrics holds the performance expectations +type PerformanceMetrics struct { + MinMsPerOpLocal float64 + MaxMsPerOpLocal float64 + MinMsPerOpCICD float64 + MaxMsPerOpCICD float64 +} + func buildApiBlackBoxWithDBState(t TB, sqlFile string, expectedPeerUpdate *server.UpdateMessage) (http.Handler, server.AccountManager, chan struct{}) { store, cleanup, err := server.NewTestStoreFromSQL(context.Background(), sqlFile, t.TempDir()) if err != nil { @@ -227,3 +244,23 @@ func populateTestData(b *testing.B, am *server.DefaultAccountManager, peers, gro } } + +func evaluateBenchmarkResults(b *testing.B, name string, duration time.Duration, perfMetrics PerformanceMetrics) { + msPerOp := float64(duration.Nanoseconds()) / float64(b.N) / 1e6 + b.ReportMetric(msPerOp, "ms/op") + + minExpected := perfMetrics.MinMsPerOpLocal + maxExpected := perfMetrics.MaxMsPerOpLocal + if os.Getenv("CI") == "true" { + minExpected = perfMetrics.MinMsPerOpCICD + maxExpected = perfMetrics.MaxMsPerOpCICD + } + + if msPerOp < minExpected { + b.Fatalf("Benchmark %s failed: too fast (%.2f ms/op, minimum %.2f ms/op)", name, msPerOp, minExpected) + } + + if msPerOp > maxExpected { + b.Fatalf("Benchmark %s failed: too slow (%.2f ms/op, maximum %.2f ms/op)", name, msPerOp, maxExpected) + } +} From 0abb8ef644c65ccf1674786d7529e9a532aa3adc Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Fri, 29 Nov 2024 16:37:10 +0100 Subject: [PATCH 27/38] mark helper --- .../server/http/setupkeys_handler_benchmark_test.go | 12 ++++++------ management/server/http/tools_test.go | 2 ++ 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/management/server/http/setupkeys_handler_benchmark_test.go b/management/server/http/setupkeys_handler_benchmark_test.go index 1a6d1054bae..f0fc6069432 100644 --- a/management/server/http/setupkeys_handler_benchmark_test.go +++ b/management/server/http/setupkeys_handler_benchmark_test.go @@ -18,7 +18,7 @@ import ( ) // Map to store peers, groups, users, and setupKeys by name -var benchCases = map[string]BenchmarkCase{ +var benchCasesSetupKeys = map[string]BenchmarkCase{ "Setup Keys - XS": {Peers: 10000, Groups: 10000, Users: 10000, SetupKeys: 5}, "Setup Keys - S": {Peers: 5, Groups: 5, Users: 5, SetupKeys: 100}, "Setup Keys - M": {Peers: 100, Groups: 20, Users: 20, SetupKeys: 1000}, @@ -46,7 +46,7 @@ func BenchmarkCreateSetupKey(b *testing.B) { recorder := httptest.NewRecorder() - for name, bc := range benchCases { + for name, bc := range benchCasesSetupKeys { b.Run(name, func(b *testing.B) { apiHandler, am, _ := buildApiBlackBoxWithDBState(b, "testdata/setup_keys.sql", nil) populateTestData(b, am.(*server.DefaultAccountManager), bc.Peers, bc.Groups, bc.Users, bc.SetupKeys) @@ -92,7 +92,7 @@ func BenchmarkUpdateSetupKey(b *testing.B) { recorder := httptest.NewRecorder() - for name, bc := range benchCases { + for name, bc := range benchCasesSetupKeys { b.Run(name, func(b *testing.B) { apiHandler, am, _ := buildApiBlackBoxWithDBState(b, "testdata/setup_keys.sql", nil) populateTestData(b, am.(*server.DefaultAccountManager), bc.Peers, bc.Groups, bc.Users, bc.SetupKeys) @@ -139,7 +139,7 @@ func BenchmarkGetOneSetupKey(b *testing.B) { recorder := httptest.NewRecorder() - for name, bc := range benchCases { + for name, bc := range benchCasesSetupKeys { b.Run(name, func(b *testing.B) { apiHandler, am, _ := buildApiBlackBoxWithDBState(b, "testdata/setup_keys.sql", nil) populateTestData(b, am.(*server.DefaultAccountManager), bc.Peers, bc.Groups, bc.Users, bc.SetupKeys) @@ -173,7 +173,7 @@ func BenchmarkGetAllSetupKeys(b *testing.B) { recorder := httptest.NewRecorder() - for name, bc := range benchCases { + for name, bc := range benchCasesSetupKeys { b.Run(name, func(b *testing.B) { apiHandler, am, _ := buildApiBlackBoxWithDBState(b, "testdata/setup_keys.sql", nil) populateTestData(b, am.(*server.DefaultAccountManager), bc.Peers, bc.Groups, bc.Users, bc.SetupKeys) @@ -207,7 +207,7 @@ func BenchmarkDeleteSetupKey(b *testing.B) { recorder := httptest.NewRecorder() - for name, bc := range benchCases { + for name, bc := range benchCasesSetupKeys { b.Run(name, func(b *testing.B) { apiHandler, am, _ := buildApiBlackBoxWithDBState(b, "testdata/setup_keys.sql", nil) populateTestData(b, am.(*server.DefaultAccountManager), bc.Peers, bc.Groups, bc.Users, bc.SetupKeys) diff --git a/management/server/http/tools_test.go b/management/server/http/tools_test.go index 2f5464cbf41..3c6dd58cbe9 100644 --- a/management/server/http/tools_test.go +++ b/management/server/http/tools_test.go @@ -246,6 +246,8 @@ func populateTestData(b *testing.B, am *server.DefaultAccountManager, peers, gro } func evaluateBenchmarkResults(b *testing.B, name string, duration time.Duration, perfMetrics PerformanceMetrics) { + b.Helper() + msPerOp := float64(duration.Nanoseconds()) / float64(b.N) / 1e6 b.ReportMetric(msPerOp, "ms/op") From c6f12bebccd3fcc9f4a18d2f7d8807fae3fb410f Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Fri, 29 Nov 2024 16:51:20 +0100 Subject: [PATCH 28/38] update expected benchmark timings --- management/server/http/setupkeys_handler_benchmark_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/management/server/http/setupkeys_handler_benchmark_test.go b/management/server/http/setupkeys_handler_benchmark_test.go index f0fc6069432..797250859ac 100644 --- a/management/server/http/setupkeys_handler_benchmark_test.go +++ b/management/server/http/setupkeys_handler_benchmark_test.go @@ -165,7 +165,7 @@ func BenchmarkGetAllSetupKeys(b *testing.B) { "Peers - L": {MinMsPerOpLocal: 30, MaxMsPerOpLocal: 50, MinMsPerOpCICD: 50, MaxMsPerOpCICD: 100}, "Groups - L": {MinMsPerOpLocal: 30, MaxMsPerOpLocal: 50, MinMsPerOpCICD: 50, MaxMsPerOpCICD: 100}, "Users - L": {MinMsPerOpLocal: 30, MaxMsPerOpLocal: 50, MinMsPerOpCICD: 50, MaxMsPerOpCICD: 100}, - "Setup Keys - XL": {MinMsPerOpLocal: 140, MaxMsPerOpLocal: 220, MinMsPerOpCICD: 300, MaxMsPerOpCICD: 500}, + "Setup Keys - XL": {MinMsPerOpLocal: 140, MaxMsPerOpLocal: 220, MinMsPerOpCICD: 250, MaxMsPerOpCICD: 500}, } log.SetOutput(io.Discard) From 65c99ee77118bbed94ed874c75550e40dadb5c9b Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Fri, 29 Nov 2024 17:15:51 +0100 Subject: [PATCH 29/38] update expected benchmark timings --- .../http/setupkeys_handler_benchmark_test.go | 18 +++++++++--------- management/server/http/tools_test.go | 6 +++++- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/management/server/http/setupkeys_handler_benchmark_test.go b/management/server/http/setupkeys_handler_benchmark_test.go index 797250859ac..632daf4b54c 100644 --- a/management/server/http/setupkeys_handler_benchmark_test.go +++ b/management/server/http/setupkeys_handler_benchmark_test.go @@ -70,7 +70,7 @@ func BenchmarkCreateSetupKey(b *testing.B) { apiHandler.ServeHTTP(recorder, req) } - evaluateBenchmarkResults(b, name, time.Since(start), expectedMetrics[name]) + evaluateBenchmarkResults(b, name, time.Since(start), expectedMetrics[name], recorder) }) } } @@ -117,7 +117,7 @@ func BenchmarkUpdateSetupKey(b *testing.B) { apiHandler.ServeHTTP(recorder, req) } - evaluateBenchmarkResults(b, name, time.Since(start), expectedMetrics[name]) + evaluateBenchmarkResults(b, name, time.Since(start), expectedMetrics[name], recorder) }) } } @@ -151,7 +151,7 @@ func BenchmarkGetOneSetupKey(b *testing.B) { apiHandler.ServeHTTP(recorder, req) } - evaluateBenchmarkResults(b, name, time.Since(start), expectedMetrics[name]) + evaluateBenchmarkResults(b, name, time.Since(start), expectedMetrics[name], recorder) }) } } @@ -161,10 +161,10 @@ func BenchmarkGetAllSetupKeys(b *testing.B) { "Setup Keys - XS": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 10}, "Setup Keys - S": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, "Setup Keys - M": {MinMsPerOpLocal: 5, MaxMsPerOpLocal: 10, MinMsPerOpCICD: 5, MaxMsPerOpCICD: 20}, - "Setup Keys - L": {MinMsPerOpLocal: 30, MaxMsPerOpLocal: 50, MinMsPerOpCICD: 50, MaxMsPerOpCICD: 100}, - "Peers - L": {MinMsPerOpLocal: 30, MaxMsPerOpLocal: 50, MinMsPerOpCICD: 50, MaxMsPerOpCICD: 100}, - "Groups - L": {MinMsPerOpLocal: 30, MaxMsPerOpLocal: 50, MinMsPerOpCICD: 50, MaxMsPerOpCICD: 100}, - "Users - L": {MinMsPerOpLocal: 30, MaxMsPerOpLocal: 50, MinMsPerOpCICD: 50, MaxMsPerOpCICD: 100}, + "Setup Keys - L": {MinMsPerOpLocal: 30, MaxMsPerOpLocal: 50, MinMsPerOpCICD: 30, MaxMsPerOpCICD: 150}, + "Peers - L": {MinMsPerOpLocal: 30, MaxMsPerOpLocal: 50, MinMsPerOpCICD: 30, MaxMsPerOpCICD: 150}, + "Groups - L": {MinMsPerOpLocal: 30, MaxMsPerOpLocal: 50, MinMsPerOpCICD: 30, MaxMsPerOpCICD: 150}, + "Users - L": {MinMsPerOpLocal: 30, MaxMsPerOpLocal: 50, MinMsPerOpCICD: 30, MaxMsPerOpCICD: 150}, "Setup Keys - XL": {MinMsPerOpLocal: 140, MaxMsPerOpLocal: 220, MinMsPerOpCICD: 250, MaxMsPerOpCICD: 500}, } @@ -185,7 +185,7 @@ func BenchmarkGetAllSetupKeys(b *testing.B) { apiHandler.ServeHTTP(recorder, req) } - evaluateBenchmarkResults(b, name, time.Since(start), expectedMetrics[name]) + evaluateBenchmarkResults(b, name, time.Since(start), expectedMetrics[name], recorder) }) } } @@ -220,7 +220,7 @@ func BenchmarkDeleteSetupKey(b *testing.B) { apiHandler.ServeHTTP(recorder, req) } - evaluateBenchmarkResults(b, name, time.Since(start), expectedMetrics[name]) + evaluateBenchmarkResults(b, name, time.Since(start), expectedMetrics[name], recorder) }) } } diff --git a/management/server/http/tools_test.go b/management/server/http/tools_test.go index 3c6dd58cbe9..f23171bc520 100644 --- a/management/server/http/tools_test.go +++ b/management/server/http/tools_test.go @@ -245,9 +245,13 @@ func populateTestData(b *testing.B, am *server.DefaultAccountManager, peers, gro } -func evaluateBenchmarkResults(b *testing.B, name string, duration time.Duration, perfMetrics PerformanceMetrics) { +func evaluateBenchmarkResults(b *testing.B, name string, duration time.Duration, perfMetrics PerformanceMetrics, recorder *httptest.ResponseRecorder) { b.Helper() + if recorder.Code != http.StatusOK { + b.Fatalf("Benchmark %s failed: unexpected status code %d", name, recorder.Code) + } + msPerOp := float64(duration.Nanoseconds()) / float64(b.N) / 1e6 b.ReportMetric(msPerOp, "ms/op") From 2dfe7caf38054120ecda005dd46dec36e03696c1 Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Fri, 29 Nov 2024 17:22:20 +0100 Subject: [PATCH 30/38] add dummy keys to delete --- management/server/http/setupkeys_handler_benchmark_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/management/server/http/setupkeys_handler_benchmark_test.go b/management/server/http/setupkeys_handler_benchmark_test.go index 632daf4b54c..25574286933 100644 --- a/management/server/http/setupkeys_handler_benchmark_test.go +++ b/management/server/http/setupkeys_handler_benchmark_test.go @@ -210,12 +210,11 @@ func BenchmarkDeleteSetupKey(b *testing.B) { for name, bc := range benchCasesSetupKeys { b.Run(name, func(b *testing.B) { apiHandler, am, _ := buildApiBlackBoxWithDBState(b, "testdata/setup_keys.sql", nil) - populateTestData(b, am.(*server.DefaultAccountManager), bc.Peers, bc.Groups, bc.Users, bc.SetupKeys) + populateTestData(b, am.(*server.DefaultAccountManager), bc.Peers, bc.Groups, bc.Users, 1000) b.ResetTimer() start := time.Now() for i := 0; i < b.N; i++ { - // depending on the test case we may fail do delete keys as no more keys are there req := buildRequest(b, nil, http.MethodGet, "/api/setup-keys/"+"oldkey-"+strconv.Itoa(i), testAdminId) apiHandler.ServeHTTP(recorder, req) } From 430092cb995d06a18ecd470778bf4242a3819d9a Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Fri, 29 Nov 2024 17:37:27 +0100 Subject: [PATCH 31/38] fix operation --- management/server/http/setupkeys_handler_benchmark_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/management/server/http/setupkeys_handler_benchmark_test.go b/management/server/http/setupkeys_handler_benchmark_test.go index 25574286933..67dd39f9fc9 100644 --- a/management/server/http/setupkeys_handler_benchmark_test.go +++ b/management/server/http/setupkeys_handler_benchmark_test.go @@ -160,7 +160,7 @@ func BenchmarkGetAllSetupKeys(b *testing.B) { var expectedMetrics = map[string]PerformanceMetrics{ "Setup Keys - XS": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 10}, "Setup Keys - S": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, - "Setup Keys - M": {MinMsPerOpLocal: 5, MaxMsPerOpLocal: 10, MinMsPerOpCICD: 5, MaxMsPerOpCICD: 20}, + "Setup Keys - M": {MinMsPerOpLocal: 5, MaxMsPerOpLocal: 10, MinMsPerOpCICD: 5, MaxMsPerOpCICD: 30}, "Setup Keys - L": {MinMsPerOpLocal: 30, MaxMsPerOpLocal: 50, MinMsPerOpCICD: 30, MaxMsPerOpCICD: 150}, "Peers - L": {MinMsPerOpLocal: 30, MaxMsPerOpLocal: 50, MinMsPerOpCICD: 30, MaxMsPerOpCICD: 150}, "Groups - L": {MinMsPerOpLocal: 30, MaxMsPerOpLocal: 50, MinMsPerOpCICD: 30, MaxMsPerOpCICD: 150}, @@ -215,7 +215,7 @@ func BenchmarkDeleteSetupKey(b *testing.B) { b.ResetTimer() start := time.Now() for i := 0; i < b.N; i++ { - req := buildRequest(b, nil, http.MethodGet, "/api/setup-keys/"+"oldkey-"+strconv.Itoa(i), testAdminId) + req := buildRequest(b, nil, http.MethodDelete, "/api/setup-keys/"+"oldkey-"+strconv.Itoa(i), testAdminId) apiHandler.ServeHTTP(recorder, req) } From ccff4b429f8a2107c158a14d0f3d16b09d7a76d2 Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Fri, 29 Nov 2024 17:42:10 +0100 Subject: [PATCH 32/38] fix create test group --- management/server/http/setupkeys_handler_benchmark_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/management/server/http/setupkeys_handler_benchmark_test.go b/management/server/http/setupkeys_handler_benchmark_test.go index 67dd39f9fc9..224ea3f3e3a 100644 --- a/management/server/http/setupkeys_handler_benchmark_test.go +++ b/management/server/http/setupkeys_handler_benchmark_test.go @@ -55,7 +55,7 @@ func BenchmarkCreateSetupKey(b *testing.B) { start := time.Now() for i := 0; i < b.N; i++ { requestBody := api.CreateSetupKeyRequest{ - AutoGroups: []string{"someGroupID"}, + AutoGroups: []string{testGroupId}, ExpiresIn: expiresIn, Name: newKeyName + strconv.Itoa(i), Type: "reusable", From 798bddd69d40aaad3b33e0e0cd69a5026888696c Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Sun, 1 Dec 2024 23:14:14 +0100 Subject: [PATCH 33/38] update expected benchmark timings --- .../http/setupkeys_handler_benchmark_test.go | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/management/server/http/setupkeys_handler_benchmark_test.go b/management/server/http/setupkeys_handler_benchmark_test.go index 224ea3f3e3a..7a60c1f743e 100644 --- a/management/server/http/setupkeys_handler_benchmark_test.go +++ b/management/server/http/setupkeys_handler_benchmark_test.go @@ -31,14 +31,14 @@ var benchCasesSetupKeys = map[string]BenchmarkCase{ func BenchmarkCreateSetupKey(b *testing.B) { var expectedMetrics = map[string]PerformanceMetrics{ - "Setup Keys - XS": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, - "Setup Keys - S": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, - "Setup Keys - M": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, - "Setup Keys - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, - "Peers - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, - "Groups - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, - "Users - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, - "Setup Keys - XL": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, + "Setup Keys - XS": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 14}, + "Setup Keys - S": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 14}, + "Setup Keys - M": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 14}, + "Setup Keys - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 14}, + "Peers - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 14}, + "Groups - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 14}, + "Users - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 14}, + "Setup Keys - XL": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 14}, } log.SetOutput(io.Discard) @@ -77,14 +77,14 @@ func BenchmarkCreateSetupKey(b *testing.B) { func BenchmarkUpdateSetupKey(b *testing.B) { var expectedMetrics = map[string]PerformanceMetrics{ - "Setup Keys - XS": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 3, MaxMsPerOpCICD: 15}, - "Setup Keys - S": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 3, MaxMsPerOpCICD: 15}, - "Setup Keys - M": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 3, MaxMsPerOpCICD: 15}, - "Setup Keys - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 3, MaxMsPerOpCICD: 15}, - "Peers - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 3, MaxMsPerOpCICD: 15}, - "Groups - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 3, MaxMsPerOpCICD: 15}, - "Users - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 3, MaxMsPerOpCICD: 15}, - "Setup Keys - XL": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 3, MaxMsPerOpCICD: 15}, + "Setup Keys - XS": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 2, MaxMsPerOpCICD: 15}, + "Setup Keys - S": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 2, MaxMsPerOpCICD: 15}, + "Setup Keys - M": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 2, MaxMsPerOpCICD: 15}, + "Setup Keys - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 2, MaxMsPerOpCICD: 15}, + "Peers - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 2, MaxMsPerOpCICD: 15}, + "Groups - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 2, MaxMsPerOpCICD: 15}, + "Users - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 2, MaxMsPerOpCICD: 15}, + "Setup Keys - XL": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 2, MaxMsPerOpCICD: 15}, } log.SetOutput(io.Discard) @@ -165,7 +165,7 @@ func BenchmarkGetAllSetupKeys(b *testing.B) { "Peers - L": {MinMsPerOpLocal: 30, MaxMsPerOpLocal: 50, MinMsPerOpCICD: 30, MaxMsPerOpCICD: 150}, "Groups - L": {MinMsPerOpLocal: 30, MaxMsPerOpLocal: 50, MinMsPerOpCICD: 30, MaxMsPerOpCICD: 150}, "Users - L": {MinMsPerOpLocal: 30, MaxMsPerOpLocal: 50, MinMsPerOpCICD: 30, MaxMsPerOpCICD: 150}, - "Setup Keys - XL": {MinMsPerOpLocal: 140, MaxMsPerOpLocal: 220, MinMsPerOpCICD: 250, MaxMsPerOpCICD: 500}, + "Setup Keys - XL": {MinMsPerOpLocal: 140, MaxMsPerOpLocal: 220, MinMsPerOpCICD: 200, MaxMsPerOpCICD: 500}, } log.SetOutput(io.Discard) From 86a9bc755a99247027fba28fdce0efbf98f0c88c Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Sun, 1 Dec 2024 23:36:02 +0100 Subject: [PATCH 34/38] update expected benchmark timings --- .../http/setupkeys_handler_benchmark_test.go | 64 +++++++++---------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/management/server/http/setupkeys_handler_benchmark_test.go b/management/server/http/setupkeys_handler_benchmark_test.go index 7a60c1f743e..804de8ee8d6 100644 --- a/management/server/http/setupkeys_handler_benchmark_test.go +++ b/management/server/http/setupkeys_handler_benchmark_test.go @@ -31,14 +31,14 @@ var benchCasesSetupKeys = map[string]BenchmarkCase{ func BenchmarkCreateSetupKey(b *testing.B) { var expectedMetrics = map[string]PerformanceMetrics{ - "Setup Keys - XS": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 14}, - "Setup Keys - S": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 14}, - "Setup Keys - M": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 14}, - "Setup Keys - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 14}, - "Peers - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 14}, - "Groups - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 14}, - "Users - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 14}, - "Setup Keys - XL": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 14}, + "Setup Keys - XS": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 16}, + "Setup Keys - S": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 16}, + "Setup Keys - M": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 16}, + "Setup Keys - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 16}, + "Peers - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 16}, + "Groups - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 16}, + "Users - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 16}, + "Setup Keys - XL": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 16}, } log.SetOutput(io.Discard) @@ -77,14 +77,14 @@ func BenchmarkCreateSetupKey(b *testing.B) { func BenchmarkUpdateSetupKey(b *testing.B) { var expectedMetrics = map[string]PerformanceMetrics{ - "Setup Keys - XS": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 2, MaxMsPerOpCICD: 15}, - "Setup Keys - S": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 2, MaxMsPerOpCICD: 15}, - "Setup Keys - M": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 2, MaxMsPerOpCICD: 15}, - "Setup Keys - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 2, MaxMsPerOpCICD: 15}, - "Peers - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 2, MaxMsPerOpCICD: 15}, - "Groups - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 2, MaxMsPerOpCICD: 15}, - "Users - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 2, MaxMsPerOpCICD: 15}, - "Setup Keys - XL": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 2, MaxMsPerOpCICD: 15}, + "Setup Keys - XS": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 2, MaxMsPerOpCICD: 16}, + "Setup Keys - S": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 2, MaxMsPerOpCICD: 16}, + "Setup Keys - M": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 2, MaxMsPerOpCICD: 16}, + "Setup Keys - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 2, MaxMsPerOpCICD: 16}, + "Peers - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 2, MaxMsPerOpCICD: 16}, + "Groups - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 2, MaxMsPerOpCICD: 16}, + "Users - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 2, MaxMsPerOpCICD: 16}, + "Setup Keys - XL": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 2, MaxMsPerOpCICD: 16}, } log.SetOutput(io.Discard) @@ -124,14 +124,14 @@ func BenchmarkUpdateSetupKey(b *testing.B) { func BenchmarkGetOneSetupKey(b *testing.B) { var expectedMetrics = map[string]PerformanceMetrics{ - "Setup Keys - XS": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, - "Setup Keys - S": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, - "Setup Keys - M": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, - "Setup Keys - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, - "Peers - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, - "Groups - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, - "Users - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, - "Setup Keys - XL": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, + "Setup Keys - XS": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 16}, + "Setup Keys - S": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 16}, + "Setup Keys - M": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 16}, + "Setup Keys - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 16}, + "Peers - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 16}, + "Groups - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 16}, + "Users - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 16}, + "Setup Keys - XL": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 16}, } log.SetOutput(io.Discard) @@ -192,14 +192,14 @@ func BenchmarkGetAllSetupKeys(b *testing.B) { func BenchmarkDeleteSetupKey(b *testing.B) { var expectedMetrics = map[string]PerformanceMetrics{ - "Setup Keys - XS": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, - "Setup Keys - S": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, - "Setup Keys - M": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, - "Setup Keys - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, - "Peers - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, - "Groups - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, - "Users - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, - "Setup Keys - XL": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, + "Setup Keys - XS": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 16}, + "Setup Keys - S": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 16}, + "Setup Keys - M": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 16}, + "Setup Keys - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 16}, + "Peers - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 16}, + "Groups - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 16}, + "Users - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 16}, + "Setup Keys - XL": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 16}, } log.SetOutput(io.Discard) From b3384292e380ca19e2a8b8baa1b6e97b131d4be1 Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Sun, 1 Dec 2024 23:53:49 +0100 Subject: [PATCH 35/38] update expected benchmark timings --- management/server/account_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/management/server/account_test.go b/management/server/account_test.go index 8d44c664b7e..5a8ec60fc01 100644 --- a/management/server/account_test.go +++ b/management/server/account_test.go @@ -3099,10 +3099,10 @@ func BenchmarkLoginPeer_NewPeer(b *testing.B) { }{ {"Small", 50, 5, 107, 120, 107, 140}, {"Medium", 500, 100, 105, 140, 105, 170}, - {"Large", 5000, 200, 180, 220, 180, 320}, + {"Large", 5000, 200, 180, 220, 180, 340}, {"Small single", 50, 10, 107, 120, 105, 140}, {"Medium single", 500, 10, 105, 140, 105, 170}, - {"Large 5", 5000, 15, 180, 220, 180, 320}, + {"Large 5", 5000, 15, 180, 220, 180, 340}, } log.SetOutput(io.Discard) From 384bf510fb6aede71fc01289e7916cb6797ed6c8 Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Mon, 2 Dec 2024 00:16:20 +0100 Subject: [PATCH 36/38] update expected benchmark timings --- management/server/http/setupkeys_handler_benchmark_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/management/server/http/setupkeys_handler_benchmark_test.go b/management/server/http/setupkeys_handler_benchmark_test.go index 804de8ee8d6..b4f44c77225 100644 --- a/management/server/http/setupkeys_handler_benchmark_test.go +++ b/management/server/http/setupkeys_handler_benchmark_test.go @@ -158,9 +158,9 @@ func BenchmarkGetOneSetupKey(b *testing.B) { func BenchmarkGetAllSetupKeys(b *testing.B) { var expectedMetrics = map[string]PerformanceMetrics{ - "Setup Keys - XS": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 10}, - "Setup Keys - S": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, - "Setup Keys - M": {MinMsPerOpLocal: 5, MaxMsPerOpLocal: 10, MinMsPerOpCICD: 5, MaxMsPerOpCICD: 30}, + "Setup Keys - XS": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 12}, + "Setup Keys - S": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 2, MinMsPerOpCICD: 1, MaxMsPerOpCICD: 15}, + "Setup Keys - M": {MinMsPerOpLocal: 5, MaxMsPerOpLocal: 10, MinMsPerOpCICD: 5, MaxMsPerOpCICD: 40}, "Setup Keys - L": {MinMsPerOpLocal: 30, MaxMsPerOpLocal: 50, MinMsPerOpCICD: 30, MaxMsPerOpCICD: 150}, "Peers - L": {MinMsPerOpLocal: 30, MaxMsPerOpLocal: 50, MinMsPerOpCICD: 30, MaxMsPerOpCICD: 150}, "Groups - L": {MinMsPerOpLocal: 30, MaxMsPerOpLocal: 50, MinMsPerOpCICD: 30, MaxMsPerOpCICD: 150}, From 437d3dac408604d7ed5eeef93fdc4b2981afdca6 Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Mon, 2 Dec 2024 10:39:22 +0100 Subject: [PATCH 37/38] update expected benchmark timings --- management/server/http/setupkeys_handler_benchmark_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/management/server/http/setupkeys_handler_benchmark_test.go b/management/server/http/setupkeys_handler_benchmark_test.go index b4f44c77225..97dcdf70d10 100644 --- a/management/server/http/setupkeys_handler_benchmark_test.go +++ b/management/server/http/setupkeys_handler_benchmark_test.go @@ -165,7 +165,7 @@ func BenchmarkGetAllSetupKeys(b *testing.B) { "Peers - L": {MinMsPerOpLocal: 30, MaxMsPerOpLocal: 50, MinMsPerOpCICD: 30, MaxMsPerOpCICD: 150}, "Groups - L": {MinMsPerOpLocal: 30, MaxMsPerOpLocal: 50, MinMsPerOpCICD: 30, MaxMsPerOpCICD: 150}, "Users - L": {MinMsPerOpLocal: 30, MaxMsPerOpLocal: 50, MinMsPerOpCICD: 30, MaxMsPerOpCICD: 150}, - "Setup Keys - XL": {MinMsPerOpLocal: 140, MaxMsPerOpLocal: 220, MinMsPerOpCICD: 200, MaxMsPerOpCICD: 500}, + "Setup Keys - XL": {MinMsPerOpLocal: 140, MaxMsPerOpLocal: 220, MinMsPerOpCICD: 150, MaxMsPerOpCICD: 500}, } log.SetOutput(io.Discard) From bb4696f9b47a547a0cb3ec4ee695f190b31533ce Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Mon, 2 Dec 2024 11:38:17 +0100 Subject: [PATCH 38/38] update expected benchmark timings --- .../http/setupkeys_handler_benchmark_test.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/management/server/http/setupkeys_handler_benchmark_test.go b/management/server/http/setupkeys_handler_benchmark_test.go index 97dcdf70d10..e6a9673b4f6 100644 --- a/management/server/http/setupkeys_handler_benchmark_test.go +++ b/management/server/http/setupkeys_handler_benchmark_test.go @@ -77,14 +77,14 @@ func BenchmarkCreateSetupKey(b *testing.B) { func BenchmarkUpdateSetupKey(b *testing.B) { var expectedMetrics = map[string]PerformanceMetrics{ - "Setup Keys - XS": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 2, MaxMsPerOpCICD: 16}, - "Setup Keys - S": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 2, MaxMsPerOpCICD: 16}, - "Setup Keys - M": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 2, MaxMsPerOpCICD: 16}, - "Setup Keys - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 2, MaxMsPerOpCICD: 16}, - "Peers - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 2, MaxMsPerOpCICD: 16}, - "Groups - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 2, MaxMsPerOpCICD: 16}, - "Users - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 2, MaxMsPerOpCICD: 16}, - "Setup Keys - XL": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 2, MaxMsPerOpCICD: 16}, + "Setup Keys - XS": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 2, MaxMsPerOpCICD: 19}, + "Setup Keys - S": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 2, MaxMsPerOpCICD: 19}, + "Setup Keys - M": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 2, MaxMsPerOpCICD: 19}, + "Setup Keys - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 2, MaxMsPerOpCICD: 19}, + "Peers - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 2, MaxMsPerOpCICD: 19}, + "Groups - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 2, MaxMsPerOpCICD: 19}, + "Users - L": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 2, MaxMsPerOpCICD: 19}, + "Setup Keys - XL": {MinMsPerOpLocal: 0.5, MaxMsPerOpLocal: 3, MinMsPerOpCICD: 2, MaxMsPerOpCICD: 19}, } log.SetOutput(io.Discard)