From fe5adbbbdc0862736de37026808d4cd72adf4dab Mon Sep 17 00:00:00 2001 From: Royce Remer Date: Tue, 29 Oct 2024 22:41:55 -0700 Subject: [PATCH] Add new [lfs_client].BATCH_SIZE and [server].LFS_MAX_BATCH_SIZE config settings. (#32307) This contains two backwards-compatible changes: * in the lfs http_client, the number of lfs oids requested per batch is loaded from lfs_client#BATCH_SIZE and defaulted to the previous value of 20 * in the lfs server/service, the max number of lfs oids allowed in a batch api request is loaded from server#LFS_MAX_BATCH_SIZE and defaults to 'nil' which equates to the previous behavior of 'infinite' This fixes #32306 --------- Signed-off-by: Royce Remer Co-authored-by: wxiaoguang (cherry picked from commit c60e4dc1095ef90a790582cacfad27c972637bb2) Conflicts: - services/lfs/server.go Conflict due to our Quota implementation. Resolved by manually adding the change after the quota check. --- custom/conf/app.example.ini | 8 ++++++++ modules/lfs/http_client.go | 5 ++--- modules/setting/lfs.go | 21 +++++++++++++++++---- modules/setting/lfs_test.go | 16 ++++++++++++++++ services/lfs/server.go | 5 +++++ 5 files changed, 48 insertions(+), 7 deletions(-) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index 3f0e9c447d..f0fd40da5f 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -328,6 +328,10 @@ RUN_USER = ; git ;; Maximum number of locks returned per page ;LFS_LOCKS_PAGING_NUM = 50 ;; +;; When clients make lfs batch requests, reject them if there are more pointers than this number +;; zero means 'unlimited' +;LFS_MAX_BATCH_SIZE = 0 +;; ;; Allow graceful restarts using SIGHUP to fork ;ALLOW_GRACEFUL_RESTARTS = true ;; @@ -2672,6 +2676,10 @@ LEVEL = Info ;; override the minio base path if storage type is minio ;MINIO_BASE_PATH = lfs/ +;[lfs_client] +;; When mirroring an upstream lfs endpoint, limit the number of pointers in each batch request to this number +;BATCH_SIZE = 20 + ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; settings for packages, will override storage setting diff --git a/modules/lfs/http_client.go b/modules/lfs/http_client.go index 4859fe61e1..aa9e744d72 100644 --- a/modules/lfs/http_client.go +++ b/modules/lfs/http_client.go @@ -16,10 +16,9 @@ import ( "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/proxy" + "code.gitea.io/gitea/modules/setting" ) -const httpBatchSize = 20 - // HTTPClient is used to communicate with the LFS server // https://github.com/git-lfs/git-lfs/blob/main/docs/api/batch.md type HTTPClient struct { @@ -30,7 +29,7 @@ type HTTPClient struct { // BatchSize returns the preferred size of batchs to process func (c *HTTPClient) BatchSize() int { - return httpBatchSize + return setting.LFSClient.BatchSize } func newHTTPClient(endpoint *url.URL, httpTransport *http.Transport) *HTTPClient { diff --git a/modules/setting/lfs.go b/modules/setting/lfs.go index 750101747f..ebb234a3ef 100644 --- a/modules/setting/lfs.go +++ b/modules/setting/lfs.go @@ -10,22 +10,31 @@ import ( "code.gitea.io/gitea/modules/generate" ) -// LFS represents the configuration for Git LFS +// LFS represents the server-side configuration for Git LFS. +// Ideally these options should be in a section like "[lfs_server]", +// but they are in "[server]" section due to historical reasons. +// Could be refactored in the future while keeping backwards compatibility. var LFS = struct { StartServer bool `ini:"LFS_START_SERVER"` JWTSecretBytes []byte `ini:"-"` HTTPAuthExpiry time.Duration `ini:"LFS_HTTP_AUTH_EXPIRY"` MaxFileSize int64 `ini:"LFS_MAX_FILE_SIZE"` LocksPagingNum int `ini:"LFS_LOCKS_PAGING_NUM"` + MaxBatchSize int `ini:"LFS_MAX_BATCH_SIZE"` Storage *Storage }{} +// LFSClient represents configuration for Gitea's LFS clients, for example: mirroring upstream Git LFS +var LFSClient = struct { + BatchSize int `ini:"BATCH_SIZE"` +}{} + func loadLFSFrom(rootCfg ConfigProvider) error { + mustMapSetting(rootCfg, "lfs_client", &LFSClient) + + mustMapSetting(rootCfg, "server", &LFS) sec := rootCfg.Section("server") - if err := sec.MapTo(&LFS); err != nil { - return fmt.Errorf("failed to map LFS settings: %v", err) - } lfsSec, _ := rootCfg.GetSection("lfs") @@ -52,6 +61,10 @@ func loadLFSFrom(rootCfg ConfigProvider) error { LFS.LocksPagingNum = 50 } + if LFSClient.BatchSize < 1 { + LFSClient.BatchSize = 20 + } + LFS.HTTPAuthExpiry = sec.Key("LFS_HTTP_AUTH_EXPIRY").MustDuration(24 * time.Hour) if !LFS.StartServer || !InstallLock { diff --git a/modules/setting/lfs_test.go b/modules/setting/lfs_test.go index c7f16379b2..324965781d 100644 --- a/modules/setting/lfs_test.go +++ b/modules/setting/lfs_test.go @@ -100,3 +100,19 @@ STORAGE_TYPE = minio assert.EqualValues(t, "gitea", LFS.Storage.MinioConfig.Bucket) assert.EqualValues(t, "lfs/", LFS.Storage.MinioConfig.BasePath) } + +func Test_LFSClientServerConfigs(t *testing.T) { + iniStr := ` +[server] +LFS_MAX_BATCH_SIZE = 100 +[lfs_client] +# will default to 20 +BATCH_SIZE = 0 +` + cfg, err := NewConfigProviderFromData(iniStr) + assert.NoError(t, err) + + assert.NoError(t, loadLFSFrom(cfg)) + assert.EqualValues(t, 100, LFS.MaxBatchSize) + assert.EqualValues(t, 20, LFSClient.BatchSize) +} diff --git a/services/lfs/server.go b/services/lfs/server.go index a300de19c4..225dfdb024 100644 --- a/services/lfs/server.go +++ b/services/lfs/server.go @@ -192,6 +192,11 @@ func BatchHandler(ctx *context.Context) { } } + if setting.LFS.MaxBatchSize != 0 && len(br.Objects) > setting.LFS.MaxBatchSize { + writeStatus(ctx, http.StatusRequestEntityTooLarge) + return + } + contentStore := lfs_module.NewContentStore() var responseObjects []*lfs_module.ObjectResponse