Skip to content

Repository clones are not cleaned up individually or reliably #1386

@kriswest

Description

@kriswest

Describe the bug
As discussed in another issue (see #1384 (comment) and #1384 (comment)), repository clones are not cleaned up individually (the entire .remote folder is deleted in clearBareClone) or reliably (the clone is not cleared up at all if a processor before clearBareClone produces an error, the proxy being stopped, restarted or crashing can produce the same result). This can result in a push being permanently blocked by the error, e.g.:

$ git push upstream-dev-finos
Enumerating objects: 15, done.
Counting objects: 100% (15/15), done.
Delta compression using up to 4 threads
Compressing objects: 100% (8/8), done.
Writing objects: 100% (9/9), 879 bytes | 175.00 KiB/s, done.
Total 9 (delta 7), reused 3 (delta 1), pack-reused 0 (from 0)
remote:         An error occurred when executing the chain: CheckoutConflictError: Your local changes to the following files would be overwritten by checkout: sr
/components/Navbars/DashboardNavbarLinks.tsx, src/ui/views/User/UserProfile.tsx
fatal: the remote end hung up unexpectedly
fatal: the remote end hung up unexpectedly
error: failed to push some refs to 'https://*****/github.com/finos/git-proxy.git'

Requiring manual intervention (delete the .remote folder) to unpick.

Further:

  • multiple concurrent requests relating to the same push can conflict with each other by attempting to use the same folder (whose name is set via action.id, which is set based on the commit SHA).
  • multiple concurrent requests relating to the different pushes can conflict with each other as, due to async functions used to handle them, they can be processed concurrently and clearBareClone deletes the whole .remote folder rather than just the one relating to that particular push.

To Reproduce

  1. Failing to clean-up on errors:
    @fabiovincenzi reports:

    if any step between pullRemote and clearBareClone fails, cleanup never happens. I tested this with gitleaks blocking a push .remote remains on disk with a full clone.

  2. Failing to clean-up on shutdown/restart:

    • Push a change to a large repository
    • Stop the proxy while it is processing, after it has cloned the repository (easier to do if its a large change...)_
    • Confirm that the .remote/* folder for the push exists
    • Check that local changes have been applied in order to calculate the diff - or apply a local change of your own
    • Restart the proxy
    • Push the change again - it should fail as shown above (refusing to overwrite local changes).

Expected behaviour

  • The .remote folder is cleared up on startup (in case it was left in a poor state on restart
  • .remote/* checkout folders are cleared up reliably on errors and after commits are successfully processed
  • .remote/* folders are cleaned up individually, so that multiple concurrent requests for different pushes do not conflict with each other
  • Multiple concurrent requests relating to the same repository/SHAs should should be handled such that the first request succeeds and subsequent requests should fail until the first has finished processing.

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions