-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Add GraphQL mutation for configuring code host rate limits #56150
base: main
Are you sure you want to change the base?
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,16 +1,32 @@ | ||
| package graphqlbackend | ||
|
|
||
| import ( | ||
| "context" | ||
|
|
||
| "github.com/graph-gophers/graphql-go" | ||
| "github.com/sourcegraph/sourcegraph/internal/auth" | ||
| "github.com/sourcegraph/sourcegraph/internal/database" | ||
| "github.com/sourcegraph/sourcegraph/internal/types" | ||
| "github.com/sourcegraph/sourcegraph/lib/errors" | ||
| ) | ||
|
|
||
| type codeHostResolver struct { | ||
| ch *types.CodeHost | ||
| db database.DB | ||
| } | ||
|
|
||
| type SetCodeHostRateLimitsArgs struct { | ||
| Input SetCodeHostRateLimitsInput | ||
| } | ||
|
|
||
| type SetCodeHostRateLimitsInput struct { | ||
| CodeHostID graphql.ID | ||
| APIQuota int32 | ||
| APIReplenishmentIntervalSeconds int32 | ||
| GitQuota int32 | ||
| GitReplenishmentIntervalSeconds int32 | ||
| } | ||
|
|
||
| func (r *codeHostResolver) ID() graphql.ID { | ||
| return MarshalCodeHostID(r.ch.ID) | ||
| } | ||
|
|
@@ -65,3 +81,34 @@ func (r *codeHostResolver) ExternalServices(args *CodeHostExternalServicesArgs) | |
| } | ||
| return &externalServiceConnectionResolver{db: r.db, opt: opt}, nil | ||
| } | ||
|
|
||
| func (r *schemaResolver) SetCodeHostRateLimits(ctx context.Context, args SetCodeHostRateLimitsArgs) (*EmptyResponse, error) { | ||
| // Security 🚨: Code Hosts may only be updated by site admins. | ||
| if err := auth.CheckCurrentUserIsSiteAdmin(ctx, r.db); err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| if args.Input.APIQuota < 0 || args.Input.GitQuota < 0 || args.Input.APIReplenishmentIntervalSeconds < 0 || args.Input.GitReplenishmentIntervalSeconds < 0 { | ||
| return nil, errors.New("rate limit settings must be positive integers") | ||
| } | ||
|
|
||
| codeHostID, err := UnmarshalCodeHostID(args.Input.CodeHostID) | ||
| if err != nil { | ||
| return nil, errors.Wrap(err, "invalid code host id") | ||
| } | ||
|
|
||
| codeHost, err := r.db.CodeHosts().GetByID(ctx, codeHostID) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| codeHost.APIRateLimitQuota = &args.Input.APIQuota | ||
| codeHost.APIRateLimitIntervalSeconds = &args.Input.APIReplenishmentIntervalSeconds | ||
| codeHost.GitRateLimitQuota = &args.Input.GitQuota | ||
| codeHost.GitRateLimitIntervalSeconds = &args.Input.GitReplenishmentIntervalSeconds | ||
|
|
||
| err = r.db.CodeHosts().Update(ctx, codeHost) | ||
|
||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| return &EmptyResponse{}, err | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,223 @@ | ||
| package graphqlbackend | ||
|
|
||
| import ( | ||
| "context" | ||
| "testing" | ||
|
|
||
| "github.com/sourcegraph/log/logtest" | ||
| "github.com/sourcegraph/sourcegraph/internal/database/dbmocks" | ||
| "github.com/sourcegraph/sourcegraph/internal/types" | ||
| "github.com/sourcegraph/sourcegraph/lib/errors" | ||
| "github.com/stretchr/testify/assert" | ||
| ) | ||
|
|
||
| func TestSchemaResolver_SetCodeHostRateLimits_NotASiteAdmin(t *testing.T) { | ||
| logger := logtest.Scoped(t) | ||
| db := dbmocks.NewMockDB() | ||
| r := &schemaResolver{logger: logger, db: db} | ||
|
|
||
| usersStore := dbmocks.NewMockUserStore() | ||
| usersStore.GetByCurrentAuthUserFunc.SetDefaultReturn(&types.User{SiteAdmin: false}, nil) | ||
| db.UsersFunc.SetDefaultReturn(usersStore) | ||
|
|
||
| _, err := r.SetCodeHostRateLimits(context.Background(), SetCodeHostRateLimitsArgs{ | ||
| Input: SetCodeHostRateLimitsInput{}, | ||
| }) | ||
| assert.NotNil(t, err) | ||
varsanojidan marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| assert.Equal(t, "must be site admin", err.Error()) | ||
| } | ||
|
|
||
| func TestSchemaResolver_SetCodeHostRateLimits_InvalidConfigs(t *testing.T) { | ||
| logger := logtest.Scoped(t) | ||
| db := dbmocks.NewMockDB() | ||
| r := &schemaResolver{logger: logger, db: db} | ||
| ctx := context.Background() | ||
| wantErr := errors.New("rate limit settings must be positive integers") | ||
varsanojidan marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| tests := []struct { | ||
| name string | ||
| args SetCodeHostRateLimitsArgs | ||
| wantErr error | ||
| }{ | ||
| { | ||
| name: "Negative APIQuota", | ||
| args: SetCodeHostRateLimitsArgs{ | ||
| Input: SetCodeHostRateLimitsInput{ | ||
| CodeHostID: "Q29kZUhvc3Q6MQ==", | ||
| APIQuota: -1, | ||
| }, | ||
| }, | ||
| wantErr: wantErr, | ||
| }, | ||
| { | ||
| name: "Negative APIReplenishmentIntervalSeconds", | ||
| args: SetCodeHostRateLimitsArgs{ | ||
| Input: SetCodeHostRateLimitsInput{ | ||
| CodeHostID: "Q29kZUhvc3Q6MQ==", | ||
| APIReplenishmentIntervalSeconds: -1, | ||
| }, | ||
| }, | ||
| wantErr: wantErr, | ||
| }, | ||
| { | ||
| name: "Negative GitQuota", | ||
| args: SetCodeHostRateLimitsArgs{ | ||
| Input: SetCodeHostRateLimitsInput{ | ||
| CodeHostID: "Q29kZUhvc3Q6MQ==", | ||
| GitQuota: -1, | ||
| }, | ||
| }, | ||
| wantErr: wantErr, | ||
| }, | ||
| { | ||
| name: "Negative GitReplenishmentIntervalSeconds", | ||
| args: SetCodeHostRateLimitsArgs{ | ||
| Input: SetCodeHostRateLimitsInput{ | ||
| CodeHostID: "Q29kZUhvc3Q6MQ==", | ||
| GitReplenishmentIntervalSeconds: -1, | ||
| }, | ||
| }, | ||
| wantErr: wantErr, | ||
| }, | ||
| } | ||
|
|
||
| for _, test := range tests { | ||
| t.Run(test.name, func(t *testing.T) { | ||
| usersStore := dbmocks.NewMockUserStore() | ||
| usersStore.GetByCurrentAuthUserFunc.SetDefaultReturn(&types.User{SiteAdmin: true}, nil) | ||
| db.UsersFunc.SetDefaultReturn(usersStore) | ||
|
|
||
| _, err := r.SetCodeHostRateLimits(ctx, test.args) | ||
| assert.NotNil(t, err) | ||
varsanojidan marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| assert.Equal(t, "rate limit settings must be positive integers", err.Error()) | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func TestSchemaResolver_SetCodeHostRateLimits_InvalidCodeHostID(t *testing.T) { | ||
| logger := logtest.Scoped(t) | ||
| db := dbmocks.NewMockDB() | ||
| r := &schemaResolver{logger: logger, db: db} | ||
| wantErr := errors.New("test error") | ||
|
|
||
| usersStore := dbmocks.NewMockUserStore() | ||
| usersStore.GetByCurrentAuthUserFunc.SetDefaultReturn(&types.User{SiteAdmin: true}, nil) | ||
| db.UsersFunc.SetDefaultReturn(usersStore) | ||
|
|
||
| codeHostStore := dbmocks.NewMockCodeHostStore() | ||
| codeHostStore.GetByIDFunc.SetDefaultHook(func(ctx context.Context, id int32) (*types.CodeHost, error) { | ||
| return nil, wantErr | ||
| }) | ||
| db.CodeHostsFunc.SetDefaultReturn(codeHostStore) | ||
|
|
||
| _, err := r.SetCodeHostRateLimits(context.Background(), SetCodeHostRateLimitsArgs{ | ||
| Input: SetCodeHostRateLimitsInput{CodeHostID: ""}, | ||
| }) | ||
| assert.NotNil(t, err) | ||
| assert.Equal(t, "invalid code host id: invalid graphql.ID", err.Error()) | ||
| } | ||
|
|
||
| func TestSchemaResolver_SetCodeHostRateLimits_GetCodeHostByIDError(t *testing.T) { | ||
| logger := logtest.Scoped(t) | ||
| db := dbmocks.NewMockDB() | ||
| r := &schemaResolver{logger: logger, db: db} | ||
| wantErr := errors.New("test error") | ||
|
|
||
| usersStore := dbmocks.NewMockUserStore() | ||
| usersStore.GetByCurrentAuthUserFunc.SetDefaultReturn(&types.User{SiteAdmin: true}, nil) | ||
| db.UsersFunc.SetDefaultReturn(usersStore) | ||
|
|
||
| codeHostStore := dbmocks.NewMockCodeHostStore() | ||
| codeHostStore.GetByIDFunc.SetDefaultHook(func(ctx context.Context, id int32) (*types.CodeHost, error) { | ||
| return nil, wantErr | ||
| }) | ||
| db.CodeHostsFunc.SetDefaultReturn(codeHostStore) | ||
|
|
||
| _, err := r.SetCodeHostRateLimits(context.Background(), SetCodeHostRateLimitsArgs{ | ||
| Input: SetCodeHostRateLimitsInput{CodeHostID: "Q29kZUhvc3Q6MQ=="}, | ||
| }) | ||
| assert.NotNil(t, err) | ||
| assert.Equal(t, wantErr.Error(), err.Error()) | ||
| } | ||
|
|
||
| func TestSchemaResolver_SetCodeHostRateLimits_UpdateCodeHostError(t *testing.T) { | ||
| logger := logtest.Scoped(t) | ||
| db := dbmocks.NewMockDB() | ||
| r := &schemaResolver{logger: logger, db: db} | ||
| wantErr := errors.New("test error") | ||
|
|
||
| usersStore := dbmocks.NewMockUserStore() | ||
| usersStore.GetByCurrentAuthUserFunc.SetDefaultReturn(&types.User{SiteAdmin: true}, nil) | ||
| db.UsersFunc.SetDefaultReturn(usersStore) | ||
|
|
||
| codeHostStore := dbmocks.NewMockCodeHostStore() | ||
| codeHostStore.GetByIDFunc.SetDefaultHook(func(ctx context.Context, id int32) (*types.CodeHost, error) { | ||
| assert.Equal(t, int32(1), id) | ||
| return &types.CodeHost{ID: 1}, nil | ||
| }) | ||
| codeHostStore.UpdateFunc.SetDefaultHook(func(ctx context.Context, host *types.CodeHost) error { | ||
| return wantErr | ||
| }) | ||
| db.CodeHostsFunc.SetDefaultReturn(codeHostStore) | ||
|
|
||
| _, err := r.SetCodeHostRateLimits(context.Background(), SetCodeHostRateLimitsArgs{ | ||
| Input: SetCodeHostRateLimitsInput{CodeHostID: "Q29kZUhvc3Q6MQ=="}, | ||
| }) | ||
| assert.NotNil(t, err) | ||
| assert.Equal(t, wantErr.Error(), err.Error()) | ||
| } | ||
|
|
||
| func TestSchemaResolver_SetCodeHostRateLimits_Success(t *testing.T) { | ||
| db := dbmocks.NewMockDB() | ||
| ctx := context.Background() | ||
| setCodeHostRateLimitsInput := SetCodeHostRateLimitsInput{ | ||
| CodeHostID: "Q29kZUhvc3Q6MQ==", | ||
| APIQuota: 1, | ||
| APIReplenishmentIntervalSeconds: 2, | ||
| GitQuota: 3, | ||
| GitReplenishmentIntervalSeconds: 4, | ||
| } | ||
|
|
||
| usersStore := dbmocks.NewMockUserStore() | ||
| usersStore.GetByCurrentAuthUserFunc.SetDefaultReturn(&types.User{SiteAdmin: true}, nil) | ||
| db.UsersFunc.SetDefaultReturn(usersStore) | ||
|
|
||
| codeHostStore := dbmocks.NewMockCodeHostStore() | ||
| codeHostStore.GetByIDFunc.SetDefaultHook(func(ctx context.Context, id int32) (*types.CodeHost, error) { | ||
| assert.Equal(t, int32(1), id) | ||
| return &types.CodeHost{ID: 1}, nil | ||
| }) | ||
| codeHostStore.UpdateFunc.SetDefaultHook(func(ctx context.Context, host *types.CodeHost) error { | ||
| assert.Equal(t, setCodeHostRateLimitsInput.APIQuota, *(host.APIRateLimitQuota)) | ||
| assert.Equal(t, setCodeHostRateLimitsInput.APIReplenishmentIntervalSeconds, *(host.APIRateLimitIntervalSeconds)) | ||
| assert.Equal(t, setCodeHostRateLimitsInput.GitQuota, *(host.GitRateLimitQuota)) | ||
| assert.Equal(t, setCodeHostRateLimitsInput.GitReplenishmentIntervalSeconds, *(host.GitRateLimitIntervalSeconds)) | ||
| return nil | ||
| }) | ||
| db.CodeHostsFunc.SetDefaultReturn(codeHostStore) | ||
|
|
||
| variables := map[string]any{ | ||
| "input": map[string]any{ | ||
| "codeHostID": string(setCodeHostRateLimitsInput.CodeHostID), | ||
| "apiQuota": setCodeHostRateLimitsInput.APIQuota, | ||
| "apiReplenishmentIntervalSeconds": setCodeHostRateLimitsInput.APIReplenishmentIntervalSeconds, | ||
| "gitQuota": setCodeHostRateLimitsInput.GitQuota, | ||
| "gitReplenishmentIntervalSeconds": setCodeHostRateLimitsInput.GitReplenishmentIntervalSeconds, | ||
| }, | ||
| } | ||
| RunTest(t, &Test{ | ||
| Context: ctx, | ||
| Schema: mustParseGraphQLSchema(t, db), | ||
| Variables: variables, | ||
| Query: `mutation setCodeHostRateLimits($input:SetCodeHostRateLimitsInput!) { | ||
| setCodeHostRateLimits(input:$input) { | ||
| alwaysNil | ||
| } | ||
| }`, | ||
| ExpectedResult: `{ | ||
| "setCodeHostRateLimits": { | ||
| "alwaysNil": null | ||
| } | ||
| }`, | ||
| }) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10604,6 +10604,43 @@ type PerforceChangelist { | |
| commit: GitCommit! | ||
| } | ||
|
|
||
| extend type Mutation { | ||
| """ | ||
| Updates a code host's rate limit configurations. All rate limit values must be positive integers. | ||
| """ | ||
| setCodeHostRateLimits(input: SetCodeHostRateLimitsInput!): EmptyResponse | ||
| } | ||
|
|
||
| """ | ||
| SetCodeHostRateLimitsInput represents the input for configuring rate limits for a code host. | ||
| """ | ||
| input SetCodeHostRateLimitsInput { | ||
| """ | ||
| ID of the code host for which rate limits are being set. | ||
| """ | ||
| codeHostID: ID! | ||
|
|
||
| """ | ||
| The maximum number of API requests allowed per time window defined by apiReplenishmentIntervalSeconds. | ||
| """ | ||
| apiQuota: Int! | ||
|
|
||
| """ | ||
| The time interval at which the apiQuota's worth of API requests are replenished. | ||
| """ | ||
| apiReplenishmentIntervalSeconds: Int! | ||
|
|
||
| """ | ||
| The maximum number of Git requests allowed per time window defined by gitReplenishmentIntervalSeconds. | ||
| """ | ||
| gitQuota: Int! | ||
|
|
||
| """ | ||
| The time interval at which the gitQuota's worth of Git requests are replenished. | ||
| """ | ||
| gitReplenishmentIntervalSeconds: Int! | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I made all of the rate limit configs required for the update because this will be used by a UI that would update all of the rate limits for a code host at the same time, and translates to one Lmk if you think otherwise 😄. |
||
| } | ||
|
|
||
| extend type Query { | ||
| """ | ||
| List of all configured code hosts on this instance. | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.