-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support LFS purely over SSH protocol #17554
Comments
Hm, git-lfs has supported SSH protocol I think. ref: git-lfs/git-lfs#4446 |
good to know, but how to use it? looks Gitea hasn't support it yet:
|
@ibigbug Could you change the title to support a pure ssh lfs protocol something like that? Yes. Since that PR was merged recently, Gitea itself didn't support that at the moment. But it should fallback to use http/https protocol. |
thanks @lunny keen to see Gitea can make LFS over SSH work. |
@lunny any updates? |
Nobody are working on this issue. |
@lunny thanks for letting me know. How to get someone to take a look at this? |
In fact, it is not that difficult to implement git-lfs-transfer. We can write such a command based on rust/golang to support the git lfs pure ssh protocol. bk2204 implements a great example: Inside our company, I have used golang to simulate the git-lfs-transfer command in the ssh server from this project, and it is currently running stably. |
This feature would be great! 👍 |
@fcharlie did you end up using this project internally and did it work as expected or were there some complications? |
Since the prune ssh protocol does not support OSS signature download, and AI needs to download a large number of large files, we disabled it for performance reasons. Here is also an implementation reference for the SSH protocol: |
💎 $300 bounty • CommitGo, Inc.Steps to solve:
Thank you for contributing to go-gitea/gitea! Add a bounty • Share on socials
|
^ that was me that created this bounty (trialing new bounty platform) |
/attempt #17554 @techknowlogick, I've started look into this.
|
/attempt #17554 Options |
Hi, is this still being actively worked on or is it up for grabs? Also just to be clear, integrating charm.sh's (pretty good looking) https://github.com/charmbracelet/git-lfs-transfer implementation into Gitea successfully such that all LFS functionality starts working over SSH would be an acceptable fix, correct? That is, we don't need a new, in-house implementation of the protocol? |
@ConcurrentCrab, I've not been able to get far on this task so it is indeed up for grabs. |
Thank you for the confirmation, @jemiluv8 I'm interested in trying my hand at this, so it'd be great if @techknowlogick or another project member could confirm the scope of the task, in regards to my above question. |
/attempt #17554 Options |
Huh, this was more straightforward than I expected it to be. Very rough, needs cleanups:
For the question I posed above, I assumed a dependency on an external implementation was fine (please confirm?), so all it requires is to have an implementation of To the best of my knowledge, this preserves the security model, since the How do I know it works? if you ran
where a Now:
|
Hi @ConcurrentCrab this is indeed available, and we'd love for you to work on it. Thanks for submitting a WIP PR. In terms of scope, it's "users can interact with LFS using SSH (using either the built-in ssh server, or the integration with opensshd)". Adding the external dep on charm.sh is a-ok. |
Hi @techknowlogick,
Thanks for clarifying that. To be crystal clear, this is not a build-time dependency, as in pulling in their libraries. This is a run-time dependency on the binary being present in the environment (any compliant implementation would do), just like we're already depending on the git package being installed for the As I mentioned, the patch in the PR already makes pure SSH LFS sessions work, according to my experiments I outlined above. But this is very much the minimum-changes-required version of this patch, conceivably can be termed a 'hack'. I think it'd be worth making some refactors to the logic in that handler. There's already quite a bit of convoluted logic in there, so I'd feel bad leaving it there with even more surprise ifs-and-elses :) And I'll add all the relevant useful information into the commits, as you suggested. Regarding the security model I think we're mostly fine, except at one point it does seem to care about the "verb": Line 140 in 621e1ff
That line seems to be from this commit fixing a bug related to the upload command: 95013fd That seems important. I'll be looking more into what exactly that might mean, as the "upload" mode of our new command should probably be on that list of exceptions too. |
Whoops, called it too early ;). The network transfer indeed is working, but the binary isn't placing the objects where Gitea expects them to be. Huh. Going to look into that. |
Ah. It seems both Charm and Scutiger store the LFS assets in the |
Gitea stores all LFS blobs in same directory so that same files in multiple repositories could reuse them (especially important for forking), also need to keep in mind that Gitea stores all LFS references to repositories that specific blob is used in database. LFS storage can be not only filesystem but also S3 |
Hi @lafriks, Thanks for sharing this information.
Ah, that sounds reasonable.
I see. That does significantly complicate things. This certainly doesn't sound like something that could be done by an out-of-process client anymore, not the db stuff and definitely not the S3 stuff. The git protocols take special care to avoid stuff like that (mostly by having a single source of truth), so that multiple protocols like SSH, "dumb" HTTP, "smart" HTTP, etc. can worth together without a hitch. And that repos on the server are just normal bare repos, so that representations are symmetric on both server and client. This sort of makes that moot. So how would you suggest approaching this? I would think the simplest way would be throwing away the out-of-process client, and implementing an in-process reader/writer that calls the same functions the HTTP LFS API does. But not being aware of the architecture of the program, I'm open to suggestions. |
I think that in-process and internal API to reuse HTTP logic would be the way to go imho |
Alright, mostly done with the rote work. What I ended up doing was vendoring the transport package from the charm.sh library (earlier I tried just importing it but the certain differences between the file-based and Gitea API made that unfeasible), modifying it a bit, and adding a "backend" that proxies to the Gitea internal store. Just need to figure out why transfers are throwing 500s :) |
Aaaand that should be it. Pushes and clones all seem to work properly, and metadata objects are registered like they should be. I think support for Pure SSH LFS is complete :) Marking draft PR as final now. |
Continued in #31516 I guess I had to resubmit with a new PR for the algora bot to see. |
Finally get everything working, and a I think the PR is largely complete, but I would... suggest holding off on merging it until a fixed version of |
You can test it but obvs throw a fixed build of either that or set |
Hi @techknowlogick, It'd be nice to have some kind of a response to the PR, even if that response is "hi, no, haven't gotten to it yet but will soon". It doesn't really feel nice to be ghosted for a month after having done all this work (up to and including fixing bugs in the LFS client) :) |
Hi, @ConcurrentCrab. I'm so sorry for missing the notifications and leaving you hanging. I just tested a build of your PR, and ran into the long hang from git-lfs that you described. I don't think that's a blocker to get this merged, but perhaps LFS over SSH support could be disabled by default in the config, and then in documentation I could add something around disabling automultiplex prior to enabling it? |
🎉🎈 @ConcurrentCrab has been awarded $360! 🎈🎊 |
Feature Description
The current LFS related operations only using
settings.AppURL
as the endpoint:gitea/services/lfs/server.go
Line 48 in bc6df18
One scenario is I want to have server.ROOT_URL(
settings.AppURL
) to be external faced URL to browse the site.And I want to have
server. SSH_DOMAIN
to be my internal domain to clone and push code.But the current implementation only looks at
AppURL
(ROOT_URL
), shouldSSH_DOMAIN
be considered for LFS operations?Screenshots
No response
The text was updated successfully, but these errors were encountered: