From db5a8da26577ec4aece2bd3d0d0d1e1383d7766f Mon Sep 17 00:00:00 2001 From: DmitryFrolovTri <23313323+DmitryFrolovTri@users.noreply.github.com> Date: Tue, 23 May 2023 04:23:59 +0000 Subject: [PATCH 1/2] Refactor hook_pre_recieve to increase speed of size checking --- routers/private/hook_pre_receive.go | 153 ++++++++++++++++------------ 1 file changed, 86 insertions(+), 67 deletions(-) diff --git a/routers/private/hook_pre_receive.go b/routers/private/hook_pre_receive.go index ddde140d17..e0be78c4dd 100644 --- a/routers/private/hook_pre_receive.go +++ b/routers/private/hook_pre_receive.go @@ -9,6 +9,7 @@ import ( "os" "strconv" "strings" + "time" "code.gitea.io/gitea/models" asymkey_model "code.gitea.io/gitea/models/asymkey" @@ -104,70 +105,72 @@ func (ctx *preReceiveContext) AssertCreatePullRequest() bool { return true } -// CalculateSizeOfAddedObjects calculates the total size of objects provided as output from rev-list command -func CalculateSizeOfAddedObjects(ctx *gitea_context.PrivateContext, opts *git.RunOpts, revlistObjects string) int64 { - // Calculate the size of added Objects. - var totalSize int64 - for _, object := range strings.Split(revlistObjects, "\n") { - if len(object) == 0 { - continue - } - objectID := strings.Split(object, " ")[0] - objectSizeStr, _, err := git.NewCommand(ctx, "cat-file", "-s").AddDynamicArguments(objectID).RunStdString(opts) - if err != nil { - log.Trace("CalculateSizeOfAddedObjects: Error during git cat-file -s on object %s", objectID) - return totalSize - } - objectSize, _ := strconv.ParseInt(strings.TrimSpace(objectSizeStr), 10, 64) - if err != nil { - log.Trace("CalculateSizeOfAddedObjects: Error during ParseInt on string: '%s'", objectSizeStr) - return totalSize - } - totalSize += objectSize +// CalculateSizeOfObject calculates the size of one git object via git cat-file -s command +func CalculateSizeOfObject(ctx *gitea_context.PrivateContext, opts *git.RunOpts, objectID string) (objectSize int64) { + objectSizeStr, _, err := git.NewCommand(ctx, "cat-file", "-s").AddDynamicArguments(objectID).RunStdString(opts) + if err != nil { + log.Trace("CalculateSizeOfRemovedObjects: Error during git cat-file -s on object: %s", objectID) + return } - return totalSize + + objectSize, _ = strconv.ParseInt(strings.TrimSpace(objectSizeStr), 10, 64) + if err != nil { + log.Trace("CalculateSizeOfRemovedObjects: Error during ParseInt on string '%s'", objectID) + return + } + return } -// CalculateSizeOfRemovedObjects calculates the size of removed objects provided as output from rev-list command -// and confirms that the object is not referenced anywhere -func CalculateSizeOfRemovedObjects(ctx *gitea_context.PrivateContext, opts *git.RunOpts, revlistObjects string) int64 { - var totalSize int64 - for _, object := range strings.Split(revlistObjects, "\n") { +// CalculateSizeOfObjects calculates the size of objects added and removed from the repository by new commit +func CalculateSizeOfObjects(ctx *gitea_context.PrivateContext, opts *git.RunOpts, newCommitObjects map[string]bool, oldCommitObjects map[string]bool, otherCommitObjects map[string]bool) (addedSize int64, removedSize int64) { + + // Calculate size of objects that were added + for objectID := range newCommitObjects { + if _, exists := oldCommitObjects[objectID]; !exists { + // objectID is not referenced in the list of objects of old commit so it is a new object + // Calculate its size and add it to the addedSize + addedSize += CalculateSizeOfObject(ctx, opts, objectID) + } + // We might check here if new object is not already in the rest of repo to be precise + // However our goal is to prevent growth of repository so on determination of addedSize + // We can skip this preciseness, addedSize will be more then real addedSize + // TODO - do not count size of object that is referenced in other part of repo but not referenced neither in old nor new commit + // git will not add the object twice + } + + // Calculate size of objects that were removed + for objectID := range oldCommitObjects { + if _, exists := newCommitObjects[objectID]; !exists { + // objectID is not referenced in the list of new commit objects so it was possibly removed + if _, exists := otherCommitObjects[objectID]; !exists { + // objectID is not referenced in rest of the objects of the repository so it was removed + // Calculate its size and add it to the addedSize + removedSize += CalculateSizeOfObject(ctx, opts, objectID) + } + } + + } + return +} + +// ConvertObjectsToMap takes a newline-separated string of git objects and +// converts it into a map for efficient lookup. +func ConvertObjectsToMap(objects string) map[string]bool { + objectsMap := make(map[string]bool) + for _, object := range strings.Split(objects, "\n") { if len(object) == 0 { continue } objectID := strings.Split(object, " ")[0] - - // Confirm that the object is still reachable from anywhere in the repository. - isReachable, _, err := git.NewCommand(ctx, "rev-list", "--objects", "--all", "--").AddDynamicArguments(objectID).RunStdString(opts) - if err != nil { - log.Trace("CalculateSizeOfRemovedObjects: Error during git rev-list --objects --all on object: %s", objectID) - return totalSize - } - - if isReachable != "" { - // The object is still reachable, therefore we don't add it's size - continue - } - - objectSizeStr, _, err := git.NewCommand(ctx, "cat-file", "-s").AddDynamicArguments(objectID).RunStdString(opts) - if err != nil { - log.Trace("CalculateSizeOfRemovedObjects: Error during git cat-file -s on object: %s", objectID) - return totalSize - } - - objectSize, _ := strconv.ParseInt(strings.TrimSpace(objectSizeStr), 10, 64) - if err != nil { - log.Trace("CalculateSizeOfRemovedObjects: Error during ParseInt on string '%s'", objectID) - return totalSize - } - totalSize += objectSize + objectsMap[objectID] = true } - return totalSize + return objectsMap } // HookPreReceive checks whether a individual commit is acceptable func HookPreReceive(ctx *gitea_context.PrivateContext) { + startTime := time.Now() + opts := web.GetForm(ctx).(*private.HookOptions) ourCtx := &preReceiveContext{ @@ -178,8 +181,8 @@ func HookPreReceive(ctx *gitea_context.PrivateContext) { repo := ourCtx.Repo.Repository - var removedSize int64 var addedSize int64 + var removedSize int64 // Calculating total size of the push using git count-objects pushSize, err := git.CountObjectsWithEnv(ctx, repo.RepoPath(), ourCtx.env) @@ -191,7 +194,7 @@ func HookPreReceive(ctx *gitea_context.PrivateContext) { return } - // Cash whether the repository would breach the size limit after the operation + // Cache whether the repository would breach the size limit after the operation isRepoOversized := repo.RepoSizeIsOversized(pushSize.Size) log.Trace("Push size %d", pushSize.Size) @@ -203,28 +206,41 @@ func HookPreReceive(ctx *gitea_context.PrivateContext) { // If operation is in potential breach of size limit prepare data for analysis if isRepoOversized { - - // Objects that are in newCommitID but not in oldCommitID are added - addedObjects, _, err := git.NewCommand(ctx, "rev-list", "--objects").AddDynamicArguments(newCommitID, "^"+oldCommitID).RunStdString(&git.RunOpts{Dir: repo.RepoPath(), Env: ourCtx.env}) + // Create cache of objects in old commit + gitObjects, _, err := git.NewCommand(ctx, "rev-list", "--objects").AddDynamicArguments(oldCommitID).RunStdString(&git.RunOpts{Dir: repo.RepoPath(), Env: ourCtx.env}) if err != nil { - log.Error("Unable to list objects in %s and not in %s in %-v Error: %v", newCommitID, oldCommitID, repo, err) + log.Error("Unable to list objects in old commit: %s in %-v Error: %v", oldCommitID, repo, err) ctx.JSON(http.StatusInternalServerError, private.Response{ - Err: fmt.Sprintf("Fail to list objects added: %v", err), + Err: fmt.Sprintf("Fail to list objects in old commit: %v", err), }) return } - addedSize += CalculateSizeOfAddedObjects(ctx, &git.RunOpts{Dir: repo.RepoPath(), Env: ourCtx.env}, addedObjects) + oldCommitObjects := ConvertObjectsToMap(gitObjects) - // Objects that are in oldCommitID but not in newCommitID are removed - removedObjects, _, err := git.NewCommand(ctx, "rev-list", "--objects").AddDynamicArguments(oldCommitID, "^"+newCommitID).RunStdString(&git.RunOpts{Dir: repo.RepoPath(), Env: ourCtx.env}) + // Create cache of objects in new commit + gitObjects, _, err = git.NewCommand(ctx, "rev-list", "--objects").AddDynamicArguments(newCommitID).RunStdString(&git.RunOpts{Dir: repo.RepoPath(), Env: ourCtx.env}) if err != nil { - log.Error("Unable to list objects in %s and not in %s in %-v Error: %v", oldCommitID, newCommitID, repo, err) + log.Error("Unable to list objects in new commit %s in %-v Error: %v", newCommitID, repo, err) ctx.JSON(http.StatusInternalServerError, private.Response{ - Err: fmt.Sprintf("Fail to list objects removed: %v", err), + Err: fmt.Sprintf("Fail to list objects in new commit: %v", err), }) return } - removedSize += CalculateSizeOfRemovedObjects(ctx, &git.RunOpts{Dir: repo.RepoPath(), Env: ourCtx.env}, removedObjects) + newCommitObjects := ConvertObjectsToMap(gitObjects) + + // Create cache of objects that are in the repository but not part of old or new commit + gitObjects, _, err = git.NewCommand(ctx, "rev-list", "--objects", "--all").AddDynamicArguments("^"+oldCommitID, "^"+newCommitID).RunStdString(&git.RunOpts{Dir: repo.RepoPath(), Env: ourCtx.env}) + if err != nil { + log.Error("Unable to list objects in the repo that are missing from both old %s and new %s commits in %-v Error: %v", oldCommitID, newCommitID, repo, err) + ctx.JSON(http.StatusInternalServerError, private.Response{ + Err: fmt.Sprintf("Fail to list objects missing from both old and new commits: %v", err), + }) + return + } + otherCommitObjects := ConvertObjectsToMap(gitObjects) + + // Calculate size that was added and removed by the new commit + addedSize, removedSize = CalculateSizeOfObjects(ctx, &git.RunOpts{Dir: repo.RepoPath(), Env: ourCtx.env}, newCommitObjects, oldCommitObjects, otherCommitObjects) } switch { @@ -242,9 +258,12 @@ func HookPreReceive(ctx *gitea_context.PrivateContext) { } } + duration := time.Since(startTime) + log.Warn("Addition in size is: %d, removal in size is: %d, limit size: %s, push size: %d. Took %s seconds.", addedSize, removedSize, base.FileSize(repo.GetActualSizeLimit()), pushSize.Size, duration) + // If total of commits add more size then they remove and we are in a potential breach of size limit -- abort - if (addedSize > removedSize) && isRepoOversized { // Check next size if we are not deleting a reference - log.Warn("Forbidden: new repo size is over limitation: %s", base.FileSize(repo.GetActualSizeLimit())) + if (addedSize > removedSize) && isRepoOversized { + log.Warn("Forbidden: new repo size %s is over limitation of %s. Push size: %s. Took %s seconds.", base.FileSize(addedSize-removedSize), base.FileSize(repo.GetActualSizeLimit()), base.FileSize(pushSize.Size), duration) ctx.JSON(http.StatusForbidden, private.Response{ UserMsg: fmt.Sprintf("Repository size is over limitation of %s", base.FileSize(repo.GetActualSizeLimit())), }) From 08c65ffb1725d9fad8421705245270f8ceb87a4e Mon Sep 17 00:00:00 2001 From: truecode112 Date: Tue, 23 May 2023 08:18:41 +0300 Subject: [PATCH 2/2] Update for TestScript to Reduce RepoSize --- modules/repository/create.go | 20 ++++++++ routers/private/hook_pre_receive.go | 6 ++- services/repository/push.go | 10 ++-- tests/integration/git_test.go | 79 ++++++++++++++++++++++++++++- 4 files changed, 109 insertions(+), 6 deletions(-) diff --git a/modules/repository/create.go b/modules/repository/create.go index 63be5cac25..8240600be1 100644 --- a/modules/repository/create.go +++ b/modules/repository/create.go @@ -332,6 +332,26 @@ func UpdateRepoSize(ctx context.Context, repo *repo_model.Repository) error { return fmt.Errorf("updateSize: GetLFSMetaObjects: %w", err) } + if setting.EnableSizeLimit && repo.GetActualSizeLimit() > 0 && size+lfsSize > repo.GetActualSizeLimit() { + // return fmt.Errorf("updateSize: Git Reflog Failed Size(%d) > Limit(%d)", size+lfsSize, repo.GetActualSizeLimit()) + + _, _, err = git.NewCommand(git.DefaultContext, "reflog", "expire", "--expire-unreachable=all", "--all").RunStdString(&git.RunOpts{Dir: repo.RepoPath()}) // Push + if err != nil { + return fmt.Errorf("updateSize: Git Reflog Failed: %w", err) + } + + _, _, err = git.NewCommand(git.DefaultContext, "gc", "--prune=now").RunStdString(&git.RunOpts{Dir: repo.RepoPath()}) // Push + if err != nil { + return fmt.Errorf("updateSize: Git GC Failed: %w", err) + } + + size, err = getDirectorySize(repo.RepoPath()) + if err != nil { + return fmt.Errorf("updateSize: %w", err) + } + // return fmt.Errorf("updateSize: Git Reflog Failed Size(%d) > Limit(%d)", size+lfsSize, repo.GetActualSizeLimit()) + } + return repo_model.UpdateRepoSize(ctx, repo.ID, size+lfsSize) } diff --git a/routers/private/hook_pre_receive.go b/routers/private/hook_pre_receive.go index 3813ce66b4..a4e1ad94f6 100644 --- a/routers/private/hook_pre_receive.go +++ b/routers/private/hook_pre_receive.go @@ -204,6 +204,10 @@ func HookPreReceive(ctx *gitea_context.PrivateContext) { // If operation is in potential breach of size limit prepare data for analysis if isRepoOversized { + // ctx.JSON(http.StatusForbidden, private.Response{ + // UserMsg: fmt.Sprintf("oldCommitID(%s) newCommitID(%s)", oldCommitID, newCommitID), + // }) + // Objects that are in newCommitID but not in oldCommitID are added addedObjects, _, err := git.NewCommand(ctx, "rev-list", "--objects").AddDynamicArguments(newCommitID, "^"+oldCommitID).RunStdString(&git.RunOpts{Dir: repo.RepoPath(), Env: ourCtx.env}) if err != nil { @@ -246,7 +250,7 @@ func HookPreReceive(ctx *gitea_context.PrivateContext) { if (addedSize > removedSize) && isRepoOversized { // Check next size if we are not deleting a reference log.Warn("Forbidden: new repo size is over limitation: %s", base.FileSize(repo.GetActualSizeLimit())) ctx.JSON(http.StatusForbidden, private.Response{ - UserMsg: fmt.Sprintf("Repository size is over limitation of %s", base.FileSize(repo.GetActualSizeLimit())), + UserMsg: fmt.Sprintf("Repository size is over limitation of %s addedSize(%d) removedSize(%d) repo(%s)", base.FileSize(repo.GetActualSizeLimit()), addedSize, removedSize, repo.RepoPath()), }) return } diff --git a/services/repository/push.go b/services/repository/push.go index c7ea8f336e..a6ba284bc7 100644 --- a/services/repository/push.go +++ b/services/repository/push.go @@ -93,9 +93,9 @@ func pushUpdates(optsList []*repo_module.PushUpdateOptions) error { } defer gitRepo.Close() - if err = repo_module.UpdateRepoSize(ctx, repo); err != nil { - log.Error("Failed to update size for repository: %v", err) - } + // if err = repo_module.UpdateRepoSize(ctx, repo); err != nil { + // log.Error("Failed to update size for repository: %v", err) + // } addTags := make([]string, 0, len(optsList)) delTags := make([]string, 0, len(optsList)) @@ -293,6 +293,10 @@ func pushUpdates(optsList []*repo_module.PushUpdateOptions) error { return fmt.Errorf("UpdateRepositoryUpdatedTime: %w", err) } + if err = repo_module.UpdateRepoSize(ctx, repo); err != nil { + log.Error("Failed to update size for repository: %v", err) + } + return nil } diff --git a/tests/integration/git_test.go b/tests/integration/git_test.go index cdff57f21f..d2bb383a71 100644 --- a/tests/integration/git_test.go +++ b/tests/integration/git_test.go @@ -35,7 +35,7 @@ import ( ) const ( - littleSize = 1024 // 1ko + littleSize = 1024 * 10 // 1ko bigSize = 128 * 1024 * 1024 // 128Mo ) @@ -60,7 +60,8 @@ func testGit(t *testing.T, u *url.URL) { dstPath := t.TempDir() - dstForkedPath := t.TempDir() + // dstForkedPath := t.TempDir() + dstForkedPath4Reduce := t.TempDir() t.Run("CreateRepoInDifferentUser", doAPICreateRepository(forkedUserCtx, false)) t.Run("AddUserAsCollaborator", doAPIAddCollaborator(forkedUserCtx, httpContext.Username, perm.AccessModeRead)) @@ -104,6 +105,38 @@ func testGit(t *testing.T, u *url.URL) { defer tests.PrintCurrentTest(t)() // TODO doDeleteCommitAndPush(t, littleSize, dstPath, "data-file-") }) + + t.Run("ReduceRepoSize", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + u.Path = forkedUserCtx.GitPath() + u.User = url.UserPassword(forkedUserCtx.Username, userPassword) + + t.Run("Clone", doGitClone(dstForkedPath4Reduce, u)) + fmt.Fprintf(os.Stdout, "dstForkedPath4Reduce = %s\n", dstForkedPath4Reduce) + doCommitAndPush(t, littleSize, dstForkedPath4Reduce, "data-file-") + bigOne := doCommitAndPush(t, bigSize, dstForkedPath4Reduce, "my-file-") + repo, err := repo_model.GetRepositoryByOwnerAndName(db.DefaultContext, forkedUserCtx.Username, forkedUserCtx.Reponame) + assert.NoError(t, err) + orgGitRepoSize := repo.Size //doGetCountObjects(t, dstForkedPath4Reduce) + fmt.Fprintf(os.Stdout, "Original GitSize = %d\n", orgGitRepoSize) + + t.Run("APISetRepoSizeLimit", doAPISetRepoSizeLimit(forkedUserCtx, forkedUserCtx.Username, forkedUserCtx.Reponame, bigSize/2)) + fmt.Fprintf(os.Stdout, "Original GitSize = %d\n", orgGitRepoSize) + + //Delete Object from Oversized Repository, This should be Accepdted + doDeleteObjectAndClean(t, dstForkedPath4Reduce, bigOne) + + repo, err = repo_model.GetRepositoryByOwnerAndName(db.DefaultContext, forkedUserCtx.Username, forkedUserCtx.Reponame) + assert.NoError(t, err) + reducedGitRepoSize := repo.Size //doGetCountObjects(t, dstForkedPath4Reduce) + + fmt.Fprintf(os.Stdout, "Reduced GitSize = %d\n", reducedGitRepoSize) + + assert.Less(t, reducedGitRepoSize, orgGitRepoSize, "Repo size is not reduced.") + + setting.SaveGlobalRepositorySetting(false, 0) + }) + // TODO delete branch // TODO delete tag // TODO add big commit that will be over with the push @@ -325,6 +358,20 @@ func lockFileTest(t *testing.T, filename, repoPath string) { assert.NoError(t, err) } +func doDeleteObjectAndClean(t *testing.T, repoPath, filename string) { + var err error + err = deleteCommit(repoPath, "user4@example.com", "User Four", filename) + assert.NoError(t, err) + _, _, err = git.NewCommand(git.DefaultContext, "push", "origin", "master").RunStdString(&git.RunOpts{Dir: repoPath}) // Push + assert.NoError(t, err) + + // _, _, err = git.NewCommand(git.DefaultContext, "reflog", "expire", "--expire-unreachable=all", "--all").RunStdString(&git.RunOpts{Dir: repoPath}) // Push + // assert.NoError(t, err) + // _, _, err = git.NewCommand(git.DefaultContext, "gc", "--prune=now").RunStdString(&git.RunOpts{Dir: repoPath}) // Push + // assert.NoError(t, err) + +} + func doCommitAndPush(t *testing.T, size int, repoPath, prefix string) string { name, err := generateCommitWithNewData(size, repoPath, "user2@example.com", "User Two", prefix) assert.NoError(t, err) @@ -341,6 +388,34 @@ func doCommitAndPushWithExpectedError(t *testing.T, size int, repoPath, prefix s return name } +func deleteCommit(repoPath, email, fullName, filename string) error { + + var err error + globalArgs := git.AllowLFSFiltersArgs() + + cmd := git.NewCommand(git.DefaultContext, "rm").AddDashesAndList(filename) + + _, _, err = cmd.RunStdString(&git.RunOpts{Dir: repoPath}) // Push + if err != nil { + return err + } + + return git.CommitChangesWithArgs(repoPath, globalArgs, git.CommitChangesOptions{ + Committer: &git.Signature{ + Email: email, + Name: fullName, + When: time.Now(), + }, + Author: &git.Signature{ + Email: email, + Name: fullName, + When: time.Now(), + }, + Message: fmt.Sprintf("Testing commit @ %v", time.Now()), + }) + +} + func generateCommitWithNewData(size int, repoPath, email, fullName, prefix string) (string, error) { // Generate random file bufSize := 4 * 1024